Topology now factors in affinity and global requirements (subnets) when computing spread

pkg/apis/provisioning/v1alpha5/provisioner.go

@@ -17,10 +17,10 @@ package v1alpha5
 import (
 	"sort"
 
-	"github.com/awslabs/karpenter/pkg/utils/functional"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
 )
 
 // ProvisionerSpec is the top level provisioner specification. Provisioners
@@ -95,105 +95,128 @@ type ProvisionerList struct {
 }
 
 // Zones for the constraints
-func (c *Constraints) Zones() []string {
-	return c.Requirements.GetLabelValues(v1.LabelTopologyZone)
+func (r Requirements) Zones() sets.String {
+	return r.Requirement(v1.LabelTopologyZone)
 }
 
 // InstanceTypes for the constraints
-func (c *Constraints) InstanceTypes() []string {
-	return c.Requirements.GetLabelValues(v1.LabelInstanceTypeStable)
+func (r Requirements) InstanceTypes() sets.String {
+	return r.Requirement(v1.LabelInstanceTypeStable)
 }
 
 // Architectures for the constraints
-func (c *Constraints) Architectures() []string {
-	return c.Requirements.GetLabelValues(v1.LabelArchStable)
+func (r Requirements) Architectures() sets.String {
+	return r.Requirement(v1.LabelArchStable)
 }
 
 // OperatingSystems for the constraints
-func (c *Constraints) OperatingSystems() []string {
-	return c.Requirements.GetLabelValues(v1.LabelOSStable)
+func (r Requirements) OperatingSystems() sets.String {
+	return r.Requirement(v1.LabelOSStable)
 }
 
-// Consolidate and copy the constraints
-func (c *Constraints) Consolidate() *Constraints {
-	// Combine labels and requirements
-	combined := append(Requirements{}, c.Requirements...)
-	for key, value := range c.Labels {
-		combined = append(combined, v1.NodeSelectorRequirement{Key: key, Operator: v1.NodeSelectorOpIn, Values: []string{value}})
+func (r Requirements) WithProvisioner(provisioner Provisioner) Requirements {
+	return r.
+		With(provisioner.Spec.Requirements).
+		WithLabels(provisioner.Spec.Labels).
+		WithLabels(map[string]string{ProvisionerNameLabelKey: provisioner.Name})
+}
+
+func (r Requirements) With(requirements Requirements) Requirements {
+	return append(r, requirements...)
+}
+
+func (r Requirements) WithLabels(labels map[string]string) Requirements {
+	for key, value := range labels {
+		r = append(r, v1.NodeSelectorRequirement{Key: key, Operator: v1.NodeSelectorOpIn, Values: []string{value}})
 	}
-	// Simplify to a single OpIn per label
-	requirements := Requirements{}
-	for _, label := range combined.GetLabels() {
+	return r
+}
+
+func (r Requirements) WithPod(pod *v1.Pod) Requirements {
+	for key, value := range pod.Spec.NodeSelector {
+		r = append(r, v1.NodeSelectorRequirement{Key: key, Operator: v1.NodeSelectorOpIn, Values: []string{value}})
+	}
+	if pod.Spec.Affinity == nil || pod.Spec.Affinity.NodeAffinity == nil {
+		return r
+	}
+	// Select heaviest preference and treat as a requirement. An outer loop will iteratively unconstrain them if unsatisfiable.
+	if preferred := pod.Spec.Affinity.NodeAffinity.PreferredDuringSchedulingIgnoredDuringExecution; len(preferred) > 0 {
+		sort.Slice(preferred, func(i int, j int) bool { return preferred[i].Weight > preferred[j].Weight })
+		r = append(r, preferred[0].Preference.MatchExpressions...)
+	}
+	// Select first requirement. An outer loop will iteratively remove OR requirements if unsatisfiable
+	if pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution != nil &&
+		len(pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms) > 0 {
+		r = append(r, pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions...)
+	}
+	return r
+}
+
+// Consolidate combines In and NotIn requirements for each unique key, producing
+// an equivalent minimal representation of the requirements. This is useful as
+// requirements may be appended from a variety of sources and then consolidated.
+// Caution: If a key has contains a `NotIn` operator without a corresponding
+// `In` operator, the requirement will permanently be [] after consolidation. To
+// avoid this, include the broadest `In` requirements before consolidating.
+func (r Requirements) Consolidate() (requirements Requirements) {
+	for _, key := range r.Keys() {
 		requirements = append(requirements, v1.NodeSelectorRequirement{
-			Key:      label,
+			Key:      key,
 			Operator: v1.NodeSelectorOpIn,
-			Values:   combined.GetLabelValues(label),
+			Values:   r.Requirement(key).UnsortedList(),
 		})
 	}
-	return &Constraints{
-		Labels:       c.Labels,
-		Taints:       c.Taints,
-		Requirements: requirements,
-		Provider:     c.Provider,
-	}
+	return requirements
 }
 
-// With adds additional requirements from the pods
-func (r Requirements) With(pods ...*v1.Pod) Requirements {
-	for _, pod := range pods {
-		for key, value := range pod.Spec.NodeSelector {
-			r = append(r, v1.NodeSelectorRequirement{Key: key, Operator: v1.NodeSelectorOpIn, Values: []string{value}})
-		}
-		if pod.Spec.Affinity == nil || pod.Spec.Affinity.NodeAffinity == nil {
-			continue
-		}
-		// Select heaviest preference and treat as a requirement. An outer loop will iteratively unconstrain them if unsatisfiable.
-		if preferred := pod.Spec.Affinity.NodeAffinity.PreferredDuringSchedulingIgnoredDuringExecution; len(preferred) > 0 {
-			sort.Slice(preferred, func(i int, j int) bool { return preferred[i].Weight > preferred[j].Weight })
-			r = append(r, preferred[0].Preference.MatchExpressions...)
-		}
-		// Select first requirement. An outer loop will iteratively remove OR requirements if unsatisfiable
-		if pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution != nil &&
-			len(pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms) > 0 {
-			r = append(r, pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions...)
+func (r Requirements) CustomLabels() map[string]string {
+	labels := map[string]string{}
+	for _, key := range r.Keys() {
+		if !WellKnownLabels.Has(key) {
+			if requirement := r.Requirement(key); len(requirement) > 0 {
+				labels[key] = requirement.UnsortedList()[0]
+			}
 		}
 	}
-	return r
+	return labels
 }
 
-// GetLabels returns unique set of the label keys from the requirements
-func (r Requirements) GetLabels() []string {
-	keys := map[string]bool{}
+func (r Requirements) WellKnown() (requirements Requirements) {
 	for _, requirement := range r {
-		keys[requirement.Key] = true
-	}
-	result := []string{}
-	for key := range keys {
-		result = append(result, key)
+		if WellKnownLabels.Has(requirement.Key) {
+			requirements = append(requirements, requirement)
+		}
 	}
-	return result
+	return requirements
 }
 
-// GetLabelValues for the provided key constrained by the requirements
-func (r Requirements) GetLabelValues(label string) []string {
-	var result []string
-	if known, ok := WellKnownLabels[label]; ok {
-		result = known
+// Keys returns unique set of the label keys from the requirements
+func (r Requirements) Keys() []string {
+	keys := sets.NewString()
+	for _, requirement := range r {
+		keys.Insert(requirement.Key)
 	}
+	return keys.UnsortedList()
+}
+
+// Requirements for the provided key, nil if unconstrained
+func (r Requirements) Requirement(key string) sets.String {
+	var result sets.String
 	// OpIn
 	for _, requirement := range r {
-		if requirement.Key == label && requirement.Operator == v1.NodeSelectorOpIn {
-			result = functional.IntersectStringSlice(result, requirement.Values)
+		if requirement.Key == key && requirement.Operator == v1.NodeSelectorOpIn {
+			if result == nil {
+				result = sets.NewString(requirement.Values...)
+			} else {
+				result = result.Intersection(sets.NewString(requirement.Values...))
+			}
 		}
 	}
 	// OpNotIn
 	for _, requirement := range r {
-		if requirement.Key == label && requirement.Operator == v1.NodeSelectorOpNotIn {
-			result = functional.StringSliceWithout(result, requirement.Values...)
+		if requirement.Key == key && requirement.Operator == v1.NodeSelectorOpNotIn {
+			result = result.Difference(sets.NewString(requirement.Values...))
 		}
 	}
-	if len(result) == 0 {
-		result = []string{}
-	}
 	return result
 }

pkg/apis/provisioning/v1alpha5/provisioner_validation.go

@@ -82,7 +82,6 @@ func (c *Constraints) Validate(ctx context.Context) (errs *apis.FieldError) {
 		c.validateLabels(),
 		c.validateTaints(),
 		c.validateRequirements(),
-		c.Consolidate().Requirements.Validate(),
 		ValidateHook(ctx, c),
 	)
 }
@@ -95,11 +94,8 @@ func (c *Constraints) validateLabels() (errs *apis.FieldError) {
 		for _, err := range validation.IsValidLabelValue(value) {
 			errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s, %s", value, err), fmt.Sprintf("labels[%s]", key)))
 		}
-		if known, ok := WellKnownLabels[key]; ok && !functional.ContainsString(known, value) {
-			errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s not in %s", value, known), fmt.Sprintf("labels[%s]", key)))
-		}
 		if _, ok := WellKnownLabels[key]; !ok && IsRestrictedLabelDomain(key) {
-			errs = errs.Also(apis.ErrInvalidKeyName(key, "labels", "label prefix not supported"))
+			errs = errs.Also(apis.ErrInvalidKeyName(key, "labels", "label domain not allowed"))
 		}
 	}
 	return errs
@@ -157,8 +153,8 @@ func (c *Constraints) validateRequirements() (errs *apis.FieldError) {
 }
 
 func (r Requirements) Validate() (errs *apis.FieldError) {
-	for _, label := range r.GetLabels() {
-		if len(r.GetLabelValues(label)) == 0 {
+	for _, label := range r.Keys() {
+		if r.Requirement(label).Len() == 0 {
 			errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("%s is too constrained", label)))
 		}
 	}
@@ -173,11 +169,6 @@ func validateRequirement(requirement v1.NodeSelectorRequirement) (errs *apis.Fie
 		for _, err := range validation.IsValidLabelValue(value) {
 			errs = errs.Also(apis.ErrInvalidArrayValue(fmt.Sprintf("%s, %s", value, err), "values", i))
 		}
-		if known, ok := WellKnownLabels[requirement.Key]; ok {
-			if !functional.ContainsString(known, value) {
-				errs = errs.Also(apis.ErrInvalidArrayValue(fmt.Sprintf("%s not in %s", value, known), "values", i))
-			}
-		}
 	}
 	if !functional.ContainsString(SupportedNodeSelectorOps, string(requirement.Operator)) {
 		errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s not in %s", requirement.Operator, SupportedNodeSelectorOps), "operator"))

pkg/apis/provisioning/v1alpha5/provisioner_validation_test.go

@@ -77,30 +77,12 @@ var _ = Describe("Validation", func() {
 				Expect(provisioner.Validate(ctx)).ToNot(Succeed())
 			}
 		})
-		It("should fail for restricted prefixes when not well known labels", func() {
+		It("should fail for restricted label domains", func() {
 			for _, label := range RestrictedLabelDomains {
 				provisioner.Spec.Labels = map[string]string{label + "/unknown": randomdata.SillyName()}
 				Expect(provisioner.Validate(ctx)).ToNot(Succeed())
 			}
 		})
-		It("should succeed for well known label values", func() {
-			WellKnownLabels[v1.LabelTopologyZone] = []string{"test-1", "test1"}
-			WellKnownLabels[v1.LabelInstanceTypeStable] = []string{"test-1", "test1"}
-			WellKnownLabels[v1.LabelArchStable] = []string{"test-1", "test1"}
-			WellKnownLabels[v1.LabelOSStable] = []string{"test-1", "test1"}
-			for key, values := range WellKnownLabels {
-				for _, value := range values {
-					provisioner.Spec.Labels = map[string]string{key: value}
-					Expect(provisioner.Validate(ctx)).To(Succeed())
-				}
-			}
-		})
-		It("should fail for invalid well known label values", func() {
-			for key := range WellKnownLabels {
-				provisioner.Spec.Labels = map[string]string{key: "unknown"}
-				Expect(provisioner.Validate(ctx)).ToNot(Succeed())
-			}
-		})
 	})
 	Context("Taints", func() {
 		It("should succeed for valid taints", func() {
@@ -143,38 +125,5 @@ var _ = Describe("Validation", func() {
 				Expect(provisioner.Validate(ctx)).ToNot(Succeed())
 			}
 		})
-		It("should validate well known labels", func() {
-			WellKnownLabels[v1.LabelTopologyZone] = []string{"test"}
-			provisioner.Spec.Requirements = Requirements{{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}}
-			Expect(provisioner.Validate(ctx)).To(Succeed())
-			provisioner.Spec.Labels = map[string]string{}
-			provisioner.Spec.Requirements = Requirements{{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}}
-			Expect(provisioner.Validate(ctx)).To(Succeed())
-			provisioner.Spec.Labels = map[string]string{v1.LabelTopologyZone: "test"}
-			provisioner.Spec.Requirements = Requirements{{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}}
-			Expect(provisioner.Validate(ctx)).To(Succeed())
-			provisioner.Spec.Labels = map[string]string{v1.LabelTopologyZone: "test"}
-			provisioner.Spec.Requirements = Requirements{{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpNotIn, Values: []string{"test"}}}
-			Expect(provisioner.Validate(ctx)).ToNot(Succeed())
-			provisioner.Spec.Labels = map[string]string{v1.LabelTopologyZone: "test"}
-			provisioner.Spec.Requirements = Requirements{{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"unknown"}}}
-			Expect(provisioner.Validate(ctx)).ToNot(Succeed())
-		})
-		It("should validate custom labels", func() {
-			provisioner.Spec.Requirements = Requirements{{Key: "test", Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}}
-			Expect(provisioner.Validate(ctx)).To(Succeed())
-			provisioner.Spec.Labels = map[string]string{}
-			provisioner.Spec.Requirements = Requirements{{Key: "test", Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}}
-			Expect(provisioner.Validate(ctx)).To(Succeed())
-			provisioner.Spec.Labels = map[string]string{"test": "test"}
-			provisioner.Spec.Requirements = Requirements{{Key: "test", Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}}
-			Expect(provisioner.Validate(ctx)).To(Succeed())
-			provisioner.Spec.Labels = map[string]string{"test": "test"}
-			provisioner.Spec.Requirements = Requirements{{Key: "test", Operator: v1.NodeSelectorOpNotIn, Values: []string{"test"}}}
-			Expect(provisioner.Validate(ctx)).ToNot(Succeed())
-			provisioner.Spec.Labels = map[string]string{"test": "test"}
-			provisioner.Spec.Requirements = Requirements{{Key: "test", Operator: v1.NodeSelectorOpIn, Values: []string{"unknown"}}}
-			Expect(provisioner.Validate(ctx)).ToNot(Succeed())
-		})
 	})
 })

pkg/apis/provisioning/v1alpha5/register.go

@@ -22,6 +22,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"knative.dev/pkg/apis"
 )
 
@@ -45,19 +46,19 @@ var (
 		EmptinessTimestampAnnotationKey,
 		v1.LabelHostname,
 	}
-	// WellKnownLabels supported by karpenter and their allowable values
-	WellKnownLabels = map[string][]string{
-		v1.LabelTopologyZone:       {},
-		v1.LabelInstanceTypeStable: {},
-		v1.LabelArchStable:         {},
-		v1.LabelOSStable:           {},
-	}
 	// These are either prohibited by the kubelet or reserved by karpenter
 	RestrictedLabelDomains = []string{
 		"kubernetes.io",
 		"k8s.io",
 		"karpenter.sh",
 	}
+	// WellKnownLabels supported by karpenter
+	WellKnownLabels = sets.NewString(
+		v1.LabelTopologyZone,
+		v1.LabelInstanceTypeStable,
+		v1.LabelArchStable,
+		v1.LabelOSStable,
+	)
 	DefaultHook  = func(ctx context.Context, constraints *Constraints) {}
 	ValidateHook = func(ctx context.Context, constraints *Constraints) *apis.FieldError { return nil }
 )

pkg/cloudprovider/aws/apis/v1alpha1/provider_defaults.go

@@ -34,7 +34,10 @@ func (c *Constraints) Default(ctx context.Context) {
 }
 
 func (c *Constraints) defaultCapacityTypes() {
-	if functional.ContainsString(c.Consolidate().Requirements.GetLabels(), CapacityTypeLabel) {
+	if _, ok := c.Labels[CapacityTypeLabel]; ok {
+		return
+	}
+	if functional.ContainsString(c.Requirements.Keys(), CapacityTypeLabel) {
 		return
 	}
 	c.Requirements = append(c.Requirements, v1.NodeSelectorRequirement{
@@ -45,7 +48,10 @@ func (c *Constraints) defaultCapacityTypes() {
 }
 
 func (c *Constraints) defaultArchitecture() {
-	if functional.ContainsString(c.Consolidate().Requirements.GetLabels(), v1.LabelArchStable) {
+	if _, ok := c.Labels[v1.LabelArchStable]; ok {
+		return
+	}
+	if functional.ContainsString(c.Requirements.Keys(), v1.LabelArchStable) {
 		return
 	}
 	c.Requirements = append(c.Requirements, v1.NodeSelectorRequirement{

pkg/cloudprovider/aws/apis/v1alpha1/register.go

@@ -44,6 +44,6 @@ var (
 func init() {
 	Scheme.AddKnownTypes(schema.GroupVersion{Group: v1alpha5.ExtensionsGroup, Version: "v1alpha1"}, &AWS{})
 	v1alpha5.RestrictedLabels = append(v1alpha5.RestrictedLabels, AWSLabelPrefix)
-	v1alpha5.WellKnownLabels[CapacityTypeLabel] = []string{CapacityTypeSpot, CapacityTypeOnDemand}
 	v1alpha5.RestrictedLabelDomains = append(v1alpha5.RestrictedLabelDomains, AWSRestrictedLabelDomains...)
+	v1alpha5.WellKnownLabels.Insert(CapacityTypeLabel)
 }