Added an option to disable namespace-metadata#11782
Conversation
Signed-off-by: shinigami-777 <[email protected]>
Signed-off-by: shinigami-777 <[email protected]>
shinigami-777
changed the title
|
Providing namespace-metadata disable option #11585 |
alpeb
left a comment
alpeb
left a comment
There was a problem hiding this comment.
Hi @shinigami-777 , thanks for the contribution. Please add a comment to this PR describing your change, and a reference to the issue it fixes. Also make sure you test your change before submitting, to validate it addresses the issue.
jaeger/charts/linkerd-jaeger/templates/namespace-metadata-rbac.yaml
Outdated
Show resolved
Hide resolved
jaeger/charts/linkerd-jaeger/templates/namespace-metadata-rbac.yaml
Outdated
Show resolved
Hide resolved
Signed-off-by: shinigami-777 <[email protected]>
Signed-off-by: shinigami-777 <[email protected]>
alpeb
left a comment
alpeb
left a comment
There was a problem hiding this comment.
I've added a suggestion for a different naming of this new setting. After applying that, please run ./bin/helm-docs to regenerate the helm values documentation in the charts README.md files.
| # -- adding an option to disble namespace-metadata jobs | ||
| skipNamespaceMetadataJobs: false |
There was a problem hiding this comment.
| # -- adding an option to disble namespace-metadata jobs | |
| skipNamespaceMetadataJobs: false | |
| # -- Creates a Job that adds necessary metadata to the extension's namespace | |
| # during install; disable if lack of privileges require doing this manually | |
| createNamespaceMetadataJobs: true |
There was a problem hiding this comment.
Thanks for the suggestion @alpeb .I have made the suggested changes and regenerated the helm values documentation in the charts README.md files.
…ntation in charts README.md files Signed-off-by: shinigami-777 <[email protected]>
alpeb
left a comment
alpeb
left a comment
There was a problem hiding this comment.
Thanks @shinigami-777 , this looks good to me. I'd like however to amend my previous comment on naming: can you rename the setting createNamespaceMetadataJob, given that it's just one job (per extension)? Sorry for the nit picking!
| - kind: ServiceAccount | ||
| name: namespace-metadata | ||
| namespace: {{.Release.Namespace}} | ||
| {{-end}} |
There was a problem hiding this comment.
You require spaces here or else the template breaks, as you can see in the integration tests results.
| {{-end}} | |
| {{- end }} |
There was a problem hiding this comment.
Thanks @alpeb . Added spaces between those end statements.
There was a problem hiding this comment.
Thanks, can you address my comment about naming above? After that, this PR looks fine from my perspective.
Signed-off-by: shinigami-777 <[email protected]>
Signed-off-by: shinigami-777 <[email protected]>
Yes I have renamed the setting |
I still see |
I can't see the |
|
I meant to say the setting should be |
Signed-off-by: shinigami-777 <[email protected]>
Yes I have updated it to |
alpeb
left a comment
alpeb
left a comment
There was a problem hiding this comment.
Thanks @shinigami-777 , this looks good to me!
Thanks @alpeb for the guidance. |
3 participants
Fixes #11585
Added option in
values.yamlin extensions charts to disable the namspace-metadata jobs for helm based installations.The
createNamespaceMetadataJobflag should be set to false from cli to disable it. Disable if lack of privileges require doing it manually.