Skip to content
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

Added Annotations section to Deployment template #142

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

VF-mbrauer
Copy link

To make it possible to have an annotations section defined which we can fill with data. Right now it is static data and just filled with checksum/config:
This makes it more flexible to give also other customized key/value pairs defined there.

To make it possible to have an annotations section defined which we can fill with data. Right now it is static data and just filled with `checksum/config:`
This makes it more flexible to give also other customized key/value pairs defined there.
@nabadger nabadger self-requested a review October 23, 2020 07:34
@@ -29,6 +29,9 @@ spec:
release: {{ .Release.Name }}
annotations:
checksum/config: {{ include (print $.Template.BasePath "/configmap.yaml") . | sha256sum }}
{{- with .Values.annotations }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we call this podAnnotations - since these are only applied to the pod-spec and not top-level deployment.

In addition could we provide defaults in values.yaml to make it clear it's an option, someting like:

podAnnotations: { }

nodeSelector: {}

tolerations: []

affinity: {}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @nabadger, sorry for my late reply. Yes we could do that exactly as you described. So podAnnotations is fine as well as the definition of `podAnnotations: { } in the value file.

@nabadger
Copy link
Contributor

@VF-mbrauer - just wondering if you saw my reply or not, and whether these changes are still required?

Thanks

jengo added a commit to jengo/dex-k8s-authenticator that referenced this pull request Feb 12, 2021
@jengo
Copy link

jengo commented Feb 23, 2021

I would personally like to see this merged in. I need annotations to allow connecting to hashicorp vault.

@VF-mbrauer
Copy link
Author

@VF-mbrauer - just wondering if you saw my reply or not, and whether these changes are still required?

Thanks

Hi @nabadger, apologize for the late reply. I found also another way of implementation on our side via Admission Controller functionality, but I still see this feature as important as for users not implementing this that way I mentioned. Therefore this should be implemented the way we discussed already. Thanks.

@nabadger
Copy link
Contributor

nabadger commented Mar 5, 2021

@VF-mbrauer would you be able to rename this to podAnnotations? If not I can do this in a different MR and merge it in :)

@VF-mbrauer
Copy link
Author

Hi @nabadger, yes of course. Feel free to amend for me and get it merged. Then we can finally close this thread. Thanks.

@VF-mbrauer
Copy link
Author

@nabadger Any news here. Did you amend and merged that in the meantime?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants