From 7329b73a7bf05f792ffe5aa77ab299e40c8358a0 Mon Sep 17 00:00:00 2001 From: Stavros Kois <47820033+stavros-k@users.noreply.github.com> Date: Tue, 26 Dec 2023 22:23:32 +0200 Subject: [PATCH] fix(persistence): respect tpl enabled in all places (#664) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Description** ⚒️ Fixes # **⚙️ Type of change** - [ ] ⚙️ Feature/App addition - [ ] 🪛 Bugfix - [ ] ⚠️ Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] 🔃 Refactor of current code **🧪 How Has This Been Tested?** **📃 Notes:** **✔️ Checklist:** - [ ] ⚖️ My code follows the style guidelines of this project - [ ] 👀 I have performed a self-review of my own code - [ ] #️⃣ I have commented my code, particularly in hard-to-understand areas - [ ] 📄 I have made corresponding changes to the documentation - [ ] ⚠️ My changes generate no new warnings - [ ] 🧪 I have added tests to this description that prove my fix is effective or that my feature works - [ ] ⬆️ I increased versions for any altered app according to semantic versioning **➕ App addition** If this PR is an app addition please make sure you have done the following. - [ ] 🪞 I have opened a PR on [truecharts/containers](https://github.com/truecharts/containers) adding the container to TrueCharts mirror repo. - [ ] 🖼️ I have added an icon in the Chart's root directory called `icon.png` --- _Please don't blindly check all the boxes. Read them and only check those that apply. Those checkboxes are there for the reviewer to see what is this all about and the status of this PR with a quick glance._ --- library/common-test/tests/cnpg/stop_test.yaml | 49 ++++++++++++- .../tests/container/volumeMounts_test.yaml | 72 +++++++++++++++++++ .../common-test/tests/ingress/rules_test.yaml | 18 +++++ .../common-test/tests/ingress/stop_test.yaml | 23 ++++++ .../common-test/tests/service/stop_test.yaml | 40 +++++++++++ .../spec_test.yaml | 2 + .../spec_test.yaml | 2 + library/common/Chart.yaml | 2 +- .../class/velero/_backupStorageLocation.tpl | 4 +- .../class/velero/_volumeSnapshotLocation.tpl | 4 +- .../lib/container/_securityContext.tpl | 6 +- .../templates/lib/container/_volumeMounts.tpl | 7 +- .../templates/lib/ingress/_serviceData.tpl | 19 ++++- .../templates/lib/pod/_podSecurityContext.tpl | 6 +- library/common/templates/lib/pod/_volumes.tpl | 6 +- .../lib/storage/_volumeClaimTemplates.tpl | 6 +- .../common/templates/lib/util/_stopAll.tpl | 4 ++ 17 files changed, 255 insertions(+), 15 deletions(-) diff --git a/library/common-test/tests/cnpg/stop_test.yaml b/library/common-test/tests/cnpg/stop_test.yaml index d92700ed..98060e2d 100644 --- a/library/common-test/tests/cnpg/stop_test.yaml +++ b/library/common-test/tests/cnpg/stop_test.yaml @@ -62,7 +62,7 @@ tests: content: cnpg.io/hibernation: "on" - - it: should set hibernate and 0 instances on ixChartContext - isStopped + - it: should set hibernate and 0 instances on ixChartContext - isStopped (true) set: operator: verify: @@ -94,7 +94,7 @@ tests: content: cnpg.io/hibernation: "on" - - it: should not set hibernate and 0 instances on ixChartContext - isStopped + - it: should not set hibernate and 0 instances on ixChartContext - isStopped (false) set: operator: verify: @@ -137,3 +137,48 @@ tests: path: spec content: instances: 2 + + - it: should not set hibernate and 0 instances on ixChartContext - isStopped (true) and ignoreIsStopped (true) + set: + operator: + verify: + enabled: false + global: + namespace: ix-something + ignoreIsStopped: true + ixChartContext: + storageClassName: some-storage-class + isStopped: true + cnpg: + mypg: + enabled: true + user: app + database: app + hibernate: false + instances: 2 + asserts: + # Cluster, Pooler and 2 Secrets + - hasDocuments: + count: 4 + - documentIndex: &clusterDoc 1 + isKind: + of: Cluster + - documentIndex: *clusterDoc + isAPIVersion: + of: postgresql.cnpg.io/v1 + - documentIndex: *clusterDoc + isSubset: + path: metadata.annotations + content: + cnpg.io/hibernation: "off" + - documentIndex: &poolerDoc 0 + isKind: + of: Pooler + - documentIndex: *poolerDoc + isAPIVersion: + of: postgresql.cnpg.io/v1 + - documentIndex: *poolerDoc + isSubset: + path: spec + content: + instances: 2 diff --git a/library/common-test/tests/container/volumeMounts_test.yaml b/library/common-test/tests/container/volumeMounts_test.yaml index bfc54c86..b6f2bc48 100644 --- a/library/common-test/tests/container/volumeMounts_test.yaml +++ b/library/common-test/tests/container/volumeMounts_test.yaml @@ -952,6 +952,78 @@ tests: mountPath: /dev/sdb readOnly: false + - it: should respect tpl in enabled + set: + image: *image + flag: false + workload: + workload-name1: + enabled: true + primary: true + type: Deployment + podSpec: + containers: + container-name1: + enabled: true + primary: true + imageSelector: image + probes: *probes + persistence: + some-vol: + enabled: "{{ .Values.flag }}" + type: emptyDir + mountPath: /some/path + asserts: + - documentIndex: &deploymentDoc 0 + isKind: + of: Deployment + - documentIndex: *deploymentDoc + isAPIVersion: + of: apps/v1 + - documentIndex: *deploymentDoc + equal: + path: spec.template.spec.volumes + value: + - name: devshm + emptyDir: + medium: Memory + sizeLimit: 8Gi + - name: shared + emptyDir: {} + - name: tmp + emptyDir: + medium: Memory + sizeLimit: 8Gi + - name: varlogs + emptyDir: + medium: Memory + sizeLimit: 8Gi + - name: varrun + emptyDir: + medium: Memory + sizeLimit: 8Gi + + - documentIndex: *deploymentDoc + equal: + # some-vol should not be present + path: spec.template.spec.containers[0].volumeMounts + value: + - mountPath: /dev/shm + name: devshm + readOnly: false + - mountPath: /shared + name: shared + readOnly: false + - mountPath: /tmp + name: tmp + readOnly: false + - mountPath: /var/logs + name: varlogs + readOnly: false + - mountPath: /var/run + name: varrun + readOnly: false + # Failures - it: should fail with invalid mountPropagation set: diff --git a/library/common-test/tests/ingress/rules_test.yaml b/library/common-test/tests/ingress/rules_test.yaml index 572423a4..a31d3424 100644 --- a/library/common-test/tests/ingress/rules_test.yaml +++ b/library/common-test/tests/ingress/rules_test.yaml @@ -80,6 +80,14 @@ tests: name: some-other-service expandObjectName: false port: 8080 + - host: "host-target-primary-service" + paths: + - path: /subpath + overrideService: + # It is primary, so the backend target + # must be just the fullname + name: my-service + port: 8080 integrations: *integrations asserts: - documentIndex: &ingressDoc 1 @@ -120,6 +128,16 @@ tests: name: some-other-service port: number: 8080 + - host: "host-target-primary-service" + http: + paths: + - path: /subpath + pathType: Prefix + backend: + service: + name: test-release-name-common-test + port: + number: 8080 - it: should pass with ingress created with rules with targetSelector set: diff --git a/library/common-test/tests/ingress/stop_test.yaml b/library/common-test/tests/ingress/stop_test.yaml index 63715d18..d956dde1 100644 --- a/library/common-test/tests/ingress/stop_test.yaml +++ b/library/common-test/tests/ingress/stop_test.yaml @@ -91,3 +91,26 @@ tests: - documentIndex: *ingressDoc isNull: path: spec.ingressClassName + + - it: should ignore isStopped in ixChartContext + set: + operator: *operator + global: + namespace: ix-something + ignoreIsStopped: true + ixChartContext: + storageClassName: some-storage-class + isStopped: true + service: *service + ingress: *ingress + asserts: + - documentIndex: &ingressDoc 1 + isKind: + of: Ingress + - documentIndex: *ingressDoc + equal: + path: metadata.name + value: test-release-name-common-test + - documentIndex: *ingressDoc + isNull: + path: spec.ingressClassName diff --git a/library/common-test/tests/service/stop_test.yaml b/library/common-test/tests/service/stop_test.yaml index 8b9487e0..59f3670d 100644 --- a/library/common-test/tests/service/stop_test.yaml +++ b/library/common-test/tests/service/stop_test.yaml @@ -118,3 +118,43 @@ tests: equal: path: spec.type value: LoadBalancer + + - it: should ignore isStopped in ixChartContext + set: + global: + namespace: ix-something + ignoreIsStopped: true + ixChartContext: + storageClassName: some-storage-class + isStopped: true + service: &service + my-service1: + enabled: true + primary: true + type: LoadBalancer + ports: + port-name: + enabled: true + primary: true + port: 12344 + workload: &workload + my-workload: + enabled: true + primary: true + type: Deployment + podSpec: {} + asserts: + - documentIndex: &serviceDoc 1 + isKind: + of: Service + - documentIndex: *serviceDoc + isAPIVersion: + of: v1 + - documentIndex: *serviceDoc + equal: + path: metadata.name + value: test-release-name-common-test + - documentIndex: *serviceDoc + equal: + path: spec.type + value: LoadBalancer diff --git a/library/common-test/tests/veleroBackupStorageLocation/spec_test.yaml b/library/common-test/tests/veleroBackupStorageLocation/spec_test.yaml index 056de01d..fd03dfa8 100644 --- a/library/common-test/tests/veleroBackupStorageLocation/spec_test.yaml +++ b/library/common-test/tests/veleroBackupStorageLocation/spec_test.yaml @@ -143,6 +143,8 @@ tests: region: "{{ .Values.region }}" s3ForcePathStyle: "{{ .Values.useS3PathStyle }}" bool: false + shouldNotBeIncluded: "" + alsoShouldNotBeIncluded: asserts: - documentIndex: &backStoreLocDoc 1 isKind: diff --git a/library/common-test/tests/veleroVolumeSnapshotLocation/spec_test.yaml b/library/common-test/tests/veleroVolumeSnapshotLocation/spec_test.yaml index 3945cba5..e18ad5f5 100644 --- a/library/common-test/tests/veleroVolumeSnapshotLocation/spec_test.yaml +++ b/library/common-test/tests/veleroVolumeSnapshotLocation/spec_test.yaml @@ -126,6 +126,8 @@ tests: region: "{{ .Values.region }}" s3ForcePathStyle: "{{ .Values.useS3PathStyle }}" bool: false + shouldNotBeIncluded: "" + alsoShouldNotBeIncluded: asserts: - documentIndex: &volSnapLocDoc 1 isKind: diff --git a/library/common/Chart.yaml b/library/common/Chart.yaml index 8d687df1..2ac44e1d 100644 --- a/library/common/Chart.yaml +++ b/library/common/Chart.yaml @@ -15,4 +15,4 @@ maintainers: name: common sources: null type: library -version: 17.1.3 +version: 17.1.4 diff --git a/library/common/templates/class/velero/_backupStorageLocation.tpl b/library/common/templates/class/velero/_backupStorageLocation.tpl index a4aa21c9..bbc68d55 100644 --- a/library/common/templates/class/velero/_backupStorageLocation.tpl +++ b/library/common/templates/class/velero/_backupStorageLocation.tpl @@ -40,9 +40,9 @@ spec: {{- with $objectData.config }} config: {{- range $k, $v := . }} - {{- if $v -}} + {{- if and (not (kindIs "invalid" $v)) (ne (toString $v) "") }} {{ $k }}: {{ tpl (toString $v) $rootCtx | quote }} - {{- end -}} + {{- end -}} {{- end -}} {{- end -}} {{- with $objectData.backupSyncPeriod }} diff --git a/library/common/templates/class/velero/_volumeSnapshotLocation.tpl b/library/common/templates/class/velero/_volumeSnapshotLocation.tpl index 66318a22..8a7d9eb5 100644 --- a/library/common/templates/class/velero/_volumeSnapshotLocation.tpl +++ b/library/common/templates/class/velero/_volumeSnapshotLocation.tpl @@ -40,9 +40,9 @@ spec: {{- with $objectData.config }} config: {{- range $k, $v := . }} - {{- if $v -}} + {{- if and (not (kindIs "invalid" $v)) (ne (toString $v) "") }} {{ $k }}: {{ tpl (toString $v) $rootCtx | quote }} - {{- end -}} + {{- end -}} {{- end -}} {{- end -}} {{- end -}} diff --git a/library/common/templates/lib/container/_securityContext.tpl b/library/common/templates/lib/container/_securityContext.tpl index 3552a2f8..d1af2532 100644 --- a/library/common/templates/lib/container/_securityContext.tpl +++ b/library/common/templates/lib/container/_securityContext.tpl @@ -52,7 +52,11 @@ objectData: The object data to be used to render the container. {{- $mustPrivileged := false -}} {{- range $persistenceName, $persistenceValues := $rootCtx.Values.persistence -}} - {{- if $persistenceValues.enabled -}} + {{- $enabled := (include "tc.v1.common.lib.util.enabled" (dict + "rootCtx" $rootCtx "objectData" $persistenceValues + "name" $persistenceName "caller" "Security Context" + "key" "persistence")) -}} + {{- if (eq $enabled "true") -}} {{- if eq $persistenceValues.type "device" -}} {{- $volume := (fromJson (include "tc.v1.common.lib.container.volumeMount.isSelected" (dict "persistenceName" $persistenceName "persistenceValues" $persistenceValues "objectData" $objectData "key" "persistence"))) -}} {{- if $volume -}} {{/* If a volume is returned, it means that the container has an assigned device */}} diff --git a/library/common/templates/lib/container/_volumeMounts.tpl b/library/common/templates/lib/container/_volumeMounts.tpl index f8339f81..dfe5190d 100644 --- a/library/common/templates/lib/container/_volumeMounts.tpl +++ b/library/common/templates/lib/container/_volumeMounts.tpl @@ -13,8 +13,13 @@ objectData: The object data to be used to render the container. {{- $codeServerIgnoredTypes := (list "configmap" "secret" "vct") -}} {{- range $persistenceName, $persistenceValues := $rootCtx.Values.persistence -}} + {{- $enabled := (include "tc.v1.common.lib.util.enabled" (dict + "rootCtx" $rootCtx "objectData" $persistenceValues + "name" $persistenceName "caller" "Volume Mount" + "key" "persistence")) -}} + {{/* TLDR: Enabled + Not VCT without STS */}} - {{- if and $persistenceValues.enabled (not (and (eq $persistenceValues.type "vct") (ne $objectData.podType "StatefulSet"))) -}} + {{- if and (eq $enabled "true") (not (and (eq $persistenceValues.type "vct") (ne $objectData.podType "StatefulSet"))) -}} {{/* Dont try to mount configmap/sercet/vct to codeserver */}} {{- if not (and (eq $objectData.shortName "codeserver") (mustHas $persistenceValues.type $codeServerIgnoredTypes)) -}} {{- $volMount := (fromJson (include "tc.v1.common.lib.container.volumeMount.isSelected" (dict "persistenceName" $persistenceName "persistenceValues" $persistenceValues "objectData" $objectData))) -}} diff --git a/library/common/templates/lib/ingress/_serviceData.tpl b/library/common/templates/lib/ingress/_serviceData.tpl index 3b66494d..3190dbf0 100644 --- a/library/common/templates/lib/ingress/_serviceData.tpl +++ b/library/common/templates/lib/ingress/_serviceData.tpl @@ -8,14 +8,27 @@ {{- with $override -}} {{- $name := .name -}} {{- $expandName := (include "tc.v1.common.lib.util.expandName" (dict - "rootCtx" $rootCtx "objectData" . "name" .name + "rootCtx" $rootCtx "objectData" . "name" $name "caller" "Ingress" "key" "overrideService" )) -}} + {{/* Init */}} + {{- $expName := $name -}} + + {{/* Expand if needed */}} {{- if eq $expandName "true" -}} - {{- $name = (printf "%s-%s" $fullname .name) -}} + {{/* But first check if the svc is primary */}} + {{- $svc := (get $rootCtx.Values.service $name) | default dict -}} + + {{- if $svc.primary -}} {{/* If primary, use fullname */}} + {{- $expName = $fullname -}} + {{- else -}} {{/* If not primary, use fullname + name */}} + {{- $expName = (printf "%s-%s" $fullname $name) -}} + {{- end -}} + {{- end -}} - {{- $svcData = (dict "name" $name "port" .port) -}} + + {{- $svcData = (dict "name" $expName "port" .port) -}} {{- end -}} {{- $svcData | toYaml -}} diff --git a/library/common/templates/lib/pod/_podSecurityContext.tpl b/library/common/templates/lib/pod/_podSecurityContext.tpl index dc6451b6..9dc5ac6d 100644 --- a/library/common/templates/lib/pod/_podSecurityContext.tpl +++ b/library/common/templates/lib/pod/_podSecurityContext.tpl @@ -40,7 +40,11 @@ objectData: The object data to be used to render the Pod. {{- $podSelected := false -}} {{- range $persistenceName, $persistenceValues := $rootCtx.Values.persistence -}} - {{- if $persistenceValues.enabled -}} + {{- $enabled := (include "tc.v1.common.lib.util.enabled" (dict + "rootCtx" $rootCtx "objectData" $persistenceValues + "name" $persistenceName "caller" "Pod Security Context" + "key" "persistence")) -}} + {{- if (eq $enabled "true") -}} {{- if $persistenceValues.targetSelectAll -}} {{- $podSelected = true -}} {{- else if and $persistenceValues.targetSelector (kindIs "map" $persistenceValues.targetSelector) -}} diff --git a/library/common/templates/lib/pod/_volumes.tpl b/library/common/templates/lib/pod/_volumes.tpl index 10ec8c9f..d234c3b1 100644 --- a/library/common/templates/lib/pod/_volumes.tpl +++ b/library/common/templates/lib/pod/_volumes.tpl @@ -9,7 +9,11 @@ objectData: The object data to be used to render the Pod. {{- $objectData := .objectData -}} {{- range $name, $persistenceValues := $rootCtx.Values.persistence -}} - {{- if $persistenceValues.enabled -}} + {{- $enabled := (include "tc.v1.common.lib.util.enabled" (dict + "rootCtx" $rootCtx "objectData" $persistenceValues + "name" $name "caller" "Volumes" + "key" "persistence")) -}} + {{- if (eq $enabled "true") -}} {{- $persistence := (mustDeepCopy $persistenceValues) -}} {{- $_ := set $persistence "shortName" $name -}} diff --git a/library/common/templates/lib/storage/_volumeClaimTemplates.tpl b/library/common/templates/lib/storage/_volumeClaimTemplates.tpl index eb5e77bc..a01fec1f 100644 --- a/library/common/templates/lib/storage/_volumeClaimTemplates.tpl +++ b/library/common/templates/lib/storage/_volumeClaimTemplates.tpl @@ -9,8 +9,12 @@ objectData: The object data to be used to render the Pod. {{- $objectData := .objectData -}} {{- range $name, $vctValues := $rootCtx.Values.persistence -}} + {{- $enabled := (include "tc.v1.common.lib.util.enabled" (dict + "rootCtx" $rootCtx "objectData" $vctValues + "name" $name "caller" "Volume Claim Templates" + "key" "persistence")) -}} - {{- if and $vctValues.enabled (eq $vctValues.type "vct") -}} + {{- if and (eq $enabled "true") (eq $vctValues.type "vct") -}} {{- $vct := (mustDeepCopy $vctValues) -}} {{- $selected := false -}} diff --git a/library/common/templates/lib/util/_stopAll.tpl b/library/common/templates/lib/util/_stopAll.tpl index dfd4d368..0de74cdc 100644 --- a/library/common/templates/lib/util/_stopAll.tpl +++ b/library/common/templates/lib/util/_stopAll.tpl @@ -10,6 +10,10 @@ {{- if .isStopped -}} {{- $stop = true -}} {{- end -}} + {{/* Useful for cases like external-service */}} + {{- if $rootCtx.Values.global.ignoreIsStopped -}} + {{- $stop = "" -}} + {{- end -}} {{- end -}} {{- $stop -}}