fix(cli): clear Status.ConfigUpdates on a full nodewright reset

docs/user-guide/cli.md

@@ -231,7 +231,9 @@ kubectl nodewright reset gpu-init --package pkg1:1.0 --confirm --skip-batch-rese
 
 > **Note:** By default, `reset` also resets the deployment policy batch state so the next rollout starts from batch 1, and clears node ordering state (`NodeOrderOffset` and `NodePriority`) so `SKYHOOK_NODE_ORDER` restarts from `0`. Use `--skip-batch-reset` to preserve the existing batch and ordering state.
 >
-> **Note:** `reset` and `node reset` never uncordon a node. On nodes the NodeWright still selects they also leave its `nodewright.nvidia.com/cordon_<name>` annotation in place, so the re-run releases the cordon when it completes; the operator does not release a cordon it holds no annotation for (see [Cordons NodeWright Did Not Create](../architecture/interrupt-flow.md#cordons-nodewright-did-not-create)). A full reset removes the annotation only where no re-run will come, because the NodeWright no longer exists or no longer selects the node; after that, uncordon the node yourself, only once no other `nodewright.nvidia.com/cordon_*` or `runtimeRequiredCordon` annotation remains and nothing else placed the cordon (see [Orphaned Cordon Recovery](../architecture/interrupt-flow.md#orphaned-cordon-recovery)). `reset --package` only edits the package's `nodeState` entry and never touches the annotation; it requires the NodeWright to exist. Against an operator after v0.19.1, CLI v0.4.0 and earlier remove the annotation unconditionally, so after a reset with one of them, `kubectl uncordon` the node once the re-run completes, subject to the same check.
+> **Note:** During a non-dry-run reset, `reset` attempts to clear the NodeWright's recorded config changes (`Status.ConfigUpdates`) after at least one node reset succeeds and no node-state update error occurs, so the restarted run is not affected by keys left by earlier ConfigMap edits. With `--package` only that package's keys are cleared, and only if no node still holds another version of it. This is independent of `--skip-batch-reset`. If the status patch fails for an error other than `NotFound`, `reset` prints a warning and continues; `NotFound` (the NodeWright no longer exists) is ignored.
+>
+> **Note:** `reset` and `node reset` never uncordon a node. On nodes the NodeWright still selects they also leave its `nodewright.nvidia.com/cordon_<name>` annotation in place, so the re-run releases the cordon when it completes; the operator does not release a cordon it holds no annotation for (see [Cordons NodeWright Did Not Create](../architecture/interrupt-flow.md#cordons-nodewright-did-not-create)). A full reset removes the annotation only where no re-run will come, because the NodeWright no longer exists or no longer selects the node; after that, uncordon the node yourself, only once no other `nodewright.nvidia.com/cordon_*` or `runtimeRequiredCordon` annotation remains and nothing else placed the cordon (see [Orphaned Cordon Recovery](../architecture/interrupt-flow.md#orphaned-cordon-recovery)). `reset --package` edits only the package's `nodeState` entry and its `configUpdates` keys, and never touches the annotation; it requires the NodeWright to exist. Against an operator after v0.19.1, CLI v0.4.0 and earlier remove the annotation unconditionally, so after a reset with one of them, `kubectl uncordon` the node once the re-run completes, subject to the same check.
 
 #### `--package <name>[:<version>]`
 
@@ -243,6 +245,12 @@ each node's `nodeState` annotation instead of removing the whole annotation:
   are left untouched and reported in the command output.
 - If the package was the last entry in the annotation, the entire annotation
   is removed (matching the behavior of a full reset).
+- The package's recorded config changes (`Status.ConfigUpdates[<name>]`) are
+  cleared once every node was updated, so the re-run does not skip the
+  package's own interrupt. They are keyed by name only, so they are kept while
+  any node still holds another version of the package (for example after
+  `pkg1:2.0` when nodes also hold `pkg1:1.0`). Other packages' keys are left
+  alone.
 - Batch state is **deliberately not reset** on this path regardless of
   `--skip-batch-reset`: restarting the full rollout from batch 1 to recover a
   single package would be disproportionate. Pair with

k8s-tests/chainsaw/cli/reset/README.md

@@ -12,12 +12,14 @@ Validates the nodewright reset command for resetting all nodes in a NodeWright.
    - Run `nodewright reset <name>`
    - Assert all nodes are reset to initial state
 4. Verify node annotations are cleared
+5. Seed `status.configUpdates` before the reset, and verify the reset clears it
 
 ## Key Features Tested
 
 - `nodewright reset` - Resets all nodes for a NodeWright
 - Node state cleanup
 - Annotation removal
+- Recorded config changes (`status.configUpdates`) cleared
 
 ## Files
 

k8s-tests/chainsaw/cli/reset/chainsaw-test.yaml

@@ -51,7 +51,18 @@ spec:
     - assert:
         file: assert-nodewright-disabled.yaml
 
-  # Step 3: Test reset command
+  # Step 3: Record config changes the way an earlier ConfigMap edit would have, so reset has
+  # something to clear (they are per package and survive a node-state reset)
+  - name: seed-config-updates
+    try:
+    - script:
+        timeout: 30s
+        content: |
+          kubectl patch nodewright cli-reset-test --subresource=status --type=merge \
+            -p '{"status":{"configUpdates":{"cli-reset-pkg":["game.properties"]}}}'
+          test -n "$(kubectl get nodewright cli-reset-test -o jsonpath='{.status.configUpdates}')"
+
+  # Step 4: Test reset command
   - name: test-reset
     try:
     - script:
@@ -60,3 +71,13 @@ spec:
           ../nodewright-cli reset cli-reset-test --confirm
     - assert:
         file: assert-nodes-reset.yaml
+
+  # Step 5: Reset clears the recorded config changes, or the restarted run would skip a package's
+  # own interrupt
+  - name: config-updates-cleared
+    try:
+    - script:
+        timeout: 30s
+        content: |
+          out="$(kubectl get nodewright cli-reset-test -o jsonpath='{.status.configUpdates}')"
+          test -z "$out" || { echo "configUpdates not cleared: $out"; exit 1; }

operator/cmd/cli/app/reset.go

@@ -26,6 +26,7 @@ import (
 	"strings"
 
 	"github.com/spf13/cobra"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/NVIDIA/nodewright/operator/api/nodewright/v1alpha1"
@@ -108,7 +109,11 @@ that have state for the specified NodeWright.
 
 By default, this command also resets the deployment policy batch state, allowing
 the rollout to start fresh from batch 1. Use --skip-batch-reset to preserve the
-existing batch state.`,
+existing batch state.
+
+It also clears the config changes recorded in the NodeWright's status (only the
+package's with --package, and only if no node keeps another version of it), so keys
+from earlier ConfigMap edits do not carry into the restarted run.`,
 		Example: `  # Reset all nodes for gpu-init NodeWright
   kubectl nodewright reset gpu-init --confirm
 
@@ -248,6 +253,13 @@ func runReset(ctx context.Context, cmd *cobra.Command, kubeClient *client.Client
 		_, _ = fmt.Fprintf(cmd.OutOrStdout(), "\nSuccessfully reset %d node(s) for NodeWright %q\n", successCount, skyhookName)
 	}
 
+	// Every node restarts from apply, so keys recorded by earlier config changes must not
+	// outlive the reset: they make the next full cycle skip the package's own interrupt.
+	// Skipped on partial failure, where some nodes still carry state that relies on them.
+	if successCount > 0 && len(updateErrors) == 0 {
+		resetConfigUpdatesForReset(ctx, cmd, kubeClient, skyhookName, "")
+	}
+
 	// Reset batch state unless --skip-batch-reset is set
 	if !opts.skipBatchReset {
 		resetBatchStateForReset(ctx, cmd, kubeClient, skyhookName)
@@ -306,6 +318,24 @@ func resetNodeAnnotations(
 	return successCount, updateErrors
 }
 
+// resetConfigUpdatesForReset clears Status.ConfigUpdates, or only pkg's keys when pkg is set,
+// if a dynamic client is available
+func resetConfigUpdatesForReset(ctx context.Context, cmd *cobra.Command, kubeClient *client.Client, skyhookName, pkg string) {
+	if kubeClient.Dynamic() == nil {
+		return
+	}
+
+	// Raw merge patch: null deletes the omitempty map or the package's entry
+	patch := []byte(`{"status":{"configUpdates":null}}`)
+	if pkg != "" {
+		patch, _ = json.Marshal(map[string]any{"status": map[string]any{"configUpdates": map[string]any{pkg: nil}}})
+	}
+	if err := utils.PatchSkyhookStatusRaw(ctx, kubeClient.Dynamic(), skyhookName, patch); err != nil && !apierrors.IsNotFound(err) {
+		// NotFound: the NodeWright is gone (orphaned-cordon cleanup), so there is nothing to clear
+		_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Warning: failed to clear config updates: %v\n", err)
+	}
+}
+
 // resetBatchStateForReset resets the batch state for a NodeWright if dynamic client is available
 func resetBatchStateForReset(ctx context.Context, cmd *cobra.Command, kubeClient *client.Client, skyhookName string) {
 	if kubeClient.Dynamic() == nil {
@@ -360,6 +390,33 @@ func splitPackageRef(s string) (name, version string, hasVersion bool, err error
 // state (which restarts the entire rollout from batch 1) would be wildly
 // disproportionate to a one-package recovery, so this path deliberately
 // skips it regardless of --skip-batch-reset.
+// packageResetTarget is one node a package reset edits, with the state it keeps.
+type packageResetTarget struct {
+	nodeName  string
+	remaining v1alpha1.NodeState
+	emptied   bool
+}
+
+// packageStillHeld reports whether any node still holds an entry for the package once the
+// targets have their remaining state, that is, another version of it.
+func packageStillHeld(nodeStates map[string]v1alpha1.NodeState, targets []packageResetTarget, name string) bool {
+	after := make(map[string]v1alpha1.NodeState, len(nodeStates))
+	for n, ns := range nodeStates {
+		after[n] = ns
+	}
+	for _, t := range targets {
+		after[t.nodeName] = t.remaining
+	}
+	for _, ns := range after {
+		for _, status := range ns {
+			if status.Name == name {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 func runPackageReset(
 	ctx context.Context,
 	cmd *cobra.Command,
@@ -384,12 +441,7 @@ func runPackageReset(
 
 	annotationKey := nodeStateAnnotationPrefix + skyhookName
 
-	type target struct {
-		nodeName  string
-		remaining v1alpha1.NodeState
-		emptied   bool
-	}
-	var targets []target
+	var targets []packageResetTarget
 	nodeNames := make([]string, 0, len(nodeStates))
 	for n := range nodeStates {
 		nodeNames = append(nodeNames, n)
@@ -414,7 +466,7 @@ func runPackageReset(
 		if !matched {
 			continue
 		}
-		targets = append(targets, target{
+		targets = append(targets, packageResetTarget{
 			nodeName:  nodeName,
 			remaining: next,
 			emptied:   len(next) == 0,
@@ -480,5 +532,10 @@ func runPackageReset(
 	}
 
 	_, _ = fmt.Fprintf(cmd.OutOrStdout(), "\nSuccessfully updated %d node(s)\n", success)
+	// Same reasoning as a full reset, scoped to this package's keys. The keys are per name, so
+	// keep them while any node still holds another version of the package.
+	if firstErr == nil && !packageStillHeld(nodeStates, targets, name) {
+		resetConfigUpdatesForReset(ctx, cmd, kubeClient, skyhookName, name)
+	}
 	return firstErr
 }

operator/cmd/cli/app/reset_test.go

@@ -447,6 +447,8 @@ var _ = Describe("Reset Command", func() {
 			mockDynamic.On("Resource", schema.GroupVersionResource{
 				Group: "nodewright.nvidia.com", Version: "v1alpha1", Resource: "nodewrights",
 			}).Return(mockNSRes)
+			mockNSRes.On("Patch", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+				Return(&unstructured.Unstructured{}, nil).Maybe()
 			kubeClient = client.NewWithClientsAndConfig(fakeKube, mockDynamic, nil)
 			cliCtx = context.NewCLIContext(nil)
 			out = &bytes.Buffer{}
@@ -527,6 +529,179 @@ var _ = Describe("Reset Command", func() {
 		})
 	})
 
+	// Keys recorded by earlier config changes make the next full cycle skip the package's
+	// own interrupt, so a full reset has to clear them (issue #776).
+	Describe("config updates", func() {
+		var (
+			fakeKube   *fake.Clientset
+			mockNSRes  *mockdynamic.NamespaceableResourceInterface
+			kubeClient *client.Client
+			cliCtx     *context.CLIContext
+			cmd        *cobra.Command
+		)
+		const name = "demo"
+		const clearPatch = `{"status":{"configUpdates":null}}`
+
+		BeforeEach(func() {
+			fakeKube = fake.NewClientset()
+			mockDynamic := &mockdynamic.Interface{}
+			mockNSRes = &mockdynamic.NamespaceableResourceInterface{}
+			mockDynamic.On("Resource", schema.GroupVersionResource{
+				Group: "nodewright.nvidia.com", Version: "v1alpha1", Resource: "nodewrights",
+			}).Return(mockNSRes)
+			u := &unstructured.Unstructured{}
+			u.SetName(name)
+			u.SetAnnotations(map[string]string{"nodewright.nvidia.com/version": "v0.15.0"})
+			mockNSRes.On("Get", mock.Anything, name, mock.Anything, mock.Anything).Return(u, nil)
+			mockNSRes.On("Patch", mock.Anything, name, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+				Return(&unstructured.Unstructured{}, nil).Maybe()
+			kubeClient = client.NewWithClientsAndConfig(fakeKube, mockDynamic, nil)
+			cliCtx = context.NewCLIContext(nil)
+			cmd = &cobra.Command{}
+			cmd.SetOut(&bytes.Buffer{})
+			cmd.SetErr(&bytes.Buffer{})
+
+			_, err := fakeKube.CoreV1().Nodes().Create(gocontext.Background(), &corev1.Node{
+				ObjectMeta: metav1.ObjectMeta{Name: "worker-1", Annotations: map[string]string{nodeStateAnnotationPrefix + name: "{}"}},
+			}, metav1.CreateOptions{})
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		patchedWith := func(body string) bool {
+			for _, call := range mockNSRes.Calls {
+				if call.Method == "Patch" && string(call.Arguments.Get(3).([]byte)) == body {
+					return true
+				}
+			}
+			return false
+		}
+
+		It("clears Status.ConfigUpdates on a full reset", func() {
+			opts := &resetOptions{confirm: true}
+			Expect(runReset(gocontext.Background(), cmd, kubeClient, name, opts, cliCtx)).To(Succeed())
+
+			Expect(patchedWith(clearPatch)).To(BeTrue())
+		})
+
+		It("clears it even when batch state is preserved", func() {
+			opts := &resetOptions{confirm: true, skipBatchReset: true}
+			Expect(runReset(gocontext.Background(), cmd, kubeClient, name, opts, cliCtx)).To(Succeed())
+
+			Expect(patchedWith(clearPatch)).To(BeTrue())
+		})
+
+		It("leaves it alone when resetting a node fails", func() {
+			fakeKube.PrependReactor("patch", "nodes", func(ktesting.Action) (bool, runtime.Object, error) {
+				return true, nil, fmt.Errorf("apiserver unreachable")
+			})
+			opts := &resetOptions{confirm: true}
+			Expect(runReset(gocontext.Background(), cmd, kubeClient, name, opts, cliCtx)).To(Succeed())
+
+			Expect(patchedWith(clearPatch)).To(BeFalse())
+		})
+
+		Describe("with --package", func() {
+			const pkgClearPatch = `{"status":{"configUpdates":{"pkg1":null}}}`
+
+			BeforeEach(func() {
+				_, err := fakeKube.CoreV1().Nodes().Update(gocontext.Background(), &corev1.Node{
+					ObjectMeta: metav1.ObjectMeta{Name: "worker-1", Annotations: map[string]string{
+						nodeStateAnnotationPrefix + name: `{"pkg1|1.0":{"name":"pkg1","version":"1.0","stage":"config","state":"complete"}}`,
+					}},
+				}, metav1.UpdateOptions{})
+				Expect(err).NotTo(HaveOccurred())
+			})
+
+			It("clears only that package's keys", func() {
+				opts := &resetOptions{confirm: true, pkg: "pkg1"}
+				Expect(runReset(gocontext.Background(), cmd, kubeClient, name, opts, cliCtx)).To(Succeed())
+
+				Expect(patchedWith(pkgClearPatch)).To(BeTrue())
+				Expect(patchedWith(clearPatch)).To(BeFalse())
+			})
+
+			It("leaves them alone when resetting a node fails", func() {
+				fakeKube.PrependReactor("patch", "nodes", func(ktesting.Action) (bool, runtime.Object, error) {
+					return true, nil, fmt.Errorf("apiserver unreachable")
+				})
+				opts := &resetOptions{confirm: true, pkg: "pkg1"}
+				Expect(runReset(gocontext.Background(), cmd, kubeClient, name, opts, cliCtx)).NotTo(Succeed())
+
+				Expect(patchedWith(pkgClearPatch)).To(BeFalse())
+			})
+
+			It("keeps them when a node still holds another version of the package", func() {
+				_, err := fakeKube.CoreV1().Nodes().Update(gocontext.Background(), &corev1.Node{
+					ObjectMeta: metav1.ObjectMeta{Name: "worker-1", Annotations: map[string]string{
+						nodeStateAnnotationPrefix + name: `{"pkg1|1.0":{"name":"pkg1","version":"1.0","stage":"config","state":"complete"},` +
+							`"pkg1|2.0":{"name":"pkg1","version":"2.0","stage":"config","state":"complete"}}`,
+					}},
+				}, metav1.UpdateOptions{})
+				Expect(err).NotTo(HaveOccurred())
+
+				opts := &resetOptions{confirm: true, pkg: "pkg1:2.0"}
+				Expect(runReset(gocontext.Background(), cmd, kubeClient, name, opts, cliCtx)).To(Succeed())
+
+				Expect(patchedWith(pkgClearPatch)).To(BeFalse())
+			})
+
+			It("keeps them when an untargeted node holds another version", func() {
+				_, err := fakeKube.CoreV1().Nodes().Create(gocontext.Background(), &corev1.Node{
+					ObjectMeta: metav1.ObjectMeta{Name: "worker-2", Annotations: map[string]string{
+						nodeStateAnnotationPrefix + name: `{"pkg1|2.0":{"name":"pkg1","version":"2.0","stage":"config","state":"complete"}}`,
+					}},
+				}, metav1.CreateOptions{})
+				Expect(err).NotTo(HaveOccurred())
+
+				opts := &resetOptions{confirm: true, pkg: "pkg1:1.0"}
+				Expect(runReset(gocontext.Background(), cmd, kubeClient, name, opts, cliCtx)).To(Succeed())
+
+				Expect(patchedWith(pkgClearPatch)).To(BeFalse())
+			})
+
+			It("clears them when a version-filtered reset leaves no other version", func() {
+				opts := &resetOptions{confirm: true, pkg: "pkg1:1.0"}
+				Expect(runReset(gocontext.Background(), cmd, kubeClient, name, opts, cliCtx)).To(Succeed())
+
+				Expect(patchedWith(pkgClearPatch)).To(BeTrue())
+			})
+
+			It("leaves them alone when no node holds the package", func() {
+				opts := &resetOptions{confirm: true, pkg: "other"}
+				Expect(runReset(gocontext.Background(), cmd, kubeClient, name, opts, cliCtx)).To(Succeed())
+
+				Expect(patchedWith(`{"status":{"configUpdates":{"other":null}}}`)).To(BeFalse())
+			})
+		})
+
+		It("does not warn when the NodeWright is already gone", func() {
+			mockNSRes.ExpectedCalls = nil
+			mockNSRes.On("Get", mock.Anything, name, mock.Anything, mock.Anything).Return(
+				(*unstructured.Unstructured)(nil),
+				apierrors.NewNotFound(schema.GroupResource{Group: "nodewright.nvidia.com", Resource: "nodewrights"}, name))
+			mockNSRes.On("Patch", mock.Anything, name, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+				Return((*unstructured.Unstructured)(nil),
+					apierrors.NewNotFound(schema.GroupResource{Group: "nodewright.nvidia.com", Resource: "nodewrights"}, name))
+			stderr := &bytes.Buffer{}
+			cmd.SetErr(stderr)
+
+			opts := &resetOptions{confirm: true, skipBatchReset: true}
+			Expect(runReset(gocontext.Background(), cmd, kubeClient, name, opts, cliCtx)).To(Succeed())
+
+			Expect(patchedWith(clearPatch)).To(BeTrue())
+			Expect(stderr.String()).NotTo(ContainSubstring("clear config updates"))
+		})
+
+		It("leaves it alone on a dry run", func() {
+			dryRunCtx := context.NewCLIContext(context.NewCLIConfig())
+			dryRunCtx.GlobalFlags.DryRun = true
+			opts := &resetOptions{confirm: true}
+			Expect(runReset(gocontext.Background(), cmd, kubeClient, name, opts, dryRunCtx)).To(Succeed())
+
+			Expect(patchedWith(clearPatch)).To(BeFalse())
+		})
+	})
+
 	Describe("--package", func() {
 		var (
 			fakeKube    *fake.Clientset
@@ -569,6 +744,8 @@ var _ = Describe("Reset Command", func() {
 			mockNSRes := &mockdynamic.NamespaceableResourceInterface{}
 			mockDynamic.On("Resource", skyhookGVR).Return(mockNSRes)
 			mockNSRes.On("Get", mock.Anything, name, mock.Anything, mock.Anything).Return(u, nil)
+			mockNSRes.On("Patch", mock.Anything, name, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+				Return(&unstructured.Unstructured{}, nil).Maybe()
 		}
 
 		addNode := func(name string, ns v1alpha1.NodeState) {