From e3194d53345c86bc4244f1340c8f6ecbb3da441e Mon Sep 17 00:00:00 2001 From: vishal Date: Mon, 23 Mar 2020 17:48:21 -0400 Subject: [PATCH 1/2] Autofill availability zones based on primary instance --- cli/cmd/lib_cluster_config.go | 2 +- pkg/types/clusterconfig/availability_zones.go | 4 ++-- pkg/types/clusterconfig/clusterconfig.go | 15 +++------------ 3 files changed, 6 insertions(+), 15 deletions(-) diff --git a/cli/cmd/lib_cluster_config.go b/cli/cmd/lib_cluster_config.go index 5d9c226082..55ff8f051a 100644 --- a/cli/cmd/lib_cluster_config.go +++ b/cli/cmd/lib_cluster_config.go @@ -231,7 +231,7 @@ func getClusterUpdateConfig(cachedClusterConfig clusterconfig.Config, awsCreds A return nil, clusterconfig.ErrorConfiguredWhenSpotIsNotEnabled(clusterconfig.SpotConfigKey) } - if !strset.New(userClusterConfig.SpotConfig.InstanceDistribution...).IsEqual(strset.New(cachedClusterConfig.SpotConfig.InstanceDistribution...)) { + if len(userClusterConfig.SpotConfig.InstanceDistribution) != 0 && !strset.New(userClusterConfig.SpotConfig.InstanceDistribution...).IsEqual(strset.New(cachedClusterConfig.SpotConfig.InstanceDistribution...)) { return nil, errors.Wrap(clusterconfig.ErrorConfigCannotBeChangedOnUpdate(clusterconfig.InstanceDistributionKey, cachedClusterConfig.SpotConfig.InstanceDistribution), clusterconfig.SpotConfigKey) } diff --git a/pkg/types/clusterconfig/availability_zones.go b/pkg/types/clusterconfig/availability_zones.go index fa67259613..6f0830eed2 100644 --- a/pkg/types/clusterconfig/availability_zones.go +++ b/pkg/types/clusterconfig/availability_zones.go @@ -34,13 +34,13 @@ func (cc *Config) validateAvailabilityZones(awsClient *aws.Client) error { } if len(cc.AvailabilityZones) == 0 { - if err := cc.setDefaultAvailabilityZones(awsClient, extraInstances...); err != nil { + if err := cc.setDefaultAvailabilityZones(awsClient); err != nil { return err } return nil } - if err := cc.validateUserAvailabilityZones(awsClient, extraInstances...); err != nil { + if err := cc.validateUserAvailabilityZones(awsClient); err != nil { return err } diff --git a/pkg/types/clusterconfig/clusterconfig.go b/pkg/types/clusterconfig/clusterconfig.go index 170c7a3a11..4e55d42cd2 100644 --- a/pkg/types/clusterconfig/clusterconfig.go +++ b/pkg/types/clusterconfig/clusterconfig.go @@ -483,24 +483,12 @@ func (cc *Config) Validate(awsClient *aws.Client) error { } } - // instance_distribution cleanup must be performed before availability_zone cleanup - if cc.Spot != nil && *cc.Spot && len(cc.SpotConfig.InstanceDistribution) >= 0 { - cleanedDistribution := []string{*cc.InstanceType} - for _, instanceType := range cc.SpotConfig.InstanceDistribution { - if instanceType != *cc.InstanceType { - cleanedDistribution = append(cleanedDistribution, instanceType) - } - } - cc.SpotConfig.InstanceDistribution = cleanedDistribution - } - if err := cc.validateAvailabilityZones(awsClient); err != nil { return errors.Wrap(err, AvailabilityZonesKey) } if cc.Spot != nil && *cc.Spot { cc.AutoFillSpot(awsClient) - chosenInstance := aws.InstanceMetadatas[*cc.Region][*cc.InstanceType] compatibleSpots := CompatibleSpotInstances(awsClient, chosenInstance, cc.SpotConfig.MaxPrice, _spotInstanceDistributionLength) if len(compatibleSpots) == 0 { @@ -640,6 +628,9 @@ func CompatibleSpotInstances(awsClient *aws.Client, targetInstance aws.InstanceM func AutoGenerateSpotConfig(awsClient *aws.Client, spotConfig *SpotConfig, region string, instanceType string) error { chosenInstance := aws.InstanceMetadatas[region][instanceType] + if len(spotConfig.InstanceDistribution) == 0 { + spotConfig.InstanceDistribution = append(spotConfig.InstanceDistribution, instanceType) + } if len(spotConfig.InstanceDistribution) == 1 { compatibleSpots := CompatibleSpotInstances(awsClient, chosenInstance, spotConfig.MaxPrice, _spotInstanceDistributionLength) if len(compatibleSpots) == 0 { From ee8347c620782d0a3a3eb85963f018cecb686efa Mon Sep 17 00:00:00 2001 From: vishal Date: Mon, 23 Mar 2020 19:22:11 -0400 Subject: [PATCH 2/2] Respond to PR --- cli/cmd/lib_cluster_config.go | 2 +- pkg/types/clusterconfig/availability_zones.go | 9 --------- pkg/types/clusterconfig/clusterconfig.go | 9 +++++++-- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/cli/cmd/lib_cluster_config.go b/cli/cmd/lib_cluster_config.go index 55ff8f051a..5d9c226082 100644 --- a/cli/cmd/lib_cluster_config.go +++ b/cli/cmd/lib_cluster_config.go @@ -231,7 +231,7 @@ func getClusterUpdateConfig(cachedClusterConfig clusterconfig.Config, awsCreds A return nil, clusterconfig.ErrorConfiguredWhenSpotIsNotEnabled(clusterconfig.SpotConfigKey) } - if len(userClusterConfig.SpotConfig.InstanceDistribution) != 0 && !strset.New(userClusterConfig.SpotConfig.InstanceDistribution...).IsEqual(strset.New(cachedClusterConfig.SpotConfig.InstanceDistribution...)) { + if !strset.New(userClusterConfig.SpotConfig.InstanceDistribution...).IsEqual(strset.New(cachedClusterConfig.SpotConfig.InstanceDistribution...)) { return nil, errors.Wrap(clusterconfig.ErrorConfigCannotBeChangedOnUpdate(clusterconfig.InstanceDistributionKey, cachedClusterConfig.SpotConfig.InstanceDistribution), clusterconfig.SpotConfigKey) } diff --git a/pkg/types/clusterconfig/availability_zones.go b/pkg/types/clusterconfig/availability_zones.go index 6f0830eed2..b1f2077ada 100644 --- a/pkg/types/clusterconfig/availability_zones.go +++ b/pkg/types/clusterconfig/availability_zones.go @@ -24,15 +24,6 @@ import ( var _azBlacklist = strset.New("us-east-1e") func (cc *Config) validateAvailabilityZones(awsClient *aws.Client) error { - var extraInstances []string - if cc.Spot != nil && *cc.Spot && len(cc.SpotConfig.InstanceDistribution) >= 0 { - for _, instanceType := range cc.SpotConfig.InstanceDistribution { - if instanceType != *cc.InstanceType { - extraInstances = append(extraInstances, instanceType) - } - } - } - if len(cc.AvailabilityZones) == 0 { if err := cc.setDefaultAvailabilityZones(awsClient); err != nil { return err diff --git a/pkg/types/clusterconfig/clusterconfig.go b/pkg/types/clusterconfig/clusterconfig.go index 4e55d42cd2..bd6a9cc73d 100644 --- a/pkg/types/clusterconfig/clusterconfig.go +++ b/pkg/types/clusterconfig/clusterconfig.go @@ -628,9 +628,14 @@ func CompatibleSpotInstances(awsClient *aws.Client, targetInstance aws.InstanceM func AutoGenerateSpotConfig(awsClient *aws.Client, spotConfig *SpotConfig, region string, instanceType string) error { chosenInstance := aws.InstanceMetadatas[region][instanceType] - if len(spotConfig.InstanceDistribution) == 0 { - spotConfig.InstanceDistribution = append(spotConfig.InstanceDistribution, instanceType) + cleanedDistribution := []string{instanceType} + for _, spotInstance := range spotConfig.InstanceDistribution { + if spotInstance != instanceType { + cleanedDistribution = append(cleanedDistribution, spotInstance) + } } + spotConfig.InstanceDistribution = cleanedDistribution + if len(spotConfig.InstanceDistribution) == 1 { compatibleSpots := CompatibleSpotInstances(awsClient, chosenInstance, spotConfig.MaxPrice, _spotInstanceDistributionLength) if len(compatibleSpots) == 0 {