From 51d2f5880ddff9323b7ad256c1aa8acb941be18c Mon Sep 17 00:00:00 2001 From: Todd Neal Date: Tue, 19 Jul 2022 15:56:48 -0500 Subject: [PATCH] feat: in the absence of instance-type/family filtering, provide a default (#2153) Provide a more opinionated set of default instance types if the provisioner doesn't supply its own list of instance types/families. --- pkg/cloudprovider/aws/cloudprovider.go | 86 ++++++++++++++++--- pkg/cloudprovider/aws/instance.go | 14 ++- pkg/cloudprovider/aws/suite_test.go | 5 ++ .../en/preview/upgrade-guide/_index.md | 7 +- 4 files changed, 93 insertions(+), 19 deletions(-) diff --git a/pkg/cloudprovider/aws/cloudprovider.go b/pkg/cloudprovider/aws/cloudprovider.go index 9ea871d24a85..b7e3969b135c 100644 --- a/pkg/cloudprovider/aws/cloudprovider.go +++ b/pkg/cloudprovider/aws/cloudprovider.go @@ -29,6 +29,14 @@ import ( "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/ssm" "github.com/patrickmn/go-cache" + "github.com/samber/lo" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/transport" + "knative.dev/pkg/logging" + "knative.dev/pkg/ptr" + k8sClient "sigs.k8s.io/controller-runtime/pkg/client" awsv1alpha1 "github.com/aws/karpenter/pkg/apis/awsnodetemplate/v1alpha1" "github.com/aws/karpenter/pkg/apis/provisioning/v1alpha5" @@ -38,15 +46,6 @@ import ( "github.com/aws/karpenter/pkg/utils/functional" "github.com/aws/karpenter/pkg/utils/injection" "github.com/aws/karpenter/pkg/utils/project" - - k8sClient "sigs.k8s.io/controller-runtime/pkg/client" - - v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/transport" - "knative.dev/pkg/logging" - "knative.dev/pkg/ptr" ) const ( @@ -140,7 +139,38 @@ func (c *CloudProvider) GetInstanceTypes(ctx context.Context, provisioner *v1alp if err != nil { return nil, err } - return c.instanceTypeProvider.Get(ctx, aws) + instanceTypes, err := c.instanceTypeProvider.Get(ctx, aws) + if err != nil { + return nil, err + } + + // if the provisioner is not supplying a list of instance types or families, perform some filtering to get instance + // types that are suitable for general workloads + if c.useOpinionatedInstanceFilter(provisioner.Spec.Requirements...) { + instanceTypes = lo.Filter(instanceTypes, func(it cloudprovider.InstanceType, _ int) bool { + cit, ok := it.(*InstanceType) + if !ok { + return true + } + + // c3, m3 and r3 aren't current generation but are fine for general workloads + if functional.HasAnyPrefix(*cit.InstanceType, "c3", "m3", "r3") { + return false + } + + // filter out all non-current generation + if cit.CurrentGeneration != nil && !*cit.CurrentGeneration { + return false + } + + // t2 is current generation but has different bursting behavior and u- isn't widely available + if functional.HasAnyPrefix(*cit.InstanceType, "t2", "u-") { + return false + } + return true + }) + } + return instanceTypes, nil } func (c *CloudProvider) Delete(ctx context.Context, node *v1.Node) error { @@ -205,3 +235,39 @@ func (c *CloudProvider) getProvider(ctx context.Context, provider *runtime.RawEx } return aws, nil } + +func (c *CloudProvider) useOpinionatedInstanceFilter(provisionerRequirements ...v1.NodeSelectorRequirement) bool { + var instanceTypeRequirement, familyRequirement v1.NodeSelectorRequirement + + for _, r := range provisionerRequirements { + if r.Key == v1.LabelInstanceTypeStable { + instanceTypeRequirement = r + } else if r.Key == v1alpha1.LabelInstanceFamily { + familyRequirement = r + } + } + // no provisioner instance type filtering, so use our opinionated list + if instanceTypeRequirement.Operator == "" && familyRequirement.Operator == "" { + return true + } + + for _, req := range []v1.NodeSelectorRequirement{instanceTypeRequirement, familyRequirement} { + switch req.Operator { + case v1.NodeSelectorOpIn: + // provisioner supplies its own list of instance types/families, so use that instead of filtering + return false + case v1.NodeSelectorOpNotIn: + // provisioner further restricts instance types/families, so we can possibly use our list and it will + // be filtered more + case v1.NodeSelectorOpExists: + // provisioner explicitly is asking for no filtering + return false + case v1.NodeSelectorOpDoesNotExist: + // this shouldn't match any instance type at provisioning time, but avoid filtering anyway + return false + } + } + + // provisioner requirements haven't prevented us from filtering + return true +} diff --git a/pkg/cloudprovider/aws/instance.go b/pkg/cloudprovider/aws/instance.go index c0d9166f7a0f..31dcfed76bfb 100644 --- a/pkg/cloudprovider/aws/instance.go +++ b/pkg/cloudprovider/aws/instance.go @@ -71,7 +71,7 @@ func NewInstanceProvider(ctx context.Context, ec2api ec2iface.EC2API, instanceTy // If spot is not used, the instanceTypes are not required to be sorted // because we are using ec2 fleet's lowest-price OD allocation strategy func (p *InstanceProvider) Create(ctx context.Context, provider *v1alpha1.AWS, nodeRequest *cloudprovider.NodeRequest) (*v1.Node, error) { - nodeRequest.InstanceTypeOptions = p.filterInstanceTypes(nodeRequest.InstanceTypeOptions) + nodeRequest.InstanceTypeOptions = p.prioritizeInstanceTypes(nodeRequest.InstanceTypeOptions) if len(nodeRequest.InstanceTypeOptions) > MaxInstanceTypes { nodeRequest.InstanceTypeOptions = nodeRequest.InstanceTypeOptions[0:MaxInstanceTypes] } @@ -354,10 +354,10 @@ func (p *InstanceProvider) getCapacityType(nodeRequest *cloudprovider.NodeReques return v1alpha1.CapacityTypeOnDemand } -// filterInstanceTypes is used to eliminate less desirable instance types (like GPUs) from the list of possible instance types when +// prioritizeInstanceTypes is used to eliminate less desirable instance types (like GPUs) from the list of possible instance types when // a set of more appropriate instance types would work. If a set of more desirable instance types is not found, then the original slice // of instance types are returned. -func (p *InstanceProvider) filterInstanceTypes(instanceTypes []cloudprovider.InstanceType) []cloudprovider.InstanceType { +func (p *InstanceProvider) prioritizeInstanceTypes(instanceTypes []cloudprovider.InstanceType) []cloudprovider.InstanceType { var genericInstanceTypes []cloudprovider.InstanceType for _, it := range instanceTypes { it := it.(*InstanceType) @@ -365,14 +365,12 @@ func (p *InstanceProvider) filterInstanceTypes(instanceTypes []cloudprovider.Ins if !functional.HasAnyPrefix(*it.InstanceType, "m", "c", "r", "a", "t", "i") { continue } - // deprioritize some older instance types including 1st/2nd gen burstable, compute and graviton - if functional.HasAnyPrefix(*it.InstanceType, "t1", "t2", "a1", "c1", "u-") { - continue - } - // deprioritize metal + // deprioritize metal even if our opinionated filter isn't apply due to something like an instance family + // requirement if aws.BoolValue(it.BareMetal) { continue } + itRes := it.Resources() if !resources.IsZero(itRes[v1alpha1.ResourceAWSNeuron]) || !resources.IsZero(itRes[v1alpha1.ResourceAMDGPU]) || diff --git a/pkg/cloudprovider/aws/suite_test.go b/pkg/cloudprovider/aws/suite_test.go index 2999aa0fa3f4..f7787a9f7720 100644 --- a/pkg/cloudprovider/aws/suite_test.go +++ b/pkg/cloudprovider/aws/suite_test.go @@ -255,6 +255,11 @@ var _ = Describe("Allocation", func() { } }) It("should launch on metal", func() { + // add a provisioner requirement for instance type exists to remove our default filter for metal sizes + provisioner.Spec.Requirements = append(provisioner.Spec.Requirements, v1.NodeSelectorRequirement{ + Key: v1.LabelInstanceTypeStable, + Operator: v1.NodeSelectorOpExists, + }) ExpectApplied(ctx, env.Client, provisioner) for _, pod := range ExpectProvisioned(ctx, env.Client, controller, test.UnschedulablePod(test.PodOptions{ diff --git a/website/content/en/preview/upgrade-guide/_index.md b/website/content/en/preview/upgrade-guide/_index.md index ce1fc6ff1e1a..a0e55dde3034 100644 --- a/website/content/en/preview/upgrade-guide/_index.md +++ b/website/content/en/preview/upgrade-guide/_index.md @@ -100,7 +100,7 @@ By adopting this practice we allow our users who are early adopters to test out ## Upgrading to v0.14.0+ * v0.14.0 changes the way Karpenter discovers its dynamically generated AWS launch templates to use a tag rather than a Name scheme. The previous name scheme was `Karpenter-${CLUSTER_NAME}-*` which could collide with user created launch templates that Karpenter should not manage. The new scheme uses a tag on the launch template `karpenter.k8s.aws/cluster: ${CLUSTER_NAME}`. As a result, Karpenter will not clean-up dynamically generated launch templates using the old name scheme. You can manually clean these up with the following commands: -``` +```bash ## Find launch templates that match the naming pattern and you do not want to keep aws ec2 describe-launch-templates --filters="Name=launch-template-name,Values=Karpenter-${CLUSTER_NAME}-*" @@ -108,6 +108,11 @@ aws ec2 describe-launch-templates --filters="Name=launch-template-name,Values=Ka aws ec2 delete-launch-template --launch-template-id ``` +* v0.14.0 introduces additional instance type filtering if there are no `node.kubernetes.io/instance-type` or `karpenter.k8s.aws/instance-family` requirements that restrict instance types specified on the provisioner. This prevents Karpenter from launching bare metal and some older non-current generation instance types unless the provisioner has been explicitly configured to allow them. If you specify an instance type or family requirement that supplies a list of instance-types or families, that list will be used regardless of filtering. The filtering can also be completely eliminated by adding an `Exists` requirement for instance type or family. +```yaml + - key: node.kubernetes.io/instance-type + operator: Exists +``` ## Upgrading to v0.13.0+ * v0.13.0 introduces a new CRD named `AWSNodeTemplate` which can be used to specify AWS Cloud Provider parameters. Everything that was previously specified under `spec.provider` in the Provisioner resource, can now be specified in the spec of the new resource. The use of `spec.provider` is deprecated but will continue to function to maintain backwards compatibility for the current API version (v1alpha5) of the Provisioner resource. v0.13.0 also introduces support for custom user data that doesn't require the use of a custom launch template. The user data can be specified in-line in the AWSNodeTemplate resource. Read the [UserData documentation here](../aws/user-data) to get started.