Skip to content

Commit bae9dd8

Browse files
aryangorwadeshivamerla
aryangorwade
authored and
shivamerla
committed
Some kserve validations added.
Signed-off-by: Aryan <gorwadearyan@gmail.com>
1 parent 32e4604 commit bae9dd8

File tree

2 files changed

+158
-14
lines changed

2 files changed

+158
-14
lines changed

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

Lines changed: 53 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package v1alpha1
1919
import (
2020
"fmt"
2121
"reflect"
22+
"strings"
2223

2324
corev1 "k8s.io/api/core/v1"
2425
networkingv1 "k8s.io/api/networking/v1"
@@ -36,6 +37,27 @@ var validPVCAccessModeStrs = []corev1.PersistentVolumeAccessMode{
3637
corev1.ReadWriteOncePod,
3738
}
3839

40+
// validateNIMServiceSpec aggregates all structural validation checks for a NIMService
41+
// object. It is intended to be invoked by both ValidateCreate and ValidateUpdate to
42+
// ensure the resource is well-formed before any other validation (e.g. immutability)
43+
// is performed.
44+
func validateNIMServiceSpec(spec *appsv1alpha1.NIMServiceSpec, fldPath *field.Path, kubeVersion string) field.ErrorList {
45+
errList := field.ErrorList{}
46+
47+
// Validate individual sections of the spec using existing helper functions.
48+
errList = append(errList, validateImageConfiguration(&spec.Image, fldPath.Child("image"))...)
49+
errList = append(errList, validateAuthSecret(&spec.AuthSecret, fldPath.Child("authSecret"))...)
50+
errList = append(errList, validateServiceStorageConfiguration(&spec.Storage, fldPath.Child("storage"))...)
51+
errList = append(errList, validateExposeConfiguration(&spec.Expose, fldPath.Child("expose").Child("ingress"))...)
52+
errList = append(errList, validateMetricsConfiguration(&spec.Metrics, fldPath.Child("metrics"))...)
53+
errList = append(errList, validateScaleConfiguration(&spec.Scale, fldPath.Child("scale"))...)
54+
errList = append(errList, validateResourcesConfiguration(spec.Resources, fldPath.Child("resources"))...)
55+
errList = append(errList, validateDRAResourcesConfiguration(spec, fldPath, kubeVersion)...)
56+
errList = append(errList, validateKServeConfiguration(spec, fldPath)...)
57+
58+
return errList
59+
}
60+
3961
func validateImageConfiguration(image *appsv1alpha1.Image, fldPath *field.Path) field.ErrorList {
4062
errList := field.ErrorList{}
4163
if image.Repository == "" {
@@ -212,22 +234,39 @@ func validateResourcesConfiguration(resources *corev1.ResourceRequirements, fldP
212234
return errList
213235
}
214236

215-
// validateNIMServiceSpec aggregates all structural validation checks for a NIMService
216-
// object. It is intended to be invoked by both ValidateCreate and ValidateUpdate to
217-
// ensure the resource is well-formed before any other validation (e.g. immutability)
218-
// is performed.
219-
func validateNIMServiceSpec(spec *appsv1alpha1.NIMServiceSpec, fldPath *field.Path, kubeVersion string) field.ErrorList {
237+
// validateKServeonfiguration implements required KServe validations.
238+
func validateKServeConfiguration(spec *appsv1alpha1.NIMServiceSpec, fldPath *field.Path) field.ErrorList {
220239
errList := field.ErrorList{}
221240

222-
// Validate individual sections of the spec using existing helper functions.
223-
errList = append(errList, validateImageConfiguration(&spec.Image, fldPath.Child("image"))...)
224-
errList = append(errList, validateAuthSecret(&spec.AuthSecret, fldPath.Child("authSecret"))...)
225-
errList = append(errList, validateServiceStorageConfiguration(&spec.Storage, fldPath.Child("storage"))...)
226-
errList = append(errList, validateExposeConfiguration(&spec.Expose, fldPath.Child("expose").Child("ingress"))...)
227-
errList = append(errList, validateMetricsConfiguration(&spec.Metrics, fldPath.Child("metrics"))...)
228-
errList = append(errList, validateScaleConfiguration(&spec.Scale, fldPath.Child("scale"))...)
229-
errList = append(errList, validateResourcesConfiguration(spec.Resources, fldPath.Child("resources"))...)
230-
errList = append(errList, validateDRAResourcesConfiguration(spec, fldPath, kubeVersion)...)
241+
platformIsKServe := spec.InferencePlatform == appsv1alpha1.PlatformTypeKServe
242+
243+
// mode is the value, and annotated is true if the key-value pair exist.
244+
mode, annotated := spec.Annotations["serving.kserve.org/deploymentMode"]
245+
// If the annotation is absent, kserve defaults to serverless.
246+
serverless := !annotated || strings.EqualFold(mode, "serverless")
247+
248+
// When Spec.InferencePlatform is "kserve" and used in "serverless" mode:
249+
if platformIsKServe && serverless {
250+
// Spec.Scale (autoscaling) cannot be set.
251+
if spec.Scale.Enabled != nil && *spec.Scale.Enabled {
252+
errList = append(errList, field.Forbidden(fldPath.Child("scale").Child("enabled"), fmt.Sprintf("%s (autoscaling) cannot be set when KServe runs in serverless mode", fldPath.Child("scale"))))
253+
}
254+
255+
// Spec.Expose.Ingress cannot be set.
256+
if spec.Expose.Ingress.Enabled != nil && *spec.Expose.Ingress.Enabled {
257+
errList = append(errList, field.Forbidden(fldPath.Child("expose").Child("ingress").Child("enabled"), fmt.Sprintf("%s cannot be set when KServe runs in serverless mode", fldPath.Child("expose").Child("ingress"))))
258+
}
259+
260+
// Spec.Metrics.ServiceMonitor cannot be set.
261+
if spec.Metrics.Enabled != nil && *spec.Metrics.Enabled {
262+
errList = append(errList, field.Forbidden(fldPath.Child("metrics").Child("enabled"), fmt.Sprintf("%s cannot be set when KServe runs in serverless mode", fldPath.Child("metrics").Child("serviceMonitor"))))
263+
}
264+
}
265+
266+
// Spec.MultiNode cannot be enabled when inferencePlatform is kserve.
267+
if platformIsKServe && spec.MultiNode != nil {
268+
errList = append(errList, field.Forbidden(fldPath.Child("multiNode"), "cannot be set when KServe runs in serverless mode"))
269+
}
231270

232271
return errList
233272
}

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

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,3 +548,108 @@ func TestValidatePVCImmutability(t *testing.T) {
548548
})
549549
}
550550
}
551+
552+
// TestValidateKServeConfiguration covers all KServe-specific validation rules.
553+
func TestValidateKServeConfiguration(t *testing.T) {
554+
fld := field.NewPath("spec")
555+
556+
trueVal := true
557+
558+
tests := []struct {
559+
name string
560+
modify func(*appsv1alpha1.NIMService)
561+
wantErrs int
562+
}{
563+
{
564+
name: "standalone platform – no errors",
565+
modify: func(ns *appsv1alpha1.NIMService) {
566+
ns.Spec.InferencePlatform = appsv1alpha1.PlatformTypeStandalone
567+
},
568+
wantErrs: 0,
569+
},
570+
{
571+
name: "kserve serverless (annotation absent) – valid",
572+
modify: func(ns *appsv1alpha1.NIMService) {
573+
ns.Spec.InferencePlatform = appsv1alpha1.PlatformTypeKServe
574+
// No annotation ⇒ serverless by default.
575+
},
576+
wantErrs: 0,
577+
},
578+
{
579+
name: "kserve serverless (annotation present) – autoscaling set",
580+
modify: func(ns *appsv1alpha1.NIMService) {
581+
ns.Spec.InferencePlatform = appsv1alpha1.PlatformTypeKServe
582+
ns.Spec.Annotations = map[string]string{"serving.kserve.org/deploymentMode": "Serverless"}
583+
ns.Spec.Scale.Enabled = &trueVal
584+
},
585+
wantErrs: 1,
586+
},
587+
{
588+
name: "kserve serverless – ingress set",
589+
modify: func(ns *appsv1alpha1.NIMService) {
590+
ns.Spec.InferencePlatform = appsv1alpha1.PlatformTypeKServe
591+
ingEnabled := true
592+
ns.Spec.Expose.Ingress.Enabled = &ingEnabled
593+
},
594+
wantErrs: 1,
595+
},
596+
{
597+
name: "kserve serverless – servicemonitor set",
598+
modify: func(ns *appsv1alpha1.NIMService) {
599+
ns.Spec.InferencePlatform = appsv1alpha1.PlatformTypeKServe
600+
ns.Spec.Metrics.Enabled = &trueVal
601+
},
602+
wantErrs: 1,
603+
},
604+
{
605+
name: "kserve serverless – all prohibited set",
606+
modify: func(ns *appsv1alpha1.NIMService) {
607+
ns.Spec.InferencePlatform = appsv1alpha1.PlatformTypeKServe
608+
ns.Spec.Scale.Enabled = &trueVal
609+
ingEnabled := true
610+
ns.Spec.Expose.Ingress.Enabled = &ingEnabled
611+
ns.Spec.Metrics.Enabled = &trueVal
612+
},
613+
wantErrs: 3,
614+
},
615+
{
616+
name: "kserve rawdeployment – allowed autoscaling, but multidnode forbidden",
617+
modify: func(ns *appsv1alpha1.NIMService) {
618+
ns.Spec.InferencePlatform = appsv1alpha1.PlatformTypeKServe
619+
ns.Spec.Annotations = map[string]string{"serving.kserve.org/deploymentMode": "RawDeployment"}
620+
ns.Spec.Scale.Enabled = &trueVal // should be fine
621+
ns.Spec.MultiNode = &appsv1alpha1.NimServiceMultiNodeConfig{Size: 1}
622+
},
623+
wantErrs: 1, // only multiNode should trigger
624+
},
625+
{
626+
name: "kserve – multidnode alone",
627+
modify: func(ns *appsv1alpha1.NIMService) {
628+
ns.Spec.InferencePlatform = appsv1alpha1.PlatformTypeKServe
629+
ns.Spec.MultiNode = &appsv1alpha1.NimServiceMultiNodeConfig{Size: 2}
630+
},
631+
wantErrs: 1,
632+
},
633+
}
634+
635+
for _, tc := range tests {
636+
t.Run(tc.name, func(t *testing.T) {
637+
ns := baseNIMService()
638+
// Ensure nested structs are initialised to avoid nil panics when we set sub-fields.
639+
ns.Spec.Scale = appsv1alpha1.Autoscaling{}
640+
ns.Spec.Expose = appsv1alpha1.Expose{}
641+
ns.Spec.Expose.Ingress = appsv1alpha1.Ingress{}
642+
ns.Spec.Metrics = appsv1alpha1.Metrics{}
643+
644+
tc.modify(ns)
645+
646+
errs := validateKServeConfiguration(&ns.Spec, fld)
647+
if got := len(errs); got != tc.wantErrs {
648+
for i, err := range errs {
649+
t.Logf(" %d: %s", i+1, err.Error())
650+
}
651+
t.Fatalf("got %d errs, want %d", got, tc.wantErrs)
652+
}
653+
})
654+
}
655+
}

0 commit comments

Comments
 (0)