From d59365b9280ff2d3aca753e39dacc69860c185fa Mon Sep 17 00:00:00 2001 From: Riley Rice Date: Wed, 30 Sep 2026 13:38:17 -0700 Subject: [PATCH 1/3] docs(release): give the agent its own release/agent/vX.Y.x branch family (#708) Release branches were operator-centric: release/vX.Y.x carried the operator, the chart and whatever agent version the chart pinned, and an agent change reached users as a chart patch on that line. That fit while agent releases were small fixes. It does not fit agent/v7.0.0, the Go rewrite, which ships between operator minors: tagging it on release/vX.Y.x would have forced either an early operator minor or a language rewrite dressed as a chart patch. The agent now has its own family, release/agent/vX.Y.x, cut from main when an agent minor or major is ready. All agent/vX.Y.* tags (RCs, finals, patches) are cut there; operator and chart tags stay on release/vX.Y.x. The two families meet at the chart: a new agent version reaches users through a chart patch that bumps the agent pin in chart/values.yaml, cherry-picked to the active operator/chart line. release-process.md gets the branch table, the rationale and a worked agent release, replacing the "Agent-Only Changes" recipe that tagged the agent on the operator line; versioning.md's branching section describes both families. gen-changelog.sh sources a patch section from the line's release branch, and it assumed release/vX.Y.x for every component, so cutting an agent patch would have looked for a branch that does not exist and skipped the RELEASE_NOTES.md promotion with a misleading reason. It now resolves the branch per component. release-tag.sh warns when HEAD is not on the component's branch family, since a tag on the wrong line is the mistake this layout exists to prevent. The repository's `release` ruleset now lists refs/heads/release/agent/* alongside refs/heads/release/*. Ruleset patterns are fnmatch, where neither * nor a bare ** crosses a /, so the nested family needs its own entry (or refs/heads/release/**/*); the doc says so. merge-gate.yaml already triggers on release/**, an Actions glob where ** does span /. Signed-off-by: Riley Rice (cherry picked from commit a74c5dd624cd9afc756091db75ebe28617936821) --- docs/contributing/release-process.md | 91 +++++++++++++++++++--------- docs/operations/versioning.md | 22 +++---- scripts/gen-changelog.sh | 19 ++++-- scripts/release-tag.sh | 15 +++++ 4 files changed, 103 insertions(+), 44 deletions(-) diff --git a/docs/contributing/release-process.md b/docs/contributing/release-process.md index b51d747a4..0e270ddf6 100644 --- a/docs/contributing/release-process.md +++ b/docs/contributing/release-process.md @@ -4,7 +4,16 @@ Step-by-step process for releasing NodeWright components using **release branche ## Release Branch Strategy -At feature-freeze a release branch is cut from `main`. All release candidates and the final release for that minor version are tagged on that branch, and every later patch for the same minor line is cherry-picked back to the same branch and tagged from there. The release branch is the single source of truth for everything that ships under one minor version — once it exists, nothing for that minor goes anywhere else. +A release branch is cut from `main` when a line is ready to stabilize: the operator/chart line at feature-freeze, the agent line when an agent minor or major is ready (see the table below). All release candidates and the final release for that minor version are tagged on that branch, and every later patch for the same minor line is cherry-picked back to the same branch and tagged from there. The release branch is the single source of truth for everything that ships under one minor version — once it exists, nothing for that minor goes anywhere else. + +There are two branch families, one per release cadence: + +| Branch | Tags cut on it | Cut when | +| --- | --- | --- | +| `release/vX.Y.x` | `operator/vX.Y.*`, `chart/vX.Y.*` | The operator minor reaches feature-freeze. The chart defines the compatible set, so it always moves with the operator. | +| `release/agent/vX.Y.x` | `agent/vX.Y.*` | An agent minor or major is ready to ship, on its own schedule. | + +The agent gets its own family because it is versioned independently and its releases do not line up with the operator's: `agent/v7.0.0` (the Go rewrite) ships between operator minors, and neither forcing an early operator minor nor shipping a rewrite as a chart patch would have been honest. The operator and chart stay together on one branch because the chart's job is to pin a compatible set. The two families meet at the chart: a new agent version reaches users when the chart's agent pin is bumped, which is a normal chart patch on `release/vX.Y.x` (see [Agent Releases](#agent-releases)). **Flow (one minor line):** @@ -33,12 +42,13 @@ gitGraph **Key principles:** -- **Branch first, then tag.** Always cut the release branch before the first RC. Tags only live on release branches, never on `main`. +- **Branch first, then tag.** Always cut the release branch before the first RC. Tags only live on release branches, never on `main`: operator and chart tags on `release/vX.Y.x`, agent tags on `release/agent/vX.Y.x`. - **Cherry-pick from `main`.** Any fix or feature destined for a release lands on `main` first, then is cherry-picked to the release branch. The release branch is never the place to *develop* — only to *stabilize and ship*. - Rare exception: a change that is genuinely release-branch-only (e.g. a `chart/Chart.yaml` version bump for that line) can be committed directly to the release branch via a feature branch and PR. -- **RCs are the validation gate.** Cut `-rc.1`, `-rc.2`, … on the release branch until you're happy. When an RC is approved, make a single `Chart.yaml` bump commit dropping the `-rc.N` suffix and tag `vX.Y.0` on that commit — no other code changes between the last good RC and the final release. -- **Patches stay on the same branch.** `v0.16.1`, `v0.16.2`, … are all cut from `release/v0.16.x` — cherry-pick the fix from `main`, bump `chart/Chart.yaml`, tag. -- **Component naming:** Operator drives the release; agent often reuses the previous version; chart always gets tagged because `Chart.yaml` (and therefore `appVersion`) moves with every release. +- **RCs are the validation gate.** Cut `-rc.1`, `-rc.2`, … on the release branch until you're happy. When an RC is approved, tag the final on the one bookkeeping commit made right after it, with no code changes in between. For the operator and chart that commit is the `Chart.yaml` bump dropping the `-rc.N` suffix; for the agent it is the changelog cut (see [Agent Releases](#agent-releases)). +- **Patches stay on the same branch.** `v0.16.1`, `v0.16.2`, … are all cut from `release/v0.16.x` — cherry-pick the fix from `main`, bump `chart/Chart.yaml`, tag. Agent patches do the same on `release/agent/vX.Y.x`, with the changelog cut in place of the chart bump. +- **Component naming:** Operator drives the `release/vX.Y.x` line; chart always gets tagged there because `Chart.yaml` (and therefore `appVersion`) moves with every release. The agent is tagged only on its own `release/agent/vX.Y.x` line, and a chart release picks up whichever agent version its `values.yaml` pins. +- **Branch protection and CI already know both families.** `merge-gate.yaml` runs on `release/**` (Actions glob, where `**` spans `/`). The repository's `release` ruleset uses fnmatch, where neither `*` nor a bare `**` crosses a `/`, so it lists both `refs/heads/release/*` and `refs/heads/release/agent/*` (`refs/heads/release/**/*` would also cover both); agent release branches get the same deletion, force-push and pull-request rules. ### Major/Minor Release Workflow @@ -68,8 +78,9 @@ 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. -# 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. +# 6. Cut the final release. No code changes land after the last good RC; the +# only commit between it and the final tag is the Chart.yaml bump to v0.16.0 +# (drop the -rc.N suffix). git commit -am "release: v0.16.0" git push origin release/v0.16.x git tag operator/v0.16.0 @@ -136,37 +147,61 @@ git commit -am "release: v0.16.1" git push origin release/v0.16.x # 4. Tag the components that changed and push *every* tag you created. -# The push list MUST include the agent tag if you tagged the agent above — -# otherwise the agent tag stays local and CI never sees it. +# Never tag the agent here: an agent fix ships from release/agent/vX.Y.x +# (see Agent Releases) and reaches this line as a values.yaml pin bump. git tag operator/v0.16.1 # If operator changed -git tag agent/v6.4.1 # Only if agent changed (rare) git tag chart/v0.16.1 # Chart always gets tagged -git push origin operator/v0.16.1 agent/v6.4.1 chart/v0.16.1 # drop any tag you didn't create +git push origin operator/v0.16.1 chart/v0.16.1 # drop the operator tag if you didn't create it ``` If the fix is urgent enough to need its own RC cycle, repeat the RC workflow above (e.g. `operator/v0.16.1-rc.1`) before tagging `v0.16.1`. -### Agent-Only Changes +### Agent Releases -Agent-only fixes don't need a new minor; they ride on the active release branch as a chart patch. +The agent releases on its own branch family, `release/agent/vX.Y.x`, and reaches users through a chart patch that bumps the agent pin. The two steps are deliberately separate: the agent tag proves the image, the chart tag decides who gets it. ```bash -# Land the agent fix on main, then cherry-pick to the active release branch. -git checkout release/v0.16.x -git cherry-pick -x - -# Bump chart/Chart.yaml to reference the new agent version (e.g. update the -# agent tag/digest under controllerManager.manager.agent and bump the chart -# version to v0.16.1). -git commit -am "release: v0.16.1 (agent v6.4.1)" -git push origin release/v0.16.x - -# Tag and push the components that changed. -git tag agent/v6.4.1 -git tag chart/v0.16.1 -git push origin agent/v6.4.1 chart/v0.16.1 +# 1. Cut the agent release branch from main when the agent minor/major is ready. +git fetch origin +git branch release/agent/v7.0.x origin/main +git push -u origin release/agent/v7.0.x + +# 2. Tag the RC on that branch and push it. This publishes agent:v7.0.0-rc.1 +# (signed, with per-platform SBOM and VEX attestations) and a GitHub +# pre-release; `latest` does not move for an RC. +git tag agent/v7.0.0-rc.1 release/agent/v7.0.x +git push origin agent/v7.0.0-rc.1 + +# 3. Validate the RC against a real cluster. Point a NodeWright's package at it +# with `agentImageOverride`, or install a chart with the pin overridden: +# --set controllerManager.manager.agent.tag=v7.0.0-rc.1 \ +# --set controllerManager.manager.agent.digest= +# Fixes land on main first and are cherry-picked to release/agent/v7.0.x; +# tag -rc.2, -rc.3, ... until clean. + +# 4. Cut the final. No code changes land after the last good RC; the only +# commit between it and the final tag is the changelog cut, which does not +# touch the image: run `scripts/gen-changelog.sh agent v7.0.0` on the agent +# branch to cut the CHANGELOG section and promote RELEASE_NOTES.md to the +# `## agent/v7.0.0` heading release.yml prepends to the release body, commit, +# then tag. +git tag agent/v7.0.0 release/agent/v7.0.x +git push origin agent/v7.0.0 + +# 5. Ship it in a chart. On main, bump the agent tag/digest under +# controllerManager.manager.agent in chart/values.yaml, then cherry-pick to +# the active operator/chart line and release a chart patch as usual. +git checkout release/v0.19.x +git cherry-pick -x +# bump chart/Chart.yaml to v0.19.1 +git commit -am "release: v0.19.1 (agent v7.0.0)" +git push origin release/v0.19.x +git tag chart/v0.19.1 +git push origin chart/v0.19.1 ``` +Agent patches (`agent/v7.0.1`, ...) follow the same shape as operator patches: cherry-pick the fix from `main` onto `release/agent/v7.0.x`, cut the changelog section there, tag there, then bump the chart pin on `main` and cherry-pick that to the chart line. + ### Release-Branch-Only Changes (rare) If a change genuinely doesn't belong on `main` — for example, the `chart/Chart.yaml` version bump for `v0.16.1`, or a backport that doesn't apply cleanly and needs to be re-implemented for the older line — open it as a feature branch off the release branch and PR it back to the release branch. **Default to cherry-picking from `main` first; only diverge when there's a clear reason the change can't exist there.** @@ -271,7 +306,7 @@ make release-tag ### Where a cut section is sourced from (patch vs. minor) -When you cut a **patch** (`vX.Y.Z` whose `X.Y` already has a final tag), the new section is sourced from that line's release branch, `release/vX.Y.x`, not from your current `HEAD`. A patch ships only the fixes cherry-picked onto its release branch; `main` meanwhile carries unrelated work bound for the next minor (a breaking API change, a dependency bump, …), and ranging `prevTag..HEAD` on `main` would sweep all of that into the patch. Sourcing from the release branch gives exactly what the patch ships, and lets you run the generator from any branch. +When you cut a **patch** (`vX.Y.Z` whose `X.Y` already has a final tag), the new section is sourced from that line's release branch, `release/vX.Y.x` (or `release/agent/vX.Y.x` for the agent), not from your current `HEAD`. A patch ships only the fixes cherry-picked onto its release branch; `main` meanwhile carries unrelated work bound for the next minor (a breaking API change, a dependency bump, …), and ranging `prevTag..HEAD` on `main` would sweep all of that into the patch. Sourcing from the release branch gives exactly what the patch ships, and lets you run the generator from any branch. Practical consequences for a patch cut: diff --git a/docs/operations/versioning.md b/docs/operations/versioning.md index 65203acfb..5ab618937 100644 --- a/docs/operations/versioning.md +++ b/docs/operations/versioning.md @@ -66,30 +66,28 @@ image: "ghcr.io/nvidia/skyhook/operator:0.7.0" ## Release Branching Strategy -NodeWright uses **release branches** to manage patches and maintenance releases: +NodeWright uses **release branches** to manage patches and maintenance releases, in two families: ```bash -release/v0.8.x # Contains operator v0.8.0 + agent v6.3.0 + chart v0.8.x -release/v0.9.x # Contains operator v0.9.0 + (agent v6.3.0*) + chart v0.9.x -release/v0.10.x # Contains operator v0.10.0 + (agent v6.3.0*) + chart v0.10.x +release/v0.18.x # operator v0.18.* + chart v0.18.* (chart pins the agent it ships with) +release/v0.19.x # operator v0.19.* + chart v0.19.* +release/agent/v7.0.x # agent v7.0.* only; the chart picks it up through its agent pin ``` -*Agent versions may not change every release - operator drives the release cycle ### Why Release Branches: -- **Operator-centric releases** - most releases are driven by operator features and bugs -- **Chart defines compatibility** - each branch contains a tested, compatible set of all components -- **Agent follows operator** - agent changes typically only require chart patch releases +- **Chart defines compatibility** - each `release/vX.Y.x` branch contains a tested, compatible set: the operator, the chart, and the agent version the chart pins +- **The agent releases on its own cadence** - `release/agent/vX.Y.x` carries only the agent, so an agent major (such as the v7 Go rewrite) neither forces an early operator minor nor ships as a chart patch - **Simplified patches** - fix bugs in the context of the full integrated system - **Connected git history** - preserves relationships between operator, agent, and chart changes ### Branch Workflow: 1. **Main development** happens on `main` branch -2. **Release preparation** creates `release/v{MAJOR.MINOR}.x` branch (typically driven by operator changes) -3. **Patch releases** are developed and tagged from release branches -4. **Agent-only changes** usually result in chart patch releases (no new release branch) -5. **Critical fixes** may be backported from `main` to release branches +2. **Operator release preparation** creates `release/v{MAJOR.MINOR}.x`; operator and chart RCs, finals and patches are tagged there +3. **Agent release preparation** creates `release/agent/v{MAJOR.MINOR}.x`; agent RCs, finals and patches are tagged there +4. **A new agent version reaches users** through a chart patch that bumps the agent pin in `chart/values.yaml` on the active `release/vX.Y.x` +5. **Critical fixes** may be backported from `main` to either kind of release branch ## Go Module Support diff --git a/scripts/gen-changelog.sh b/scripts/gen-changelog.sh index 23bce8208..9d7d1f43f 100755 --- a/scripts/gen-changelog.sh +++ b/scripts/gen-changelog.sh @@ -264,6 +264,16 @@ ROOT="$(git rev-list --max-parents=0 HEAD | tail -1)" # # A minor/major has no backport branch to read from, so it falls through to the # normal "commits after the latest tag, walked from HEAD" behaviour. +# +# Operator and chart share release/vX.Y.x; the agent has its own family, +# release/agent/vX.Y.x (docs/contributing/release-process.md, Agent Releases). +release_branch() { + case "$COMPONENT" in + agent) printf 'release/agent/%s.x\n' "$1" ;; + *) printf 'release/%s.x\n' "$1" ;; + esac +} + CUT_RANGE="" CUT_REF="" NOTES_SKIP_REASON="" @@ -272,7 +282,8 @@ if [[ -n "$NEXT_VERSION" ]]; then prev_same_line=$(printf '%s\n' "${TAGS[@]}" | grep -E "^${COMPONENT}/${next_mm//./\\.}\." | tail -1 || true) if [[ -n "$prev_same_line" ]]; then - for _cand in "release/${next_mm}.x" "origin/release/${next_mm}.x"; do + line_branch="$(release_branch "$next_mm")" + for _cand in "$line_branch" "origin/$line_branch"; do if git rev-parse --verify --quiet "${_cand}^{commit}" >/dev/null; then CUT_REF="$_cand" break @@ -280,7 +291,7 @@ if [[ -n "$NEXT_VERSION" ]]; then done if [[ -z "$CUT_REF" ]]; then echo "ERROR: cutting patch ${COMPONENT}/${NEXT_VERSION}, but no release branch" >&2 - echo " release/${next_mm}.x (or origin/release/${next_mm}.x) exists." >&2 + echo " ${line_branch} (or origin/${line_branch}) exists." >&2 echo " Create it and cherry-pick the fix(es) onto it first." >&2 echo " See docs/contributing/release-process.md (Patch Release Workflow)." >&2 exit 1 @@ -291,8 +302,8 @@ if [[ -n "$NEXT_VERSION" ]]; then # working tree. On release/vX.Y.x that is exactly the cherry-picks the patch # ships; on main it is the *next minor's* notes, and promoting it here would # file them under a patch. Skip rather than mis-attribute. - if [[ "$(git rev-parse --abbrev-ref HEAD)" != "release/${next_mm}.x" ]]; then - NOTES_SKIP_REASON="HEAD is $(git rev-parse --abbrev-ref HEAD), not release/${next_mm}.x" + if [[ "$(git rev-parse --abbrev-ref HEAD)" != "$line_branch" ]]; then + NOTES_SKIP_REASON="HEAD is $(git rev-parse --abbrev-ref HEAD), not ${line_branch}" fi echo "${C_DIM}Patch cut: sourcing ${COMPONENT}/${NEXT_VERSION} from ${CUT_REF} (${CUT_RANGE})${C_RESET}" >&2 fi diff --git a/scripts/release-tag.sh b/scripts/release-tag.sh index 10b0a9c81..ae3a19c19 100755 --- a/scripts/release-tag.sh +++ b/scripts/release-tag.sh @@ -138,6 +138,21 @@ fi HEAD_REF="$(git rev-parse --abbrev-ref HEAD) @ $(git rev-parse --short HEAD)" +# Tags live on their component's release branch family: agent tags on +# release/agent/vX.Y.x, operator and chart tags on release/vX.Y.x, nothing on +# main (docs/contributing/release-process.md, Release Branch Strategy). Warn +# rather than refuse: the branch may legitimately be checked out under another +# name, and the person tagging can see the HEAD line below. +HEAD_BRANCH="$(git rev-parse --abbrev-ref HEAD)" +case "$COMPONENT" in + agent) EXPECTED_FAMILY="release/agent/" ;; + *) EXPECTED_FAMILY="release/" ;; +esac +if [[ "$HEAD_BRANCH" != "${EXPECTED_FAMILY}"* ]] || [[ "$COMPONENT" != agent && "$HEAD_BRANCH" == release/agent/* ]]; then + echo + echo "${C_YELLOW}WARNING: HEAD is '${HEAD_BRANCH}'; ${COMPONENT} tags are expected on a ${EXPECTED_FAMILY}vX.Y.x branch.${C_RESET}" +fi + echo echo "About to tag (on ${C_BOLD}${HEAD_REF}${C_RESET}): ${C_BOLD}${C_GREEN}${TAG}${C_RESET}" echo "${C_DIM}Reminder: the release commit (chart bump, CHANGELOG) should already be committed at HEAD.${C_RESET}" From f37547c03f743ab6a7226665080e0da9ffab2ae7 Mon Sep 17 00:00:00 2001 From: Riley Rice Date: Thu, 1 Oct 2026 11:18:40 -0700 Subject: [PATCH 2/3] test(agent): assert what gracefulShutdown guarantees in sigterm_grace (#713) sigterm_grace asserted that progress held exactly one "start" and one "end" after the package pod was deleted mid-step. That is a transient state. Deleting the pod makes the Job count the attempt as failed whatever the agent exits with, so a replacement pod runs the apply stage again, and the shellscript steps declare idempotence: true, which both agents read as "run regardless of the completion flag". apply.sh runs twice by design, and the count passed in CI only by being taken within seconds of the first "end", before the replacement pod appended its own pair. On a real cluster the settled file had two pairs. The test now lets the package converge, then checks the property gracefulShutdown actually guarantees: the attempt that received SIGTERM finished. apply.sh tags each progress line with an attempt id, and one awk on the node checks that the id on the first "start" also appears on an "end" and that every start has an end, printing the counts when it fails so check_node.sh shows why. Counts or timing alone cannot catch the #672 regression: a killed first attempt leaves "start A, start B, end B", whose only end still comes a full sleep after the first start. Two docs record what running both agents on one node during release testing showed. agent/RELEASE_NOTES.md gains the v7 log-file differences the [out]/[err] entry did not cover: stderr is merged into the step's .log at mode 0600, where the Python agent wrote a 0644 .log and a .log.err that neither agent reaps, and interrupt logs are one -.log per interrupt run, reaped to five, where the Python agent wrote one per operation and never reaped them; the directory is the same. The Breaking entry now claims unchanged log directories rather than unchanged log paths. docs/architecture/lifecycle.md said deletion cleans the state annotations off nodes and removes the runtime-required taint; the finalizer keeps nodeState_ and version_ while they record packages whose files remain on the host, and never touches the taint (#714), and the Deletion list now says so. Closes #710 Closes #711 Closes #712 Signed-off-by: Riley Rice Co-authored-by: ayuskauskas (cherry picked from commit 0b2d791185fd10773aabe81f95e65d7a2d498992) --- agent/RELEASE_NOTES.md | 21 ++++++++++++-- docs/architecture/lifecycle.md | 11 ++++++-- .../sigterm_grace/chainsaw-test.yaml | 28 ++++++++++++------- .../sigterm_grace/nodewright.yaml | 9 ++++-- 4 files changed, 51 insertions(+), 18 deletions(-) diff --git a/agent/RELEASE_NOTES.md b/agent/RELEASE_NOTES.md index 61a3b3579..37eb75099 100644 --- a/agent/RELEASE_NOTES.md +++ b/agent/RELEASE_NOTES.md @@ -11,9 +11,11 @@ For the full commit-level log see CHANGELOG.md. removed.** The next release is `agent/v7.0.0`; `agent/v6.x` images are the Python agent and stay on GHCR. The operator-facing contract is unchanged: positional arguments, environment variables, exit codes, and the flag, - history, interrupt-marker and log paths written on the host, as verified by - the operator-agent chainsaw suite that ran against both implementations - before the cutover. Both implementations honour each other's on-host state, + history, interrupt-marker and log directories written on the host, as + verified by the operator-agent chainsaw suite that ran against both + implementations before the cutover. Two log file names inside those + directories do change; see Behavior Changes. Both implementations honour + each other's on-host state, so moving a node between `v6.x` and `v7.x` in either direction does not re-run completed steps or interrupts, with the single `node_restart` exception described under Upgrade and Rollback below. @@ -30,6 +32,19 @@ For the full commit-level log see CHANGELOG.md. timestamp.** Step and interrupt output is written to the log file as the script produced it. Anything parsing those files for the prefix must stop expecting it. +- **Step stderr lands in the same host log file as stdout, and log files are + mode 0600.** The Python agent wrote stdout to `-.log` (0644) + and stderr to a sibling `-.log.err`. Neither agent reaps the + zero-byte `.log.err` files a `v6.x` node already carries; remove them by hand + if the clutter matters. +- **Interrupt logs are one file per interrupt run, and are reaped.** The + directory is unchanged, + `///interrupts/`, + but the file is now `-.log` for the whole interrupt, kept to + the newest five like step logs. The Python agent wrote + `_-.log` per operation and never removed any; those + names do not match the new reaping pattern, so they stay on a `v6.x` node + until removed by hand. - **`SKYHOOK_AGENT_BUFFER_LIMIT` is no longer read or printed.** It only tuned the Python agent's stream reader and had no equivalent in Go. - **The CycloneDX SBOM moved from the multi-platform index digest to each platform manifest digest, and an OpenVEX document is now published alongside it.** An SBOM describes exactly one root filesystem, so one attached to a multi-platform index described neither child truthfully, and a consumer who resolved `linux/amd64` and enumerated referrers on that manifest found nothing. diff --git a/docs/architecture/lifecycle.md b/docs/architecture/lifecycle.md index c74152113..1ccb5b849 100644 --- a/docs/architecture/lifecycle.md +++ b/docs/architecture/lifecycle.md @@ -386,8 +386,15 @@ Deleting a NodeWright does not delete the changes it made to your hosts. The operator: 1. Runs the uninstall workflow for every package with `uninstall.enabled: true` -2. Cleans its metadata off the nodes — state annotations, cordons it owns, and - the runtime-required taint +2. Cleans its metadata off the nodes — the status labels, annotations and + conditions it owns, and the cordons it holds. The `nodeState_` and + `version_` annotations are kept as long as they still record packages + whose files remain on the host (a non-absent entry means "installed"; see + [CR Deletion in the uninstall guide](../user-guide/uninstall.md#cr-deletion-finalizer)), + and removed once nothing remains. + The runtime-required taint is not touched here; only the completion path in + [runtime-required](../user-guide/runtime-required.md#when-is-the-runtime-required-taint-removed-from-a-node) + removes it. 3. Releases the finalizer, at which point the object disappears Packages without `uninstall.enabled` are simply forgotten, not reversed. Their diff --git a/k8s-tests/operator-agent/sigterm_grace/chainsaw-test.yaml b/k8s-tests/operator-agent/sigterm_grace/chainsaw-test.yaml index 5dfed03cf..248903b5e 100644 --- a/k8s-tests/operator-agent/sigterm_grace/chainsaw-test.yaml +++ b/k8s-tests/operator-agent/sigterm_grace/chainsaw-test.yaml @@ -85,18 +85,26 @@ spec: ## SIGTERM writes "end". progress=/var/lib/skyhook/sigterm-grace-agent-operator/progress ../check_node.sh kind-worker "cat $progress" "^end" 75 - ## "end" alone does not prove survival: a killed step leaves no flag, so the - ## retry pod re-runs apply.sh from scratch and appends a second "start" and - ## then its own "end". Exactly one of each is what pins the first attempt as - ## the one that finished. grep -c prints the count, so match it as a whole line. - ../check_node.sh kind-worker "grep -c '^start' $progress" "^1$" 2 - ../check_node.sh kind-worker "grep -c '^end' $progress" "^1$" 2 - assert: - ## The retry pod finds apply.sh's completion flag, skips it and finishes the - ## remaining stages, so the package converges. A Go agent that had killed the - ## step would instead re-run it from scratch here, and the absence of "end" - ## above would already have failed the test. + ## Let the package converge before counting attempts. Deleting the pod makes the + ## Job count that attempt as failed whatever the agent exits with (a pod deleted + ## mid-init never reaches Succeeded), so a replacement pod runs the apply stage + ## again, and this package's steps declare idempotence: true (Disabled), which + ## both agents read as "run regardless of the completion flag". apply.sh therefore + ## runs twice by design, and a count taken right after the first "end" only ever + ## saw one pair by racing the replacement pod. file: assert.yaml + - script: + content: | + set -e + ## What gracefulShutdown guarantees is that the attempt that received SIGTERM + ## finished, so the check is on identity, not counts or timing: the attempt id + ## on the first "start" must appear on an "end", and every start must have an + ## end. In the #672 regression the killed attempt's id never gets an end. One + ## awk does both and prints the counts when it fails, since check_node.sh only + ## shows what the command printed. + progress=/var/lib/skyhook/sigterm-grace-agent-operator/progress + ../check_node.sh kind-worker "awk '\$1 == \"start\" {starts++; if (!first) first = \$2} \$1 == \"end\" {ends++; if (\$2 == first) finished = 1} END {if (finished && starts == ends) print \"first-finished\"; else print \"first=\" (finished ? \"finished\" : \"killed\") \" starts=\" starts \" ends=\" ends}' $progress" "^first-finished$" 2 - finally: - delete: file: nodewright.yaml diff --git a/k8s-tests/operator-agent/sigterm_grace/nodewright.yaml b/k8s-tests/operator-agent/sigterm_grace/nodewright.yaml index b9db8200c..f9b1ca9b9 100644 --- a/k8s-tests/operator-agent/sigterm_grace/nodewright.yaml +++ b/k8s-tests/operator-agent/sigterm_grace/nodewright.yaml @@ -36,14 +36,17 @@ spec: configMap: # Progress markers go to the package's state root rather than # $SKYHOOK_DIR: the copy dir is per-attempt, and the retry after the - # pod delete must read what the killed attempt wrote. + # pod delete must read what the killed attempt wrote. Each line carries + # an attempt id so the test can tell whose "end" it is reading; the + # timestamp is only there for reading the file by hand. apply.sh: | #!/bin/bash progress=/var/lib/skyhook/sigterm-grace-agent-operator/progress + attempt="$$-$RANDOM" mkdir -p "$(dirname "$progress")" - echo "start $(date +%s)" >> "$progress" + echo "start $attempt $(date +%s)" >> "$progress" sleep 60 - echo "end $(date +%s)" >> "$progress" + echo "end $attempt $(date +%s)" >> "$progress" apply_check.sh: | #!/bin/bash echo "apply checked" From 672b1fbc9494510e9c1d55ce088d0aa2083cf6c9 Mon Sep 17 00:00:00 2001 From: ayuskauskas Date: Wed, 30 Sep 2026 15:57:18 -0700 Subject: [PATCH 3/3] chore: add NOTICE, release checksums and read-only workflow tokens (#720) * docs: add a root NOTICE and link the published docs from the README The repository had no NOTICE at its root, only THIRD_PARTY_NOTICES.md, and the README never pointed at the published documentation at docs.nvidia.com/nodewright, which fern/docs.yml publishes docs/ to. Add a plain-text Apache-2.0 NOTICE naming the project, its copyright holder and first-publication year, its license, and where third-party licenses are listed. Link the hosted docs next to the README sentence that points at docs/. Signed-off-by: Alex Yuskauskas * docs: explain how issue priority is set and how to question it Priority lives in the native GitHub Priority issue field, set by maintainers during triage, but only .github/labels.yml and the agent instructions said so. Contributors had no way to learn that priority is never a label, what raises it, or how to ask for it to change. Add an "Issue priority" subsection under Filing Issues that states the existing practice: the field and its four values, that it is not a label and not set by reporters, the one factor SUPPORT.md already documents, and that a contributor who disagrees comments on the issue with context for a maintainer to decide. Signed-off-by: Alex Yuskauskas * ci: attach a verified checksums.txt to operator, agent and chart releases release.yml uploaded only the component's THIRD_PARTY_NOTICES file, so operator, agent and chart releases carried no checksums, while CLI releases already attach checksums.txt through cli-release.yaml. The upload step now copies the notices file it already selects for the tag's component into a fresh directory under its asset name, writes checksums.txt there in the CLI's format (sha256sum output keyed by asset name), verifies it with sha256sum -c, and uploads both files from that directory in one --clobber call. The file is staged rather than hashed in place because hashing the bare asset name at the repo root would read the chart rollup on an operator or agent tag. A missing or empty notices file now fails the step before anything is uploaded. Signed-off-by: Alex Yuskauskas * docs(contributing): note the checksums.txt beside the release notices release.yml now attaches checksums.txt next to the notices file on the releases it creates. Say so in the Release upload bullet, and say which releases lack it: those published before the step, and tags on a release branch cut before it, since a tag runs release.yml as it is at the tagged commit. Worded like the :latest caveat for release branches that predate #631. Signed-off-by: Alex Yuskauskas * ci: default the remaining workflow tokens to read-only Nine workflows either declared no top-level permissions or granted writes there. The repository's default workflow token is write, so every job in them that declared nothing ran with write access on push, tag and same-repository pull-request events, and OpenSSF Token-Permissions scores that 0. Each of the nine now declares permissions: contents: read at top level, the form the other workflows already use. Jobs that already declared their own permissions are unchanged. Of the jobs that inherited the default, only operator-ci's tests job needs more than contents: read: it logs in to ghcr.io with GITHUB_TOKEN to pull charts and images, and pushes only to the local ctlptl registry, so it gets packages: read. The rest check out, run linters, compare needs results, or upload coverage, which needs no write scope: Coveralls posts its comment from its own account. The two Fern workflows granted their writes at top level. They move, unchanged, to the one job each file runs: contents and pull-requests write for publish-fern-docs' create-pull-request step, and pull-requests write plus actions read for the preview comment job. Signed-off-by: Alex Yuskauskas --------- Signed-off-by: Alex Yuskauskas (cherry picked from commit 48a1f22c5c4796d8f4d75defa2fbadf6463c1a09) Signed-off-by: Riley Rice --- .github/workflows/agentless-container.yaml | 3 +++ .github/workflows/cli-release.yaml | 3 +++ .github/workflows/commit-linting.yaml | 4 ++++ .../workflows/fern-docs-preview-comment.yml | 8 ++++++-- .github/workflows/lint-ci.yaml | 3 +++ .github/workflows/operator-ci.yaml | 8 ++++++++ .github/workflows/publish-fern-docs.yml | 8 ++++++-- .github/workflows/release.yml | 20 +++++++++++++++++-- .github/workflows/security-checkov.yaml | 3 +++ CONTRIBUTING.md | 8 ++++++++ NOTICE | 9 +++++++++ README.md | 2 +- docs/contributing/release-process.md | 2 ++ 13 files changed, 74 insertions(+), 7 deletions(-) create mode 100644 NOTICE diff --git a/.github/workflows/agentless-container.yaml b/.github/workflows/agentless-container.yaml index c49f1152c..bd4fe6ce7 100644 --- a/.github/workflows/agentless-container.yaml +++ b/.github/workflows/agentless-container.yaml @@ -31,6 +31,9 @@ on: - containers/agentless/** - .github/workflows/agentless-container.yaml +permissions: + contents: read + # NOTE: we may want to switch to matrix build for multi-platform support if this is taking too long # https://docs.docker.com/build/ci/github-actions/multi-platform/#distribute-build-across-multiple-runners diff --git a/.github/workflows/cli-release.yaml b/.github/workflows/cli-release.yaml index 4e3c403d3..ad5f0a585 100644 --- a/.github/workflows/cli-release.yaml +++ b/.github/workflows/cli-release.yaml @@ -22,6 +22,9 @@ on: tags: - cli/* +permissions: + contents: read + env: # Opt all JS actions into Node 24 ahead of GitHub's Node 20 phase-out. FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true diff --git a/.github/workflows/commit-linting.yaml b/.github/workflows/commit-linting.yaml index 952a422db..e2398ae43 100644 --- a/.github/workflows/commit-linting.yaml +++ b/.github/workflows/commit-linting.yaml @@ -18,6 +18,10 @@ name: Commit Linting on: pull_request: types: [opened, edited, reopened, synchronize, ready_for_review] + +permissions: + contents: read + jobs: commit-linting: name: Git commit linting diff --git a/.github/workflows/fern-docs-preview-comment.yml b/.github/workflows/fern-docs-preview-comment.yml index cf2fd79b1..1a401509e 100644 --- a/.github/workflows/fern-docs-preview-comment.yml +++ b/.github/workflows/fern-docs-preview-comment.yml @@ -31,8 +31,7 @@ on: types: [completed] permissions: - pull-requests: write - actions: read + contents: read concurrency: group: ${{ github.workflow }}-${{ github.event.workflow_run.head_repository.full_name || github.repository }}-${{ github.event.workflow_run.head_branch || github.event.workflow_run.id }} @@ -44,6 +43,11 @@ jobs: # runs-on: linux-amd64-cpu8 # NVIDIA self-hosted runner timeout-minutes: 15 if: ${{ github.event.workflow_run.conclusion == 'success' }} + permissions: + # Posts or updates the preview comment on the PR. + pull-requests: write + # Downloads the fern-preview artifact from the triggering build run. + actions: read steps: - name: Download fern sources and metadata uses: actions/download-artifact@v8 diff --git a/.github/workflows/lint-ci.yaml b/.github/workflows/lint-ci.yaml index f47e2ce5b..a655f0e30 100644 --- a/.github/workflows/lint-ci.yaml +++ b/.github/workflows/lint-ci.yaml @@ -31,6 +31,9 @@ on: branches: - main +permissions: + contents: read + env: # Opt all JS actions into Node 24 ahead of GitHub's Node 20 phase-out. FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true diff --git a/.github/workflows/operator-ci.yaml b/.github/workflows/operator-ci.yaml index ae636143f..386989d58 100644 --- a/.github/workflows/operator-ci.yaml +++ b/.github/workflows/operator-ci.yaml @@ -54,6 +54,9 @@ on: - k8s-tests/** - chart/** +permissions: + contents: read + ## these envs control the build and test process below env: REGISTRY: ghcr.io @@ -113,6 +116,11 @@ jobs: tests: runs-on: ubuntu-latest needs: [k8s-test-versions] + permissions: + contents: read + # For the ghcr.io login below: the suites pull from ghcr.io and never + # push there (push-local-image targets the local ctlptl registry). + packages: read strategy: matrix: # Standard E2E tests on all supported K8s versions diff --git a/.github/workflows/publish-fern-docs.yml b/.github/workflows/publish-fern-docs.yml index ad3d01866..528b49c23 100644 --- a/.github/workflows/publish-fern-docs.yml +++ b/.github/workflows/publish-fern-docs.yml @@ -69,8 +69,7 @@ on: # - "chart/v[0-9]*.[0-9]*.[0-9]*" permissions: - contents: write - pull-requests: write + contents: read concurrency: group: fern-publish @@ -91,6 +90,11 @@ jobs: runs-on: ubuntu-latest # runs-on: linux-amd64-cpu8 # NVIDIA self-hosted runner timeout-minutes: 20 + permissions: + # For create-pull-request: it pushes the registry branch and opens the + # PR that persists it to main. + contents: write + pull-requests: write steps: - name: Checkout repository uses: actions/checkout@v7 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 762fbd667..875ad65dd 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -33,6 +33,9 @@ on: - 'agent/**' - 'chart/**' +permissions: + contents: read + jobs: release: runs-on: ubuntu-latest @@ -193,7 +196,7 @@ jobs: ${PRERELEASE_FLAG} fi - - name: Upload third-party notices to release + - name: Upload third-party notices and checksums to release env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | @@ -207,7 +210,20 @@ jobs: exit 1 ;; esac - gh release upload "${{ github.ref_name }}" "${NOTICES_FILE}" --clobber + if [[ ! -s "${NOTICES_FILE}" ]]; then + echo "ERROR: ${NOTICES_FILE} is missing or empty." >&2 + exit 1 + fi + # checksums.txt names each asset as the release does, by basename, so + # `sha256sum -c checksums.txt` works beside the downloads. Hashing that + # name here at the repo root would read the chart rollup on an operator + # or agent tag, so stage the exact file alone under its asset name and + # hash, verify and upload from there. + ASSET_DIR="$(mktemp -d)" + ASSET="$(basename "${NOTICES_FILE}")" + cp "${NOTICES_FILE}" "${ASSET_DIR}/${ASSET}" + ( cd "${ASSET_DIR}" && sha256sum "${ASSET}" > checksums.txt && sha256sum -c --strict checksums.txt ) + gh release upload "${{ github.ref_name }}" "${ASSET_DIR}/${ASSET}" "${ASSET_DIR}/checksums.txt" --clobber publish-chart: if: startsWith(github.ref_name, 'chart/') diff --git a/.github/workflows/security-checkov.yaml b/.github/workflows/security-checkov.yaml index 7893502cc..b58089f8d 100644 --- a/.github/workflows/security-checkov.yaml +++ b/.github/workflows/security-checkov.yaml @@ -26,6 +26,9 @@ on: paths: - 'chart/**' +permissions: + contents: read + env: # Opt all JS actions into Node 24 ahead of GitHub's Node 20 phase-out. FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d7c9c4c59..70639ad73 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -22,6 +22,14 @@ Maintainers, decision-making, and the process for becoming a maintainer are docu - **Questions**: Use [GitHub Discussions](https://github.com/NVIDIA/nodewright/discussions). - **Security vulnerabilities**: Do **not** file a public issue. See [SECURITY.md](SECURITY.md). +### Issue priority + +Maintainers communicate an issue's priority with its **Priority** field (Urgent / High / Medium / Low), which they set in the issue sidebar during triage. Priority is never a label, and reporters do not set it; the issue forms do not ask for it. + +Critical bugs and security vulnerabilities are prioritized, as [SUPPORT.md](SUPPORT.md#what-to-expect) says. Report a security vulnerability through [SECURITY.md](SECURITY.md) rather than an issue. + +If you think an issue's priority is wrong, comment on the issue with the context, such as its impact, and a maintainer decides. + ## Claiming an Issue Want to work on an issue? Claim it so others know it is taken. Comment on the issue and a bot will handle the assignment: diff --git a/NOTICE b/NOTICE new file mode 100644 index 000000000..01b73042c --- /dev/null +++ b/NOTICE @@ -0,0 +1,9 @@ +NodeWright (formerly Skyhook) +Copyright (c) 2024 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + +This product includes software developed at +NVIDIA CORPORATION (https://www.nvidia.com/). + +This project is licensed under the Apache License 2.0; see LICENSE. + +Third-party components and their licenses are listed in THIRD_PARTY_NOTICES.md. diff --git a/README.md b/README.md index b33c3c7e2..c607ac0a0 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ **NodeWright** is a Kubernetes-aware package manager for cluster administrators to safely modify and maintain underlying host declaratively at scale. -**New here?** Start with the **[Quickstart](docs/getting-started/quickstart.md)** to install NodeWright and run a package on one node in about five minutes, or the **[Overview](docs/getting-started/overview.md)** for what NodeWright is and when to reach for it. Full docs live in [`docs/`](docs/README.md). +**New here?** Start with the **[Quickstart](docs/getting-started/quickstart.md)** to install NodeWright and run a package on one node in about five minutes, or the **[Overview](docs/getting-started/overview.md)** for what NodeWright is and when to reach for it. Full docs live in [`docs/`](docs/README.md) and are published at [docs.nvidia.com/nodewright](https://docs.nvidia.com/nodewright). > **Note:** NodeWright is being renamed from Skyhook, and the rename has now landed for the core surfaces. The Helm chart, operator image, CLI (`kubectl nodewright`), and the CRDs (`nodewright.nvidia.com/v1alpha1`, Kind `NodeWright`; `DeploymentPolicy` moves to the same group) are published under `nodewright`. Existing `skyhook.nvidia.com`/`Skyhook` resources keep working during the transition: the operator auto-imports them to NodeWright and preserves per-node state (no package re-run), and legacy writes emit a deprecation warning. See the [migration guide](docs/getting-started/migration.md). > diff --git a/docs/contributing/release-process.md b/docs/contributing/release-process.md index 0e270ddf6..9a96c2374 100644 --- a/docs/contributing/release-process.md +++ b/docs/contributing/release-process.md @@ -717,6 +717,8 @@ Tag resolution reads the local clone, so run `git fetch --tags` before `make not - `agent/v*` → `agent/THIRD_PARTY_NOTICES.md` - `chart/v*` → root `THIRD_PARTY_NOTICES.md` (the combined rollup, since chart packages both images) + The workflow also attaches `checksums.txt`, the SHA-256 of that notices file keyed by its asset name, so `sha256sum -c checksums.txt` verifies the downloaded file. Releases published before the checksum step carry none. A tag push runs `release.yml` as it is at the tagged commit, so a release branch cut before the step attaches only the notices file to every tag cut from it. Cherry-pick the commit that added the step to `main` onto the branch before tagging a release that should carry `checksums.txt`. + ## Rollback For problematic releases: