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.

Sign up for GitHub

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

Jump to bottom

chore: Helm chart reconfiguration #408

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

chore: Helm chart reconfiguration #408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion charts/wadm/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type: application
# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
version: "0.2.5"
version: "0.3.0"

# This is the version number of the application being deployed. This version number should be
# incremented each time you make changes to the application. Versions are not expected to
Expand Down
4 changes: 2 additions & 2 deletions charts/wadm/ci/ct-install-values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
wadm:
config:
config:
wadm:
nats:
server: "nats.default.svc.cluster.local:4222"
16 changes: 8 additions & 8 deletions charts/wadm/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,25 +51,25 @@ app.kubernetes.io/instance: {{ .Release.Name }}
{{- end }}

{{- define "wadm.nats.auth" -}}
{{- if .Values.wadm.config.nats.creds.secretName -}}
{{- if .Values.config.wadm.nats.creds.secretName -}}
- name: WADM_NATS_CREDS_FILE
value: {{ include "wadm.nats.creds_file_path" . | quote }}
{{- else if and .Values.wadm.config.nats.creds.jwt .Values.wadm.config.nats.creds.seed -}}
{{- else if and .Values.config.wadm.nats.creds.jwt .Values.config.wadm.nats.creds.seed -}}
- name: WADM_NATS_NKEY
value: {{ .Values.wadm.config.nats.creds.seed | quote }}
value: {{ .Values.config.wadm.nats.creds.seed | quote }}
- name: WADM_NATS_JWT
value: {{ .Values.wadm.config.nats.creds.jwt | quote }}
value: {{ .Values.config.wadm.nats.creds.jwt | quote }}
{{- end }}
{{- end }}

{{- define "wadm.nats.creds_file_path" }}
{{- if .Values.wadm.config.nats.creds.secretName -}}
{{- if .Values.config.wadm.nats.creds.secretName -}}
/etc/nats-creds/nats.creds
{{- end }}
{{- end }}

{{- define "wadm.nats.creds_volume_mount" -}}
{{- if .Values.wadm.config.nats.creds.secretName -}}
{{- if .Values.config.wadm.nats.creds.secretName -}}
volumeMounts:
- name: nats-creds-secret-volume
mountPath: "/etc/nats-creds"
Expand All @@ -78,7 +78,7 @@ volumeMounts:
{{- end }}

