Skip to content

Commit

Permalink
Decoupled Constraints API object from internal representation
Browse files Browse the repository at this point in the history
  • Loading branch information
ellistarn committed Mar 31, 2022
1 parent c9ab944 commit 17b953f
Show file tree
Hide file tree
Showing 23 changed files with 256 additions and 335 deletions.
7 changes: 1 addition & 6 deletions pkg/apis/provisioning/v1alpha5/provisioner_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
24 changes: 12 additions & 12 deletions pkg/apis/provisioning/v1alpha5/provisioner_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
}

Expand All @@ -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))
}
Expand All @@ -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))
Expand All @@ -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()))
Expand All @@ -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"))
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/provisioning/v1alpha5/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
4 changes: 0 additions & 4 deletions pkg/apis/provisioning/v1alpha5/requirements.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
7 changes: 0 additions & 7 deletions pkg/apis/provisioning/v1alpha5/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}},
Expand Down
12 changes: 6 additions & 6 deletions pkg/cloudprovider/aws/amifamily/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.Provider, 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
Expand All @@ -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,
}
Expand Down
30 changes: 12 additions & 18 deletions pkg/cloudprovider/aws/apis/v1alpha1/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,9 @@ 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
// Provider contains parameters specific to this cloud provider
// +kubebuilder:object:root=true
type AWS struct {
type Provider struct {
// TypeMeta includes version and kind of the extensions, inferred if not provided.
// +optional
metav1.TypeMeta `json:",inline"`
Expand Down Expand Up @@ -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) (*Provider, 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)
p := &Provider{}
_, gvk, err := Codec.UniversalDeserializer().Decode(provider.Raw, nil, p)
if err != nil {
return nil, err
}
if gvk != nil {
aws.SetGroupVersionKind(*gvk)
p.SetGroupVersionKind(*gvk)
}
return &Constraints{constraints, aws}, nil
return p, nil
}

func (a *AWS) Serialize(constraints *v1alpha5.Constraints) error {
if constraints.Provider == nil {
func (p *Provider) 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)
bytes, err := json.Marshal(p)
if err != nil {
return err
}
constraints.Provider.Raw = bytes
provider.Raw = bytes
return nil
}
57 changes: 0 additions & 57 deletions pkg/cloudprovider/aws/apis/v1alpha1/provider_defaults.go

This file was deleted.

Loading

0 comments on commit 17b953f

Please sign in to comment.