fix(operator): surface Blocked condition and event when non-interrupt…

chart/templates/nodewright-crd.yaml

@@ -588,8 +588,10 @@ spec:
                 description: Packages are the DAG of packages to be applied to nodes.
                 type: object
               podNonInterruptLabels:
-                description: PodNonInterruptLabels are a set of labels we want to
-                  monitor pods for whether they Interruptible
+                description: |-
+                  PodNonInterruptLabels are a set of labels we want to monitor pods for whether they are interruptible.
+                  Matching Pending or Running pods block pre-drain progress indefinitely; spec.drainConfig.timeout does not
+                  apply to this barrier. A Blocked condition with reason NonInterruptPodsRunning is surfaced while this barrier holds.
                 properties:
                   matchExpressions:
                     description: matchExpressions is a list of label selector requirements.

chart/templates/skyhook-crd.yaml

@@ -569,8 +569,10 @@ spec:
                 description: Packages are the DAG of packages to be applied to nodes.
                 type: object
               podNonInterruptLabels:
-                description: PodNonInterruptLabels are a set of labels we want to
-                  monitor pods for whether they Interruptible
+                description: |-
+                  PodNonInterruptLabels are a set of labels we want to monitor pods for whether they are interruptible.
+                  Matching Pending or Running pods block pre-drain progress indefinitely; spec.drainConfig.timeout does not
+                  apply to this barrier. A Blocked condition with reason NonInterruptPodsRunning is surfaced while this barrier holds.
                 properties:
                   matchExpressions:
                     description: matchExpressions is a list of label selector requirements.

docs/architecture/interrupt-flow.md

@@ -163,7 +163,12 @@ matching more closely: the unschedulable toleration check uses Kubernetes
 owner reference, and mirror/static pods are ignored.
 
 `podNonInterruptLabels` remains a pre-drain barrier. Matching pods must finish
-or move away before the operator starts the configurable drain step.
+or move away before the operator starts the configurable drain step. The node is cordoned
+before entering this barrier and remains cordoned throughout the wait so no replacement
+workloads can schedule on it. This wait is unbounded and drain timeout does not apply.
+While this barrier holds, administrators will see a `Blocked` condition with reason
+`NonInterruptPodsRunning` reporting which nodes are held, along with Warning events on
+both the NodeWright and affected Node objects detailing the hold.
 
 ### When Drain Is Complete
 

docs/architecture/operator-status.md

@@ -83,6 +83,9 @@ The truncation cap exists to keep condition payloads bounded for etcd object siz
 
 The operator also sets additional condition types that may be useful for troubleshooting:
 
+- `Blocked`: rollout progress is blocked on one or more nodes. Reasons include:
+  - `NonInterruptPodsRunning`: node drain is held because pods matching `spec.podNonInterruptLabels` are still running or pending on selected nodes
+  - `DependencyUninstalled`: a required package dependency is being or has been uninstalled
 - `TaintNotTolerable`: selected nodes are skipped because their taints are not tolerated by the NodeWright
 - `NodesIgnored`: selected nodes are skipped because they have the ignore label set
 - `ApplyPackage`: the controller is applying a package to a node

docs/user-guide/custom-resource.md

@@ -319,7 +319,7 @@ spec:
 
 This is a barrier that runs *before* the configurable drain — it is not a drain
 exclusion. The operator will wait indefinitely for a matching pod that never
-finishes, so pair it with `drainConfig.timeout` if you need a bound.
+finishes. This wait is unbounded and drain timeout does not apply.
 
 **Note the asymmetry with `nodeSelectors`:** an empty `podNonInterruptLabels` is
 special-cased to mean *no pods are protected*, not *all of them*. The two fields

operator/RELEASE_NOTES.md

@@ -7,6 +7,27 @@ For the full commit-level log see CHANGELOG.md.
 
 ### Bug Fixes
 
