- Notifications
You must be signed in to change notification settings - Fork1k
feat(helm/provisioner): support deploying multiple provisioners in same namespace#15637
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| # coder.serviceAccount.name -- The service account name | ||
| name:coder | ||
| # coder.serviceAccount.name -- Whether to create the service account or use existing service account | ||
| # coder.serviceAccount.disableCreate -- Whether to create the service account or use existing service account. |
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.
self review: drive-by typo fix
| pskSecretName:"coder-provisioner-psk" | ||
| keySecretName:"coder-provisionerd-key" | ||
| keySecretKey:"provisionerd-key" |
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.
self-review: advertising provisioner keys as the preferred auth method
| extraTemplates: | ||
| - | | ||
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: some-other-config | ||
| namespace: {{ .Release.Namespace }} | ||
| data: | ||
| key: some-other-value |
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.
self-review: this isn't strictly necessary, but I figured it could be a potential point of confusion, so elected to clarify it here.
| keySecretKey:"provisionerd-key" | ||
| ``` | ||
| ## Specific Examples |
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.
self-review: these examples are essentially lifted from our tests
| name: other-coder-provisioner | ||
| provisionerDaemon: | ||
| # ... | ||
| nameOverride: "other-coder-provisioner" |
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.
self-review: this setting is essentially buried inlibcoder, but it seems to be the best way to do this. Introducing a new separate variable here is just going to make things more complicated.
| {{- if not .Values.coder.serviceAccount.disableCreate }} | ||
| {{ include "libcoder.serviceaccount" (list . "coder.serviceaccount") }} | ||
| {{- end }} |
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.
self-review: this was added forhelm/coder in#14817 but not ported over here.
I'm not sure if it would be better to do it in libcoder, folks don't seem to look in there much.
| type: RuntimeDefault | ||
| volumeMounts: [] | ||
| restartPolicy: Always | ||
| serviceAccountName: other-coder-provisioner |
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.
self-review: the fact thatnameOverride has no bearing onserviceAccountName explicitly allows us to reference a pre-existing service account
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.
LGTM, but will let someone with more helm experience approve
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.
LGTM
7876dc5 intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Fixes#15437
coder.serviceAccount.disableCreate(originally added tohelm/coderinfeat(helm): add setting to disable service account creation #14817).helm/provisioner/README.mdon deploying multiple provisioners in the same namespace leveragingnameOverride.This allows us to support the following use-cases:
nameOverrideandcoder.serviceAccount.Namecoder.serviceAccount.disableCreate=true,coder.serviceAccount.workspacePerms=falseandcoder.serviceAccount.name=<name of existing sa>.Validated
nameOverrideusing kustomize: