From 4a0c33e0e117b3330dbe2fd424beed0a79c803fe Mon Sep 17 00:00:00 2001 From: Julien Balestra Date: Wed, 3 Jun 2020 15:16:18 +0200 Subject: [PATCH 1/2] cluster-autoscaler: use generated instance types Signed-off-by: Julien Balestra --- .../cloudprovider/aws/aws_cloud_provider.go | 26 ++++++++++++------- .../cloudprovider/aws/aws_manager.go | 9 ++++--- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/aws/aws_cloud_provider.go b/cluster-autoscaler/cloudprovider/aws/aws_cloud_provider.go index 0cb32bc92fe0..ec126d2a84de 100644 --- a/cluster-autoscaler/cloudprovider/aws/aws_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/aws/aws_cloud_provider.go @@ -49,16 +49,13 @@ var ( type awsCloudProvider struct { awsManager *AwsManager resourceLimiter *cloudprovider.ResourceLimiter - // InstanceTypes is a map of ec2 resources - instanceTypes map[string]*InstanceType } // BuildAwsCloudProvider builds CloudProvider implementation for AWS. -func BuildAwsCloudProvider(awsManager *AwsManager, instanceTypes map[string]*InstanceType, resourceLimiter *cloudprovider.ResourceLimiter) (cloudprovider.CloudProvider, error) { +func BuildAwsCloudProvider(awsManager *AwsManager, resourceLimiter *cloudprovider.ResourceLimiter) (cloudprovider.CloudProvider, error) { aws := &awsCloudProvider{ awsManager: awsManager, resourceLimiter: resourceLimiter, - instanceTypes: instanceTypes, } return aws, nil } @@ -347,10 +344,8 @@ func BuildAWS(opts config.AutoscalingOptions, do cloudprovider.NodeGroupDiscover } // Generate EC2 list - var instanceTypes map[string]*InstanceType - var lastUpdateTime string + instanceTypes, lastUpdateTime := GetStaticEC2InstanceTypes() if opts.AWSUseStaticInstanceList { - instanceTypes, lastUpdateTime = GetStaticEC2InstanceTypes() klog.Warningf("Use static EC2 Instance Types and list could be outdated. Last update time: %s", lastUpdateTime) } else { region, err := GetCurrentAwsRegion() @@ -358,10 +353,21 @@ func BuildAWS(opts config.AutoscalingOptions, do cloudprovider.NodeGroupDiscover klog.Fatalf("Failed to get AWS Region: %v", err) } - instanceTypes, err = GenerateEC2InstanceTypes(region) + generatedInstanceTypes, err := GenerateEC2InstanceTypes(region) if err != nil { klog.Fatalf("Failed to generate AWS EC2 Instance Types: %v", err) } + // fallback on the static list if we miss any instance types in the generated output + // credits to: https://github.com/lyft/cni-ipvlan-vpc-k8s/pull/80 + for k, v := range instanceTypes { + _, ok := generatedInstanceTypes[k] + if ok { + continue + } + klog.Infof("Using static instance type %s", k) + generatedInstanceTypes[k] = v + } + instanceTypes = generatedInstanceTypes keys := make([]string, 0, len(instanceTypes)) for key := range instanceTypes { @@ -371,12 +377,12 @@ func BuildAWS(opts config.AutoscalingOptions, do cloudprovider.NodeGroupDiscover klog.Infof("Successfully load %d EC2 Instance Types %s", len(keys), keys) } - manager, err := CreateAwsManager(config, do) + manager, err := CreateAwsManager(config, do, instanceTypes) if err != nil { klog.Fatalf("Failed to create AWS Manager: %v", err) } - provider, err := BuildAwsCloudProvider(manager, instanceTypes, rl) + provider, err := BuildAwsCloudProvider(manager, rl) if err != nil { klog.Fatalf("Failed to create AWS cloud provider: %v", err) } diff --git a/cluster-autoscaler/cloudprovider/aws/aws_manager.go b/cluster-autoscaler/cloudprovider/aws/aws_manager.go index fa33bc9a7324..cd6b645fe0b4 100644 --- a/cluster-autoscaler/cloudprovider/aws/aws_manager.go +++ b/cluster-autoscaler/cloudprovider/aws/aws_manager.go @@ -59,6 +59,7 @@ type AwsManager struct { ec2Service ec2Wrapper asgCache *asgCache lastRefresh time.Time + instanceTypes map[string]*InstanceType } type asgTemplate struct { @@ -172,6 +173,7 @@ func createAWSManagerInternal( discoveryOpts cloudprovider.NodeGroupDiscoveryOptions, autoScalingService *autoScalingWrapper, ec2Service *ec2Wrapper, + instanceTypes map[string]*InstanceType, ) (*AwsManager, error) { cfg, err := readAWSCloudConfig(configReader) @@ -216,6 +218,7 @@ func createAWSManagerInternal( autoScalingService: *autoScalingService, ec2Service: *ec2Service, asgCache: cache, + instanceTypes: instanceTypes, } if err := manager.forceRefresh(); err != nil { @@ -241,8 +244,8 @@ func readAWSCloudConfig(config io.Reader) (*provider_aws.CloudConfig, error) { } // CreateAwsManager constructs awsManager object. -func CreateAwsManager(configReader io.Reader, discoveryOpts cloudprovider.NodeGroupDiscoveryOptions) (*AwsManager, error) { - return createAWSManagerInternal(configReader, discoveryOpts, nil, nil) +func CreateAwsManager(configReader io.Reader, discoveryOpts cloudprovider.NodeGroupDiscoveryOptions, instanceTypes map[string]*InstanceType) (*AwsManager, error) { + return createAWSManagerInternal(configReader, discoveryOpts, nil, nil, instanceTypes) } // Refresh is called before every main loop and can be used to dynamically update cloud provider state. @@ -314,7 +317,7 @@ func (m *AwsManager) getAsgTemplate(asg *asg) (*asgTemplate, error) { return nil, err } - if t, ok := InstanceTypes[instanceTypeName]; ok { + if t, ok := m.instanceTypes[instanceTypeName]; ok { return &asgTemplate{ InstanceType: t, Region: region, From 52c06594b1009ef2cb2af098e9ecd3966635600b Mon Sep 17 00:00:00 2001 From: GuyTempleton Date: Mon, 15 Jun 2020 08:59:01 +0100 Subject: [PATCH 2/2] Fix AWS CA tests for InstanceType generation changes --- .../aws/aws_cloud_provider_test.go | 6 ++-- .../cloudprovider/aws/aws_manager_test.go | 29 +++++++++++-------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/aws/aws_cloud_provider_test.go b/cluster-autoscaler/cloudprovider/aws/aws_cloud_provider_test.go index 803c1bf7ce04..585a88622cb0 100644 --- a/cluster-autoscaler/cloudprovider/aws/aws_cloud_provider_test.go +++ b/cluster-autoscaler/cloudprovider/aws/aws_cloud_provider_test.go @@ -133,8 +133,7 @@ func testProvider(t *testing.T, m *AwsManager) *awsCloudProvider { map[string]int64{cloudprovider.ResourceNameCores: 1, cloudprovider.ResourceNameMemory: 10000000}, map[string]int64{cloudprovider.ResourceNameCores: 10, cloudprovider.ResourceNameMemory: 100000000}) - instanceTypes, _ := GetStaticEC2InstanceTypes() - provider, err := BuildAwsCloudProvider(m, instanceTypes, resourceLimiter) + provider, err := BuildAwsCloudProvider(m, resourceLimiter) assert.NoError(t, err) return provider.(*awsCloudProvider) } @@ -144,8 +143,7 @@ func TestBuildAwsCloudProvider(t *testing.T) { map[string]int64{cloudprovider.ResourceNameCores: 1, cloudprovider.ResourceNameMemory: 10000000}, map[string]int64{cloudprovider.ResourceNameCores: 10, cloudprovider.ResourceNameMemory: 100000000}) - instanceTypes, _ := GetStaticEC2InstanceTypes() - _, err := BuildAwsCloudProvider(testAwsManager, instanceTypes, resourceLimiter) + _, err := BuildAwsCloudProvider(testAwsManager, resourceLimiter) assert.NoError(t, err) } diff --git a/cluster-autoscaler/cloudprovider/aws/aws_manager_test.go b/cluster-autoscaler/cloudprovider/aws/aws_manager_test.go index ad62e471ea7b..1d69f0d8e37e 100644 --- a/cluster-autoscaler/cloudprovider/aws/aws_manager_test.go +++ b/cluster-autoscaler/cloudprovider/aws/aws_manager_test.go @@ -301,8 +301,8 @@ func TestFetchExplicitAsgs(t *testing.T) { // #1449 Without AWS_REGION getRegion() lookup runs till timeout during tests. defer resetAWSRegion(os.LookupEnv("AWS_REGION")) os.Setenv("AWS_REGION", "fanghorn") - // fetchExplicitASGs is called at manager creation time. - m, err := createAWSManagerInternal(nil, do, &autoScalingWrapper{s, map[string]string{}}, nil) + instanceTypes, _ := GetStaticEC2InstanceTypes() + m, err := createAWSManagerInternal(nil, do, &autoScalingWrapper{s, newLaunchConfigurationInstanceTypeCache()}, nil, instanceTypes) assert.NoError(t, err) asgs := m.asgCache.Get() @@ -330,7 +330,8 @@ func TestBuildInstanceType(t *testing.T) { // #1449 Without AWS_REGION getRegion() lookup runs till timeout during tests. defer resetAWSRegion(os.LookupEnv("AWS_REGION")) os.Setenv("AWS_REGION", "fanghorn") - m, err := createAWSManagerInternal(nil, cloudprovider.NodeGroupDiscoveryOptions{}, nil, &ec2Wrapper{s}) + instanceTypes, _ := GetStaticEC2InstanceTypes() + m, err := createAWSManagerInternal(nil, cloudprovider.NodeGroupDiscoveryOptions{}, nil, &ec2Wrapper{s}, instanceTypes) assert.NoError(t, err) asg := asg{ @@ -345,7 +346,7 @@ func TestBuildInstanceType(t *testing.T) { func TestBuildInstanceTypeMixedInstancePolicyOverride(t *testing.T) { ltName, ltVersion, instanceType := "launcher", "1", "t2.large" - instanceTypes := []string{} + instanceTypeOverrides := []string{} s := &EC2Mock{} s.On("DescribeLaunchTemplateVersions", &ec2.DescribeLaunchTemplateVersionsInput{ @@ -363,14 +364,15 @@ func TestBuildInstanceTypeMixedInstancePolicyOverride(t *testing.T) { defer resetAWSRegion(os.LookupEnv("AWS_REGION")) os.Setenv("AWS_REGION", "fanghorn") - m, err := createAWSManagerInternal(nil, cloudprovider.NodeGroupDiscoveryOptions{}, nil, &ec2Wrapper{s}) + instanceTypes, _ := GetStaticEC2InstanceTypes() + m, err := createAWSManagerInternal(nil, cloudprovider.NodeGroupDiscoveryOptions{}, nil, &ec2Wrapper{s}, instanceTypes) assert.NoError(t, err) lt := &launchTemplate{name: ltName, version: ltVersion} asg := asg{ MixedInstancesPolicy: &mixedInstancesPolicy{ launchTemplate: lt, - instanceTypesOverrides: instanceTypes, + instanceTypesOverrides: instanceTypeOverrides, }, } @@ -382,25 +384,26 @@ func TestBuildInstanceTypeMixedInstancePolicyOverride(t *testing.T) { func TestBuildInstanceTypeMixedInstancePolicyNoOverride(t *testing.T) { ltName, ltVersion := "launcher", "1" - instanceTypes := []string{"m4.xlarge", "m5.xlarge"} + instanceTypeOverrides := []string{"m4.xlarge", "m5.xlarge"} defer resetAWSRegion(os.LookupEnv("AWS_REGION")) os.Setenv("AWS_REGION", "fanghorn") - m, err := createAWSManagerInternal(nil, cloudprovider.NodeGroupDiscoveryOptions{}, nil, &ec2Wrapper{}) + instanceTypes, _ := GetStaticEC2InstanceTypes() + m, err := createAWSManagerInternal(nil, cloudprovider.NodeGroupDiscoveryOptions{}, nil, &ec2Wrapper{}, instanceTypes) assert.NoError(t, err) lt := &launchTemplate{name: ltName, version: ltVersion} asg := asg{ MixedInstancesPolicy: &mixedInstancesPolicy{ launchTemplate: lt, - instanceTypesOverrides: instanceTypes, + instanceTypesOverrides: instanceTypeOverrides, }, } builtInstanceType, err := m.buildInstanceType(&asg) assert.NoError(t, err) - assert.Equal(t, instanceTypes[0], builtInstanceType) + assert.Equal(t, instanceTypeOverrides[0], builtInstanceType) } func TestGetASGTemplate(t *testing.T) { @@ -454,7 +457,8 @@ func TestGetASGTemplate(t *testing.T) { // #1449 Without AWS_REGION getRegion() lookup runs till timeout during tests. defer resetAWSRegion(os.LookupEnv("AWS_REGION")) os.Setenv("AWS_REGION", "fanghorn") - m, err := createAWSManagerInternal(nil, cloudprovider.NodeGroupDiscoveryOptions{}, nil, &ec2Wrapper{s}) + instanceTypes, _ := GetStaticEC2InstanceTypes() + m, err := createAWSManagerInternal(nil, cloudprovider.NodeGroupDiscoveryOptions{}, nil, &ec2Wrapper{s}, instanceTypes) assert.NoError(t, err) asg := &asg{ @@ -536,7 +540,8 @@ func TestFetchAutoAsgs(t *testing.T) { defer resetAWSRegion(os.LookupEnv("AWS_REGION")) os.Setenv("AWS_REGION", "fanghorn") // fetchAutoASGs is called at manager creation time, via forceRefresh - m, err := createAWSManagerInternal(nil, do, &autoScalingWrapper{s, map[string]string{}}, nil) + instanceTypes, _ := GetStaticEC2InstanceTypes() + m, err := createAWSManagerInternal(nil, do, &autoScalingWrapper{s, newLaunchConfigurationInstanceTypeCache()}, nil, instanceTypes) assert.NoError(t, err) asgs := m.asgCache.Get()