test(agent): assert what gracefulShutdown guarantees in sigterm_grace

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 `<step>-<timestamp>.log` (0644)
+  and stderr to a sibling `<step>-<timestamp>.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,
+  `<SKYHOOK_LOG_DIR>/<package name in the NodeWright>/<version>/interrupts/`,
+  but the file is now `<type>-<timestamp>.log` for the whole interrupt, kept to
+  the newest five like step logs. The Python agent wrote
+  `<type>_<index>-<timestamp>.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.

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_<name>` and
+   `version_<name>` 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

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

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"