From 7a47af523d5dfde110c1c0cab3144b4f488df6ae Mon Sep 17 00:00:00 2001 From: Oleksandr Malyga Date: Thu, 2 Feb 2023 13:55:13 +0100 Subject: [PATCH 1/5] add ttlSecondsAfterFinished parameter to migrate-database-job and create-user-job --- chart/templates/jobs/create-user-job.yaml | 3 +++ chart/templates/jobs/migrate-database-job.yaml | 3 +++ chart/values.schema.json | 16 ++++++++++++++++ chart/values.yaml | 4 ++++ tests/charts/test_create_user_job.py | 8 ++++++++ tests/charts/test_migrate_database_job.py | 8 ++++++++ 6 files changed, 42 insertions(+) diff --git a/chart/templates/jobs/create-user-job.yaml b/chart/templates/jobs/create-user-job.yaml index a4bb2bde9f899..c9aa43e106198 100644 --- a/chart/templates/jobs/create-user-job.yaml +++ b/chart/templates/jobs/create-user-job.yaml @@ -48,6 +48,9 @@ metadata: {{- $annotations | toYaml | nindent 4 }} {{- end }} spec: + {{- if .Values.createUserJob.ttlSecondsAfterFinished }} + ttlSecondsAfterFinished: {{ .Values.createUserJob.ttlSecondsAfterFinished }} + {{- end }} template: metadata: labels: diff --git a/chart/templates/jobs/migrate-database-job.yaml b/chart/templates/jobs/migrate-database-job.yaml index 67e61abf48dc0..ce92f79978fb4 100644 --- a/chart/templates/jobs/migrate-database-job.yaml +++ b/chart/templates/jobs/migrate-database-job.yaml @@ -48,6 +48,9 @@ metadata: {{- $annotations | toYaml | nindent 4 }} {{- end }} spec: + {{- if .Values.migrateDatabaseJob.ttlSecondsAfterFinished }} + ttlSecondsAfterFinished: {{ .Values.migrateDatabaseJob.ttlSecondsAfterFinished }} + {{- end }} template: metadata: labels: diff --git a/chart/values.schema.json b/chart/values.schema.json index 8bd29c0f6b7f9..0cc6b4d6d903f 100644 --- a/chart/values.schema.json +++ b/chart/values.schema.json @@ -2945,6 +2945,14 @@ "type": "boolean", "default": true }, + "ttlSecondsAfterFinished": { + "description": "Limit the lifetime of the job object after it finished execution", + "type": [ + "integer", + "null" + ], + "default": null + }, "env": { "description": "Add additional env vars to the create user job pod.", "type": "array", @@ -3144,6 +3152,14 @@ "description": "Specify if you want additional configured env vars applied to this job", "type": "boolean", "default": true + }, + "ttlSecondsAfterFinished": { + "description": "Limit the lifetime of the job object after it finished execution", + "type": [ + "integer", + "null" + ], + "default": null } } }, diff --git a/chart/values.yaml b/chart/values.yaml index 339bcb844d902..ac675f006b0ec 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -759,6 +759,8 @@ scheduler: # Airflow create user job settings createUserJob: + # Limit the lifetime of the job object after it finished execution. + ttlSecondsAfterFinished: ~ # Command to use when running the create user job (templated). command: ~ # Args to use when running the create user job (templated). @@ -837,6 +839,8 @@ createUserJob: # Airflow database migration job settings migrateDatabaseJob: enabled: true + # Limit the lifetime of the job object after it finished execution. + ttlSecondsAfterFinished: ~ # Command to use when running the migrate database job (templated). command: ~ # Args to use when running the migrate database job (templated). diff --git a/tests/charts/test_create_user_job.py b/tests/charts/test_create_user_job.py index 92fe5e22ceaa4..dde07efa07ad2 100644 --- a/tests/charts/test_create_user_job.py +++ b/tests/charts/test_create_user_job.py @@ -241,6 +241,14 @@ def test_should_disable_custom_env(self): assert {"name": "foo", "value": "bar"} not in envs assert {"name": "extraFoo", "value": "extraBar"} not in envs + def test_job_ttl_after_finish(self): + docs = render_chart( + values={"ttlSecondsAfterFinished": 0}, + show_only=["templates/jobs/create-user-job.yaml"], + ) + ttl = jmespath.search("spec.ttlSecondsAfterFinished", docs[0]) + assert ttl == 0 + @pytest.mark.parametrize( "airflow_version, expected_arg", [ diff --git a/tests/charts/test_migrate_database_job.py b/tests/charts/test_migrate_database_job.py index d25fdbb4b8f3f..c9663e0edd290 100644 --- a/tests/charts/test_migrate_database_job.py +++ b/tests/charts/test_migrate_database_job.py @@ -235,6 +235,14 @@ def test_should_add_global_volume_and_global_volume_mount(self): "spec.template.spec.containers[0].volumeMounts[-1]", docs[0] ) + def test_job_ttl_after_finish(self): + docs = render_chart( + values={"ttlSecondsAfterFinished": 0}, + show_only=["templates/jobs/migrate-database-job.yaml"], + ) + ttl = jmespath.search("spec.ttlSecondsAfterFinished", docs[0]) + assert ttl == 0 + @pytest.mark.parametrize( "airflow_version, expected_arg", [ From ba9e6277249e6db2bc612e14dc5372413f6ed2c1 Mon Sep 17 00:00:00 2001 From: Oleksandr Malyga Date: Thu, 2 Feb 2023 17:16:33 +0100 Subject: [PATCH 2/5] fixed ttlSecondsAfterFinished tests --- tests/charts/test_create_user_job.py | 17 +++++++++++++++-- tests/charts/test_migrate_database_job.py | 17 +++++++++++++++-- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/tests/charts/test_create_user_job.py b/tests/charts/test_create_user_job.py index dde07efa07ad2..0dd6c96bc6835 100644 --- a/tests/charts/test_create_user_job.py +++ b/tests/charts/test_create_user_job.py @@ -241,14 +241,27 @@ def test_should_disable_custom_env(self): assert {"name": "foo", "value": "bar"} not in envs assert {"name": "extraFoo", "value": "extraBar"} not in envs - def test_job_ttl_after_finish(self): + def test_job_ttl_after_finished(self): docs = render_chart( - values={"ttlSecondsAfterFinished": 0}, + values={"migrateDatabaseJob": {"ttlSecondsAfterFinished": 1}}, + show_only=["templates/jobs/create-user-job.yaml"], + ) + ttl = jmespath.search("spec.ttlSecondsAfterFinished", docs[0]) + assert ttl == 1 + + def test_job_ttl_after_finished_zero(self): + docs = render_chart( + values={"migrateDatabaseJob": {"ttlSecondsAfterFinished": 0}}, show_only=["templates/jobs/create-user-job.yaml"], ) ttl = jmespath.search("spec.ttlSecondsAfterFinished", docs[0]) assert ttl == 0 + def test_job_ttl_after_finished_nil(self): + docs = render_chart(show_only=["templates/jobs/create-user-job.yaml"]) + spec = jmespath.search("spec", docs[0]) + assert "ttlSecondsAfterFinished" not in spec + @pytest.mark.parametrize( "airflow_version, expected_arg", [ diff --git a/tests/charts/test_migrate_database_job.py b/tests/charts/test_migrate_database_job.py index c9663e0edd290..e3b233c1d1c12 100644 --- a/tests/charts/test_migrate_database_job.py +++ b/tests/charts/test_migrate_database_job.py @@ -235,14 +235,27 @@ def test_should_add_global_volume_and_global_volume_mount(self): "spec.template.spec.containers[0].volumeMounts[-1]", docs[0] ) - def test_job_ttl_after_finish(self): + def test_job_ttl_after_finished(self): docs = render_chart( - values={"ttlSecondsAfterFinished": 0}, + values={"migrateDatabaseJob": {"ttlSecondsAfterFinished": 1}}, + show_only=["templates/jobs/migrate-database-job.yaml"], + ) + ttl = jmespath.search("spec.ttlSecondsAfterFinished", docs[0]) + assert ttl == 1 + + def test_job_ttl_after_finished_zero(self): + docs = render_chart( + values={"migrateDatabaseJob": {"ttlSecondsAfterFinished": 0}}, show_only=["templates/jobs/migrate-database-job.yaml"], ) ttl = jmespath.search("spec.ttlSecondsAfterFinished", docs[0]) assert ttl == 0 + def test_job_ttl_after_finished_nil(self): + docs = render_chart(show_only=["templates/jobs/migrate-database-job.yaml"]) + spec = jmespath.search("spec", docs[0]) + assert "ttlSecondsAfterFinished" not in spec + @pytest.mark.parametrize( "airflow_version, expected_arg", [ From 058b846c0d08b56eae5af8bf9cf64cdf90d1cb16 Mon Sep 17 00:00:00 2001 From: Oleksandr Malyga Date: Thu, 2 Feb 2023 17:37:03 +0100 Subject: [PATCH 3/5] fix ttlSecondsAfterFinished population when value is zero --- chart/templates/jobs/create-user-job.yaml | 2 +- chart/templates/jobs/migrate-database-job.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/chart/templates/jobs/create-user-job.yaml b/chart/templates/jobs/create-user-job.yaml index c9aa43e106198..ca6826dd7d11d 100644 --- a/chart/templates/jobs/create-user-job.yaml +++ b/chart/templates/jobs/create-user-job.yaml @@ -48,7 +48,7 @@ metadata: {{- $annotations | toYaml | nindent 4 }} {{- end }} spec: - {{- if .Values.createUserJob.ttlSecondsAfterFinished }} + {{- if not (kindIs "invalid" .Values.createUserJob.ttlSecondsAfterFinished) }} ttlSecondsAfterFinished: {{ .Values.createUserJob.ttlSecondsAfterFinished }} {{- end }} template: diff --git a/chart/templates/jobs/migrate-database-job.yaml b/chart/templates/jobs/migrate-database-job.yaml index ce92f79978fb4..d5b1ca9a11527 100644 --- a/chart/templates/jobs/migrate-database-job.yaml +++ b/chart/templates/jobs/migrate-database-job.yaml @@ -48,7 +48,7 @@ metadata: {{- $annotations | toYaml | nindent 4 }} {{- end }} spec: - {{- if .Values.migrateDatabaseJob.ttlSecondsAfterFinished }} + {{- if not (kindIs "invalid" .Values.migrateDatabaseJob.ttlSecondsAfterFinished) }} ttlSecondsAfterFinished: {{ .Values.migrateDatabaseJob.ttlSecondsAfterFinished }} {{- end }} template: From 97daab46dbe0af29678f2f70f56474c1606ed551 Mon Sep 17 00:00:00 2001 From: Oleksandr Malyga Date: Thu, 2 Feb 2023 18:52:23 +0100 Subject: [PATCH 4/5] fixed createUserJob helm tests --- tests/charts/test_create_user_job.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/charts/test_create_user_job.py b/tests/charts/test_create_user_job.py index 0dd6c96bc6835..3ca5880fd50bf 100644 --- a/tests/charts/test_create_user_job.py +++ b/tests/charts/test_create_user_job.py @@ -243,7 +243,7 @@ def test_should_disable_custom_env(self): def test_job_ttl_after_finished(self): docs = render_chart( - values={"migrateDatabaseJob": {"ttlSecondsAfterFinished": 1}}, + values={"createUserJob": {"ttlSecondsAfterFinished": 1}}, show_only=["templates/jobs/create-user-job.yaml"], ) ttl = jmespath.search("spec.ttlSecondsAfterFinished", docs[0]) @@ -251,7 +251,7 @@ def test_job_ttl_after_finished(self): def test_job_ttl_after_finished_zero(self): docs = render_chart( - values={"migrateDatabaseJob": {"ttlSecondsAfterFinished": 0}}, + values={"createUserJob": {"ttlSecondsAfterFinished": 0}}, show_only=["templates/jobs/create-user-job.yaml"], ) ttl = jmespath.search("spec.ttlSecondsAfterFinished", docs[0]) From 317915439a53a21094b171816a88754672ea0ced Mon Sep 17 00:00:00 2001 From: Oleksandr Malyga Date: Mon, 13 Feb 2023 12:25:31 +0100 Subject: [PATCH 5/5] add default value of 300 to ttlSecondsAfterFinished --- chart/values.schema.json | 4 ++-- chart/values.yaml | 4 ++-- tests/charts/test_create_user_job.py | 5 ++++- tests/charts/test_migrate_database_job.py | 5 ++++- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/chart/values.schema.json b/chart/values.schema.json index 0cc6b4d6d903f..d2d64eb739cc1 100644 --- a/chart/values.schema.json +++ b/chart/values.schema.json @@ -2951,7 +2951,7 @@ "integer", "null" ], - "default": null + "default": 300 }, "env": { "description": "Add additional env vars to the create user job pod.", @@ -3159,7 +3159,7 @@ "integer", "null" ], - "default": null + "default": 300 } } }, diff --git a/chart/values.yaml b/chart/values.yaml index ac675f006b0ec..7ab1aff0fdf40 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -760,7 +760,7 @@ scheduler: # Airflow create user job settings createUserJob: # Limit the lifetime of the job object after it finished execution. - ttlSecondsAfterFinished: ~ + ttlSecondsAfterFinished: 300 # Command to use when running the create user job (templated). command: ~ # Args to use when running the create user job (templated). @@ -840,7 +840,7 @@ createUserJob: migrateDatabaseJob: enabled: true # Limit the lifetime of the job object after it finished execution. - ttlSecondsAfterFinished: ~ + ttlSecondsAfterFinished: 300 # Command to use when running the migrate database job (templated). command: ~ # Args to use when running the migrate database job (templated). diff --git a/tests/charts/test_create_user_job.py b/tests/charts/test_create_user_job.py index 3ca5880fd50bf..5c1e94dfd30aa 100644 --- a/tests/charts/test_create_user_job.py +++ b/tests/charts/test_create_user_job.py @@ -258,7 +258,10 @@ def test_job_ttl_after_finished_zero(self): assert ttl == 0 def test_job_ttl_after_finished_nil(self): - docs = render_chart(show_only=["templates/jobs/create-user-job.yaml"]) + docs = render_chart( + values={"createUserJob": {"ttlSecondsAfterFinished": None}}, + show_only=["templates/jobs/create-user-job.yaml"], + ) spec = jmespath.search("spec", docs[0]) assert "ttlSecondsAfterFinished" not in spec diff --git a/tests/charts/test_migrate_database_job.py b/tests/charts/test_migrate_database_job.py index e3b233c1d1c12..45a1b378df74e 100644 --- a/tests/charts/test_migrate_database_job.py +++ b/tests/charts/test_migrate_database_job.py @@ -252,7 +252,10 @@ def test_job_ttl_after_finished_zero(self): assert ttl == 0 def test_job_ttl_after_finished_nil(self): - docs = render_chart(show_only=["templates/jobs/migrate-database-job.yaml"]) + docs = render_chart( + values={"migrateDatabaseJob": {"ttlSecondsAfterFinished": None}}, + show_only=["templates/jobs/migrate-database-job.yaml"], + ) spec = jmespath.search("spec", docs[0]) assert "ttlSecondsAfterFinished" not in spec