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.

Sign up for GitHub

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

Automated cherry pick of #3185: cluster-autoscaler: use generated instance types #3222: Fix AWS CA tests for InstanceType generation changes #3335

Merged
merged 2 commits into from
Jul 21, 2020
Merged

Automated cherry pick of #3185: cluster-autoscaler: use generated instance types #3222: Fix AWS CA tests for InstanceType generation changes #3335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a0c33e
cluster-autoscaler: use generated instance types
JulienBalestra Jun 3, 2020
52c0659
Fix AWS CA tests for InstanceType generation changes
gjtempleton Jun 15, 2020
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
26 changes: 16 additions & 10 deletions cluster-autoscaler/cloudprovider/aws/aws_cloud_provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -347,21 +344,30 @@ 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()
if err != nil {
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 {
Expand All @@ -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)
}
Expand Down
6 changes: 2 additions & 4 deletions cluster-autoscaler/cloudprovider/aws/aws_cloud_provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}

Expand Down
9 changes: 6 additions & 3 deletions cluster-autoscaler/cloudprovider/aws/aws_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ type AwsManager struct {
ec2Service ec2Wrapper
asgCache *asgCache
lastRefresh time.Time
instanceTypes map[string]*InstanceType
}

type asgTemplate struct {
Expand Down Expand Up @@ -172,6 +173,7 @@ func createAWSManagerInternal(
discoveryOpts cloudprovider.NodeGroupDiscoveryOptions,
autoScalingService *autoScalingWrapper,
ec2Service *ec2Wrapper,
instanceTypes map[string]*InstanceType,
) (*AwsManager, error) {

cfg, err := readAWSCloudConfig(configReader)
Expand Down Expand Up @@ -216,6 +218,7 @@ func createAWSManagerInternal(
autoScalingService: *autoScalingService,
ec2Service: *ec2Service,
asgCache: cache,
instanceTypes: instanceTypes,
}

if err := manager.forceRefresh(); err != nil {
Expand All @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
29 changes: 17 additions & 12 deletions cluster-autoscaler/cloudprovider/aws/aws_manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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,
},
}

Expand All @@ -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) {
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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()
Expand Down