+- **A `Blocked` status condition (reason `NonInterruptPodsRunning`) and a Warning event are
+  now surfaced when `spec.podNonInterruptLabels` blocks node drain.** Previously,
+  the operator held the node in `Ready=False` / `Progressing` with no condition or
+  event indicating why or which pods were causing the hold. When nodes with packages
+  requiring interrupt are held at this barrier:
+  - A `Blocked` condition (status `True`, reason `NonInterruptPodsRunning`) is set on the
+    NodeWright once per reconcile pass. Its message identifies the blocked nodes using standard
+    node list formatting: up to 10 nodes (`wrapper.ReadyConditionNodeListLimit`) are listed by
+    name (e.g. `1 node blocked by non-interrupt pods (node-a). Waiting.` or `2 nodes blocked by non-interrupt pods (node-a, node-b). Waiting.`).
+    If more than 10 nodes are blocked, the full list is logged at info level once per pass and the
+    condition message is summarized with `(list truncated; see controller logs)`.
+  - A Warning event with reason `Drain` (`EventsReasonSkyhookDrain`) is emitted on the NodeWright
+    object on transition from unblocked to blocked, reporting the held nodes.
+  - A supplementary Warning event is also emitted on each affected Node object detailing the
+    namespace-qualified blocking pods and package being held.
+  - If another `Blocked` reason is already active (such as `DependencyUninstalled`), the
+    `NonInterruptPodsRunning` condition and NodeWright Warning event are deferred and will only appear
+    once that other condition clears. Once all matching non-interrupt pods finish or
+    terminate, the `NonInterruptPodsRunning` condition is removed and drain proceeds,
+    preserving any unrelated `Blocked` condition that may also be active.
+
 - **Adding and removing the finalizer from a natively authored NodeWright no
   longer rewrites its spec.** Both paths now use optimistic, metadata-only merge
   patches, preserving concurrent finalizer changes and user-authored resource

operator/api/nodewright/v1alpha1/nodewright_types.go

@@ -45,7 +45,9 @@ type NodeWrightSpec struct {
 	//+kubebuilder:default=false
 	Serial bool `json:"serial,omitempty"`
 
-	// PodNonInterruptLabels are a set of labels we want to monitor pods for whether they Interruptible
+	// PodNonInterruptLabels are a set of labels we want to monitor pods for whether they are interruptible.
+	// Matching Pending or Running pods block pre-drain progress indefinitely; spec.drainConfig.timeout does not
+	// apply to this barrier. A Blocked condition with reason NonInterruptPodsRunning is surfaced while this barrier holds.
 	PodNonInterruptLabels metav1.LabelSelector `json:"podNonInterruptLabels,omitempty"`
 
 	// NodeSelector are a set of labels we want to monitor nodes for applying packages too

operator/config/crd/bases/nodewright.nvidia.com_nodewrights.yaml

@@ -593,8 +593,10 @@ spec:
                 description: Packages are the DAG of packages to be applied to nodes.
                 type: object
               podNonInterruptLabels:
-                description: PodNonInterruptLabels are a set of labels we want to
-                  monitor pods for whether they Interruptible
+                description: |-
+                  PodNonInterruptLabels are a set of labels we want to monitor pods for whether they are interruptible.
+                  Matching Pending or Running pods block pre-drain progress indefinitely; spec.drainConfig.timeout does not
+                  apply to this barrier. A Blocked condition with reason NonInterruptPodsRunning is surfaced while this barrier holds.
                 properties:
                   matchExpressions:
                     description: matchExpressions is a list of label selector requirements.

operator/internal/controller/cluster_state_v2.go

@@ -31,6 +31,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
@@ -614,7 +615,12 @@ func (s *skyhookNodes) UpdateBlockedCondition() error {
 			Message:            strings.Join(blockedMsgs, "; "),
 		})
 	} else {
-		wrapper.RemoveSkyhookConditionTypes(s.skyhook, wrapper.SkyhookConditionBlocked)
+		// Preserving NonInterruptPodsRunning here allows updateDrainBlockedCondition to own
+		// and clear its own reason; clearing it here would clobber in-flight drain waits.
+		existing := meta.FindStatusCondition(s.skyhook.Status.Conditions, wrapper.SkyhookConditionBlocked)
+		if existing != nil && existing.Reason != wrapper.SkyhookReasonNonInterruptPodsRunning {
+			wrapper.RemoveSkyhookConditionTypes(s.skyhook, wrapper.SkyhookConditionBlocked)
+		}
 	}
 
 	return nil

operator/internal/controller/skyhook_controller.go

@@ -44,6 +44,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	policyv1 "k8s.io/api/policy/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -608,12 +609,123 @@ func (r *SkyhookReconciler) refreshSkyhookConditions(ctx context.Context, cluste
 	if err := skyhook.UpdateBlockedCondition(); err != nil {
 		return fmt.Errorf("error updating blocked condition: %w", err)
 	}
+	if err := r.updateDrainBlockedCondition(ctx, skyhook); err != nil {
+		return fmt.Errorf("error updating drain blocked condition: %w", err)
+	}
 	if err := skyhook.UpdateUninstallConditions(); err != nil {
 		return fmt.Errorf("error updating uninstall conditions: %w", err)
 	}
 	return nil
 }
 
