This is an automated email from the ASF dual-hosted git repository.
eladkal pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 3a3078bf53 fix(helm): safer worker pod annotations (#35309)
3a3078bf53 is described below
commit 3a3078bf53029aa289d812a6349f0e1685a31a79
Author: Seiji 誠 次 <[email protected]>
AuthorDate: Wed Nov 1 03:38:22 2023 -0300
fix(helm): safer worker pod annotations (#35309)
* chore(helm): safer worker pod annotations
* chore(helm): safer worker pod annotations
---------
Co-authored-by: hakuno <[email protected]>
Co-authored-by: Hussein Awala <[email protected]>
---
chart/templates/workers/worker-deployment.yaml | 10 +-
helm_tests/airflow_core/test_worker.py | 149 ++++++++++++++++++++++++-
2 files changed, 148 insertions(+), 11 deletions(-)
diff --git a/chart/templates/workers/worker-deployment.yaml
b/chart/templates/workers/worker-deployment.yaml
index a1fdfc72c4..6074302195 100644
--- a/chart/templates/workers/worker-deployment.yaml
+++ b/chart/templates/workers/worker-deployment.yaml
@@ -37,6 +37,8 @@
{{- $containerLifecycleHooks := or .Values.workers.containerLifecycleHooks
.Values.containerLifecycleHooks }}
{{- $containerLifecycleHooksLogGroomerSidecar := or
.Values.workers.logGroomerSidecar.containerLifecycleHooks
.Values.containerLifecycleHooks }}
{{- $containerLifecycleHooksKerberosSidecar := or
.Values.workers.kerberosSidecar.containerLifecycleHooks
.Values.containerLifecycleHooks }}
+{{- $safeToEvict := dict "cluster-autoscaler.kubernetes.io/safe-to-evict"
(.Values.workers.safeToEvict | toString) }}
+{{- $podAnnotations := mergeOverwrite .Values.airflowPodAnnotations
$safeToEvict .Values.workers.podAnnotations }}
apiVersion: apps/v1
kind: {{ if $persistence }}StatefulSet{{ else }}Deployment{{ end }}
metadata:
@@ -92,12 +94,8 @@ spec:
checksum/airflow-config: {{ include (print $.Template.BasePath
"/configmaps/configmap.yaml") . | sha256sum }}
checksum/extra-configmaps: {{ include (print $.Template.BasePath
"/configmaps/extra-configmaps.yaml") . | sha256sum }}
checksum/extra-secrets: {{ include (print $.Template.BasePath
"/secrets/extra-secrets.yaml") . | sha256sum }}
- cluster-autoscaler.kubernetes.io/safe-to-evict: {{
.Values.workers.safeToEvict | quote }}
- {{- if .Values.airflowPodAnnotations }}
- {{- toYaml .Values.airflowPodAnnotations | nindent 8 }}
- {{- end }}
- {{- if .Values.workers.podAnnotations }}
- {{- toYaml .Values.workers.podAnnotations | nindent 8 }}
+ {{- if $podAnnotations }}
+ {{- toYaml $podAnnotations | nindent 8 }}
{{- end }}
spec:
{{- if .Values.workers.runtimeClassName }}
diff --git a/helm_tests/airflow_core/test_worker.py
b/helm_tests/airflow_core/test_worker.py
index 3eae2d413a..87ba0e30e8 100644
--- a/helm_tests/airflow_core/test_worker.py
+++ b/helm_tests/airflow_core/test_worker.py
@@ -652,19 +652,139 @@ class TestWorker:
assert jmespath.search("metadata.annotations",
docs[0])["test_annotation"] == "test_annotation_value"
@pytest.mark.parametrize(
- "evictionStr, evictionBool",
- [("true", True), ("false", False)],
+ "globalScope, localScope, precedence",
+ [
+ ({}, {}, "true"),
+ ({}, {"safeToEvict": True}, "true"),
+ ({}, {"safeToEvict": False}, "false"),
+ (
+ {},
+ {
+ "podAnnotations":
{"cluster-autoscaler.kubernetes.io/safe-to-evict": "true"},
+ "safeToEvict": True,
+ },
+ "true",
+ ),
+ (
+ {},
+ {
+ "podAnnotations":
{"cluster-autoscaler.kubernetes.io/safe-to-evict": "true"},
+ "safeToEvict": False,
+ },
+ "true",
+ ),
+ (
+ {},
+ {
+ "podAnnotations":
{"cluster-autoscaler.kubernetes.io/safe-to-evict": "false"},
+ "safeToEvict": True,
+ },
+ "false",
+ ),
+ (
+ {},
+ {
+ "podAnnotations":
{"cluster-autoscaler.kubernetes.io/safe-to-evict": "false"},
+ "safeToEvict": False,
+ },
+ "false",
+ ),
+ (
+ {"cluster-autoscaler.kubernetes.io/safe-to-evict": "true"},
+ {"safeToEvict": True},
+ "true",
+ ),
+ (
+ {"cluster-autoscaler.kubernetes.io/safe-to-evict": "true"},
+ {"safeToEvict": False},
+ "false",
+ ),
+ (
+ {"cluster-autoscaler.kubernetes.io/safe-to-evict": "false"},
+ {"safeToEvict": True},
+ "true",
+ ),
+ (
+ {"cluster-autoscaler.kubernetes.io/safe-to-evict": "false"},
+ {"safeToEvict": False},
+ "false",
+ ),
+ (
+ {"cluster-autoscaler.kubernetes.io/safe-to-evict": "true"},
+ {
+ "podAnnotations":
{"cluster-autoscaler.kubernetes.io/safe-to-evict": "true"},
+ "safeToEvict": False,
+ },
+ "true",
+ ),
+ (
+ {"cluster-autoscaler.kubernetes.io/safe-to-evict": "true"},
+ {
+ "podAnnotations":
{"cluster-autoscaler.kubernetes.io/safe-to-evict": "false"},
+ "safeToEvict": False,
+ },
+ "false",
+ ),
+ (
+ {"cluster-autoscaler.kubernetes.io/safe-to-evict": "false"},
+ {
+ "podAnnotations":
{"cluster-autoscaler.kubernetes.io/safe-to-evict": "true"},
+ "safeToEvict": False,
+ },
+ "true",
+ ),
+ (
+ {"cluster-autoscaler.kubernetes.io/safe-to-evict": "false"},
+ {
+ "podAnnotations":
{"cluster-autoscaler.kubernetes.io/safe-to-evict": "false"},
+ "safeToEvict": False,
+ },
+ "false",
+ ),
+ (
+ {"cluster-autoscaler.kubernetes.io/safe-to-evict": "true"},
+ {
+ "podAnnotations":
{"cluster-autoscaler.kubernetes.io/safe-to-evict": "true"},
+ "safeToEvict": True,
+ },
+ "true",
+ ),
+ (
+ {"cluster-autoscaler.kubernetes.io/safe-to-evict": "true"},
+ {
+ "podAnnotations":
{"cluster-autoscaler.kubernetes.io/safe-to-evict": "false"},
+ "safeToEvict": True,
+ },
+ "false",
+ ),
+ (
+ {"cluster-autoscaler.kubernetes.io/safe-to-evict": "false"},
+ {
+ "podAnnotations":
{"cluster-autoscaler.kubernetes.io/safe-to-evict": "true"},
+ "safeToEvict": True,
+ },
+ "true",
+ ),
+ (
+ {"cluster-autoscaler.kubernetes.io/safe-to-evict": "false"},
+ {
+ "podAnnotations":
{"cluster-autoscaler.kubernetes.io/safe-to-evict": "false"},
+ "safeToEvict": True,
+ },
+ "false",
+ ),
+ ],
)
- def test_safetoevict_annotations(self, evictionStr, evictionBool):
+ def test_safetoevict_annotations(self, globalScope, localScope,
precedence):
docs = render_chart(
- values={"workers": {"safeToEvict": evictionBool}},
+ values={"airflowPodAnnotations": globalScope, "workers":
localScope},
show_only=["templates/workers/worker-deployment.yaml"],
)
assert (
jmespath.search("spec.template.metadata.annotations", docs[0])[
"cluster-autoscaler.kubernetes.io/safe-to-evict"
]
- == evictionStr
+ == precedence
)
def test_should_add_extra_volume_claim_templates(self):
@@ -716,6 +836,25 @@ class TestWorker:
"spec.volumeClaimTemplates[2].spec.resources.requests.storage",
docs[0]
)
+ @pytest.mark.parametrize(
+ "globalScope, localScope, precedence",
+ [
+ ({"scope": "global"}, {"podAnnotations": {}}, "global"),
+ ({}, {"podAnnotations": {"scope": "local"}}, "local"),
+ ({"scope": "global"}, {"podAnnotations": {"scope": "local"}},
"local"),
+ ({}, {}, None),
+ ],
+ )
+ def test_podannotations_precedence(self, globalScope, localScope,
precedence):
+ docs = render_chart(
+ values={"airflowPodAnnotations": globalScope, "workers":
localScope},
+ show_only=["templates/workers/worker-deployment.yaml"],
+ )
+ if precedence:
+ assert jmespath.search("spec.template.metadata.annotations",
docs[0])["scope"] == precedence
+ else:
+ assert jmespath.search("spec.template.metadata.annotations.scope",
docs[0]) is None
+
class TestWorkerLogGroomer(LogGroomerTestBase):
"""Worker groomer."""
