Simplified validation logic and deferred correctness checks to requir…

…ements.Compatible()
aws · Mar 20, 2022 · 1731409 · 1731409
1 parent 50ce3a8
commit 1731409
Show file tree

Hide file tree

Showing 2 changed files with 0 additions and 37 deletions.
diff --git a/pkg/apis/provisioning/v1alpha5/requirements.go b/pkg/apis/provisioning/v1alpha5/requirements.go
@@ -100,8 +100,6 @@ func (r Requirements) Add(requirements ...v1.NodeSelectorRequirement) Requiremen
 			values = sets.NewComplementSet()
 		case v1.NodeSelectorOpDoesNotExist:
 			values = sets.NewSet()
-		default:
-			values = sets.NewSet()
 		}
 		if existing, ok := r.requirements[requirement.Key]; ok {
 			values = values.Intersection(existing)
@@ -180,19 +178,6 @@ func (r Requirements) Validate() (errs error) {
 		if !SupportedNodeSelectorOps.Has(string(requirement.Operator)) {
 			errs = multierr.Append(errs, fmt.Errorf("operator %s not in %s for key %s", requirement.Operator, SupportedNodeSelectorOps.UnsortedList(), requirement.Key))
 		}
-		// Excludes cases when DoesNotExists appears together with In, NotIn, Exists
-		if requirement.Operator == v1.NodeSelectorOpDoesNotExist {
-			if r.hasRequirement(withKeyAndOperator(requirement.Key, v1.NodeSelectorOpIn)) ||
-				r.hasRequirement(withKeyAndOperator(requirement.Key, v1.NodeSelectorOpNotIn)) ||
-				r.hasRequirement(withKeyAndOperator(requirement.Key, v1.NodeSelectorOpExists)) {
-				errs = multierr.Append(errs, fmt.Errorf("operator %s cannot coexist with other operators for key %s", v1.NodeSelectorOpDoesNotExist, requirement.Key))
-			}
-		}
-	}
-	for key := range r.Keys() {
-		if r.Get(key).Len() == 0 && !r.hasRequirement(withKeyAndOperator(key, v1.NodeSelectorOpDoesNotExist)) {
-			errs = multierr.Append(errs, fmt.Errorf("no feasible value for key %s", key))
-		}
 	}
 	return errs
 }
@@ -216,21 +201,6 @@ func (r Requirements) Compatible(requirements Requirements) (errs error) {
 	return errs
 }
 
-func (r Requirements) hasRequirement(f func(v1.NodeSelectorRequirement) bool) bool {
-	for _, requirement := range r.Requirements {
-		if f(requirement) {
-			return true
-		}
-	}
-	return false
-}
-
-func withKeyAndOperator(key string, operator v1.NodeSelectorOperator) func(v1.NodeSelectorRequirement) bool {
-	return func(requirement v1.NodeSelectorRequirement) bool {
-		return key == requirement.Key && requirement.Operator == operator
-	}
-}
-
 func (r *Requirements) MarshalJSON() ([]byte, error) {
 	if r.Requirements == nil {
 		r.Requirements = []v1.NodeSelectorRequirement{}

diff --git a/pkg/apis/provisioning/v1alpha5/suite_test.go b/pkg/apis/provisioning/v1alpha5/suite_test.go
@@ -185,13 +185,6 @@ var _ = Describe("Validation", func() {
 				Expect(provisioner.Validate(ctx)).To(Succeed())
 			}
 		})
-		It("should fail because no feasible value", func() {
-			provisioner.Spec.Requirements = NewRequirements(
-				v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test"}},
-				v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"bar"}},
-			)
-			Expect(provisioner.Validate(ctx)).ToNot(Succeed())
-		})
 		It("should allow non-empty set after removing overlapped value", func() {
 			provisioner.Spec.Requirements = NewRequirements(
 				v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test", "foo"}},