Skip to content

Commit b0249cf

Browse files
committed
fix(controller): do not apply driver upgrade annotation when driver is disabled
The applyDriverAutoUpgradeAnnotation() function was applying the nvidia.com/gpu-driver-upgrade-enabled annotation to GPU nodes even when driver.enabled=false. This occurred because the function only checked if driver.upgradePolicy.autoUpgrade was true, without verifying that the driver component itself was enabled. This fix adds a check for Driver.IsEnabled() before applying the annotation, ensuring it is only set when: 1. Driver is enabled 2. Auto-upgrade policy exists and is enabled 3. Sandbox workloads are disabled Added unit tests to validate the fix and prevent regression. Fixes #1277 Signed-off-by: Anuj Dube <[email protected]>
1 parent c476d34 commit b0249cf

File tree

2 files changed

+70
-1
lines changed

2 files changed

+70
-1
lines changed

controllers/state_manager.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,8 @@ func (n *ClusterPolicyController) applyDriverAutoUpgradeAnnotation() error {
438438
updateRequired := false
439439
value := "true"
440440
annotationValue, annotationExists := node.Annotations[driverAutoUpgradeAnnotationKey]
441-
if n.singleton.Spec.Driver.UpgradePolicy != nil &&
441+
if n.singleton.Spec.Driver.IsEnabled() &&
442+
n.singleton.Spec.Driver.UpgradePolicy != nil &&
442443
n.singleton.Spec.Driver.UpgradePolicy.AutoUpgrade &&
443444
!n.sandboxEnabled {
444445
// check if we need to add the annotation

controllers/state_manager_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package controllers
1919
import (
2020
"testing"
2121

22+
upgrade_v1alpha1 "github.com/NVIDIA/k8s-operator-libs/api/upgrade/v1alpha1"
2223
corev1 "k8s.io/api/core/v1"
2324

2425
gpuv1 "github.com/NVIDIA/gpu-operator/api/nvidia/v1"
@@ -186,3 +187,70 @@ func TestHasMIGCapableGPU(t *testing.T) {
186187
}
187188
}
188189
}
190+
191+
func TestShouldApplyDriverAutoUpgradeAnnotation(t *testing.T) {
192+
tests := []struct {
193+
name string
194+
driverEnabled bool
195+
autoUpgradeEnabled bool
196+
sandboxEnabled bool
197+
wantAnnotation bool
198+
}{
199+
{
200+
name: "driver enabled with auto-upgrade",
201+
driverEnabled: true,
202+
autoUpgradeEnabled: true,
203+
sandboxEnabled: false,
204+
wantAnnotation: true,
205+
},
206+
{
207+
name: "driver disabled with auto-upgrade enabled - should NOT apply annotation",
208+
driverEnabled: false,
209+
autoUpgradeEnabled: true,
210+
sandboxEnabled: false,
211+
wantAnnotation: false,
212+
},
213+
{
214+
name: "driver enabled but auto-upgrade disabled",
215+
driverEnabled: true,
216+
autoUpgradeEnabled: false,
217+
sandboxEnabled: false,
218+
wantAnnotation: false,
219+
},
220+
{
221+
name: "driver enabled with auto-upgrade but sandbox enabled",
222+
driverEnabled: true,
223+
autoUpgradeEnabled: true,
224+
sandboxEnabled: true,
225+
wantAnnotation: false,
226+
},
227+
{
228+
name: "all disabled",
229+
driverEnabled: false,
230+
autoUpgradeEnabled: false,
231+
sandboxEnabled: false,
232+
wantAnnotation: false,
233+
},
234+
}
235+
236+
for _, tc := range tests {
237+
t.Run(tc.name, func(t *testing.T) {
238+
// Create a mock ClusterPolicy
239+
clusterPolicy := &gpuv1.ClusterPolicy{}
240+
clusterPolicy.Spec.Driver.Enabled = &tc.driverEnabled
241+
clusterPolicy.Spec.Driver.UpgradePolicy = &upgrade_v1alpha1.DriverUpgradePolicySpec{
242+
AutoUpgrade: tc.autoUpgradeEnabled,
243+
}
244+
245+
// Simulate the logic from applyDriverAutoUpgradeAnnotation
246+
shouldApply := clusterPolicy.Spec.Driver.IsEnabled() &&
247+
clusterPolicy.Spec.Driver.UpgradePolicy != nil &&
248+
clusterPolicy.Spec.Driver.UpgradePolicy.AutoUpgrade &&
249+
!tc.sandboxEnabled
250+
251+
if shouldApply != tc.wantAnnotation {
252+
t.Errorf("Expected annotation to be applied: %v, but got: %v", tc.wantAnnotation, shouldApply)
253+
}
254+
})
255+
}
256+
}

0 commit comments

Comments
 (0)