-
Notifications
You must be signed in to change notification settings - Fork 5
Adds Postgres Manifests and removes Postgres Helm Chart dependency #151
Conversation
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.
@Abhi94N I had some minor observations but other than that things look ok.
@@ -134,8 +134,11 @@ spec: | |||
configMapKeyRef: | |||
name: hub-illumidesk-cm | |||
key: POSTGRES_NBGRADER_USER | |||
- name: POSTGRES_NBGRADER_PASSWORD | |||
value: {{ .Values.graderSetupService.postgresNBGraderPassword | default "illumidesk" }} | |||
- name: NBGRADER_PASSWORD |
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.
Our graderservice source code only understands POSTGRES_NBGRADER_PASSWORD
.
JUPYTERHUB_API_TOKEN: {{ .Values.illumideskSettings.jupyterhubAPIToken | default "" | b64enc | quote }} | ||
JUPYTERHUB_CRYPT_KEY: {{ .Values.illumideskSettings.jupyterhubCryptKey | default "" | b64enc | quote }} | ||
POSTGRES_JUPYTERHUB_PASSWORD: {{ .Values.illumideskSettings.postgresHubPassword | default "postgres" | quote}} | ||
NBGRADER_PASSWORD: {{ .Values.illumideskSettings.postgresGraderPassword | default "postgres" | quote}} |
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.
For consistency, we should change this value to POSTGRES_NBGRADER_PASSWORD
in all locations (secrets and cm's).
resources: | ||
requests: | ||
cpu: 100m | ||
memory: 100Mi |
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.
Remove requests values and set limits to have an upper bound limit. We should also surface these settings in our values.yaml
and update the README.md with these options.
valueFrom: | ||
secretKeyRef: | ||
name: hub-illumidesk-secret | ||
key: NBGRADER_PASSWORD |
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.
Change to POSTGRES_NBGRADER_PASSWORD
.
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
closes #44
mostly closes ##53 as it creates the appropriate secrets so secret values can be passed as envar references. This works for every secret value except
POSTGRES_NBGRADER_PASSWORD