Skip to content

Commit e466ce3

Browse files
Fix gpuop-cfg panic when validating a ClusterPolicy with no GDS configuration (#2792)
Fix gpuop-cfg panic when validating a ClusterPolicy with no GDS configuration --------- Signed-off-by: Karthikeyan Valliyurnatt <kvalliyurnat@nvidia.com>
1 parent 98a3a22 commit e466ce3

3 files changed

Lines changed: 37 additions & 13 deletions

File tree

api/nvidia/v1/clusterpolicy_types.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package v1
1919
import (
2020
"fmt"
2121
"os"
22+
"reflect"
2223
"strings"
2324

2425
kata_v1alpha1 "github.com/NVIDIA/k8s-kata-manager/api/v1alpha1/config"
@@ -2087,6 +2088,14 @@ func imagePath(repository string, image string, version string, imagePathEnvName
20872088

20882089
// ImagePath sets image path for given component type
20892090
func ImagePath(spec interface{}) (string, error) {
2091+
if spec == nil {
2092+
return "", fmt.Errorf("invalid nil spec to construct image path")
2093+
}
2094+
value := reflect.ValueOf(spec)
2095+
if value.Kind() == reflect.Pointer && value.IsNil() {
2096+
return "", fmt.Errorf("invalid nil spec to construct image path: %T", spec)
2097+
}
2098+
20902099
switch v := spec.(type) {
20912100
case *DriverSpec:
20922101
config := spec.(*DriverSpec)
@@ -2128,8 +2137,7 @@ func ImagePath(spec interface{}) (string, error) {
21282137
config := spec.(*DriverManagerSpec)
21292138
return imagePath(config.Repository, config.Image, config.Version, "DRIVER_MANAGER_IMAGE")
21302139
case *GPUDirectStorageSpec:
2131-
config := spec.(*GPUDirectStorageSpec)
2132-
return imagePath(config.Repository, config.Image, config.Version, "GDS_IMAGE")
2140+
return imagePath(v.Repository, v.Image, v.Version, "GDS_IMAGE")
21332141
case *GDRCopySpec:
21342142
config := spec.(*GDRCopySpec)
21352143
return imagePath(config.Repository, config.Image, config.Version, "GDRCOPY_IMAGE")

api/nvidia/v1/clusterpolicy_types_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,4 +71,18 @@ func TestImagePath(t *testing.T) {
7171
assert.Empty(t, path)
7272
assert.ErrorContains(t, err, "invalid type to construct image path")
7373
})
74+
75+
t.Run("nil spec errors", func(t *testing.T) {
76+
path, err := ImagePath(nil)
77+
require.Error(t, err)
78+
assert.Empty(t, path)
79+
assert.ErrorContains(t, err, "invalid nil spec")
80+
})
81+
82+
t.Run("typed nil spec errors", func(t *testing.T) {
83+
path, err := ImagePath((*GPUDirectStorageSpec)(nil))
84+
require.Error(t, err)
85+
assert.Empty(t, path)
86+
assert.ErrorContains(t, err, "invalid nil spec")
87+
})
7488
}

cmd/gpuop-cfg/validate/clusterpolicy/images.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -106,17 +106,19 @@ func validateImages(ctx context.Context, spec *v1.ClusterPolicySpec) error {
106106
return fmt.Errorf("failed to validate image %s: %v", path, err)
107107
}
108108

109-
// GPUDirectStorage
110-
path, err = v1.ImagePath(spec.GPUDirectStorage)
111-
if err != nil {
112-
return fmt.Errorf("failed to construct the image path: %v", err)
113-
}
114-
// For GDS driver, we must append the os-tag
115-
path += "-ubuntu22.04"
116-
117-
err = validateImage(ctx, path)
118-
if err != nil {
119-
return fmt.Errorf("failed to validate image %s: %v", path, err)
109+
// GPUDirectStorage is optional and nil when GDS is omitted from the ClusterPolicy.
110+
if spec.GPUDirectStorage != nil {
111+
path, err = v1.ImagePath(spec.GPUDirectStorage)
112+
if err != nil {
113+
return fmt.Errorf("failed to construct the image path: %v", err)
114+
}
115+
// For GDS driver, we must append the os-tag
116+
path += "-ubuntu22.04"
117+
118+
err = validateImage(ctx, path)
119+
if err != nil {
120+
return fmt.Errorf("failed to validate image %s: %v", path, err)
121+
}
120122
}
121123

122124
// VFIOManager

0 commit comments

Comments
 (0)