Miretpl commented on code in PR #62497:
URL: https://github.com/apache/airflow/pull/62497#discussion_r2902320542
##########
helm-tests/tests/helm_tests/airflow_aux/test_annotations.py:
##########
@@ -260,11 +260,271 @@ def test_annotations_are_added(self, values, show_only,
expected_annotations):
assert k in obj["metadata"]["annotations"]
assert v == obj["metadata"]["annotations"][k]
+ @pytest.mark.parametrize(
+ ("values", "show_only", "expected_annotations"),
+ [
+ (
+ {
+ "executor": "KubernetesExecutor",
+ "cleanup": {
+ "enabled": True,
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/cleanup/cleanup-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "databaseCleanup": {
+ "enabled": True,
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+
"templates/database-cleanup/database-cleanup-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "scheduler": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/scheduler/scheduler-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "apiServer": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/api-server/api-server-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "workers": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/workers/worker-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "flower": {
+ "enabled": True,
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/flower/flower-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "statsd": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/statsd/statsd-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "redis": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/redis/redis-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "pgbouncer": {
+ "enabled": True,
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/pgbouncer/pgbouncer-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "createUserJob": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/jobs/create-user-job-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "migrateDatabaseJob": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/jobs/migrate-database-job-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "triggerer": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/triggerer/triggerer-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "dagProcessor": {
+ "enabled": True,
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/dag-processor/dag-processor-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "airflowVersion": "2.10.0",
+ "webserver": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/webserver/webserver-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ ],
+ )
+ def test_annotations_are_tpl_evaluated(self, values, show_only,
expected_annotations):
+ """Test that Helm template expressions in annotation values are
evaluated via tpl."""
+ k8s_objects = render_chart(
+ values=values,
+ show_only=[show_only],
+ )
+
+ assert len(k8s_objects) == 1
+ obj = k8s_objects[0]
+
+ for k, v in expected_annotations.items():
+ assert k in obj["metadata"]["annotations"]
+ assert obj["metadata"]["annotations"][k] == v
+
+ def test_annotations_mixed_static_and_templated(self):
+ """Test that mixed static and templated annotation values render
correctly."""
+ k8s_objects = render_chart(
+ values={
+ "airflowVersion": "2.10.0",
Review Comment:
```suggestion
"airflowVersion": "2.11.0",
```
##########
chart/templates/workers/worker-serviceaccount.yaml:
##########
@@ -48,8 +48,11 @@ metadata:
{{- if or (.Values.labels) (.Values.workers.labels) }}
{{- mustMerge .Values.workers.labels .Values.labels | toYaml | nindent 4
}}
{{- end }}
- {{- with .Values.workers.serviceAccount.annotations}}
- annotations: {{- toYaml . | nindent 4 }}
+ {{- with .Values.workers.serviceAccount.annotations }}
+ annotations:
+ {{- range $key, $value := . }}
+ {{- printf "%s: %s" $key (tpl $value $ | quote) | nindent 4 }}
Review Comment:
In short - Copilot is right. Workers are using a modified global context for
values to make Worker Sets (defined under `workers.celery.sets` section) to
work. Using the original global context, their annotations will not work with
the Worker Set feature.
##########
helm-tests/tests/helm_tests/airflow_aux/test_annotations.py:
##########
@@ -260,11 +260,271 @@ def test_annotations_are_added(self, values, show_only,
expected_annotations):
assert k in obj["metadata"]["annotations"]
assert v == obj["metadata"]["annotations"][k]
+ @pytest.mark.parametrize(
+ ("values", "show_only", "expected_annotations"),
+ [
+ (
+ {
+ "executor": "KubernetesExecutor",
+ "cleanup": {
+ "enabled": True,
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/cleanup/cleanup-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "databaseCleanup": {
+ "enabled": True,
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+
"templates/database-cleanup/database-cleanup-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "scheduler": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/scheduler/scheduler-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "apiServer": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/api-server/api-server-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "workers": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/workers/worker-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "flower": {
+ "enabled": True,
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/flower/flower-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "statsd": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/statsd/statsd-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "redis": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/redis/redis-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "pgbouncer": {
+ "enabled": True,
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/pgbouncer/pgbouncer-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "createUserJob": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/jobs/create-user-job-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "migrateDatabaseJob": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/jobs/migrate-database-job-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "triggerer": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/triggerer/triggerer-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "dagProcessor": {
+ "enabled": True,
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/dag-processor/dag-processor-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "airflowVersion": "2.10.0",
Review Comment:
```suggestion
"airflowVersion": "2.11.0",
```
On main we have a support for Airflow >=2.11 versions.
##########
helm-tests/tests/helm_tests/airflow_aux/test_annotations.py:
##########
@@ -260,11 +260,271 @@ def test_annotations_are_added(self, values, show_only,
expected_annotations):
assert k in obj["metadata"]["annotations"]
assert v == obj["metadata"]["annotations"][k]
+ @pytest.mark.parametrize(
+ ("values", "show_only", "expected_annotations"),
+ [
+ (
+ {
+ "executor": "KubernetesExecutor",
+ "cleanup": {
+ "enabled": True,
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/cleanup/cleanup-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "databaseCleanup": {
+ "enabled": True,
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+
"templates/database-cleanup/database-cleanup-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "scheduler": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/scheduler/scheduler-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "apiServer": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/api-server/api-server-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "workers": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/workers/worker-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "flower": {
+ "enabled": True,
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/flower/flower-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "statsd": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/statsd/statsd-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "redis": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/redis/redis-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "pgbouncer": {
+ "enabled": True,
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/pgbouncer/pgbouncer-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "createUserJob": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/jobs/create-user-job-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "migrateDatabaseJob": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/jobs/migrate-database-job-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "triggerer": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/triggerer/triggerer-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "dagProcessor": {
+ "enabled": True,
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/dag-processor/dag-processor-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ (
+ {
+ "airflowVersion": "2.10.0",
+ "webserver": {
+ "serviceAccount": {
+ "annotations": {
+ "eks.amazonaws.com/role-arn": "arn:aws:iam::{{
.Release.Name }}:role/airflow",
+ },
+ },
+ },
+ },
+ "templates/webserver/webserver-serviceaccount.yaml",
+ {
+ "eks.amazonaws.com/role-arn":
"arn:aws:iam::release-name:role/airflow",
+ },
+ ),
+ ],
+ )
+ def test_annotations_are_tpl_evaluated(self, values, show_only,
expected_annotations):
+ """Test that Helm template expressions in annotation values are
evaluated via tpl."""
+ k8s_objects = render_chart(
+ values=values,
+ show_only=[show_only],
+ )
+
+ assert len(k8s_objects) == 1
+ obj = k8s_objects[0]
+
+ for k, v in expected_annotations.items():
+ assert k in obj["metadata"]["annotations"]
+ assert obj["metadata"]["annotations"][k] == v
Review Comment:
We always expect the same annotation, so we could move the definition here
it simplify the test case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
