add validation webhooks for maxFailedTrialCount and parallelTrialCount

kubeflow · Aug 21, 2022 · 4d59b1f · 4d59b1f
1 parent fe4d6e7
commit 4d59b1f
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 6 deletions.
diff --git a/.github/workflows/template-setup-e2e-test/action.yaml b/.github/workflows/template-setup-e2e-test/action.yaml
@@ -20,6 +20,6 @@ runs:
       uses: docker/setup-buildx-action@v2
 
     - name: Set Up Go env
-      uses: actions/setup-go@v2
+      uses: actions/setup-go@v3
       with:
-        go-version: 1.17.10
+        go-version-file: go.mod
diff --git a/pkg/webhook/v1beta1/experiment/validator/validator.go b/pkg/webhook/v1beta1/experiment/validator/validator.go
@@ -74,15 +74,26 @@ func (g *DefaultValidator) ValidateExperiment(instance, oldInst *experimentsv1be
 		return fmt.Errorf(msg)
 	}
 
-	if instance.Spec.MaxFailedTrialCount != nil && *instance.Spec.MaxFailedTrialCount < 0 {
-		return fmt.Errorf("spec.maxFailedTrialCount should not be less than 0")
+	if instance.Spec.MaxFailedTrialCount != nil {
+		if *instance.Spec.MaxFailedTrialCount < 0 {
+			return fmt.Errorf("spec.maxFailedTrialCount should not be less than 0")
+		}
+		if instance.Spec.MaxTrialCount != nil && *instance.Spec.MaxFailedTrialCount > *instance.Spec.MaxTrialCount {
+			return fmt.Errorf("spec.maxFailedTrialCount should be less than or equal to spec.maxTrialCount")
+		}
 	}
 	if instance.Spec.MaxTrialCount != nil && *instance.Spec.MaxTrialCount <= 0 {
 		return fmt.Errorf("spec.maxTrialCount must be greater than 0")
 	}
-	if instance.Spec.ParallelTrialCount != nil && *instance.Spec.ParallelTrialCount <= 0 {
-		return fmt.Errorf("spec.parallelTrialCount must be greater than 0")
+	if instance.Spec.ParallelTrialCount != nil {
+		if *instance.Spec.ParallelTrialCount <= 0 {
+			return fmt.Errorf("spec.parallelTrialCount must be greater than 0")
+		}
+		if instance.Spec.MaxTrialCount != nil && *instance.Spec.ParallelTrialCount > *instance.Spec.MaxTrialCount {
+			return fmt.Errorf("spec.paralelTrialCount should be less than or equal to spec.maxTrialCount")
+		}
 	}
+
 	if oldInst != nil {
 		// We should validate restart only if appropriate fields are changed.
 		// Otherwise check below is triggered when experiment is deleted.

diff --git a/pkg/webhook/v1beta1/experiment/validator/validator_test.go b/pkg/webhook/v1beta1/experiment/validator/validator_test.go
@@ -284,6 +284,54 @@ func TestValidateExperiment(t *testing.T) {
 			Err:             true,
 			testDescription: "Invalid feasible space in parameters",
 		},
+		{
+			Instance: func() *experimentsv1beta1.Experiment {
+				maxTrialCount := int32(5)
+				invalidMaxFailedTrialCount := int32(6)
+				i := newFakeInstance()
+				i.Spec.MaxTrialCount = &maxTrialCount
+				i.Spec.MaxFailedTrialCount = &invalidMaxFailedTrialCount
+				return i
+			}(),
+			Err:             true,
+			testDescription: "maxFailedTrialCount greater than maxTrialCount",
+		},
+		{
+			Instance: func() *experimentsv1beta1.Experiment {
+				maxTrialCount := int32(5)
+				validMaxFailedTrialCount := int32(5)
+				i := newFakeInstance()
+				i.Spec.MaxTrialCount = &maxTrialCount
+				i.Spec.MaxFailedTrialCount = &validMaxFailedTrialCount
+				return i
+			}(),
+			Err:             false,
+			testDescription: "maxFailedTrialCount equal to maxTrialCount",
+		},
+		{
+			Instance: func() *experimentsv1beta1.Experiment {
+				maxTrialCount := int32(5)
+				invalidParallelTrialCount := int32(6)
+				i := newFakeInstance()
+				i.Spec.MaxTrialCount = &maxTrialCount
+				i.Spec.ParallelTrialCount = &invalidParallelTrialCount
+				return i
+			}(),
+			Err:             true,
+			testDescription: "parallelTrialCount greater than maxTrialCount",
+		},
+		{
+			Instance: func() *experimentsv1beta1.Experiment {
+				maxTrialCount := int32(5)
+				validParallelTrialCount := int32(5)
+				i := newFakeInstance()
+				i.Spec.MaxTrialCount = &maxTrialCount
+				i.Spec.ParallelTrialCount = &validParallelTrialCount
+				return i
+			}(),
+			Err:             false,
+			testDescription: "parallelTrialCount equal to maxTrialCount",
+		},
 	}
 
 	for _, tc := range tcs {