Decoupled Constraints API object from internal representation (#1603)

pkg/apis/provisioning/v1alpha5/provisioner_defaults.go

@@ -20,10 +20,5 @@ import (
 
 // SetDefaults for the provisioner
 func (p *Provisioner) SetDefaults(ctx context.Context) {
-	p.Spec.Constraints.Default(ctx)
-}
-
-// Default the constraints
-func (c *Constraints) Default(ctx context.Context) {
-	DefaultHook(ctx, c)
+	DefaultHook(ctx, p)
 }

pkg/apis/provisioning/v1alpha5/provisioner_validation.go

@@ -35,6 +35,7 @@ func (p *Provisioner) Validate(ctx context.Context) (errs *apis.FieldError) {
 	return errs.Also(
 		apis.ValidateObjectMetadata(p).ViaField("metadata"),
 		p.Spec.validate(ctx).ViaField("spec"),
+		ValidateHook(ctx, p),
 	)
 }
 
@@ -61,17 +62,16 @@ func (s *ProvisionerSpec) validateTTLSecondsAfterEmpty() (errs *apis.FieldError)
 }
 
 // Validate the constraints
-func (c *Constraints) Validate(ctx context.Context) (errs *apis.FieldError) {
+func (s *ProvisionerSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
 	return errs.Also(
-		c.validateLabels(),
-		c.validateTaints(),
-		c.validateRequirements(),
-		ValidateHook(ctx, c),
+		s.validateLabels(),
+		s.validateTaints(),
+		s.validateRequirements(),
 	)
 }
 
-func (c *Constraints) validateLabels() (errs *apis.FieldError) {
-	for key, value := range c.Labels {
+func (s *ProvisionerSpec) validateLabels() (errs *apis.FieldError) {
+	for key, value := range s.Labels {
 		for _, err := range validation.IsQualifiedName(key) {
 			errs = errs.Also(apis.ErrInvalidKeyName(key, "labels", err))
 		}
@@ -85,8 +85,8 @@ func (c *Constraints) validateLabels() (errs *apis.FieldError) {
 	return errs
 }
 
-func (c *Constraints) validateTaints() (errs *apis.FieldError) {
-	for i, taint := range c.Taints {
+func (s *ProvisionerSpec) validateTaints() (errs *apis.FieldError) {
+	for i, taint := range s.Taints {
 		// Validate Key
 		if len(taint.Key) == 0 {
 			errs = errs.Also(apis.ErrInvalidArrayValue(errs, "taints", i))
@@ -113,9 +113,9 @@ func (c *Constraints) validateTaints() (errs *apis.FieldError) {
 // This function is used by the provisioner validation webhook to verify the provisioner requirements.
 // 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) {
+func (s *ProvisionerSpec) validateRequirements() (errs *apis.FieldError) {
 	var err error
-	for _, requirement := range c.Requirements.Requirements {
+	for _, requirement := range s.Requirements.Requirements {
 		// Ensure requirements operator is allowed
 		if !SupportedProvisionerOps.Has(string(requirement.Operator)) {
 			err = multierr.Append(err, fmt.Errorf("key %s has an unsupported operator %s, provisioner only supports %s", requirement.Key, requirement.Operator, SupportedProvisionerOps.UnsortedList()))
@@ -124,7 +124,7 @@ func (c *Constraints) validateRequirements() (errs *apis.FieldError) {
 			err = multierr.Append(err, e)
 		}
 	}
-	err = multierr.Append(err, c.Requirements.Validate())
+	err = multierr.Append(err, s.Requirements.Validate())
 	if err != nil {
 		errs = errs.Also(apis.ErrInvalidValue(err, "requirements"))
 	}

pkg/apis/provisioning/v1alpha5/register.go

@@ -24,8 +24,8 @@ import (
 )
 
 var (
-	DefaultHook  = func(ctx context.Context, constraints *Constraints) {}
-	ValidateHook = func(ctx context.Context, constraints *Constraints) *apis.FieldError { return nil }
+	DefaultHook  = func(ctx context.Context, provisoner *Provisioner) {}
+	ValidateHook = func(ctx context.Context, provisoner *Provisioner) *apis.FieldError { return nil }
 )
 
 var (

pkg/apis/provisioning/v1alpha5/requirements.go

@@ -163,10 +163,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))
 		}
-		// Combined requirements must have some possible value unless Operator=DoesNotExist.
-		if values := r.Get(requirement.Key); values.Len() == 0 && requirement.Operator != v1.NodeSelectorOpDoesNotExist {
-			errs = multierr.Append(errs, fmt.Errorf("no feasible value for key %s", requirement.Key))
-		}
 	}
 	return errs
 }

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"}},

pkg/cloudprovider/aws/amifamily/resolver.go

@@ -85,10 +85,10 @@ func New(ssm ssmiface.SSMAPI, c *cache.Cache) *Resolver {
 
 // Resolve generates launch templates using the static options and dynamically generates launch template parameters.
 // Multiple ResolvedTemplates are returned based on the instanceTypes passed in to support special AMIs for certain instance types like GPUs.
-func (r Resolver) Resolve(ctx context.Context, constraints *v1alpha1.Constraints, instanceTypes []cloudprovider.InstanceType, options *Options) ([]*LaunchTemplate, error) {
-	amiFamily := r.getAMIFamily(constraints.AMIFamily, options)
+func (r Resolver) Resolve(ctx context.Context, provider *v1alpha1.AWS, nodeRequest *cloudprovider.NodeRequest, options *Options) ([]*LaunchTemplate, error) {
+	amiFamily := r.getAMIFamily(provider.AMIFamily, options)
 	amiIDs := map[string][]cloudprovider.InstanceType{}
-	for _, instanceType := range instanceTypes {
+	for _, instanceType := range nodeRequest.InstanceTypeOptions {
 		amiID, err := r.amiProvider.Get(ctx, instanceType, amiFamily.SSMAlias(options.KubernetesVersion, instanceType))
 		if err != nil {
 			return nil, err
@@ -99,9 +99,9 @@ func (r Resolver) Resolve(ctx context.Context, constraints *v1alpha1.Constraints
 	for amiID, instanceTypes := range amiIDs {
 		resolved := &LaunchTemplate{
 			Options:             options,
-			UserData:            amiFamily.UserData(constraints.KubeletConfiguration, constraints.Taints, options.Labels, options.CABundle, instanceTypes),
-			BlockDeviceMappings: constraints.BlockDeviceMappings,
-			MetadataOptions:     constraints.MetadataOptions,
+			UserData:            amiFamily.UserData(nodeRequest.Template.KubeletConfiguration, nodeRequest.Template.Taints, options.Labels, options.CABundle, instanceTypes),
+			BlockDeviceMappings: provider.BlockDeviceMappings,
+			MetadataOptions:     provider.MetadataOptions,
 			AMIID:               amiID,
 			InstanceTypes:       instanceTypes,
 		}

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

@@ -24,12 +24,6 @@ import (
 	"github.com/aws/karpenter/pkg/apis/provisioning/v1alpha5"
 )
 
-// Constraints wraps generic constraints with AWS specific parameters
-type Constraints struct {
-	*v1alpha5.Constraints
-	*AWS
-}
-
 // AWS contains parameters specific to this cloud provider
 // +kubebuilder:object:root=true
 type AWS struct {
@@ -192,29 +186,29 @@ type BlockDevice struct {
 	VolumeType *string `json:"volumeType,omitempty"`
 }
 
-func Deserialize(constraints *v1alpha5.Constraints) (*Constraints, error) {
-	if constraints.Provider == nil {
+func Deserialize(provider *v1alpha5.Provider) (*AWS, error) {
+	if provider == nil {
 		return nil, fmt.Errorf("invariant violated: spec.provider is not defined. Is the defaulting webhook installed?")
 	}
-	aws := &AWS{}
-	_, gvk, err := Codec.UniversalDeserializer().Decode(constraints.Provider.Raw, nil, aws)
+	a := &AWS{}
+	_, gvk, err := Codec.UniversalDeserializer().Decode(provider.Raw, nil, a)
 	if err != nil {
 		return nil, err
 	}
 	if gvk != nil {
-		aws.SetGroupVersionKind(*gvk)
+		a.SetGroupVersionKind(*gvk)
 	}
-	return &Constraints{constraints, aws}, nil
+	return a, nil
 }
 
-func (a *AWS) Serialize(constraints *v1alpha5.Constraints) error {
-	if constraints.Provider == nil {
+func (a *AWS) Serialize(provider *v1alpha5.Provider) error {
+	if provider == nil {
 		return fmt.Errorf("invariant violated: spec.provider is not defined. Is the defaulting webhook installed?")
 	}
 	bytes, err := json.Marshal(a)
 	if err != nil {
 		return err
 	}
-	constraints.Provider.Raw = bytes
+	provider.Raw = bytes
 	return nil
 }

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

@@ -15,12 +15,13 @@ limitations under the License.
 package v1alpha1
 
 import (
-	"github.com/aws/aws-sdk-go/service/ec2"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 
+	"github.com/aws/aws-sdk-go/service/ec2"
+
 	"github.com/aws/karpenter/pkg/apis/provisioning/v1alpha5"
 )
 

pkg/cloudprovider/aws/cloudprovider.go

@@ -101,45 +101,59 @@ func NewCloudProvider(ctx context.Context, options cloudprovider.Options) *Cloud
 
 // Create a node given the constraints.
 func (c *CloudProvider) Create(ctx context.Context, nodeRequest *cloudprovider.NodeRequest) (*v1.Node, error) {
-	vendorConstraints, err := v1alpha1.Deserialize(nodeRequest.Constraints)
+	vendorConstraints, err := v1alpha1.Deserialize(nodeRequest.Template.Provider)
 	if err != nil {
 		return nil, err
 	}
-	return c.instanceProvider.Create(ctx, vendorConstraints, nodeRequest.InstanceTypeOptions)
+	return c.instanceProvider.Create(ctx, vendorConstraints, nodeRequest)
 }
 
 // GetInstanceTypes returns all available InstanceTypes despite accepting a Constraints struct (note that it does not utilize Requirements)
 func (c *CloudProvider) GetInstanceTypes(ctx context.Context, provider *v1alpha5.Provider) ([]cloudprovider.InstanceType, error) {
-	vendorConstraints, err := v1alpha1.Deserialize(&v1alpha5.Constraints{Provider: provider})
+	aws, err := v1alpha1.Deserialize(provider)
 	if err != nil {
 		return nil, apis.ErrGeneric(err.Error())
 	}
-	return c.instanceTypeProvider.Get(ctx, vendorConstraints.AWS)
+	return c.instanceTypeProvider.Get(ctx, aws)
 }
 
 func (c *CloudProvider) Delete(ctx context.Context, node *v1.Node) error {
 	return c.instanceProvider.Terminate(ctx, node)
 }
 
 // Validate the provisioner
-func (c *CloudProvider) Validate(ctx context.Context, constraints *v1alpha5.Constraints) *apis.FieldError {
-	vendorConstraints, err := v1alpha1.Deserialize(constraints)
+func (c *CloudProvider) Validate(ctx context.Context, provisioner *v1alpha5.Provisioner) *apis.FieldError {
+	provider, err := v1alpha1.Deserialize(provisioner.Spec.Provider)
 	if err != nil {
 		return apis.ErrGeneric(err.Error())
 	}
-	return vendorConstraints.AWS.Validate()
+	return provider.Validate()
 }
 
 // Default the provisioner
-func (c *CloudProvider) Default(ctx context.Context, constraints *v1alpha5.Constraints) {
-	vendorConstraints, err := v1alpha1.Deserialize(constraints)
-	if err != nil {
-		logging.FromContext(ctx).Errorf("Failed to deserialize provider, %s", err)
-		return
-	}
-	vendorConstraints.Default(ctx)
-	if err := vendorConstraints.Serialize(constraints); err != nil {
-		logging.FromContext(ctx).Errorf("Failed to serialize provider, %s", err)
+func (c *CloudProvider) Default(ctx context.Context, provisioner *v1alpha5.Provisioner) {
+	defaultLabels(provisioner)
+}
+
+func defaultLabels(provisioner *v1alpha5.Provisioner) {
+	for key, value := range map[string]string{
+		v1alpha5.LabelCapacityType: ec2.DefaultTargetCapacityTypeOnDemand,
+		v1.LabelArchStable:         v1alpha5.ArchitectureAmd64,
+	} {
+		hasLabel := false
+		if _, ok := provisioner.Spec.Labels[key]; ok {
+			hasLabel = true
+		}
+		for _, requirement := range provisioner.Spec.Requirements.Requirements {
+			if requirement.Key == key {
+				hasLabel = true
+			}
+		}
+		if !hasLabel {
+			provisioner.Spec.Requirements.Requirements = append(provisioner.Spec.Requirements.Requirements, v1.NodeSelectorRequirement{
+				Key: key, Operator: v1.NodeSelectorOpIn, Values: []string{value},
+			})
+		}
 	}
 }