[GSOC] Add validator for feasible space distribution (#2404)

* added validator for feasible space distribution Signed-off-by: Shashank Mittal <[email protected]> validation logic fixed added unit test added unit test for valid distribution requested changes made Update pkg/webhook/v1beta1/experiment/validator/validator.go Co-authored-by: Yuki Iwai <[email protected]> fmt * fmt fix Signed-off-by: Shashank Mittal <[email protected]> --------- Signed-off-by: Shashank Mittal <[email protected]>
kubeflow · Aug 20, 2024 · 4964d04 · 4964d04
1 parent abd1c42
commit 4964d04
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 4 deletions.
diff --git a/pkg/webhook/v1beta1/experiment/validator/validator.go b/pkg/webhook/v1beta1/experiment/validator/validator.go
@@ -19,7 +19,6 @@ package validator
 import (
 	"encoding/json"
 	"fmt"
-	"k8s.io/apimachinery/pkg/util/validation/field"
 	"path/filepath"
 	"regexp"
 	"strconv"
@@ -30,6 +29,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
@@ -264,6 +264,17 @@ func (g *DefaultValidator) validateParameters(parameters []experimentsv1beta1.Pa
 				param.ParameterType, fmt.Sprintf("parameterType: %v is not supported", param.ParameterType)))
 		}
 
+		if param.FeasibleSpace.Distribution != "" {
+			if param.FeasibleSpace.Distribution != experimentsv1beta1.DistributionUniform &&
+				param.FeasibleSpace.Distribution != experimentsv1beta1.DistributionLogUniform &&
+				param.FeasibleSpace.Distribution != experimentsv1beta1.DistributionNormal &&
+				param.FeasibleSpace.Distribution != experimentsv1beta1.DistributionLogNormal &&
+				param.FeasibleSpace.Distribution != experimentsv1beta1.DistributionUnknown {
+				allErrs = append(allErrs, field.Invalid(parametersPath.Index(i).Child("feasibleSpace").Child("distribution"),
+					param.FeasibleSpace.Distribution, fmt.Sprintf("distribution: %v is not supported", param.FeasibleSpace.Distribution)))
+			}
+		}
+
 		if equality.Semantic.DeepEqual(param.FeasibleSpace, experimentsv1beta1.FeasibleSpace{}) {
 			allErrs = append(allErrs, field.Required(parametersPath.Index(i).Child("feasibleSpace"),
 				"feasibleSpace must be specified"))

diff --git a/pkg/webhook/v1beta1/experiment/validator/validator_test.go b/pkg/webhook/v1beta1/experiment/validator/validator_test.go
@@ -18,12 +18,13 @@ package validator
 
 import (
 	"errors"
+	"testing"
+
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	experimentutil "github.com/kubeflow/katib/pkg/controller.v1beta1/experiment/util"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/apimachinery/pkg/util/validation/field"
-	"testing"
 
 	"go.uber.org/mock/gomock"
 	batchv1 "k8s.io/api/batch/v1"
@@ -454,6 +455,22 @@ func TestValidateParameters(t *testing.T) {
 			},
 			testDescription: "Not empty max for categorical parameter type",
 		},
+		{
+			parameters: func() []experimentsv1beta1.ParameterSpec {
+				ps := newFakeInstance().Spec.Parameters
+				ps[0].FeasibleSpace.Distribution = "invalid-distribution"
+				return ps
+			}(),
+			wantErr: field.ErrorList{
+				field.Invalid(field.NewPath("spec").Child("parameters").Index(0).Child("feasibleSpace").Child("distribution"), "", ""),
+			},
+			testDescription: "Invalid distribution type",
+		},
+		{
+			parameters:      newFakeInstance().Spec.Parameters,
+			wantErr:         nil,
+			testDescription: "Valid parameters case",
+		},
 	}
 
 	for _, tc := range tcs {
@@ -1374,8 +1391,9 @@ func newFakeInstance() *experimentsv1beta1.Experiment {
 					Name:          "lr",
 					ParameterType: experimentsv1beta1.ParameterTypeInt,
 					FeasibleSpace: experimentsv1beta1.FeasibleSpace{
-						Max: "5",
-						Min: "1",
+						Max:          "5",
+						Min:          "1",
+						Distribution: experimentsv1beta1.DistributionUniform,
 					},
 				},
 				{