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

Feat: add helm chart #831

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Feat: add helm chart #831

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5ccd9e5
add helm chart
didlawowo Oct 25, 2024
2668200
Merge branch 'main' into feat/helm-chart
didlawowo Oct 29, 2024
35cd368
Merge branch 'main' into feat/helm-chart
didlawowo Nov 13, 2024
ba1a7ef
Merge branch 'main' into feat/helm-chart
didlawowo Nov 15, 2024
59a520e
Merge branch 'main' into feat/helm-chart
didlawowo Dec 22, 2024
ee8b360
Merge branch 'main' into feat/helm-chart
didlawowo Dec 26, 2024
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
6 changes: 6 additions & 0 deletions deployment/helm/Chart.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
dependencies:
- name: postgresql
repository: https://charts.bitnami.com/bitnami
version: 16.0.3
digest: sha256:60749d8dc7032d9cc478a2141c05d4ba31dc9e7fea45a3c4f91471eb11f375a9
generated: "2024-10-16T21:35:01.311619+02:00"
10 changes: 10 additions & 0 deletions deployment/helm/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
apiVersion: v2
name: wren
type: application
version: 1.0.0

dependencies:
- name: postgresql
version: "=16.0.3"
repository: "https://charts.bitnami.com/bitnami"
condition: postgres.enabled
Binary file added deployment/helm/charts/postgresql-16.0.3.tgz
View file
Open in desktop
Binary file not shown.
54 changes: 54 additions & 0 deletions deployment/helm/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
{{/*
Expand the name of the chart.
*/}}
{{- define "application.name" -}}
{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" -}}
{{- end -}}

{{/*
Create a default fully qualified app name.
We truncate at 63 chars because some Kubernetes name fields are limited to this (by the DNS naming spec).
*/}}
{{- define "application.fullname" -}}
{{- if .Values.fullnameOverride -}}
{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" -}}
{{- else -}}
{{- $name := default .Chart.Name .Values.nameOverride -}}
{{- if contains $name .Release.Name -}}
{{- .Release.Name | trunc 63 | trimSuffix "-" -}}
{{- else -}}
{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" -}}
{{- end -}}
{{- end -}}
{{- end -}}

{{/*
Create chart name and version as used by the chart label.
*/}}
{{- define "application.chart" -}}
{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" -}}
{{- end -}}

{{/*
Generate labels.
*/}}
{{- define "application.labels" -}}
helm.sh/chart: {{ template "application.chart" . }}
{{ include "application.selectorLabels" . }}
app.kubernetes.io/part-of: {{ template "application.name" . }}
app.kubernetes.io/managed-by: {{ .Release.Service }}
{{- if .Chart.AppVersion }}
app.kubernetes.io/version: {{ .Chart.AppVersion }}
{{- end }}
{{- if .Values.commonLabels}}
{{ toYaml .Values.commonLabels }}
{{- end }}
{{- end }}

{{/*
Selector labels.
*/}}
{{- define "application.selectorLabels" -}}
app.kubernetes.io/name: {{ template "application.name" . }}
app.kubernetes.io/instance: {{ .Release.Name }}
{{- end }}
36 changes: 36 additions & 0 deletions deployment/helm/templates/configmaps.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{{/* Create configMaps from .env files (convert key=value to key: value) */}}
{{- range $dotenv := .Values.configMaps.dotenv }}
---
apiVersion: v1
kind: ConfigMap
metadata:
name: {{ $dotenv.name }}
labels: {{- include "application.labels" $ | nindent 4 }}
annotations:
helm.sh/hook: pre-install, pre-upgrade
helm.sh/hook-weight: "-3"
checksum/config: {{ (tpl ($.Files.Glob $dotenv.path).AsConfig $ ) | sha256sum }}
namespace: {{ $.Release.Namespace }}
data:
{{- range $.Files.Lines $dotenv.path }}
{{- if . }}
{{ (splitList "=" . | first ) }}: {{ (splitList "=" . | rest | join "=") | trim | quote }}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix potential issue with environment values containing '=' characters

The current implementation might break for environment values containing '=' characters. Consider using a more robust parsing approach.

-  {{ (splitList "=" . | first ) }}: {{ (splitList "=" . | rest | join "=") | trim | quote }}
+  {{- $parts := splitList "=" . -}}
+  {{- if gt (len $parts) 1 -}}
+  {{ first $parts }}: {{ rest $parts | join "=" | trim | quote }}
+  {{- end -}}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{{ (splitList "=" . | first ) }}: {{ (splitList "=" . | rest | join "=") | trim | quote }}
{{- $parts := splitList "=" . -}}
{{- if gt (len $parts) 1 -}}
{{ first $parts }}: {{ rest $parts | join "=" | trim | quote }}
{{- end -}}

{{- end }}
{{- end }}
Comment on lines +15 to +19
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add validation for required files

Add validation to ensure the specified files exist before attempting to read them.

+{{- if not ($.Files.Glob $dotenv.path) }}
+  {{- fail (printf "ConfigMap error: File %s not found" $dotenv.path) }}
+{{- end }}
 {{- range $.Files.Lines $dotenv.path }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{{- range $.Files.Lines $dotenv.path }}
{{- if . }}
{{ (splitList "=" . | first ) }}: {{ (splitList "=" . | rest | join "=") | trim | quote }}
{{- end }}
{{- end }}
{{- if not ($.Files.Glob $dotenv.path) }}
{{- fail (printf "ConfigMap error: File %s not found" $dotenv.path) }}
{{- end }}
{{- range $.Files.Lines $dotenv.path }}
{{- if . }}
{{ (splitList "=" . | first ) }}: {{ (splitList "=" . | rest | join "=") | trim | quote }}
{{- end }}
{{- end }}

{{- end }}
{{/* Create configMaps from files */}}
{{- range $file := .Values.configMaps.files }}
---
apiVersion: v1
kind: ConfigMap
metadata:
name: {{ $file.name }}
labels: {{- include "application.labels" $ | nindent 4 }}
annotations:
helm.sh/hook: pre-install, pre-upgrade
helm.sh/hook-weight: "-3"
checksum/config: {{ (tpl ($.Files.Glob $file.path).AsConfig $ ) | sha256sum }}
namespace: {{ $.Release.Namespace }}
data:
{{ ($.Files.Glob $file.path).AsConfig | indent 2 }}
{{- end }}
88 changes: 88 additions & 0 deletions deployment/helm/templates/deployment-api.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
---
apiVersion: apps/v1
kind: Deployment
metadata:
name: {{ template "application.fullname" . }}-api
labels: {{- include "application.labels" . | nindent 4 }}
namespace: {{ .Release.Namespace }}
spec:
replicas: {{ .Values.application.api.replicas }}
selector:
matchLabels:
app: wren-api
strategy: {{ toYaml .Values.strategy | nindent 4 }}
template:
metadata:
labels:
app: wren-api
release: {{ .Release.Name }}
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
heritage: "{{ .Release.Service }}"
annotations:
{{- with .Values.podAnnotations }}
{{- toYaml . | nindent 8 }}
{{- end }}
{{- range $dotenv := .Values.secrets.dotenv }}
checksum/secret-{{ $dotenv.name }}: {{ (tpl ($.Files.Glob $dotenv.path).AsSecrets $ ) | sha256sum }}
{{- end }}
{{- range $dotenv := .Values.configMaps.dotenv }}
checksum/config-{{ $dotenv.name }}: {{ (tpl ($.Files.Glob $dotenv.path).AsConfig $ ) | sha256sum }}
{{- end }}
spec:
serviceAccountName: {{ default (include "application.fullname" .) .Values.serviceAccount.name }}
{{- with .Values.initContainers }}
initContainers: {{- toYaml . | nindent 8 }}
{{- end }}
runtimeClassName: nvidia
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Make runtime class configurable

The runtime class is hardcoded to "nvidia", which may not be available in all environments. Consider making it configurable through values.yaml.

-      runtimeClassName: nvidia
+      {{- with .Values.application.api.runtimeClassName }}
+      runtimeClassName: {{ . }}
+      {{- end }}

Committable suggestion skipped: line range outside the PR's diff.

containers:
- name: {{ default .Values.application.api.containerName }}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix container name template syntax

The default function requires two arguments: a default value and the value to check. The current syntax is incorrect.

-        - name: {{ default .Values.application.api.containerName  }}
+        - name: {{ .Values.application.api.containerName | default "wren-api" }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: {{ default .Values.application.api.containerName }}
- name: {{ .Values.application.api.containerName | default "wren-api" }}

image: {{ .Values.application.api.image.repository }}:{{ .Values.application.api.image.tag | default .Chart.AppVersion }}
imagePullPolicy: {{ .Values.application.api.image.pullPolicy }}
{{- with .Values.application.api.ports }}
ports: {{- toYaml . | nindent 12 }}
{{- end }}

{{- with .Values.application.api.command }}
command: {{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.application.api.args }}
args: {{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.resources.api }}
resources: {{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.env }}
env: {{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.envFrom }}
envFrom: {{- toYaml . | nindent 12 }}
{{- end }}

{{- with .Values.livenessProbe }}
livenessProbe:
httpGet:
path: {{ .httpGet.path }}
port: {{ .httpGet.port }}
scheme: {{ .httpGet.scheme }}
initialDelaySeconds: {{ .initialDelaySeconds }}
periodSeconds: {{ .periodSeconds }}
timeoutSeconds: {{ .timeoutSeconds }}
successThreshold: {{ .successThreshold }}
failureThreshold: {{ .failureThreshold }}
{{- end }}
{{- with .Values.readinessProbe }}
readinessProbe:
httpGet:
path: {{ .httpGet.path }}
port: {{ .httpGet.port }}
scheme: {{ .httpGet.scheme }}
initialDelaySeconds: {{ .initialDelaySeconds }}
periodSeconds: {{ .periodSeconds }}
timeoutSeconds: {{ .timeoutSeconds }}
successThreshold: {{ .successThreshold }}
failureThreshold: {{ .failureThreshold }}
{{- end }}

{{- with .Values.nodeSelector }}
nodeSelector: {{- toYaml . | nindent 8 }}
{{- end }}
90 changes: 90 additions & 0 deletions deployment/helm/templates/deployment-engine.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
---
apiVersion: apps/v1
kind: Deployment
metadata:
name: {{ template "application.fullname" . }}-engine
labels: {{- include "application.labels" . | nindent 4 }}
namespace: {{ .Release.Namespace }}
spec:
replicas: {{ .Values.application.engine.replicas }}
selector:
matchLabels:
app: wren-engine
strategy: {{ toYaml .Values.strategy | nindent 4 }}
template:
metadata:
labels:
app: wren-engine
release: {{ .Release.Name }}
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
heritage: "{{ .Release.Service }}"
annotations:
{{- with .Values.podAnnotations }}
{{- toYaml . | nindent 8 }}
{{- end }}
{{- range $dotenv := .Values.secrets.dotenv }}
checksum/secret-{{ $dotenv.name }}: {{ (tpl ($.Files.Glob $dotenv.path).AsSecrets $ ) | sha256sum }}
{{- end }}
{{- range $dotenv := .Values.configMaps.dotenv }}
checksum/config-{{ $dotenv.name }}: {{ (tpl ($.Files.Glob $dotenv.path).AsConfig $ ) | sha256sum }}
{{- end }}
spec:
serviceAccountName: {{ default (include "application.fullname" .) .Values.serviceAccount.name }}
{{- with .Values.application.engine.initContainers }}
initContainers: {{- toYaml . | nindent 8 }}
{{- end }}
containers:
- name: {{ default .Values.application.engine.containerName }}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix container name template syntax

The default function requires two arguments: a default value and the value to check. The current syntax is incorrect.

-        - name: {{ default .Values.application.engine.containerName   }}
+        - name: {{ .Values.application.engine.containerName | default "wren-engine" }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: {{ default .Values.application.engine.containerName }}
- name: {{ .Values.application.engine.containerName | default "wren-engine" }}

image: {{ .Values.application.engine.image.repository }}:{{ .Values.application.engine.image.tag | default .Chart.AppVersion }}
imagePullPolicy: {{ .Values.application.engine.image.pullPolicy }}
{{- with .Values.application.engine.ports }}
ports: {{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.application.engine.command }}
command: {{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.application.engine.args }}
args: {{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.resources.engine }}
resources: {{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.env }}
env: {{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.envFrom }}
envFrom: {{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.volumeMounts }}
volumeMounts: {{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.livenessProbe }}
livenessProbe:
httpGet:
path: {{ .httpGet.path }}
port: {{ .httpGet.port }}
scheme: {{ .httpGet.scheme }}
initialDelaySeconds: {{ .initialDelaySeconds }}
periodSeconds: {{ .periodSeconds }}
timeoutSeconds: {{ .timeoutSeconds }}
successThreshold: {{ .successThreshold }}
failureThreshold: {{ .failureThreshold }}
{{- end }}
Comment on lines +61 to +72
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Customize health check configurations

The health check configurations are currently shared across all components using .Values.livenessProbe and .Values.readinessProbe. Each component should have its own health check configuration.

-          {{- with .Values.livenessProbe }}
+          {{- with .Values.application.api.livenessProbe }}
-          {{- with .Values.readinessProbe }}
+          {{- with .Values.application.api.readinessProbe }}

Apply similar changes to frontend and engine deployments.

Also applies to: 73-84

{{- with .Values.readinessProbe }}
readinessProbe:
httpGet:
path: {{ .httpGet.path }}
port: {{ .httpGet.port }}
scheme: {{ .httpGet.scheme }}
initialDelaySeconds: {{ .initialDelaySeconds }}
periodSeconds: {{ .periodSeconds }}
timeoutSeconds: {{ .timeoutSeconds }}
successThreshold: {{ .successThreshold }}
failureThreshold: {{ .failureThreshold }}
{{- end }}
{{- with .Values.volumes }}
volumes: {{- toYaml . | nindent 8 }}
{{- end }}
{{- with .Values.nodeSelector }}
nodeSelector: {{- toYaml . | nindent 8 }}
{{- end }}
86 changes: 86 additions & 0 deletions deployment/helm/templates/deployment-frontend.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
---
apiVersion: apps/v1
kind: Deployment
metadata:
name: {{ template "application.fullname" . }}-frontend
labels: {{- include "application.labels" . | nindent 4 }}
namespace: {{ .Release.Namespace }}
spec:
replicas: {{ .Values.application.frontend.replicas }}
selector:
matchLabels:
app: wren-frontend
strategy: {{ toYaml .Values.strategy | nindent 4 }}
template:
metadata:
labels:
app: wren-frontend
release: {{ .Release.Name }}
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
heritage: "{{ .Release.Service }}"
annotations:
{{- with .Values.podAnnotations }}
{{- toYaml . | nindent 8 }}
{{- end }}
{{- range $dotenv := .Values.secrets.dotenv }}
checksum/secret-{{ $dotenv.name }}: {{ (tpl ($.Files.Glob $dotenv.path).AsSecrets $ ) | sha256sum }}
{{- end }}
{{- range $dotenv := .Values.configMaps.dotenv }}
checksum/config-{{ $dotenv.name }}: {{ (tpl ($.Files.Glob $dotenv.path).AsConfig $ ) | sha256sum }}
{{- end }}
spec:
serviceAccountName: {{ default (include "application.fullname" .) .Values.serviceAccount.name }}
{{- with .Values.initContainers }}
initContainers: {{- toYaml . | nindent 8 }}
{{- end }}
Comment on lines +31 to +35
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add security context for frontend pod

Frontend containers should run with restricted permissions. Consider adding a security context configuration.

    spec:
      serviceAccountName: {{ default (include "application.fullname" .) .Values.serviceAccount.name }}
+     securityContext:
+       runAsNonRoot: true
+       runAsUser: 1000
+       fsGroup: 1000
      {{- with .Values.initContainers }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
spec:
serviceAccountName: {{ default (include "application.fullname" .) .Values.serviceAccount.name }}
{{- with .Values.initContainers }}
initContainers: {{- toYaml . | nindent 8 }}
{{- end }}
spec:
serviceAccountName: {{ default (include "application.fullname" .) .Values.serviceAccount.name }}
securityContext:
runAsNonRoot: true
runAsUser: 1000
fsGroup: 1000
{{- with .Values.initContainers }}
initContainers: {{- toYaml . | nindent 8 }}
{{- end }}

containers:
- name: {{ default .Values.application.frontend.containerName }}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix container name template syntax

The default function requires two arguments: a default value and the value to check. The current syntax is incorrect.

-        - name: {{ default .Values.application.frontend.containerName   }}
+        - name: {{ .Values.application.frontend.containerName | default "wren-frontend" }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: {{ default .Values.application.frontend.containerName }}
- name: {{ .Values.application.frontend.containerName | default "wren-frontend" }}

image: {{ .Values.application.frontend.image.repository }}:{{ .Values.application.frontend.image.tag | default .Chart.AppVersion }}
imagePullPolicy: {{ .Values.application.frontend.image.pullPolicy }}
{{- with .Values.application.frontend.ports }}
ports: {{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.application.frontend.command }}
command: {{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.application.frontend.args }}
args: {{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.resources.frontend }}
resources: {{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.env }}
env: {{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.envFrom }}
envFrom: {{- toYaml . | nindent 12 }}
{{- end }}

{{- with .Values.livenessProbe }}
livenessProbe:
httpGet:
path: {{ .httpGet.path }}
port: {{ .httpGet.port }}
scheme: {{ .httpGet.scheme }}
initialDelaySeconds: {{ .initialDelaySeconds }}
periodSeconds: {{ .periodSeconds }}
timeoutSeconds: {{ .timeoutSeconds }}
successThreshold: {{ .successThreshold }}
failureThreshold: {{ .failureThreshold }}
{{- end }}
{{- with .Values.readinessProbe }}
readinessProbe:
httpGet:
path: {{ .httpGet.path }}
port: {{ .httpGet.port }}
scheme: {{ .httpGet.scheme }}
initialDelaySeconds: {{ .initialDelaySeconds }}
periodSeconds: {{ .periodSeconds }}
timeoutSeconds: {{ .timeoutSeconds }}
successThreshold: {{ .successThreshold }}
failureThreshold: {{ .failureThreshold }}
{{- end }}

{{- with .Values.nodeSelector }}
nodeSelector: {{- toYaml . | nindent 8 }}
{{- end }}
Loading