Skip to content

Commit 1be2180

Browse files
aryangorwadearyangorwade
committed
r
1 parent 7623902 commit 1be2180

7 files changed

Lines changed: 112 additions & 76 deletions

File tree

‎cmd/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ func main() {
260260

261261
// nolint:goconst
262262
// Parse ENABLE_WEBHOOKS environment variable once as a boolean.
263-
enableWebhooks := true
263+
var enableWebhooks bool
264264
if val, ok := os.LookupEnv("ENABLE_WEBHOOKS"); ok {
265265
var err error
266266
enableWebhooks, err = strconv.ParseBool(val)

‎config/manager/kustomization.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,5 @@ apiVersion: kustomize.config.k8s.io/v1beta1
44
kind: Kustomization
55
images:
66
- name: controller
7-
newName: nvcr.io/nvidia/cloud-native/nim-operator
8-
newTag: v1.0.0
7+
newName: localhost:5000/k8s-nim-operator
8+
newTag: dev

‎deployments/helm/k8s-nim-operator/values.yaml‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
operator:
22
replicas: 1
33
upgradeCRD: true
4-
image:
5-
repository: ghcr.io/nvidia/k8s-nim-operator
6-
tag: main
4+
image:
5+
repository: localhost:5000/k8s-nim-operator
6+
tag: dev
7+
78
pullSecrets: []
89
pullPolicy: Always
910
args:
@@ -46,7 +47,7 @@ operator:
4647
- ALL
4748
admissionController:
4849
# -- Deploy with admission controller.
49-
enabled: true
50+
enabled: false
5051
# -- Use cert-manager for generating self-signed certificate.
5152
useCertManager: true
5253
# certificate:

‎internal/webhook/apps/v1alpha1/nimcache_webhook.go‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,9 @@ func (v *NIMCacheCustomValidator) ValidateCreate(_ context.Context, obj runtime.
6868
}
6969
nimcachelog.V(4).Info("Validation for NIMCache upon creation", "name", nimcache.GetName())
7070

71-
errList := field.ErrorList{}
72-
7371
fldPath := field.NewPath("nimcache").Child("spec")
74-
errList = append(errList, validateNIMSourceConfiguration(&nimcache.Spec.Source, fldPath.Child("source"))...)
75-
errList = append(errList, validateNIMCacheStorageConfiguration(&nimcache.Spec.Storage, fldPath.Child("storage"))...)
76-
errList = append(errList, validateProxyConfiguration(nimcache.Spec.Proxy, fldPath.Child("proxy"))...)
72+
// Perform structural validation via helper.
73+
errList := validateNIMCacheSpec(&nimcache.Spec, fldPath)
7774

7875
if len(errList) > 0 {
7976
return nil, errList.ToAggregate()
@@ -91,6 +88,11 @@ func (v *NIMCacheCustomValidator) ValidateUpdate(_ context.Context, oldObj, newO
9188
}
9289
nimcachelog.V(4).Info("Validation for NIMCache upon update", "name", nimcache.GetName())
9390

91+
fldPath := field.NewPath("nimcache").Child("spec")
92+
93+
// Begin by validating the new spec structurally.
94+
errList := validateNIMCacheSpec(&nimcache.Spec, fldPath)
95+
9496
oldNIMCache, ok := oldObj.(*appsv1alpha1.NIMCache)
9597
if !ok {
9698
return nil, fmt.Errorf("expected a NIMCache object for oldObj but got %T", oldObj)
@@ -100,8 +102,7 @@ func (v *NIMCacheCustomValidator) ValidateUpdate(_ context.Context, oldObj, newO
100102
return nil, fmt.Errorf("expected a NIMCache object for newObj but got %T", newObj)
101103
}
102104

103-
errList := field.ErrorList{}
104-
105+
// Append immutability errors after structural checks.
105106
errList = append(errList, validateImmutableNIMCacheSpec(oldNIMCache, newNIMCache, field.NewPath("nimcache"))...)
106107

107108
if len(errList) > 0 {

‎internal/webhook/apps/v1alpha1/nimcache_webhook_validation_helper.go‎

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package v1alpha1
1818

1919
import (
2020
"fmt"
21+
"reflect"
2122
"regexp"
2223
"strings"
2324

@@ -78,7 +79,7 @@ func validateModel(model *appsv1alpha1.ModelSpec, fldPath *field.Path) field.Err
7879

7980
for _, profile := range model.Profiles {
8081
if profile == "all" && len(model.Profiles) != 1 {
81-
errList = append(errList, field.Invalid(fldPath.Child("profiles"), model.Profiles, "%s must only have a single entry when it contains 'all'"))
82+
errList = append(errList, field.Invalid(fldPath.Child("profiles"), model.Profiles, "must only have a single entry when it contains 'all'"))
8283
break
8384
}
8485
}
@@ -127,31 +128,35 @@ func validateNIMCacheStorageConfiguration(storage *appsv1alpha1.NIMCacheStorage,
127128
errList := field.ErrorList{}
128129

129130
// Spec.Storage must not be empty
130-
if storage.PVC.Create == nil && storage.PVC.Name == "" && storage.PVC.StorageClass == "" &&
131-
storage.PVC.Size == "" && storage.PVC.VolumeAccessMode == "" && storage.PVC.SubPath == "" &&
132-
len(storage.PVC.Annotations) == 0 {
131+
if reflect.DeepEqual(storage.PVC, appsv1alpha1.PersistentVolumeClaim{}) {
133132
errList = append(errList, field.Required(fldPath, "must not be empty"))
134133
}
134+
135+
errList = append(errList, validatePVCConfiguration(&storage.PVC, fldPath.Child("pvc"))...)
135136

136-
// Removed invalid comparison of a struct containing maps.
137+
return errList
138+
}
139+
140+
func validatePVCConfiguration(pvc *appsv1alpha1.PersistentVolumeClaim, fldPath *field.Path) field.ErrorList {
141+
errList := field.ErrorList{}
137142

138143
// If PVC.Create is False, PVC.Name cannot be empty
139-
if storage.PVC.Create != nil && !*storage.PVC.Create && storage.PVC.Name == "" {
140-
errList = append(errList, field.Required(fldPath.Child("pvc").Child("name"), fmt.Sprintf("must be provided when %s is false", fldPath.Child("pvc").Child("create"))))
144+
if (pvc.Create == nil || !*pvc.Create) && pvc.Name == "" {
145+
errList = append(errList, field.Required(fldPath.Child("name"), fmt.Sprintf("must be provided when %s is false", fldPath.Child("create"))))
141146
}
142147

143148
// If PVC.VolumeAccessMode is defined, it must be one of the valid modes
144-
if storage.PVC.VolumeAccessMode != "" {
149+
if pvc.VolumeAccessMode != "" {
145150
found := false
146151
for _, vm := range validPVCAccessModeStrs {
147-
if storage.PVC.VolumeAccessMode == vm {
152+
if pvc.VolumeAccessMode == vm {
148153
found = true
149154
break
150155
}
151156
}
152157

153158
if !found {
154-
errList = append(errList, field.Invalid(fldPath.Child("pvc").Child("volumeAccessMode"), storage.PVC.VolumeAccessMode, "unrecognized volumeAccessMode"))
159+
errList = append(errList, field.Invalid(fldPath.Child("volumeAccessMode"), pvc.VolumeAccessMode, "unrecognized volumeAccessMode"))
155160
}
156161
}
157162

@@ -190,6 +195,28 @@ func validateProxyConfiguration(proxy *appsv1alpha1.ProxySpec, fldPath *field.Pa
190195
return errList
191196
}
192197

198+
// validateNIMCacheSpec aggregates all structural validation rules for a NIMCache
199+
// resource. This central function is intended for use by both webhook
200+
// ValidateCreate and ValidateUpdate methods so that they share identical
201+
// well-formedness checks.
202+
//
203+
// Parameters:
204+
// – spec: pointer to the NIMCacheSpec being validated.
205+
// – fldPath: field path pointing at the root of the spec (typically
206+
// field.NewPath("nimcache").Child("spec")).
207+
//
208+
// Returns a field.ErrorList with any validation errors encountered.
209+
func validateNIMCacheSpec(spec *appsv1alpha1.NIMCacheSpec, fldPath *field.Path) field.ErrorList {
210+
errList := field.ErrorList{}
211+
212+
// Delegate to existing granular validators.
213+
errList = append(errList, validateNIMSourceConfiguration(&spec.Source, fldPath.Child("source"))...)
214+
errList = append(errList, validateNIMCacheStorageConfiguration(&spec.Storage, fldPath.Child("storage"))...)
215+
errList = append(errList, validateProxyConfiguration(spec.Proxy, fldPath.Child("proxy"))...)
216+
217+
return errList
218+
}
219+
193220
func validateImmutableNIMCacheSpec(oldNIMCache, newNIMCache *appsv1alpha1.NIMCache, fldPath *field.Path) field.ErrorList {
194221
errList := field.ErrorList{}
195222

‎internal/webhook/apps/v1alpha1/nimservice_webhook.go‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -92,18 +92,10 @@ func (v *NIMServiceCustomValidator) ValidateCreate(_ context.Context, obj runtim
9292
}
9393
nimservicelog.V(4).Info("Validation for NIMService upon creation", "name", nimservice.GetName())
9494

95-
errList := field.ErrorList{}
96-
9795
fldPath := field.NewPath("nimservice").Child("spec")
98-
errList = append(errList, validateImageConfiguration(&nimservice.Spec.Image, fldPath.Child("image"))...)
99-
errList = append(errList, validateAuthSecret(&nimservice.Spec.AuthSecret, fldPath.Child("authSecret"))...)
100-
errList = append(errList, validateServiceStorageConfiguration(&nimservice.Spec.Storage, fldPath.Child("storage"))...)
101-
errList = append(errList, validateExposeConfiguration(&nimservice.Spec.Expose, fldPath.Child("expose").Child("ingress"))...)
102-
errList = append(errList, validateMetricsConfiguration(&nimservice.Spec.Metrics, fldPath.Child("metrics"))...)
103-
errList = append(errList, validateScaleConfiguration(&nimservice.Spec.Scale, fldPath.Child("scale"))...)
104-
errList = append(errList, validateDRAResourcesVersionCompatibility(v.k8sVersion, fldPath.Child("draResources"))...)
105-
errList = append(errList, validateResourcesConfiguration(nimservice.Spec.Resources, fldPath.Child("resources"))...)
106-
errList = append(errList, validateDRAResourcesConfiguration(&nimservice.Spec, fldPath)...)
96+
97+
// Perform comprehensive spec validation via helper.
98+
errList := validateNIMServiceSpec(v.k8sVersion, &nimservice.Spec, fldPath)
10799

108100
if len(errList) > 0 {
109101
return nil, errList.ToAggregate()
@@ -120,11 +112,13 @@ func (v *NIMServiceCustomValidator) ValidateUpdate(_ context.Context, oldObj, ne
120112
}
121113
nimservicelog.V(4).Info("Validation for NIMService upon update", "name", nimservice.GetName())
122114

115+
fldPath := field.NewPath("nimservice").Child("spec")
116+
// Start with structural validation to ensure the updated object is well formed.
117+
errList := validateNIMServiceSpec(v.k8sVersion, &nimservice.Spec, fldPath)
118+
123119
// All fields of NIMService.Spec are mutable, except for:
124120
// - Spec.MultiNode
125121
// - If PVC has been created with PVC.Create = true, reject any updates to any fields of PVC object
126-
errList := field.ErrorList{}
127-
128122
oldNIMService, ok := oldObj.(*appsv1alpha1.NIMService)
129123
if !ok {
130124
return nil, fmt.Errorf("expected a NIMService object for oldObj but got %T", oldObj)

‎internal/webhook/apps/v1alpha1/nimservice_webhook_validation_helper.go‎

Lines changed: 52 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,14 @@ package v1alpha1
1818

1919
import (
2020
"fmt"
21+
"reflect"
2122

2223
corev1 "k8s.io/api/core/v1"
24+
networkingv1 "k8s.io/api/networking/v1"
2325
"k8s.io/apimachinery/pkg/api/equality"
2426
"k8s.io/apimachinery/pkg/util/validation/field"
2527

28+
"github.com/NVIDIA/k8s-nim-operator/api/apps/v1alpha1"
2629
appsv1alpha1 "github.com/NVIDIA/k8s-nim-operator/api/apps/v1alpha1"
2730
"github.com/NVIDIA/k8s-nim-operator/internal/utils"
2831
)
@@ -93,25 +96,7 @@ func validateServiceStorageConfiguration(storage *appsv1alpha1.NIMServiceStorage
9396

9497
// Enforcing PVC rules if defined
9598
if pvcDefined {
96-
97-
// If PVC.Create is False, PVC.Name cannot be empty
98-
if storage.PVC.Create != nil && !*storage.PVC.Create && storage.PVC.Name == "" {
99-
errList = append(errList, field.Required(fldPath.Child("pvc").Child("name"), fmt.Sprintf("must be provided when %s is false", fldPath.Child("pvc").Child("create"))))
100-
}
101-
102-
if storage.PVC.VolumeAccessMode != "" {
103-
found := false
104-
for _, vm := range validPVCAccessModeStrs {
105-
if storage.PVC.VolumeAccessMode == vm {
106-
found = true
107-
break
108-
}
109-
}
110-
111-
if !found {
112-
errList = append(errList, field.Invalid(fldPath.Child("pvc").Child("volumeAccessMode"), storage.PVC.VolumeAccessMode, "unrecognized volumeAccessMode"))
113-
}
114-
}
99+
errList = append(errList, validatePVCConfiguration(&storage.PVC, fldPath.Child("pvc"))...)
115100
}
116101

117102
return errList
@@ -133,20 +118,22 @@ func validateDRAResourcesConfiguration(spec *appsv1alpha1.NIMServiceSpec, fldPat
133118
if dra.ResourceClaimName != nil && *dra.ResourceClaimName != "" {
134119
errList = append(errList, field.Forbidden(
135120
draResourcesPath.Index(i).Child("resourceClaimName"),
136-
fmt.Sprintf("must not be set when %s > 1 or %s, only %s is allowed", fldPath.Child("replicas"), fldPath.Child("scale").Child("enabled"), draResourcesPath.Index(i).Child("draResourceClaimTemplateName"))))
121+
fmt.Sprintf("must not be set when %s > 1 or %s, only %s is allowed", fldPath.Child("replicas"), fldPath.Child("scale").Child("enabled"), draResourcesPath.Index(i).Child("resourceClaimTemplateName"))))
137122
}
138123
}
139-
} else {
140-
141-
// If DRAResources is not empty, all DRAResources objects must have a unique DRAResource.ResourceClaimName
142-
draResources := spec.DRAResources
143-
if len(draResources) > 0 {
144-
seen := make(map[string]struct{})
145-
for i, dra := range draResources {
146-
if dra.ResourceClaimName == nil {
147-
errList = append(errList, field.Required(draResourcesPath.Index(i).Child("resourceClaimName"), "must not be empty"))
148-
continue
149-
}
124+
}
125+
// If DRAResources has multiple entries with DRAResource.ResourceClaimName, they have to be unique names. Additionally, only one of DRAResource.ResourceClaimName or DRAResource.ResourceClaimTemplateName must be defined.
126+
draResources := spec.DRAResources
127+
if len(draResources) > 0 {
128+
seen := make(map[string]struct{})
129+
for i, dra := range draResources {
130+
// Must have only one of the two defined
131+
if (dra.ResourceClaimName == nil && dra.ResourceClaimTemplateName == nil) || (dra.ResourceClaimName != nil && dra.ResourceClaimTemplateName != nil) {
132+
errList = append(errList, field.Invalid(draResourcesPath.Index(i), dra, "must specify exactly one of resourceClaimName or resourceClaimTemplateName"))
133+
continue
134+
}
135+
136+
if dra.ResourceClaimName != nil {
150137
if _, exists := seen[*dra.ResourceClaimName]; exists {
151138
errList = append(errList, field.Duplicate(draResourcesPath.Index(i).Child("resourceClaimName"), *dra.ResourceClaimName))
152139
}
@@ -169,9 +156,7 @@ func validateAuthSecret(authSecret *string, fldPath *field.Path) field.ErrorList
169156
func validateExposeConfiguration(expose *appsv1alpha1.Expose, fldPath *field.Path) field.ErrorList {
170157
errList := field.ErrorList{}
171158
if expose.Ingress.Enabled != nil && *expose.Ingress.Enabled {
172-
spec := expose.Ingress.Spec
173-
if spec.IngressClassName == nil && spec.DefaultBackend == nil &&
174-
len(spec.TLS) == 0 && len(spec.Rules) == 0 {
159+
if reflect.DeepEqual(expose.Ingress.Spec, networkingv1.IngressSpec{}) {
175160
errList = append(errList, field.Required(fldPath.Child("spec"), fmt.Sprintf("must be defined if %s is true", fldPath.Child("enabled"))))
176161
}
177162
}
@@ -182,9 +167,7 @@ func validateExposeConfiguration(expose *appsv1alpha1.Expose, fldPath *field.Pat
182167
func validateMetricsConfiguration(metrics *appsv1alpha1.Metrics, fldPath *field.Path) field.ErrorList {
183168
errList := field.ErrorList{}
184169
if metrics.Enabled != nil && *metrics.Enabled {
185-
sm := metrics.ServiceMonitor
186-
if len(sm.AdditionalLabels) == 0 && len(sm.Annotations) == 0 &&
187-
sm.Interval == "" && sm.ScrapeTimeout == "" {
170+
if reflect.DeepEqual(metrics.ServiceMonitor, v1alpha1.ServiceMonitor{}) {
188171
errList = append(errList, field.Required(fldPath.Child("serviceMonitor"), fmt.Sprintf("must be defined if %s is true", fldPath.Child("enabled"))))
189172
}
190173
}
@@ -195,8 +178,7 @@ func validateMetricsConfiguration(metrics *appsv1alpha1.Metrics, fldPath *field.
195178
func validateScaleConfiguration(scale *appsv1alpha1.Autoscaling, fldPath *field.Path) field.ErrorList {
196179
errList := field.ErrorList{}
197180
if scale.Enabled != nil && *scale.Enabled {
198-
hpa := scale.HPA
199-
if hpa.MinReplicas == nil && hpa.MaxReplicas == 0 && len(hpa.Metrics) == 0 && hpa.Behavior == nil {
181+
if reflect.DeepEqual(scale.HPA, v1alpha1.HorizontalPodAutoscalerSpec{}) {
200182
errList = append(errList, field.Required(fldPath.Child("hpa"), fmt.Sprintf("must be defined if %s is true", fldPath.Child("enabled"))))
201183
}
202184
}
@@ -226,6 +208,37 @@ func validateResourcesConfiguration(resources *corev1.ResourceRequirements, fldP
226208
return errList
227209
}
228210

211+
// validateNIMServiceSpec aggregates all structural validation checks for a NIMService
212+
// object. It is intended to be invoked by both ValidateCreate and ValidateUpdate to
213+
// ensure the resource is well-formed before any other validation (e.g. immutability)
214+
// is performed.
215+
//
216+
// Parameters:
217+
//
218+
// – kubeVersion: the Kubernetes version string against which version-specific
219+
// checks (e.g. DRA resources) are evaluated.
220+
// – spec: the NIMServiceSpec being validated.
221+
// – fldPath: the field path pointing at the root of the spec (typically
222+
// field.NewPath("nimservice").Child("spec")).
223+
//
224+
// Returns a field.ErrorList containing any validation errors discovered.
225+
func validateNIMServiceSpec(kubeVersion string, spec *appsv1alpha1.NIMServiceSpec, fldPath *field.Path) field.ErrorList {
226+
errList := field.ErrorList{}
227+
228+
// Validate individual sections of the spec using existing helper functions.
229+
errList = append(errList, validateImageConfiguration(&spec.Image, fldPath.Child("image"))...)
230+
errList = append(errList, validateAuthSecret(&spec.AuthSecret, fldPath.Child("authSecret"))...)
231+
errList = append(errList, validateServiceStorageConfiguration(&spec.Storage, fldPath.Child("storage"))...)
232+
errList = append(errList, validateExposeConfiguration(&spec.Expose, fldPath.Child("expose").Child("ingress"))...)
233+
errList = append(errList, validateMetricsConfiguration(&spec.Metrics, fldPath.Child("metrics"))...)
234+
errList = append(errList, validateScaleConfiguration(&spec.Scale, fldPath.Child("scale"))...)
235+
errList = append(errList, validateDRAResourcesVersionCompatibility(kubeVersion, fldPath.Child("draResources"))...)
236+
errList = append(errList, validateResourcesConfiguration(spec.Resources, fldPath.Child("resources"))...)
237+
errList = append(errList, validateDRAResourcesConfiguration(spec, fldPath)...)
238+
239+
return errList
240+
}
241+
229242
// validateMultiNodeImmutability ensures that the MultiNode field remains unchanged after creation.
230243
func validateMultiNodeImmutability(oldNs, newNs *appsv1alpha1.NIMService, fldPath *field.Path) field.ErrorList {
231244
errList := field.ErrorList{}

0 commit comments

Comments
 (0)