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.

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 reloader to operator-wandb, add reloader annotations to wan… #356

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
1 change: 1 addition & 0 deletions .github/workflows/pr-release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ jobs:
run: |
helm repo add bitnami https://charts.bitnami.com/bitnami
helm repo add prometheus-community https://prometheus-community.github.io/helm-charts
helm repo add stakater https://stakater.github.io/stakater-charts
helm repo update

- name: Build deps install script
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ jobs:
run: |
helm repo add bitnami https://charts.bitnami.com/bitnami
helm repo add prometheus-community https://prometheus-community.github.io/helm-charts
helm repo add stakater https://stakater.github.io/stakater-charts
helm repo update

- name: Build deps install script
Expand Down
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,6 @@ license.txt
test-values.yaml
.DS_Store
secret.*.yaml
.idea/
.idea/

tilt-settings.json
64 changes: 64 additions & 0 deletions Tiltfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
#default values
settings = {
"allowedContexts": [
"docker-desktop",
"minikube",
"kind-kind",
],
"installMinio": True,
"values": "./test-configs/operator-wandb/default.yaml",
}

# global settings
settings.update(read_json(
"tilt-settings.json",
default={},
))

if k8s_context() in settings.get("allowedContexts"):
print("Context is allowed")
else:
fail("Selected context is not in allow list")

allow_k8s_contexts(settings.get("allowed_k8s_contexts"))

if settings.get("installMinio"):
k8s_yaml('./test-configs/minio/default.yaml')
k8s_resource(
'minio',
'Minio',
objects=[
'minio:service',
'minio:namespace'
]
)

k8s_yaml(helm('./charts/operator-wandb', 'wandb', values=['./charts/operator-wandb/values.yaml', settings.get("values")]))
k8s_resource('wandb-app', port_forwards=8080, objects=['wandb-app:ServiceAccount:default', 'wandb-app-config:secret:default'])
k8s_resource('wandb-console', port_forwards=8082, objects=['wandb-console:ServiceAccount:default', 'wandb-console:clusterrole:default', 'wandb-console:clusterrolebinding:default'])
k8s_resource('wandb-otel-daemonset',objects=['wandb-otel-daemonset:ServiceAccount:default', 'wandb-otel-daemonset:clusterrole:default', 'wandb-otel-daemonset:clusterrolebinding:default'])
k8s_resource('wandb-parquet',objects=['wandb-parquet:ServiceAccount:default'])
k8s_resource('wandb-prometheus-server',objects=['wandb-prometheus-server:ServiceAccount:default', 'wandb-prometheus-server:clusterrole:default', 'wandb-prometheus-server:clusterrolebinding:default'])
k8s_resource('wandb-redis-master',objects=['wandb-redis-master:ServiceAccount:default'])
k8s_resource('wandb-weave',objects=['wandb-weave:ServiceAccount:default'])
k8s_resource(
new_name='wandb-configs',
objects=[
'wandb-bucket-configmap:configmap:default',
'wandb-bucket:secret:default',
'wandb-ca-certs:configmap:default',
'wandb-clickhouse:secret:default',
'wandb-global-secret:secret:default',
'wandb-glue-configmap:configmap:default',
'wandb-glue-secret:secret:default',
'wandb-gorilla-configmap:configmap:default',
'wandb-gorilla-secret:secret:default',
'wandb-kafka-configmap:configmap:default',
'wandb-kafka:secret:default',
'wandb-mysql-configmap:configmap:default',
'wandb-redis-configmap:configmap:default',
'wandb-redis-secret:secret:default',
]
)
k8s_resource(new_name='DO NOT REFRESH THESE', objects=['wandb-mysql:secret:default', 'wandb-gorilla-session-key:secret:default'])
Copy link

@coderabbitai coderabbitai bot Feb 28, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Clarify "DO NOT REFRESH THESE" resource

The resource name "DO NOT REFRESH THESE" is concerning as it suggests special handling requirements. Add a comment explaining why these resources should not be refreshed.

-k8s_resource(new_name='DO NOT REFRESH THESE', objects=['wandb-mysql:secret:default', 'wandb-gorilla-session-key:secret:default'])
+# These secrets contain sensitive data that should be preserved between refreshes
+# Refreshing them could cause data loss or service disruption
+k8s_resource(new_name='DO NOT REFRESH THESE', objects=['wandb-mysql:secret:default', 'wandb-gorilla-session-key:secret:default'])
📝 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
k8s_resource(new_name='DO NOT REFRESH THESE', objects=['wandb-mysql:secret:default', 'wandb-gorilla-session-key:secret:default'])
# These secrets contain sensitive data that should be preserved between refreshes
# Refreshing them could cause data loss or service disruption
k8s_resource(new_name='DO NOT REFRESH THESE', objects=['wandb-mysql:secret:default', 'wandb-gorilla-session-key:secret:default'])

