From 39d2f29b956ee377b1fd258fb19cf17ea62d32a5 Mon Sep 17 00:00:00 2001 From: Damien Duportal Date: Mon, 2 Oct 2023 17:43:43 +0200 Subject: [PATCH 1/2] feat(mirrorbits-lite) allow overriding ingress and PV(C)s based on global value from parent chart Signed-off-by: Damien Duportal --- charts/httpd/Chart.yaml | 2 +- charts/httpd/templates/_helpers.tpl | 18 +++ charts/httpd/templates/deployment.yaml | 16 +-- charts/httpd/templates/ingress.yaml | 2 +- charts/httpd/templates/mocks/_helpers.tpl | 3 + charts/httpd/templates/persistentVolume.yaml | 2 +- .../templates/persistentVolumeClaim.yaml | 2 +- charts/httpd/tests/custom_values_test.yaml | 114 ++++++++++-------- charts/httpd/tests/defaults_test.yaml | 77 ++++++------ charts/httpd/tests/parent_values_test.yaml | 95 +++++++++++++++ 10 files changed, 228 insertions(+), 103 deletions(-) create mode 100644 charts/httpd/templates/mocks/_helpers.tpl create mode 100644 charts/httpd/tests/parent_values_test.yaml diff --git a/charts/httpd/Chart.yaml b/charts/httpd/Chart.yaml index 374012e95..465a4641c 100644 --- a/charts/httpd/Chart.yaml +++ b/charts/httpd/Chart.yaml @@ -1,4 +1,4 @@ apiVersion: v1 description: httpd helm chart for Kubernetes name: httpd -version: 0.1.0 +version: 0.2.0 diff --git a/charts/httpd/templates/_helpers.tpl b/charts/httpd/templates/_helpers.tpl index 5975c1ec2..bae9e3421 100644 --- a/charts/httpd/templates/_helpers.tpl +++ b/charts/httpd/templates/_helpers.tpl @@ -43,3 +43,21 @@ app.kubernetes.io/version: {{ .Chart.AppVersion | quote }} {{- end }} app.kubernetes.io/managed-by: {{ .Release.Service }} {{- end -}} + +{{/* +Data directory volume definition. Might be defined from parent chart templates or autonomously +based on the presence of the global value provided by the parent chart. +*/}} +{{- define "httpd.html-volume" -}} +{{- if (dig "global" "storage" "enabled" false .Values.AsMap) -}} +persistentVolumeClaim: + claimName: {{ include "mirrorbits-parent.pvc-name" . }} +{{- else }} + {{- if or .Values.repository.persistentVolumeClaim.enabled .Values.repository.reuseExistingPersistentVolumeClaim }} +persistentVolumeClaim: + claimName: {{ .Values.repository.name | default (printf "%s-binary" (include "httpd.fullname" .)) }} + {{- else }} +emptyDir: {} + {{- end -}} +{{- end -}} +{{- end -}} diff --git a/charts/httpd/templates/deployment.yaml b/charts/httpd/templates/deployment.yaml index bac836126..f3959ada8 100644 --- a/charts/httpd/templates/deployment.yaml +++ b/charts/httpd/templates/deployment.yaml @@ -1,3 +1,4 @@ +--- apiVersion: apps/v1 kind: Deployment metadata: @@ -38,15 +39,13 @@ spec: resources: {{- toYaml .Values.resources | nindent 12 }} volumeMounts: - {{ if or .Values.repository.persistentVolumeClaim.enabled .Values.repository.reuseExistingPersistentVolumeClaim -}} - - name: binary - mountPath: /usr/local/apache2/htdocs - readOnly: true - {{- end }} - name: conf mountPath: /usr/local/apache2/conf/httpd.conf subPath: httpd.conf readOnly: true + - name: html + mountPath: /usr/local/apache2/htdocs + readOnly: true {{- with .Values.nodeSelector }} nodeSelector: {{- toYaml . | nindent 8 }} @@ -63,8 +62,5 @@ spec: - name: conf configMap: name: {{ include "httpd.fullname" . }} - {{ if or .Values.repository.persistentVolumeClaim.enabled .Values.repository.reuseExistingPersistentVolumeClaim -}} - - name: binary - persistentVolumeClaim: - claimName: {{ .Values.repository.name | default (printf "%s-binary" (include "httpd.fullname" .)) }} - {{- end }} + - name: html + {{- include "httpd.html-volume" . | nindent 10}} diff --git a/charts/httpd/templates/ingress.yaml b/charts/httpd/templates/ingress.yaml index 5f7e5ead1..40c683187 100644 --- a/charts/httpd/templates/ingress.yaml +++ b/charts/httpd/templates/ingress.yaml @@ -1,4 +1,4 @@ -{{- if .Values.ingress.enabled -}} +{{- if and .Values.ingress.enabled (not (dig "global" "ingress" "enabled" false .Values.AsMap)) -}} {{- $fullName := include "httpd.fullname" . -}} {{- $svcPort := .Values.service.port -}} apiVersion: networking.k8s.io/v1 diff --git a/charts/httpd/templates/mocks/_helpers.tpl b/charts/httpd/templates/mocks/_helpers.tpl new file mode 100644 index 000000000..a1a914914 --- /dev/null +++ b/charts/httpd/templates/mocks/_helpers.tpl @@ -0,0 +1,3 @@ +{{- define "mirrorbits-parent.pvc-name" -}} +parent-chart-shared-data +{{- end -}} diff --git a/charts/httpd/templates/persistentVolume.yaml b/charts/httpd/templates/persistentVolume.yaml index 62f46a39b..8d8a996e1 100644 --- a/charts/httpd/templates/persistentVolume.yaml +++ b/charts/httpd/templates/persistentVolume.yaml @@ -1,4 +1,4 @@ -{{ if $.Values.repository.persistentVolume.enabled -}} +{{ if and .Values.repository.persistentVolume.enabled (not (dig "global" "ingress" "enabled" false .Values.AsMap)) -}} --- apiVersion: v1 kind: PersistentVolume diff --git a/charts/httpd/templates/persistentVolumeClaim.yaml b/charts/httpd/templates/persistentVolumeClaim.yaml index 1d183f39a..268185747 100644 --- a/charts/httpd/templates/persistentVolumeClaim.yaml +++ b/charts/httpd/templates/persistentVolumeClaim.yaml @@ -1,4 +1,4 @@ -{{ if .Values.repository.persistentVolumeClaim.enabled -}} +{{ if and .Values.repository.persistentVolumeClaim.enabled (not (dig "global" "ingress" "enabled" false .Values.AsMap)) -}} --- apiVersion: v1 kind: PersistentVolumeClaim diff --git a/charts/httpd/tests/custom_values_test.yaml b/charts/httpd/tests/custom_values_test.yaml index 0844a7f8c..cbb5cefe4 100644 --- a/charts/httpd/tests/custom_values_test.yaml +++ b/charts/httpd/tests/custom_values_test.yaml @@ -4,6 +4,20 @@ templates: - ingress.yaml - secret.yaml # Direct dependency of deployment.yaml - service.yaml + - persistentVolume.yaml + - persistentVolumeClaim.yaml +set: + image: + pullPolicy: Never + repository: + reuseExistingPersistentVolumeClaim: true + ingress: + enabled: true + hosts: + - host: chart-example.local + paths: + - path: / + pathType: IfNotPresent tests: - it: Should set the correct service selector labels when a fullNameOverride is specified template: service.yaml @@ -37,9 +51,6 @@ tests: value: "RELEASE-NAME" - it: should define a customized deployment template: deployment.yaml - set: - image: - pullPolicy: Never asserts: - hasDocuments: count: 1 @@ -48,33 +59,39 @@ tests: - equal: path: spec.template.spec.containers[*].imagePullPolicy value: Never - - it: should define a "binary" volume if reuseExistingPersistentVolumeClaim is true + - it: should define a volume 'html' if reuseExistingPersistentVolumeClaim is true template: deployment.yaml - set: - repository: - reuseExistingPersistentVolumeClaim: true asserts: - hasDocuments: count: 1 - isKind: of: Deployment - - lengthEqual: - path: spec.template.spec.volumes - count: 2 - - contains: - path: spec.template.spec.volumes - content: - name: "binary" - any: true - - lengthEqual: - path: spec.template.spec.containers[0].volumeMounts - count: 2 - - contains: - path: spec.template.spec.containers[0].volumeMounts - content: - name: "binary" - any: true - - it: should define a "binary" volume if repository.persistentVolumeClaim.enabled is true + - equal: + path: spec.template.spec.volumes[0].name + value: conf + - equal: + path: spec.template.spec.containers[0].volumeMounts[0].name + value: conf + - equal: + path: spec.template.spec.containers[0].volumeMounts[0].readOnly + value: true + # HTML volume uses an existing PVC (specified by values) + - equal: + path: spec.template.spec.volumes[1].name + value: html + - equal: + path: spec.template.spec.volumes[1].persistentVolumeClaim.claimName + value: httpd-binary + - equal: + path: spec.template.spec.containers[0].volumeMounts[1].name + value: html + - equal: + path: spec.template.spec.containers[0].volumeMounts[1].readOnly + value: true + - equal: + path: spec.template.spec.containers[0].volumeMounts[1].mountPath + value: /usr/local/apache2/htdocs + - it: should define a managed "html" volume if repository.persistentVolumeClaim.enabled is true template: deployment.yaml set: repository: @@ -85,32 +102,33 @@ tests: count: 1 - isKind: of: Deployment - - lengthEqual: - path: spec.template.spec.volumes - count: 2 - - contains: - path: spec.template.spec.volumes - content: - name: "binary" - any: true - - lengthEqual: - path: spec.template.spec.containers[0].volumeMounts - count: 2 - - contains: - path: spec.template.spec.containers[0].volumeMounts - content: - name: "binary" - any: true + - equal: + path: spec.template.spec.volumes[0].name + value: conf + - equal: + path: spec.template.spec.containers[0].volumeMounts[0].name + value: conf + - equal: + path: spec.template.spec.containers[0].volumeMounts[0].readOnly + value: true + # HTML volume uses an existing PVC (specified by values) + - equal: + path: spec.template.spec.volumes[1].name + value: html + - equal: + path: spec.template.spec.volumes[1].persistentVolumeClaim.claimName + value: httpd-binary + - equal: + path: spec.template.spec.containers[0].volumeMounts[1].name + value: html + - equal: + path: spec.template.spec.containers[0].volumeMounts[1].readOnly + value: true + - equal: + path: spec.template.spec.containers[0].volumeMounts[1].mountPath + value: /usr/local/apache2/htdocs - it: should create ingress with pathType set to the specified custom value by default template: ingress.yaml - set: - ingress: - enabled: true - hosts: - - host: chart-example.local - paths: - - path: / - pathType: IfNotPresent asserts: - hasDocuments: count: 1 diff --git a/charts/httpd/tests/defaults_test.yaml b/charts/httpd/tests/defaults_test.yaml index de9972759..53c6b3085 100644 --- a/charts/httpd/tests/defaults_test.yaml +++ b/charts/httpd/tests/defaults_test.yaml @@ -5,30 +5,15 @@ templates: - ingress.yaml - secret.yaml # Direct dependency of deployment(.*).yaml - service.yaml + - persistentVolume.yaml + - persistentVolumeClaim.yaml tests: - it: should not create any ingress by default template: ingress.yaml asserts: - hasDocuments: count: 0 - - it: should create ingress with pathType set to Prefix by default - template: ingress.yaml - set: - ingress: - enabled: true - hosts: - - host: chart-example.local - paths: - - path: / - asserts: - - hasDocuments: - count: 1 - - isKind: - of: Ingress - - equal: - path: spec.rules[0].http.paths[0].pathType - value: "Prefix" - - it: should set the correct service selector labels + - it: should create a service with default values template: service.yaml asserts: - hasDocuments: @@ -41,20 +26,7 @@ tests: - equal: path: spec.selector["app.kubernetes.io/instance"] value: "RELEASE-NAME" - - it: Should set the correct deployment metadata labels - template: deployment.yaml - asserts: - - hasDocuments: - count: 1 - - isKind: - of: Deployment - - equal: - path: spec.template.metadata.labels["app.kubernetes.io/name"] - value: "httpd" - - equal: - path: spec.template.metadata.labels["app.kubernetes.io/instance"] - value: "RELEASE-NAME" - - it: should define the default deployment with default imagePullPolicy and metadata labels, and only a "conf" volume + - it: should create a Deployment with default values template: deployment.yaml asserts: - hasDocuments: @@ -68,17 +40,40 @@ tests: path: spec.template.metadata.labels["app.kubernetes.io/instance"] value: "RELEASE-NAME" - equal: - path: spec.template.spec.containers[0].imagePullPolicy + path: "spec.template.spec.containers[*].imagePullPolicy" value: IfNotPresent - - lengthEqual: - path: spec.template.spec.volumes - count: 1 - equal: path: spec.template.spec.volumes[0].name - value: "conf" - - lengthEqual: - path: spec.template.spec.containers[0].volumeMounts - count: 1 + value: conf - equal: path: spec.template.spec.containers[0].volumeMounts[0].name - value: "conf" + value: conf + - equal: + path: spec.template.spec.containers[0].volumeMounts[0].readOnly + value: true + # HTML volume is an emptydir by default + - equal: + path: spec.template.spec.volumes[1].name + value: html + - equal: + path: spec.template.spec.volumes[1].emptyDir + value: {} + - equal: + path: spec.template.spec.containers[0].volumeMounts[1].name + value: html + - equal: + path: spec.template.spec.containers[0].volumeMounts[1].readOnly + value: true + - equal: + path: spec.template.spec.containers[0].volumeMounts[1].mountPath + value: /usr/local/apache2/htdocs + - it: should not define any persistent volume + template: persistentVolume.yaml + asserts: + - hasDocuments: + count: 0 + - it: should not define any PV claim + template: persistentVolumeClaim.yaml + asserts: + - hasDocuments: + count: 0 diff --git a/charts/httpd/tests/parent_values_test.yaml b/charts/httpd/tests/parent_values_test.yaml new file mode 100644 index 000000000..1cc6c2e48 --- /dev/null +++ b/charts/httpd/tests/parent_values_test.yaml @@ -0,0 +1,95 @@ +suite: Tests with custom values from parent (umbrella ingress and umbrella PVC) +templates: + - deployment.yaml + - ingress.yaml + - secret.yaml # Direct dependency of deployment.yaml + - persistentVolume.yaml + - persistentVolumeClaim.yaml +set: + # Mock chart parent inherited global values passed to subcharts + global: + storage: + enabled: true + ingress: + enabled: true + # Try to specify a ingress which must be ignored (parent prevails) + ingress: + enabled: true + hosts: + - host: chart-example.local + paths: + - path: / + pathType: IfNotPresent + # Try to specify a PV which must be ignored (parent prevails) + repository: + name: httpd-binary + persistentVolumeClaim: + enabled: true + spec: + accessModes: + - ReadWriteMany + storageClassName: azurefile-csi-premium + resources: + requests: + storage: 1000Gi + volumeName: mirrorbits-binary + persistentVolume: + enabled: true + spec: + capacity: + storage: 1000Gi + storageClassName: azurefile-csi-premium + accessModes: + - ReadWriteMany + persistentVolumeReclaimPolicy: Retain + csi: + driver: file.csi.azure.com + readOnly: false + volumeHandle: httpd-binary # make sure this volumeid is unique for every identical share in the cluster + volumeAttributes: + resourceGroup: httpd-rg + shareName: httpd + nodeStageSecretRef: + name: httpd-data + namespace: httpd + mountOptions: + - dir_mode=0755 +tests: + - it: should define a customized "mirrorbits-lite" deployment + template: deployment.yaml + asserts: + - hasDocuments: + count: 1 + - isKind: + of: Deployment + # HTML volume (references a claim from parent chart) + - equal: + path: spec.template.spec.volumes[1].name + value: html + - equal: + path: spec.template.spec.volumes[1].persistentVolumeClaim.claimName + value: parent-chart-shared-data + - equal: + path: spec.template.spec.containers[0].volumeMounts[1].name + value: html + - equal: + path: spec.template.spec.containers[0].volumeMounts[1].readOnly + value: true + - equal: + path: spec.template.spec.containers[0].volumeMounts[1].mountPath + value: /usr/local/apache2/htdocs + - it: should not create any ingress + template: ingress.yaml + asserts: + - hasDocuments: + count: 0 + - it: should not define any persistent volume + template: persistentVolume.yaml + asserts: + - hasDocuments: + count: 0 + - it: should not define any PV claim + template: persistentVolumeClaim.yaml + asserts: + - hasDocuments: + count: 0 From c26f18c593447fc3831c39be36eb6ab8d8b09edf Mon Sep 17 00:00:00 2001 From: Damien Duportal Date: Tue, 3 Oct 2023 09:59:06 +0200 Subject: [PATCH 2/2] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Hervé Le Meur <91831478+lemeurherve@users.noreply.github.com> --- charts/httpd/tests/defaults_test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/httpd/tests/defaults_test.yaml b/charts/httpd/tests/defaults_test.yaml index 53c6b3085..42ee7269d 100644 --- a/charts/httpd/tests/defaults_test.yaml +++ b/charts/httpd/tests/defaults_test.yaml @@ -3,7 +3,7 @@ templates: - configmap.yaml - deployment.yaml - ingress.yaml - - secret.yaml # Direct dependency of deployment(.*).yaml + - secret.yaml # Direct dependency of deployment.yaml - service.yaml - persistentVolume.yaml - persistentVolumeClaim.yaml