Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions examples/app/templates/batch-job.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ spec:
backoffLimit: {{ .Values.batchJob.backoffLimit }}
template:
spec:
affinity: {{- toYaml .Values.batchJob.affinity | nindent 8 }}
containers:
- command:
- perl
Expand Down
1 change: 1 addition & 0 deletions examples/app/templates/cron-job.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ spec:
spec:
template:
spec:
affinity: {{- toYaml .Values.cronJob.affinity | nindent 12 }}
containers:
- command:
- /bin/sh
Expand Down
1 change: 1 addition & 0 deletions examples/app/templates/daemonset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ spec:
name: fluentd-elasticsearch
{{- include "app.selectorLabels" . | nindent 8 }}
spec:
affinity: {{- toYaml .Values.fluentdElasticsearch.affinity | nindent 8 }}
containers:
- env:
- name: KUBERNETES_CLUSTER_DOMAIN
Expand Down
1 change: 1 addition & 0 deletions examples/app/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ spec:
app: myapp
{{- include "app.selectorLabels" . | nindent 8 }}
spec:
affinity: {{- toYaml .Values.myapp.affinity | nindent 8 }}
containers:
- args: {{- toYaml .Values.myapp.app.args | nindent 8 }}
command:
Expand Down
1 change: 1 addition & 0 deletions examples/app/templates/statefulset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ spec:
labels:
app: nginx
spec:
affinity: {{- toYaml .Values.web.affinity | nindent 8 }}
containers:
- env:
- name: KUBERNETES_CLUSTER_DOMAIN
Expand Down
11 changes: 11 additions & 0 deletions examples/app/values.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
batchJob:
affinity: {}
backoffLimit: 4
nodeSelector: {}
pi:
Expand All @@ -8,6 +9,7 @@ batchJob:
tolerations: []
topologySpreadConstraints: []
cronJob:
affinity: {}
hello:
image:
repository: busybox
Expand All @@ -18,6 +20,7 @@ cronJob:
tolerations: []
topologySpreadConstraints: []
fluentdElasticsearch:
affinity: {}
fluentdElasticsearch:
image:
repository: quay.io/fluentd_elasticsearch/fluentd
Expand Down Expand Up @@ -64,6 +67,13 @@ mySecretVars:
var1: ""
var2: ""
myapp:
affinity:
nodeAffinity:
requiredDuringSchedulingIgnoredDuringExecution:
nodeSelectorTerms:
- matchExpressions:
- key: node-role.kubernetes.io/control-plane
operator: Exists
app:
args:
- --health-probe-bind-address=:8081
Expand Down Expand Up @@ -140,6 +150,7 @@ pvc:
storageLimit: 5Gi
storageRequest: 3Gi
web:
affinity: {}
nginx:
image:
repository: registry.k8s.io/nginx-slim
Expand Down
21 changes: 21 additions & 0 deletions pkg/processor/pod/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,27 @@ func ProcessSpec(objName string, appMeta helmify.AppMetadata, spec corev1.PodSpe
}
}

// process affinity if presented:
err = unstructured.SetNestedField(specMap, fmt.Sprintf(`{{- toYaml .Values.%s.affinity | nindent %d }}`, objName, nindent), "affinity")
if err != nil {
return nil, nil, err
}
if spec.Affinity != nil {
affinityMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(spec.Affinity)
if err != nil {
return nil, nil, err
}
err = unstructured.SetNestedField(values, affinityMap, objName, "affinity")
if err != nil {
return nil, nil, err
}
} else {
err = unstructured.SetNestedField(values, map[string]interface{}{}, objName, "affinity")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure that we should set affinity if it was not presented in source manifest. what is the use-case for it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Affinity is more advanced nodeSelector, i followed its implementation

https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#affinity-and-anti-affinity

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

understood. but here we always add affinity to values.yaml even if affinity value was not presented in source manifest. I dont think that it should be templated if it was not presented in source.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you are suggesting that helmify creators should add affinity: {} to allow setting up affinity?

In my opinion it is useful for this to always be exposed as its always benefitial for installers of such helm to have affinity as field they can setup.

If you want i can change it that it requires atleast affinity: {} for some exposure, but i think this way is "better"

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are many fields in deployment spec and not all of them are templated and moved to values.yaml by default.

if helmify is templating only presented values, then user has control to output. User can add/delete affinity: {} in source to have or not have it in resulted chart.

if err != nil {
return nil, nil, err
}
}

