Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decoupled Constraints API object from internal representation #1603

Merged
merged 1 commit into from
Mar 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.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
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
24 changes: 9 additions & 15 deletions pkg/cloudprovider/aws/apis/v1alpha1/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
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) (*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
}
57 changes: 0 additions & 57 deletions pkg/cloudprovider/aws/apis/v1alpha1/provider_defaults.go

This file was deleted.

3 changes: 2 additions & 1 deletion pkg/cloudprovider/aws/apis/v1alpha1/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down
26 changes: 0 additions & 26 deletions pkg/cloudprovider/aws/apis/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

46 changes: 30 additions & 16 deletions pkg/cloudprovider/aws/cloudprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
})
}
}
}

Expand Down
Loading