From 2dfebc9602a9bf6848616fb3740c0f25a455d650 Mon Sep 17 00:00:00 2001 From: Amanuel Engeda <74629455+engedaam@users.noreply.github.com> Date: Fri, 8 Sep 2023 16:30:20 -0700 Subject: [PATCH] chore: k8s Version Provider (#4596) --- pkg/controllers/nodeclass/suite_test.go | 4 +- pkg/operator/operator.go | 5 +- pkg/providers/amifamily/ami.go | 53 +++++------------- pkg/providers/amifamily/ami_test.go | 2 +- pkg/providers/launchtemplate/suite_test.go | 2 +- pkg/providers/version/version.go | 64 ++++++++++++++++++++++ pkg/test/environment.go | 6 +- 7 files changed, 89 insertions(+), 47 deletions(-) create mode 100644 pkg/providers/version/version.go diff --git a/pkg/controllers/nodeclass/suite_test.go b/pkg/controllers/nodeclass/suite_test.go index 0799b3d63164..0f79fbd3fdbd 100644 --- a/pkg/controllers/nodeclass/suite_test.go +++ b/pkg/controllers/nodeclass/suite_test.go @@ -458,7 +458,7 @@ var _ = Describe("AWSNodeTemplateController", func() { }) }) It("should resolve amiSelector AMIs and requirements into status", func() { - version := lo.Must(awsEnv.AMIProvider.KubeServerVersion(ctx)) + version := lo.Must(awsEnv.VersionProvider.Get(ctx)) awsEnv.SSMAPI.Parameters = map[string]string{ fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2/recommended/image_id", version): "ami-id-123", @@ -576,7 +576,7 @@ var _ = Describe("AWSNodeTemplateController", func() { }, nodeTemplate.Status.AMIs) }) It("should resolve amiSelector AMis and requirements into status when all SSM aliases don't resolve", func() { - version := lo.Must(awsEnv.AMIProvider.KubeServerVersion(ctx)) + version := lo.Must(awsEnv.VersionProvider.Get(ctx)) // This parameter set doesn't include any of the Nvidia AMIs awsEnv.SSMAPI.Parameters = map[string]string{ fmt.Sprintf("/aws/service/bottlerocket/aws-k8s-%s/x86_64/latest/image_id", version): "ami-id-123", diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index 1bec5197faad..bc3d94232b6a 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -54,6 +54,7 @@ import ( "github.com/aws/karpenter/pkg/providers/pricing" "github.com/aws/karpenter/pkg/providers/securitygroup" "github.com/aws/karpenter/pkg/providers/subnet" + "github.com/aws/karpenter/pkg/providers/version" "github.com/aws/karpenter/pkg/utils/project" ) @@ -126,8 +127,8 @@ func NewOperator(ctx context.Context, operator *operator.Operator) (context.Cont ec2api, *sess.Config.Region, ) - amiProvider := amifamily.NewProvider(operator.GetClient(), operator.KubernetesInterface, ssm.New(sess), ec2api, - cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval), cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) + versionProvider := version.NewProvider(operator.KubernetesInterface, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) + amiProvider := amifamily.NewProvider(versionProvider, ssm.New(sess), ec2api, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) amiResolver := amifamily.New(amiProvider) launchTemplateProvider := launchtemplate.NewProvider( ctx, diff --git a/pkg/providers/amifamily/ami.go b/pkg/providers/amifamily/ami.go index 390ddce91d2d..76c8d741f61b 100644 --- a/pkg/providers/amifamily/ami.go +++ b/pkg/providers/amifamily/ami.go @@ -21,9 +21,6 @@ import ( "strings" "time" - "k8s.io/client-go/kubernetes" - "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/ec2/ec2iface" @@ -39,6 +36,7 @@ import ( corev1beta1 "github.com/aws/karpenter-core/pkg/apis/v1beta1" "github.com/aws/karpenter/pkg/apis/v1alpha1" "github.com/aws/karpenter/pkg/apis/v1beta1" + "github.com/aws/karpenter/pkg/providers/version" "github.com/aws/karpenter-core/pkg/cloudprovider" "github.com/aws/karpenter-core/pkg/scheduling" @@ -47,13 +45,11 @@ import ( ) type Provider struct { - cache *cache.Cache - kubernetesVersionCache *cache.Cache - ssm ssmiface.SSMAPI - kubeClient client.Client - ec2api ec2iface.EC2API - cm *pretty.ChangeMonitor - kubernetesInterface kubernetes.Interface + cache *cache.Cache + ssm ssmiface.SSMAPI + ec2api ec2iface.EC2API + cm *pretty.ChangeMonitor + versionProvider *version.Provider } type AMI struct { @@ -106,37 +102,14 @@ func (a AMIs) MapToInstanceTypes(instanceTypes []*cloudprovider.InstanceType, is return amiIDs } -const ( - kubernetesVersionCacheKey = "kubernetesVersion" -) - -func NewProvider(kubeClient client.Client, kubernetesInterface kubernetes.Interface, ssm ssmiface.SSMAPI, ec2api ec2iface.EC2API, - cache, kubernetesVersionCache *cache.Cache) *Provider { +func NewProvider(versionProvider *version.Provider, ssm ssmiface.SSMAPI, ec2api ec2iface.EC2API, cache *cache.Cache) *Provider { return &Provider{ - cache: cache, - kubernetesVersionCache: kubernetesVersionCache, - ssm: ssm, - kubeClient: kubeClient, - ec2api: ec2api, - cm: pretty.NewChangeMonitor(), - kubernetesInterface: kubernetesInterface, - } -} - -func (p *Provider) KubeServerVersion(ctx context.Context) (string, error) { - if version, ok := p.kubernetesVersionCache.Get(kubernetesVersionCacheKey); ok { - return version.(string), nil - } - serverVersion, err := p.kubernetesInterface.Discovery().ServerVersion() - if err != nil { - return "", err - } - version := fmt.Sprintf("%s.%s", serverVersion.Major, strings.TrimSuffix(serverVersion.Minor, "+")) - p.kubernetesVersionCache.SetDefault(kubernetesVersionCacheKey, version) - if p.cm.HasChanged("kubernetes-version", version) { - logging.FromContext(ctx).With("version", version).Debugf("discovered kubernetes version") + cache: cache, + ssm: ssm, + ec2api: ec2api, + cm: pretty.NewChangeMonitor(), + versionProvider: versionProvider, } - return version, nil } // Get Returning a list of AMIs with its associated requirements @@ -166,7 +139,7 @@ func (p *Provider) getDefaultAMIs(ctx context.Context, nodeClass *v1beta1.NodeCl return images.(AMIs), nil } amiFamily := GetAMIFamily(nodeClass.Spec.AMIFamily, options) - kubernetesVersion, err := p.KubeServerVersion(ctx) + kubernetesVersion, err := p.versionProvider.Get(ctx) if err != nil { return nil, fmt.Errorf("getting kubernetes version %w", err) } diff --git a/pkg/providers/amifamily/ami_test.go b/pkg/providers/amifamily/ami_test.go index 24b60ed4d604..f12036c44224 100644 --- a/pkg/providers/amifamily/ami_test.go +++ b/pkg/providers/amifamily/ami_test.go @@ -123,7 +123,7 @@ var _ = AfterSuite(func() { var _ = Describe("AMIProvider", func() { var version string BeforeEach(func() { - version = lo.Must(awsEnv.AMIProvider.KubeServerVersion(ctx)) + version = lo.Must(awsEnv.VersionProvider.Get(ctx)) nodeClass = test.NodeClass() }) It("should succeed to resolve AMIs (AL2)", func() { diff --git a/pkg/providers/launchtemplate/suite_test.go b/pkg/providers/launchtemplate/suite_test.go index ab6eccb910c0..2c952e8d553f 100644 --- a/pkg/providers/launchtemplate/suite_test.go +++ b/pkg/providers/launchtemplate/suite_test.go @@ -1659,7 +1659,7 @@ var _ = Describe("LaunchTemplates", func() { Expect(awsEnv.EC2API.CalledWithCreateLaunchTemplateInput.Len()).To(Equal(0)) }) It("should choose amis from SSM if no selector specified in AWSNodeTemplate", func() { - version := lo.Must(awsEnv.AMIProvider.KubeServerVersion(ctx)) + version := lo.Must(awsEnv.VersionProvider.Get(ctx)) awsEnv.SSMAPI.Parameters = map[string]string{ fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2/recommended/image_id", version): "test-ami-123", } diff --git a/pkg/providers/version/version.go b/pkg/providers/version/version.go new file mode 100644 index 000000000000..7f057734240b --- /dev/null +++ b/pkg/providers/version/version.go @@ -0,0 +1,64 @@ +/* +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package version + +import ( + "context" + "fmt" + "strings" + + "github.com/patrickmn/go-cache" + "k8s.io/client-go/kubernetes" + "knative.dev/pkg/logging" + + "github.com/aws/karpenter-core/pkg/utils/pretty" +) + +const ( + kubernetesVersionCacheKey = "kubernetesVersion" +) + +// Provider get the APIServer version. This will be initialized at start up and allows karpenter to have an understanding of the cluster version +// for decision making. The version is cached to help reduce the amount of calls made to the API Server + +type Provider struct { + cache *cache.Cache + cm *pretty.ChangeMonitor + kubernetesInterface kubernetes.Interface +} + +func NewProvider(kubernetesInterface kubernetes.Interface, cache *cache.Cache) *Provider { + return &Provider{ + cm: pretty.NewChangeMonitor(), + cache: cache, + kubernetesInterface: kubernetesInterface, + } +} + +func (p *Provider) Get(ctx context.Context) (string, error) { + if version, ok := p.cache.Get(kubernetesVersionCacheKey); ok { + return version.(string), nil + } + serverVersion, err := p.kubernetesInterface.Discovery().ServerVersion() + if err != nil { + return "", err + } + version := fmt.Sprintf("%s.%s", serverVersion.Major, strings.TrimSuffix(serverVersion.Minor, "+")) + p.cache.SetDefault(kubernetesVersionCacheKey, version) + if p.cm.HasChanged("kubernetes-version", version) { + logging.FromContext(ctx).With("version", version).Debugf("discovered kubernetes version") + } + return version, nil +} diff --git a/pkg/test/environment.go b/pkg/test/environment.go index aac5e79111b4..34af278641ba 100644 --- a/pkg/test/environment.go +++ b/pkg/test/environment.go @@ -31,6 +31,7 @@ import ( "github.com/aws/karpenter/pkg/providers/pricing" "github.com/aws/karpenter/pkg/providers/securitygroup" "github.com/aws/karpenter/pkg/providers/subnet" + "github.com/aws/karpenter/pkg/providers/version" coretest "github.com/aws/karpenter-core/pkg/test" @@ -60,6 +61,7 @@ type Environment struct { PricingProvider *pricing.Provider AMIProvider *amifamily.Provider AMIResolver *amifamily.Resolver + VersionProvider *version.Provider LaunchTemplateProvider *launchtemplate.Provider } @@ -82,7 +84,8 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment pricingProvider := pricing.NewProvider(ctx, fakePricingAPI, ec2api, "") subnetProvider := subnet.NewProvider(ec2api, subnetCache) securityGroupProvider := securitygroup.NewProvider(ec2api, securityGroupCache) - amiProvider := amifamily.NewProvider(env.Client, env.KubernetesInterface, ssmapi, ec2api, ec2Cache, kubernetesVersionCache) + versionProvider := version.NewProvider(env.KubernetesInterface, kubernetesVersionCache) + amiProvider := amifamily.NewProvider(versionProvider, ssmapi, ec2api, ec2Cache) amiResolver := amifamily.New(amiProvider) instanceTypesProvider := instancetype.NewProvider("", instanceTypeCache, ec2api, subnetProvider, unavailableOfferingsCache, pricingProvider) launchTemplateProvider := @@ -128,6 +131,7 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment PricingProvider: pricingProvider, AMIProvider: amiProvider, AMIResolver: amiResolver, + VersionProvider: versionProvider, LaunchTemplateProvider: launchTemplateProvider, } }