From 821306eed6c8ed3d99706fcb7000bd80a081f398 Mon Sep 17 00:00:00 2001 From: Brian Lockwood Date: Wed, 23 Sep 2026 13:21:00 -0700 Subject: [PATCH] docs: require PRs to state what was not tested on a real node CI's e2e suites run on kind, whose nodes are containers sharing the runner's kernel. Nothing in CI performs a real reboot: the reboot interrupts under k8s-tests/chainsaw/ run the agentless image, and the real-agent suite uses only noop interrupts. Systemd shutdown ordering, kernel module and driver installs, GPU workloads, and real workload drains are equally out of reach. Neither AGENTS.md nor the PR template said so, and AGENTS.md described kind as "a real cluster". PRs touching those paths therefore listed unit and kind results and left reviewers unable to tell a claim that was verified on a node from one that was only reasoned about. Add a Testing section to the PR template with an explicit "Not exercised" line, a matching rule in AGENTS.md's Tests section, and a paragraph in CONTRIBUTING.md. Not having run on real hardware stays an acceptable answer; the requirement is only that it is stated. Signed-off-by: Brian Lockwood --- .claude/CLAUDE.md | 3 ++- .github/PULL_REQUEST_TEMPLATE.md | 9 +++++++++ CONTRIBUTING.md | 2 ++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 411962a5..69d0040c 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -94,7 +94,7 @@ make test # hatch test with coverage make build # hatch build → dist/ ``` -E2E tests use [chainsaw](https://kyverno.github.io/chainsaw/) against a real cluster, driven from `k8s-tests/chainsaw/{skyhook,cli,helm,deployment-policy}`. They require a kind cluster set up via `make create-kind-cluster` (or the 15-node variant for deployment-policy). `operator-agent-tests` additionally requires `AGENT_IMAGE=…` to be set. +E2E tests use [chainsaw](https://kyverno.github.io/chainsaw/) against a live kind cluster (a real apiserver and kubelet, but not real hardware: see *Tests* below), driven from `k8s-tests/chainsaw/{skyhook,cli,helm,deployment-policy}`. They require a kind cluster set up via `make create-kind-cluster` (or the 15-node variant for deployment-policy). `operator-agent-tests` additionally requires `AGENT_IMAGE=…` to be set. ## Architecture @@ -263,6 +263,7 @@ If you find yourself writing a long comment to explain a clever block, consider - Run a single describe with `ginkgo --focus "text"`. - Unit tests use **envtest** (a fake apiserver); e2e tests use **chainsaw** against a kind cluster. Don't mix the two — a test that needs real pods running belongs under `k8s-tests/chainsaw/`, not `internal/controller`. - For mocking, regenerate with `make generate-mocks` after editing an interface — hand-written mocks under `internal/controller/mock/` will drift. +- **Kind is not real hardware, so say what a PR did not exercise.** Kind nodes are containers sharing the runner's kernel. Neither envtest nor chainsaw can exercise a real reboot, systemd shutdown ordering, kernel module or driver installs, GPU workloads, or drains of long-running real workloads. The `reboot` interrupts under `k8s-tests/chainsaw/` run the `agentless` image, which never touches the host, and the real-agent suite (`k8s-tests/operator-agent/`) uses only `noop` interrupts. When a change touches one of those paths (interrupts, signal handling, cordon/drain, `on_host` step execution, or anything else that mutates the host), the PR's *Testing* section must say which of them ran on a real node (OS, plus hardware where it matters) and which rest only on unit tests or reasoning. "Not run on a real node" is an acceptable answer. Leaving it unsaid is not, because a reviewer cannot tell a verified claim from an argued one. ### Anti-patterns diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 7c897546..b97b1aa4 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -2,6 +2,15 @@ +## Testing + + +- [ ] Unit tests +- [ ] e2e on kind (CI or local) +- [ ] Real node: + +**Not exercised:** + ## Checklist - [ ] I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/nodewright/blob/main/CONTRIBUTING.md). diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6ad5d611..382815f1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -73,6 +73,8 @@ make license-header-check # the same gate CI runs `make test` in `operator/` is heavy — it runs four flavors of e2e and expects a running cluster (`make create-kind-cluster`). For iteration, `make unit-tests` is usually what you want; let CI run the rest. +CI's e2e suites run on kind, whose nodes are containers sharing the runner's kernel, so they cannot exercise a real reboot, systemd shutdown ordering, kernel module or driver installs, GPU workloads, or drains of real workloads. If your change touches interrupts, signal handling, cordon/drain, `on_host` step execution, or anything else that mutates the host, fill in the PR template's *Testing* section with what ran on a real node and what was not exercised at all. Saying a path was not run on real hardware is fine; the point is that reviewers can tell. + Prefer the Makefile over raw `go test` / `golangci-lint` invocations. The targets encode `-mod=vendor`, license-header formatting, envtest setup, and CRD/deepcopy generation ordering; calling the tools directly skips some of that and produces drift. Run `make help` to see what is available. ### Dependency updates