-
Notifications
You must be signed in to change notification settings - Fork 83
feat(nvca): extend control-plane validator with HA checks, DaemonSet n2n, and route CR type check #781
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?
feat(nvca): extend control-plane validator with HA checks, DaemonSet n2n, and route CR type check #781
Changes from all commits
c68325e
1b05986
cc062af
793bb32
21b85e8
eeb2c30
257ac5e
29180fb
603233d
8c5bafe
f810b22
8e332e6
812fdbb
ee8379d
a42d058
e2e6049
b9ac001
93e24c6
8b516d7
208ce1c
f861d3c
9decc75
2381281
88d34da
93e96db
1aab2ac
76bf31c
c2ec3ee
8ae4120
09b2714
97fc59c
bb19cef
0f7aa33
4d3772c
257f6eb
3409174
bfdd257
7f7e943
81d8d22
97c34a9
e006ed4
6968d31
6ead067
542cebe
b22f19c
7775574
1deb25a
1336433
4e01a0f
96e0e10
eb0223d
eb70aa5
4bfffb4
e73191d
7b1cabb
0b8d5bd
e59a849
804dae3
d09b9bd
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 |
|---|---|---|
|
|
@@ -30,6 +30,16 @@ spec: | |
| metadata: | ||
| labels: | ||
| {{- include "nvcaop.baseSelectorLabels" . | nindent 8 }} | ||
| {{- if (include "nvcaop.clusterValidatorEnabled" .) }} | ||
| {{- $cv := include "nvcaop.clusterValidatorConfig" . | fromYaml }} | ||
| {{- if eq (lower (trim (toString $cv.role))) "control-plane" }} | ||
| annotations: | ||
| # The operator starts the validator's first run when it starts, once | ||
| # per Job spec. A change to that spec restarts the operator, so an | ||
| # upgrade that changes only the validator's values runs it too. | ||
| checksum/cluster-validator-job: {{ include "nvcaop.clusterValidatorJobSpec" . | sha256sum }} | ||
| {{- end }} | ||
| {{- end }} | ||
| spec: | ||
| serviceAccountName: {{ include "nvcaop.serviceAccountName" . }} | ||
| automountServiceAccountToken: true | ||
|
|
@@ -95,6 +105,25 @@ spec: | |
| valueFrom: | ||
| fieldRef: | ||
| fieldPath: metadata.namespace | ||
| # Deliberately pinned to the compute-plane check set, and deliberately | ||
| # NOT wired to clusterValidator.role. This chart is only installed on | ||
| # compute-plane clusters, and a failed validation is fatal here, so | ||
| # running the control-plane set would gate operator startup on | ||
| # control-plane HA: a single OpenBao pod stuck Terminating, or the | ||
| # Gateway API CRDs simply being absent on a compute-only cluster, would | ||
| # leave the operator in Init:CrashLoopBackOff. | ||
| - name: VALIDATOR_ROLE | ||
| value: "compute-plane" | ||
| {{- /* Normalized like the validator's own parseRole, so a role of | ||
| "Control-Plane" still keeps this container off the summary. */}} | ||
| {{- if eq (lower (trim (toString $cv.role))) "control-plane" }} | ||
| # The CronJob owns the summary under the control-plane role, so this | ||
| # container must not republish a compute-plane summary over it: the GPU | ||
| # keys would reappear and the control-plane keys be pruned on every | ||
| # operator restart. Preflight mode keeps the checks and drops the write. | ||
| - name: VALIDATOR_PREFLIGHT | ||
| value: "true" | ||
| {{- end }} | ||
| resources: | ||
| requests: | ||
| cpu: {{ $cv.resources.requests.cpu | quote }} | ||
|
|
@@ -123,6 +152,20 @@ spec: | |
| fieldPath: metadata.namespace | ||
| - name: DEPLOYMENT_NAME | ||
| value: {{ include "nvcaop.fullname" . | quote }} | ||
| {{- if (include "nvcaop.clusterValidatorEnabled" .) }} | ||
| {{- $cv := include "nvcaop.clusterValidatorConfig" . | fromYaml }} | ||
| # The agent publishes the cluster-validator metrics baseline only | ||
| # where the validator runs. | ||
| - name: NVCA_CLUSTER_VALIDATOR_ENABLED | ||
| value: "true" | ||
| {{- if eq (lower (trim (toString $cv.role))) "control-plane" }} | ||
| # The init container publishes no summary under the control-plane | ||
| # role, so the operator runs the CronJob once at startup instead. It is | ||
| # not a release resource, so the install does not wait on it. | ||
| - name: NVCA_CLUSTER_VALIDATOR_CRONJOB | ||
| value: {{ printf "%s-cluster-validator" (include "nvcaop.fullname" .) | quote }} | ||
| {{- end }} | ||
|
Comment on lines
+161
to
+167
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. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Restart the operator when the validator Job spec changes. The initial run starts only when the operator pod starts. A values-only
📍 Affects 2 files
🤖 Prompt for AI Agents |
||
| {{- end }} | ||
| - name: NGC_API_URL | ||
| value: {{ .Values.ngcConfig.apiURL }} | ||
| - name: NGC_SERVICE_KEY_FILE | ||
|
|
||
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.
The compute-plane pin is right, and the GPU gate still works. Side effect: under
role=control-plane,PREFLIGHTmutes this container and nothing else runs at install, so no cluster-validator summary is published until the first CronJob tick, which can be up to 3 hours.On a fresh install with
enabled=trueandrole=control-plane(thesrc/.../deploymentscopy behaves the same), the agent baseline stays atnvca_cluster_validator_ready=0("NVCF-Not-Ready") andlast_run_timestamp=0until the next0 */3 * * *boundary, even on a healthy cluster. If the CronJob never writes (its hardcoded tolerations atcronjob.yaml:127-133, a pull failure or an RBAC failure),last_runstays 0, and the staleness alert (METRICS.md:1517-1522, which needslast_run > 0) never fires. METRICS.md:1405 still says the init container writes the summary.Consider a post-install Job (Helm hook) that runs the CronJob's pod spec once, or
startingDeadlineSecondsplus a first run at install.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.
Added a one-shot Job under the control-plane role. It runs the CronJob's own spec at each install and upgrade, named per release revision, and is not a Helm hook, so a Not-Ready verdict cannot fail the release. The two share one template helper, so they cannot drift. METRICS.md now describes this, and adds an alert for last_run == 0, since the staleness alert excludes a validator that never ran. clusterValidator.tolerations lets an operator add to the hardcoded tolerations. lint_helm.sh checks both chart copies (4e01a0f).