-
Notifications
You must be signed in to change notification settings - Fork 151
Optionally expose node locality (region, zone, host) to pod #134
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
base: master
Are you sure you want to change the base?
Optionally expose node locality (region, zone, host) to pod #134
Conversation
2dd218d to
391d5b6
Compare
391d5b6 to
c27da88
Compare
|
Rebased |
c27da88 to
b9946d2
Compare
b9946d2 to
f6ca266
Compare
|
Rebased |
| {{- with .Values.labels }} | ||
| {{- toYaml . | nindent 4 }} | ||
| {{- end }} | ||
| annotations: |
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.
I don't like this approach, helm hooks annotations should always be present or someone who wants to add new annotations will override helms one
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.
You can add something like extraJobAnnotations and list them after helm hooks
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.
Hi! The purpose of this was to make it possible to remove the annotations.
I've moved this discussion into a new PR: #206
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.
Got it. Thanks!
f6ca266 to
3c92778
Compare
|
Rebased PR |
Discussion: Has this been investigated before? I imagine that most people want these locality tags in multi-region setups. This PR adds them from well-known node labels. I'm happy to transfer ownership of my proof-of-concept
initContainerthat mounts the node labels in the statefulset.TLDR: This pullrequest automatically adds the CLI start flag values