From 97e9714832b9702d81a7f39bcd8cc5faf564158d Mon Sep 17 00:00:00 2001 From: Ellis Tarn Date: Fri, 28 Jan 2022 10:59:54 -0800 Subject: [PATCH] Cleanups for requirements refactor PR --- .../crds/karpenter.sh_provisioners.yaml | 3 +- pkg/apis/provisioning/v1alpha5/constraints.go | 12 +- .../v1alpha5/provisioner_validation.go | 4 +- .../provisioning/v1alpha5/requirements.go | 309 ++++++++---------- pkg/apis/provisioning/v1alpha5/suite_test.go | 96 ++---- .../v1alpha5/zz_generated.deepcopy.go | 10 +- .../aws/apis/v1alpha1/provider_defaults.go | 5 +- pkg/cloudprovider/aws/instance.go | 7 +- pkg/cloudprovider/aws/suite_test.go | 6 +- .../provisioning/binpacking/packable.go | 4 +- pkg/controllers/provisioning/controller.go | 2 +- .../provisioning/scheduling/suite_test.go | 80 +++-- .../provisioning/scheduling/topology.go | 11 +- .../provisioning/scheduling/topologygroup.go | 4 +- pkg/utils/sets/sets.go | 62 ++-- 15 files changed, 263 insertions(+), 352 deletions(-) diff --git a/charts/karpenter/crds/karpenter.sh_provisioners.yaml b/charts/karpenter/crds/karpenter.sh_provisioners.yaml index 7f4d637faf22..ff6dac202d4d 100644 --- a/charts/karpenter/crds/karpenter.sh_provisioners.yaml +++ b/charts/karpenter/crds/karpenter.sh_provisioners.yaml @@ -4,7 +4,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.4.1 + controller-gen.kubebuilder.io/version: v0.7.0 creationTimestamp: null name: provisioners.karpenter.sh spec: @@ -170,7 +170,6 @@ spec: transitioned from one status to another. We use VolatileTime in place of metav1.Time to exclude this from creating equality.Semantic differences (all other things held constant). - format: date-time type: string message: description: A human readable message indicating details about diff --git a/pkg/apis/provisioning/v1alpha5/constraints.go b/pkg/apis/provisioning/v1alpha5/constraints.go index f6aef10b45ed..30e3d8457a58 100644 --- a/pkg/apis/provisioning/v1alpha5/constraints.go +++ b/pkg/apis/provisioning/v1alpha5/constraints.go @@ -33,7 +33,7 @@ type Constraints struct { // +optional Taints Taints `json:"taints,omitempty"` // Requirements are layered with Labels and applied to every node. - Requirements *Requirements `json:"requirements,inline,omitempty"` + Requirements Requirements `json:"requirements,inline,omitempty"` // KubeletConfiguration are options passed to the kubelet when provisioning nodes //+optional KubeletConfiguration KubeletConfiguration `json:"kubeletConfiguration,omitempty"` @@ -52,13 +52,13 @@ func (c *Constraints) ValidatePod(pod *v1.Pod) error { return err } // Test if pod requirements are valid - podRequirements := NewRequirements(PodRequirements(pod)...) - if errs := podRequirements.Validate(); errs != nil { + requirements := NewPodRequirements(pod) + if errs := requirements.Validate(); errs != nil { return fmt.Errorf("pod requirements not feasible, %v", errs) } // Test if pod requirements are compatible - if errs := c.Requirements.Compatible(podRequirements); errs != nil { - return fmt.Errorf("incompatible requirements %w", errs) + if errs := c.Requirements.Compatible(requirements); errs != nil { + return fmt.Errorf("incompatible requirements, %w", errs) } return nil } @@ -66,7 +66,7 @@ func (c *Constraints) ValidatePod(pod *v1.Pod) error { func (c *Constraints) Tighten(pod *v1.Pod) *Constraints { return &Constraints{ Labels: c.Labels, - Requirements: c.Requirements.Add(PodRequirements(pod)...).WellKnown(), + Requirements: c.Requirements.Add(NewPodRequirements(pod).Requirements...).WellKnown(), Taints: c.Taints, Provider: c.Provider, KubeletConfiguration: c.KubeletConfiguration, diff --git a/pkg/apis/provisioning/v1alpha5/provisioner_validation.go b/pkg/apis/provisioning/v1alpha5/provisioner_validation.go index e9df59b06484..b8e5d17476f0 100644 --- a/pkg/apis/provisioning/v1alpha5/provisioner_validation.go +++ b/pkg/apis/provisioning/v1alpha5/provisioner_validation.go @@ -136,8 +136,8 @@ func (c *Constraints) validateTaints() (errs *apis.FieldError) { // When this function is called, the provisioner's requirments do not include the requirements from labels. // Provisioner requirements only support well known labels. func (c *Constraints) validateRequirements() (errs *apis.FieldError) { - // validate if requirement keys are well known labels - for _, key := range c.Requirements.Keys() { + // Ensure requirements are well known + for key := range c.Requirements.Keys() { if !WellKnownLabels.Has(key) { errs = errs.Also(apis.ErrInvalidKeyName(fmt.Sprintf("%s not in %v", key, WellKnownLabels.UnsortedList()), "key")) } diff --git a/pkg/apis/provisioning/v1alpha5/requirements.go b/pkg/apis/provisioning/v1alpha5/requirements.go index a4189b2b2954..412e7bf8f76a 100644 --- a/pkg/apis/provisioning/v1alpha5/requirements.go +++ b/pkg/apis/provisioning/v1alpha5/requirements.go @@ -81,116 +81,37 @@ var ( ) ) +// Requirements are an alias type that wrap []v1.NodeSelectorRequirement and +// include an efficient set representation under the hood. Since its underlying +// types are slices and maps, this type should not be used as a pointer. type Requirements struct { // Requirements are layered with Labels and applied to every node. Requirements []v1.NodeSelectorRequirement `json:"requirements,omitempty"` - Allows map[string]sets.Set `json:"-"` + requirements map[string]sets.Set `json:"-"` } -func NewRequirements(requirements ...v1.NodeSelectorRequirement) *Requirements { - result := Requirements{ - Requirements: []v1.NodeSelectorRequirement{}, - Allows: map[string]sets.Set{}, - } - return result.Add(requirements...) -} - -// Add function returns a new Requirements object with new requirements inserted. -// It is critical to keep the original Requirements object immutable to keep the -// requirement hashing behavior consistent. -func (r *Requirements) Add(requirements ...v1.NodeSelectorRequirement) *Requirements { - if r == nil { - return NewRequirements(requirements...) - } - rd := r.DeepCopy() - for _, requirement := range requirements { - if newKey, ok := NormalizedLabels[requirement.Key]; ok { - requirement.Key = newKey - } - rd.Requirements = append(rd.Requirements, requirement) - switch requirement.Operator { - case v1.NodeSelectorOpIn: - rd.Allows[requirement.Key] = rd.Allow(requirement.Key).Intersection(sets.NewSet(requirement.Values...)) - case v1.NodeSelectorOpNotIn: - rd.Allows[requirement.Key] = rd.Allow(requirement.Key).Intersection(sets.NewComplementSet(requirement.Values...)) - } - } - return rd -} - -func (r *Requirements) MarshalJSON() ([]byte, error) { - return json.Marshal(r.Requirements) -} - -func (r *Requirements) UnmarshalJSON(b []byte) error { - var requirements []v1.NodeSelectorRequirement - json.Unmarshal(b, &requirements) - result := NewRequirements(requirements...) - r.Requirements = result.Requirements - r.Allows = result.Allows - return nil -} - -// Allow returns the sets of values allowed by all included requirements -// Allow follows a denylist method. Values are allowed except specified -func (r *Requirements) Allow(key string) sets.Set { - if r == nil { - return sets.NewComplementSet() - } - if _, ok := r.Allows[key]; !ok { - return sets.NewComplementSet() - } - return r.Allows[key] +// NewRequirements constructs requiremnets from NodeSelectorRequirements +func NewRequirements(requirements ...v1.NodeSelectorRequirement) Requirements { + return Requirements{requirements: map[string]sets.Set{}}.Add(requirements...) } -// Validate validates the feasibility of the requirements. -func (r *Requirements) Validate() (errs *apis.FieldError) { - if r == nil { - return errs - } - for index, requirement := range r.Requirements { - for _, err := range validation.IsQualifiedName(requirement.Key) { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s, %s", requirement.Key, err), "key")) - } - for _, value := range requirement.Values { - for _, err := range validation.IsValidLabelValue(value) { - errs = errs.Also(apis.ErrInvalidArrayValue(fmt.Sprintf("%s, %s", value, err), "values", index)) - } - - } - 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.Allow(requirement.Key).IsComplement { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("Operator In and DoesNotExists conflicts, %s", requirement.Key), "values")) - } - } - for _, key := range r.Keys() { - values := r.Allow(key) - if values.Len() == 0 { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("no feasible value for requirement, %s", key), "values")) - } - - } - return errs -} - -// FromLabels constructs requriements from labels -func LabelRequirements(labels map[string]string) []v1.NodeSelectorRequirement { +// NewLabelRequirements constructs requriements from labels +func NewLabelRequirements(labels map[string]string) Requirements { requirements := []v1.NodeSelectorRequirement{} for key, value := range labels { requirements = append(requirements, v1.NodeSelectorRequirement{Key: key, Operator: v1.NodeSelectorOpIn, Values: []string{value}}) } - return requirements + return NewRequirements(requirements...) } -func PodRequirements(pod *v1.Pod) []v1.NodeSelectorRequirement { +// NewPodRequirements constructs requirements from a pod +func NewPodRequirements(pod *v1.Pod) Requirements { requirements := []v1.NodeSelectorRequirement{} for key, value := range pod.Spec.NodeSelector { requirements = append(requirements, v1.NodeSelectorRequirement{Key: key, Operator: v1.NodeSelectorOpIn, Values: []string{value}}) } if pod.Spec.Affinity == nil || pod.Spec.Affinity.NodeAffinity == nil { - return requirements + return NewRequirements(requirements...) } // The legal operators for pod affinity and anti-affinity are In, NotIn, Exists, DoesNotExist. // Select heaviest preference and treat as a requirement. An outer loop will iteratively unconstrain them if unsatisfiable. @@ -203,124 +124,160 @@ func PodRequirements(pod *v1.Pod) []v1.NodeSelectorRequirement { len(pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms) > 0 { requirements = append(requirements, pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions...) } - return requirements + return NewRequirements(requirements...) } -// Compatible returns errors with detailed messages when requirements are not compatible -// Compatible is non-commutative (i.e., A.Compatible(B) != B.Compatible(A)) -func (r *Requirements) Compatible(requirements *Requirements) (errs *apis.FieldError) { - if r == nil { - errs = errs.Also(apis.ErrInvalidValue("nil requirements", "values")) - return errs - } - allKeys := stringsets.NewString(r.Keys()...).Union(stringsets.NewString(requirements.Keys()...)) - for key := range allKeys { - if err := r.isKeyDefined(key, requirements); err != nil { - errs = errs.Also(err) - } - if err := r.hasCommons(key, requirements); err != nil { - errs = errs.Also(err) - } - if err := r.satisfyExistOperators(key, requirements); err != nil { - errs = errs.Also(err) +func (r Requirements) WellKnown() Requirements { + requirements := []v1.NodeSelectorRequirement{} + for _, requirement := range r.Requirements { + if WellKnownLabels.Has(requirement.Key) { + requirements = append(requirements, requirement) } } - return errs + return NewRequirements(requirements...) } -// isKeyDefined returns errors when the given requirements requires a key that is not defined by this requirements -// This is a non-commutative function designed to catch the following cases: -// * Pod Spec has a label selector but provisioner doesn't have the label -// * One pod is not compatible with a schedule (group of pods) due to extra requirements -func (r *Requirements) isKeyDefined(key string, requirements *Requirements) (errs *apis.FieldError) { - if !requirements.Allow(key).IsComplement && requirements.Allow(key).Len() != 0 && len(r.filter(key)) == 0 { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("key %s, need %s but not defined", key, requirements.Allow(key)), "values")) +// Add function returns a new Requirements object with new requirements inserted. +func (r Requirements) Add(requirements ...v1.NodeSelectorRequirement) Requirements { + // Deep copy to avoid mutating existing requirements + r = *r.DeepCopy() + if r.requirements == nil { + r.requirements = map[string]sets.Set{} } - return errs + for _, requirement := range requirements { + if normalized, ok := NormalizedLabels[requirement.Key]; ok { + requirement.Key = normalized + } + r.Requirements = append(r.Requirements, requirement) + switch requirement.Operator { + case v1.NodeSelectorOpIn: + r.requirements[requirement.Key] = r.Values(requirement.Key).Intersection(sets.NewSet(requirement.Values...)) + case v1.NodeSelectorOpNotIn: + r.requirements[requirement.Key] = r.Values(requirement.Key).Intersection(sets.NewComplementSet(requirement.Values...)) + } + } + return r } -// hasCommons returns errors when there is no overlap among the requirements' allowed values for a provided key. -func (r *Requirements) hasCommons(key string, requirements *Requirements) (errs *apis.FieldError) { - if r.Allow(key).Intersection(requirements.Allow(key)).Len() == 0 { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("no common values for key %s, %s not in %s", key, r.Allow(key), requirements.Allow(key)), "values")) +// Keys returns unique set of the label keys from the requirements +func (r Requirements) Keys() stringsets.String { + keys := stringsets.NewString() + for _, requirement := range r.Requirements { + keys.Insert(requirement.Key) } - return errs + return keys } -// satisfyExistOperators returns errors with detailed messages when Exist or DoesNotExist operators requirements are violated. -func (r *Requirements) satisfyExistOperators(key string, requirements *Requirements) (errs *apis.FieldError) { - rf1 := r.filter(key) - rf2 := requirements.filter(key) - for r1, r2 := range map[*[]v1.NodeSelectorRequirement]*[]v1.NodeSelectorRequirement{ - &rf1: &rf2, - &rf2: &rf1, - } { - for _, requirement := range *r1 { - switch requirement.Operator { - case v1.NodeSelectorOpExists: - if len(*r2) == 0 { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("Exist operator violation, %s", key), "values")) - } - case v1.NodeSelectorOpDoesNotExist: - if len(*r2) > 0 { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("DoesNotExist operator violation, %s", key), "values")) - } - } - - } +// Values returns the sets of values allowed by all included requirements +// following a denylist method. Values are allowed except specified +func (r Requirements) Values(key string) sets.Set { + if _, ok := r.requirements[key]; !ok { + return sets.NewComplementSet() } - return errs + return r.requirements[key] } -func (r *Requirements) Zones() sets.Set { - return r.Allow(v1.LabelTopologyZone) +func (r Requirements) Zones() stringsets.String { + return r.Values(v1.LabelTopologyZone).Values() } -func (r *Requirements) InstanceTypes() sets.Set { - return r.Allow(v1.LabelInstanceTypeStable) +func (r Requirements) InstanceTypes() stringsets.String { + return r.Values(v1.LabelInstanceTypeStable).Values() } -func (r *Requirements) Architectures() sets.Set { - return r.Allow(v1.LabelArchStable) +func (r Requirements) Architectures() stringsets.String { + return r.Values(v1.LabelArchStable).Values() } -func (r *Requirements) OperatingSystems() sets.Set { - return r.Allow(v1.LabelOSStable) +func (r Requirements) OperatingSystems() stringsets.String { + return r.Values(v1.LabelOSStable).Values() } -func (r *Requirements) CapacityTypes() sets.Set { - return r.Allow(LabelCapacityType) +func (r Requirements) CapacityTypes() stringsets.String { + return r.Values(LabelCapacityType).Values() } -func (r *Requirements) WellKnown() *Requirements { - requirements := []v1.NodeSelectorRequirement{} - for _, requirement := range r.Requirements { - if WellKnownLabels.Has(requirement.Key) { - requirements = append(requirements, requirement) +// Validate validates the feasibility of the requirements. +func (r Requirements) Validate() (errs *apis.FieldError) { + for i, requirement := range r.Requirements { + for _, err := range validation.IsQualifiedName(requirement.Key) { + errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s, %s", requirement.Key, err), "key")) + } + for _, value := range requirement.Values { + for _, err := range validation.IsValidLabelValue(value) { + errs = errs.Also(apis.ErrInvalidArrayValue(fmt.Sprintf("%s, %s", value, err), "values", i)) + } + } + 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 { + errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("operator %s and %s conflict", v1.NodeSelectorOpDoesNotExist, v1.NodeSelectorOpDoesNotExist), "operator")) } } - return NewRequirements(requirements...) + for key := range r.Keys() { + values := r.Values(key) + if values.Len() == 0 { + errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("no feasible value for requirement, %s", key), "values")) + } + } + return errs } -// Keys returns unique set of the label keys from the requirements -func (r *Requirements) Keys() []string { - if r == nil { - return []string{} +// Compatible ensures the provided requirements can be met. It is +// non-commutative (i.e., A.Compatible(B) != B.Compatible(A)) +func (r Requirements) Compatible(requirements Requirements) (errs *apis.FieldError) { + for i, key := range r.Keys().Union(requirements.Keys()).UnsortedList() { + // Key must be defined if required + if values := requirements.Values(key); values.Len() != 0 && !values.Complement && !r.hasRequirement(withKey(key)) { + errs = errs.Also(apis.ErrInvalidValue("is not defined", "key")).ViaFieldIndex("requirements", i) + } + // Values must overlap + 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) + } + } + // 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) + } + } } - keys := make([]string, 0, len(r.Requirements)) + return errs +} + +func (r Requirements) hasRequirement(f func(v1.NodeSelectorRequirement) bool) bool { for _, requirement := range r.Requirements { - keys = append(keys, requirement.Key) + if f(requirement) { + return true + } } + return false +} - return sets.NewSet(keys...).Values() +func withKey(key string) func(v1.NodeSelectorRequirement) bool { + return func(requirement v1.NodeSelectorRequirement) bool { return requirement.Key == key } } -func (r *Requirements) filter(key string) []v1.NodeSelectorRequirement { - result := []v1.NodeSelectorRequirement{} - for _, requirement := range r.Requirements { - if requirement.Key == key { - result = append(result, requirement) - } +func withKeyAndOperator(key string, operator v1.NodeSelectorOperator) func(v1.NodeSelectorRequirement) bool { + return func(requirement v1.NodeSelectorRequirement) bool { + return key == requirement.Key && requirement.Operator == operator } - return result +} + +func (r *Requirements) MarshalJSON() ([]byte, error) { + return json.Marshal(r.Requirements) +} + +func (r *Requirements) UnmarshalJSON(b []byte) error { + var requirements []v1.NodeSelectorRequirement + json.Unmarshal(b, &requirements) + *r = NewRequirements(requirements...) + return nil } diff --git a/pkg/apis/provisioning/v1alpha5/suite_test.go b/pkg/apis/provisioning/v1alpha5/suite_test.go index dd91ba9eda52..0b73d57db6ff 100644 --- a/pkg/apis/provisioning/v1alpha5/suite_test.go +++ b/pkg/apis/provisioning/v1alpha5/suite_test.go @@ -26,7 +26,6 @@ import ( "knative.dev/pkg/ptr" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" ) @@ -44,16 +43,8 @@ var _ = Describe("Validation", func() { BeforeEach(func() { provisioner = &Provisioner{ - ObjectMeta: metav1.ObjectMeta{ - Name: strings.ToLower(randomdata.SillyName()), - }, - Spec: ProvisionerSpec{ - Limits: Limits{ - Resources: v1.ResourceList{ - v1.ResourceCPU: *resource.NewScaledQuantity(10, 0), - }, - }, - }, + ObjectMeta: metav1.ObjectMeta{Name: strings.ToLower(randomdata.SillyName())}, + Spec: ProvisionerSpec{}, } }) @@ -143,8 +134,8 @@ var _ = Describe("Validation", func() { provisioner.Spec.Requirements = NewRequirements( v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}, v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpNotIn, Values: []string{"bar"}}, - v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpExists, Values: []string{}}, - v1.NodeSelectorRequirement{Key: v1.LabelOSStable, Operator: v1.NodeSelectorOpDoesNotExist, Values: []string{}}, + v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpExists}, + v1.NodeSelectorRequirement{Key: v1.LabelOSStable, Operator: v1.NodeSelectorOpDoesNotExist}, ) Expect(provisioner.Validate(ctx)).To(Succeed()) }) @@ -193,7 +184,7 @@ var _ = Describe("Validation", func() { 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, Values: []string{}}, + v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpDoesNotExist}, ) Expect(provisioner.Validate(ctx)).ToNot(Succeed()) }) @@ -201,74 +192,51 @@ var _ = Describe("Validation", func() { provisioner.Spec.Requirements = NewRequirements( v1.NodeSelectorRequirement{Key: v1.LabelFailureDomainBetaZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}, ) - Expect(provisioner.Spec.Requirements.Keys()).To(Equal([]string{v1.LabelTopologyZone})) + Expect(provisioner.Spec.Requirements.Keys().UnsortedList()).To(Equal([]string{v1.LabelTopologyZone})) }) 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.NodeSelectorOpIn, Values: []string{"foo"}}, - ) + 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{"foo"}}) Expect(A.Compatible(B)).To(Succeed()) }) It("A should fail to 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.NodeSelectorOpIn, Values: []string{"bar"}}, - ) + 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()) }) 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.NodeSelectorOpNotIn, Values: []string{"foo"}}, - ) + A := NewRequirements(v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test", "foo"}}) + 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() { + 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() { + 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() { + 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() - B := NewRequirements( - v1.NodeSelectorRequirement{Key: v1.LabelTopologyRegion, Operator: v1.NodeSelectorOpNotIn, Values: []string{"foo"}}, - ) + 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() { A := NewRequirements() - B := NewRequirements( - v1.NodeSelectorRequirement{Key: v1.LabelTopologyRegion, Operator: v1.NodeSelectorOpIn, 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 fail to 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.LabelTopologyRegion, Operator: v1.NodeSelectorOpExists, Values: []string{}}, - ) - Expect(A.Compatible(B)).ToNot(Succeed()) - }) - 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.LabelTopologyRegion, Operator: v1.NodeSelectorOpDoesNotExist, Values: []string{}}, - ) - Expect(A.Compatible(B)).To(Succeed()) - }) - It("A should fail to 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, Values: []string{}}, - ) + 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}) Expect(A.Compatible(B)).ToNot(Succeed()) }) }) diff --git a/pkg/apis/provisioning/v1alpha5/zz_generated.deepcopy.go b/pkg/apis/provisioning/v1alpha5/zz_generated.deepcopy.go index ec59ff218bfc..9d40280ebfdd 100644 --- a/pkg/apis/provisioning/v1alpha5/zz_generated.deepcopy.go +++ b/pkg/apis/provisioning/v1alpha5/zz_generated.deepcopy.go @@ -43,11 +43,7 @@ func (in *Constraints) DeepCopyInto(out *Constraints) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.Requirements != nil { - in, out := &in.Requirements, &out.Requirements - *out = new(Requirements) - (*in).DeepCopyInto(*out) - } + in.Requirements.DeepCopyInto(&out.Requirements) in.KubeletConfiguration.DeepCopyInto(&out.KubeletConfiguration) if in.Provider != nil { in, out := &in.Provider, &out.Provider @@ -238,8 +234,8 @@ func (in *Requirements) DeepCopyInto(out *Requirements) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.Allows != nil { - in, out := &in.Allows, &out.Allows + if in.requirements != nil { + in, out := &in.requirements, &out.requirements *out = make(map[string]sets.Set, len(*in)) for key, val := range *in { (*out)[key] = val.DeepCopy() diff --git a/pkg/cloudprovider/aws/apis/v1alpha1/provider_defaults.go b/pkg/cloudprovider/aws/apis/v1alpha1/provider_defaults.go index 4255b9674d6d..301034ad808f 100644 --- a/pkg/cloudprovider/aws/apis/v1alpha1/provider_defaults.go +++ b/pkg/cloudprovider/aws/apis/v1alpha1/provider_defaults.go @@ -18,7 +18,6 @@ import ( "context" "github.com/aws/karpenter/pkg/apis/provisioning/v1alpha5" - "github.com/aws/karpenter/pkg/utils/functional" v1 "k8s.io/api/core/v1" ) @@ -32,7 +31,7 @@ func (c *Constraints) defaultCapacityTypes() { if _, ok := c.Labels[v1alpha5.LabelCapacityType]; ok { return } - if functional.ContainsString(c.Requirements.Keys(), v1alpha5.LabelCapacityType) { + if c.Requirements.Keys().Has(v1alpha5.LabelCapacityType) { return } c.Requirements = c.Requirements.Add(v1.NodeSelectorRequirement{ @@ -46,7 +45,7 @@ func (c *Constraints) defaultArchitecture() { if _, ok := c.Labels[v1.LabelArchStable]; ok { return } - if functional.ContainsString(c.Requirements.Keys(), v1.LabelArchStable) { + if c.Requirements.Keys().Has(v1.LabelArchStable) { return } c.Requirements = c.Requirements.Add(v1.NodeSelectorRequirement{ diff --git a/pkg/cloudprovider/aws/instance.go b/pkg/cloudprovider/aws/instance.go index fc12a1ae6597..bd2f3202cd31 100644 --- a/pkg/cloudprovider/aws/instance.go +++ b/pkg/cloudprovider/aws/instance.go @@ -27,6 +27,7 @@ import ( "go.uber.org/multierr" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" "knative.dev/pkg/logging" "github.com/aws/karpenter/pkg/apis/provisioning/v1alpha5" @@ -34,8 +35,6 @@ import ( "github.com/aws/karpenter/pkg/cloudprovider/aws/apis/v1alpha1" "github.com/aws/karpenter/pkg/utils/injection" "github.com/aws/karpenter/pkg/utils/options" - "github.com/aws/karpenter/pkg/utils/sets" - stringsets "k8s.io/apimachinery/pkg/util/sets" ) type InstanceProvider struct { @@ -180,7 +179,7 @@ func (p *InstanceProvider) getLaunchTemplateConfigs(ctx context.Context, constra // getOverrides creates and returns launch template overrides for the cross product of instanceTypeOptions and subnets (with subnets being constrained by // zones and the offerings in instanceTypeOptions) -func (p *InstanceProvider) getOverrides(instanceTypeOptions []cloudprovider.InstanceType, subnets []*ec2.Subnet, zones sets.Set, capacityType string) []*ec2.FleetLaunchTemplateOverridesRequest { +func (p *InstanceProvider) getOverrides(instanceTypeOptions []cloudprovider.InstanceType, subnets []*ec2.Subnet, zones sets.String, capacityType string) []*ec2.FleetLaunchTemplateOverridesRequest { var overrides []*ec2.FleetLaunchTemplateOverridesRequest for i, instanceType := range instanceTypeOptions { for _, offering := range instanceType.Offerings() { @@ -318,7 +317,7 @@ func getInstanceID(node *v1.Node) (*string, error) { } func combineFleetErrors(errors []*ec2.CreateFleetError) (errs error) { - unique := stringsets.NewString() + unique := sets.NewString() for _, err := range errors { unique.Insert(fmt.Sprintf("%s: %s", aws.StringValue(err.ErrorCode), aws.StringValue(err.ErrorMessage))) } diff --git a/pkg/cloudprovider/aws/suite_test.go b/pkg/cloudprovider/aws/suite_test.go index 4336a2887533..388f507b20e0 100644 --- a/pkg/cloudprovider/aws/suite_test.go +++ b/pkg/cloudprovider/aws/suite_test.go @@ -573,10 +573,8 @@ var _ = Describe("Allocation", func() { It("should default requirements", func() { provisioner.SetDefaults(ctx) - capacityType := provisioner.Spec.Requirements.CapacityTypes().Values() - architectures := provisioner.Spec.Requirements.Architectures().Values() - Expect(capacityType).To(ConsistOf(v1alpha1.CapacityTypeOnDemand)) - Expect(architectures).To(ConsistOf(v1alpha5.ArchitectureAmd64)) + Expect(provisioner.Spec.Requirements.CapacityTypes().UnsortedList()).To(ConsistOf(v1alpha1.CapacityTypeOnDemand)) + Expect(provisioner.Spec.Requirements.Architectures().UnsortedList()).To(ConsistOf(v1alpha5.ArchitectureAmd64)) }) }) Context("Validation", func() { diff --git a/pkg/controllers/provisioning/binpacking/packable.go b/pkg/controllers/provisioning/binpacking/packable.go index 74e59bf7f848..81a3252e79d2 100644 --- a/pkg/controllers/provisioning/binpacking/packable.go +++ b/pkg/controllers/provisioning/binpacking/packable.go @@ -22,7 +22,6 @@ import ( "github.com/aws/karpenter/pkg/apis/provisioning/v1alpha5" "github.com/aws/karpenter/pkg/cloudprovider" "github.com/aws/karpenter/pkg/utils/resources" - "github.com/aws/karpenter/pkg/utils/sets" "go.uber.org/multierr" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -188,8 +187,7 @@ func (p *Packable) validateArchitecture(constraints *v1alpha5.Constraints) error } func (p *Packable) validateOperatingSystems(constraints *v1alpha5.Constraints) error { - operatingSystem := sets.NewSet(p.OperatingSystems().UnsortedList()...) - if constraints.Requirements.OperatingSystems().Intersection(operatingSystem).Len() == 0 { + if constraints.Requirements.OperatingSystems().Intersection(p.OperatingSystems()).Len() == 0 { return fmt.Errorf("operating systems %s not in %s", p.Name(), constraints.Requirements.OperatingSystems()) } return nil diff --git a/pkg/controllers/provisioning/controller.go b/pkg/controllers/provisioning/controller.go index 5062b42c2bdb..954d0580dcd6 100644 --- a/pkg/controllers/provisioning/controller.go +++ b/pkg/controllers/provisioning/controller.go @@ -101,7 +101,7 @@ func (c *Controller) Apply(ctx context.Context, provisioner *v1alpha5.Provisione provisioner.Spec.Labels = functional.UnionStringMaps(provisioner.Spec.Labels, map[string]string{v1alpha5.ProvisionerNameLabelKey: provisioner.Name}) provisioner.Spec.Requirements = provisioner.Spec.Requirements. Add(requirements(instanceTypes)...). - Add(v1alpha5.LabelRequirements(provisioner.Spec.Labels)...) + Add(v1alpha5.NewLabelRequirements(provisioner.Spec.Labels).Requirements...) if err := provisioner.Spec.Requirements.Validate(); err != nil { return fmt.Errorf("provisioner requirements validation failed, %v", err) } diff --git a/pkg/controllers/provisioning/scheduling/suite_test.go b/pkg/controllers/provisioning/scheduling/suite_test.go index aee5ce06f5bb..85fd9daab369 100644 --- a/pkg/controllers/provisioning/scheduling/suite_test.go +++ b/pkg/controllers/provisioning/scheduling/suite_test.go @@ -78,7 +78,8 @@ var _ = AfterEach(func() { ExpectProvisioningCleanedUp(ctx, env.Client, provisioners) }) -var _ = Describe("Combined Constraints", func() { + +var _ = Describe("Constraints", func() { Context("Custom Labels", func() { It("should schedule unconstrained pods that don't have matching node selectors", func() { provisioner.Spec.Labels = map[string]string{"test-key": "test-value"} @@ -112,6 +113,43 @@ var _ = Describe("Combined Constraints", func() { ))[0] ExpectNotScheduled(ctx, env.Client, pod) }) + It("should schedule the pod with Exists operator and defined key", func() { + provisioner.Spec.Requirements = v1alpha5.NewRequirements( + v1.NodeSelectorRequirement{Key: "foo", Operator: v1.NodeSelectorOpIn, Values: []string{"bar"}}) + pod := ExpectProvisioned(ctx, env.Client, selectionController, provisioners, provisioner, test.UnschedulablePod( + test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{ + {Key: "foo", Operator: v1.NodeSelectorOpExists}, + }}, + ))[0] + ExpectScheduled(ctx, env.Client, pod) + }) + It("should not schedule the pod with Exists operator and undefined key", func() { + pod := ExpectProvisioned(ctx, env.Client, selectionController, provisioners, provisioner, test.UnschedulablePod( + test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{ + {Key: "foo", Operator: v1.NodeSelectorOpExists}, + }}, + ))[0] + ExpectNotScheduled(ctx, env.Client, pod) + }) + It("should not schedule the pod with DoesNotExists operator and defined key", func() { + provisioner.Spec.Requirements = v1alpha5.NewRequirements( + v1.NodeSelectorRequirement{Key: "foo", Operator: v1.NodeSelectorOpIn, Values: []string{"bar"}}) + pod := ExpectProvisioned(ctx, env.Client, selectionController, provisioners, provisioner, test.UnschedulablePod( + test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{ + {Key: "foo", Operator: v1.NodeSelectorOpDoesNotExist}, + }}, + ))[0] + ExpectNotScheduled(ctx, env.Client, pod) + }) + It("should schedule the pod with DoesNotExists operator and undefined key", func() { + provisioner.Spec.Requirements = v1alpha5.NewRequirements() + pod := ExpectProvisioned(ctx, env.Client, selectionController, provisioners, provisioner, test.UnschedulablePod( + test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{ + {Key: "foo", Operator: v1.NodeSelectorOpDoesNotExist, Values: []string{}}, + }}, + ))[0] + ExpectScheduled(ctx, env.Client, pod) + }) It("should schedule pods that have matching preferences", func() { provisioner.Spec.Labels = map[string]string{"test-key": "test-value"} pod := ExpectProvisioned(ctx, env.Client, selectionController, provisioners, provisioner, test.UnschedulablePod( @@ -165,46 +203,6 @@ var _ = Describe("Combined Constraints", func() { ))[0] ExpectNotScheduled(ctx, env.Client, pod) }) - It("should schedule the pod with Exists operator and defined key", func() { - provisioner.Spec.Requirements = v1alpha5.NewRequirements( - v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test-zone-1"}}) - pod := ExpectProvisioned(ctx, env.Client, selectionController, provisioners, provisioner, test.UnschedulablePod( - test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{ - {Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpExists, Values: []string{}}, - }}, - ))[0] - ExpectScheduled(ctx, env.Client, pod) - }) - It("should not schedule the pod with Exists operator and undefined key", func() { - provisioner.Spec.Requirements = v1alpha5.NewRequirements( - v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test-zone-1"}}) - pod := ExpectProvisioned(ctx, env.Client, selectionController, provisioners, provisioner, test.UnschedulablePod( - test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{ - {Key: "foo", Operator: v1.NodeSelectorOpExists, Values: []string{}}, - }}, - ))[0] - ExpectNotScheduled(ctx, env.Client, pod) - }) - It("should not schedule the pod with DoesNotExists operator and defined key", func() { - provisioner.Spec.Requirements = v1alpha5.NewRequirements( - v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test-zone-1"}}) - pod := ExpectProvisioned(ctx, env.Client, selectionController, provisioners, provisioner, test.UnschedulablePod( - test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{ - {Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpDoesNotExist, Values: []string{}}, - }}, - ))[0] - ExpectNotScheduled(ctx, env.Client, pod) - }) - It("should schedule the pod with Exists operator and undefined key", func() { - provisioner.Spec.Requirements = v1alpha5.NewRequirements( - v1.NodeSelectorRequirement{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test-zone-1"}}) - pod := ExpectProvisioned(ctx, env.Client, selectionController, provisioners, provisioner, test.UnschedulablePod( - test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{ - {Key: "foo", Operator: v1.NodeSelectorOpDoesNotExist, Values: []string{}}, - }}, - ))[0] - ExpectScheduled(ctx, env.Client, pod) - }) It("should schedule compatible requirements with Operator=In", func() { pod := ExpectProvisioned(ctx, env.Client, selectionController, provisioners, provisioner, test.UnschedulablePod( test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{ diff --git a/pkg/controllers/provisioning/scheduling/topology.go b/pkg/controllers/provisioning/scheduling/topology.go index 30096c6735f0..5834fff7070a 100644 --- a/pkg/controllers/provisioning/scheduling/topology.go +++ b/pkg/controllers/provisioning/scheduling/topology.go @@ -46,7 +46,9 @@ func (t *Topology) Inject(ctx context.Context, constraints *v1alpha5.Constraints return fmt.Errorf("computing topology, %w", err) } for _, pod := range topologyGroup.Pods { - domain := topologyGroup.NextDomain(constraints.Requirements.Add(v1alpha5.PodRequirements(pod)...).Allow(topologyGroup.Constraint.TopologyKey)) + domain := topologyGroup.NextDomain(constraints.Requirements.Add(v1alpha5.NewPodRequirements(pod).Requirements...). + Values(topologyGroup.Constraint.TopologyKey). + Values()) pod.Spec.NodeSelector = functional.UnionStringMaps(pod.Spec.NodeSelector, map[string]string{topologyGroup.Constraint.TopologyKey: domain}) } } @@ -79,7 +81,7 @@ func (t *Topology) computeCurrentTopology(ctx context.Context, constraints *v1al case v1.LabelHostname: return t.computeHostnameTopology(topologyGroup, constraints) case v1.LabelTopologyZone: - return t.computeZonalTopology(ctx, constraints.Requirements, topologyGroup) + return t.computeZonalTopology(ctx, constraints, topologyGroup) default: return nil } @@ -108,9 +110,8 @@ func (t *Topology) computeHostnameTopology(topologyGroup *TopologyGroup, constra // topology skew calculations will only include the current viable zone // selection. For example, if a cloud provider or provisioner changes the viable // set of nodes, topology calculations will rebalance the new set of zones. -func (t *Topology) computeZonalTopology(ctx context.Context, requirements *v1alpha5.Requirements, topologyGroup *TopologyGroup) error { - zones := requirements.Zones().Values() - topologyGroup.Register(zones...) +func (t *Topology) computeZonalTopology(ctx context.Context, constraints *v1alpha5.Constraints, topologyGroup *TopologyGroup) error { + topologyGroup.Register(constraints.Requirements.Zones().UnsortedList()...) if err := t.countMatchingPods(ctx, topologyGroup); err != nil { return fmt.Errorf("getting matching pods, %w", err) } diff --git a/pkg/controllers/provisioning/scheduling/topologygroup.go b/pkg/controllers/provisioning/scheduling/topologygroup.go index 2aa6b4ee06b8..00de9e542c56 100644 --- a/pkg/controllers/provisioning/scheduling/topologygroup.go +++ b/pkg/controllers/provisioning/scheduling/topologygroup.go @@ -17,8 +17,8 @@ package scheduling import ( "math" - "github.com/aws/karpenter/pkg/utils/sets" v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/sets" ) func NewTopologyGroup(pod *v1.Pod, constraint v1.TopologySpreadConstraint) *TopologyGroup { @@ -51,7 +51,7 @@ func (t *TopologyGroup) Increment(domain string) { } // NextDomain chooses a domain within the constraints that minimizes skew -func (t *TopologyGroup) NextDomain(requirement sets.Set) string { +func (t *TopologyGroup) NextDomain(requirement sets.String) string { minDomain := "" minCount := math.MaxInt32 for domain, count := range t.spread { diff --git a/pkg/utils/sets/sets.go b/pkg/utils/sets/sets.go index a86369baa590..fbd16f5cfae9 100644 --- a/pkg/utils/sets/sets.go +++ b/pkg/utils/sets/sets.go @@ -23,76 +23,74 @@ import ( // Set is a logical set of string values for the requirements. // It supports representations using complement operator. -// e.g., if C={"A", "B"}, setting isComplement = true means +// e.g., if C={"A", "B"}, setting complement = true means // C' contains every possible string values other than "A" and "B" type Set struct { - Members sets.String `json:"Members,omitempty"` - IsComplement bool `json:"allows,omitempty"` + values sets.String + Complement bool } func NewSet(values ...string) Set { return Set{ - Members: sets.NewString(values...), - IsComplement: false, + values: sets.NewString(values...), + Complement: false, } } func NewComplementSet(values ...string) Set { return Set{ - Members: sets.NewString(values...), - IsComplement: true, + values: sets.NewString(values...), + Complement: true, } } // DeepCopy creates a deep copy of the set object // It is required by the Kubernetes CRDs code generation func (s Set) DeepCopy() Set { - members := s.Members.UnsortedList() return Set{ - Members: sets.NewString(members...), - IsComplement: s.IsComplement, + values: sets.NewString(s.values.UnsortedList()...), + Complement: s.Complement, } } -// Values returns the members of the set. +// Values returns the values of the set. // If the set has an infinite size, returns an error -func (s Set) Values() []string { - if s.IsComplement { +func (s Set) Values() sets.String { + if s.Complement { panic("infinite set") } - return s.Members.UnsortedList() + return s.values } func (s Set) String() string { - if s.IsComplement { - return fmt.Sprintf("%v' (complement set)", s.Members.UnsortedList()) + if s.Complement { + return fmt.Sprintf("%v' (complement set)", s.values.UnsortedList()) } - return fmt.Sprintf("%v", s.Members.UnsortedList()) - + return fmt.Sprintf("%v", s.values.UnsortedList()) } // Has returns true if and only if item is contained in the set. func (s Set) Has(value string) bool { - if s.IsComplement { - return !s.Members.Has(value) + if s.Complement { + return !s.values.Has(value) } - return s.Members.Has(value) + return s.values.Has(value) } // Intersection returns a new set containing the common values func (s Set) Intersection(set Set) Set { - if s.IsComplement { - if set.IsComplement { - s.Members = s.Members.Union(set.Members) + if s.Complement { + if set.Complement { + s.values = s.values.Union(set.values) } else { - s.Members = set.Members.Difference(s.Members) - s.IsComplement = false + s.values = set.values.Difference(s.values) + s.Complement = false } } else { - if set.IsComplement { - s.Members = s.Members.Difference(set.Members) + if set.Complement { + s.values = s.values.Difference(set.values) } else { - s.Members = s.Members.Intersection(set.Members) + s.values = s.values.Intersection(set.values) } } return s @@ -100,8 +98,8 @@ func (s Set) Intersection(set Set) Set { // Len returns the size of the set. func (s Set) Len() int { - if s.IsComplement { - return math.MaxInt64 - s.Members.Len() + if s.Complement { + return math.MaxInt64 - s.values.Len() } - return s.Members.Len() + return s.values.Len() }