-
Notifications
You must be signed in to change notification settings - Fork 94
chore(dynamo-platform): coordinate NATS removal with Dynamo 1.4+ bump #1983
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
fff1669
5bc368b
0a19166
053a47a
89db397
46b20ba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,40 +1,54 @@ | ||
| # EKS Dynamo Networking Prerequisites | ||
|
|
||
| For `*-eks-ubuntu-inference-dynamo` recipes, AICR configures | ||
| `dynamo-platform` with Kubernetes-native discovery and the standard NATS | ||
| event plane for KV-cache and runtime events: | ||
| - `nats` on TCP `4222` | ||
|
|
||
| This NATS dependency is new as of the Dynamo 1.2 bump, which switched discovery | ||
| to the NATS event plane. A cluster whose system-node security group only | ||
| allowlisted the pre-1.2 control-plane ports will not have `4222` open, so a | ||
| bundle that worked on Dynamo 1.0.x can start failing purely from the version | ||
| bump — add the `4222` rule below. | ||
|
|
||
| Frontend-to-worker inference request/response traffic is separate: Dynamo 1.2 | ||
| defaults `DYN_REQUEST_PLANE` to TCP, and AICR does not override it to NATS. The | ||
| worker runtime relays local vLLM ZMQ KV-cache events onto the NATS-backed event | ||
| plane so the KV router or EPP can consume live cache state. | ||
|
|
||
| If system components and GPU workloads are on different node groups/security groups, these ports may be blocked from GPU nodes to system nodes. Typical symptoms: | ||
| - `JetStream not available` (NATS unreachable) | ||
| - Dynamo frontend and vLLM worker pods stuck in `CrashLoopBackOff`, with | ||
| `Exception: Failed to connect to NATS: timed out` in the frontend log | ||
| - Worker startup probes failing with `connection refused` because the process | ||
| exits before serving | ||
| - The `inference-perf` performance validator failing after its workload-readiness | ||
| (10 min) and health (5 min) gates lapse — roughly 15 min — while `deployment` | ||
| and `conformance` pass; the workload never reaches a ready state | ||
| `dynamo-platform` with Kubernetes-native discovery. As of the Dynamo 1.4+ | ||
| bump, AICR no longer installs bundled NATS by default: the request plane | ||
| defaults to TCP and the KV event plane defaults to ZMQ | ||
| (`ai-dynamo/dynamo#11951`). This removes the old `4222` NATS requirement, | ||
| but it does **not** remove the underlying cross-nodegroup networking | ||
| requirement — the request plane and KV events are now **direct | ||
| frontend↔worker pod-to-pod connections** instead of both sides talking to a | ||
| `dynamo-platform-nats` StatefulSet on the system nodegroup, and Frontend | ||
| pods still run on the system nodegroup while workers run on the GPU | ||
| nodegroup, so traffic still crosses the same GPU↔system nodegroup SG | ||
| boundary as before. | ||
|
|
||
| > **TODO before merging (tracked in NVIDIA/aicr#1836):** the port(s) below | ||
| > are not yet confirmed against a real Dynamo 1.4+ EKS deployment. What's | ||
| > known from the AICR recipes: the ZMQ KV-event endpoint is set explicitly | ||
| > per worker via `--kv-events-config`, e.g. | ||
| > `{"enable_kv_cache_events":true,"publisher":"zmq","endpoint":"tcp://*:5557"}` | ||
| > (see `tests/manifests/dynamo-vllm-smoke-test.yaml`), offset by `+dp_rank` | ||
| > for dp_rank > 0. The TCP request plane does not have one fixed, | ||
| > documented port the way NATS had `4222` — confirm the actual listening | ||
| > port(s) on a live cluster before finalizing the SG rule below: | ||
| > ```shell | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Add a blank line before the fenced block.
🧰 Tools🪛 markdownlint-cli2 (0.23.1)[warning] 25-25: Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) 🤖 Prompt for AI AgentsSource: Linters/SAST tools
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 Nitpick — MD031 — fenced block not preceded by a blank line Inside the blockquote, line 24 is immediately followed by the opening ```shell fence at line 25 with no blank Blast radius: markdownlint / doc build; cosmetic. Fix: Insert a blank blockquote line ( |
||
| > kubectl exec -n dynamo-system <frontend-pod> -- ss -tlnp | ||
| > kubectl exec -n dynamo-system <worker-pod> -- ss -tlnp | ||
| > ``` | ||
|
|
||
| If the GPU and system node groups sit in different security groups, these | ||
| ports may be blocked from GPU nodes to the frontend's node (and vice versa). | ||
| Typical symptoms: | ||
| - Dynamo frontend and vLLM worker pods stuck in `CrashLoopBackOff`, or a | ||
| frontend that starts cleanly but never successfully routes a request | ||
| through to a worker | ||
| - Worker startup probes failing with `connection refused` because the | ||
| process exits before serving | ||
| - The `inference-perf` performance validator failing after its | ||
| workload-readiness (10 min) and health (5 min) gates lapse — roughly | ||
| 15 min — while `deployment` and `conformance` pass; the workload never | ||
| reaches a ready state | ||
|
|
||
| You can confirm reachability directly from a GPU node before re-running. The | ||
| toleration is required because the GPU node groups on these clusters are | ||
| tainted (`NoSchedule`/`NoExecute`); without it the probe pod stays `Pending` | ||
| and never runs: | ||
|
|
||
| ```shell | ||
| kubectl run nats-probe --rm -i --restart=Never --image=busybox:1.36 \ | ||
| kubectl run tcp-probe --rm -i --restart=Never --image=busybox:1.36 \ | ||
| --overrides='{"spec":{"nodeSelector":{"<gpu-node-label-key>":"<value>"},"tolerations":[{"operator":"Exists"}]}}' \ | ||
| -- sh -c 'nc -zv -w 5 dynamo-platform-nats.dynamo-system.svc.cluster.local 4222' | ||
| -- sh -c 'nc -zv -w 5 <worker-pod-ip-or-svc> <PORT>' | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| ``` | ||
|
|
||
| The conformance validator's `ai-service-metrics` check adds a third requirement: | ||
|
|
@@ -64,8 +78,9 @@ SG rule below remains the reliable cluster-side guarantee. | |
|
|
||
| ## Required Security Group Rules | ||
|
|
||
| Allow ingress from the GPU node security group to the system node security group on: | ||
| - TCP `4222` - NATS event plane (dynamo-platform) | ||
| Allow ingress from the GPU node security group to the system node security | ||
| group on: | ||
| - TCP `<PORT>` - Dynamo request plane + KV events (dynamo-platform) — confirm exact port(s) on-cluster, see TODO above | ||
|
mohityadav8 marked this conversation as resolved.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 Major — Networking doc ships placeholders + 'TODO before merging' Lines 16 (TODO before merging, #1836), 51, 83, 114 carry unresolved placeholders in the probe, the required SG-rule list, and the AWS CLI example. This doc is authoritative — linked from docs/README.md, docs/integrator/index.md, and deep-linked from docs/user/validation.md#required-security-group-rules — so an operator with split GPU/system SGs cannot open the correct rule from it, and hits the exact CrashLoop/timeout symptoms it describes. Blast radius: Operators cannot complete the SG rule; overlaps mchmarny's standing MAJOR. Fix: Confirm the real request-plane + ZMQ ports on a live 1.4.1 EKS cluster (per the doc's own ss -tlnp step), fill all three s and remove the TODO — or hold the rewrite until #1836 resolves ports. |
||
| - TCP `9090` - Prometheus (required for the `ai-service-metrics` conformance check) | ||
|
|
||
| The `9090` rule is required as a fallback guarantee: the orchestrator *prefers* | ||
|
|
@@ -94,9 +109,9 @@ aws ec2 describe-instances \ | |
| --query "Reservations[0].Instances[0].SecurityGroups[*].GroupId" \ | ||
| --output text | ||
|
|
||
| # 2) Allow NATS + Prometheus from GPU SG -> system SG | ||
| # 2) Allow Dynamo request/event-plane + Prometheus from GPU SG -> system SG | ||
| aws ec2 authorize-security-group-ingress --group-id <system-sg-id> \ | ||
| --protocol tcp --port 4222 --source-group <gpu-sg-id> | ||
| --protocol tcp --port <PORT> --source-group <gpu-sg-id> | ||
|
|
||
| aws ec2 authorize-security-group-ingress --group-id <system-sg-id> \ | ||
| --protocol tcp --port 9090 --source-group <gpu-sg-id> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,11 +19,11 @@ A machine-readable **CycloneDX 1.6 JSON** companion to this page is produced by | |
| <!-- BEGIN AICR-BOM --> | ||
| ## Summary | ||
|
|
||
| - Components: **44** | ||
| - Unique images: **100** | ||
| - Distinct registries: **11** | ||
| - Components: **37** | ||
| - Unique images: **88** | ||
| - Distinct registries: **10** | ||
|
|
||
| Registries: `602401143452.dkr.ecr.us-west-2.amazonaws.com`, `cr.agentgateway.dev`, `docker.io`, `gcr.io`, `ghcr.io`, `gke.gcr.io`, `nvcr.io`, `public.ecr.aws`, `quay.io`, `registry.k8s.io`, `us-docker.pkg.dev` | ||
| Registries: `602401143452.dkr.ecr.us-west-2.amazonaws.com`, `cr.agentgateway.dev`, `docker.io`, `gcr.io`, `ghcr.io`, `gke.gcr.io`, `nvcr.io`, `quay.io`, `registry.k8s.io`, `us-docker.pkg.dev` | ||
|
|
||
| _Rendering fidelity:_ `catalog-parity: charts are rendered with the shared recipes/components/<name>/values.yaml; per-recipe overlay overrides are not applied` | ||
|
|
||
|
|
@@ -33,12 +33,10 @@ _Rendering fidelity:_ `catalog-parity: charts are rendered with the shared recip | |
| |-----------|------|-------|----------------|--------| | ||
| | agentgateway | helm | agentgateway | v1.3.1 | 1 | | ||
| | agentgateway-crds | helm | agentgateway-crds | v1.3.1 | 0 | | ||
| | aws-ebs-csi-driver | helm | aws-ebs-csi-driver/aws-ebs-csi-driver | 2.59.0 | 6 | | ||
| | aws-ebs-csi-driver | helm | aws-ebs-csi-driver/aws-ebs-csi-driver | 2.59.0 | 0 | | ||
| | aws-efa | helm | aws-efa-k8s-device-plugin | v0.5.29 | 1 | | ||
| | cert-manager | helm | jetstack/cert-manager | v1.20.2 | 4 | | ||
| | cert-manager-ocp | manifest | — | — | 0 | | ||
| | cert-manager-ocp-olm | manifest | — | — | 0 | | ||
| | dynamo-platform | helm | dynamo-platform | 1.2.1 | 3 | | ||
| | dynamo-platform | helm | dynamo-platform | 1.4.1 | 1 | | ||
| | gatekeeper | helm | gatekeeper/gatekeeper | 3.22.2 | 3 | | ||
| | gcp-driver-installer | manifest | — | — | 3 | | ||
| | gke-nccl-tcpxo | manifest | — | — | 4 | | ||
|
|
@@ -65,10 +63,8 @@ _Rendering fidelity:_ `catalog-parity: charts are rendered with the shared recip | |
| | nodewright-customizations | manifest | — | — | 5 | | ||
| | nodewright-operator | helm | nodewright | v0.17.1 | 3 | | ||
| | nvidia-dra-driver-gpu | helm | dra-driver-nvidia-gpu | 0.4.1 | 1 | | ||
| | nvidia-dra-driver-gpu-ocp | helm | dra-driver-nvidia-gpu | 0.4.1 | 1 | | ||
| | nvsentinel | helm | nvsentinel | v1.20.0 | 6 | | ||
| | prometheus-adapter | helm | prometheus-community/prometheus-adapter | 5.3.0 | 1 | | ||
| | prometheus-adapter-ocp | helm | prometheus-community/prometheus-adapter | 5.3.0 | 1 | | ||
| | nvsentinel | helm | nvsentinel | v1.9.0 | 6 | | ||
| | prometheus-adapter | helm | prometheus-community/prometheus-adapter | 5.3.0 | 0 | | ||
| | prometheus-operator-crds | helm | prometheus-community/prometheus-operator-crds | 28.0.1 | 0 | | ||
| | slinky-slurm | helm | slurm | 1.2.0 | 5 | | ||
| | slinky-slurm-operator | helm | slurm-operator | 1.2.0 | 2 | | ||
|
|
@@ -97,12 +93,9 @@ _No images extracted._ | |
|
|
||
| ### aws-ebs-csi-driver | ||
|
|
||
| - `public.ecr.aws/csi-components/csi-attacher:v4.11.0-eksbuild.4` | ||
| - `public.ecr.aws/csi-components/csi-node-driver-registrar:v2.16.0-eksbuild.4` | ||
| - `public.ecr.aws/csi-components/csi-provisioner:v6.2.0-eksbuild.3` | ||
| - `public.ecr.aws/csi-components/csi-resizer:v2.1.0-eksbuild.4` | ||
| - `public.ecr.aws/csi-components/livenessprobe:v2.18.0-eksbuild.4` | ||
| - `public.ecr.aws/ebs-csi-driver/aws-ebs-csi-driver:v1.59.0` | ||
| > Warning: [INTERNAL] helm template failed: signal: killed | ||
|
|
||
| _No images extracted._ | ||
|
Comment on lines
+96
to
+98
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Do not treat failed Helm renders as zero-image inventories. Both entries omit required images because
📍 Affects 1 file
🤖 Prompt for AI Agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These helm template: signal: killed failures for aws-ebs-csi-driver and prometheus-adapter are pre-existing - they appear in the current main branch BOM as well and are not introduced by this PR. The make bom-docs output is deterministic given the current environment; fixing the render failures is out of scope for this change.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
file="docs/user/container-images.md"
echo "Current branch:"
git branch --show-current
echo
echo "Available main references:"
git show-ref --verify --quiet refs/remotes/origin/main && echo "origin/main exists" || true
git show-ref --verify --quiet refs/heads/main && echo "local main exists" || true
echo
base_ref=""
if git show-ref --verify --quiet refs/remotes/origin/main; then
base_ref="origin/main"
elif git show-ref --verify --quiet refs/heads/main; then
base_ref="main"
else
echo "No main reference is available for comparison." >&2
exit 2
fi
for ref in HEAD "$base_ref"; do
echo "===== $ref: aws-ebs-csi-driver ====="
git show "${ref}:${file}" | sed -n '/^### aws-ebs-csi-driver$/,/^### /p' | sed '$d'
echo
echo "===== $ref: prometheus-adapter ====="
git show "${ref}:${file}" | sed -n '/^### prometheus-adapter$/,/^### /p' | sed '$d'
echo
done
echo "===== Diff for the two sections against $base_ref ====="
git diff --unified=3 "$base_ref"...HEAD -- "$file" | \
rg -n -C 3 'aws-ebs-csi-driver|prometheus-adapter|helm template failed|No images extracted' || trueLength of output: 4617
This PR replaces those inventories with You are interacting with an AI system. |
||
|
|
||
| ### aws-efa | ||
|
|
||
|
|
@@ -125,9 +118,7 @@ _No images extracted._ | |
|
|
||
| ### dynamo-platform | ||
|
|
||
| - `nats:2.10.21-alpine` | ||
| - `natsio/nats-server-config-reloader:0.16.0` | ||
| - `nvcr.io/nvidia/ai-dynamo/kubernetes-operator:1.2.1` | ||
| - `nvcr.io/nvidia/ai-dynamo/kubernetes-operator:1.4.1` | ||
|
|
||
| ### gatekeeper | ||
|
|
||
|
|
@@ -298,7 +289,9 @@ _No images extracted._ | |
|
|
||
| ### prometheus-adapter | ||
|
|
||
| - `registry.k8s.io/prometheus-adapter/prometheus-adapter:v0.12.0` | ||
| > Warning: [INTERNAL] helm template failed: signal: killed | ||
|
|
||
| _No images extracted._ | ||
|
|
||
| ### prometheus-adapter-ocp | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MAJOR: The replacement networking contract is explicitly unfinished: the probe and security-group commands retain
<PORT>, AWS UAT still allows only the removed NATS port 4222, and current user guidance still describes a NATS-backed event plane. On supported EKS installs with separate GPU and system security groups, AICR can install while TCP requests or ZMQ KV events remain blocked, and current UAT will not detect it.Minimum correction: establish the exact 1.4.1 listener ports and traffic directions, replace every placeholder with actionable rules and probes, and synchronize UAT and user guidance to that tested TCP/ZMQ contract.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mchmarny I've pushed the 1.4.1 fix for the registry blocker.
For the networking doc - I don't have access to a live 1.4.1 EKS cluster to confirm the TCP request-plane port. The ZMQ KV-events port 5557 is confirmed from the smoke test manifest, but the request-plane port needs a live ss -tlnp run on the actual cluster. Could you or @yuanchen8911 share that output? Once I have the real port I can immediately replace , update the SG rules, fix the probe commands, and push the final doc.