diff --git a/charts/karpenter/crds/karpenter.sh_provisioners.yaml b/charts/karpenter/crds/karpenter.sh_provisioners.yaml index ff6dac202d4d..0430c3b2a457 100644 --- a/charts/karpenter/crds/karpenter.sh_provisioners.yaml +++ b/charts/karpenter/crds/karpenter.sh_provisioners.yaml @@ -1,10 +1,9 @@ - --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.7.0 + controller-gen.kubebuilder.io/version: v0.8.0 creationTimestamp: null name: provisioners.karpenter.sh spec: diff --git a/pkg/apis/provisioning/v1alpha5/requirements.go b/pkg/apis/provisioning/v1alpha5/requirements.go index 412e7bf8f76a..2e41f488d7ec 100644 --- a/pkg/apis/provisioning/v1alpha5/requirements.go +++ b/pkg/apis/provisioning/v1alpha5/requirements.go @@ -211,7 +211,10 @@ func (r Requirements) Validate() (errs *apis.FieldError) { if !functional.ContainsString(SupportedNodeSelectorOps, string(requirement.Operator)) { errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s not in %s", requirement.Operator, SupportedNodeSelectorOps), "operator")) } - if requirement.Operator == v1.NodeSelectorOpDoesNotExist && !r.Values(requirement.Key).Complement { + // Excludes cases when DoesNotExists appears together with In, NotIn, Exists + if requirement.Operator == v1.NodeSelectorOpDoesNotExist && (r.hasRequirement(withKeyAndOperator(requirement.Key, v1.NodeSelectorOpIn)) || + r.hasRequirement(withKeyAndOperator(requirement.Key, v1.NodeSelectorOpNotIn)) || + r.hasRequirement(withKeyAndOperator(requirement.Key, v1.NodeSelectorOpExists))) { errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("operator %s and %s conflict", v1.NodeSelectorOpDoesNotExist, v1.NodeSelectorOpDoesNotExist), "operator")) } } @@ -236,17 +239,24 @@ func (r Requirements) Compatible(requirements Requirements) (errs *apis.FieldErr if values := r.Values(key); values.Intersection(requirements.Values(key)).Len() == 0 { errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s not in %s", values, requirements.Values(key)), "values")).ViaFieldIndex("requirements", i) } - // Exists incompatible with DoesNotExist or undefined - if requirements.hasRequirement(withKeyAndOperator(key, v1.NodeSelectorOpExists)) { - if r.hasRequirement(withKeyAndOperator(key, v1.NodeSelectorOpDoesNotExist)) || !r.hasRequirement(withKey(key)) { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s prohibits %s", v1.NodeSelectorOpExists, v1.NodeSelectorOpDoesNotExist), "operator")).ViaFieldIndex("requirements", i) + + for r1, r2 := range map[*Requirements]*Requirements{ + &r: &requirements, + &requirements: &r, + } { + // Exists incompatible with DoesNotExist or undefined + if r1.hasRequirement(withKeyAndOperator(key, v1.NodeSelectorOpExists)) { + if r2.hasRequirement(withKeyAndOperator(key, v1.NodeSelectorOpDoesNotExist)) || !r2.hasRequirement(withKey(key)) { + errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s prohibits %s", v1.NodeSelectorOpExists, v1.NodeSelectorOpDoesNotExist), "operator")).ViaFieldIndex("requirements", i) + } } - } - // DoesNotExist requires DoesNotExist or undefined - if requirements.hasRequirement(withKeyAndOperator(key, v1.NodeSelectorOpDoesNotExist)) { - if !(r.hasRequirement(withKeyAndOperator(key, v1.NodeSelectorOpDoesNotExist)) || !r.hasRequirement(withKey(key))) { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s requires %s", v1.NodeSelectorOpDoesNotExist, v1.NodeSelectorOpDoesNotExist), "operator")).ViaFieldIndex("requirements", i) + // DoesNotExist requires DoesNotExist or undefined + if r1.hasRequirement(withKeyAndOperator(key, v1.NodeSelectorOpDoesNotExist)) { + if !(r2.hasRequirement(withKeyAndOperator(key, v1.NodeSelectorOpDoesNotExist)) || !r2.hasRequirement(withKey(key))) { + errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s requires %s", v1.NodeSelectorOpDoesNotExist, v1.NodeSelectorOpDoesNotExist), "operator")).ViaFieldIndex("requirements", i) + } } + } } return errs diff --git a/pkg/apis/provisioning/v1alpha5/suite_test.go b/pkg/apis/provisioning/v1alpha5/suite_test.go index 0b73d57db6ff..6037f6ce3ba6 100644 --- a/pkg/apis/provisioning/v1alpha5/suite_test.go +++ b/pkg/apis/provisioning/v1alpha5/suite_test.go @@ -181,11 +181,13 @@ var _ = Describe("Validation", func() { provisioner.Spec.Requirements = NewRequirements() Expect(provisioner.Validate(ctx)).To(Succeed()) }) - It("should fail because In and DoesNotExists conflicting", func() { - provisioner.Spec.Requirements = NewRequirements( - v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}, - v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpDoesNotExist}, - ) + It("should fail because DoesNotExists conflicting", func() { + for _, op := range []v1.NodeSelectorOperator{v1.NodeSelectorOpIn, v1.NodeSelectorOpNotIn, v1.NodeSelectorOpExists} { + provisioner.Spec.Requirements = NewRequirements( + v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOperator(op), Values: []string{"test"}}, + v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpDoesNotExist}, + ) + } Expect(provisioner.Validate(ctx)).ToNot(Succeed()) }) It("should normalize aliased labels", func() { @@ -199,7 +201,7 @@ var _ = Describe("Validation", func() { B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"foo"}}) Expect(A.Compatible(B)).To(Succeed()) }) - It("A should fail to be compatible to B, operator", func() { + It("A should fail to be compatible to B, operaton, no overlap", func() { A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test", "foo"}}) B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"bar"}}) Expect(A.Compatible(B)).ToNot(Succeed()) @@ -209,35 +211,126 @@ var _ = Describe("Validation", func() { B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpNotIn, Values: []string{"foo"}}) Expect(A.Compatible(B)).To(Succeed()) }) - It("A should fail to be compatible to B, operator", func() { + It("A should fail to be compatible to B, operator, cancel out", func() { A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"foo"}}) B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpNotIn, Values: []string{"foo"}}) Expect(A.Compatible(B)).ToNot(Succeed()) }) - It("A should be compatible to B, operator", func() { + It("A should be compatible to B, operator", func() { A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test", "foo"}}) B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpExists}) Expect(A.Compatible(B)).To(Succeed()) }) - It("A should fail to be compatible to B, operator", func() { + It("A should fail to be compatible to B, operator, conflicting", func() { A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test", "foo"}}) B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpDoesNotExist}) Expect(A.Compatible(B)).ToNot(Succeed()) }) + It("A should be compatible to B, operator", func() { + A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpNotIn, Values: []string{"foo"}}) + B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test", "foo"}}) + Expect(A.Compatible(B)).To(Succeed()) + }) + It("A should fail to be compatible to B, operator, cancel out", func() { + A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpNotIn, Values: []string{"foo"}}) + B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"foo"}}) + Expect(A.Compatible(B)).ToNot(Succeed()) + }) + It("A should be compatible to B, operator", func() { + A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpNotIn, Values: []string{"foo"}}) + B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpNotIn, Values: []string{"test", "foo"}}) + Expect(A.Compatible(B)).To(Succeed()) + }) + It("A should be compatible to B, operator", func() { + A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpNotIn, Values: []string{"test", "foo"}}) + B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpExists}) + Expect(A.Compatible(B)).To(Succeed()) + }) + It("A should fail to be compatible to B, operator, conflicting", func() { + A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpNotIn, Values: []string{"test", "foo"}}) + B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpDoesNotExist}) + Expect(A.Compatible(B)).ToNot(Succeed()) + }) + + It("A should be compatible to B, operator", func() { + A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpExists}) + B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"foo"}}) + Expect(A.Compatible(B)).To(Succeed()) + }) + It("A should be compatible to B, operator", func() { + A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpExists}) + B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpNotIn, Values: []string{"foo"}}) + Expect(A.Compatible(B)).To(Succeed()) + }) + It("A should be compatible to B, operator", func() { + A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpExists}) + B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpExists}) + Expect(A.Compatible(B)).To(Succeed()) + }) + It("A should fail to be compatible to B, operaton, conflicting", func() { + A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpExists}) + B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpDoesNotExist}) + Expect(A.Compatible(B)).ToNot(Succeed()) + }) + It("A should fail to be compatible to B, operator, conflicting", func() { + A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpDoesNotExist}) + B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"foo"}}) + Expect(A.Compatible(B)).ToNot(Succeed()) + }) + It("A should fail to be compatible to B, operator, conflicting", func() { + A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpDoesNotExist}) + B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpNotIn, Values: []string{"foo"}}) + Expect(A.Compatible(B)).ToNot(Succeed()) + }) + It("A should fail to be compatible to B, operator, conflicting", func() { + A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpDoesNotExist}) + B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpExists}) + Expect(A.Compatible(B)).ToNot(Succeed()) + }) + It("A should be compatible to B, operator", func() { + A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpDoesNotExist}) + B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpDoesNotExist}) + Expect(A.Compatible(B)).To(Succeed()) + }) + It("A should be compatible to B, operator", func() { + A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"foo"}}) + B := NewRequirements() + Expect(A.Compatible(B)).To(Succeed()) + }) + It("A should fail to be compatible to B, operator, indirectional", func() { + A := NewRequirements() + B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"foo"}}) + Expect(A.Compatible(B)).ToNot(Succeed()) + }) It("A should be compatible to B, operator", func() { A := NewRequirements() B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpNotIn, Values: []string{"foo"}}) Expect(A.Compatible(B)).To(Succeed()) }) - It("A should fail to be compatible to B, operator", func() { + It("A should be compatible to B, operator", func() { + A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpNotIn, Values: []string{"foo"}}) + B := NewRequirements() + Expect(A.Compatible(B)).To(Succeed()) + }) + It("A should fail to be compatible to B,, operator, conflicting", func() { A := NewRequirements() - B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"foo"}}) + B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpExists}) Expect(A.Compatible(B)).ToNot(Succeed()) }) - It("A should not be compatible to B, operator", func() { - A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test", "foo"}}) - B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpDoesNotExist}) + It("A should fail to be compatible to B, operator, conflicting", func() { + A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpExists}) + B := NewRequirements() Expect(A.Compatible(B)).ToNot(Succeed()) }) + It("A should be compatible to B, operator", func() { + A := NewRequirements() + B := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpDoesNotExist, Values: []string{"foo"}}) + Expect(A.Compatible(B)).To(Succeed()) + }) + It("A should be compatible to B, operator", func() { + A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpDoesNotExist, Values: []string{"foo"}}) + B := NewRequirements() + Expect(A.Compatible(B)).To(Succeed()) + }) }) }) diff --git a/pkg/controllers/provisioning/scheduling/suite_test.go b/pkg/controllers/provisioning/scheduling/suite_test.go index 85fd9daab369..12fb4836fb39 100644 --- a/pkg/controllers/provisioning/scheduling/suite_test.go +++ b/pkg/controllers/provisioning/scheduling/suite_test.go @@ -78,7 +78,6 @@ var _ = AfterEach(func() { ExpectProvisioningCleanedUp(ctx, env.Client, provisioners) }) - var _ = Describe("Constraints", func() { Context("Custom Labels", func() { It("should schedule unconstrained pods that don't have matching node selectors", func() {