// process tolerations if presented:
err = unstructured.SetNestedField(specMap, fmt.Sprintf(`{{- toYaml .Values.%s.tolerations | nindent %d }}`, objName, nindent), "tolerations")
if err != nil {
Expand Down
95 changes: 95 additions & 0 deletions pkg/processor/pod/pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,35 @@ spec:
runAsNonRoot: true
runAsUser: 65532

`
strDeploymentWithAffinity = `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
labels:
app: nginx
spec:
replicas: 3
selector:
matchLabels:
app: nginx
template:
metadata:
labels:
app: nginx
spec:
containers:
- name: nginx
image: localhost:6001/my_project:latest
affinity:
nodeAffinity:
requiredDuringSchedulingIgnoredDuringExecution:
nodeSelectorTerms:
- matchExpressions:
- key: node-role.kubernetes.io/control-plane
operator: Exists

`
)

Expand Down Expand Up @@ -171,6 +200,7 @@ func Test_pod_Process(t *testing.T) {
"topologySpreadConstraints": "{{- toYaml .Values.nginx.topologySpreadConstraints | nindent 8 }}",
"nodeSelector": "{{- toYaml .Values.nginx.nodeSelector | nindent 8 }}",
"serviceAccountName": `{{ include ".serviceAccountName" . }}`,
"affinity": "{{- toYaml .Values.nginx.affinity | nindent 8 }}",
}, specMap)

assert.Equal(t, helmify.Values{
Expand All @@ -188,6 +218,7 @@ func Test_pod_Process(t *testing.T) {
"nodeSelector": map[string]interface{}{},
"tolerations": []interface{}{},
"topologySpreadConstraints": []interface{}{},
"affinity": map[string]interface{}{},
},
}, tmpl)
})
Expand Down Expand Up @@ -221,6 +252,7 @@ func Test_pod_Process(t *testing.T) {
"serviceAccountName": `{{ include ".serviceAccountName" . }}`,
"tolerations": "{{- toYaml .Values.nginx.tolerations | nindent 8 }}",
"topologySpreadConstraints": "{{- toYaml .Values.nginx.topologySpreadConstraints | nindent 8 }}",
"affinity": "{{- toYaml .Values.nginx.affinity | nindent 8 }}",
}, specMap)

assert.Equal(t, helmify.Values{
Expand All @@ -234,6 +266,7 @@ func Test_pod_Process(t *testing.T) {
"nodeSelector": map[string]interface{}{},
"tolerations": []interface{}{},
"topologySpreadConstraints": []interface{}{},
"affinity": map[string]interface{}{},
},
}, tmpl)
})
Expand Down Expand Up @@ -267,6 +300,7 @@ func Test_pod_Process(t *testing.T) {
"serviceAccountName": `{{ include ".serviceAccountName" . }}`,
"tolerations": "{{- toYaml .Values.nginx.tolerations | nindent 8 }}",
"topologySpreadConstraints": "{{- toYaml .Values.nginx.topologySpreadConstraints | nindent 8 }}",
"affinity": "{{- toYaml .Values.nginx.affinity | nindent 8 }}",
}, specMap)

assert.Equal(t, helmify.Values{
Expand All @@ -280,6 +314,7 @@ func Test_pod_Process(t *testing.T) {
"nodeSelector": map[string]interface{}{},
"tolerations": []interface{}{},
"topologySpreadConstraints": []interface{}{},
"affinity": map[string]interface{}{},
},
}, tmpl)
})
Expand Down Expand Up @@ -313,6 +348,7 @@ func Test_pod_Process(t *testing.T) {
"serviceAccountName": `{{ include ".serviceAccountName" . }}`,
"tolerations": "{{- toYaml .Values.nginx.tolerations | nindent 8 }}",
"topologySpreadConstraints": "{{- toYaml .Values.nginx.topologySpreadConstraints | nindent 8 }}",
"affinity": "{{- toYaml .Values.nginx.affinity | nindent 8 }}",
}, specMap)

assert.Equal(t, helmify.Values{
Expand All @@ -326,6 +362,7 @@ func Test_pod_Process(t *testing.T) {
"nodeSelector": map[string]interface{}{},
"tolerations": []interface{}{},
"topologySpreadConstraints": []interface{}{},
"affinity": map[string]interface{}{},
},
}, tmpl)
})
Expand Down Expand Up @@ -354,6 +391,7 @@ func Test_pod_Process(t *testing.T) {
"serviceAccountName": `{{ include ".serviceAccountName" . }}`,
"tolerations": "{{- toYaml .Values.nginx.tolerations | nindent 8 }}",
"topologySpreadConstraints": "{{- toYaml .Values.nginx.topologySpreadConstraints | nindent 8 }}",
"affinity": "{{- toYaml .Values.nginx.affinity | nindent 8 }}",
}, specMap)

assert.Equal(t, helmify.Values{
Expand All @@ -373,8 +411,65 @@ func Test_pod_Process(t *testing.T) {
"nodeSelector": map[string]interface{}{},
"tolerations": []interface{}{},
"topologySpreadConstraints": []interface{}{},
"affinity": map[string]interface{}{},
},
}, tmpl)
})
t.Run("deployment with affinity", func(t *testing.T) {
var deploy appsv1.Deployment
obj := internal.GenerateObj(strDeploymentWithAffinity)
err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, &deploy)
specMap, tmpl, err := ProcessSpec("nginx", &metadata.Service{}, deploy.Spec.Template.Spec, 0)
assert.NoError(t, err)
assert.Equal(t, map[string]interface{}{
"containers": []interface{}{
map[string]interface{}{
"env": []interface{}{
map[string]interface{}{
"name": "KUBERNETES_CLUSTER_DOMAIN",
"value": "{{ quote .Values.kubernetesClusterDomain }}",
},
},
"image": "{{ .Values.nginx.nginx.image.repository }}:{{ .Values.nginx.nginx.image.tag | default .Chart.AppVersion }}",
"name": "nginx",
"resources": map[string]interface{}{},
},
},
"nodeSelector": "{{- toYaml .Values.nginx.nodeSelector | nindent 8 }}",
"serviceAccountName": `{{ include ".serviceAccountName" . }}`,
"tolerations": "{{- toYaml .Values.nginx.tolerations | nindent 8 }}",
"topologySpreadConstraints": "{{- toYaml .Values.nginx.topologySpreadConstraints | nindent 8 }}",
"affinity": "{{- toYaml .Values.nginx.affinity | nindent 8 }}",
}, specMap)

assert.Equal(t, helmify.Values{
"nginx": map[string]interface{}{
"affinity": map[string]interface{}{
"nodeAffinity": map[string]interface{}{
"requiredDuringSchedulingIgnoredDuringExecution": map[string]interface{}{
"nodeSelectorTerms": []interface{}{
map[string]interface{}{
"matchExpressions": []interface{}{
map[string]interface{}{
"key": "node-role.kubernetes.io/control-plane",
"operator": "Exists",
},
},
},
},
},
},
},
"nginx": map[string]interface{}{
"image": map[string]interface{}{
"repository": "localhost:6001/my_project",
"tag": "latest",
},
},
"nodeSelector": map[string]interface{}{},
"tolerations": []interface{}{},
"topologySpreadConstraints": []interface{}{},
},
}, tmpl)
})
}
9 changes: 8 additions & 1 deletion test_data/sample-app.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,13 @@ spec:
nodeSelector:
region: east
type: user-node
affinity:
nodeAffinity:
requiredDuringSchedulingIgnoredDuringExecution:
nodeSelectorTerms:
- matchExpressions:
- key: node-role.kubernetes.io/control-plane
operator: Exists
terminationGracePeriodSeconds: 10
volumes:
- configMap:
Expand Down Expand Up @@ -166,7 +173,7 @@ metadata:
spec:
ipFamilyPolicy: SingleStack
ipFamilies:
- IPv4
- IPv4
ports:
- name: https
port: 8443
Expand Down