From ef99e61f8357939596f4eb684e2fff77d4c14f9e Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Tue, 27 Jun 2017 00:19:08 -0400 Subject: [PATCH] Query AWS to determine available instance types --- upup/pkg/fi/cloudup/awsup/aws_cloud.go | 85 +++++++++++ upup/pkg/fi/cloudup/awsup/mock_aws_cloud.go | 14 ++ .../fi/cloudup/populate_instancegroup_spec.go | 135 ++++++------------ .../fi/cloudup/populateinstancegroup_test.go | 23 --- 4 files changed, 146 insertions(+), 111 deletions(-) diff --git a/upup/pkg/fi/cloudup/awsup/aws_cloud.go b/upup/pkg/fi/cloudup/awsup/aws_cloud.go index 605e33519a..2754978744 100644 --- a/upup/pkg/fi/cloudup/awsup/aws_cloud.go +++ b/upup/pkg/fi/cloudup/awsup/aws_cloud.go @@ -31,6 +31,7 @@ import ( "github.com/aws/aws-sdk-go/service/route53" "github.com/aws/aws-sdk-go/service/route53/route53iface" "github.com/golang/glog" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/upup/pkg/fi" "k8s.io/kubernetes/federation/pkg/dnsprovider" @@ -116,6 +117,9 @@ type AWSCloud interface { // WithTags created a copy of AWSCloud with the specified default-tags bound WithTags(tags map[string]string) AWSCloud + + // DefaultInstanceType determines a suitable instance type for the specified instance group + DefaultInstanceType(cluster *kops.Cluster, ig *kops.InstanceGroup) (string, error) } type awsCloudImplementation struct { @@ -749,3 +753,84 @@ func (c *awsCloudImplementation) FindVPCInfo(vpcID string) (*fi.VPCInfo, error) return vpcInfo, nil } + +// DefaultInstanceType determines an instance type for the specified cluster & instance group +func (c *awsCloudImplementation) DefaultInstanceType(cluster *kops.Cluster, ig *kops.InstanceGroup) (string, error) { + var candidates []string + + switch ig.Spec.Role { + case kops.InstanceGroupRoleMaster: + // Some regions do not (currently) support the m3 family; the c4 large is the cheapest non-burstable instance + // (us-east-2, ca-central-1, eu-west-2, ap-northeast-2). + // Also some accounts are no longer supporting m3 in us-east-1 zones + candidates = []string{"m3.medium", "c4.large"} + + case kops.InstanceGroupRoleNode: + candidates = []string{"t2.medium"} + + case kops.InstanceGroupRoleBastion: + candidates = []string{"t2.micro"} + + default: + return "", fmt.Errorf("unhandled role %q", ig.Spec.Role) + } + + // Find the AZs the InstanceGroup targets + igZones := sets.NewString() + for _, subnetName := range ig.Spec.Subnets { + var subnet *kops.ClusterSubnetSpec + for i := range cluster.Spec.Subnets { + if cluster.Spec.Subnets[i].Name == subnetName { + subnet = &cluster.Spec.Subnets[i] + } + } + if subnet == nil { + return "", fmt.Errorf("subnet %q is not defined in cluster", subnetName) + } + igZones.Insert(subnet.Zone) + } + + // TODO: Validate that instance type exists in all AZs, but skip AZs that don't support any VPC stuff + for _, instanceType := range candidates { + zones, err := c.zonesWithInstanceType(instanceType) + if err != nil { + return "", err + } + if zones.IsSuperset(igZones) { + return instanceType, nil + } else { + glog.V(2).Infof("can't use instance type %q, available in zones %v but need %v", instanceType, zones, igZones) + } + } + + return "", fmt.Errorf("could not find a suitable supported instance type for the instance group %q (type %q) in region %q", ig.Name, ig.Spec.Role, c.region) +} + +// supportsInstanceType uses the DescribeReservedInstancesOfferings API call to determine if an instance type is supported in a region +func (c *awsCloudImplementation) zonesWithInstanceType(instanceType string) (sets.String, error) { + glog.V(4).Infof("checking if instance type %q is supported in region %q", instanceType, c.region) + request := &ec2.DescribeReservedInstancesOfferingsInput{} + request.InstanceTenancy = aws.String("default") + request.IncludeMarketplace = aws.Bool(false) + request.OfferingClass = aws.String("standard") + request.OfferingType = aws.String("No Upfront") + request.ProductDescription = aws.String("Linux/UNIX (Amazon VPC)") + request.InstanceType = aws.String(instanceType) + + zones := sets.NewString() + + response, err := c.ec2.DescribeReservedInstancesOfferings(request) + if err != nil { + return zones, fmt.Errorf("error checking if instance type %q is supported in region %q: %v", instanceType, c.region, err) + } + + for _, item := range response.ReservedInstancesOfferings { + if aws.StringValue(item.InstanceType) == instanceType { + zones.Insert(aws.StringValue(item.AvailabilityZone)) + } else { + glog.Warningf("skipping non-matching instance type offering: %v", item) + } + } + + return zones, nil +} diff --git a/upup/pkg/fi/cloudup/awsup/mock_aws_cloud.go b/upup/pkg/fi/cloudup/awsup/mock_aws_cloud.go index d39168f50e..ff3957a574 100644 --- a/upup/pkg/fi/cloudup/awsup/mock_aws_cloud.go +++ b/upup/pkg/fi/cloudup/awsup/mock_aws_cloud.go @@ -194,3 +194,17 @@ func (c *MockAWSCloud) Route53() route53iface.Route53API { func (c *MockAWSCloud) FindVPCInfo(id string) (*fi.VPCInfo, error) { return nil, fmt.Errorf("MockAWSCloud FindVPCInfo not implemented") } + +// DefaultInstanceType determines an instance type for the specified cluster & instance group +func (c *MockAWSCloud) DefaultInstanceType(cluster *kops.Cluster, ig *kops.InstanceGroup) (string, error) { + switch ig.Spec.Role { + case kops.InstanceGroupRoleMaster: + return "m3.medium", nil + case kops.InstanceGroupRoleNode: + return "t2.medium", nil + case kops.InstanceGroupRoleBastion: + return "t2.micro", nil + default: + return "", fmt.Errorf("MockAWSCloud DefaultInstanceType does not handle %s", ig.Spec.Role) + } +} diff --git a/upup/pkg/fi/cloudup/populate_instancegroup_spec.go b/upup/pkg/fi/cloudup/populate_instancegroup_spec.go index 7320fde6de..25c08635e8 100644 --- a/upup/pkg/fi/cloudup/populate_instancegroup_spec.go +++ b/upup/pkg/fi/cloudup/populate_instancegroup_spec.go @@ -31,29 +31,18 @@ import ( // Default Machine types for various types of instance group machine const ( - defaultNodeMachineTypeAWS = "t2.medium" defaultNodeMachineTypeGCE = "n1-standard-2" defaultNodeMachineTypeVSphere = "vsphere_node" - defaultBastionMachineTypeAWS = "t2.micro" defaultBastionMachineTypeGCE = "f1-micro" defaultBastionMachineTypeVSphere = "vsphere_bastion" defaultMasterMachineTypeGCE = "n1-standard-1" - defaultMasterMachineTypeAWS = "m3.medium" defaultMasterMachineTypeVSphere = "vsphere_master" defaultVSphereNodeImage = "kops_ubuntu_16_04.ova" ) -var masterMachineTypeExceptions = map[string]string{ - // Some regions do not (currently) support the m3 family; the c4 large is the cheapest non-burstable instance - "us-east-2": "c4.large", - "ca-central-1": "c4.large", - "eu-west-2": "c4.large", - "ap-northeast-2": "c4.large", -} - var awsDedicatedInstanceExceptions = map[string]bool{ "t2.nano": true, "t2.micro": true, @@ -77,7 +66,11 @@ func PopulateInstanceGroupSpec(cluster *kops.Cluster, input *kops.InstanceGroup, // TODO: Clean up if ig.IsMaster() { if ig.Spec.MachineType == "" { - ig.Spec.MachineType = defaultMasterMachineType(cluster) + ig.Spec.MachineType, err = defaultMachineType(cluster, ig) + if err != nil { + return nil, fmt.Errorf("error assigning default machine type for masters: %v", err) + } + } if ig.Spec.MinSize == nil { ig.Spec.MinSize = fi.Int32(1) @@ -87,7 +80,10 @@ func PopulateInstanceGroupSpec(cluster *kops.Cluster, input *kops.InstanceGroup, } } else if ig.Spec.Role == kops.InstanceGroupRoleBastion { if ig.Spec.MachineType == "" { - ig.Spec.MachineType = defaultBastionMachineType(cluster) + ig.Spec.MachineType, err = defaultMachineType(cluster, ig) + if err != nil { + return nil, fmt.Errorf("error assigning default machine type for bastions: %v", err) + } } if ig.Spec.MinSize == nil { ig.Spec.MinSize = fi.Int32(1) @@ -97,7 +93,10 @@ func PopulateInstanceGroupSpec(cluster *kops.Cluster, input *kops.InstanceGroup, } } else { if ig.Spec.MachineType == "" { - ig.Spec.MachineType = defaultNodeMachineType(cluster) + ig.Spec.MachineType, err = defaultMachineType(cluster, ig) + if err != nil { + return nil, fmt.Errorf("error assigning default machine type for nodes: %v", err) + } } if ig.Spec.MinSize == nil { ig.Spec.MinSize = fi.Int32(2) @@ -151,88 +150,48 @@ func PopulateInstanceGroupSpec(cluster *kops.Cluster, input *kops.InstanceGroup, return ig, nil } -// defaultNodeMachineType returns the default MachineType for nodes, based on the cloudprovider -func defaultNodeMachineType(cluster *kops.Cluster) string { +// defaultMachineType returns the default MachineType for the instance group, based on the cloudprovider +func defaultMachineType(cluster *kops.Cluster, ig *kops.InstanceGroup) (string, error) { switch kops.CloudProviderID(cluster.Spec.CloudProvider) { case kops.CloudProviderAWS: - return defaultNodeMachineTypeAWS - case kops.CloudProviderGCE: - return defaultNodeMachineTypeGCE - case kops.CloudProviderVSphere: - return defaultNodeMachineTypeVSphere - default: - glog.V(2).Infof("Cannot set default MachineType for CloudProvider=%q", cluster.Spec.CloudProvider) - return "" - } -} - -// defaultMasterMachineType returns the default MachineType for masters, based on the cloudprovider -func defaultMasterMachineType(cluster *kops.Cluster) string { - // TODO: We used to have logic like the following... - // {{ if gt .TotalNodeCount 500 }} - // MasterMachineType: n1-standard-32 - // {{ else if gt .TotalNodeCount 250 }} - //MasterMachineType: n1-standard-16 - //{{ else if gt .TotalNodeCount 100 }} - //MasterMachineType: n1-standard-8 - //{{ else if gt .TotalNodeCount 10 }} - //MasterMachineType: n1-standard-4 - //{{ else if gt .TotalNodeCount 5 }} - //MasterMachineType: n1-standard-2 - //{{ else }} - //MasterMachineType: n1-standard-1 - //{{ end }} - // - //{{ if gt TotalNodeCount 500 }} - //MasterMachineType: c4.8xlarge - //{{ else if gt TotalNodeCount 250 }} - //MasterMachineType: c4.4xlarge - //{{ else if gt TotalNodeCount 100 }} - //MasterMachineType: m3.2xlarge - //{{ else if gt TotalNodeCount 10 }} - //MasterMachineType: m3.xlarge - //{{ else if gt TotalNodeCount 5 }} - //MasterMachineType: m3.large - //{{ else }} - //MasterMachineType: m3.medium - //{{ end }} - - switch kops.CloudProviderID(cluster.Spec.CloudProvider) { - case kops.CloudProviderAWS: - region, err := awsup.FindRegion(cluster) + cloud, err := BuildCloud(cluster) if err != nil { - glog.Warningf("cannot determine region from cluster zones: %v", err) + return "", fmt.Errorf("error building cloud for AWS cluster: %v", err) } - // Check for special-cases - masterMachineType := masterMachineTypeExceptions[region] - if masterMachineType != "" { - glog.Warningf("%q instance is not available in region %q, will set master to %q instead", defaultMasterMachineTypeAWS, region, masterMachineType) - return masterMachineType - } - return defaultMasterMachineTypeAWS - case kops.CloudProviderGCE: - return defaultMasterMachineTypeGCE - case kops.CloudProviderVSphere: - return defaultMasterMachineTypeVSphere - default: - glog.V(2).Infof("Cannot set default MachineType for CloudProvider=%q", cluster.Spec.CloudProvider) - return "" - } -} -// defaultBastionMachineType returns the default MachineType for bastions, based on the cloudprovider -func defaultBastionMachineType(cluster *kops.Cluster) string { - switch kops.CloudProviderID(cluster.Spec.CloudProvider) { - case kops.CloudProviderAWS: - return defaultBastionMachineTypeAWS + instanceType, err := cloud.(awsup.AWSCloud).DefaultInstanceType(cluster, ig) + if err != nil { + return "", fmt.Errorf("error finding default machine type: %v", err) + } + return instanceType, nil + case kops.CloudProviderGCE: - return defaultBastionMachineTypeGCE + switch ig.Spec.Role { + case kops.InstanceGroupRoleMaster: + return defaultMasterMachineTypeGCE, nil + + case kops.InstanceGroupRoleNode: + return defaultNodeMachineTypeGCE, nil + + case kops.InstanceGroupRoleBastion: + return defaultBastionMachineTypeGCE, nil + } + case kops.CloudProviderVSphere: - return defaultBastionMachineTypeVSphere - default: - glog.V(2).Infof("Cannot set default MachineType for CloudProvider=%q", cluster.Spec.CloudProvider) - return "" + switch ig.Spec.Role { + case kops.InstanceGroupRoleMaster: + return defaultMasterMachineTypeVSphere, nil + + case kops.InstanceGroupRoleNode: + return defaultNodeMachineTypeVSphere, nil + + case kops.InstanceGroupRoleBastion: + return defaultBastionMachineTypeVSphere, nil + } } + + glog.V(2).Infof("Cannot set default MachineType for CloudProvider=%q, Role=%q", cluster.Spec.CloudProvider, ig.Spec.Role) + return "", nil } // defaultImage returns the default Image, based on the cloudprovider diff --git a/upup/pkg/fi/cloudup/populateinstancegroup_test.go b/upup/pkg/fi/cloudup/populateinstancegroup_test.go index a4af4b286e..1efbed99ad 100644 --- a/upup/pkg/fi/cloudup/populateinstancegroup_test.go +++ b/upup/pkg/fi/cloudup/populateinstancegroup_test.go @@ -61,29 +61,6 @@ func TestPopulateInstanceGroup_Role_Required(t *testing.T) { expectErrorFromPopulateInstanceGroup(t, cluster, g, channel, "Role") } -func Test_defaultMasterMachineType(t *testing.T) { - cluster := buildMinimalCluster() - - tests := map[string]string{ - "us-east-1b": "m3.medium", - "us-east-2b": "c4.large", - "eu-west-1b": "m3.medium", - } - - for zone, expected := range tests { - cluster.Spec.Subnets = []api.ClusterSubnetSpec{ - { - Name: "subnet-" + zone, - Zone: zone, - }, - } - actual := defaultMasterMachineType(cluster) - if actual != expected { - t.Fatalf("zone=%q actual=%q; expected=%q", zone, actual, expected) - } - } -} - func expectErrorFromPopulateInstanceGroup(t *testing.T, cluster *api.Cluster, g *api.InstanceGroup, channel *api.Channel, message string) { _, err := PopulateInstanceGroupSpec(cluster, g, channel) if err == nil {