jedcunningham commented on code in PR #22913:
URL: https://github.com/apache/airflow/pull/22913#discussion_r848528864
##########
chart/templates/scheduler/scheduler-deployment.yaml:
##########
@@ -243,10 +243,10 @@ spec:
{{- else if .Values.dags.gitSync.enabled }}
- name: dags
emptyDir: {}
- {{- end }}
{{- if and .Values.dags.gitSync.enabled
.Values.dags.gitSync.sshKeySecret }}
Review Comment:
```suggestion
{{- if .Values.dags.gitSync.sshKeySecret }}
```
We are already in a gitsync enabled block.
##########
tests/charts/test_git_sync_scheduler.py:
##########
@@ -131,6 +131,24 @@ def test_validate_if_ssh_params_are_added(self):
"secret": {"secretName": "ssh-secret", "defaultMode": 288},
} in jmespath.search("spec.template.spec.volumes", docs[0])
+ def test_validate_sshkeysecret_not_added_when_persistence_is_enabled(self):
+ docs = render_chart(
+ values={
+ "dags": {
+ "gitSync": {
+ "enabled": True,
+ "containerName": "git-sync-test",
+ "sshKeySecret": "ssh-secret",
+ "knownHosts": None,
+ "branch": "test-branch",
+ },
+ "persistence": {"enabled": True},
+ }
+ },
+ show_only=["templates/scheduler/scheduler-deployment.yaml"],
+ )
+ assert {"name": "git-sync-ssh-key"} not in
jmespath.search("spec.template.spec.volumes", docs[0])
Review Comment:
I'm not sure this test is right. It should be here for scheduler, no?
Also, this might be a better way to test:
```suggestion
assert "git-sync-ssh-key" not in
jmespath.search("spec.template.spec.volumes[].name", docs[0])
```
Basically, get the names of the volumes and make sure it's there or not.
This passes because the sshkey volume has more than just the name in it.
--
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]
