diff --git a/.github/workflows/docker_build_monailabel.yml b/.github/workflows/docker_build_monailabel.yml new file mode 100644 index 000000000..863d23f96 --- /dev/null +++ b/.github/workflows/docker_build_monailabel.yml @@ -0,0 +1,178 @@ +# Copyright (c) Guy's and St Thomas' NHS Foundation Trust & King's College London +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +name: Build and Push Docker Image for MONAI Label + +# Direct push trigger like orthanc / xnat_* — monailabel has no test suite to +# gate on. workflow_dispatch covers branch-pinned testing (see CLAUDE.md +# "Docker image builds"). +on: + workflow_dispatch: + push: + branches: [main, develop] + paths: + - "trust/monailabel/**" + - ".github/workflows/docker_build_monailabel.yml" + # PR runs build + the import smoke only — the push steps are gated below, + # so nothing is published from a pull request. + pull_request: + paths: + - "trust/monailabel/**" + - ".github/workflows/docker_build_monailabel.yml" + +permissions: + contents: read + +jobs: + build-and-push: + # Skip on forks: they cannot push to ghcr.io/londonaicentre. + if: github.repository == 'londonaicentre/FLIP' + runs-on: ubuntu-latest + permissions: # override top-level read-only default to allow GHCR push + contents: read + packages: write + defaults: + run: + working-directory: ./trust/monailabel + env: + REGISTRY: ghcr.io + IMAGE_NAME: londonaicentre/monailabel + BUILD_ARGS: "" + steps: + - name: Checkout code + uses: actions/checkout@v5 + with: + repository: ${{ github.event.workflow_run.head_repository.full_name }} + ref: ${{ github.event.workflow_run.head_sha }} + + # The image is ~10.4 GB (torch + CUDA runtime wheels + SAM2); a stock + # ubuntu-latest runner has ~14 GB free, which the build blows through in + # intermediate layers. Reclaim the ~25 GB of preinstalled toolchains this + # job never uses. Plain rm rather than a third-party action so the + # workflow keeps the repo's actions/* -only supply-chain posture. + - name: Reclaim runner disk space + working-directory: / + run: | + df -h / | tail -1 + sudo rm -rf /usr/share/dotnet /usr/local/lib/android /opt/ghc \ + /opt/hostedtoolcache/CodeQL /usr/local/.ghcup /usr/local/share/boost + sudo docker system prune -af --volumes >/dev/null 2>&1 || true + df -h / | tail -1 + # Assert the reclaim actually delivered. `rm -rf` on a path that no longer exists + # exits 0, so if a future runner image relocates these toolchains this step stays + # green and the build instead dies with an opaque ENOSPC deep into a torch layer. + # Fail here, where the cause is obvious. Floor is ~2x the finished image. + FREE_GB=$(df -BG --output=avail / | tail -1 | tr -dc '0-9') + echo "Free after reclaim: ${FREE_GB} GB" + if [[ "${FREE_GB}" -lt 25 ]]; then + echo "::error::Only ${FREE_GB} GB free after reclaim; the ~10.4 GB image needs ~25 GB of headroom for intermediate layers. The runner image's preinstalled toolchain paths have probably moved — update this step." + exit 1 + fi + + - name: Determine tags + id: tags + env: + GH_REF_NAME: ${{ github.ref_name }} + GH_EVENT_NAME: ${{ github.event_name }} + GH_REF: ${{ github.ref }} + GH_SHA: ${{ github.sha }} + GH_WR_BRANCH: ${{ github.event.workflow_run.head_branch }} + GH_WR_EVENT: ${{ github.event.workflow_run.event }} + run: | + TAGS="${REGISTRY}/${IMAGE_NAME}:${{ github.sha }}" + + # An empty sha would silently publish a mutable literal `sha-` tag — refuse. + [[ -n "$GH_SHA" ]] || { echo "::error::empty commit SHA — cannot compute the sha- tag"; exit 1; } + + # Immutable short-SHA tag (FLIP#751), pushed uniformly on every publish. + # Length 7 must match the tag resolution in deploy/providers/AWS/Makefile. + TAGS="${TAGS},${REGISTRY}/${IMAGE_NAME}:sha-${GH_SHA:0:7}" + + # Branch name sanitization + SAFE_REF_NAME=$(echo "$GH_REF_NAME" | sed 's/[^a-zA-Z0-9]/-/g') + TAGS="${TAGS},${REGISTRY}/${IMAGE_NAME}:${SAFE_REF_NAME}" + + # Branch number (if starts with number) + if [[ "$GH_REF_NAME" =~ ^[0-9]+ ]]; then + BRANCH_NUM=$(echo "$GH_REF_NAME" | grep -oE '^[0-9]+') + TAGS="${TAGS},${REGISTRY}/${IMAGE_NAME}:${BRANCH_NUM}" + fi + + # PR Number (if PR) + if [[ "$GH_EVENT_NAME" == "pull_request" ]]; then + PR_NUMBER=$(echo "$GH_REF" | awk -F / '{print $3}') + TAGS="${TAGS},${REGISTRY}/${IMAGE_NAME}:pr-${PR_NUMBER}" + fi + + # Determine if this is a merge/push to main or develop + BRANCH_NAME="" + if [[ "$GH_EVENT_NAME" == "workflow_run" && "$GH_WR_EVENT" == "push" ]]; then + BRANCH_NAME="$GH_WR_BRANCH" + elif [[ "$GH_EVENT_NAME" == "push" ]]; then + BRANCH_NAME="$GH_REF_NAME" + fi + + if [[ "$BRANCH_NAME" == "main" ]]; then + TAGS="${TAGS},${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:prod" + elif [[ "$BRANCH_NAME" == "develop" ]]; then + TAGS="${TAGS},${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:stag" + fi + echo "tags=${TAGS}" >> $GITHUB_OUTPUT + echo "Generated tags: ${TAGS}" + + - name: Log in to GitHub Container Registry + if: github.event_name != 'pull_request' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_ACTOR: ${{ github.actor }} + run: echo "$GH_TOKEN" | docker login $REGISTRY -u "$GH_ACTOR" --password-stdin + + # The Dockerfile itself asserts the two known-bad states at build time + # (unpatched XNAT datastore auth, non-CUDA-12 torch), so a green build + # already guarantees those. + - name: Build Docker image + env: + DOCKER_TAGS: ${{ steps.tags.outputs.tags }} + run: | + IFS=',' read -ra TAG_ARRAY <<< "$DOCKER_TAGS" + TAG_FLAGS=() + for tag in "${TAG_ARRAY[@]}"; do + TAG_FLAGS+=("-t" "$tag") + done + # shellcheck disable=SC2086 + docker build $BUILD_ARGS "${TAG_FLAGS[@]}" . + + # No GPU on the runner, so this stays at import level: the stack that + # must coexist (monailabel @ pinned commit, torch cu12, sam2, curl for + # the entrypoint's probes) actually loads in the built image. + - name: Smoke test — runtime stack imports + env: + DOCKER_TAGS: ${{ steps.tags.outputs.tags }} + run: | + TAG="${DOCKER_TAGS%%,*}" + docker run --rm --entrypoint sh "$TAG" -c ' + curl --version >/dev/null && + python -c " + import monailabel, torch, sam2 + from monailabel.datastore.xnat import XNATDatastore + print(\"monailabel\", monailabel.__version__, \"torch\", torch.__version__) + "' + + - name: Push Docker image + if: github.event_name != 'pull_request' + env: + DOCKER_TAGS: ${{ steps.tags.outputs.tags }} + run: | + IFS=',' read -ra TAG_ARRAY <<< "$DOCKER_TAGS" + for tag in "${TAG_ARRAY[@]}"; do + docker push "$tag" + done diff --git a/.github/workflows/test_helm_chart.yml b/.github/workflows/test_helm_chart.yml index 541b9c65e..d084fd067 100644 --- a/.github/workflows/test_helm_chart.yml +++ b/.github/workflows/test_helm_chart.yml @@ -106,6 +106,35 @@ jobs: --set omopDb.external.host=test.example.com \ > /dev/null + # monailabel.enabled defaults false, so every other render skips its whole body — + # render coverage only: the kind install job must NOT enable it (the image is + # ~10.4 GB and needs a GPU, neither of which kind has). Also asserts the + # publicUrl `required` guard actually fires, since it is the one setting that + # cannot be defaulted (the clinician's browser calls it — see trust/README.md). + - name: Render template (monailabel enabled) + run: | + helm template trust-release deploy/providers/kubernetes/ \ + --set monailabel.enabled=true \ + --set monailabel.publicUrl=http://node.example.com:30030 > /tmp/monailabel.yaml + if ! grep -q "flip-trust.*-monailabel" /tmp/monailabel.yaml; then + echo "::error::monailabel resources did not render with monailabel.enabled=true" + exit 1 + fi + # Assert the guard fires AND fires for the right reason. Testing only "helm exited + # non-zero" would be satisfied by a typo'd chart path, an unrelated schema error, or + # any broken sibling template — a vacuous pass of exactly the kind this workflow + # calls out elsewhere. Capture stderr and match the guard's own message. + if helm template trust-release deploy/providers/kubernetes/ \ + --set monailabel.enabled=true > /tmp/monailabel-noguard.out 2>&1; then + echo "::error::monailabel rendered without publicUrl — the required guard is gone" + exit 1 + fi + if ! grep -q "monailabel.publicUrl is required" /tmp/monailabel-noguard.out; then + echo "::error::render without publicUrl failed, but not on the publicUrl guard:" + cat /tmp/monailabel-noguard.out + exit 1 + fi + # omopDb.vocabLoad.s3Bucket defaults to "" (the licensed bundle has no public # mirror — FLIP#842/843), so every other render in this job skips the # vocab-load Job. Without this step its ~110-line body is never rendered in diff --git a/AGENTS.md b/AGENTS.md index d6abe7522..bce5ca15e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -450,7 +450,7 @@ GitHub Actions: `test_flip_api.yml`, `test_flip_ui.yml`, `test_trust_*.yml`, `fl ### Docker image builds: gated on tests, manual trigger for branches -**The application `docker_build_*.yml` workflows (`flip_api`, `trust_trust_api`, `trust_imaging_api`, `trust_data_access_api`, `omop_db`) auto-publish to GHCR only after their service's test workflow passes on `develop` or `main`.** They trigger via `workflow_run` on the matching test workflow (`FLIP API CI`, `Trust - Trust API CI`, etc.) and a job-level `if` gates on `workflow_run.conclusion == 'success'` — a red test suite never publishes. Path filtering is inherited from the test workflow, so a build still only fires when that service changed. (`orthanc`, `xnat_*` keep their direct push trigger — they have no separate test workflow to gate on; `orthanc` instead runs an in-job auth smoke test between build and push, and also on PRs touching `trust/orthanc/**`, so a red smoke never publishes — FLIP-PT-091; `flip-ui` is a CI smoke test that never publishes.) +**The application `docker_build_*.yml` workflows (`flip_api`, `trust_trust_api`, `trust_imaging_api`, `trust_data_access_api`, `omop_db`) auto-publish to GHCR only after their service's test workflow passes on `develop` or `main`.** They trigger via `workflow_run` on the matching test workflow (`FLIP API CI`, `Trust - Trust API CI`, etc.) and a job-level `if` gates on `workflow_run.conclusion == 'success'` — a red test suite never publishes. Path filtering is inherited from the test workflow, so a build still only fires when that service changed. (`orthanc`, `xnat_*`, `monailabel` keep their direct push trigger — they have no separate test workflow to gate on; `orthanc` instead runs an in-job auth smoke test between build and push, and also on PRs touching `trust/orthanc/**`, so a red smoke never publishes — FLIP-PT-091; `monailabel` likewise runs an in-job import smoke and reclaims runner disk first, its image being ~10.4 GB against ubuntu-latest's ~14 GB free; `flip-ui` is a CI smoke test that never publishes.) Every publish also pushes an immutable **`sha-`** tag (first 7 chars of the built commit) alongside the mutable `:stag`/`:prod` tags. Hub ECS deploys pin these sha tags via task-definition revisions — `make deploy-centralhub` resolves the env branch tip's tag, `make rollback-centralhub` repoints at the previous revision (FLIP#751; see `deploy/providers/AWS/README.md` "Central Hub deploys and rollback"). `deploy-centralhub` also prints an **FL quiesce reminder** (FLIP#770; on `PROD=true` it adds an interactive are-you-sure confirmation, stag stays non-interactive): replacing `fl-server-net-1` kills any in-flight training run, so enable deployment mode first — it pauses FL job pickup (queued jobs hold; the running job finishes and frees its net) — and wait until the hub's `GET /fl/quiesce` reports deployment mode ON and no BUSY net, making "enable mode → wait → deploy → disable" the standard redeploy workflow. diff --git a/CLAUDE.md b/CLAUDE.md index 21fd4a5c9..f2b215a3f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -450,7 +450,7 @@ GitHub Actions: `test_flip_api.yml`, `test_flip_ui.yml`, `test_trust_*.yml`, `fl ### Docker image builds: gated on tests, manual trigger for branches -**The application `docker_build_*.yml` workflows (`flip_api`, `trust_trust_api`, `trust_imaging_api`, `trust_data_access_api`, `omop_db`) auto-publish to GHCR only after their service's test workflow passes on `develop` or `main`.** They trigger via `workflow_run` on the matching test workflow (`FLIP API CI`, `Trust - Trust API CI`, etc.) and a job-level `if` gates on `workflow_run.conclusion == 'success'` — a red test suite never publishes. Path filtering is inherited from the test workflow, so a build still only fires when that service changed. (`orthanc`, `xnat_*` keep their direct push trigger — they have no separate test workflow to gate on; `orthanc` instead runs an in-job auth smoke test between build and push, and also on PRs touching `trust/orthanc/**`, so a red smoke never publishes — FLIP-PT-091; `flip-ui` is a CI smoke test that never publishes.) +**The application `docker_build_*.yml` workflows (`flip_api`, `trust_trust_api`, `trust_imaging_api`, `trust_data_access_api`, `omop_db`) auto-publish to GHCR only after their service's test workflow passes on `develop` or `main`.** They trigger via `workflow_run` on the matching test workflow (`FLIP API CI`, `Trust - Trust API CI`, etc.) and a job-level `if` gates on `workflow_run.conclusion == 'success'` — a red test suite never publishes. Path filtering is inherited from the test workflow, so a build still only fires when that service changed. (`orthanc`, `xnat_*`, `monailabel` keep their direct push trigger — they have no separate test workflow to gate on; `orthanc` instead runs an in-job auth smoke test between build and push, and also on PRs touching `trust/orthanc/**`, so a red smoke never publishes — FLIP-PT-091; `monailabel` likewise runs an in-job import smoke and reclaims runner disk first, its image being ~10.4 GB against ubuntu-latest's ~14 GB free; `flip-ui` is a CI smoke test that never publishes.) Every publish also pushes an immutable **`sha-`** tag (first 7 chars of the built commit) alongside the mutable `:stag`/`:prod` tags. Hub ECS deploys pin these sha tags via task-definition revisions — `make deploy-centralhub` resolves the env branch tip's tag, `make rollback-centralhub` repoints at the previous revision (FLIP#751; see `deploy/providers/AWS/README.md` "Central Hub deploys and rollback"). `deploy-centralhub` also prints an **FL quiesce reminder** (FLIP#770; on `PROD=true` it adds an interactive are-you-sure confirmation, stag stays non-interactive): replacing `fl-server-net-1` kills any in-flight training run, so enable deployment mode first — it pauses FL job pickup (queued jobs hold; the running job finishes and frees its net) — and wait until the hub's `GET /fl/quiesce` reports deployment mode ON and no BUSY net, making "enable mode → wait → deploy → disable" the standard redeploy workflow. diff --git a/deploy/providers/kubernetes/README.md b/deploy/providers/kubernetes/README.md index 67d922c61..3fa8cc99c 100644 --- a/deploy/providers/kubernetes/README.md +++ b/deploy/providers/kubernetes/README.md @@ -210,6 +210,48 @@ Available services: | `observability.loki` | Log aggregation | Yes | | `observability.alloy` | Log collection agent | No (DaemonSet) | | `observability.grafana` | Metrics dashboard | Yes | +| `monailabel` | Optional AI-assisted annotation in the XNAT OHIF viewer (off by default) | Yes | + +### MONAI Label (optional) + +Off by default — needs an NVIDIA GPU node and a ~10.4 GB image. Enable with: + +```yaml +monailabel: + enabled: true + publicUrl: "http://:30030" # REQUIRED: the clinician's BROWSER calls this — + # XNAT stores it and never proxies it +``` + +Chart-specific notes (the full operational guide, including the stock-viewer quirks, is +[trust/README.md#monai-label-optional](../../../trust/README.md#monai-label-optional)): + +- Reads DICOM straight off xnat-web's archive PVC (read-only `archive` subPath). With the + default `ReadWriteOnce` storage the pod is pinned to xnat-web's node + (`coScheduleWithXnat: true`); only set it `false` on an RWX storage class. +- Exposed as a `NodePort` (default `30030`) so the browser can reach it. When + `service.allowExternalIngress` and `networkPolicies.enabled` are set, a NetworkPolicy opens + the **pod** port (`monailabel.port`, default `8030`) through the namespace's default-deny + ingress — NodePort traffic is DNAT'd to the pod port before policy evaluation, so that is + the port the rule names, not `30030`. +- **Scope that policy with `monailabel.allowedIngressCIDRs`.** Left empty (the default) the + ingress rule has no `from:`, and a NetworkPolicy rule without `from:` matches **every** + source, in and out of cluster. That is the fallback because the chart cannot know where + clinicians browse from, but the API behind it is unauthenticated (`monailabel.authEnable` + is upstream's own switch and needs an OAuth realm FLIP does not run) and holds the XNAT + service-account credentials. Set the subnets your clinicians use, e.g. + `allowedIngressCIDRs: ["10.0.0.0/8"]`. +- Pretrained weights (incl. the ~900 MB SAM checkpoint) persist in the + `-monailabel-models` PVC; first start on a cold volume takes minutes (the startup + probe allows 30). +- The chart's XNAT already ships the `ohif-viewer` plugin in its roster (`xnat.web.plugins`), + so no extra plugin step — unlike the compose trust, which deliberately excludes it (FLIP#662). +- **No `export_mask` converter here.** The compose trust registers a container-service command + that turns saved DICOM-SEG assessors into the NIfTI that FL training consumes + (`trust/xnat/xnat/config/configure-export-mask.sh`, run from `xnat-configure`); the chart's + `xnat-init-job` has no equivalent. So on Kubernetes the annotation UI works but masks are + not auto-converted — the inverse of the compose trust, which has the converter and excludes + the viewer. ### External Service Override @@ -469,13 +511,14 @@ old install on the previous chart version. - **Pod Security & container hardening**: the chart-created namespace carries Pod Security Standards labels (`enforce=baseline`, `warn`/`audit=restricted` by default — tune via `podSecurity.*`), and the stateless services - (trust-api, imaging-api, data-access-api, fl-client) + (trust-api, imaging-api, data-access-api, fl-client) plus `monailabel` apply a container `securityContext` (`allowPrivilegeEscalation: false`, drop `ALL` capabilities, `seccompProfile: RuntimeDefault`) from `.Values.securityContext`. `runAsNonRoot` / `readOnlyRootFilesystem` are left opt-in (image-dependent). - **Remaining for full `restricted` enforcement:** the stateful images - (`xnat-web`, `xnat-db`, `omop-db`, `orthanc`) need `fsGroup`/chown init - containers before they can run non-root. + **Remaining for full `restricted` enforcement:** the images that own a + PersistentVolume (`xnat-web`, `xnat-db`, `omop-db`, `orthanc`, and `monailabel` + — whose weights PVC the server writes at model-load time) need `fsGroup`/chown + init containers before they can run non-root. ## Development diff --git a/deploy/providers/kubernetes/templates/monailabel.yaml b/deploy/providers/kubernetes/templates/monailabel.yaml new file mode 100644 index 000000000..4689e7260 --- /dev/null +++ b/deploy/providers/kubernetes/templates/monailabel.yaml @@ -0,0 +1,271 @@ +# Copyright (c) 2026 Guy's and St Thomas' NHS Foundation Trust & King's College London +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +{{- if .Values.monailabel.enabled }} +# Optional MONAI Label server: AI-assisted annotation (SAM click-to-segment, DeepEdit +# multi-organ) in the XNAT OHIF viewer. Mirrors the compose overlay +# (trust/deploy/compose_trust.*.monailabel.yml) — see trust/README.md#monai-label-optional +# for the operational caveats (browser-reachable URL, unauthenticated API, stock-viewer +# quirks). Unlike compose, the chart's XNAT already ships the ohif-viewer plugin in its +# roster, so there is no separate plugin step here. +# +# The entrypoint handles XNAT registration itself: it polls XNAT and the service account, +# starts the server, then PUTs monailabel.publicUrl to /xapi/ohifaiaa/servers and fails +# loudly if the registration is rejected. +apiVersion: apps/v1 +kind: Deployment +metadata: + name: {{ include "flip-trust.fullname" . }}-monailabel + namespace: {{ include "flip-trust.namespace" . }} + labels: + {{- include "flip-trust.labels" . | nindent 4 }} + app.kubernetes.io/component: monailabel +spec: + replicas: 1 + # Recreate: the weights PVC is ReadWriteOnce and the server holds GPU memory — a rolling + # update would deadlock on both. + strategy: + type: Recreate + selector: + matchLabels: + {{- include "flip-trust.selectorLabels" . | nindent 6 }} + app.kubernetes.io/component: monailabel + template: + metadata: + labels: + {{- include "flip-trust.selectorLabels" . | nindent 8 }} + app.kubernetes.io/component: monailabel + spec: + enableServiceLinks: false + {{- if .Values.imagePullSecrets }} + imagePullSecrets: +{{ include "flip-trust.imagePullSecrets" . | nindent 8 }} + {{- end }} + {{- if .Values.monailabel.gpu.enabled }} + tolerations: + - key: nvidia.com/gpu + operator: Exists + effect: NoSchedule + {{- end }} + {{- if .Values.monailabel.coScheduleWithXnat }} + # The XNAT archive lives on xnat-web's data PVC. With the default ReadWriteOnce + # access mode that volume can only be mounted on the node where xnat-web runs, so + # this pod must land there too. Set coScheduleWithXnat: false only on an RWX + # storage class. + affinity: + podAffinity: + requiredDuringSchedulingIgnoredDuringExecution: + - labelSelector: + matchLabels: + {{- include "flip-trust.selectorLabels" . | nindent 18 }} + app.kubernetes.io/component: xnat-web + topologyKey: kubernetes.io/hostname + {{- end }} + containers: + - name: monailabel + image: "{{ .Values.monailabel.image.repository }}:{{ .Values.monailabel.image.tag }}" + imagePullPolicy: {{ .Values.monailabel.image.pullPolicy }} + {{- /* + Same chart-global container hardening the other network-facing services apply + (allowPrivilegeEscalation: false, drop ALL, seccompProfile: RuntimeDefault). + These defaults are image-agnostic — they do not require a non-root image, so they + are safe here even though this image still runs as root. runAsNonRoot for the + PVC-holding services is FLIP#530 work (needs fsGroup/chown), and MONAI Label is + listed there with the other stateful images. + */}} + {{- with .Values.securityContext }} + securityContext: + {{- toYaml . | nindent 12 }} + {{- end }} + env: + - name: MONAI_LABEL_PORT + value: {{ .Values.monailabel.port | quote }} + - name: MONAI_LABEL_DATASTORE + value: "xnat" + - name: MONAI_LABEL_DATASTORE_URL + value: "http://xnat-web:8080" + - name: MONAI_LABEL_DATASTORE_USERNAME + valueFrom: + secretKeyRef: + name: {{ if .Values.secrets.create }}{{ include "flip-trust.fullname" . }}-secrets{{ else }}{{ .Values.secrets.existingName }}{{ end }} + key: xnat-service-user + - name: MONAI_LABEL_DATASTORE_PASSWORD + valueFrom: + secretKeyRef: + name: {{ if .Values.secrets.create }}{{ include "flip-trust.fullname" . }}-secrets{{ else }}{{ .Values.secrets.existingName }}{{ end }} + key: xnat-service-password + # Mounted at the archive itself, not its parent: the datastore resolves files + # as + , so a parent + # path silently misses and falls back to per-scan HTTP downloads. + - name: MONAI_LABEL_DATASTORE_ASSET_PATH + value: "/workspace/xnat-data/archive" + - name: MONAI_LABEL_DATASTORE_PROJECT + value: {{ .Values.monailabel.projects | quote }} + - name: MONAI_LABEL_MODELS + value: {{ .Values.monailabel.models | quote }} + # Handed to the OHIF viewer, which calls it from the clinician's BROWSER — + # XNAT never proxies it. Must resolve outside the cluster, which the chart + # cannot derive, so it is required rather than defaulted. + - name: MONAI_LABEL_PUBLIC_URL + value: {{ required "monailabel.publicUrl is required when monailabel.enabled — the URL the clinician's browser uses to reach the MONAI Label service (e.g. http://:)" .Values.monailabel.publicUrl | quote }} + # Explicit, not inherited. MONAI Label defaults this to False and its RBAC + # dependencies are no-ops while it is, so every /datastore route is open to + # whoever can reach the port. Enabling it needs an OAuth realm FLIP does not run, + # so containment is monailabel.allowedIngressCIDRs on the NetworkPolicy below. + - name: MONAI_LABEL_AUTH_ENABLE + value: {{ .Values.monailabel.authEnable | quote }} + ports: + - name: http + containerPort: {{ .Values.monailabel.port }} + # Probes hit /openapi.json (answers in ms), NOT /info/: with the XNAT datastore, + # /info/ enumerates every experiment on the trust — one XML request each — so on a + # populated trust it cannot answer inside any sane probe timeout, the pod never + # goes Ready, and the Service refuses connections while the server inside is + # actually up (found live on a 900-experiment trust). + # Model load downloads pretrained weights on a cold volume (minutes, incl. the + # ~900MB SAM checkpoint); the startup probe gives it half an hour before the + # readiness probe takes over. + startupProbe: + httpGet: + path: /openapi.json + port: http + periodSeconds: 15 + failureThreshold: 120 + timeoutSeconds: 5 + readinessProbe: + httpGet: + path: /openapi.json + port: http + periodSeconds: 30 + timeoutSeconds: 5 + {{- /* + Merge the GPU reservation INTO the operator's resources rather than emitting a + second `limits:` block. Emitting both produces two sibling `limits:` keys under + `resources:`, and YAML resolves duplicates last-wins — so any operator who sets + monailabel.resources.limits (e.g. a memory cap, near-mandatory for a torch server + this size) would silently lose nvidia.com/gpu. The pod still tolerates the GPU + taint, so it lands on the GPU node looking correct and runs on CPU. + merge's dest wins, so an explicit operator nvidia.com/gpu overrides gpu.count. + */}} + {{- $resources := deepCopy (.Values.monailabel.resources | default dict) }} + {{- if .Values.monailabel.gpu.enabled }} + {{- $gpuLimit := dict "nvidia.com/gpu" (.Values.monailabel.gpu.count | toString) }} + {{- $_ := set $resources "limits" (merge ($resources.limits | default dict) $gpuLimit) }} + {{- end }} + resources: + {{- toYaml $resources | nindent 12 }} + volumeMounts: + - name: xnat-data + mountPath: /workspace/xnat-data/archive + subPath: archive + # Labels are written back through the XNAT REST API, never the filesystem. + readOnly: true + - name: models + mountPath: /workspace/app/radiology/model + - name: shm + mountPath: /dev/shm + volumes: + - name: xnat-data + persistentVolumeClaim: + claimName: {{ include "flip-trust.fullname" . }}-xnat-web + - name: models + persistentVolumeClaim: + claimName: {{ include "flip-trust.fullname" . }}-monailabel-models + - name: shm + emptyDir: + medium: Memory + sizeLimit: {{ .Values.monailabel.shmSize }} +--- +apiVersion: v1 +kind: Service +metadata: + name: monailabel + namespace: {{ include "flip-trust.namespace" . }} + labels: + {{- include "flip-trust.labels" . | nindent 4 }} + app.kubernetes.io/component: monailabel +spec: + # The clinician's browser must reach this directly (see MONAI_LABEL_PUBLIC_URL above), + # so NodePort/LoadBalancer is the working default posture; ClusterIP only makes sense + # behind an ingress that fronts it. + type: {{ .Values.monailabel.service.type }} + selector: + {{- include "flip-trust.selectorLabels" . | nindent 4 }} + app.kubernetes.io/component: monailabel + ports: + - name: http + port: {{ .Values.monailabel.port }} + targetPort: http + {{- if and (eq .Values.monailabel.service.type "NodePort") .Values.monailabel.service.nodePort }} + nodePort: {{ .Values.monailabel.service.nodePort }} + {{- end }} +--- +# Pretrained model weights (incl. the SAM checkpoint) — fetched on first start, persisted +# so a pod restart does not re-download them. +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: {{ include "flip-trust.fullname" . }}-monailabel-models + namespace: {{ include "flip-trust.namespace" . }} + labels: + {{- include "flip-trust.labels" . | nindent 4 }} + app.kubernetes.io/component: monailabel +spec: + accessModes: + - ReadWriteOnce + {{- if .Values.monailabel.persistence.storageClassName }} + storageClassName: {{ .Values.monailabel.persistence.storageClassName }} + {{- end }} + resources: + requests: + storage: {{ .Values.monailabel.persistence.size }} +{{- if and .Values.networkPolicies.enabled .Values.monailabel.service.allowExternalIngress }} +--- +# The namespace default-denies ingress; the MONAI Label API must be reachable from the +# clinician's browser (outside the cluster), so open exactly its port on exactly its pod. +# The API is unauthenticated and holds the XNAT service-account credentials — matching the +# compose deployment's posture, where the port is host-published; restrict who can reach +# the node port at the network layer. +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: {{ include "flip-trust.fullname" . }}-monailabel-ingress + namespace: {{ include "flip-trust.namespace" . }} + labels: + {{- include "flip-trust.labels" . | nindent 4 }} +spec: + podSelector: + matchLabels: + {{- include "flip-trust.selectorLabels" . | nindent 6 }} + app.kubernetes.io/component: monailabel + ingress: + - ports: + - port: {{ .Values.monailabel.port }} + protocol: TCP + {{- /* + An ingress rule with no `from:` allows EVERY source, in and out of cluster. That is + the fallback when allowedIngressCIDRs is empty, because the clinician's browser calls + this pod directly and the chart cannot know where from. Set allowedIngressCIDRs to the + subnets your clinicians actually browse from and the rule scopes to them instead — + strongly recommended, since the API behind it is unauthenticated and holds the XNAT + service-account credentials. + */}} + {{- with .Values.monailabel.allowedIngressCIDRs }} + from: + {{- range . }} + - ipBlock: + cidr: {{ . | quote }} + {{- end }} + {{- end }} + policyTypes: + - Ingress +{{- end }} +{{- end }} diff --git a/deploy/providers/kubernetes/values.schema.json b/deploy/providers/kubernetes/values.schema.json index 9ce746c52..424b0efc8 100644 --- a/deploy/providers/kubernetes/values.schema.json +++ b/deploy/providers/kubernetes/values.schema.json @@ -4,7 +4,10 @@ "description": "Schema for values.yaml of the flip-trust Helm chart", "type": "object", "additionalProperties": true, - "required": ["trustName", "trustNumber"], + "required": [ + "trustName", + "trustNumber" + ], "properties": { "trustName": { "type": "string", @@ -17,7 +20,11 @@ }, "environment": { "type": "string", - "enum": ["production", "staging", "development"], + "enum": [ + "production", + "staging", + "development" + ], "description": "Deployment environment" }, "logLevel": { @@ -30,23 +37,34 @@ }, "flBackend": { "type": "string", - "enum": ["nvflare", "flower"], + "enum": [ + "nvflare", + "flower" + ], "description": "FL backend selection" }, "namespace": { "type": "object", "additionalProperties": true, "properties": { - "create": { "type": "boolean" }, - "name": { "type": "string" } + "create": { + "type": "boolean" + }, + "name": { + "type": "string" + } } }, "secrets": { "type": "object", "additionalProperties": true, "properties": { - "create": { "type": "boolean" }, - "existingName": { "type": "string" }, + "create": { + "type": "boolean" + }, + "existingName": { + "type": "string" + }, "data": { "type": "object", "additionalProperties": true @@ -57,8 +75,13 @@ "type": "object", "additionalProperties": true, "properties": { - "enabled": { "type": "boolean" }, - "replicas": { "type": "integer", "minimum": 1 }, + "enabled": { + "type": "boolean" + }, + "replicas": { + "type": "integer", + "minimum": 1 + }, "resources": { "type": "object", "additionalProperties": true, @@ -66,15 +89,23 @@ "requests": { "type": "object", "properties": { - "memory": { "type": "string" }, - "cpu": { "type": "string" } + "memory": { + "type": "string" + }, + "cpu": { + "type": "string" + } } }, "limits": { "type": "object", "properties": { - "memory": { "type": "string" }, - "cpu": { "type": "string" } + "memory": { + "type": "string" + }, + "cpu": { + "type": "string" + } } } } @@ -86,17 +117,29 @@ "liveness": { "type": "object", "properties": { - "initialDelaySeconds": { "type": "integer" }, - "periodSeconds": { "type": "integer" }, - "timeoutSeconds": { "type": "integer" } + "initialDelaySeconds": { + "type": "integer" + }, + "periodSeconds": { + "type": "integer" + }, + "timeoutSeconds": { + "type": "integer" + } } }, "readiness": { "type": "object", "properties": { - "initialDelaySeconds": { "type": "integer" }, - "periodSeconds": { "type": "integer" }, - "timeoutSeconds": { "type": "integer" } + "initialDelaySeconds": { + "type": "integer" + }, + "periodSeconds": { + "type": "integer" + }, + "timeoutSeconds": { + "type": "integer" + } } } } @@ -104,16 +147,26 @@ "image": { "type": "object", "properties": { - "repository": { "type": "string" }, - "tag": { "type": "string" }, - "pullPolicy": { "type": "string" } + "repository": { + "type": "string" + }, + "tag": { + "type": "string" + }, + "pullPolicy": { + "type": "string" + } } }, "service": { "type": "object", "properties": { - "port": { "type": "integer" }, - "type": { "type": "string" } + "port": { + "type": "integer" + }, + "type": { + "type": "string" + } } } } @@ -122,8 +175,13 @@ "type": "object", "additionalProperties": true, "properties": { - "enabled": { "type": "boolean" }, - "replicas": { "type": "integer", "minimum": 1 }, + "enabled": { + "type": "boolean" + }, + "replicas": { + "type": "integer", + "minimum": 1 + }, "resources": { "type": "object", "additionalProperties": true, @@ -131,15 +189,23 @@ "requests": { "type": "object", "properties": { - "memory": { "type": "string" }, - "cpu": { "type": "string" } + "memory": { + "type": "string" + }, + "cpu": { + "type": "string" + } } }, "limits": { "type": "object", "properties": { - "memory": { "type": "string" }, - "cpu": { "type": "string" } + "memory": { + "type": "string" + }, + "cpu": { + "type": "string" + } } } } @@ -151,17 +217,29 @@ "liveness": { "type": "object", "properties": { - "initialDelaySeconds": { "type": "integer" }, - "periodSeconds": { "type": "integer" }, - "timeoutSeconds": { "type": "integer" } + "initialDelaySeconds": { + "type": "integer" + }, + "periodSeconds": { + "type": "integer" + }, + "timeoutSeconds": { + "type": "integer" + } } }, "readiness": { "type": "object", "properties": { - "initialDelaySeconds": { "type": "integer" }, - "periodSeconds": { "type": "integer" }, - "timeoutSeconds": { "type": "integer" } + "initialDelaySeconds": { + "type": "integer" + }, + "periodSeconds": { + "type": "integer" + }, + "timeoutSeconds": { + "type": "integer" + } } } } @@ -169,16 +247,26 @@ "image": { "type": "object", "properties": { - "repository": { "type": "string" }, - "tag": { "type": "string" }, - "pullPolicy": { "type": "string" } + "repository": { + "type": "string" + }, + "tag": { + "type": "string" + }, + "pullPolicy": { + "type": "string" + } } }, "service": { "type": "object", "properties": { - "port": { "type": "integer" }, - "type": { "type": "string" } + "port": { + "type": "integer" + }, + "type": { + "type": "string" + } } } } @@ -187,8 +275,13 @@ "type": "object", "additionalProperties": true, "properties": { - "enabled": { "type": "boolean" }, - "replicas": { "type": "integer", "minimum": 1 }, + "enabled": { + "type": "boolean" + }, + "replicas": { + "type": "integer", + "minimum": 1 + }, "resources": { "type": "object", "additionalProperties": true, @@ -196,15 +289,23 @@ "requests": { "type": "object", "properties": { - "memory": { "type": "string" }, - "cpu": { "type": "string" } + "memory": { + "type": "string" + }, + "cpu": { + "type": "string" + } } }, "limits": { "type": "object", "properties": { - "memory": { "type": "string" }, - "cpu": { "type": "string" } + "memory": { + "type": "string" + }, + "cpu": { + "type": "string" + } } } } @@ -216,17 +317,29 @@ "liveness": { "type": "object", "properties": { - "initialDelaySeconds": { "type": "integer" }, - "periodSeconds": { "type": "integer" }, - "timeoutSeconds": { "type": "integer" } + "initialDelaySeconds": { + "type": "integer" + }, + "periodSeconds": { + "type": "integer" + }, + "timeoutSeconds": { + "type": "integer" + } } }, "readiness": { "type": "object", "properties": { - "initialDelaySeconds": { "type": "integer" }, - "periodSeconds": { "type": "integer" }, - "timeoutSeconds": { "type": "integer" } + "initialDelaySeconds": { + "type": "integer" + }, + "periodSeconds": { + "type": "integer" + }, + "timeoutSeconds": { + "type": "integer" + } } } } @@ -234,16 +347,26 @@ "image": { "type": "object", "properties": { - "repository": { "type": "string" }, - "tag": { "type": "string" }, - "pullPolicy": { "type": "string" } + "repository": { + "type": "string" + }, + "tag": { + "type": "string" + }, + "pullPolicy": { + "type": "string" + } } }, "service": { "type": "object", "properties": { - "port": { "type": "integer" }, - "type": { "type": "string" } + "port": { + "type": "integer" + }, + "type": { + "type": "string" + } } } } @@ -252,22 +375,32 @@ "type": "object", "additionalProperties": true, "properties": { - "enabled": { "type": "boolean" }, + "enabled": { + "type": "boolean" + }, "resources": { "type": "object", "properties": { "requests": { "type": "object", "properties": { - "memory": { "type": "string" }, - "cpu": { "type": "string" } + "memory": { + "type": "string" + }, + "cpu": { + "type": "string" + } } }, "limits": { "type": "object", "properties": { - "memory": { "type": "string" }, - "cpu": { "type": "string" } + "memory": { + "type": "string" + }, + "cpu": { + "type": "string" + } } } } @@ -275,8 +408,12 @@ "gpu": { "type": "object", "properties": { - "enabled": { "type": "boolean" }, - "count": { "type": "integer" } + "enabled": { + "type": "boolean" + }, + "count": { + "type": "integer" + } } } } @@ -285,16 +422,33 @@ "type": "object", "additionalProperties": true, "properties": { - "enabled": { "type": "boolean" }, + "enabled": { + "type": "boolean" + }, "vocabLoad": { "type": "object", "properties": { - "enabled": { "type": "boolean" }, - "s3Bucket": { "type": "string" }, - "bundleName": { "type": "string" }, - "workDirSize": { "type": "string", "minLength": 1 }, - "fetchResources": { "type": "object", "minProperties": 1 }, - "loadResources": { "type": "object", "minProperties": 1 } + "enabled": { + "type": "boolean" + }, + "s3Bucket": { + "type": "string" + }, + "bundleName": { + "type": "string" + }, + "workDirSize": { + "type": "string", + "minLength": 1 + }, + "fetchResources": { + "type": "object", + "minProperties": 1 + }, + "loadResources": { + "type": "object", + "minProperties": 1 + } } }, "resources": { @@ -303,15 +457,23 @@ "requests": { "type": "object", "properties": { - "memory": { "type": "string" }, - "cpu": { "type": "string" } + "memory": { + "type": "string" + }, + "cpu": { + "type": "string" + } } }, "limits": { "type": "object", "properties": { - "memory": { "type": "string" }, - "cpu": { "type": "string" } + "memory": { + "type": "string" + }, + "cpu": { + "type": "string" + } } } } @@ -322,23 +484,36 @@ "type": "object", "additionalProperties": true, "properties": { - "enabled": { "type": "boolean" }, - "replicas": { "type": "integer", "minimum": 1 }, + "enabled": { + "type": "boolean" + }, + "replicas": { + "type": "integer", + "minimum": 1 + }, "resources": { "type": "object", "properties": { "requests": { "type": "object", "properties": { - "memory": { "type": "string" }, - "cpu": { "type": "string" } + "memory": { + "type": "string" + }, + "cpu": { + "type": "string" + } } }, "limits": { "type": "object", "properties": { - "memory": { "type": "string" }, - "cpu": { "type": "string" } + "memory": { + "type": "string" + }, + "cpu": { + "type": "string" + } } } } @@ -349,26 +524,38 @@ "type": "object", "additionalProperties": true, "properties": { - "enabled": { "type": "boolean" }, + "enabled": { + "type": "boolean" + }, "web": { "type": "object", "properties": { - "enabled": { "type": "boolean" }, + "enabled": { + "type": "boolean" + }, "resources": { "type": "object", "properties": { "requests": { "type": "object", "properties": { - "memory": { "type": "string" }, - "cpu": { "type": "string" } + "memory": { + "type": "string" + }, + "cpu": { + "type": "string" + } } }, "limits": { "type": "object", "properties": { - "memory": { "type": "string" }, - "cpu": { "type": "string" } + "memory": { + "type": "string" + }, + "cpu": { + "type": "string" + } } } } @@ -384,19 +571,25 @@ "loki": { "type": "object", "properties": { - "enabled": { "type": "boolean" } + "enabled": { + "type": "boolean" + } } }, "alloy": { "type": "object", "properties": { - "enabled": { "type": "boolean" } + "enabled": { + "type": "boolean" + } } }, "grafana": { "type": "object", "properties": { - "enabled": { "type": "boolean" } + "enabled": { + "type": "boolean" + } } } } @@ -405,28 +598,167 @@ "type": "object", "additionalProperties": true, "properties": { - "enabled": { "type": "boolean" }, - "minReplicas": { "type": "integer" }, - "maxReplicas": { "type": "integer" } + "enabled": { + "type": "boolean" + }, + "minReplicas": { + "type": "integer" + }, + "maxReplicas": { + "type": "integer" + } } }, "networkPolicies": { "type": "object", "additionalProperties": true, "properties": { - "enabled": { "type": "boolean" }, + "enabled": { + "type": "boolean" + }, "allowedEgressCIDRsWithPorts": { "type": "array", "description": "Egress rules combining CIDRs with a specific port (e.g. fl-server gRPC on 8002)", "items": { "type": "object", - "required": ["cidrs", "port"], + "required": [ + "cidrs", + "port" + ], "properties": { - "cidrs": { "type": "array", "items": { "type": "string" } }, - "port": { "type": "integer" }, - "protocol": { "type": "string", "enum": ["TCP", "UDP", "SCTP"] } + "cidrs": { + "type": "array", + "items": { + "type": "string" + } + }, + "port": { + "type": "integer" + }, + "protocol": { + "type": "string", + "enum": [ + "TCP", + "UDP", + "SCTP" + ] + } + } + } + } + } + }, + "monailabel": { + "type": "object", + "properties": { + "enabled": { + "type": "boolean" + }, + "image": { + "type": "object", + "properties": { + "repository": { + "type": "string" + }, + "tag": { + "type": "string" + }, + "pullPolicy": { + "type": "string", + "enum": [ + "Always", + "IfNotPresent", + "Never" + ] } } + }, + "port": { + "type": "integer", + "minimum": 1, + "maximum": 65535 + }, + "models": { + "type": "string" + }, + "projects": { + "type": "string" + }, + "publicUrl": { + "type": "string" + }, + "shmSize": { + "type": "string" + }, + "gpu": { + "type": "object", + "properties": { + "enabled": { + "type": "boolean" + }, + "count": { + "type": "integer", + "minimum": 0 + } + } + }, + "coScheduleWithXnat": { + "type": "boolean" + }, + "service": { + "type": "object", + "properties": { + "type": { + "type": "string", + "enum": [ + "ClusterIP", + "NodePort", + "LoadBalancer" + ] + }, + "nodePort": { + "anyOf": [ + { + "type": "integer", + "minimum": 30000, + "maximum": 32767 + }, + { + "type": "null" + }, + { + "type": "string", + "maxLength": 0 + } + ] + }, + "allowExternalIngress": { + "type": "boolean" + } + } + }, + "allowedIngressCIDRs": { + "type": "array", + "items": { + "type": "string" + } + }, + "authEnable": { + "type": "boolean" + }, + "persistence": { + "type": "object", + "properties": { + "size": { + "type": "string" + }, + "storageClassName": { + "type": "string" + } + } + }, + "resources": { + "type": "object" } } } diff --git a/deploy/providers/kubernetes/values.yaml b/deploy/providers/kubernetes/values.yaml index 12a9d7776..7e61cf13f 100644 --- a/deploy/providers/kubernetes/values.yaml +++ b/deploy/providers/kubernetes/values.yaml @@ -615,7 +615,7 @@ xnat: container-service: "https://github.com/NrgXnat/container-service/releases/download/3.8.1/container-service-3.8.1-fat.jar" dicom-query-retrieve: "https://api.bitbucket.org/2.0/repositories/xnatdev/dicom-query-retrieve/downloads/dicom-query-retrieve-3.0.0-xpl.jar" batch-launch: "https://api.bitbucket.org/2.0/repositories/xnatx/xnatx-batch-launch-plugin/downloads/batch-launch-0.9.0-xpl.jar" - ohif-viewer: "https://xnat.org/files/ohif-viewer-xnat-plugin/ohif-viewer-3.7.2.jar" + ohif-viewer: "https://xnat.org/files/ohif-viewer-xnat-plugin/ohif-viewer-3.8.0-fat.jar" env: XNAT_EMAIL: "" XNAT_MIN_HEAP: "1024m" @@ -668,6 +668,63 @@ xnat: memory: "256Mi" cpu: "200m" +# --------------------------------------------------------------------------- +# MONAI Label — optional AI-assisted annotation in the XNAT OHIF viewer +# --------------------------------------------------------------------------- +# Off by default: needs an NVIDIA GPU node and pulls a ~10.4GB image. Mirrors the compose +# overlay (trust/deploy/compose_trust.*.monailabel.yml); operational caveats — the +# browser-reachable URL, the unauthenticated API, the stock-viewer quirks — are documented +# in trust/README.md#monai-label-optional. +monailabel: + enabled: false + image: + repository: ghcr.io/londonaicentre/monailabel + tag: stag + pullPolicy: IfNotPresent + port: 8030 + # Radiology models to load. 'all' loads nine, each downloading its own weights; the SAM + # tasks (sam_2d/sam_3d) register whenever the sam2 package is importable, independent of + # this list. + models: "deepedit" + # Comma-separated XNAT project allowlist; empty means every project on this trust. + projects: "" + # REQUIRED when enabled: the URL the clinician's BROWSER uses to reach the service + # (e.g. http://:30030). XNAT stores it and hands it to the OHIF viewer — it is + # never proxied, so a cluster-internal name will not work. + publicUrl: "" + # /dev/shm for dataloader workers (emptyDir, medium: Memory). + shmSize: 8Gi + gpu: + enabled: true + count: 1 + # Pin the pod to xnat-web's node so the archive PVC (ReadWriteOnce by default) can be + # mounted read-only here too. Set false only on an RWX storage class. + coScheduleWithXnat: true + service: + type: NodePort + # Fixed nodePort (30000-32767) so publicUrl stays stable; empty lets the cluster pick. + nodePort: 30030 + # Adds a NetworkPolicy opening the service port on this pod through the namespace's + # default-deny ingress — required for browser access when networkPolicies.enabled. + allowExternalIngress: true + # Sources allowed through that NetworkPolicy, as CIDRs. EMPTY MEANS EVERY SOURCE, in and + # out of cluster — a NetworkPolicy ingress rule with no `from:` matches all. That is the + # fallback because the clinician's browser calls this pod directly and the chart cannot + # know where from, but it is not a good default to leave in place: the API behind it is + # unauthenticated (see authEnable) and holds the XNAT service-account credentials. Set the + # subnets your clinicians browse from, e.g. ["10.0.0.0/8"]. + allowedIngressCIDRs: [] + # MONAI Label's own auth. Its default is False and its RBAC dependencies are no-ops while + # it is, so every /datastore route is open to whoever reaches the port. Enabling it needs + # an OAuth realm (MONAI_LABEL_AUTH_REALM_URI et al) that FLIP does not run — so treat + # allowedIngressCIDRs, not this, as the control until such a realm exists. + authEnable: false + persistence: + # Pretrained weights (incl. the ~900MB SAM checkpoint). + size: 20Gi + storageClassName: "" + resources: {} + # --------------------------------------------------------------------------- # Observability Stack — Loki, Alloy, Grafana # --------------------------------------------------------------------------- diff --git a/trust/.env.GSTT.development.example b/trust/.env.GSTT.development.example index 18165e5a2..0afc16e2f 100644 --- a/trust/.env.GSTT.development.example +++ b/trust/.env.GSTT.development.example @@ -85,6 +85,31 @@ NUM_AVAILABLE_GPUS=1 MEMORY_PER_GPU_IN_GIB=7 MIN_CLIENTS=1 +# ── MONAI Label (optional AI-assisted annotation in the XNAT OHIF viewer) ── +# Off by default: it needs an NVIDIA GPU (NUM_AVAILABLE_GPUS>0) and pulls a large image. +# Enable for this trust with `make up-trust KIT= MONAI_LABEL=true`, or set it here. +MONAI_LABEL=false +MONAI_LABEL_PORT=8030 +# Radiology models to load. 'all' loads nine, each downloading its own pretrained weights. +MONAI_LABEL_MODELS=deepedit +# XNAT projects the server may read. Comma-separated; empty means every project on this trust. +MONAI_LABEL_PROJECTS= +# The URL the OHIF viewer calls FROM THE CLINICIAN'S BROWSER — not a Docker-internal name. +# Empty falls back to http://localhost:$MONAI_LABEL_PORT, which only works when the browser +# runs on the trust host itself. +MONAI_LABEL_PUBLIC_URL= +MONAI_LABEL_SHM_SIZE=8gb +# Host interface MONAI_LABEL_PORT is published on. Loopback by default: the MONAI Label API +# is unauthenticated (see MONAI_LABEL_AUTH_ENABLE) and holds this trust's XNAT service-account +# credentials, so anyone who can reach the port can read every study and write labels back as +# an XNAT admin. To let clinicians reach it, either front it through the XNAT nginx (which also +# fixes mixed content on an HTTPS XNAT) or set this to an interface you have firewalled. +MONAI_LABEL_BIND_HOST=127.0.0.1 +# MONAI Label's own auth. Its upstream default is false and its RBAC checks are no-ops while +# it is. Enabling it needs an OAuth realm FLIP does not run, so keep the containment at +# MONAI_LABEL_BIND_HOST until one exists. +MONAI_LABEL_AUTH_ENABLE=false + # Site-enforced FL privacy policy (NVFLARE only; FLIP#851) — this trust's own # update-privacy filter, enforced regardless of the researcher's app config. # See trust/.env.example for the full parameter list; unset = no site policy. diff --git a/trust/.env.KCH.development.example b/trust/.env.KCH.development.example index 154239b0a..f5cd749c3 100644 --- a/trust/.env.KCH.development.example +++ b/trust/.env.KCH.development.example @@ -85,6 +85,31 @@ NUM_AVAILABLE_GPUS=1 MEMORY_PER_GPU_IN_GIB=7 MIN_CLIENTS=1 +# ── MONAI Label (optional AI-assisted annotation in the XNAT OHIF viewer) ── +# Off by default: it needs an NVIDIA GPU (NUM_AVAILABLE_GPUS>0) and pulls a large image. +# Enable for this trust with `make up-trust KIT= MONAI_LABEL=true`, or set it here. +MONAI_LABEL=false +MONAI_LABEL_PORT=8032 +# Radiology models to load. 'all' loads nine, each downloading its own pretrained weights. +MONAI_LABEL_MODELS=deepedit +# XNAT projects the server may read. Comma-separated; empty means every project on this trust. +MONAI_LABEL_PROJECTS= +# The URL the OHIF viewer calls FROM THE CLINICIAN'S BROWSER — not a Docker-internal name. +# Empty falls back to http://localhost:$MONAI_LABEL_PORT, which only works when the browser +# runs on the trust host itself. +MONAI_LABEL_PUBLIC_URL= +MONAI_LABEL_SHM_SIZE=8gb +# Host interface MONAI_LABEL_PORT is published on. Loopback by default: the MONAI Label API +# is unauthenticated (see MONAI_LABEL_AUTH_ENABLE) and holds this trust's XNAT service-account +# credentials, so anyone who can reach the port can read every study and write labels back as +# an XNAT admin. To let clinicians reach it, either front it through the XNAT nginx (which also +# fixes mixed content on an HTTPS XNAT) or set this to an interface you have firewalled. +MONAI_LABEL_BIND_HOST=127.0.0.1 +# MONAI Label's own auth. Its upstream default is false and its RBAC checks are no-ops while +# it is. Enabling it needs an OAuth realm FLIP does not run, so keep the containment at +# MONAI_LABEL_BIND_HOST until one exists. +MONAI_LABEL_AUTH_ENABLE=false + # Site-enforced FL privacy policy (NVFLARE only; FLIP#851) — this trust's own # update-privacy filter, enforced regardless of the researcher's app config. # See trust/.env.example for the full parameter list; unset = no site policy. diff --git a/trust/.env.example b/trust/.env.example index 21f69cc1b..46db5ab14 100644 --- a/trust/.env.example +++ b/trust/.env.example @@ -123,6 +123,33 @@ NUM_AVAILABLE_GPUS=1 MEMORY_PER_GPU_IN_GIB=7 MIN_CLIENTS=1 +# ── MONAI Label (optional AI-assisted annotation in the XNAT OHIF viewer) ── +# Off by default: it needs an NVIDIA GPU (NUM_AVAILABLE_GPUS>0), pulls a large image, and +# re-introduces the XNAT OHIF viewer plugin the trust XNAT deliberately excludes (FLIP#662). +# See trust/README.md#monai-label-optional before enabling. +MONAI_LABEL=false +# Host port. Give co-hosted trusts distinct values (the shipped dev pair uses 8030 / 8032). +MONAI_LABEL_PORT=8030 +# Radiology models to load. 'all' loads nine, each downloading its own pretrained weights. +MONAI_LABEL_MODELS=deepedit +# XNAT projects the server may read. Comma-separated; empty means every project on this trust. +MONAI_LABEL_PROJECTS= +# The URL the OHIF viewer calls FROM THE CLINICIAN'S BROWSER — not a Docker-internal name. +# Empty falls back to http://localhost:$MONAI_LABEL_PORT, which only works when the browser +# runs on the trust host itself. +MONAI_LABEL_PUBLIC_URL= +MONAI_LABEL_SHM_SIZE=8gb +# Host interface MONAI_LABEL_PORT is published on. Loopback by default: the MONAI Label API +# is unauthenticated (see MONAI_LABEL_AUTH_ENABLE) and holds this trust's XNAT service-account +# credentials, so anyone who can reach the port can read every study and write labels back as +# an XNAT admin. To let clinicians reach it, either front it through the XNAT nginx (which also +# fixes mixed content on an HTTPS XNAT) or set this to an interface you have firewalled. +MONAI_LABEL_BIND_HOST=127.0.0.1 +# MONAI Label's own auth. Its upstream default is false and its RBAC checks are no-ops while +# it is. Enabling it needs an OAuth realm FLIP does not run, so keep the containment at +# MONAI_LABEL_BIND_HOST until one exists. +MONAI_LABEL_AUTH_ENABLE=false + # Site-enforced FL privacy policy (NVFLARE backend only; FLIP#851). THIS trust's # own update-privacy filter, rendered into the fl-client's NVFLARE privacy.json # at container start: it composes ON TOP of whatever filter the researcher's app diff --git a/trust/AGENTS.md b/trust/AGENTS.md index 0052fd57c..2aac213f0 100644 --- a/trust/AGENTS.md +++ b/trust/AGENTS.md @@ -100,6 +100,7 @@ GHCR login from `~/.docker/config.json`. | `deploy/compose_trust.production.yml` | Prod Docker Compose (GHCR images; declares the `trust-local-{loki,grafana}-data` named volumes as defaults) | | `deploy/compose_trust.{env}.{flower\|nvflare}.yml` | FL backend variants | | `deploy/compose_trust.{env}.gpu.yml` | GPU passthrough overlay — added by `up-trust` / `up-fl-clients-kit` via `GPU_OVERRIDE` only when the kit's `NUM_AVAILABLE_GPUS > 0`; reserves host NVIDIA GPU(s) for the fl-client. `up-trust-ec2` never applies it (the EC2 t3.xlarge is GPU-less, so the fl-client is CPU-only there regardless of the kit) | +| `deploy/compose_trust.{env}.monailabel.yml` | Optional MONAI Label server (AI-assisted annotation in the XNAT OHIF viewer) — added by `up-trust` / `build` via `MONAILABEL_OVERRIDE` only when the kit (or CLI) sets `MONAI_LABEL=true`. Requires `NUM_AVAILABLE_GPUS > 0`; `up-trust` refuses otherwise. Dev builds from `trust/monailabel/`; production pulls `ghcr.io/londonaicentre/monailabel:${DOCKER_TAG}` (published by `docker_build_monailabel.yml`) and `up-trust` prints a FLIP#662 warning — enabling it re-introduces the OHIF viewer plugin the trust XNAT deliberately excludes. Bind-mounts the trust's XNAT archive read-only at the path `MONAI_LABEL_XNAT_ARCHIVE_DIR` resolves to — anchored at `xnat/` because the kit's `XNAT_DATA_DIR` is written relative to `trust/xnat/`. Published on `MONAI_LABEL_BIND_HOST` (default `127.0.0.1`) — the API is unauthenticated and holds the XNAT service account, so it is loopback-only until an operator fronts it through the XNAT nginx or firewalls an interface. `MONAI_LABEL=true` **also** gates `configure-export-mask.sh` in `xnat/Makefile`'s `xnat-configure` (the DICOM-SEG → NIfTI converter command + its site-wide subscription); enabling MONAI Label on an already-running trust needs a `make -C trust/xnat xnat-configure KIT=` re-run. `up-trust-ec2` refuses `MONAI_LABEL=true` outright (no GPU on the t3.xlarge). See [README.md](README.md#monai-label-optional) | | `deploy/compose_trust-1_override.yml` | Dev trust-1 host-port bindings | | `.env..` | Per-trust kit file, e.g. `.env.GSTT.development`, `.env..production` (TRUST_API_KEY, TRUST_INTERNAL_SERVICE_KEY, FL_KIT_SLOT, FL_KIT_SLOT_NUMBER, EXPECTED_TRUST_ID, host-local ports/dirs, **FL_KIT_DIR** — root of the FL participant kit, default `/opt/flip/fl-kit` matching the Ansible-staged EC2 path); gitignored. Templates: per-trust dev examples `.env.GSTT.development.example` / `.env.KCH.development.example`; the generic scaffold base `.env.example`, consumed by `make new-trust`. Same kit-file schema everywhere — `make -C trust up-trust KIT= PROD=` is the only dispatch | diff --git a/trust/CLAUDE.md b/trust/CLAUDE.md index 28710d842..2814fb320 100644 --- a/trust/CLAUDE.md +++ b/trust/CLAUDE.md @@ -100,6 +100,7 @@ GHCR login from `~/.docker/config.json`. | `deploy/compose_trust.production.yml` | Prod Docker Compose (GHCR images; declares the `trust-local-{loki,grafana}-data` named volumes as defaults) | | `deploy/compose_trust.{env}.{flower\|nvflare}.yml` | FL backend variants | | `deploy/compose_trust.{env}.gpu.yml` | GPU passthrough overlay — added by `up-trust` / `up-fl-clients-kit` via `GPU_OVERRIDE` only when the kit's `NUM_AVAILABLE_GPUS > 0`; reserves host NVIDIA GPU(s) for the fl-client. `up-trust-ec2` never applies it (the EC2 t3.xlarge is GPU-less, so the fl-client is CPU-only there regardless of the kit) | +| `deploy/compose_trust.{env}.monailabel.yml` | Optional MONAI Label server (AI-assisted annotation in the XNAT OHIF viewer) — added by `up-trust` / `build` via `MONAILABEL_OVERRIDE` only when the kit (or CLI) sets `MONAI_LABEL=true`. Requires `NUM_AVAILABLE_GPUS > 0`; `up-trust` refuses otherwise. Dev builds from `trust/monailabel/`; production pulls `ghcr.io/londonaicentre/monailabel:${DOCKER_TAG}` (published by `docker_build_monailabel.yml`) and `up-trust` prints a FLIP#662 warning — enabling it re-introduces the OHIF viewer plugin the trust XNAT deliberately excludes. Bind-mounts the trust's XNAT archive read-only at the path `MONAI_LABEL_XNAT_ARCHIVE_DIR` resolves to — anchored at `xnat/` because the kit's `XNAT_DATA_DIR` is written relative to `trust/xnat/`. Published on `MONAI_LABEL_BIND_HOST` (default `127.0.0.1`) — the API is unauthenticated and holds the XNAT service account, so it is loopback-only until an operator fronts it through the XNAT nginx or firewalls an interface. `MONAI_LABEL=true` **also** gates `configure-export-mask.sh` in `xnat/Makefile`'s `xnat-configure` (the DICOM-SEG → NIfTI converter command + its site-wide subscription); enabling MONAI Label on an already-running trust needs a `make -C trust/xnat xnat-configure KIT=` re-run. `up-trust-ec2` refuses `MONAI_LABEL=true` outright (no GPU on the t3.xlarge). See [README.md](README.md#monai-label-optional) | | `deploy/compose_trust-1_override.yml` | Dev trust-1 host-port bindings | | `.env..` | Per-trust kit file, e.g. `.env.GSTT.development`, `.env..production` (TRUST_API_KEY, TRUST_INTERNAL_SERVICE_KEY, FL_KIT_SLOT, FL_KIT_SLOT_NUMBER, EXPECTED_TRUST_ID, host-local ports/dirs, **FL_KIT_DIR** — root of the FL participant kit, default `/opt/flip/fl-kit` matching the Ansible-staged EC2 path); gitignored. Templates: per-trust dev examples `.env.GSTT.development.example` / `.env.KCH.development.example`; the generic scaffold base `.env.example`, consumed by `make new-trust`. Same kit-file schema everywhere — `make -C trust up-trust KIT= PROD=` is the only dispatch | diff --git a/trust/Makefile b/trust/Makefile index 938ffd584..ba36b1564 100644 --- a/trust/Makefile +++ b/trust/Makefile @@ -105,7 +105,14 @@ ORTHANC_CMD=make -C ./orthanc # (compose tolerates absence of --env-file just fine). __ENV_FILE_FLAG := $(if $(wildcard $(KIT_FILE)),--env-file $(KIT_FILE),) DOCKER_COMPOSE_CMD=docker compose $(__ENV_FILE_FLAG) --project-directory . -f $(COMMON_COMPOSE_FILE) -f $(FL_BACKEND_COMPOSE_FILE) -DEBUG_OVERRIDE_COMPOSE_COMMAND=docker compose $(__ENV_FILE_FLAG) --project-directory . -f $(COMMON_COMPOSE_FILE) -f $(FL_BACKEND_COMPOSE_FILE) -f deploy/compose_trust.development.debug.override.yml +# $(MONAILABEL_OVERRIDE) is load-bearing here even though nothing in the debug override +# touches monailabel: the debug targets run `up --remove-orphans`, so a compose invocation +# that omits this overlay sees the running monailabel container as an orphan and DELETES it. +# GPU_OVERRIDE / TRUST_OVERRIDE only ever modify existing services, which is why omitting +# them was harmless; MONAILABEL_OVERRIDE is the first overlay here that ADDS one. Defined +# below at MONAILABEL_OVERRIDE — safe to reference here because this is a recursive (`=`) +# assignment, expanded when the recipe runs. +DEBUG_OVERRIDE_COMPOSE_COMMAND=docker compose $(__ENV_FILE_FLAG) --project-directory . -f $(COMMON_COMPOSE_FILE) -f $(FL_BACKEND_COMPOSE_FILE) $(MONAILABEL_OVERRIDE) -f deploy/compose_trust.development.debug.override.yml # UP_PULL_FLAGS controls the pull/build behaviour of the up-trust targets. # @@ -171,6 +178,45 @@ TRUST_OVERRIDE := $(if $(filter Trust_1,$(FL_KIT_SLOT)),-f deploy/compose_trust- GPU_OVERRIDE := $(if $(filter-out 0,$(strip $(NUM_AVAILABLE_GPUS))),-f deploy/compose_trust.$(__DCKR_SUFFIX).gpu.yml,) GPU_STATUS_MSG := $(if $(GPU_OVERRIDE),🖥️ GPU passthrough on — fl-client will reserve $(NUM_AVAILABLE_GPUS) NVIDIA GPU(s),⚠️ fl-client starting CPU-only — NUM_AVAILABLE_GPUS=$(or $(strip $(NUM_AVAILABLE_GPUS)),unset). Set it >0 on a GPU host to enable passthrough.) +# MONAI Label: AI-assisted annotation for the XNAT OHIF viewer. Off unless the kit (or the +# command line) sets MONAI_LABEL=true, because it needs an NVIDIA GPU and pulls a large +# image — a trust that does not want it must be unaffected. Same overlay mechanism as +# GPU_OVERRIDE above. +MONAI_LABEL ?= false +MONAILABEL_OVERRIDE := $(if $(filter true,$(strip $(MONAI_LABEL))),-f deploy/compose_trust.$(__DCKR_SUFFIX).monailabel.yml,) + +# The overlay bind-mounts this trust's XNAT archive read-only so MONAI Label reads DICOM +# off disk instead of pulling every scan over HTTP. XNAT_DATA_DIR comes from the kit but is +# written relative to trust/xnat/ (that Makefile abspath's it from there), so anchor a +# relative value at xnat/ — abspath'ing it from trust/ would silently resolve one level +# too high and mount a non-existent directory. Keeps the default in step with +# trust/xnat/Makefile's XNAT_DATA_DIR. +__MONAI_LABEL_XNAT_DIR := $(or $(strip $(XNAT_DATA_DIR)),$(if $(filter true stag,$(PROD)),/opt/flip/xnat-trust$(TRUST_NUM),./xnat-data-trust$(TRUST_NUM))) +MONAI_LABEL_XNAT_ARCHIVE_DIR := $(abspath $(if $(filter /%,$(__MONAI_LABEL_XNAT_DIR)),$(__MONAI_LABEL_XNAT_DIR),xnat/$(__MONAI_LABEL_XNAT_DIR)))/xnat-data/archive +export MONAI_LABEL_XNAT_ARCHIVE_DIR + +# A GPU-less host cannot run MONAI Label at all, so that stays a hard refusal. Under PROD +# it runs from the published ghcr.io/londonaicentre/monailabel image +# (docker_build_monailabel.yml) — allowed, but with an explicit warning: it re-introduces +# the XNAT OHIF viewer plugin that the trust XNAT deliberately excludes (FLIP#662 +# bulk-import livelock). Intended for hybrid/on-prem annotation trusts; think twice on a +# trust that pulls large cohorts. +define require_monailabel_supported + @if [ "$(strip $(MONAI_LABEL))" = "true" ] && [ -z "$(GPU_OVERRIDE)" ]; then \ + echo "❌ MONAI_LABEL=true needs a GPU, but NUM_AVAILABLE_GPUS=$(or $(strip $(NUM_AVAILABLE_GPUS)),unset)"; \ + echo " for KIT=$(KIT). Set NUM_AVAILABLE_GPUS>0 in trust/$(KIT_FILE) on a GPU host."; \ + exit 1; \ + fi + @if [ "$(strip $(MONAI_LABEL))" = "true" ] && [ -n "$(filter true stag,$(PROD))" ]; then \ + echo "⚠️ MONAI_LABEL=true with PROD=$(PROD): AI-assisted annotation needs the XNAT OHIF"; \ + echo " viewer plugin, which trust XNAT deliberately excludes because of the bulk-import"; \ + echo " livelock it drives (FLIP#662). Enable only on trusts that accept that trade-off"; \ + echo " (hybrid/on-prem annotation trusts, not high-volume cohort pullers)."; \ + fi +endef + +MONAI_LABEL_STATUS_MSG := $(if $(MONAILABEL_OVERRIDE),🧠 MONAI Label on — serving $(or $(strip $(MONAI_LABEL_MODELS)),deepedit) at $(or $(strip $(MONAI_LABEL_PUBLIC_URL)),http://localhost:$(or $(strip $(MONAI_LABEL_PORT)),8030)),) + # TRUST_NAME (passed to trust-api for display/logging) prefers the kit's # registered name, falling back to the assigned slot then the CODE so a # not-yet-migrated kit (no TRUST_NAME) still resolves. nvflare's FL-material @@ -315,7 +361,7 @@ define check_slot_not_taken endef build: - $(TRUST_VARS) $(TRUST_BUILD_VARS) ${DOCKER_COMPOSE_CMD} -p $(or $(TRUST_PROJECT),trust) build + $(TRUST_VARS) $(TRUST_BUILD_VARS) ${DOCKER_COMPOSE_CMD} $(MONAILABEL_OVERRIDE) -p $(or $(TRUST_PROJECT),trust) build # Bring up the trust stack selected by KIT (e.g. KIT=Trust_1). Reads # trust/.env. for that trust's ports / dirs / credentials. The @@ -330,11 +376,13 @@ up-trust: update-omop-data update-orthanc-data create-networks @echo "🚢 Starting Trust services (KIT=$(KIT))..." @echo "⚙️ Using kit file trust/$(KIT_FILE)" @echo "🧠 FL_BACKEND=$(FL_BACKEND) ($(FL_BACKEND_COMPOSE_FILE))" + $(require_monailabel_supported) $(call check_omop_data_dir,$(or $(OMOP_DATA_DIR),./omop-db/volumes/Trust_$(TRUST_NUM)/db_data)) $(check_slot_not_taken) mkdir -p $(or $(BASE_IMAGES_DOWNLOAD_DIR),./data/$(KIT)) @echo "$(GPU_STATUS_MSG)" - $(TRUST_VARS) DEBUG=${DEBUG} ${DOCKER_COMPOSE_CMD} $(GPU_OVERRIDE) $(TRUST_OVERRIDE) -p $(TRUST_PROJECT) up -d $(UP_PULL_FLAGS) + @$(if $(MONAILABEL_OVERRIDE),echo "$(MONAI_LABEL_STATUS_MSG)",:) + $(TRUST_VARS) DEBUG=${DEBUG} ${DOCKER_COMPOSE_CMD} $(GPU_OVERRIDE) $(MONAILABEL_OVERRIDE) $(TRUST_OVERRIDE) -p $(TRUST_PROJECT) up -d $(UP_PULL_FLAGS) @echo "🩻 Starting XNAT for $(KIT)..." $(MAKE) -C xnat up-xnat KIT=$(KIT) PROD=$(PROD) @@ -342,7 +390,7 @@ down-trust: $(require_kit) $(check_slot_not_taken) @echo "🛑 Stopping Trust services (KIT=$(KIT))..." - $(TRUST_VARS) ${DOCKER_COMPOSE_CMD} $(TRUST_OVERRIDE) -p $(TRUST_PROJECT) down --remove-orphans + $(TRUST_VARS) ${DOCKER_COMPOSE_CMD} $(MONAILABEL_OVERRIDE) $(TRUST_OVERRIDE) -p $(TRUST_PROJECT) down --remove-orphans @echo "🩻 Stopping XNAT for $(KIT)..." $(MAKE) -C xnat down-xnat KIT=$(KIT) PROD=$(PROD) @@ -370,6 +418,16 @@ up-trust-ec2: @echo "⚙️ Using kit file trust/$(KIT_FILE)" @echo "🧠 FL_BACKEND=$(FL_BACKEND) ($(FL_BACKEND_COMPOSE_FILE))" @echo "🖥️ EC2 trust runs CPU-only by design — GPU overlay omitted + NUM_AVAILABLE_GPUS forced to 0 (t3.xlarge has no GPU)." + @# Refuse rather than silently drop the overlay. require_monailabel_supported is not + @# enough here: it keys off GPU_OVERRIDE, which a kit seeded with the template default + @# NUM_AVAILABLE_GPUS=1 leaves non-empty even though the t3.xlarge has no GPU. Without + @# this the kit says MONAI Label is on, the deploy says nothing, and no server ever runs. + @if [ "$(strip $(MONAI_LABEL))" = "true" ]; then \ + echo "❌ MONAI_LABEL=true is not supported on the EC2 trust path — this instance type"; \ + echo " has no GPU (that is why the GPU overlay is omitted above). Set MONAI_LABEL=false"; \ + echo " in trust/$(KIT_FILE), or deploy this trust on a GPU host with up-trust."; \ + exit 1; \ + fi $(TRUST_VARS) NUM_AVAILABLE_GPUS=0 DEBUG=${DEBUG} ${DOCKER_COMPOSE_CMD} -p $(TRUST_PROJECT) up -d --pull always @echo "🩻 Starting XNAT for $(KIT)..." $(MAKE) -C xnat up-xnat KIT=$(KIT) PROD=$(PROD) diff --git a/trust/README.md b/trust/README.md index c511b9065..3b8b2e9ab 100644 --- a/trust/README.md +++ b/trust/README.md @@ -121,6 +121,124 @@ See dedicated README under [omop-db/README.md](omop-db/README.md) for instructio `make up` (and `make up-trust KIT=`) brings up that trust's XNAT automatically — it is no longer a separate step. See the dedicated README under [xnat/README.md](xnat/README.md) for standalone XNAT management and debugging. +## MONAI Label (optional) + +MONAI Label adds AI-assisted annotation to the XNAT OHIF viewer: a **MONAI Label** menu in the +viewer's Masks panel that runs a segmentation model over the scan on screen and lets a user +correct the result interactively. + +> **Prerequisite: the XNAT OHIF Viewer plugin, which FLIP does not install by default** — it +> drives the bulk-import livelock in FLIP#662 (see the plugin table in +> [xnat/README.md](xnat/README.md)). Enabling MONAI Label means accepting it back onto that +> trust's XNAT. + +It is **off by default** — it needs an NVIDIA GPU on the trust host and pulls a large image, so +a trust that does not want it is unaffected. Enable it per trust: + +```sh +make up-trust KIT= MONAI_LABEL=true # or set MONAI_LABEL=true in trust/.env.. +``` + +The trust's kit file carries the rest of the settings: + +| Variable | Default | Notes | +| --- | --- | --- | +| `MONAI_LABEL` | `false` | Master switch. Requires `NUM_AVAILABLE_GPUS>0`. | +| `MONAI_LABEL_PORT` | `8030` | Host port the server listens on. | +| `MONAI_LABEL_MODELS` | `deepedit` | Comma-separated radiology models. `all` loads nine, each downloading its own weights. | +| `MONAI_LABEL_PROJECTS` | *(empty)* | XNAT projects the server may read. Empty means **every** project on this trust. | +| `MONAI_LABEL_PUBLIC_URL` | `http://localhost:$MONAI_LABEL_PORT` | See below — this one matters. | +| `MONAI_LABEL_SHM_SIZE` | `8gb` | Shared memory for dataloader workers. | +| `MONAI_LABEL_BIND_HOST` | `127.0.0.1` | Host interface `MONAI_LABEL_PORT` is published on. Loopback by default — see below. | +| `MONAI_LABEL_AUTH_ENABLE` | `false` | MONAI Label's own auth. Upstream default; enabling it needs an OAuth realm FLIP does not run. | + +**`MONAI_LABEL_PUBLIC_URL` is the setting people get wrong.** XNAT stores this URL and hands it +to the OHIF viewer, which calls it **from the clinician's browser** — XNAT never proxies the +request. So it must resolve on the clinician's machine: a Docker service name, or `0.0.0.0`, +will not work. The default only works when the browser runs on the trust host itself. Two +further consequences on a real trust: + +- If XNAT is served over **HTTPS**, the browser blocks a plain-`http://` MONAI Label URL as + mixed content. Terminate TLS in front of MONAI Label, or serve it through the XNAT nginx so + it is same-origin. +- The MONAI Label API is **unauthenticated** and holds this trust's XNAT service-account + credentials. Anyone who can reach the port can enumerate every study the server can see, + download identifiable DICOM, and write labels back to XNAT as an admin. `MONAI_LABEL_AUTH_ENABLE` + is upstream's own switch, but turning it on requires an OAuth realm FLIP does not run — so + the containment is the network, not the app. + + Because of that, **`MONAI_LABEL_BIND_HOST` defaults to `127.0.0.1`**: out of the box the port + is published on loopback only, and the trust host gains no listener reachable from the + network (the trust deployment model is otherwise strictly outbound — see + [Deployment Architecture](../CLAUDE.md)). That default is deliberately *not* browser-usable + from another machine. To give clinicians access, pick one: + + 1. **Serve it through the XNAT nginx** (recommended) — same-origin with XNAT, so it also + resolves the mixed-content problem above, and it inherits XNAT's TLS. + 2. **Set `MONAI_LABEL_BIND_HOST`** to an interface you have explicitly firewalled to the + clinical network, and set `MONAI_LABEL_PUBLIC_URL` to match. + +**Each user must switch the panel on themselves**, once per browser: in the viewer, *Options → +Preferences → Experimental*, tick **MONAILabel Tools**. A **MONAI Label** entry then appears in +the Masks panel. Registering the server is automatic, but this flag is not, and it cannot be +defaulted from the server — the viewer keeps it in the browser's `localStorage`. Until it is +ticked, a perfectly working server is simply absent from the viewer; that is the first thing to +check when it "isn't showing up", followed by `GET /xapi/ohifaiaa/servers` returning the URL and +that URL opening in the browser. + +Two known stock-viewer defects (both client-side — the server returns valid masks throughout, +verifiable in the monailabel container's logs): + +- *"Empty mask was returned by the model run"* from every interactive model (`sam_2d`, + `deepgrow_*`) while `deepedit_seg` still works: the tab's segment state has gone stale — + **reload the tab**. +- Interactive (DeepGrow-type) results always land in the **first segment**, regardless of which + segment is selected — the viewer routes the mask through an active-segment index that segment + selection does not update (observed in the OHIF **viewer frontend** 3.7.2 that ships inside + XNAT OHIF **plugin** 3.8.0; the 3.7.0 frontend did not have the reworked segment store — + note the frontend and plugin carry separate version numbers). Until fixed upstream, treat + SAM/DeepGrow as single-target — rename the first segment afterwards — and use `deepedit_seg` + for multi-organ work. + +### Getting labels back out: the `export_mask` pipeline + +A mask saved in the viewer lands in XNAT as a **DICOM-SEG assessor**, which FL training cannot +read — it consumes NIfTI. `trust/xnat/xnat/config/configure-export-mask.sh` closes that gap: it +registers an `export_mask` container-service command plus a site-wide event subscription on +image-assessor creation, so every saved segmentation is converted to NIfTI and uploaded back to +the session automatically. Nothing upstream does this step. + +Two things to know: + +- **It is registered only when `MONAI_LABEL=true`.** `make xnat-configure` gates it (see + `xnat-configure-export-mask` in `trust/xnat/Makefile`), so a trust without MONAI Label gets + neither the converter command nor the subscription. **If you enable MONAI Label on a trust + that was already up, re-run `make -C trust/xnat xnat-configure KIT=`** (or bring XNAT + up again) — otherwise the annotation UI works but masks are never converted. +- **The converter image is currently `atriaybagur/aic-ohif-dicomseg-to-nifti:latest`** — a + personal Docker Hub namespace on a mutable tag, pulled and run on the trust network with + XNAT admin credentials injected by the container service. Moving it to + `ghcr.io/londonaicentre` on an immutable digest is outstanding; until then, treat enabling + MONAI Label as also trusting that image. + +Notes: + +- **SAM is always on**, independently of `MONAI_LABEL_MODELS`: the radiology app registers + `sam_2d` and `sam_3d` (interactive click-to-segment) whenever the `sam2` package is + importable. Their checkpoint is a further ~900 MB fetched from HuggingFace on first start, + into the same persisted model directory. Upstream's `--conf sam2 false` would disable them, + but `entrypoint.sh` passes only `--conf models`, so there is currently no way to turn SAM off + from the kit file — it would need an entrypoint change. +- The server reads DICOM straight off this trust's XNAT archive (mounted read-only), falling + back to HTTP downloads only for scans it cannot resolve there. +- Pretrained weights are fetched on first start and persisted in the `monailabel-models` + volume, so a container recreate does not re-download them. +- Production (`PROD=true|stag`) runs the published `ghcr.io/londonaicentre/monailabel` image + (built by `docker_build_monailabel.yml`; dev builds locally from `trust/monailabel/`). + Enabling it under `PROD` prints a warning: it re-introduces the OHIF viewer plugin that trust + XNAT deliberately excludes (FLIP#662) — intended for hybrid/on-prem annotation trusts, not + high-volume cohort pullers. + ## Running standalone (remote trust operator) If you are operating a trust on a host that does not have the hub's diff --git a/trust/deploy/compose_trust.development.monailabel.yml b/trust/deploy/compose_trust.development.monailabel.yml new file mode 100644 index 000000000..9671710c5 --- /dev/null +++ b/trust/deploy/compose_trust.development.monailabel.yml @@ -0,0 +1,78 @@ +# Copyright (c) Guy's and St Thomas' NHS Foundation Trust & King's College London +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +# MONAI Label overlay (development). Applied by trust/Makefile only when the kit sets +# MONAI_LABEL=true AND declares NUM_AVAILABLE_GPUS > 0 (see MONAILABEL_OVERRIDE). Kept in +# an overlay rather than the base compose so a trust that does not want it — or has no +# GPU — brings the stack up unchanged. Mirrors the GPU / TRUST_OVERRIDE pattern. + +services: + + monailabel: + build: + # Relative to the compose project directory (trust/), which is what --project-directory + # pins — NOT to this file's location under trust/deploy/. + context: ./monailabel + dockerfile: Dockerfile + volumes: + # MONAI Label resolves DICOM straight off the XNAT archive instead of pulling every + # scan over HTTP. Its XNAT datastore builds the path as + # + + # so this must be mounted at the archive itself, not at its parent — pointing one + # level too high makes every lookup miss and silently fall back to HTTP downloads. + # Read-only: labels are written back through the XNAT REST API, never the filesystem. + - ${MONAI_LABEL_XNAT_ARCHIVE_DIR}:/workspace/xnat-data/archive:ro + # Pretrained weights are fetched on every server start; persist them so a container + # recreate does not re-download every model. + - monailabel-models:/workspace/app/radiology/model + environment: + - MONAI_LABEL_PORT=${MONAI_LABEL_PORT:-8030} + - MONAI_LABEL_DATASTORE=xnat + - MONAI_LABEL_DATASTORE_URL=${XNAT_URL:-http://xnat-web:8080} + - MONAI_LABEL_DATASTORE_USERNAME=${XNAT_SERVICE_USER} + - MONAI_LABEL_DATASTORE_PASSWORD=${XNAT_SERVICE_PASSWORD} + - MONAI_LABEL_DATASTORE_ASSET_PATH=/workspace/xnat-data/archive + # Without this the datastore walks every project on the trust's XNAT, issuing one + # request per experiment. Comma-separated; empty means every project. + - MONAI_LABEL_DATASTORE_PROJECT=${MONAI_LABEL_PROJECTS:-} + # Which radiology models to load. 'all' loads nine, each downloading its own weights. + - MONAI_LABEL_MODELS=${MONAI_LABEL_MODELS:-deepedit} + # Reached from the clinician's browser, NOT from inside the trust network. + - MONAI_LABEL_PUBLIC_URL=${MONAI_LABEL_PUBLIC_URL:-http://localhost:${MONAI_LABEL_PORT:-8030}} + # Explicit, not inherited. MONAI Label's own default is False, and its RBAC dependencies + # are no-ops while it is — so every /datastore route is open to whoever can reach the + # port. Turning it on needs an OAuth realm (MONAI_LABEL_AUTH_REALM_URI et al) that FLIP + # does not run, so the containment here is the bind address below, not app auth. Stated + # rather than left implicit so the posture is visible at the point it is chosen. + - MONAI_LABEL_AUTH_ENABLE=${MONAI_LABEL_AUTH_ENABLE:-false} + ports: + # Loopback by default. The API is unauthenticated (above) and holds this trust's XNAT + # service-account credentials, so binding 0.0.0.0 would put an unauthenticated reader of + # every study — and an admin-credentialled writer — on the trust host's network, which is + # the one thing the trust deployment model rules out ("no inbound ports on trust hosts"). + # For real clinician access set MONAI_LABEL_BIND_HOST to the interface you have + # deliberately firewalled, or front it through the XNAT nginx (which also resolves the + # mixed-content problem on an HTTPS XNAT). See trust/README.md#monai-label-optional. + - "${MONAI_LABEL_BIND_HOST:-127.0.0.1}:${MONAI_LABEL_PORT:-8030}:${MONAI_LABEL_PORT:-8030}" + shm_size: ${MONAI_LABEL_SHM_SIZE:-8gb} + # count tracks NUM_AVAILABLE_GPUS so the reservation stays in lockstep with the rest of + # the trust stack, exactly as the fl-client GPU overlay does. + deploy: + resources: + reservations: + devices: + - driver: nvidia + count: ${NUM_AVAILABLE_GPUS} + capabilities: [gpu] + +volumes: + monailabel-models: diff --git a/trust/deploy/compose_trust.production.monailabel.yml b/trust/deploy/compose_trust.production.monailabel.yml new file mode 100644 index 000000000..5af5bd9f2 --- /dev/null +++ b/trust/deploy/compose_trust.production.monailabel.yml @@ -0,0 +1,70 @@ +# Copyright (c) Guy's and St Thomas' NHS Foundation Trust & King's College London +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +# MONAI Label overlay (production). Applied by trust/Makefile only when the kit sets +# MONAI_LABEL=true AND declares NUM_AVAILABLE_GPUS > 0 (see MONAILABEL_OVERRIDE). +# Image-only like the rest of the production compose — published by +# .github/workflows/docker_build_monailabel.yml. +# +# Enabling this re-introduces the XNAT OHIF viewer plugin that the trust XNAT +# deliberately excludes (FLIP#662 bulk-import livelock): intended for hybrid/on-prem +# annotation trusts, weigh it before enabling on a trust that pulls large cohorts. +# up-trust prints this warning when the flag is on. + +services: + + monailabel: + image: ghcr.io/londonaicentre/monailabel:${DOCKER_TAG} + restart: always + volumes: + # Read-only mount of the XNAT archive itself (not its parent — the datastore + # resolves files as + , so a parent + # path silently misses and falls back to per-scan HTTP downloads). Labels are + # written back through the XNAT REST API, never the filesystem. + - ${MONAI_LABEL_XNAT_ARCHIVE_DIR}:/workspace/xnat-data/archive:ro + # Pretrained weights are fetched on first start; persist them so a container + # recreate does not re-download every model (~GBs incl. the SAM checkpoint). + - monailabel-models:/workspace/app/radiology/model + environment: + - MONAI_LABEL_PORT=${MONAI_LABEL_PORT:-8030} + - MONAI_LABEL_DATASTORE=xnat + - MONAI_LABEL_DATASTORE_URL=${XNAT_URL:-http://xnat-web:8080} + - MONAI_LABEL_DATASTORE_USERNAME=${XNAT_SERVICE_USER} + - MONAI_LABEL_DATASTORE_PASSWORD=${XNAT_SERVICE_PASSWORD} + - MONAI_LABEL_DATASTORE_ASSET_PATH=/workspace/xnat-data/archive + # Comma-separated XNAT project allowlist; empty means every project. + - MONAI_LABEL_DATASTORE_PROJECT=${MONAI_LABEL_PROJECTS:-} + - MONAI_LABEL_MODELS=${MONAI_LABEL_MODELS:-deepedit} + # Reached from the clinician's BROWSER, never proxied by XNAT — must be + # browser-resolvable, and same-origin/TLS-fronted when XNAT is HTTPS. + - MONAI_LABEL_PUBLIC_URL=${MONAI_LABEL_PUBLIC_URL:-http://localhost:${MONAI_LABEL_PORT:-8030}} + # Explicit, not inherited — MONAI Label defaults this to False and its RBAC dependencies + # are no-ops while it is. Enabling it needs an OAuth realm FLIP does not run, so the + # containment is the bind address below. Stated so the posture is visible where chosen. + - MONAI_LABEL_AUTH_ENABLE=${MONAI_LABEL_AUTH_ENABLE:-false} + ports: + # Loopback by default — the API is unauthenticated and holds this trust's XNAT + # service-account credentials. Set MONAI_LABEL_BIND_HOST to a deliberately firewalled + # interface, or front it through the XNAT nginx (which also fixes mixed content on an + # HTTPS XNAT). See trust/README.md#monai-label-optional. + - "${MONAI_LABEL_BIND_HOST:-127.0.0.1}:${MONAI_LABEL_PORT:-8030}:${MONAI_LABEL_PORT:-8030}" + shm_size: ${MONAI_LABEL_SHM_SIZE:-8gb} + deploy: + resources: + reservations: + devices: + - driver: nvidia + count: ${NUM_AVAILABLE_GPUS} + capabilities: [gpu] + +volumes: + monailabel-models: diff --git a/trust/monailabel/Dockerfile b/trust/monailabel/Dockerfile new file mode 100644 index 000000000..272ad0509 --- /dev/null +++ b/trust/monailabel/Dockerfile @@ -0,0 +1,106 @@ +# Copyright (c) Guy's and St Thomas' NHS Foundation Trust & King's College London +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +# MONAI Label server for AI-assisted annotation in the XNAT OHIF viewer. +# +# Slim base + CUDA wheels from PyTorch's own index, matching fl-services/*/fl-base. The +# obvious alternative — nvcr.io/nvidia/pytorch — is ~20GB because it bundles the full CUDA +# toolkit, TensorRT, DALI, Apex and JupyterLab, none of which this server runs; the cu128 +# wheels carry the CUDA runtime libraries MONAI Label actually needs. +FROM python:3.12-slim-bookworm + +ARG DEBIAN_FRONTEND=noninteractive +ENV PYTHONDONTWRITEBYTECODE=1 \ + PYTHONUNBUFFERED=1 \ + PIP_NO_CACHE_DIR=1 + +WORKDIR /workspace + +# MONAI Label is installed from a pinned git commit, NOT from PyPI. +# +# The latest release (0.8.5, Nov 2024) omits `auth=self.auth` in +# XNATDatastore._request_get(), so every read against the XNAT REST API is unauthenticated. +# Opening the MONAI Label panel in the XNAT OHIF viewer then fails with HTTP 500 over an +# underlying 401 (Project-MONAI/MONAILabel#1837). The fix is PR #1838, merged 2025-06-17 +# and still unreleased. +# +# 3806289 is that merge commit, and remains the most recent commit touching xnat.py — so it +# carries every XNAT datastore fix to date while staying as close to the last release as +# possible. Re-check https://github.com/Project-MONAI/MONAILabel/releases before bumping: +# once a release after 0.8.5 exists, this can go back to a plain version pin. +ARG MONAILABEL_REF=38062894051d6fd1d3ba72cb2f8a39ad4f66f535 + +# git: MONAI Label derives its version with versioneer from repo metadata. +# libgl1/libglib2.0-0: OpenCV, pulled in transitively, needs them at import time. +# curl: entrypoint.sh probes XNAT and the server's own readiness with it, and it is the first +# tool anyone reaches for when debugging connectivity from inside the container. Unlike the +# NVIDIA base this image used to be built on, python:slim does not ship it. +RUN apt-get update \ + && apt-get install -y --no-install-recommends git curl libgl1 libglib2.0-0 \ + && rm -rf /var/lib/apt/lists/* + +# Install the torch stack FIRST, from PyTorch's CUDA-12.8 index only. +# +# --index-url (not --extra-index-url) is load-bearing. With --extra-index-url pip still sees +# PyPI, and PyPI's default torch is now a CUDA 13 build which outranks the cu128 wheel — that +# resolution pulls nvidia-*-cu13 and produces an image that cannot run here, because cu13 +# requires NVIDIA driver >= 580 and the trust hosts run 575.x. Restricting the index to cu128 +# removes the choice. cu128 also carries the Blackwell (sm_120) kernels the RTX 5090 needs. +RUN pip install --no-cache-dir torch torchvision \ + --index-url https://download.pytorch.org/whl/cu128 + +# Freeze exactly what that resolved, so the MONAI Label install below — whose dependency tree +# (monai, sam2, pytorch-ignite) also declares torch — cannot quietly swap it for a PyPI build. +RUN python - <<'PY' > /tmp/torch_constraints.txt +import importlib +for m in ("torch", "torchvision"): + try: + print(f"{m}=={importlib.import_module(m).__version__}") + except Exception: + pass +PY +RUN echo "Torch constraints:" && cat /tmp/torch_constraints.txt + +RUN pip install --no-cache-dir -c /tmp/torch_constraints.txt \ + "pydicom<3" \ + monai \ + "monailabel @ git+https://github.com/Project-MONAI/MONAILabel.git@${MONAILABEL_REF}" \ + uvicorn[standard] fastapi python-multipart \ + --extra-index-url https://download.pytorch.org/whl/cu128 + +# Fail the build rather than ship a broken image. Two ways it silently breaks, neither of +# which surfaces until runtime: +# 1. the unreleased XNAT auth fix disappearing from the pinned ref; +# 2. pip resolving a CUDA 13 torch. PyPI serves cu13 by default, and cu13 requires NVIDIA +# driver >= 580 while the trust hosts run 575.x — so it installs cleanly and then dies +# at first inference with an opaque CUDA error. +# +# The wheel's local version tag (+cu128) is the signal to check. torch.cuda.get_arch_list() +# looks like the more direct test for Blackwell (sm_120) kernels, but it returns [] whenever +# torch.cuda.is_available() is false — which it always is in a GPU-less build — so it can +# only be used at runtime, not here. +RUN python -c "\ +import inspect, sys, torch; \ +from monailabel.datastore.xnat import XNATDatastore as X; \ +v = torch.__version__; cuda = torch.version.cuda or ''; \ +print('torch', v, 'cuda', cuda); \ +'auth=self.auth' in inspect.getsource(X._request_get) or sys.exit('MONAI Label lacks the XNAT auth fix (#1838)'); \ +cuda.startswith('12.') or sys.exit(f'torch is built for CUDA {cuda}; need 12.x (cu13 needs driver >=580)'); \ +'+cu' in v or sys.exit(f'torch {v} is not a CUDA wheel')" + +# Copy the radiology app out of the installed package at build time (this is a local copy from +# the wheel's sample-apps, not a download) so a cold start does not depend on it. Pretrained +# model weights are still fetched at server start; the compose overlay persists them. +RUN monailabel apps --name radiology --download --output /workspace/app + +COPY entrypoint.sh /workspace/entrypoint.sh +ENTRYPOINT ["/bin/bash", "/workspace/entrypoint.sh"] diff --git a/trust/monailabel/entrypoint.sh b/trust/monailabel/entrypoint.sh new file mode 100755 index 000000000..4acee56c1 --- /dev/null +++ b/trust/monailabel/entrypoint.sh @@ -0,0 +1,137 @@ +#!/bin/bash +# +# Copyright (c) Guy's and St Thomas' NHS Foundation Trust & King's College London +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +# Starts the MONAI Label server against this trust's XNAT and registers it with the XNAT +# OHIF viewer so the Masks panel offers AI-assisted annotation. +# +# Required environment (supplied by trust/deploy/compose_trust.*.monailabel.yml): +# MONAI_LABEL_PORT - port the server listens on +# MONAI_LABEL_DATASTORE_URL - XNAT base URL, reachable inside the trust network +# MONAI_LABEL_DATASTORE_USERNAME - XNAT service account +# MONAI_LABEL_DATASTORE_PASSWORD - XNAT service account password +# MONAI_LABEL_PUBLIC_URL - URL the CLINICIAN'S BROWSER uses to reach this server +# MONAI_LABEL_MODELS - comma-separated radiology models to load + +set -euo pipefail + +APP_DIR=/workspace/app/radiology +SERVER_URL="http://127.0.0.1:${MONAI_LABEL_PORT}" + +# --- Wait for XNAT ------------------------------------------------------------------ +# Wall-clock bounded, and each probe carries its own timeout, so a wedged XNAT fails +# loudly instead of printing dots forever (mirrors trust/xnat/xnat/config/configure-xnat.sh). +echo "Waiting for XNAT at ${MONAI_LABEL_DATASTORE_URL} ..." +deadline=$((SECONDS + 900)) +until curl --output /dev/null --silent --head --fail \ + --connect-timeout 5 --max-time 10 "${MONAI_LABEL_DATASTORE_URL}/app/template/Login.vm"; do + if [[ "$SECONDS" -ge "$deadline" ]]; then + echo "ERROR: XNAT did not become available within 900s" >&2 + exit 1 + fi + printf '.' + sleep 5 +done +echo "XNAT is up!" + +# XNAT answers on the login page before configure-xnat.sh has finished creating the service +# account, so poll until THESE credentials actually authenticate rather than sleeping a +# fixed interval and hoping. Without this the server starts, fails to log in, and exits. +echo "Waiting for the XNAT service account to be usable..." +deadline=$((SECONDS + 900)) +until curl --output /dev/null --silent --fail \ + --connect-timeout 5 --max-time 10 \ + -u "${MONAI_LABEL_DATASTORE_USERNAME}:${MONAI_LABEL_DATASTORE_PASSWORD}" \ + "${MONAI_LABEL_DATASTORE_URL}/data/projects?format=json"; do + if [[ "$SECONDS" -ge "$deadline" ]]; then + echo "ERROR: XNAT service account '${MONAI_LABEL_DATASTORE_USERNAME}' could not authenticate within 900s" >&2 + exit 1 + fi + printf '.' + sleep 5 +done +echo "XNAT service account OK." + +# --- Start the MONAI Label server --------------------------------------------------- +# --studies is what the XNAT datastore uses as its API URL (MONAI_LABEL_DATASTORE_URL is +# not read for the xnat datastore, only for our own probes above). +echo "Starting MONAI Label server (models: ${MONAI_LABEL_MODELS}) ..." +monailabel start_server \ + --app "${APP_DIR}" \ + --studies "${MONAI_LABEL_DATASTORE_URL}" \ + --host 0.0.0.0 --port "${MONAI_LABEL_PORT}" \ + --conf models "${MONAI_LABEL_MODELS}" & +server_pid=$! + +# --- Wait for it to be serving ------------------------------------------------------ +# Poll instead of sleeping: loading models downloads pretrained weights, which takes minutes +# on a cold volume and seconds on a warm one. +# +# Probe /openapi.json, NOT /info/. With the XNAT datastore, /info/ enumerates every experiment +# on the trust — one XML request each — so on a populated trust it cannot answer inside any +# sane timeout (found live on a 900-experiment trust). Each probe would then abandon at +# --max-time, the loop would run to the deadline, and this script would exit 1 on a server that +# is up and working, without ever reaching the XNAT registration below. /openapi.json answers +# in milliseconds and proves the HTTP server is serving, which is all this gate needs to +# assert. Mirrors the k8s probes in deploy/providers/kubernetes/templates/monailabel.yaml. +echo "Waiting for MONAI Label to finish loading models..." +deadline=$((SECONDS + 1800)) +until curl --output /dev/null --silent --fail \ + --connect-timeout 5 --max-time 10 "${SERVER_URL}/openapi.json"; do + if ! kill -0 "$server_pid" 2>/dev/null; then + echo "ERROR: MONAI Label server exited during startup" >&2 + wait "$server_pid" + exit 1 + fi + if [[ "$SECONDS" -ge "$deadline" ]]; then + echo "ERROR: MONAI Label server did not become ready within 1800s" >&2 + exit 1 + fi + printf '.' + sleep 5 +done +echo "MONAI Label server is ready." + +# --- Register with the XNAT OHIF viewer --------------------------------------------- +# The OHIF viewer calls this URL from the clinician's BROWSER — XNAT's ohifaiaa xapi only +# stores and validates the string, it never proxies the request. So it must be an address +# that resolves on the clinician's machine; a Docker-internal name or 0.0.0.0 will not work. +# PUT /xapi/ohifaiaa/servers is restricted to XNAT admins, which the trust service account is. +echo "Registering MONAI Label with the XNAT OHIF viewer: ${MONAI_LABEL_PUBLIC_URL}" +register_status=$(curl -s -o /tmp/ohifaiaa-put.json --max-time 60 -w "%{http_code}" \ + -u "${MONAI_LABEL_DATASTORE_USERNAME}:${MONAI_LABEL_DATASTORE_PASSWORD}" \ + -X PUT "${MONAI_LABEL_DATASTORE_URL}/xapi/ohifaiaa/servers" \ + -H "accept: */*" -H "Content-Type: application/json" \ + -d "[\"${MONAI_LABEL_PUBLIC_URL}\"]") || register_status="000" + +if [[ "$register_status" != 2* ]]; then + # Fail rather than run on: a server that is up but unregistered looks healthy while the + # MONAI Label panel is simply absent from the viewer, which is hard to diagnose from here. + echo "ERROR: registering the MONAI Label server with XNAT failed (HTTP $register_status)" >&2 + cat /tmp/ohifaiaa-put.json 2>/dev/null || true + kill "$server_pid" 2>/dev/null || true + exit 1 +fi +echo "Registered. NOTE: each user must also enable it in the viewer under" +echo " Options -> Preferences -> Experimental -> MONAILabel Tools" + +# Hand the container's lifetime back to the server process. +# +# The trap is load-bearing: this script is PID 1, and a bare `wait` does not forward signals. +# Without it `docker stop` (and a k8s pod termination) delivers SIGTERM to bash only, bash +# exits, and the server is SIGKILLed when the grace period expires — killed mid-inference, +# with nothing in the log explaining the hard stop. Forward the signal so it shuts down +# cleanly. Exiting 143 on SIGTERM is the normal, expected result of a clean stop. +trap 'echo "Received termination signal — stopping the MONAI Label server..."; \ + kill -TERM "$server_pid" 2>/dev/null || true' TERM INT +wait "$server_pid" diff --git a/trust/xnat/Makefile b/trust/xnat/Makefile index 43e7f7fd2..283e4421a 100644 --- a/trust/xnat/Makefile +++ b/trust/xnat/Makefile @@ -12,6 +12,7 @@ .PHONY: build up down up-xnat down-xnat xnat-shell xnat-stack-down clean \ xnat-war-download xnat-plugins-download xnat-reset xnat-configure \ + xnat-configure-export-mask \ create-xnat-network test unit_test local_test # Mirrors trust/Makefile: no hub `.env.*` include. Runtime config (DOCKER_REGISTRY, @@ -84,6 +85,11 @@ endif # check_network_matches_trust below verifies rather than trusts — an unprefixed XNAT beside a # prefixed trust deploys healthily and simply cannot reach imaging-api. include ../../deploy/instance.mk +# Whether this trust runs MONAI Label. Read from the kit file by the -include above, or +# passed on the command line (which Make forwards to this sub-make automatically). Gates +# export_mask registration in xnat-configure — see xnat-configure-export-mask. Mirrors the +# same default in trust/Makefile. +MONAI_LABEL ?= false # Slot NUMBER the hub assigned this trust, read from the kit file — NOT a # substring of the CODE. Drives the stack name (xnat) and overlay network # ($(INSTANCE_PREFIX)deploy_trust-network-); trust/Makefile derives TRUST_NUM from @@ -332,7 +338,7 @@ xnat-plugins-download: echo " Set it in .env.development (see the 'S3 Buckets' block), or pass it on the command"; \ echo " line: make xnat-plugins-download FLIP_ARTIFACTS_BUCKET_NAME=."; \ exit 1; } - @./scripts/ensure_plugins.sh "xnat/plugins" "$(FLIP_ARTIFACTS_BUCKET_NAME)" "xnat-$(XNAT_VERSION)/plugins" + @MONAI_LABEL="$(strip $(MONAI_LABEL))" ./scripts/ensure_plugins.sh "xnat/plugins" "$(FLIP_ARTIFACTS_BUCKET_NAME)" "xnat-$(XNAT_VERSION)/plugins" # Creates the data-volume directory structure for one trust's XNAT stack and # wipes any previous data. Docker Swarm requires host directories to exist @@ -396,6 +402,39 @@ xnat-configure: [ -n "$$CONTAINER" ] || { echo "❌ ERROR: $(XNAT_PROJECT)_xnat-web container not found at configure time"; exit 1; }; \ docker exec $$CONTAINER bash -c 'set -o pipefail; cd /data/xnat/config && LOG_FILE="configure-xnat-$(XNAT_PROJECT).log" && rm -f "$$LOG_FILE" && bash configure-xnat.sh 2>&1 | tee "$$LOG_FILE"' && \ docker exec $$CONTAINER bash -c 'set -o pipefail; cd /data/xnat/config && LOG_FILE="configure-dcm2niix-$(XNAT_PROJECT).log" && rm -f "$$LOG_FILE" && bash configure-dcm2niix.sh 2>&1 | tee "$$LOG_FILE"' + $(MAKE) xnat-configure-export-mask XNAT_PROJECT=$(XNAT_PROJECT) + +# export_mask (DICOM-SEG -> NIfTI) is registered ONLY when this trust runs MONAI Label. +# +# It is deliberately NOT part of the unconditional chain above. The command it registers +# launches a converter container, and its subscription is site-wide (a DICOM-SEG can be +# created in any project by anyone using the OHIF viewer, so there is no per-project opt-in +# like dcm2niix's dicom_to_nifti flag). A trust that never enables MONAI Label has no OHIF +# viewer plugin installed, so it can never produce the assessors this reacts to — registering +# it there would add a converter image and a site-wide event subscription that trust never +# asked for. Gating also keeps the unconditional bring-up path free of a new failure point: +# in stag/prod the config dir comes from the image (xnat/Dockerfile `ADD config`) — only the +# development stack bind-mounts it from the host (docker-compose-stack.development.yml) — so +# against an xnat-web image predating this feature the script simply does not exist there. +# +# Consequence of gating (documented in trust/README.md): a trust that turns MONAI Label on +# later must re-run `make -C trust/xnat xnat-configure KIT=` (or bring XNAT up again) +# to register the command and subscription. +xnat-configure-export-mask: +ifeq ($(strip $(MONAI_LABEL)),true) + @[ -n "$(XNAT_PROJECT)" ] || (echo "❌ XNAT_PROJECT is required"; exit 1) + @echo "🧠 MONAI_LABEL=true — registering the export_mask (DICOM-SEG → NIfTI) command..." + CONTAINER=$$(docker ps --filter "name=$(XNAT_PROJECT)_xnat-web" -q); \ + [ -n "$$CONTAINER" ] || { echo "❌ ERROR: $(XNAT_PROJECT)_xnat-web container not found"; exit 1; }; \ + docker exec $$CONTAINER bash -c 'test -f /data/xnat/config/configure-export-mask.sh' || { \ + echo "❌ ERROR: configure-export-mask.sh is not in this xnat-web image."; \ + echo " The XNAT config dir is baked into the image, so MONAI_LABEL=true needs an"; \ + echo " xnat-web image that carries it — rebuild, or bump XNAT_TAG to a newer tag."; \ + exit 1; }; \ + docker exec $$CONTAINER bash -c 'set -o pipefail; cd /data/xnat/config && LOG_FILE="configure-export-mask-$(XNAT_PROJECT).log" && rm -f "$$LOG_FILE" && bash configure-export-mask.sh 2>&1 | tee "$$LOG_FILE"' +else + @echo "⏭️ MONAI_LABEL is not true — skipping export_mask registration." +endif # Guard: XNAT and the trust core services must share one network. up-trust brings the trust # compose up before invoking us, so the trust's real attachment is observable — compare it diff --git a/trust/xnat/README.md b/trust/xnat/README.md index f2a270d31..5bee0efe3 100644 --- a/trust/xnat/README.md +++ b/trust/xnat/README.md @@ -199,7 +199,7 @@ The following table lists the plugin versions for the XNAT version `1.10.0` used | DICOM Query-Retrieve Plugin | 3.0.0 | Yes | **Must upgrade** from 2.2.0 — rebuilt on JDK 21 + `dcm4che5`, plus a thread-leakage fix | | Container Service Plugin | 3.8.1 (JDK 8 build) | Yes | **Must upgrade** from 3.7.3 — 3.8.x is the only column the compatibility matrix ticks for 1.10.0 | | Batch Launch Plugin | 0.9.0 (JDK 8 build) | Yes | None — the matrix keeps BLP 0.9.0 for 1.10.0 | -| OHIF Viewer Plugin | 3.8.0 available; n/a here | No | None — deliberately not installed (FLIP#662) | +| OHIF Viewer Plugin | 3.8.0 available; n/a here | No | None — deliberately not installed (FLIP#662). Required *only* by the optional MONAI Label server — see [MONAI Label](../README.md#monai-label-optional) | Not applicable to FLIP, but released alongside 1.10.0: **Distributed Events 2.0.0** (only needed for load-balanced multi-node XNAT — each FLIP trust runs a single node) and **MFA 1.6.0** (FLIP does not diff --git a/trust/xnat/docker-compose-stack.yml b/trust/xnat/docker-compose-stack.yml index 8b7ebe310..84e42e444 100644 --- a/trust/xnat/docker-compose-stack.yml +++ b/trust/xnat/docker-compose-stack.yml @@ -36,6 +36,10 @@ services: - XNAT_SERVICE_PASSWORD=${XNAT_SERVICE_PASSWORD} - XNAT_PORT=${XNAT_PORT} - PACS_DICOM_PORT=${PACS_DICOM_PORT:-4242} + # Read by config/configure-export-mask.sh, which pins the export_mask container-service + # command to this trust's network so the converter can reach xnat-web to upload results. + # Named DOCKER_NETWORK_NAME at the stack level (see the networks block below). + - TRUST_NETWORK_NAME=${DOCKER_NETWORK_NAME} - XNAT_DATASOURCE_DRIVER=${XNAT_DATASOURCE_DRIVER} - XNAT_DATASOURCE_URL=${XNAT_DATASOURCE_URL} - XNAT_DATASOURCE_NAME=${XNAT_DATASOURCE_NAME} diff --git a/trust/xnat/scripts/ensure_plugins.sh b/trust/xnat/scripts/ensure_plugins.sh index 29f22fc31..b59a1498c 100755 --- a/trust/xnat/scripts/ensure_plugins.sh +++ b/trust/xnat/scripts/ensure_plugins.sh @@ -39,16 +39,26 @@ fi STAMP_FILE="${PLUGIN_DIR}/.s3-prefix" -# NOTE: ohif-viewer is intentionally NOT installed. FLIP uses XNAT purely as a -# DICOM store for FL training and never opens the OHIF viewer, but the plugin's +# NOTE: ohif-viewer is intentionally NOT installed by default. FLIP uses XNAT purely +# as a DICOM store for FL training and never opens the OHIF viewer, but the plugin's # per-session metadata-rebuild event listener is the dominant load on XNAT's # Reactor EventBus and materially drives the back-pressure livelock that wedges # bulk cohort imports (FLIP#662). It is excluded from the S3 sync below. +# +# MONAI_LABEL=true opts back in. AI-assisted annotation is delivered *through* the OHIF +# viewer — the MONAI Label panel lives in the viewer's Masks panel and registration targets +# the viewer's own /xapi/ohifaiaa endpoint — so without this plugin the MONAI Label server +# starts, fails to register, and exits. A trust enabling MONAI Label is accepting the FLIP#662 +# trade-off knowingly (up-trust prints the warning); a trust that does not is unaffected. required_prefixes=( "batch-launch-" "container-service-" "dicom-query-retrieve-" ) +if [[ "${MONAI_LABEL:-false}" == "true" ]]; then + echo "🧠 MONAI_LABEL=true — including the ohif-viewer plugin (FLIP#662 trade-off accepted)." + required_prefixes+=("ohif-viewer-") +fi expected_prefixes="$(printf '%s, ' "${required_prefixes[@]}")" expected_prefixes="${expected_prefixes%, }" @@ -122,11 +132,18 @@ else echo "🔁 Local plugins were synced from '${synced_prefix:-}' but this build needs '${S3_PREFIX}'." fi echo "📦 Syncing plugins from S3..." - # Exclude ohif-viewer: the trailing --exclude wins over --include for matching - # keys, so it is neither downloaded nor (with --delete) kept locally. See the - # required_prefixes note above (FLIP#662). + # Exclude ohif-viewer unless MONAI Label is on: the trailing --exclude wins over --include + # for matching keys, so it is neither downloaded nor (with --delete) kept locally. Dropping + # that trailing --exclude is what opts it back in. See the required_prefixes note above + # (FLIP#662). --delete means flipping MONAI_LABEL back to false also removes the jar again. + ohif_filter=(--exclude "ohif-viewer-*") + if [[ "${MONAI_LABEL:-false}" == "true" ]]; then + ohif_filter=() + fi + # ${a[@]+"${a[@]}"} rather than "${a[@]}": expanding an empty array under `set -u` is only + # safe from bash 4.4, and this runs on the operator's host bash, not a pinned image. aws s3 sync "s3://${S3_BUCKET}/${S3_PREFIX}/" "${PLUGIN_DIR}/" --delete \ - --exclude "*" --include "*.jar" --exclude "ohif-viewer-*" + --exclude "*" --include "*.jar" ${ohif_filter[@]+"${ohif_filter[@]}"} printf '%s\n' "${S3_PREFIX}" > "${STAMP_FILE}" fi @@ -134,6 +151,20 @@ missing_prefixes="$(find_missing_prefixes)" if [[ -n "${missing_prefixes}" ]]; then echo "❌ ERROR: Missing required plugin families after sync: ${missing_prefixes//$'\n'/ }" echo " Expected plugin prefixes: ${expected_prefixes}" + # The ohif-viewer jar is a known gap rather than a mistake, so name the remedy instead of + # leaving the operator to work out why the one plugin they just opted into is not published. + # It is absent from the version-keyed prefix this script syncs (verified against + # s3://flipdev-artifacts/xnat-1.10.0/plugins/); only the legacy s3:///xnat/plugins/ + # prefix has one, at 3.7.1 — too old for XNAT 1.10, which is why the k8s chart pins 3.8.0. + if [[ "${missing_prefixes}" == *"ohif-viewer-"* ]]; then + echo + echo " ohif-viewer is required because MONAI_LABEL=true, but it is not published under" + echo " s3://${S3_BUCKET}/${S3_PREFIX}/. Publish it there once, matching this XNAT version:" + echo " curl -LO https://xnat.org/files/ohif-viewer-xnat-plugin/ohif-viewer-3.8.0-fat.jar" + echo " aws s3 cp ohif-viewer-3.8.0-fat.jar s3://${S3_BUCKET}/${S3_PREFIX}/" + echo " (the k8s chart fetches that same 3.8.0 jar directly — see xnat.web.plugins.urls)." + echo " Or set MONAI_LABEL=false to bring this trust up without AI-assisted annotation." + fi exit 1 fi diff --git a/trust/xnat/tests/test_dcm2niix_subscription_cleanup.py b/trust/xnat/tests/test_dcm2niix_subscription_cleanup.py new file mode 100644 index 000000000..50638e114 --- /dev/null +++ b/trust/xnat/tests/test_dcm2niix_subscription_cleanup.py @@ -0,0 +1,117 @@ +# Copyright (c) 2026 Guy's and St Thomas' NHS Foundation Trust & King's College London +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +""" +``configure-dcm2niix.sh`` must never delete an event subscription. + +dcm2niix subscriptions are per-project, created by imaging-api from the project's +``dicom_to_nifti`` flag. Deleting one silently stops DICOM->NIfTI conversion for +that project: imports keep succeeding and simply never convert, which surfaces +much later as training with no data, pointing nowhere near the cause. + +The script used to carry a cleanup step for the site-wide subscription that older +versions of it created. That subscription stopped being created in 88adb78b +(2026-03-24), so the cleanup had nothing legitimate left to find — but it could +still match live per-project subscriptions, because: + +- XNAT does not echo a top-level ``project-id`` for them (the key is absent), so + any "is this site-wide?" test based on that field says yes; and +- imaging-api names them ``DICOM-NIfTI Conversion``, which was one of the names the + cleanup matched. + +Checked against four running trusts: it selected 12 of 12 subscriptions, all of them +live per-project rules, with no genuine leftover among them. The step was removed +rather than repaired — a cleanup for something nothing creates can only misfire. + +This test exists so it does not come back. It reads the script rather than any +snapshot of it, so re-adding a delete in any form fails here. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[3] +CONFIGURE_DCM2NIIX = REPO_ROOT / "trust/xnat/xnat/config/configure-dcm2niix.sh" +CONFIGURE_EXPORT_MASK = REPO_ROOT / "trust/xnat/xnat/config/configure-export-mask.sh" + +# Matches a DELETE aimed at the event-subscription endpoint, however it is spelled: +# `-X DELETE "$XNAT_URL/xapi/events/subscription/$ID"`, `--request DELETE …`, or with +# the URL built up in a variable first. +_SUBSCRIPTION_DELETE = re.compile( + r"(-X\s+DELETE|--request\s+DELETE)(?!.*\n?.*xapi/commands)", re.IGNORECASE +) +_SUBSCRIPTION_ENDPOINT = re.compile(r"xapi/events/subscription", re.IGNORECASE) + + +def _strip_comments(script: str) -> str: + """Drop comment lines so prose about deletion does not trip the scan. + + Args: + script: Full shell-script source. + + Returns: + The source with whole-line comments removed. + """ + return "\n".join(line for line in script.splitlines() if not line.lstrip().startswith("#")) + + +def test_dcm2niix_script_deletes_no_event_subscription() -> None: + """The guard: no executable line may DELETE an event subscription.""" + code = _strip_comments(CONFIGURE_DCM2NIIX.read_text()) + + offending = [ + line + for line in code.splitlines() + if _SUBSCRIPTION_ENDPOINT.search(line) and re.search(r"DELETE", line, re.IGNORECASE) + ] + + assert not offending, ( + "configure-dcm2niix.sh deletes an event subscription again:\n" + + "\n".join(f" {line.strip()}" for line in offending) + + "\n\nPer-project subscriptions are indistinguishable from the retired site-wide one " + "by name or by project-id (XNAT omits that key for them), so any such delete removes " + "live conversion rules. Nothing has created a site-wide subscription since 2026-03-24." + ) + + +def test_dcm2niix_script_does_not_enumerate_subscriptions_for_deletion() -> None: + """Belt and braces: it should not even fetch the subscription list. + + The removed cleanup started by GETting ``/xapi/events/subscriptions``. Nothing + else in this script has any reason to, so a reappearing read is the first step + of a reappearing delete and worth catching on its own. + """ + code = _strip_comments(CONFIGURE_DCM2NIIX.read_text()) + + assert "xapi/events/subscriptions" not in code, ( + "configure-dcm2niix.sh reads the event-subscription list again. This script " + "creates and enables a command; it has no business enumerating subscriptions." + ) + + +def test_export_mask_script_still_manages_only_its_own_subscription() -> None: + """The contrast case — deleting is fine when it is your own, matched by name. + + ``configure-export-mask.sh`` legitimately replaces its own subscription so a + re-run does not accumulate duplicates. That delete is name-matched to the + export_mask subscription it just created, never a blanket sweep, and this test + documents the difference rather than forbidding deletion outright. + """ + code = _strip_comments(CONFIGURE_EXPORT_MASK.read_text()) + + assert "EXPORT_MASK_SUBSCRIPTION_NAME" in code, ( + "configure-export-mask.sh no longer scopes its subscription delete by name; " + "an unscoped delete there would sweep up per-project dcm2niix subscriptions." + ) + deletes = [line for line in code.splitlines() if _SUBSCRIPTION_ENDPOINT.search(line) and "DELETE" in line] + assert deletes, "configure-export-mask.sh should still replace its own subscription on re-run." diff --git a/trust/xnat/xnat/config/configure-dcm2niix.sh b/trust/xnat/xnat/config/configure-dcm2niix.sh index 86a5d912c..d20893640 100644 --- a/trust/xnat/xnat/config/configure-dcm2niix.sh +++ b/trust/xnat/xnat/config/configure-dcm2niix.sh @@ -221,14 +221,23 @@ xnat_curl -X PUT "$XNAT_URL/xapi/events/prefs" \ # controlled by the dicom_to_nifti flag. This ensures dcm2niix only auto-triggers # for projects that have opted in to DICOM-to-NIfTI conversion. -# Clean up any legacy site-wide event subscriptions (from prior versions) -echo "Cleaning up legacy site-wide event subscriptions..." -SUBS=$(xnat_curl "$XNAT_URL/xapi/events/subscriptions") -SITE_SUB_IDS=$(echo "$SUBS" | jq -r '.[] | select(.["project-id"] == null or .["project-id"] == "") | .id') -for SUB_ID in $SITE_SUB_IDS; do - echo "Deleting site-wide subscription $SUB_ID..." - xnat_curl -X DELETE "$XNAT_URL/xapi/events/subscription/$SUB_ID" >/dev/null -done +# This script deliberately DELETES NO EVENT SUBSCRIPTIONS. Do not add a cleanup step here. +# +# It used to carry one, to remove the site-wide subscription that earlier versions of this +# script created. That subscription stopped being created in 88adb78b (2026-03-24), when +# dcm2niix moved to per-project subscriptions — so the cleanup had nothing legitimate left to +# find, while remaining perfectly capable of deleting the wrong thing. +# +# And it did. XNAT does not echo a top-level `project-id` for imaging-api's per-project +# subscriptions (the key is absent entirely), and imaging-api names them "DICOM-NIfTI +# Conversion" — one of the two names the cleanup matched. So both of its tests for "is this the +# old site-wide one?" answered yes for live, in-use subscriptions. Checked against four running +# trusts: it selected 12 of 12, every one of them a project's live conversion rule, and not one +# genuine site-wide leftover among them. Deleting one is silent — imports keep succeeding and +# simply never convert, surfacing much later as training with no data. +# +# Removed rather than repaired: a cleanup for a thing nothing creates can only ever misfire. +# Pinned by trust/xnat/tests/test_dcm2niix_subscription_cleanup.py. # ---------------------------------------------------------------- # VALIDATION diff --git a/trust/xnat/xnat/config/configure-export-mask.sh b/trust/xnat/xnat/config/configure-export-mask.sh new file mode 100755 index 000000000..eb319d909 --- /dev/null +++ b/trust/xnat/xnat/config/configure-export-mask.sh @@ -0,0 +1,190 @@ +#!/bin/bash +# +# Copyright (c) Guy's and St Thomas' NHS Foundation Trust & King's College London +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +# Exit on first error, undefined variable, or pipe failure. +set -euo pipefail + +# Registers the export_mask container-service command and its event subscription. +# +# When a user saves a segmentation in the XNAT OHIF viewer it lands as a DICOM-SEG assessor. +# This command converts that assessor to NIfTI and uploads it back to the session, which is +# the format FL training consumes. Nothing upstream does this — see the XNAT discussion list +# thread on MONAI Label + OHIF, where the missing DICOM-SEG -> NIfTI step is the open problem. +# +# Run ONLY on trusts that enable MONAI Label — `make xnat-configure` gates this on +# MONAI_LABEL=true (see xnat-configure-export-mask in trust/xnat/Makefile). A trust without +# MONAI Label has no OHIF viewer plugin, so it can never create the assessors this reacts to; +# registering the converter command and a site-wide subscription there would be pure blast +# radius. Turning MONAI Label on later requires re-running `make xnat-configure`. +# +# Must run AFTER configure-dcm2niix.sh, which registers the Docker backend and verifies the +# socket proxy. The subscription created here is deliberately site-wide: a DICOM-SEG can be +# created in any project by anyone using the OHIF viewer, and there is no per-project opt-in +# for it (unlike dcm2niix's dicom_to_nifti flag). +# +# Required environment variables: +# XNAT_ADMIN_USER - XNAT admin username +# XNAT_ADMIN_PASSWORD - XNAT admin password (must match what configure-xnat.sh set) +# TRUST_NETWORK_NAME - Docker network the launched container joins; it must reach xnat-web, +# because the converter pulls the session's NIfTI resources and uploads +# its output through the XNAT REST API (unlike dcm2niix, which only +# touches mounted files and therefore needs no network). + +XNAT_URL="http://xnat-web:8080" # internal to Docker network +EXPORT_MASK_NAME="export_mask" +EXPORT_MASK_SUBSCRIPTION_NAME="Convert exported OHIF masks to NIfTI" + +if [[ -z "${TRUST_NETWORK_NAME:-}" ]]; then + echo "ERROR: TRUST_NETWORK_NAME is not set. The export_mask container would start with no" >&2 + echo " route to xnat-web and every conversion would fail at upload time." >&2 + exit 1 +fi + +# Helper: curl wrapper for XNAT's REST API. Mirrors configure-dcm2niix.sh — on 2xx it emits +# the response body on stdout; on anything else it reports the request and body on stderr and +# returns non-zero, which `set -e` turns into an abort at the call site (so don't call it +# inside `if`/`||` without handling the failure). +xnat_curl() { + local response status body curl_exit=0 + response=$(curl -sS --connect-timeout 10 --max-time 120 -w '\n%{http_code}' "$@" \ + -u "${XNAT_ADMIN_USER}:${XNAT_ADMIN_PASSWORD}") || curl_exit=$? + if [[ "$curl_exit" -ne 0 ]]; then + echo "ERROR: curl transport failure (exit $curl_exit)" >&2 + echo " args: $*" >&2 + return 1 + fi + status=$(printf '%s' "$response" | tail -n1) + body=$(printf '%s' "$response" | sed '$d') + # Strip CRs so a middlebox emitting \r\n can't break the numeric guards below. + status=${status//$'\r'/} + body=${body//$'\r'/} + if ! [[ "$status" =~ ^2[0-9]{2}$ ]]; then + echo "ERROR: XNAT request failed with HTTP $status" >&2 + echo " args: $*" >&2 + echo " body: $body" >&2 + return 1 + fi + printf '%s' "$body" +} + +# ---------------------------------------------------------------- +# COMMAND +# ---------------------------------------------------------------- + +echo "Checking if $EXPORT_MASK_NAME command exists..." +COMMAND_ID=$(xnat_curl "$XNAT_URL/xapi/commands?name=$EXPORT_MASK_NAME" | jq -r '.[0].id // empty') +if [[ -n "$COMMAND_ID" ]]; then + echo "Found existing command ID: $COMMAND_ID. Deleting..." + xnat_curl -X DELETE "$XNAT_URL/xapi/commands/$COMMAND_ID" >/dev/null +fi + +# Pin the launched container to this trust's network. The JSON ships a placeholder rather +# than a name because the network is per-trust (deploy_trust-network-). +echo "Adding $EXPORT_MASK_NAME command on network $TRUST_NETWORK_NAME..." +CMD_ID=$(jq --arg network "$TRUST_NETWORK_NAME" '.network = $network' export_mask_command.json \ + | xnat_curl -X POST "$XNAT_URL/xapi/commands" -H "Content-Type: application/json" -d @-) + +# POST /xapi/commands returns the new command's numeric id as a bare JSON number (see the +# longer note in configure-dcm2niix.sh), so guard on that rather than re-GETting by name. +if ! [[ "$CMD_ID" =~ ^[0-9]+$ ]]; then + echo "ERROR: POST /xapi/commands did not return a numeric command id." >&2 + echo " body: $CMD_ID" >&2 + exit 1 +fi + +WRAPPER_NAME=$(jq -r '.xnat[0].name // empty' export_mask_command.json) +if [[ -z "$WRAPPER_NAME" ]]; then + echo "ERROR: no .xnat[0].name wrapper in export_mask_command.json." >&2 + exit 1 +fi + +echo "Command ID: $CMD_ID" +echo "Wrapper Name: $WRAPPER_NAME" + +echo "Enabling $EXPORT_MASK_NAME command site-wide..." +xnat_curl -X PUT "$XNAT_URL/xapi/commands/$CMD_ID/wrappers/$WRAPPER_NAME/enabled" >/dev/null + +# ---------------------------------------------------------------- +# EVENT SUBSCRIPTION +# ---------------------------------------------------------------- + +# Replace any previous copy so re-running this script is idempotent rather than accumulating +# duplicate subscriptions that would each launch a container for the same assessor. +echo "Removing any previous '$EXPORT_MASK_SUBSCRIPTION_NAME' subscription..." +EXISTING=$(xnat_curl "$XNAT_URL/xapi/events/subscriptions" \ + | jq -r --arg name "$EXPORT_MASK_SUBSCRIPTION_NAME" '.[] | select(.name == $name) | .id') +for SUB_ID in $EXISTING; do + echo " deleting subscription $SUB_ID" + xnat_curl -X DELETE "$XNAT_URL/xapi/events/subscription/$SUB_ID" >/dev/null +done + +echo "Creating event subscription for $EXPORT_MASK_NAME..." +sed "s/\${CMD_ID}/$CMD_ID/g" export_mask_event.json \ + | xnat_curl -X POST "$XNAT_URL/xapi/events/subscription" \ + -H "Content-Type: application/json" -d @- >/dev/null + +# ---------------------------------------------------------------- +# VALIDATION +# ---------------------------------------------------------------- + +# Every xnat_curl above aborts on a non-2xx, so reaching here means each write was accepted; +# these re-GETs check the resulting state actually persisted. +echo " " +echo "Validating export_mask setup..." +WRAPPER_ENABLED=$(xnat_curl "$XNAT_URL/xapi/commands/$CMD_ID/wrappers/$WRAPPER_NAME/enabled") +if [ "$WRAPPER_ENABLED" != "true" ]; then + echo "ERROR: export_mask wrapper is not enabled site-wide (expected true)" >&2 + echo " body: $WRAPPER_ENABLED" >&2 + exit 1 +fi + +SUBSCRIPTIONS=$(xnat_curl "$XNAT_URL/xapi/events/subscriptions") + +SUB_COUNT=$(printf '%s' "$SUBSCRIPTIONS" \ + | jq -r --arg name "$EXPORT_MASK_SUBSCRIPTION_NAME" '[.[] | select(.name == $name)] | length') +if [ "$SUB_COUNT" != "1" ]; then + echo "ERROR: expected exactly 1 '$EXPORT_MASK_SUBSCRIPTION_NAME' subscription, found $SUB_COUNT" >&2 + exit 1 +fi + +# Counting by name alone is not enough. XNAT accepts a subscription whose event-selector names +# a class it cannot resolve, and stores it happily — the subscription then simply never fires, +# so DICOM-SEG assessors are created and silently never converted. Nothing surfaces in the +# viewer, and the only trace is the absence of entries in Command History. Assert the selector +# actually round-tripped as an ImageAssessorEvent, and that the subscription is active. +SUB_SELECTOR=$(printf '%s' "$SUBSCRIPTIONS" \ + | jq -r --arg name "$EXPORT_MASK_SUBSCRIPTION_NAME" '.[] | select(.name == $name) | .["event-selector"] // empty') +SUB_ACTIVE=$(printf '%s' "$SUBSCRIPTIONS" \ + | jq -r --arg name "$EXPORT_MASK_SUBSCRIPTION_NAME" '.[] | select(.name == $name) | .active') + +echo " event-selector: $SUB_SELECTOR" +# Substring rather than equality: XNAT is free to normalise the stored form, and a spurious +# bring-up failure over cosmetic drift would be worse than the bug this guards against. +case "$SUB_SELECTOR" in + *ImageAssessorEvent*) ;; + *) + echo "ERROR: subscription event-selector is '$SUB_SELECTOR', expected an ImageAssessorEvent." >&2 + echo " XNAT event classes carry the 'Event' suffix; a selector it cannot resolve is" >&2 + echo " stored without complaint and then never fires." >&2 + exit 1 + ;; +esac + +if [ "$SUB_ACTIVE" != "true" ]; then + echo "ERROR: '$EXPORT_MASK_SUBSCRIPTION_NAME' subscription is not active (got '$SUB_ACTIVE')" >&2 + exit 1 +fi + +echo " " +echo "✅ export_mask configuration complete and validated!" diff --git a/trust/xnat/xnat/config/export_mask_command.json b/trust/xnat/xnat/config/export_mask_command.json new file mode 100644 index 000000000..669e26fe4 --- /dev/null +++ b/trust/xnat/xnat/config/export_mask_command.json @@ -0,0 +1,175 @@ +{ + "name": "export_mask", + "label": "export_mask", + "description": "Convert an OHIF DICOM-SEG assessor to NIfTI and upload it back to XNAT", + "version": "1.0", + "schema-version": "1.0", + "info-url": "https://hub.docker.com/repository/docker/atriaybagur/aic-ohif-dicomseg-to-nifti/general", + "type": "docker", + "image": "atriaybagur/aic-ohif-dicomseg-to-nifti:latest", + "network": "#PLACEHOLDER_NETWORK#", + "command-line": "uv run convert_dicom_seg_assessor_to_nifti.py -i /segmentation-in -o /output --project #PROJECT# --experiment #SESSION# --assessor #ASSESSOR#", + "mounts": [ + { + "name": "output", + "writable": true, + "path": "/output" + }, + { + "name": "segmentation-in", + "writable": false, + "path": "/segmentation-in" + }, + { + "name": "session-in", + "writable": false, + "path": "/session-in" + } + ], + "inputs": [ + { + "name": "assessor", + "description": "XNAT ID of the assessor", + "type": "string", + "required": true, + "replacement-key": "#ASSESSOR#" + }, + { + "name": "label", + "description": "XNAT ID of the assessor", + "type": "string", + "required": true, + "replacement-key": "#LABEL#" + }, + { + "name": "session", + "description": "XNAT ID of the session", + "type": "string", + "required": true, + "replacement-key": "#SESSION#" + }, + { + "name": "subject", + "description": "XNAT ID of the subject", + "type": "string", + "required": true, + "replacement-key": "#SUBJECT#" + }, + { + "name": "project", + "description": "XNAT ID of the project", + "type": "string", + "required": true, + "replacement-key": "#PROJECT#" + } + ], + "outputs": [ + { + "name": "analysis-results", + "description": "updated analysis results", + "required": true, + "mount": "output" + } + ], + "xnat": [ + { + "name": "export_mask-assessor", + "label": "export_mask", + "description": "Convert OHIF mask to NIFTI", + "contexts": [ + "icr:roiCollectionData" + ], + "external-inputs": [ + { + "name": "assessor", + "description": "Input assessor", + "type": "Assessor", + "required": true, + "load-children": true, + "matcher": "'SEG' in @.resources[*].label" + } + ], + "derived-inputs": [ + { + "name": "session", + "description": "Input session", + "type": "Session", + "required": true, + "load-children": true, + "derived-from-wrapper-input": "assessor", + "provides-files-for-command-mount": "session-in" + }, + { + "name": "assessor-id", + "description": "The assessor's id", + "type": "string", + "derived-from-wrapper-input": "assessor", + "derived-from-xnat-object-property": "id", + "provides-value-for-command-input": "assessor" + }, + { + "name": "assessor-label", + "description": "The assessor's id", + "type": "string", + "derived-from-wrapper-input": "assessor", + "derived-from-xnat-object-property": "label", + "provides-value-for-command-input": "label" + }, + { + "name": "segmentation-dicom", + "type": "Resource", + "derived-from-wrapper-input": "assessor", + "matcher": "@.label == 'SEG'", + "provides-files-for-command-mount": "segmentation-in" + }, + { + "name": "session-id", + "description": "The session's id", + "type": "string", + "derived-from-wrapper-input": "session", + "derived-from-xnat-object-property": "id", + "provides-value-for-command-input": "session" + }, + { + "name": "subject", + "description": "Subject input derived from Session", + "type": "Subject", + "required": true, + "derived-from-wrapper-input": "session" + }, + { + "name": "subject-label", + "description": "The subject's label", + "type": "string", + "derived-from-wrapper-input": "subject", + "derived-from-xnat-object-property": "label", + "provides-value-for-command-input": "subject" + }, + { + "name": "project", + "description": "Project input derived from Session", + "type": "Project", + "required": true, + "derived-from-wrapper-input": "session" + }, + { + "name": "project-id", + "description": "The project's id", + "type": "string", + "derived-from-wrapper-input": "project", + "derived-from-xnat-object-property": "id", + "provides-value-for-command-input": "project" + } + ], + "output-handlers": [ + { + "name": "analysis-resource", + "accepts-command-output": "analysis-results", + "as-a-child-of-wrapper-input": "session", + "type": "Resource", + "label": "analysis" + } + ] + } + ] +} diff --git a/trust/xnat/xnat/config/export_mask_event.json b/trust/xnat/xnat/config/export_mask_event.json new file mode 100644 index 000000000..58326ffe5 --- /dev/null +++ b/trust/xnat/xnat/config/export_mask_event.json @@ -0,0 +1,14 @@ +{ + "name": "Convert exported OHIF masks to NIfTI", + "event-selector": "org.nrg.xnat.eventservice.events.ImageAssessorEvent:CREATED", + "action-key": "org.nrg.containers.services.CommandActionProvider:${CMD_ID}", + "attributes": {}, + "active": true, + "project-id": "", + "event-filter": { + "event-type": "org.nrg.xnat.eventservice.events.ImageAssessorEvent", + "status": "CREATED", + "payload-filter": "" + }, + "act-as-event-user": false +}