Merge pull request #5282 from a7i/work-suspend-validation

work suspension: webhook validation

pkg/util/validation/validation.go

@@ -43,6 +43,7 @@ func ValidatePropagationSpec(spec policyv1alpha1.PropagationSpec) field.ErrorLis
 	}
 	allErrs = append(allErrs, ValidateFailover(spec.Failover, field.NewPath("spec").Child("failover"))...)
 	allErrs = append(allErrs, validateResourceSelectorsIfPreemptionEnabled(spec, field.NewPath("spec").Child("resourceSelectors"))...)
+	allErrs = append(allErrs, validateSuspension(spec.Suspension, field.NewPath("spec").Child("suspension"))...)
 	return allErrs
 }
 
@@ -55,18 +56,33 @@ func validateResourceSelectorsIfPreemptionEnabled(spec policyv1alpha1.Propagatio
 	var allErrs field.ErrorList
 	for index, resourceSelector := range spec.ResourceSelectors {
 		if len(resourceSelector.Name) == 0 {
-			allErrs = append(allErrs, field.Invalid(fldPath.Index(index).Child("name"), resourceSelector.Name, "name can not be empty if preemption is Always, the empty name may cause unexpected resources preemption"))
+			allErrs = append(allErrs, field.Invalid(fldPath.Index(index).Child("name"), resourceSelector.Name, "name cannot be empty if preemption is Always, the empty name may cause unexpected resources preemption"))
 		}
 	}
 	return allErrs
 }
 
+// validateSuspension validates no conflicts between dispatching and dispatchingOnClusters.
+func validateSuspension(suspension *policyv1alpha1.Suspension, fldPath *field.Path) field.ErrorList {
+	if suspension == nil {
+		return nil
+	}
+
+	if (suspension.Dispatching != nil && *suspension.Dispatching) &&
+		(suspension.DispatchingOnClusters != nil && len(suspension.DispatchingOnClusters.ClusterNames) > 0) {
+		return field.ErrorList{
+			field.Invalid(fldPath.Child("suspension"), suspension, "suspension dispatching cannot co-exist with dispatchingOnClusters.clusterNames"),
+		}
+	}
+	return nil
+}
+
 // ValidatePlacement validates a placement before creation or update.
 func ValidatePlacement(placement policyv1alpha1.Placement, fldPath *field.Path) field.ErrorList {
 	var allErrs field.ErrorList
 
 	if placement.ClusterAffinity != nil && placement.ClusterAffinities != nil {
-		allErrs = append(allErrs, field.Invalid(fldPath, placement, "clusterAffinities can not co-exist with clusterAffinity"))
+		allErrs = append(allErrs, field.Invalid(fldPath, placement, "clusterAffinities cannot co-exist with clusterAffinity"))
 	}
 
 	allErrs = append(allErrs, ValidateClusterAffinity(placement.ClusterAffinity, fldPath.Child("clusterAffinity"))...)

pkg/util/validation/validation_test.go

@@ -370,7 +370,7 @@ func TestValidatePropagationSpec(t *testing.T) {
 			expectedErr: "unsupported operator \"Exists\", must be In or NotIn",
 		},
 		{
-			name: "clusterAffinities can not co-exist with clusterAffinity",
+			name: "clusterAffinities cannot co-exist with clusterAffinity",
 			spec: policyv1alpha1.PropagationSpec{
 				Placement: policyv1alpha1.Placement{
 					ClusterAffinity: &policyv1alpha1.ClusterAffinity{
@@ -382,7 +382,7 @@ func TestValidatePropagationSpec(t *testing.T) {
 							ClusterAffinity: policyv1alpha1.ClusterAffinity{
 								ClusterNames: []string{"m1"},
 							}}}}},
-			expectedErr: "clusterAffinities can not co-exist with clusterAffinity",
+			expectedErr: "clusterAffinities cannot co-exist with clusterAffinity",
 		},
 		{
 			name: "clusterAffinities different affinities have the same affinityName",
@@ -564,7 +564,63 @@ func TestValidatePropagationSpec(t *testing.T) {
 				},
 				Preemption: policyv1alpha1.PreemptAlways,
 			},
-			expectedErr: "name can not be empty if preemption is Always, the empty name may cause unexpected resources preemption",
+			expectedErr: "name cannot be empty if preemption is Always, the empty name may cause unexpected resources preemption",
+		},
+		{
+			name: "suspension dispatching cannot co-exist with dispatchingOnClusters",
+			spec: policyv1alpha1.PropagationSpec{
+				Suspension: &policyv1alpha1.Suspension{
+					Dispatching: ptr.To(true),
+					DispatchingOnClusters: &policyv1alpha1.SuspendClusters{
+						ClusterNames: []string{"cluster-name"},
+					},
+				},
+			},
+			expectedErr: "suspension dispatching cannot co-exist with dispatchingOnClusters.clusterNames",
+		},
+		{
+			name: "suspension dispatching with nil dispatchingOnClusters is valid",
+			spec: policyv1alpha1.PropagationSpec{
+				Suspension: &policyv1alpha1.Suspension{
+					Dispatching: ptr.To(true),
+				},
+			},
+			expectedErr: "",
+		},
+		{
+			name: "suspension dispatching with empty dispatching clusters is valid",
+			spec: policyv1alpha1.PropagationSpec{
+				Suspension: &policyv1alpha1.Suspension{
+					Dispatching: ptr.To(true),
+					DispatchingOnClusters: &policyv1alpha1.SuspendClusters{
+						ClusterNames: []string{},
+					},
+				},
+			},
+			expectedErr: "",
+		},
+		{
+			name: "dispatchingOnClusters.clusterNames without dispatching is valid",
+			spec: policyv1alpha1.PropagationSpec{
+				Suspension: &policyv1alpha1.Suspension{
+					DispatchingOnClusters: &policyv1alpha1.SuspendClusters{
+						ClusterNames: []string{"cluster-name"},
+					},
+				},
+			},
+			expectedErr: "",
+		},
+		{
+			name: "dispatchingOnClusters.clusterNames with dispatching false is valid",
+			spec: policyv1alpha1.PropagationSpec{
+				Suspension: &policyv1alpha1.Suspension{
+					Dispatching: ptr.To(false),
+					DispatchingOnClusters: &policyv1alpha1.SuspendClusters{
+						ClusterNames: []string{"cluster-name"},
+					},
+				},
+			},
+			expectedErr: "",
 		},
 	}
 	for _, tt := range tests {