diff --git a/.github/workflows/operator-ci.yaml b/.github/workflows/operator-ci.yaml index 8132ca64..7353a02b 100644 --- a/.github/workflows/operator-ci.yaml +++ b/.github/workflows/operator-ci.yaml @@ -146,6 +146,16 @@ jobs: - k8s-version: ${{ needs.k8s-test-versions.outputs.ci-primary-kind-node-image-version }} test-suite: migration make-targets: "migration-test" + # Operator <-> agent contract. agent-ci runs this same suite when the AGENT + # changes, against a freshly built agent; nothing ran it when the OPERATOR + # changed, even though the operator is what builds the pod the agent runs in + # -- its args, mounts, copy dir and config.json. Every other suite here uses + # the agentless package image, which never reads that config, so a break in + # the contract could reach a release unnoticed. This row runs against the + # agent version the chart ships, which is what users pair the operator with. + - k8s-version: ${{ needs.k8s-test-versions.outputs.ci-primary-kind-node-image-version }} + test-suite: operator-agent + make-targets: "setup-kind-cluster operator-agent-tests" fail-fast: false # Continue testing other versions if one fails name: ${{ matrix.test-suite }}${{ matrix.pool && format('/{0}', matrix.pool) || '' }} (k8s-${{ matrix.k8s-version }}) steps: @@ -227,6 +237,25 @@ jobs: run: | cd operator make create-kind-cluster KIND_NODE_IMAGE_VERSION="$K8S_VERSION" + # operator-agent-tests refuses to run without an explicit AGENT_IMAGE, because the + # Makefile's global default is the agentless image -- which would pass every case + # while executing nothing. Read the pin out of the chart rather than hardcoding a + # version here, so bumping the agent in one place cannot leave this row testing an + # older one. + - name: Resolve the chart's pinned agent image + if: matrix.test-suite == 'operator-agent' + run: | + ## operator/bin/yq, not the runner's: install-deps pins the version and the + ## binaries are already restored from cache by this point in the job. + YQ=operator/bin/yq + repo=$("$YQ" -r '.controllerManager.manager.agent.repository' chart/values.yaml) + tag=$("$YQ" -r '.controllerManager.manager.agent.tag' chart/values.yaml) + if [ -z "$repo" ] || [ -z "$tag" ] || [ "$repo" = "null" ] || [ "$tag" = "null" ]; then + echo "could not read the agent image from chart/values.yaml" + exit 1 + fi + echo "AGENT_IMAGE=${repo}:${tag}" >> "$GITHUB_ENV" + echo "operator-agent tests will use ${repo}:${tag}" # Run test suite - name: Run ${{ matrix.test-suite }} tests env: diff --git a/docs/contributing/ci-test-pools.md b/docs/contributing/ci-test-pools.md index aa47b014..a4f30f9d 100644 --- a/docs/contributing/ci-test-pools.md +++ b/docs/contributing/ci-test-pools.md @@ -48,6 +48,21 @@ git error when the tag is missing. `helm-upgrade-rollback-test` pulls the publis chart from the registry instead, and only reads git tags when asked to resolve the newest release rather than a pinned one. +### `operator-agent` + +Runs `k8s-tests/operator-agent/` — the only suite that exercises the **real agent** rather than +the `agentless` package image, so the only one that proves a package's scripts run on the host. + +`agent-ci.yaml` already runs it when the *agent* changes, against a freshly built agent. This row +covers the other direction: the operator is what builds the pod the agent runs in — its args, +mounts, copy dir and `config.json` — and until this row existed, an operator change could break +that contract without any suite noticing. + +It resolves `AGENT_IMAGE` from `chart/values.yaml` rather than pinning a version in the workflow, +so bumping the agent in one place cannot leave this row testing an older one. The suite refuses to +run without an explicit `AGENT_IMAGE`, because `operator/Makefile`'s global default is the +`agentless` image, which would pass every case while executing nothing. + ### `migration` Runs `k8s-tests/migration/run.sh`: installs the last pre-rename release diff --git a/docs/contributing/release-process.md b/docs/contributing/release-process.md index 9ddbb4e6..8d93aace 100644 --- a/docs/contributing/release-process.md +++ b/docs/contributing/release-process.md @@ -65,8 +65,9 @@ git tag chart/v0.16.0-rc.1 # Tag agent only if it changed since the last released agent version. git push origin operator/v0.16.0-rc.1 chart/v0.16.0-rc.1 -# 5. Validate the RC. If issues are found, cherry-pick more fixes from main, -# bump Chart.yaml to v0.16.0-rc.2, and tag -rc.2. Repeat until clean. +# 5. Validate the RC against docs/contributing/release-validation.md. If issues are found, +# cherry-pick more fixes from main, bump Chart.yaml to the next RC (-rc.2, then -rc.3, ...), +# and tag it in full: operator/v0.16.0-rc.N and chart/v0.16.0-rc.N. Repeat until clean. # 6. Cut the final release on the same commit as the last good RC. # Bump Chart.yaml to v0.16.0 (drop the -rc.N suffix) and commit. @@ -280,7 +281,7 @@ Where possible, place a release tag on a commit that touches the component's own **Before the final release tag:** -- [ ] The last RC validated successfully +- [ ] The last RC validated successfully — [`release-validation.md`](release-validation.md), with the sign-off table filled in - [ ] `chart/Chart.yaml` bumped to the non-RC version on the same commit - [ ] No new commits between the validated RC and the release tag other than the `Chart.yaml` bump diff --git a/docs/contributing/release-validation.md b/docs/contributing/release-validation.md new file mode 100644 index 00000000..9b71919b --- /dev/null +++ b/docs/contributing/release-validation.md @@ -0,0 +1,149 @@ +# Release Validation + +The short manual pass run against a release candidate, covering what the automated suites do not. It +is what ["Validate the RC"](release-process.md) means, and what the release checklist's *"The last RC +validated successfully"* box is signed against. + +## Run the automated suites first + +Most of the product is already covered, and re-doing it by hand is wasted time. Before anything below: + +```bash +cd operator +make test # unit + e2e pools + cli-e2e + helm + operator-agent +make create-kind-cluster # must precede the next one; it recreates the local registry +make migration-test # upgrade from the last pre-rename operator +``` + +CI runs the same set on every PR (`e2e` × core/interrupt/uninstall/lifecycle, `deployment-policy`, +`cli-e2e`, `unit-tests`, `helm-tests`, `migration`), so on a clean RC these should all be green +already. What they cover, so you do not re-test it: + +| Behaviour | Where | +|---|---| +| Job shape, one Job per (package, stage), retention | `nodewright/simple-nodewright`, `hello-world` | +| `dependsOn` ordering; strict ordering across CRs | `nodewright/depends-on`, `strict-order` | +| Interrupt grouping, cordon/drain, budgets | `nodewright/interrupt-grouping`, `interrupt`, `drain-config` | +| Upgrade and downgrade paths | `nodewright/package-upgrade`, `downgrade-after-uninstall`, `downgrade-enabled-false-preserves-state` | +| Uninstall in all its shapes | `nodewright/explicit-uninstall`, `uninstall-on-delete`, `uninstall-mixed-packages`, `uninstall-cancel` | +| Failure, retry budget, timed-out Job shape | `nodewright/failure-nodewright` | +| A stage timeout firing on the clock | `nodewright/stage-timeout` | +| Attempts the kubelet refuses never marking a package `erroring` | `nodewright/kubelet-refused` | +| An evicted attempt costing no retry budget | `nodewright/disruption-casualty` | +| Pause suspending a running Job, and disable not resuming it | `nodewright/pause-suspends-jobs` | +| Config change while a stage runs | `nodewright/config-nodewright` | +| Package scripts running **on the host** via the real agent | `k8s-tests/operator-agent/`, now also run on operator changes | +| **Upgrade from the previous release without re-running packages** | `make migration-test` | +| Migration hold on an in-flight rollout | `k8s-tests/migration/skyhook-hold.yaml` | +| `helm upgrade` then `helm rollback` across the rename | `helm/helm-upgrade-rollback-test` | + +## What is left, and why it is manual + +The four cases below are what is left after automating everything that could be automated. Two are +uncovered outright; two are covered only as a *shape* where the thing worth checking is *behaviour +over time*. An hour or two, not a day. + +### Setup + +```bash +make install && make build-cli +../k8s-tests/operator-agent/setup.sh setup # debugger pod, for case 2 only +make kill +make run > /tmp/nw-operator.log 2>&1 & # detached: make run inherits your stderr +``` + +`AGENT_IMAGE` must match the package fixture, and the mismatch is silent in one direction: + +| Fixture | `AGENT_IMAGE` | +|---|---| +| `skyhook/agentless` (cases 1, 3, 4) | `ghcr.io/nvidia/skyhook/agentless:6.2.0` — the `operator/Makefile` default | +| `skyhook-packages/shellscript` (case 2) | `ghcr.io/nvidia/nodewright/agent:` — the real agent | + +Real agent + an `agentless` package fails loudly on a missing `config.json`. `agentless` + a +`shellscript` package **passes having executed nothing** — that is the one that invalidates case 2. +Confirm before asserting: `grep -o '"AgentImage":"[^"]*"' /tmp/nw-operator.log`. + +Reset between cases: delete the CR **and** clear the per-CR node annotations, the mirrored +`status_` label, any cordon or runtime-required taint, the namespace's Jobs/pods/ConfigMaps, and +(case 2) `/var/lib/skyhook/`, `/var/log/skyhook/` and the marker file. Assert a clean baseline +before starting the next one — most confusing results are the previous case's residue. + +Fixture notes: the package `version` **is** the image tag (a made-up version gets `ErrImagePull`, not +a run), and `SLEEP_LEN` is per *step*, so a stage takes roughly twice the value. + +--- + +## 1. A node disappearing mid-run + +**Not covered.** (`nodewright/cleanup-pods` sweeps orphaned *pods* after a state reset, which is a +different path.) *Destructive — run it last; recovering the node means rebuilding the cluster.* + +Delete a node while its package Job is running. Expect the Job foreground-deleted within a reconcile +or two, with an operator log line naming the reason; no replacement created; other nodes unaffected; +and the CR still reaching `complete`. + +## 2. Re-execution semantics of a real package + +**Not covered** — and the one to run before shipping any package. Needs the real agent and a +`shellscript` package whose `apply.sh` appends a line to a host file, with an `apply_check.sh` that +fails. + +Expect the script to run **once per attempt** — four executions and four lines under the default +`backoffLimit: 3`. The agent writes a completion flag but honours it only when the package declares +`Idempotence.Auto`; `shellscript` declares `Disabled`, and the agent logs +`Flag exists but idempotence is Idempotence.Disabled so running step.` Confirm the same on a +`kubectl nodewright package rerun`. **Under Jobs, a non-idempotent step is multiplied by the retry +budget** — this is the fact package authors most need to know, and nothing else demonstrates it. + +## 3. Retention actually collects, by outcome + +`nodewright/failure-nodewright` asserts the TTL **value** on a timed-out Job. What it cannot show is +the cluster acting on it. Run one succeeding and one failing package with +`JOB_TTL_SUCCEEDED=1m JOB_TTL_FAILED=3m` (or `controllerManager.manager.env.{jobTtlSucceeded,jobTtlFailed}` +on a chart install; one minute is a hard floor the operator refuses to start below). + +Expect the TTL absent until a Job finishes then set from the outcome; the succeeded Jobs **and their +pods** collected first while the failed one remains; **node state unchanged by collection**; and after +the failed Job is collected, a fresh attempt appearing — the slow-retry cadence, easily mistaken for +churn. + +## 4. Node state survives a rollback + +`helm/helm-upgrade-rollback-test` proves the rollback *mechanism* — CRDs, webhooks and resource names +survive. It does not drive a package first, so it cannot speak to state. + +Install the previous release, drive a CR to `complete`, snapshot the node's `nodewright.nvidia.com/*` +annotations, upgrade to the RC, then `helm rollback`. Expect the previous operator to come up Ready, +the CR to still read `complete`, **no package to re-run**, and the annotations to be byte-identical to +the snapshot. Note `helm rollback` with no revision targets the previous *deployed* revision — after a +failed attempt that is not the baseline, so pass the revision explicitly. + +--- + +## Known limitations to expect + +Confirm rather than re-file; a change in either is itself a signal. + +- A **successful** `uninstall` Job and its pod are deleted immediately + ([#443](https://github.com/NVIDIA/nodewright/issues/443)), so its output is unrecoverable through + Kubernetes. The agent's log on the node survives. +- An unpullable image reports `ContainerStatusUnknown` on the archived attempt rather than + `ImagePullBackOff` ([#306](https://github.com/NVIDIA/nodewright/issues/306)). + +## Sign-off + +Record the build under test (`git_sha` from the operator's startup log, plus chart and agent +versions), pass/fail per case, and the evidence path. A failure blocks the release unless it is a +known limitation above, or is explicitly accepted and filed. + +| # | Case | Operator settings | Result | +|---|---|---|---| +| 1 | node disappears mid-run (destructive, last) | defaults | | +| 2 | re-execution semantics | **real agent** | | +| 3 | retention collects by outcome | `JOB_TTL_SUCCEEDED=1m JOB_TTL_FAILED=3m` | | +| 4 | node state survives a rollback | chart install, previous release | | + +### Not covered anywhere + +Real reboots (kind nodes are containers — use `service`/`noop` interrupts), fleet scale, and image +provenance. Webhook rejection paths need a chart install; `make run` defaults `ENABLE_WEBHOOKS=false`. diff --git a/k8s-tests/chainsaw/nodewright/disruption-casualty/README.md b/k8s-tests/chainsaw/nodewright/disruption-casualty/README.md new file mode 100644 index 00000000..014d4a4f --- /dev/null +++ b/k8s-tests/chainsaw/nodewright/disruption-casualty/README.md @@ -0,0 +1,18 @@ +# disruption-casualty + +Validates that an attempt lost to a disruption costs no retry budget and is not recorded as a package failure. + +## Why this needs a real cluster + +The Job's `podFailurePolicy` ignores failures carrying the `DisruptionTarget` condition, so a drain, a preemption or a node scale-down replaces the attempt for free. Only the **eviction API** sets that condition — a plain `kubectl delete pod` does not — and only a real apiserver implements the eviction subresource. There is nothing to assert against a fake client. + +Without the rule, routine cluster maintenance walks healthy packages into `erroring` by exhausting budgets they never spent on a real failure. + +## Test Scenario + +1. Install a package with a long-running apply stage and wait for an attempt to be genuinely executing +2. Evict its pod through `POST /api/v1/namespaces//pods//eviction` +3. Poll asserting `status.failed` stays empty and node state never becomes `erroring` +4. Assert a replacement attempt exists, so step 3 did not pass by there being no pods at all + +Note the eviction has to go through `kubectl create --raw`; `kubectl create -f` on an `Eviction` object fails with `no matches for kind "Eviction" in version "policy/v1"`, because eviction is a subresource rather than a namespaced object. diff --git a/k8s-tests/chainsaw/nodewright/disruption-casualty/chainsaw-test.yaml b/k8s-tests/chainsaw/nodewright/disruption-casualty/chainsaw-test.yaml new file mode 100644 index 00000000..d1e4f23e --- /dev/null +++ b/k8s-tests/chainsaw/nodewright/disruption-casualty/chainsaw-test.yaml @@ -0,0 +1,135 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# +# 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. + +# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: disruption-casualty + labels: + pool: core +spec: + timeouts: + assert: 240s + catch: + - get: + apiVersion: nodewright.nvidia.com/v1alpha1 + kind: NodeWright + name: disruption-casualty + format: yaml + - script: + content: | + kubectl get jobs -n nodewright -l nodewright.nvidia.com/name=disruption-casualty -o yaml || true + kubectl get pods -n nodewright -l nodewright.nvidia.com/name=disruption-casualty -o wide || true + steps: + - name: evict-a-running-attempt + description: > + The Job's podFailurePolicy ignores failures carrying DisruptionTarget, so a node + drain or a preemption costs no retry. Only the eviction API sets that condition -- + a plain delete does not -- so this has to go through the subresource. + try: + - script: + content: | + ../nodewright-cli reset disruption-casualty --confirm 2>/dev/null || true + - create: + file: nodewright.yaml + ## Wait for a genuinely executing attempt. Package work runs in init containers, so the + ## pod stays Pending for the whole stage; a running init container is the signal, not + ## status.phase. Same reasoning as pause-suspends-jobs. + - script: + timeout: 240s + content: | + for i in $(seq 1 80); do + running=$(kubectl get pods -n nodewright \ + -l nodewright.nvidia.com/name=disruption-casualty,nodewright.nvidia.com/stage=apply \ + -o jsonpath='{.items[*].status.initContainerStatuses[*].state.running.startedAt}' 2>/dev/null) + if [ -n "$running" ]; then + echo "apply stage is executing (startedAt: $running)" + exit 0 + fi + sleep 3 + done + echo "no apply pod reached a running init container within timeout" + exit 1 + - script: + ## kubectl create -f on an Eviction fails with "no matches for kind Eviction in + ## version policy/v1" -- eviction is a subresource, not a namespaced object, so it + ## has to be POSTed to the pod's /eviction path with --raw. + content: | + set -e + POD=$(kubectl get pods -n nodewright \ + -l nodewright.nvidia.com/name=disruption-casualty,nodewright.nvidia.com/stage=apply \ + --no-headers | grep -v Completed | head -1 | awk '{print $1}') + echo "evicting $POD" + ## Steps get their own shell, so hand the name over on disk for the next one. + echo "$POD" > /tmp/disruption-casualty-pod + cat > /tmp/disruption-casualty-eviction.json < + status.failed staying empty is the whole claim: an evicted attempt is replaced + without being counted, so a drain cannot walk a healthy package into erroring by + exhausting its budget. + try: + - script: + timeout: 180s + content: | + ## Stops as soon as a *different* attempt pod exists: that is the moment the claim is + ## proven (the stage carried on and the budget was untouched), and holding longer only + ## costs the pool wall-clock. + evicted=$(cat /tmp/disruption-casualty-pod) + for i in $(seq 1 30); do + failed=$(kubectl get jobs -n nodewright \ + -l nodewright.nvidia.com/name=disruption-casualty,nodewright.nvidia.com/stage=apply \ + -o jsonpath='{.items[*].status.failed}') + state=$(kubectl get nodes -l nodewright.nvidia.com/test-node=skyhooke2e \ + -o jsonpath='{.items[*].metadata.annotations.nodewright\.nvidia\.com/nodeState_disruption-casualty}') + if [ -n "$failed" ] && [ "$failed" != "0" ]; then + echo "FAIL: the eviction spent a retry (status.failed=$failed); podFailurePolicy did not ignore DisruptionTarget" + exit 1 + fi + if echo "$state" | grep -q "erroring"; then + echo "FAIL: the evicted attempt was recorded as a package failure" + echo "state: $state" + exit 1 + fi + current=$(kubectl get pods -n nodewright \ + -l nodewright.nvidia.com/name=disruption-casualty,nodewright.nvidia.com/stage=apply \ + --no-headers 2>/dev/null | grep -v Completed | awk '{print $1}' | grep -v "^${evicted}$" | head -1) + echo "check ${i}: status.failed=${failed:-empty} replacement=${current:-none}" + if [ -n "$current" ]; then + echo "OK: replacement ${current} running, budget untouched, never erroring" + exit 0 + fi + sleep 5 + done + echo "FAIL: no replacement attempt appeared after the eviction" + exit 1 + - assert: + ## A replacement attempt exists, so the stage genuinely carried on rather than + ## passing the previous check by having no pods at all. + resource: + apiVersion: v1 + kind: Pod + metadata: + namespace: nodewright + labels: + nodewright.nvidia.com/name: disruption-casualty + nodewright.nvidia.com/stage: apply diff --git a/k8s-tests/chainsaw/nodewright/disruption-casualty/nodewright.yaml b/k8s-tests/chainsaw/nodewright/disruption-casualty/nodewright.yaml new file mode 100644 index 00000000..c8df8424 --- /dev/null +++ b/k8s-tests/chainsaw/nodewright/disruption-casualty/nodewright.yaml @@ -0,0 +1,36 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# +# 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. + +apiVersion: nodewright.nvidia.com/v1alpha1 +kind: NodeWright +metadata: + name: disruption-casualty +spec: + nodeSelectors: + matchLabels: + nodewright.nvidia.com/test-node: skyhooke2e + interruptionBudget: + count: 1 + packages: + ## Long enough that the attempt is still executing when it is evicted, and long enough + ## again for its replacement to be observable before the stage finishes. + bystander: + ## version is the agentless image tag; only published tags work here. + version: "1.2.3" + image: ghcr.io/nvidia/skyhook/agentless + env: + - name: SLEEP_LEN + value: "300" diff --git a/k8s-tests/chainsaw/nodewright/kubelet-refused/README.md b/k8s-tests/chainsaw/nodewright/kubelet-refused/README.md new file mode 100644 index 00000000..e9b90095 --- /dev/null +++ b/k8s-tests/chainsaw/nodewright/kubelet-refused/README.md @@ -0,0 +1,17 @@ +# kubelet-refused + +Validates that attempts the kubelet refuses to admit are not recorded as package failures. + +## Why this needs a real cluster + +Package pods carry `spec.nodeName`, so they bypass the scheduler entirely and kubelet admission is the only gate. A pod rejected there is `Failed` with **no container statuses at all** — it never ran a line of the package's script. That shape only exists on a real kubelet; a fake client has no admission step to fail. + +This is the distinction `jobFailureIsGenuine` is built around. Getting it wrong parks a stage as `erroring` and points an operator at a package, when the thing that needs attention is the node's capacity. + +## Test Scenario + +1. Install a package requesting more CPU and memory than any node has, so every attempt is refused (resource overrides are all-four-fields-or-none — see [resource-management.md](../../../../docs/operations/resource-management.md)) +2. Assert an attempt pod is `Failed` with reason `OutOfcpu` +3. Poll across the whole retry budget asserting node state **never** becomes `erroring` + +Step 3 polls rather than asserting once: the claim is that the state never flips during any attempt, not that it happens to be right at one instant. diff --git a/k8s-tests/chainsaw/nodewright/kubelet-refused/chainsaw-test.yaml b/k8s-tests/chainsaw/nodewright/kubelet-refused/chainsaw-test.yaml new file mode 100644 index 00000000..437edd69 --- /dev/null +++ b/k8s-tests/chainsaw/nodewright/kubelet-refused/chainsaw-test.yaml @@ -0,0 +1,118 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# +# 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. + +# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: kubelet-refused + labels: + pool: core +spec: + timeouts: + assert: 300s + catch: + - get: + apiVersion: nodewright.nvidia.com/v1alpha1 + kind: NodeWright + name: kubelet-refused + format: yaml + - script: + content: | + kubectl get jobs -n nodewright -l nodewright.nvidia.com/name=kubelet-refused -o yaml || true + kubectl get pods -n nodewright -l nodewright.nvidia.com/name=kubelet-refused -o wide || true + kubectl get nodes -l nodewright.nvidia.com/test-node=skyhooke2e -o yaml || true + steps: + - name: refused-attempts-spend-the-budget + description: > + An attempt the kubelet refuses never starts a container, so it produces no evidence + that the package failed -- only that it could not be placed. It still costs a retry, + which is what makes the next assertion the interesting one. + try: + - script: + content: | + ../nodewright-cli reset kubelet-refused --confirm 2>/dev/null || true + - create: + file: nodewright.yaml + - assert: + ## OutOfcpu is set by the kubelet at admission. The pod is Failed with no container + ## statuses at all, which is precisely the shape the "genuine failure" predicate has + ## to distinguish from a package that ran and exited non-zero. + resource: + apiVersion: v1 + kind: Pod + metadata: + namespace: nodewright + labels: + nodewright.nvidia.com/name: kubelet-refused + nodewright.nvidia.com/package: greedy-1.2.3 + status: + phase: Failed + reason: OutOfcpu + + - name: a-package-that-never-ran-is-not-erroring + description: > + The assertion this test exists for. Blaming the package here would park a stage that + has not executed a single line of its script, and the node -- not the package -- is + what needs attention. + try: + - script: + ## Poll rather than assert once: the budget is spent over several attempts, and the + ## claim is that the state never flips during any of them, not that it happens to be + ## right at one instant. + timeout: 240s + content: | + ## Track the highest status.failed seen rather than waiting for a fixed number. + ## The operator sweeps a terminal Job whose attempts were all refused and recreates + ## the stage, which resets the counter -- that self-heal is correct behaviour, so an + ## exit condition of "failed reaches backoffLimit+1" is a race the sweep can win. + ## Several refused attempts plus a state that never flips is the actual claim. + max=0 + for i in $(seq 1 30); do + state=$(kubectl get nodes -l nodewright.nvidia.com/test-node=skyhooke2e \ + -o jsonpath='{.items[*].metadata.annotations.nodewright\.nvidia\.com/nodeState_kubelet-refused}') + if echo "$state" | grep -q "erroring"; then + echo "FAIL: package marked erroring by attempts the kubelet refused" + echo "state: $state" + exit 1 + fi + failed=$(kubectl get jobs -n nodewright \ + -l nodewright.nvidia.com/name=kubelet-refused,nodewright.nvidia.com/stage=apply \ + -o jsonpath='{.items[*].status.failed}') + failed=${failed:-0} + [ "$failed" -gt "$max" ] 2>/dev/null && max=$failed + echo "check ${i}: failed=${failed} max=${max} state=${state:-none}" + if [ "$max" -ge 2 ] 2>/dev/null; then + echo "OK: ${max} refused attempts observed and the package never went erroring" + exit 0 + fi + sleep 5 + done + echo "FAIL: never observed repeated refusals; the fixture may no longer exceed node capacity" + exit 1 + fi + failed=$(kubectl get jobs -n nodewright \ + -l nodewright.nvidia.com/name=kubelet-refused,nodewright.nvidia.com/stage=apply \ + -o jsonpath='{.items[*].status.failed}') + echo "check ${i}: failed=${failed:-0} state=${state:-none}" + if [ "${failed:-0}" -ge 4 ] 2>/dev/null; then + echo "OK: budget spent (failed=$failed) and never erroring" + exit 0 + fi + sleep 5 + done + echo "FAIL: the retry budget was never spent; the fixture may no longer be refused" + exit 1 diff --git a/k8s-tests/chainsaw/nodewright/kubelet-refused/nodewright.yaml b/k8s-tests/chainsaw/nodewright/kubelet-refused/nodewright.yaml new file mode 100644 index 00000000..867924a4 --- /dev/null +++ b/k8s-tests/chainsaw/nodewright/kubelet-refused/nodewright.yaml @@ -0,0 +1,45 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# +# 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. + +apiVersion: nodewright.nvidia.com/v1alpha1 +kind: NodeWright +metadata: + name: kubelet-refused +spec: + nodeSelectors: + matchLabels: + nodewright.nvidia.com/test-node: skyhooke2e + interruptionBudget: + count: 1 + packages: + ## Package pods carry spec.nodeName, so they bypass the scheduler and kubelet admission + ## is the only gate. Asking for more CPU than any node has makes every attempt be + ## rejected before a container starts -- deterministic on any cluster, unlike trying to + ## provoke a real eviction. + ## + ## Resource overrides are all-four-fields-or-none (docs/operations/resource-management.md). + greedy: + ## version is the agentless image tag; only published tags work here. + version: "1.2.3" + image: ghcr.io/nvidia/skyhook/agentless + env: + - name: SLEEP_LEN + value: "5" + resources: + cpuRequest: "500" + cpuLimit: "500" + memoryRequest: "800Gi" + memoryLimit: "800Gi" diff --git a/k8s-tests/chainsaw/nodewright/pause-suspends-jobs/chainsaw-test.yaml b/k8s-tests/chainsaw/nodewright/pause-suspends-jobs/chainsaw-test.yaml index 8a2250fb..6c37ea88 100644 --- a/k8s-tests/chainsaw/nodewright/pause-suspends-jobs/chainsaw-test.yaml +++ b/k8s-tests/chainsaw/nodewright/pause-suspends-jobs/chainsaw-test.yaml @@ -159,3 +159,61 @@ spec: nodewright.nvidia.com/name: pause-suspends-jobs nodewright.nvidia.com/package: slowpoke-1.2.3 nodewright.nvidia.com/stage: apply + + - name: disable-does-not-resume-a-paused-stage + description: > + Pause and disable are separate annotations that both stop work, and a user moving + from one to the other writes both in a single edit. If clearing pause is handled + without consulting disable, that edit resumes the very stage the user was trying to + keep stopped -- a regression this reproduces directly, since the Job stays suspended + only if disable is considered too. + try: + - script: + content: | + kubectl annotate nodewright pause-suspends-jobs \ + nodewright.nvidia.com/pause=true --overwrite + - assert: + resource: + apiVersion: batch/v1 + kind: Job + metadata: + namespace: nodewright + labels: + nodewright.nvidia.com/name: pause-suspends-jobs + nodewright.nvidia.com/package: slowpoke-1.2.3 + nodewright.nvidia.com/stage: apply + spec: + suspend: true + - script: + ## Both writes in one command: this is the transition under test, and doing it as + ## two edits would let a reconcile land in between and resume before disable is set. + content: | + kubectl annotate nodewright pause-suspends-jobs \ + nodewright.nvidia.com/disable=true nodewright.nvidia.com/pause- --overwrite + - script: + ## Hold rather than assert once. The failure mode is a resume that happens on the + ## next reconcile, so a single check immediately after the edit passes even when the + ## behaviour is wrong; the Job has to still be suspended a while later. + timeout: 120s + content: | + ## Eight checks over 40s. A wrongly-handled clear resumes on the next reconcile, + ## which is sub-second here, so this is generous without paying for it in the pool. + for i in $(seq 1 8); do + suspend=$(kubectl get jobs -n nodewright \ + -l nodewright.nvidia.com/name=pause-suspends-jobs,nodewright.nvidia.com/stage=apply \ + -o jsonpath='{.items[*].spec.suspend}') + pods=$(kubectl get pods -n nodewright \ + -l nodewright.nvidia.com/name=pause-suspends-jobs,nodewright.nvidia.com/stage=apply \ + --no-headers 2>/dev/null | wc -l | tr -d ' ') + echo "check ${i}: suspend=${suspend:-none} pods=${pods}" + if [ "$suspend" != "true" ]; then + echo "FAIL: removing pause resumed the Job while the NodeWright is disabled" + exit 1 + fi + sleep 5 + done + echo "OK: the Job stayed suspended under disable" + - script: + ## Leave the object enabled so the reset in any later run starts from a normal state. + content: | + kubectl annotate nodewright pause-suspends-jobs nodewright.nvidia.com/disable- --overwrite diff --git a/k8s-tests/chainsaw/nodewright/stage-timeout/README.md b/k8s-tests/chainsaw/nodewright/stage-timeout/README.md new file mode 100644 index 00000000..12e295c7 --- /dev/null +++ b/k8s-tests/chainsaw/nodewright/stage-timeout/README.md @@ -0,0 +1,18 @@ +# stage-timeout + +Validates that a package's `stageTimeout` is enforced per attempt, and that no deadline caps the stage as a whole. + +## Why this needs a real cluster + +Unit tests cover the arithmetic — that `stageTimeout` lands on the pod template and that `0` omits the field. Nothing in a fake client runs the clock. The claim that matters to a user is that a stage which hangs is *killed*, and only the kubelet does that. + +The Job-level `activeDeadlineSeconds` was removed deliberately (#402): a whole-stage ceiling interacts badly with the retry budget, because a stage that has retried is closer to its ceiling through no fault of the attempt currently running. Its absence is asserted here so a future change cannot quietly reintroduce it. + +## Test Scenario + +1. Install a package with `stageTimeout: 20s` whose step sleeps for 600s +2. Assert the Job's **pod template** carries `activeDeadlineSeconds: 20` +3. Assert the **Job** carries no `activeDeadlineSeconds` of its own +4. Assert the attempt pod ends `Failed` with reason `DeadlineExceeded` + +The gap between 20s and 600s is the assertion: a pod reporting `DeadlineExceeded` cannot have run to completion, so the test proves the deadline fired without measuring wall-clock. diff --git a/k8s-tests/chainsaw/nodewright/stage-timeout/chainsaw-test.yaml b/k8s-tests/chainsaw/nodewright/stage-timeout/chainsaw-test.yaml new file mode 100644 index 00000000..2caff430 --- /dev/null +++ b/k8s-tests/chainsaw/nodewright/stage-timeout/chainsaw-test.yaml @@ -0,0 +1,98 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# +# 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. + +# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: stage-timeout + labels: + pool: core +spec: + timeouts: + assert: 240s + catch: + - get: + apiVersion: nodewright.nvidia.com/v1alpha1 + kind: NodeWright + name: stage-timeout + format: yaml + - script: + content: | + kubectl get jobs -n nodewright -l nodewright.nvidia.com/name=stage-timeout -o yaml || true + kubectl get pods -n nodewright -l nodewright.nvidia.com/name=stage-timeout -o wide || true + steps: + - name: the-deadline-is-per-attempt + description: > + A package stage carries its timeout on the pod template, not the Job. The Job-level + ceiling was removed so the retry budget is what bounds a stage overall; a unit test + can assert the field is written, but only a cluster runs the kubelet clock that acts + on it. + try: + - script: + content: | + ../nodewright-cli reset stage-timeout --confirm 2>/dev/null || true + - create: + file: nodewright.yaml + - assert: + resource: + apiVersion: batch/v1 + kind: Job + metadata: + namespace: nodewright + labels: + nodewright.nvidia.com/name: stage-timeout + nodewright.nvidia.com/package: laggard-1.2.3 + nodewright.nvidia.com/stage: apply + spec: + template: + spec: + ## From the package's stageTimeout. Per attempt: each retry gets its own + ## twenty seconds rather than sharing one budget across the stage. + activeDeadlineSeconds: 20 + - script: + ## A Job-level deadline would cap the whole stage including its retries, which is + ## what #402 removed. jsonpath prints nothing for an absent field, so empty is the + ## assertion; chainsaw cannot express "this field must not exist". + content: | + dl=$(kubectl get jobs -n nodewright \ + -l nodewright.nvidia.com/name=stage-timeout,nodewright.nvidia.com/stage=apply \ + -o jsonpath='{.items[*].spec.activeDeadlineSeconds}') + if [ -n "$dl" ]; then + echo "FAIL: Job carries a whole-stage deadline of ${dl}s; the ceiling should be per-attempt only" + exit 1 + fi + echo "OK: no Job-level activeDeadlineSeconds" + + - name: the-attempt-is-killed-on-the-clock + description: > + The step sleeps for 600s and the stage is allowed 20s, so a pod that reports + DeadlineExceeded can only have been killed by the deadline. It fails in place rather + than being deleted, which is what keeps its logs readable afterwards. + try: + - assert: + resource: + apiVersion: v1 + kind: Pod + metadata: + namespace: nodewright + labels: + nodewright.nvidia.com/name: stage-timeout + nodewright.nvidia.com/package: laggard-1.2.3 + nodewright.nvidia.com/stage: apply + status: + phase: Failed + reason: DeadlineExceeded diff --git a/k8s-tests/chainsaw/nodewright/stage-timeout/nodewright.yaml b/k8s-tests/chainsaw/nodewright/stage-timeout/nodewright.yaml new file mode 100644 index 00000000..327c66ea --- /dev/null +++ b/k8s-tests/chainsaw/nodewright/stage-timeout/nodewright.yaml @@ -0,0 +1,38 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# +# 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. + +apiVersion: nodewright.nvidia.com/v1alpha1 +kind: NodeWright +metadata: + name: stage-timeout +spec: + nodeSelectors: + matchLabels: + nodewright.nvidia.com/test-node: skyhooke2e + interruptionBudget: + count: 1 + packages: + ## The whole test is the gap between these two numbers: the step asks to run for ten + ## minutes, the stage is allowed twenty seconds. Anything that reports DeadlineExceeded + ## here proves the deadline fired, without the test having to measure wall-clock. + laggard: + ## version is the agentless image tag; only published tags work here. + version: "1.2.3" + image: ghcr.io/nvidia/skyhook/agentless + stageTimeout: 20s + env: + - name: SLEEP_LEN + value: "600"