From 5d6f5cd08269dda101f1e9cbc7d83b2d9052957b Mon Sep 17 00:00:00 2001 From: Callum Forrester Date: Wed, 24 Sep 2025 13:53:33 +0000 Subject: [PATCH 1/7] Editable debug mode --- helm/blueapi/templates/statefulset.yaml | 36 ++++++++++++++++++- helm/blueapi/templates/volumes.yaml | 16 +++++++++ tests/unit_tests/test_helm_chart.py | 48 ++++++++++++++++--------- 3 files changed, 83 insertions(+), 17 deletions(-) diff --git a/helm/blueapi/templates/statefulset.yaml b/helm/blueapi/templates/statefulset.yaml index d9e720bca..151a148e9 100644 --- a/helm/blueapi/templates/statefulset.yaml +++ b/helm/blueapi/templates/statefulset.yaml @@ -105,6 +105,28 @@ spec: mountPath: {{ .Values.worker.scratch.root }} mountPropagation: HostToContainer {{- end }} + {{- if .Values.debug.enabled }} + {{- if not .Values.initContainer.enabled }} + initContainers: + {{- end}} + # add in an init container to copy out virtual environment and workspaces + - name: {{ include "blueapi.fullname" . }}-init-debug + image: "{{ .Values.image.repository }}-debug:{{ .Values.image.tag }}" + imagePullPolicy: IfNotPresent + resources: + {{- .Values.initResources | default .Values.resources | toYaml | nindent 12 }} + {{- with .Values.securityContext }} + securityContext: + {{- toYaml . | nindent 12 }} + {{- end }} + {{- with .Values.initCommand }} + command: + {{- toYaml . | nindent 12 }} + {{- end }} + volumeMounts: + - name: {{ include "blueapi.fullname" . }}-develop + mountPath: /dest + {{- end }} {{- end }} containers: - name: {{ .Chart.Name }} @@ -147,7 +169,19 @@ spec: - name: venv mountPath: /venv {{- end }} - {{- if or .Values.debug.enabled (and .Values.initContainer.enabled .Values.initContainer.persistentVolume.enabled) }} + {{- if or .Values.debug.enabled }} + - mountPath: /home + name: home + - mountPath: /var/run/nslcd + name: nslcd + - name: {{ include "blueapi.fullname" . }}-develop + mountPath: /workspaces + subPath: workspaces + - name: {{ include "blueapi.fullname" . }}-develop + mountPath: /venv + subPath: venv + {{- end }} + {{- if and .Values.initContainer.enabled .Values.initContainer.persistentVolume.enabled }} - mountPath: /home name: home - mountPath: /var/run/nslcd diff --git a/helm/blueapi/templates/volumes.yaml b/helm/blueapi/templates/volumes.yaml index ba9cb362e..ac20e42f7 100644 --- a/helm/blueapi/templates/volumes.yaml +++ b/helm/blueapi/templates/volumes.yaml @@ -12,3 +12,19 @@ spec: requests: storage: 1Gi {{- end }} +--- +# PVC for debugging volume +{{- if .Values.debug.enabled }} +kind: PersistentVolumeClaim +apiVersion: v1 +metadata: + name: {{ include "blueapi.fullname" . }}-develop + labels: + app: {{ include "blueapi.fullname" . }} +spec: + accessModes: + - ReadWriteMany + resources: + requests: + storage: "1Gi" +{{- end }} diff --git a/tests/unit_tests/test_helm_chart.py b/tests/unit_tests/test_helm_chart.py index d1b3c882f..b48bbb10f 100644 --- a/tests/unit_tests/test_helm_chart.py +++ b/tests/unit_tests/test_helm_chart.py @@ -577,10 +577,10 @@ def test_init_container_venv_volume_mount( @pytest.mark.parametrize("existingClaimName", [None, "foo"]) @pytest.mark.parametrize("debug_enabled", [True, False]) def test_persistent_volume_claim_exists( - initContainer_enabled, - persistentVolume_enabled, - existingClaimName, - debug_enabled, + initContainer_enabled: bool, + persistentVolume_enabled: bool, + existingClaimName: str | None, + debug_enabled: bool, ): manifests = render_persistent_volume_chart( initContainer_enabled, @@ -589,7 +589,7 @@ def test_persistent_volume_claim_exists( debug_enabled, ) - persistent_volume_claim = { + scratch_claim = { "scratch-0.1.0": { "apiVersion": "v1", "kind": "PersistentVolumeClaim", @@ -603,9 +603,26 @@ def test_persistent_volume_claim_exists( }, } } + debug_claim = { + "blueapi-develop": { + "apiVersion": "v1", + "kind": "PersistentVolumeClaim", + "metadata": {"labels": {"app": "blueapi"}, "name": "blueapi-develop"}, + "spec": { + "accessModes": ["ReadWriteMany"], + "resources": {"requests": {"storage": "1Gi"}}, + }, + } + } + + is_existing_claim = existingClaimName is not None - if persistentVolume_enabled and not existingClaimName: - assert persistent_volume_claim == manifests["PersistentVolumeClaim"] + if (persistentVolume_enabled and not is_existing_claim) and not debug_enabled: + assert scratch_claim == manifests["PersistentVolumeClaim"] + elif not (persistentVolume_enabled and not is_existing_claim) and debug_enabled: + assert debug_claim == manifests["PersistentVolumeClaim"] + elif persistentVolume_enabled and not is_existing_claim and debug_enabled: + assert {**scratch_claim, **debug_claim} == manifests["PersistentVolumeClaim"] else: assert "PersistentVolumeClaim" not in manifests @@ -750,12 +767,12 @@ def test_main_container_venv_volume_mount( @pytest.mark.parametrize("existingClaimName", [None, "foo"]) @pytest.mark.parametrize("debug_enabled", [True, False]) def test_main_container_home_and_nslcd_volume_mounts( - initContainer_enabled, - persistentVolume_enabled, - existingClaimName, - debug_enabled, - home_volume_mount, - nslcd_volume_mount, + initContainer_enabled: bool, + persistentVolume_enabled: bool, + existingClaimName: str | None, + debug_enabled: bool, + home_volume_mount: dict, + nslcd_volume_mount: dict, ): manifests = render_persistent_volume_chart( initContainer_enabled, @@ -815,8 +832,8 @@ def test_main_container_args( @pytest.mark.parametrize("existingClaimName", [None, "foo"]) @pytest.mark.parametrize("debug_enabled", [True, False]) def test_scratch_volume_uses_correct_claimName( - existingClaimName, - debug_enabled, + existingClaimName: str | None, + debug_enabled: bool, ): manifests = render_persistent_volume_chart( True, @@ -831,7 +848,6 @@ def test_scratch_volume_uses_correct_claimName( if existingClaimName: assert claim_name == existingClaimName - assert "PersistentVolumeClaim" not in manifests else: assert claim_name == "scratch-0.1.0" assert claim_name in manifests["PersistentVolumeClaim"] From 0a85228a138a2db67ebc46f61ab15ca7f78c9fde Mon Sep 17 00:00:00 2001 From: Callum Forrester Date: Mon, 29 Sep 2025 10:06:19 +0000 Subject: [PATCH 2/7] Ensure debug editable volume actually exists --- helm/blueapi/templates/statefulset.yaml | 5 +++ tests/unit_tests/test_helm_chart.py | 43 +++++++++++++++++++++---- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/helm/blueapi/templates/statefulset.yaml b/helm/blueapi/templates/statefulset.yaml index 151a148e9..93df08d84 100644 --- a/helm/blueapi/templates/statefulset.yaml +++ b/helm/blueapi/templates/statefulset.yaml @@ -45,6 +45,11 @@ spec: sources: - configMap: name: {{ include "blueapi.fullname" . }}-config + {{- if .Values.debug.enabled }} + - name: {{ include "blueapi.fullname" . }}-develop + persistentVolumeClaim: + claimName: {{ include "blueapi.fullname" . }}-develop + {{- end }} {{- with .Values.volumes }} {{- toYaml . | nindent 6 }} {{- end }} diff --git a/tests/unit_tests/test_helm_chart.py b/tests/unit_tests/test_helm_chart.py index b48bbb10f..f973d7066 100644 --- a/tests/unit_tests/test_helm_chart.py +++ b/tests/unit_tests/test_helm_chart.py @@ -842,15 +842,46 @@ def test_scratch_volume_uses_correct_claimName( debug_enabled, ) - claim_name = manifests["StatefulSet"]["blueapi"]["spec"]["template"]["spec"][ - "volumes" - ][3]["persistentVolumeClaim"]["claimName"] + volumes = manifests["StatefulSet"]["blueapi"]["spec"]["template"]["spec"]["volumes"] + claim_names = [ + volume["persistentVolumeClaim"]["claimName"] + for volume in volumes + if "persistentVolumeClaim" in volume + ] if existingClaimName: - assert claim_name == existingClaimName + assert existingClaimName in claim_names + else: + assert "scratch-0.1.0" in claim_names + + +@pytest.mark.parametrize("initContainer_enabled", [True, False]) +@pytest.mark.parametrize("persistentVolume_enabled", [True, False]) +@pytest.mark.parametrize("existingClaimName", [None, "foo"]) +@pytest.mark.parametrize("debug_enabled", [True, False]) +def test_debug_volume_exists( + initContainer_enabled: bool, + persistentVolume_enabled: bool, + existingClaimName: str | None, + debug_enabled: bool, +): + manifests = render_persistent_volume_chart( + initContainer_enabled, + persistentVolume_enabled, + existingClaimName, + debug_enabled, + ) + + expected_volume = { + "name": "blueapi-develop", + "persistentVolumeClaim": {"claimName": "blueapi-develop"}, + } + + volumes = manifests["StatefulSet"]["blueapi"]["spec"]["template"]["spec"]["volumes"] + if debug_enabled: + assert expected_volume in volumes else: - assert claim_name == "scratch-0.1.0" - assert claim_name in manifests["PersistentVolumeClaim"] + assert expected_volume not in volumes @pytest.fixture From 8d890d69cb2a495ab89e34020103a4e87eca7afd Mon Sep 17 00:00:00 2001 From: Callum Forrester Date: Tue, 30 Sep 2025 10:10:29 +0000 Subject: [PATCH 3/7] Fix no init containers when debug enabled but init container disabled --- helm/blueapi/templates/statefulset.yaml | 9 ++++----- tests/unit_tests/test_helm_chart.py | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/helm/blueapi/templates/statefulset.yaml b/helm/blueapi/templates/statefulset.yaml index 93df08d84..72fbff2b6 100644 --- a/helm/blueapi/templates/statefulset.yaml +++ b/helm/blueapi/templates/statefulset.yaml @@ -81,8 +81,10 @@ spec: emptyDir: sizeLimit: 5Mi {{- end }} - {{- if .Values.initContainer.enabled }} + {{- if or .Values.initContainer.enabled .Values.debug.enabled }} initContainers: + {{- end}} + {{- if .Values.initContainer.enabled }} - name: setup-scratch image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}{{ ternary "-debug" "" .Values.debug.enabled }}" imagePullPolicy: {{ .Values.image.pullPolicy }} @@ -110,10 +112,8 @@ spec: mountPath: {{ .Values.worker.scratch.root }} mountPropagation: HostToContainer {{- end }} + {{- end }} {{- if .Values.debug.enabled }} - {{- if not .Values.initContainer.enabled }} - initContainers: - {{- end}} # add in an init container to copy out virtual environment and workspaces - name: {{ include "blueapi.fullname" . }}-init-debug image: "{{ .Values.image.repository }}-debug:{{ .Values.image.tag }}" @@ -132,7 +132,6 @@ spec: - name: {{ include "blueapi.fullname" . }}-develop mountPath: /dest {{- end }} - {{- end }} containers: - name: {{ .Chart.Name }} {{- with .Values.securityContext }} diff --git a/tests/unit_tests/test_helm_chart.py b/tests/unit_tests/test_helm_chart.py index f973d7066..a8bb65275 100644 --- a/tests/unit_tests/test_helm_chart.py +++ b/tests/unit_tests/test_helm_chart.py @@ -165,6 +165,23 @@ def test_init_container_spec_generated(): assert len(init_containers) == 1 +def test_init_container_spec_generated_in_debug_mode_even_if_disabled(): + manifests = render_chart( + values={ + "initContainer": { + "enabled": False, + }, + "debug": { + "enabled": True, + }, + }, + ) + init_containers = manifests["StatefulSet"]["blueapi"]["spec"]["template"]["spec"][ + "initContainers" + ] + assert len(init_containers) == 1 + + def test_init_container_spec_disablable(): manifests = render_chart( values={ From be7d24fc7c7b540352ce80543ce15b809c304a20 Mon Sep 17 00:00:00 2001 From: Callum Forrester Date: Tue, 30 Sep 2025 10:28:10 +0000 Subject: [PATCH 4/7] Fix debug suffix in wrong place --- helm/blueapi/templates/statefulset.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helm/blueapi/templates/statefulset.yaml b/helm/blueapi/templates/statefulset.yaml index 72fbff2b6..a02962530 100644 --- a/helm/blueapi/templates/statefulset.yaml +++ b/helm/blueapi/templates/statefulset.yaml @@ -116,7 +116,7 @@ spec: {{- if .Values.debug.enabled }} # add in an init container to copy out virtual environment and workspaces - name: {{ include "blueapi.fullname" . }}-init-debug - image: "{{ .Values.image.repository }}-debug:{{ .Values.image.tag }}" + image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}-debug" imagePullPolicy: IfNotPresent resources: {{- .Values.initResources | default .Values.resources | toYaml | nindent 12 }} From 63f6550b3a229ac59f9d8fee72326d8e690f5fd4 Mon Sep 17 00:00:00 2001 From: Callum Forrester Date: Tue, 30 Sep 2025 10:31:17 +0000 Subject: [PATCH 5/7] Use init command --- helm/blueapi/templates/statefulset.yaml | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/helm/blueapi/templates/statefulset.yaml b/helm/blueapi/templates/statefulset.yaml index a02962530..9e85e98ad 100644 --- a/helm/blueapi/templates/statefulset.yaml +++ b/helm/blueapi/templates/statefulset.yaml @@ -124,10 +124,22 @@ spec: securityContext: {{- toYaml . | nindent 12 }} {{- end }} - {{- with .Values.initCommand }} command: - {{- toYaml . | nindent 12 }} - {{- end }} + - /bin/bash + - -c + - | + echo "Running as account"; id + if [ -d /dest/venv ]; then + echo "Virtual environment already exists, skipping copy" + else + echo "Copying virtual env to the debugging volume" + cp -r /venv /dest/venv + echo "Copying workspaces to the debugging volume" + cp -r /workspaces /dest/workspaces + echo "Setting permissions on the debugging volume" + chmod -R o+rwX /dest/venv /dest/workspaces + fi + echo "Init container completed successfully" volumeMounts: - name: {{ include "blueapi.fullname" . }}-develop mountPath: /dest From e922f9cc27fdea45242047db0e7b7c31284032a0 Mon Sep 17 00:00:00 2001 From: Callum Forrester Date: Wed, 1 Oct 2025 11:25:06 +0000 Subject: [PATCH 6/7] Increase debug volume size --- helm/blueapi/README.md | 1 + helm/blueapi/templates/volumes.yaml | 2 +- helm/blueapi/values.schema.json | 8 ++++++++ helm/blueapi/values.yaml | 3 ++- 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/helm/blueapi/README.md b/helm/blueapi/README.md index 6a76fdae8..908202916 100644 --- a/helm/blueapi/README.md +++ b/helm/blueapi/README.md @@ -10,6 +10,7 @@ A Helm chart deploying a worker pod that runs Bluesky plans |-----|------|---------|-------------| | affinity | object | `{}` | May be required to run on specific nodes (e.g. the control machine) | | debug.enabled | bool | `false` | If enabled, disables liveness and readiness probes, and does not start the service on startup This allows connecting to the pod and starting the service manually to allow debugging on the cluster | +| debug.volume.size | string | `"2Gi"` | | | extraEnvVars | list | `[]` | Additional envVars to mount to the pod | | fullnameOverride | string | `""` | | | global | object | `{}` | Not used, but must be present for validation when using as a dependency of another chart | diff --git a/helm/blueapi/templates/volumes.yaml b/helm/blueapi/templates/volumes.yaml index ac20e42f7..403f28f85 100644 --- a/helm/blueapi/templates/volumes.yaml +++ b/helm/blueapi/templates/volumes.yaml @@ -26,5 +26,5 @@ spec: - ReadWriteMany resources: requests: - storage: "1Gi" + storage: {{ .Values.debug.volume.size }} {{- end }} diff --git a/helm/blueapi/values.schema.json b/helm/blueapi/values.schema.json index 8b80efee8..03579d36c 100644 --- a/helm/blueapi/values.schema.json +++ b/helm/blueapi/values.schema.json @@ -14,6 +14,14 @@ "enabled": { "description": "If enabled, disables liveness and readiness probes, and does not start the service on startup This allows connecting to the pod and starting the service manually to allow debugging on the cluster", "type": "boolean" + }, + "volume": { + "type": "object", + "properties": { + "size": { + "type": "string" + } + } } } }, diff --git a/helm/blueapi/values.yaml b/helm/blueapi/values.yaml index c8eed552d..ea5a45e10 100644 --- a/helm/blueapi/values.yaml +++ b/helm/blueapi/values.yaml @@ -96,7 +96,6 @@ resources: requests: cpu: 200m memory: 400Mi - # -- Override resources for init container. By default copies resources of main container. initResources: {} @@ -226,6 +225,8 @@ debug: # -- If enabled, disables liveness and readiness probes, and does not start the service on startup # This allows connecting to the pod and starting the service manually to allow debugging on the cluster enabled: false + volume: + size: 2Gi # -- Not used, but must be present for validation when using as a dependency of another chart global: {} From 19bef2fd56b07b7b643e1107d22b4608b24882da Mon Sep 17 00:00:00 2001 From: Callum Forrester Date: Thu, 2 Oct 2025 14:01:38 +0000 Subject: [PATCH 7/7] Fix tests --- tests/unit_tests/test_helm_chart.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit_tests/test_helm_chart.py b/tests/unit_tests/test_helm_chart.py index a8bb65275..d856b9a14 100644 --- a/tests/unit_tests/test_helm_chart.py +++ b/tests/unit_tests/test_helm_chart.py @@ -627,7 +627,7 @@ def test_persistent_volume_claim_exists( "metadata": {"labels": {"app": "blueapi"}, "name": "blueapi-develop"}, "spec": { "accessModes": ["ReadWriteMany"], - "resources": {"requests": {"storage": "1Gi"}}, + "resources": {"requests": {"storage": "2Gi"}}, }, } }