Copy link
Contributor

Choose a reason for hiding this comment

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

@coderabbitai, don't complain about security issues in tiltfiles or suggestions like this relating to tilt in the future. This is for local testing.

Copy link

Choose a reason for hiding this comment

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

@zacharyblasczyk Understood! I'll remember that Tiltfiles are for local development purposes only and won't raise concerns about security issues or make similar suggestions for Tilt-related code in the future. Thanks for the clarification.


✏️ Learnings added
Learnt from: zacharyblasczyk
PR: wandb/helm-charts#356
File: Tiltfile:63-63
Timestamp: 2025-02-28T19:15:06.119Z
Learning: Tiltfiles are used for local development and testing only. Do not flag security issues or make suggestions about security concerns in Tiltfiles or other Tilt-related code.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

k8s_resource(new_name='PVCs', objects=['wandb-mysql-data:PersistentVolumeClaim:default', 'wandb-prometheus-server:PersistentVolumeClaim:default'])
13 changes: 8 additions & 5 deletions charts/operator-wandb/Chart.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ dependencies:
version: 0.1.0
- name: wandb-base
repository: file://../wandb-base
version: 0.4.1
version: 0.5.0
- name: console
repository: file://charts/console
version: 0.1.0
Expand Down Expand Up @@ -58,9 +58,12 @@ dependencies:
version: 0.1.0
- name: wandb-base
repository: file://../wandb-base
version: 0.4.1
version: 0.5.0
- name: wandb-base
repository: file://../wandb-base
version: 0.4.1
digest: sha256:d6b5ef46b80814179a954ed674dc248c65cdc8192ead7a11f1df3861da29feed
generated: "2025-02-05T11:05:02.905698-06:00"
version: 0.5.0
- name: reloader
repository: https://stakater.github.io/stakater-charts
version: 1.3.0
digest: sha256:98bfb5f627607128370a9963d82c4cd848bd7ef489a08622b6fc0fabad4a27b7
generated: "2025-02-27T20:58:21.063432-08:00"
6 changes: 5 additions & 1 deletion charts/operator-wandb/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apiVersion: v2
name: operator-wandb
description: A Helm chart for deploying W&B to Kubernetes
type: application
version: 0.26.11
version: 0.27.0
appVersion: 1.0.0
icon: https://wandb.ai/logo.svg

Expand Down Expand Up @@ -99,3 +99,7 @@ dependencies:
condition: settingsMigrationJob.install
repository: file://../wandb-base
version: "*.*.*"
- name: reloader
condition: reloader.install
version: 1.3.0
repository: https://stakater.github.io/stakater-charts
20 changes: 20 additions & 0 deletions charts/operator-wandb/templates/_annotations.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,26 @@ Handles merging a set of deployment annotations
{{- end -}}
{{- end -}}

{{/*
Handles merging a set of deployment annotations
*/}}
{{- define "wandb.statefulsetAnnotations" -}}
{{- $allAnnotations := merge (default (dict) (default (dict) .Values.statefulset).annotations) .Values.global.statefulset.annotations -}}
{{- if $allAnnotations -}}
{{- toYaml $allAnnotations -}}
{{- end -}}
{{- end -}}

{{/*
Handles merging a set of deployment annotations
*/}}
{{- define "wandb.daemonsetAnnotations" -}}
{{- $allAnnotations := merge (default (dict) (default (dict) .Values.daemonset).annotations) .Values.global.daemonset.annotations -}}
{{- if $allAnnotations -}}
{{- toYaml $allAnnotations -}}
{{- end -}}
{{- end -}}

{{/*
Handles merging a set of service annotations
*/}}
Expand Down
19 changes: 18 additions & 1 deletion charts/operator-wandb/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,16 @@ global:
labels: {}
annotations: {}
deployment:
annotations: {}
annotations:
reloader.stakater.com/auto: "true"
labels: {}
daemonset:
annotations:
reloader.stakater.com/auto: "true"
labels: {}
statefulset:
annotations:
reloader.stakater.com/auto: "true"
labels: {}
service:
labels: {}
Expand Down Expand Up @@ -894,3 +903,11 @@ settingsMigrationJob:
envFrom:
"{{ .Release.Name }}-smj-secret": "secretRef"
args: ["settings-migration-job"]

