From ccffb9267b2807cd9b3721f6e55ebcf2ca395b3b Mon Sep 17 00:00:00 2001 From: Rahul Sharma Date: Fri, 24 Jul 2026 10:01:39 -0700 Subject: [PATCH] fix clusterpolicy fluctuation when nvidiadriver upgrade is happening Signed-off-by: Rahul Sharma --- controllers/clusterpolicy_controller.go | 70 ++++++- controllers/clusterpolicy_controller_test.go | 202 +++++++++++++++++++ 2 files changed, 271 insertions(+), 1 deletion(-) create mode 100644 controllers/clusterpolicy_controller_test.go diff --git a/controllers/clusterpolicy_controller.go b/controllers/clusterpolicy_controller.go index 10f13e9a1..0878114f5 100644 --- a/controllers/clusterpolicy_controller.go +++ b/controllers/clusterpolicy_controller.go @@ -19,7 +19,9 @@ package controllers import ( "context" "fmt" + "strings" + "github.com/NVIDIA/k8s-operator-libs/pkg/upgrade" "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" @@ -146,6 +148,7 @@ func (r *ClusterPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reques clusterPolicyCtrl.operatorMetrics.reconciliationTotal.Inc() overallStatus := gpuv1.Ready statesNotReady := []string{} + notReadyReasons := []string{} for { status, statusError := clusterPolicyCtrl.step() if statusError != nil { @@ -171,12 +174,31 @@ func (r *ClusterPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reques } } + if clusterPolicyCtrl.singleton.Spec.Driver.UseNvidiaDriverCRDType() { + upgradeInProgress, err := r.nvidiaDriverUpgradeIncomplete(ctx) + if err != nil { + clusterPolicyCtrl.operatorMetrics.reconciliationStatus.Set(reconciliationStatusNotReady) + clusterPolicyCtrl.operatorMetrics.reconciliationFailed.Inc() + if condErr := r.conditionUpdater.SetConditionsError(ctx, instance, conditions.ReconcileFailed, fmt.Sprintf("Failed to determine NVIDIADriver upgrade state: %s", err)); condErr != nil { + r.Log.Error(condErr, "failed to set condition") + } + return ctrl.Result{}, err + } + if upgradeInProgress { + overallStatus = gpuv1.NotReady + notReadyReasons = append(notReadyReasons, + "NVIDIADriver upgrade has not completed", + "one or more NVIDIADriver-owned Nodes are marked pending, in-progress, or failed", + ) + } + } + // if any state is not ready, requeue for reconcile after 5 seconds if overallStatus != gpuv1.Ready { clusterPolicyCtrl.operatorMetrics.reconciliationStatus.Set(reconciliationStatusNotReady) clusterPolicyCtrl.operatorMetrics.reconciliationFailed.Inc() - err := fmt.Errorf("ClusterPolicy is not ready, states not ready: %v", statesNotReady) + err := fmt.Errorf("%s", clusterPolicyNotReadyMessage(statesNotReady, notReadyReasons)) r.Log.Error(err, "ClusterPolicy not yet ready") updateCRState(ctx, r, req.NamespacedName, gpuv1.NotReady) if condErr := r.conditionUpdater.SetConditionsError(ctx, instance, conditions.OperandNotReady, err.Error()); condErr != nil { @@ -227,6 +249,52 @@ func (r *ClusterPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reques return ctrl.Result{}, nil } +// clusterPolicyNotReadyMessage formats a condition message with operand states and additional not-ready reasons. +func clusterPolicyNotReadyMessage(statesNotReady, notReadyReasons []string) string { + messageParts := []string{"ClusterPolicy is not ready"} + if len(statesNotReady) > 0 { + messageParts = append(messageParts, fmt.Sprintf("states not ready: %v", statesNotReady)) + } + messageParts = append(messageParts, notReadyReasons...) + return strings.Join(messageParts, "; ") +} + +// nvidiaDriverUpgradeIncomplete reports whether any NVIDIADriver-owned Node has a pending, active, or failed upgrade. +func (r *ClusterPolicyReconciler) nvidiaDriverUpgradeIncomplete(ctx context.Context) (bool, error) { + nodes := &corev1.NodeList{} + if err := r.List(ctx, nodes, client.HasLabels{consts.NVIDIADriverOwnerLabel}); err != nil { + return false, fmt.Errorf("failed to list nodes for NVIDIADriver upgrade state: %w", err) + } + + for _, node := range nodes.Items { + if isIncompleteDriverUpgradeState(node.Labels[upgrade.GetUpgradeStateLabelKey()]) { + return true, nil + } + } + + return false, nil +} + +// isIncompleteDriverUpgradeState reports whether a Node upgrade state keeps the aggregate driver rollout incomplete. +func isIncompleteDriverUpgradeState(state string) bool { + switch state { + case upgrade.UpgradeStateUpgradeRequired, + upgrade.UpgradeStateCordonRequired, + upgrade.UpgradeStateWaitForJobsRequired, + upgrade.UpgradeStatePodDeletionRequired, + upgrade.UpgradeStateDrainRequired, + upgrade.UpgradeStateNodeMaintenanceRequired, + upgrade.UpgradeStatePostMaintenanceRequired, + upgrade.UpgradeStatePodRestartRequired, + upgrade.UpgradeStateValidationRequired, + upgrade.UpgradeStateUncordonRequired, + upgrade.UpgradeStateFailed: + return true + default: + return false + } +} + func updateCRState(ctx context.Context, r *ClusterPolicyReconciler, namespacedName types.NamespacedName, state gpuv1.State) { // Fetch latest instance and update state to avoid version mismatch instance := &gpuv1.ClusterPolicy{} diff --git a/controllers/clusterpolicy_controller_test.go b/controllers/clusterpolicy_controller_test.go new file mode 100644 index 000000000..ecf0f2e9f --- /dev/null +++ b/controllers/clusterpolicy_controller_test.go @@ -0,0 +1,202 @@ +/** +# Copyright (c) NVIDIA CORPORATION. All rights reserved. +# +# 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. +**/ + +package controllers + +import ( + "context" + "testing" + + "github.com/NVIDIA/k8s-operator-libs/pkg/upgrade" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + gpuconsts "github.com/NVIDIA/gpu-operator/internal/consts" +) + +func TestIsIncompleteDriverUpgradeState(t *testing.T) { + tests := []struct { + name string + state string + expected bool + }{ + { + name: "unknown state is inactive", + state: upgrade.UpgradeStateUnknown, + expected: false, + }, + { + name: "upgrade required is pending and in progress", + state: upgrade.UpgradeStateUpgradeRequired, + expected: true, + }, + { + name: "done is inactive", + state: upgrade.UpgradeStateDone, + expected: false, + }, + { + name: "failed is incomplete", + state: upgrade.UpgradeStateFailed, + expected: true, + }, + { + name: "pod restart required is active", + state: upgrade.UpgradeStatePodRestartRequired, + expected: true, + }, + { + name: "uncordon required is active", + state: upgrade.UpgradeStateUncordonRequired, + expected: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.expected, isIncompleteDriverUpgradeState(tc.state)) + }) + } +} + +func TestClusterPolicyNotReadyMessage(t *testing.T) { + tests := []struct { + name string + statesNotReady []string + notReadyReasons []string + expected string + }{ + { + name: "driver upgrade only", + notReadyReasons: []string{ + "NVIDIADriver upgrade has not completed", + "one or more NVIDIADriver-owned Nodes are marked pending, in-progress, or failed", + }, + expected: "ClusterPolicy is not ready; NVIDIADriver upgrade has not completed; one or more NVIDIADriver-owned Nodes are marked pending, in-progress, or failed", + }, + { + name: "not ready states and driver upgrade", + statesNotReady: []string{"state-container-toolkit", "state-device-plugin"}, + notReadyReasons: []string{ + "NVIDIADriver upgrade has not completed", + "one or more NVIDIADriver-owned Nodes are marked pending, in-progress, or failed", + }, + expected: "ClusterPolicy is not ready; states not ready: [state-container-toolkit state-device-plugin]; NVIDIADriver upgrade has not completed; one or more NVIDIADriver-owned Nodes are marked pending, in-progress, or failed", + }, + { + name: "not ready states only", + statesNotReady: []string{"state-container-toolkit"}, + expected: "ClusterPolicy is not ready; states not ready: [state-container-toolkit]", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.expected, clusterPolicyNotReadyMessage(tc.statesNotReady, tc.notReadyReasons)) + }) + } +} + +func TestNVIDIADriverUpgradeIncomplete(t *testing.T) { + upgradeStateLabel := upgrade.GetUpgradeStateLabelKey() + + tests := []struct { + name string + nodes []client.Object + expected bool + }{ + { + name: "active upgrade state on NVIDIADriver-owned node", + nodes: []client.Object{ + nodeWithLabels("gpu-node", map[string]string{ + gpuconsts.NVIDIADriverOwnerLabel: "default", + upgradeStateLabel: upgrade.UpgradeStatePodRestartRequired, + }), + }, + expected: true, + }, + { + name: "pending upgrade keeps rollout in progress after another node completes", + nodes: []client.Object{ + nodeWithLabels("upgraded-gpu-node", map[string]string{ + gpuconsts.NVIDIADriverOwnerLabel: "default", + upgradeStateLabel: upgrade.UpgradeStateDone, + }), + nodeWithLabels("pending-gpu-node", map[string]string{ + gpuconsts.NVIDIADriverOwnerLabel: "default", + upgradeStateLabel: upgrade.UpgradeStateUpgradeRequired, + }), + }, + expected: true, + }, + { + name: "active upgrade state on unowned node is ignored", + nodes: []client.Object{ + nodeWithLabels("gpu-node", map[string]string{ + upgradeStateLabel: upgrade.UpgradeStatePodRestartRequired, + }), + }, + expected: false, + }, + { + name: "failed upgrade state keeps rollout incomplete", + nodes: []client.Object{ + nodeWithLabels("gpu-node", map[string]string{ + gpuconsts.NVIDIADriverOwnerLabel: "default", + upgradeStateLabel: upgrade.UpgradeStateFailed, + }), + }, + expected: true, + }, + { + name: "completed upgrade state is not treated as in progress", + nodes: []client.Object{ + nodeWithLabels("gpu-node", map[string]string{ + gpuconsts.NVIDIADriverOwnerLabel: "default", + upgradeStateLabel: upgrade.UpgradeStateDone, + }), + }, + expected: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, corev1.AddToScheme(scheme)) + reconciler := &ClusterPolicyReconciler{ + Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(tc.nodes...).Build(), + } + + actual, err := reconciler.nvidiaDriverUpgradeIncomplete(context.Background()) + require.NoError(t, err) + require.Equal(t, tc.expected, actual) + }) + } +} + +func nodeWithLabels(name string, labels map[string]string) *corev1.Node { + return &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: labels, + }, + } +}