feat: Seperate debug and suspend behaviours; consolidate LDAP requirements (#1130)

Refactors the container build to remove the debug container, and
properly capture the requirements on the LDAP sidecar.

This container build has been used to debug using VSCode remote
attaching to the container, or by port-forwarding from a local
workstation.

The previous `debug` behaviour of suspending has been extracted into a
`suspend` argument.

Author: DiamondJoseph

.github/workflows/_container.yml

@@ -105,7 +105,9 @@ jobs:
             type=ref,event=tag
 
       - name: package chart and push it
+        env:
+          REPO: ${{ github.repository_owner }}
         run: |
           helm dependencies update helm/blueapi
           helm package helm/blueapi --version ${GITHUB_REF##*/} --app-version ${GITHUB_REF##*/} -d /tmp/
-          helm push /tmp/blueapi-${GITHUB_REF##*/}.tgz oci://ghcr.io/diamondlightsource/charts
+          helm push /tmp/blueapi-${GITHUB_REF##*/}.tgz oci://ghcr.io/${REPO@L}/charts

.github/workflows/_debug_container.yml

@@ -45,15 +45,6 @@ jobs:
       contents: read
       packages: write
 
-  debug_container:
-    needs: [container, test]
-    uses: ./.github/workflows/_debug_container.yml
-    with:
-      publish: ${{ needs.test.result == 'success' }}
-    permissions:
-      contents: read
-      packages: write
-
   docs:
     uses: ./.github/workflows/_docs.yml
 

.github/workflows/ci.yml

@@ -27,51 +27,27 @@ RUN mkdir -p /.cache/pip; chmod o+wrX /.cache/pip
 # Requires buildkit 0.17.0
 COPY --chmod=o+wrX . /workspaces/blueapi
 WORKDIR /workspaces/blueapi
-RUN touch dev-requirements.txt && pip install --upgrade pip && pip install -c dev-requirements.txt .
-
-
-FROM build AS debug
-
-
-# Set origin to use ssh
-RUN git remote set-url origin [email protected]:DiamondLightSource/blueapi.git
-
-
-# For this pod to understand finding user information from LDAP
-RUN apt update
-RUN DEBIAN_FRONTEND=noninteractive apt install libnss-ldapd -y
-RUN sed -i 's/files/ldap files/g' /etc/nsswitch.conf
-
-# Make editable and debuggable
-RUN pip install debugpy
-RUN pip install -e .
-
-RUN groupadd -g 1000 blueapi && \
-    useradd -m -u 1000 -g blueapi blueapi
- 
-# Switch to the custom user
-USER blueapi
-
-# Alternate entrypoint to allow devcontainer to attach
-ENTRYPOINT [ "/bin/bash", "-c", "--" ]
-CMD [ "while true; do sleep 30; done;" ]
-
+RUN touch dev-requirements.txt && pip install --upgrade pip && pip install debugpy && pip install -c dev-requirements.txt .
 
 # The runtime stage copies the built venv into a slim runtime container
 FROM python:${PYTHON_VERSION}-slim AS runtime
 # Add apt-get system dependecies for runtime here if needed
-RUN apt-get update && apt-get install -y --no-install-recommends \
+RUN DEBIAN_FRONTEND=noninteractive apt-get update && apt-get install -y --no-install-recommends \
     # Git required for installing packages at runtime
     git \
+    # gdb required for attaching debugger
+    gdb \
+    # May be required if attaching devcontainer
+    libnss-ldapd \
     && rm -rf /var/lib/apt/lists/*
+
 COPY --from=build --chmod=o+wrX /venv/ /venv/
 COPY --from=build --chmod=o+wrX /.cache/pip /.cache/pip
+
 ENV PATH=/venv/bin:$PATH
 ENV PYTHONPYCACHEPREFIX=/tmp/blueapi_pycache
 
 # For this pod to understand finding user information from LDAP
-RUN apt update
-RUN DEBIAN_FRONTEND=noninteractive apt install libnss-ldapd -y
 RUN sed -i 's/files/ldap files/g' /etc/nsswitch.conf
 
 # Set the MPLCONFIGDIR environment variable to a temporary directory to avoid
@@ -81,9 +57,14 @@ RUN sed -i 's/files/ldap files/g' /etc/nsswitch.conf
 
 ENV MPLCONFIGDIR=/tmp/matplotlib
 
+# Make a path to site-packages that is invariant with python version
+# This allows our pathMapping in launch.jsons to always find build blueapi
+WORKDIR /venv/lib
+RUN ln -s python python${PYTHON_VERSION}
+
 RUN groupadd -g 1000 blueapi && \
     useradd -m -u 1000 -g blueapi blueapi
- 
+
 # Switch to the custom user
 USER blueapi
 

Dockerfile

@@ -9,7 +9,9 @@ A Helm chart deploying a worker pod that runs Bluesky plans
 | Key | Type | Default | Description |
 |-----|------|---------|-------------|
 | 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.enabled | bool | `false` | If enabled, runs debugpy, allowing port-forwarding to expose port 5678 or attached vscode instance |
+| debug.log_to_stderr | bool | `false` | If enabled configures debugpy to use the option `--log-to-stderr` |
+| debug.suspend | bool | `false` | If enabled does not start the service on startup This allows connecting to the pod and starting the service manually to allow debugging on the cluster |
 | 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 |

helm/blueapi/README.md

@@ -72,7 +72,7 @@ spec:
           type: Directory
       {{- end }}
       {{- end }}
-      {{- if or .Values.debug.enabled (and .Values.initContainer.enabled .Values.initContainer.persistentVolume.enabled)}}
+      {{- if ne 1000.0 .Values.securityContext.runAsUser }}
       - name: home  # Required for vscode to install plugins
         emptyDir:
           sizeLimit: 500Mi
@@ -83,7 +83,7 @@ spec:
       {{- if .Values.initContainer.enabled }}
       initContainers:
       - name: setup-scratch
-        image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}{{ ternary "-debug" "" .Values.debug.enabled }}"
+        image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}"
         imagePullPolicy: {{ .Values.image.pullPolicy }}
         resources:
           {{- .Values.initResources | default .Values.resources | toYaml | nindent 12 }}
@@ -116,7 +116,7 @@ spec:
           securityContext:
             {{- toYaml . | nindent 12 }}
           {{- end }}
-          image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}{{ ternary "-debug" "" .Values.debug.enabled }}"
+          image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}"
           imagePullPolicy: {{ .Values.image.pullPolicy }}
           ports:
             - name: http
@@ -151,17 +151,35 @@ spec:
             - name: venv
               mountPath: /venv
             {{- end }}
-            {{- if or .Values.debug.enabled (and .Values.initContainer.enabled .Values.initContainer.persistentVolume.enabled) }}
+            {{- if ne 1000.0 .Values.securityContext.runAsUser }}
             - mountPath: /home
               name: home
             - mountPath: /var/run/nslcd
               name: nslcd
             {{- end }}
-          {{- if not .Values.debug.enabled }}
           args:
             - "-c"
             - "/config/config.yaml"
             - "serve"
+          {{- if .Values.debug.enabled }}
+          command:
+          - "python"
+          - "-Xfrozen_modules=off"
+          - "-m"
+          - "debugpy"
+          - "--listen"
+          - "5678"
+          {{- if .Values.debug.log_to_stderr }}
+          - "--log-to-stderr"
+          {{- end }}
+          {{- if .Values.debug.suspend }}
+          - "--wait-for-client"
+          {{- end }}
+          - "--configure-subProcess"
+          - "true"
+          - "-m"
+          - "blueapi"
+          {{- end }}
           {{- with .Values.livenessProbe }}
           livenessProbe:
             {{- toYaml . | nindent 12 }}
@@ -174,13 +192,12 @@ spec:
           startupProbe:
             {{- toYaml . | nindent 12 }}
           {{- end }}
-          {{- end }}
           envFrom:
             - configMapRef:
                 name: {{ include "blueapi.fullname" . }}-otel-config
           env:
             {{- toYaml .Values.extraEnvVars | nindent 12 }}
-        {{- if or .Values.debug.enabled (and .Values.initContainer.persistentVolume.enabled .Values.initContainer.enabled )}}
+        {{- if ne 1000.0 .Values.securityContext.runAsUser }}
         - name: debug-account-sync
           image: ghcr.io/diamondlightsource/account-sync-sidecar:3.0.0
           volumeMounts:

helm/blueapi/templates/statefulset.yaml

@@ -12,7 +12,15 @@
             "type": "object",
             "properties": {
                 "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",
+                    "description": "If enabled, runs debugpy, allowing port-forwarding to expose port 5678 or attached vscode instance",
+                    "type": "boolean"
+                },
+                "log_to_stderr": {
+                    "description": "If enabled configures debugpy to use the option `--log-to-stderr`",
+                    "type": "boolean"
+                },
+                "suspend": {
+                    "description": "If enabled 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"
                 }
             }

helm/blueapi/values.schema.json

@@ -223,9 +223,13 @@ initContainer:
     existingClaimName: ""
 
 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
+  # -- If enabled, runs debugpy, allowing port-forwarding to expose port 5678 or attached vscode instance
   enabled: false
+  # -- If enabled does not start the service on startup
+  # This allows connecting to the pod and starting the service manually to allow debugging on the cluster
+  suspend: false
+  # -- If enabled configures debugpy to use the option `--log-to-stderr`
+  log_to_stderr: false
 
 # -- Not used, but must be present for validation when using as a dependency of another chart
 global: {}