{{- define "wadm.nats.creds_volume" -}}
{{- with .Values.wadm.config.nats.creds -}}
{{- with .Values.config.wadm.nats.creds -}}
{{- if .secretName -}}
volumes:
- name: nats-creds-secret-volume
Expand All @@ -89,4 +89,4 @@ volumes:
path: "nats.creds"
{{- end }}
{{- end }}
{{- end }}
{{- end }}
58 changes: 29 additions & 29 deletions charts/wadm/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,63 +31,63 @@ spec:
- name: {{ .Chart.Name }}
securityContext:
{{- toYaml .Values.securityContext | nindent 12 }}
image: "{{ .Values.wadm.image.repository }}:{{ .Values.wadm.image.tag | default .Chart.AppVersion }}"
imagePullPolicy: {{ .Values.wadm.image.pullPolicy }}
image: "{{ .Values.config.image.repository }}:{{ .Values.config.image.tag | default .Chart.AppVersion }}"
imagePullPolicy: {{ .Values.config.image.pullPolicy }}
env:
- name: WADM_NATS_SERVER
value: {{ .Values.wadm.config.nats.server | quote }}
value: {{ tpl .Values.config.wadm.nats.server . | quote }}
{{- include "wadm.nats.auth" . | nindent 12 }}
{{- if .Values.wadm.config.nats.tlsCaFile }}
{{- if .Values.config.wadm.nats.tlsCaFile }}
- name: WADM_NATS_TLS_CA_FILE
value: {{ .Values.wadm.config.nats.tlsCaFile | quote }}
value: {{ .Values.config.wadm.nats.tlsCaFile | quote }}
{{- end }}
{{- if .Values.wadm.config.hostId }}
{{- if .Values.config.wadm.hostId }}
- name: WADM_HOST_ID
value: {{ .Values.wadm.config.hostId | quote }}
value: {{ .Values.config.wadm.hostId | quote }}
{{- end }}
{{- if .Values.wadm.config.structuredLogging }}
{{- if .Values.config.wadm.structuredLogging }}
- name: WADM_STRUCTURED_LOGGING
value: {{ .Values.wadm.config.structuredLogging | quote }}
value: {{ .Values.config.wadm.structuredLogging | quote }}
{{- end }}
{{- if .Values.wadm.config.tracing }}
{{- if .Values.config.wadm.tracing }}
- name: WADM_TRACING_ENABLED
value: {{ .Values.wadm.config.tracing | quote }}
value: {{ .Values.config.wadm.tracing | quote }}
{{- end }}
{{- if .Values.wadm.config.tracingEndpoint }}
{{- if .Values.config.wadm.tracingEndpoint }}
- name: WADM_TRACING_ENDPOINT
value: {{ .Values.wadm.config.tracingEndpoint | quote }}
value: {{ .Values.config.wadm.tracingEndpoint | quote }}
{{- end }}
{{- if .Values.wadm.config.jetstreamDomain }}
{{- if .Values.config.wadm.jetstreamDomain }}
- name: WADM_JETSTREAM_DOMAIN
value: {{ .Values.wadm.config.jetstreamDomain | quote }}
value: {{ .Values.config.wadm.jetstreamDomain | quote }}
{{- end }}
{{- if .Values.wadm.config.maxJobs }}
{{- if .Values.config.wadm.maxJobs }}
- name: WADM_MAX_JOBS
value: {{ .Values.wadm.config.maxJobs }}
value: {{ .Values.config.wadm.maxJobs }}
{{- end }}
{{- if .Values.wadm.config.stateBucket }}
{{- if .Values.config.wadm.stateBucket }}
- name: WADM_STATE_BUCKET_NAME
value: {{ .Values.wadm.config.stateBucket | quote }}
value: {{ .Values.config.wadm.stateBucket | quote }}
{{- end }}
{{- if .Values.wadm.config.manifestBucket }}
{{- if .Values.config.wadm.manifestBucket }}
- name: WADM_MANIFEST_BUCKET_NAME
value: {{ .Values.wadm.config.manifestBucket | quote }}
value: {{ .Values.config.wadm.manifestBucket | quote }}
{{- end }}
{{- if .Values.wadm.config.cleanupInterval }}
{{- if .Values.config.wadm.cleanupInterval }}
- name: WADM_CLEANUP_INTERVAL
value: {{ .Values.wadm.config.cleanupInterval }}
value: {{ .Values.config.wadm.cleanupInterval }}
{{- end }}
{{- if .Values.wadm.config.apiPrefix }}
{{- if .Values.config.wadm.apiPrefix }}
- name: WADM_API_PREFIX
value: {{ .Values.wadm.config.apiPrefix }}
value: {{ .Values.config.wadm.apiPrefix }}
{{- end }}
{{- if .Values.wadm.config.streamPrefix }}
{{- if .Values.config.wadm.streamPrefix }}
- name: WADM_STREAM_PREFIX
value: {{ .Values.wadm.config.streamPrefix }}
value: {{ .Values.config.wadm.streamPrefix }}
{{- end }}
{{- if .Values.wadm.config.multitenant }}
{{- if .Values.config.wadm.multitenant }}
- name: WADM_MULTITENANT
value: {{ .Values.wadm.config.multitenant | quote }}
value: {{ .Values.config.wadm.multitenant | quote }}
{{- end }}
resources:
{{- toYaml .Values.resources | nindent 12 }}
Expand Down
11 changes: 6 additions & 5 deletions charts/wadm/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
wadm:
config:
# replicas represents the number of copies of wadm to run
replicas: 1
# image represents the image and tag for running wadm
Expand All @@ -7,7 +7,7 @@ wadm:
pullPolicy: IfNotPresent
# Overrides the image tag whose default is the chart appVersion.
tag: ""
config:
wadm:
apiPrefix: ""
streamPrefix: ""
cleanupInterval: ""
Expand Down Expand Up @@ -48,7 +48,8 @@ serviceAccount:
podAnnotations: {}
podLabels: {}

podSecurityContext: {}
podSecurityContext:
{}
Comment on lines +51 to +52
Copy link
Member

@joonas joonas Sep 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Would you mind reverting this so it's in line with the rest of the map-based entries?

Or alternatively would you mind going ahead and changing the rest of the empty map entries to match this style?

I'm not sure which format should be preferred to be honest, though Helm defaults to podSecurityContext: {} over this

# fsGroup: 1000

securityContext:
Expand All @@ -62,8 +63,8 @@ securityContext:
seccompProfile:
type: "RuntimeDefault"


resources: {}
resources:
{}
Comment on lines +66 to +67
Copy link
Member

@joonas joonas Sep 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Would you mind reverting this so it's in line with the rest of the map-based entries?

Copy link
Member Author

Choose a reason for hiding this comment

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

no problem. heads up this is literally the output from yaml linter

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough, it looks like it doesn't touch these values when they are grouped next to other values, so maybe it's worth pulling them up into a group above?

Strange behavior on the linter's part to be honest, but what can you do 🙂

# We usually recommend not to specify default resources and to leave this as a conscious
# choice for the user. This also increases chances charts run on environments with little
# resources, such as Minikube. If you do want to specify resources, uncomment the following
Expand Down