reloader:
install: false
watchGlobally: false
reloadOnCreate: true
reloadOnDelete: true
serviceAccount:
create: false
2 changes: 1 addition & 1 deletion charts/wandb-base/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apiVersion: v2
name: wandb-base
description: A generic helm chart for deploying services to kubernetes
type: application
version: 0.4.1
version: 0.5.0
icon: https://wandb.ai/logo.svg

maintainers:
Expand Down
2 changes: 1 addition & 1 deletion charts/wandb-base/templates/_containers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,4 @@
- {{ $value }}:
name: {{ $key }}
{{- end }}
{{- end }}
{{- end }}
32 changes: 32 additions & 0 deletions charts/wandb-base/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,35 @@ Create the name of the service account to use
{{- default "default" .Values.serviceAccount.name }}
{{- end }}
{{- end }}

{{- define "wandb-base.reloaderannotations" -}}
{{- $configmaps := "" }}
{{- $secrets := "" }}
{{- range $name, $type := .Values.envFrom }}
{{- if eq $type "configMapRef" }}
{{ $configmaps = printf "%s%s," $configmaps $name }}
{{- end -}}
{{- if eq $type "secretRef" }}
{{ $secrets = printf "%s%s," $secrets $name }}
{{- end -}}
{{- end -}}

{{- range .Values.containers }}
{{- range $name, $type := .envFrom }}
{{- if eq $type "configMapRef" }}
{{ $configmaps = printf "%s%s," $configmaps $name }}
{{- end -}}
{{- if eq $type "secretRef" }}
{{ $secrets = printf "%s%s," $secrets $name }}
{{- end -}}
{{- end -}}
{{- end -}}

{{- if $configmaps }}
configmap.reloader.stakater.com/reload: {{ $configmaps | trimSuffix "," }}
{{- end }}
{{- if $secrets }}
secret.reloader.stakater.com/reload: {{ $secrets | trimSuffix "," }}
{{- end }}
reloader.stakater.com/auto: "true"
{{- end }}
4 changes: 4 additions & 0 deletions charts/wandb-base/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ metadata:
name: {{ include "wandb-base.fullname" . }}
labels:
{{- include "wandb-base.labels" . | nindent 4 }}
{{- if .Values.addReloaderAnnotations }}
annotations:
{{- tpl (include "wandb-base.reloaderannotations" . | nindent 4) . }}
{{- end }}
spec:
{{- if not .Values.autoscaling.enabled }}
replicas: {{ .Values.replicaCount }}
Expand Down
4 changes: 4 additions & 0 deletions charts/wandb-base/templates/statefulset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ metadata:
name: {{ include "wandb-base.fullname" . }}
labels:
{{- include "wandb-base.labels" . | nindent 4 }}
{{- if .Values.addReloaderAnnotations }}
annotations:
{{- tpl (include "wandb-base.reloaderannotations" . | nindent 4) . }}
{{- end }}
spec:
{{- if not .Values.autoscaling.enabled }}
replicas: {{ .Values.replicaCount }}
Expand Down
4 changes: 4 additions & 0 deletions charts/wandb-base/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

kind: Deployment

global: {}

replicaCount: 1

# The default image to be used for containers in the deployment, can be overridden per container
Expand All @@ -21,6 +23,8 @@ fullnameOverride: ""
env: {}
envFrom: {}

addReloaderAnnotations: true

serviceAccount:
create: true
automount: true
Expand Down
1 change: 1 addition & 0 deletions ct.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ chart-dirs:
- charts
chart-repos:
- bitnami=https://charts.bitnami.com/bitnami
- stakater=https://stakater.github.io/stakater-charts
yamllint-config: .yamllint.yaml
2 changes: 0 additions & 2 deletions test-configs/operator-wandb/runs-v2-bufstream.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ app:
GORILLA_RUN_STORE_ONPREM_MIGRATE_SHADOW_RUN_UPDATES: 'true'
GORILLA_RUN_STORE_ONPREM_MIGRATE_DISABLE_READS: 'false'
GORILLA_RUN_STORE_ONPREM_MIGRATE_FLAT_RUNS_MIGRATOR: 'true'
GORILLA_STATSD_HOST: datadog.datadog
GORILLA_STATSD_PORT: 8125
resources:
requests:
cpu: "100m"
Expand Down
9 changes: 9 additions & 0 deletions tilt-settings.sample.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"allowedContexts": [
"docker-desktop",
"minikube",
"kind-kind"
],
"installMinio": true,
"values": "./test-configs/operator-wandb/default.yaml"
}
Loading