Skip to content

Commit 3a85e9e

Browse files
cdesiniotiscdesiniotis
committed
chore: consolidate clusterpolicy validation code
Signed-off-by: Christopher Desiniotis <cdesiniotis@nvidia.com>
1 parent 2082e91 commit 3a85e9e

6 files changed

Lines changed: 141 additions & 81 deletions

File tree

‎controllers/clusterpolicy_validator.go‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,23 +23,33 @@ import (
2323
)
2424

2525
func (n *ClusterPolicyController) validateClusterPolicy() error {
26-
err := validateDRA(n.singleton, n.draSupported)
27-
if err != nil {
26+
if err := validateDRA(&n.singleton.Spec, n.draSupported); err != nil {
2827
return fmt.Errorf("failed to validate DRA: %w", err)
2928
}
29+
30+
if err := validateNRIPlugin(&n.singleton.Spec); err != nil {
31+
return fmt.Errorf("failed to validate the NRI Plugin: %w", err)
32+
}
33+
return nil
34+
}
35+
36+
func validateNRIPlugin(spec *gpuv1.ClusterPolicySpec) error {
37+
if !spec.CDI.IsEnabled() && spec.CDI.IsNRIPluginEnabled() {
38+
return fmt.Errorf("the NRI Plugin cannot be enabled when CDI is disabled")
39+
}
3040
return nil
3141
}
3242

33-
func validateDRA(clusterpolicy *gpuv1.ClusterPolicy, draSupported bool) error {
34-
if !draSupported && clusterpolicy.Spec.DRADriver.IsEnabled() {
43+
func validateDRA(spec *gpuv1.ClusterPolicySpec, draSupported bool) error {
44+
if !draSupported && spec.DRADriver.IsEnabled() {
3545
return fmt.Errorf("the NVIDIA DRA driver for GPUs is enabled in ClusterPolicy but Dynamic Resource Allocation is not enabled in the Kubernetes cluster")
3646
}
3747

38-
if clusterpolicy.Spec.DevicePlugin.IsEnabled() && clusterpolicy.Spec.DRADriver.IsGPUsEnabled() {
48+
if spec.DevicePlugin.IsEnabled() && spec.DRADriver.IsGPUsEnabled() {
3949
return fmt.Errorf("the NVIDIA device plugin and the NVIDIA DRA driver for GPUs cannot both be enabled in ClusterPolicy")
4050
}
4151

42-
if clusterpolicy.Spec.SandboxWorkloads.IsEnabled() && clusterpolicy.Spec.DRADriver.IsEnabled() {
52+
if spec.SandboxWorkloads.IsEnabled() && spec.DRADriver.IsEnabled() {
4353
return fmt.Errorf("sandboxWorkloads and the NVIDIA DRA driver for GPUs cannot both be enabled in ClusterPolicy")
4454
}
4555

‎controllers/clusterpolicy_validator_test.go‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
/*
2+
* Copyright (c) NVIDIA CORPORATION. All rights reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package controllers
18+
19+
import (
20+
"errors"
21+
"testing"
22+
23+
"github.com/stretchr/testify/require"
24+
"k8s.io/utils/ptr"
25+
26+
gpuv1 "github.com/NVIDIA/gpu-operator/api/nvidia/v1"
27+
)
28+
29+
func TestValidateDRA(t *testing.T) {
30+
tests := []struct {
31+
description string
32+
spec *gpuv1.ClusterPolicySpec
33+
draSupported bool
34+
err error
35+
}{
36+
{
37+
description: "dra not supported, dra driver not enabled",
38+
spec: &gpuv1.ClusterPolicySpec{},
39+
},
40+
{
41+
description: "dra not supported, dra driver enabled",
42+
spec: &gpuv1.ClusterPolicySpec{
43+
DRADriver: gpuv1.DRADriverSpec{
44+
GPUs: gpuv1.DRADriverGPUs{
45+
Enabled: ptr.To(true),
46+
},
47+
},
48+
},
49+
err: errors.New("the NVIDIA DRA driver for GPUs is enabled in ClusterPolicy but Dynamic Resource Allocation is not enabled in the Kubernetes cluster"),
50+
},
51+
}
52+
53+
for _, tc := range tests {
54+
t.Run(tc.description, func(t *testing.T) {
55+
err := validateDRA(tc.spec, tc.draSupported)
56+
if tc.err == nil {
57+
require.NoError(t, err)
58+
} else {
59+
require.Error(t, err)
60+
require.Equal(t, tc.err.Error(), err.Error())
61+
}
62+
})
63+
}
64+
}
65+
66+
func TestValidateNRIPlugin(t *testing.T) {
67+
tests := []struct {
68+
description string
69+
spec *gpuv1.ClusterPolicySpec
70+
err error
71+
}{
72+
{
73+
description: "valid CDI object in spec",
74+
spec: &gpuv1.ClusterPolicySpec{
75+
CDI: gpuv1.CDIConfigSpec{
76+
Enabled: ptr.To(true),
77+
NRIPluginEnabled: ptr.To(true),
78+
},
79+
},
80+
},
81+
{
82+
description: "invalid CDI object in spec",
83+
spec: &gpuv1.ClusterPolicySpec{
84+
CDI: gpuv1.CDIConfigSpec{
85+
Enabled: ptr.To(false),
86+
NRIPluginEnabled: ptr.To(true),
87+
},
88+
},
89+
err: errors.New("the NRI Plugin cannot be enabled when CDI is disabled"),
90+
},
91+
}
92+
93+
for _, tc := range tests {
94+
t.Run(tc.description, func(t *testing.T) {
95+
err := validateNRIPlugin(tc.spec)
96+
if tc.err == nil {
97+
require.NoError(t, err)
98+
} else {
99+
require.Error(t, err)
100+
require.Equal(t, tc.err.Error(), err.Error())
101+
}
102+
})
103+
}
104+
}

‎controllers/state_manager.go‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -947,11 +947,6 @@ func (n *ClusterPolicyController) init(ctx context.Context, reconciler *ClusterP
947947
n.k8sVersion = k8sVersion
948948
n.logger.Info("Kubernetes version detected", "version", k8sVersion)
949949

950-
err = validateClusterPolicySpec(&clusterPolicy.Spec)
951-
if err != nil {
952-
return fmt.Errorf("error validating clusterpolicy: %w", err)
953-
}
954-
955950
draSupported, resourceGVR, err := IsDRASupported(n.logger)
956951
if err != nil {
957952
return fmt.Errorf("failed to detect if DRA is supported: %w", err)
@@ -988,8 +983,6 @@ func (n *ClusterPolicyController) init(ctx context.Context, reconciler *ClusterP
988983
}
989984
}
990985

991-
// TODO: combine this validation logic with the call to
992-
// ValidateClusterPolicySpec() up above
993986
err = n.validateClusterPolicy()
994987
if err != nil {
995988
return fmt.Errorf("ClusterPolicy validation failed: %w", err)
@@ -1234,10 +1227,3 @@ func (n ClusterPolicyController) isStateEnabled(stateName string) bool {
12341227
return false
12351228
}
12361229
}
1237-
1238-
func validateClusterPolicySpec(spec *gpuv1.ClusterPolicySpec) error {
1239-
if !spec.CDI.IsEnabled() && spec.CDI.IsNRIPluginEnabled() {
1240-
return fmt.Errorf("the NRI Plugin cannot be enabled when CDI is disabled")
1241-
}
1242-
return nil
1243-
}

‎controllers/state_manager_test.go‎

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package controllers
1818

1919
import (
2020
"context"
21-
"errors"
2221
"testing"
2322

2423
"github.com/stretchr/testify/require"
@@ -293,46 +292,6 @@ func TestHasMIGCapableGPU(t *testing.T) {
293292
}
294293
}
295294

296-
func TestValidateClusterPolicySpec(t *testing.T) {
297-
tests := []struct {
298-
description string
299-
spec *gpuv1.ClusterPolicySpec
300-
err error
301-
}{
302-
{
303-
description: "valid CDI object in spec",
304-
spec: &gpuv1.ClusterPolicySpec{
305-
CDI: gpuv1.CDIConfigSpec{
306-
Enabled: ptr.To(true),
307-
NRIPluginEnabled: ptr.To(true),
308-
},
309-
},
310-
},
311-
{
312-
description: "invalid CDI object in spec",
313-
spec: &gpuv1.ClusterPolicySpec{
314-
CDI: gpuv1.CDIConfigSpec{
315-
Enabled: ptr.To(false),
316-
NRIPluginEnabled: ptr.To(true),
317-
},
318-
},
319-
err: errors.New("the NRI Plugin cannot be enabled when CDI is disabled"),
320-
},
321-
}
322-
323-
for _, tc := range tests {
324-
t.Run(tc.description, func(t *testing.T) {
325-
err := validateClusterPolicySpec(tc.spec)
326-
if tc.err == nil {
327-
require.NoError(t, err)
328-
} else {
329-
require.Error(t, err)
330-
require.Equal(t, tc.err.Error(), err.Error())
331-
}
332-
})
333-
}
334-
}
335-
336295
func TestGetEffectiveStateLabels(t *testing.T) {
337296
// getEffectiveStateLabels returns labels for workload config and sandbox mode.
338297
// For container and vm-vgpu, mode has no effect. For vm-passthrough, mode selects

‎deployments/gpu-operator/templates/validation.yaml‎

Lines changed: 0 additions & 20 deletions
This file was deleted.

‎deployments/gpu-operator/templates/validations.yaml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,24 @@
11
{{- if and (eq .Values.cdi.enabled false) (eq .Values.cdi.nriPluginEnabled true) }}
22
{{ fail "the NRI Plugin cannot be enabled when CDI is disabled" }}
33
{{- end}}
4+
5+
{{- $draEnabled := or (eq .Values.draDriver.gpus.enabled true) (eq .Values.draDriver.computeDomains.enabled true) }}
6+
{{- $clusterSupportsDRA := or (.Capabilities.APIVersions.Has "resource.k8s.io/v1beta1/DeviceClass") (.Capabilities.APIVersions.Has "resource.k8s.io/v1beta2/DeviceClass") }}
7+
8+
{{- if and (eq .Values.devicePlugin.enabled true) (eq .Values.draDriver.gpus.enabled true) }}
9+
{{- $error := "" }}
10+
{{- $error = printf "%s\nThe NVIDIA device plugin and the NVIDIA DRA Driver for GPUs cannot both be enabled" $error }}
11+
{{- fail $error }}
12+
{{- end}}
13+
14+
{{- if and ($draEnabled) (not $clusterSupportsDRA) }}
15+
{{- $error := "" }}
16+
{{- $error = printf "%s\nCannot enable the NVIDIA DRA Driver for GPUs on a Kubernetes cluster that does not support DRA" $error }}
17+
{{- fail $error }}
18+
{{- end}}
19+
20+
{{- if and ($draEnabled) (eq .Values.sandboxWorkloads.enabled true) }}
21+
{{- $error := "" }}
22+
{{- $error = printf "%s\nThe NVIDIA DRA Driver for GPUs and 'sandboxWorkloads' cannot both be enabled" $error }}
23+
{{- fail $error }}
24+
{{- end}}

0 commit comments

Comments
 (0)