+// nodeNeedsInterruptDrain reports whether the node has a runnable package with an interrupt
+// that is currently at the pre-drain apply or uninstall stage, matching ProcessInterrupt's entry gate.
+func nodeNeedsInterruptDrain(ctx context.Context, node wrapper.SkyhookNode) bool {
+	if node.IsComplete() {
+		return false
+	}
+	toRun, err := node.RunNext()
+	if err != nil {
+		logger := log.FromContext(ctx)
+		logger.Error(err, "error getting next packages to run", "node", node.GetNode().Name, "nodewright", node.GetSkyhook().Name)
+		return false
+	}
+	if len(toRun) == 0 {
+		return false
+	}
+	for _, pkg := range toRun {
+		if !node.HasInterrupt(*pkg) {
+			continue
+		}
+		stage := v1alpha1.StageApply
+		if nextStage := node.NextStage(pkg); nextStage != nil {
+			stage = *nextStage
+		}
+		if stage == v1alpha1.StageApply || stage == v1alpha1.StageUninstall {
+			return true
+		}
+	}
+	return false
+}
+
+// updateDrainBlockedCondition aggregates non-interrupt pod blocking state across all in-scope
+// nodes once per reconcile pass. It updates the NodeWright-level Blocked condition and emits a
+// Warning event on the NodeWright only on a genuine transition from unblocked to blocked.
+func (r *SkyhookReconciler) updateDrainBlockedCondition(ctx context.Context, skyhook SkyhookNodes) error {
+	logger := log.FromContext(ctx)
+
+	selector, err := metav1.LabelSelectorAsSelector(&skyhook.GetSkyhook().Spec.PodNonInterruptLabels)
+	if err != nil {
+		return fmt.Errorf("error creating selector: %w", err)
+	}
+	if selector.Empty() {
+		wrapper.RemoveSkyhookConditionTypeAndReason(skyhook.GetSkyhook(), wrapper.SkyhookConditionBlocked, wrapper.SkyhookReasonNonInterruptPodsRunning)
+		return nil
+	}
+
+	// Defer NonInterruptPodsRunning if another Blocked reason (e.g. DependencyUninstalled)
+	// currently holds the single Blocked condition slot on the NodeWright.
+	existing := meta.FindStatusCondition(skyhook.GetSkyhook().Status.Conditions, wrapper.SkyhookConditionBlocked)
+	if existing != nil && existing.Reason != wrapper.SkyhookReasonNonInterruptPodsRunning {
+		return nil
+	}
+
+	var blockedNodes []string
+	for _, node := range skyhook.GetNodes() {
+		if !nodeNeedsInterruptDrain(ctx, node) {
+			continue
+		}
+
+		hasWork, _, err := r.HasNonInterruptWork(ctx, node)
+		if err != nil {
+			return fmt.Errorf("checking non-interrupt work for node [%s]: %w", node.GetNode().Name, err)
+		}
+		if hasWork {
+			blockedNodes = append(blockedNodes, node.GetNode().Name)
+		}
+	}
+
+	if len(blockedNodes) > 0 {
+		sort.Strings(blockedNodes)
+
+		if len(blockedNodes) > wrapper.ReadyConditionNodeListLimit {
+			logger.Info("Condition message truncated for non-interrupt pods", "nodewright", skyhook.GetSkyhook().Name, "nodes", blockedNodes)
+		}
+
+		nodeLabel := "node"
+		if len(blockedNodes) > 1 {
+			nodeLabel = "nodes"
+		}
+		message := fmt.Sprintf("%d %s blocked by non-interrupt pods%s. Waiting.",
+			len(blockedNodes),
+			nodeLabel,
+			wrapper.FormatNodeList(blockedNodes),
+		)
+
+		if existing == nil {
+			r.recorder.Eventf(skyhook.GetSkyhook().NodeWright, nil, corev1.EventTypeWarning, EventsReasonSkyhookDrain, wrapper.SkyhookReasonNonInterruptPodsRunning,
+				"drain blocked by non-interrupt pods on %d %s%s",
+				len(blockedNodes),
+				nodeLabel,
+				wrapper.FormatNodeList(blockedNodes),
+			)
+		}
+
+		wrapper.AddSkyhookCondition(skyhook.GetSkyhook(), metav1.Condition{
+			Type:               wrapper.SkyhookConditionBlocked,
+			Status:             metav1.ConditionTrue,
+			Reason:             wrapper.SkyhookReasonNonInterruptPodsRunning,
+			Message:            message,
+			ObservedGeneration: skyhook.GetSkyhook().Generation,
+			LastTransitionTime: metav1.Now(),
+		})
+	} else if existing != nil && existing.Reason == wrapper.SkyhookReasonNonInterruptPodsRunning {
+		wrapper.RemoveSkyhookConditionTypeAndReason(skyhook.GetSkyhook(), wrapper.SkyhookConditionBlocked, wrapper.SkyhookReasonNonInterruptPodsRunning)
+	}
+
+	return nil
+}
+
 // processSkyhooksPerNode processes all skyhooks for nodes that are ready (per-node priority ordering).
 // A node is ready for a skyhook if all higher-priority skyhooks are complete on that specific node.
 func (r *SkyhookReconciler) processSkyhooksPerNode(ctx context.Context, clusterState *clusterState, nodePicker *NodePicker, logger logr.Logger) (*ctrl.Result, error) {
@@ -2410,16 +2522,16 @@ func (r *SkyhookReconciler) HandleFinalizer(ctx context.Context, skyhook Skyhook
 	return false, nil
 }
 
-// HasNonInterruptWork returns true if pods are running on the node that are either packages, or matches the SCR selector
-func (r *SkyhookReconciler) HasNonInterruptWork(ctx context.Context, skyhookNode wrapper.SkyhookNode) (bool, error) {
+// HasNonInterruptWork returns true and the list of running or pending pod names if pods are running on the node that match the SCR selector
+func (r *SkyhookReconciler) HasNonInterruptWork(ctx context.Context, skyhookNode wrapper.SkyhookNode) (bool, []string, error) {
 
 	selector, err := metav1.LabelSelectorAsSelector(&skyhookNode.GetSkyhook().Spec.PodNonInterruptLabels)
 	if err != nil {
-		return false, fmt.Errorf("error creating selector: %w", err)
+		return false, nil, fmt.Errorf("error creating selector: %w", err)
 	}
 
 	if selector.Empty() { // when selector is empty it does not do any selecting, ie will return all pods on node.
-		return false, nil
+		return false, nil, nil
 	}
 
 	pods, err := r.dal.GetPods(ctx,
@@ -2429,21 +2541,31 @@ func (r *SkyhookReconciler) HasNonInterruptWork(ctx context.Context, skyhookNode
 		},
 	)
 	if err != nil {
-		return false, fmt.Errorf("error getting pods: %w", err)
+		return false, nil, fmt.Errorf("error getting pods: %w", err)
 	}
 
 	if pods == nil || len(pods.Items) == 0 {
-		return false, nil
+		return false, nil, nil
 	}
 
+	var podNames []string
 	for _, pod := range pods.Items {
 		switch pod.Status.Phase {
 		case corev1.PodRunning, corev1.PodPending:
-			return true, nil
+			podName := pod.Name
+			if pod.Namespace != "" {
+				podName = fmt.Sprintf("%s/%s", pod.Namespace, pod.Name)
+			}
+			podNames = append(podNames, podName)
 		}
 	}
 
-	return false, nil
+	if len(podNames) > 0 {
+		sort.Strings(podNames)
+		return true, podNames, nil
+	}
+
+	return false, nil, nil
 }
 
 func (r *SkyhookReconciler) HasRunningPackages(ctx context.Context, skyhookNode wrapper.SkyhookNode) (bool, error) {
@@ -3258,11 +3380,27 @@ func (r *SkyhookReconciler) EnsureNodeIsReadyForInterrupt(ctx context.Context, s
 		return false, nil
 	}
 
-	hasWork, err := r.HasNonInterruptWork(ctx, skyhookNode)
+	hasWork, podNames, err := r.HasNonInterruptWork(ctx, skyhookNode)
 	if err != nil {
 		return false, err
 	}
 	if hasWork { // keep waiting...
+		displayPods := podNames
+		if len(podNames) > wrapper.ReadyConditionNodeListLimit {
+			logger := log.FromContext(ctx)
+			logger.Info("Event message truncated for non-interrupt pods", "node", skyhookNode.GetNode().Name, "nodewright", skyhookNode.GetSkyhook().Name, "pods", podNames)
+			displayPods = podNames[:wrapper.ReadyConditionNodeListLimit]
+		}
+		// Condition and NodeWright-level event are managed once per reconcile pass by
+		// updateDrainBlockedCondition. Here we emit a supplementary event on the Node itself
+		// (matching DrainTimeout behavior) so node inspection reflects why drain is waiting.
+		r.recorder.Eventf(skyhookNode.GetNode(), nil, corev1.EventTypeWarning, EventsReasonSkyhookDrain, wrapper.SkyhookReasonNonInterruptPodsRunning,
+			"drain blocked by non-interrupt pods [%s] for package [%s:%s] from [nodewright:%s]",
+			strings.Join(displayPods, ", "),
+			_package.Name,
+			_package.Version,
+			skyhookNode.GetSkyhook().Name,
+		)
 		return false, nil
 	}