-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(controller): default volume/mount to emissary. Fixes #7118 #7125
Conversation
Signed-off-by: Tianchu Zhao <evantczhao@gmail.com>
volumes = append(volumes, woc.getVolumeDockerSock(tmpl)) | ||
default: | ||
volumes = append(volumes, volumeVarArgo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if I may (been working on #6714 so I saw this too): this changes behaviour because Go's switch-case doesn't fallthrough by default. Before only emissary had /var/run/argo as volume, with this change all except Docker have it as volume.
This changes anyway in #6714 (all will have that volume + mount) so I'm not sure how important it is to get the correct behaviour here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct me if I'm wrong. Because go doesn't fall through by default, we have the following and the behaviour is expected
pns/k8sapi/kubelet:
do nothing
docker:
attach dockerSock
emissary or empty (default):
attach varArgo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aah, right. My bad. Sorry about the confusion.
Signed-off-by: Tianchu Zhao <evantczhao@gmail.com>
This reverts commit 49b3f0c. Signed-off-by: Alex Collins <alex_collins@intuit.com>
fixes #7118
Don't bother creating a PR until you've done this:
make pre-commit -B
to fix codegen, lint, and commit message problems.Create your PR as a draft.
does not need to pass.
Tips: