From dc29ca4104930b20faf89eaad5f8c7d145909f23 Mon Sep 17 00:00:00 2001 From: Riley Rice Date: Wed, 23 Sep 2026 15:40:51 -0700 Subject: [PATCH] test(agent): run the service interrupt against a real agent in the operator-agent suite The only interrupt type the suite had ever run against a real agent was noop. The main chainsaw suite's reboot cases run against the agentless image and cover the operator's side of an interrupt only, so the agent's own interrupt command path -- the daemon-reload, the per-service restart, the one completion marker per operation that a retry resumes from -- had no end-to-end coverage on either agent going into the Go cutover. The service interrupt is the one type the suite can execute for real. Kind nodes run systemd as PID 1, and the package pod mounts the host root with HostToContainer propagation, so systemctl inside the agent's chroot reaches the real manager. A reboot would terminate the kind node container, so that path stays unit-tested. The package's apply.sh installs the unit in two definitions with no reload between them. The first is loaded and started (RemainAfterExit keeps it in memory) and appends to a stale marker; the second, written over the file without a daemon-reload, appends to the real marker. The interrupt runs daemon-reload as operation 0 and the restart as operation 1, so exactly one real-marker line proves the interrupt ran the real service manager exactly once, and exactly one stale line proves the daemon-reload reached the manager: a restart without an effective reload would have re-run the stale definition instead. The stale line is also the proof apply.sh ran, whose own log both agents have rotated away by the time the package completes (5 logs kept, 6 written). This is a single clean pass; the resume-from-marker path a retry would take is not exercised. Both service_restart_{0,1}.complete markers pin the per-operation layout both agents share once an interrupt finishes; the scenario records that Python writes a marker before running its operation and Go after it succeeds. post-interrupt and post-interrupt-check are asserted the way post_interrupt/ asserts them for noop, and every stage the package traverses has a script, so the "Could not find file" guard fires only on a real defect. The unit, the markers and the manager state are removed in finally as well as before the run, so a reused cluster is left as it was found. Closes #679 Signed-off-by: Riley Rice --- docs/contributing/ci-test-pools.md | 9 +- .../service_restart/assert.yaml | 43 ++++++++ .../service_restart/chainsaw-test.yaml | 104 ++++++++++++++++++ .../service_restart/nodewright.yaml | 91 +++++++++++++++ 4 files changed, 246 insertions(+), 1 deletion(-) create mode 100644 k8s-tests/operator-agent/service_restart/assert.yaml create mode 100644 k8s-tests/operator-agent/service_restart/chainsaw-test.yaml create mode 100644 k8s-tests/operator-agent/service_restart/nodewright.yaml diff --git a/docs/contributing/ci-test-pools.md b/docs/contributing/ci-test-pools.md index cb5010d32..c9f0d1dae 100644 --- a/docs/contributing/ci-test-pools.md +++ b/docs/contributing/ci-test-pools.md @@ -57,7 +57,14 @@ It covers `apply`, `config`, `interrupt`, `post-interrupt` and `uninstall`, each counterpart, plus log retention, `SKYHOOK_AGENT_WRITE_LOGS=false`, and the `check_results` / `_ALL_CHECKED` summary artifacts. `sigterm_grace` covers pod teardown mid-step: it deletes the package pod while `apply.sh` is running and asserts the step finishes inside the package's -`gracefulShutdown` rather than being killed, and that the retry then skips it. `upgrade` is **not** +`gracefulShutdown` rather than being killed, and that the retry then skips it. `service_restart` +runs the `service` interrupt for real: kind nodes run systemd, so the agent's chrooted +`systemctl daemon-reload` and `systemctl restart` act on a unit the scenario installs, and the +unit is installed in two definitions with no reload between them so that the restart can only +write the expected marker if the agent's `daemon-reload` reached the manager. It is the only +interrupt type the suite currently executes rather than `noop`: a `reboot` would terminate the +kind node container, so that path stays unit-tested, and `restart_all_services` is untested on +the kind image either way. `upgrade` is **not** covered: the `shellscript` package these scenarios use declares no `upgrade` mode in any published version, so an upgrade scenario needs a package that supports one. diff --git a/k8s-tests/operator-agent/service_restart/assert.yaml b/k8s-tests/operator-agent/service_restart/assert.yaml new file mode 100644 index 000000000..4d430172a --- /dev/null +++ b/k8s-tests/operator-agent/service_restart/assert.yaml @@ -0,0 +1,43 @@ +# 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: v1 +kind: Node +metadata: + labels: + nodewright.nvidia.com/test-node: skyhooke2e + nodewright.nvidia.com/status_service-restart-agent-operator: complete + annotations: + nodewright.nvidia.com/status_service-restart-agent-operator: complete +status: + (conditions[?type == 'nodewright.nvidia.com/service-restart-agent-operator/NotReady']): + - reason: "Complete" + status: "False" + (conditions[?type == 'nodewright.nvidia.com/service-restart-agent-operator/Erroring']): + - reason: "Not Erroring" + status: "False" +--- +apiVersion: nodewright.nvidia.com/v1alpha1 +kind: NodeWright +metadata: + name: service-restart-agent-operator +status: + status: complete + nodeStatus: + # grab values should be one and is complete + (values(@)): + - complete diff --git a/k8s-tests/operator-agent/service_restart/chainsaw-test.yaml b/k8s-tests/operator-agent/service_restart/chainsaw-test.yaml new file mode 100644 index 000000000..d3314bfd8 --- /dev/null +++ b/k8s-tests/operator-agent/service_restart/chainsaw-test.yaml @@ -0,0 +1,104 @@ +# 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: service-restart-agent-operator +spec: + timeouts: + assert: 240s + exec: 90s + steps: + - try: + - script: + content: | + ## remove annotation from last run + ../../../operator/bin/nodewright reset service-restart-agent-operator --confirm 2>/dev/null || true + - script: + content: | + ## reinstall the debug pod in case it was deleted + ../setup.sh kind-worker setup + - script: + content: | + ## clean up from prior runs, including the unit a previous run installed on the node. + ## The same block runs in finally below; this copy covers a run that never got there. + ../check_node.sh kind-worker "rm -rf /var/log/skyhook/service-restart-agent-operator || true" ".*" 2 + ../check_node.sh kind-worker "rm -rf /var/lib/skyhook/service-restart-agent-operator || true" ".*" 2 + ../check_node.sh kind-worker "systemctl stop nodewright-e2e 2>/dev/null; rm -f /etc/systemd/system/nodewright-e2e.service; systemctl daemon-reload; systemctl reset-failed nodewright-e2e 2>/dev/null; true" ".*" 2 + - apply: + file: nodewright.yaml + - assert: + file: assert.yaml + - script: + content: | + set -e + ## The unit's second definition appends one line to this marker per run, and nothing + ## starts it except the interrupt's `systemctl restart`, so exactly one line proves + ## the interrupt ran the real service manager exactly once. This is a single clean + ## pass; the resume-from-marker path a retry would take is not exercised here. + ## assert.yaml already waited for complete and a oneshot restart is synchronous, so a + ## short poll is enough. + state=/var/lib/skyhook/service-restart-agent-operator + ../check_node.sh kind-worker "grep -c . $state/service-restarts" "^1$" 10 + ## Operation 0, the daemon-reload, is what makes the manager pick up that second + ## definition. apply.sh loaded and started the first definition (the one line in the + ## stale marker) and then overwrote the file without reloading, so a restart whose + ## daemon-reload never reached the manager would have re-run the stale definition: + ## two stale lines and no real one. Exactly one stale line is what pins operation 0, + ## and it is also the proof apply.sh ran, whose own log has been rotated away by now + ## (both agents keep the 5 newest step logs and this package writes 6). + ../check_node.sh kind-worker "grep -c . $state/service-restarts-stale" "^1$" 2 + - script: + content: | + set -e + ## The service interrupt is two operations, daemon-reload then the restart, and both + ## agents leave one completion marker per operation, so the layout is the same once + ## the interrupt has finished and a retry on either agent resumes from it. They differ + ## in when the marker is written: Python before it runs the command, Go after the + ## command succeeds, so a Python pod killed mid-restart can leave a marker for a + ## restart that never completed. This scenario is a clean pass and asserts the layout. + flags=/var/lib/skyhook/service-restart-agent-operator/interrupts/flags/service-restart-agent-operator*/ + ../check_node.sh kind-worker "ls $flags" "service_restart_0.complete" + ../check_node.sh kind-worker "ls $flags" "service_restart_1.complete" + - script: + content: | + set -e + ## post-interrupt and post-interrupt-check both have to run after a service interrupt, + ## the same two assertions post_interrupt/ makes for the noop case. The inverted grep + ## is the missing-configmap-key guard from #605; every stage this package traverses + ## has a script, so it can only fire on a real defect. It sees only the surviving + ## shellscript_run.sh logs: the apply log has been rotated away (covered by the stale + ## marker above) and the interrupt's own log lives under $log/interrupts/. + log=/var/log/skyhook/service-restart-agent-operator/shellscript/1.1.1 + ../check_node.sh kind-worker "cat $log/*.log" "post interrupt ran" 10 + ../check_node.sh kind-worker "cat $log/*.log" "post interrupt checked" 10 + ../check_node.sh kind-worker "cat $log/*.log" "Could not find file" 2 true + ## Only the _ALL_CHECKED filename is attributable to post-interrupt-check; see + ## post_interrupt/ for why check_results is not asserted here. + flags=/var/lib/skyhook/service-restart-agent-operator/flags + ../check_node.sh kind-worker "ls $flags" "post-interrupt-check_ALL_CHECKED" 10 + - finally: + - delete: + file: nodewright.yaml + - script: + content: | + ## This is the only scenario that installs anything on the node outside the agent's + ## own state, so leave kind-worker as it was found: stop and remove the unit, reload + ## the manager, and drop the markers. Non-gating on purpose, like the cleanup above. + ../check_node.sh kind-worker "systemctl stop nodewright-e2e 2>/dev/null; rm -f /etc/systemd/system/nodewright-e2e.service; systemctl daemon-reload; systemctl reset-failed nodewright-e2e 2>/dev/null; true" ".*" 2 + ../check_node.sh kind-worker "rm -rf /var/lib/skyhook/service-restart-agent-operator/service-restarts /var/lib/skyhook/service-restart-agent-operator/service-restarts-stale || true" ".*" 2 diff --git a/k8s-tests/operator-agent/service_restart/nodewright.yaml b/k8s-tests/operator-agent/service_restart/nodewright.yaml new file mode 100644 index 000000000..5423c43b9 --- /dev/null +++ b/k8s-tests/operator-agent/service_restart/nodewright.yaml @@ -0,0 +1,91 @@ +# 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: + labels: + app.kubernetes.io/part-of: skyhook-operator + app.kubernetes.io/created-by: skyhook-operator + name: service-restart-agent-operator +spec: + nodeSelectors: + matchLabels: + nodewright.nvidia.com/test-node: skyhooke2e + packages: + service-restart-agent-operator: + version: "1.1.1" + image: ghcr.io/nvidia/skyhook-packages/shellscript + # The only interrupt type the suite currently executes for real: kind nodes + # run systemd as PID 1 and the host root is mounted HostToContainer, so + # systemctl inside the agent's chroot reaches the real manager. A reboot + # would terminate the kind node container; restart_all_services + # (`service procps force-reload`) is untested on the kind image either way. + interrupt: + type: service + services: [nodewright-e2e] + configMap: + # apply.sh installs the unit twice on purpose. The first definition is + # loaded into the manager and started, and RemainAfterExit keeps it in + # memory; its ExecStart appends to the *stale* marker. The file is then + # overwritten with the second definition, whose ExecStart appends to the + # real marker, WITHOUT a daemon-reload. The agent's service interrupt + # runs daemon-reload as operation 0 and the restart as operation 1, so + # a real-marker line can only come from the interrupt, and it can only + # be the real marker rather than a second stale line if operation 0 + # actually reached the manager. `%%` is systemd's escape for a literal `%`. + apply.sh: | + #!/bin/bash + set -e + unit=/etc/systemd/system/nodewright-e2e.service + state=/var/lib/skyhook/service-restart-agent-operator + mkdir -p "$state" + cat > "$unit" <<'UNIT' + [Unit] + Description=NodeWright operator-agent service-restart marker (stale definition) + + [Service] + Type=oneshot + RemainAfterExit=yes + ExecStart=/bin/sh -c 'date +%%s >> /var/lib/skyhook/service-restart-agent-operator/service-restarts-stale' + UNIT + systemctl daemon-reload + systemctl start nodewright-e2e + cat > "$unit" <<'UNIT' + [Unit] + Description=NodeWright operator-agent service-restart marker + + [Service] + Type=oneshot + RemainAfterExit=yes + ExecStart=/bin/sh -c 'date +%%s >> /var/lib/skyhook/service-restart-agent-operator/service-restarts' + UNIT + echo "unit installed" + apply_check.sh: | + #!/bin/bash + echo "apply checked" + config.sh: | + #!/bin/bash + echo "configuring" + config_check.sh: | + #!/bin/bash + echo "config checked" + post_interrupt.sh: | + #!/bin/bash + echo "post interrupt ran" + post_interrupt_check.sh: | + #!/bin/bash + echo "post interrupt checked"