From bdd63c917fcc39ee9546b197194d5851c9e85f96 Mon Sep 17 00:00:00 2001 From: Ciprian Hacman Date: Wed, 12 May 2021 11:13:06 +0300 Subject: [PATCH] Allow AWS instance types with multiple architectures Older AWS instance types support both "i386" and "x86_64" architectures: ``` $ aws ec2 describe-instance-types --instance-types t2.micro { "InstanceTypes": [ "InstanceType": "t2.micro", "ProcessorInfo": { "SupportedArchitectures": [ "i386", "x86_64" ], ``` --- upup/pkg/fi/cloudup/BUILD.bazel | 4 +- upup/pkg/fi/cloudup/awsup/mock_aws_cloud.go | 13 ++++-- ..._test.go => populate_cluster_spec_test.go} | 0 .../fi/cloudup/populate_instancegroup_spec.go | 20 +++++---- ...go => populate_instancegroup_spec_test.go} | 45 +++++++++++++++++++ upup/pkg/fi/cloudup/scratchpad | 13 ------ 6 files changed, 69 insertions(+), 26 deletions(-) rename upup/pkg/fi/cloudup/{populatecluster_test.go => populate_cluster_spec_test.go} (100%) rename upup/pkg/fi/cloudup/{populateinstancegroup_test.go => populate_instancegroup_spec_test.go} (66%) delete mode 100644 upup/pkg/fi/cloudup/scratchpad diff --git a/upup/pkg/fi/cloudup/BUILD.bazel b/upup/pkg/fi/cloudup/BUILD.bazel index 845b6a8d97..727ecf8455 100644 --- a/upup/pkg/fi/cloudup/BUILD.bazel +++ b/upup/pkg/fi/cloudup/BUILD.bazel @@ -102,8 +102,8 @@ go_test( "docker_test.go", "networking_test.go", "new_cluster_test.go", - "populatecluster_test.go", - "populateinstancegroup_test.go", + "populate_cluster_spec_test.go", + "populate_instancegroup_spec_test.go", "subnets_test.go", "template_functions_test.go", "urls_test.go", diff --git a/upup/pkg/fi/cloudup/awsup/mock_aws_cloud.go b/upup/pkg/fi/cloudup/awsup/mock_aws_cloud.go index 6e6f0a4ea7..b8fff89126 100644 --- a/upup/pkg/fi/cloudup/awsup/mock_aws_cloud.go +++ b/upup/pkg/fi/cloudup/awsup/mock_aws_cloud.go @@ -327,16 +327,23 @@ func (c *MockAWSCloud) DescribeInstanceType(instanceType string) (*ec2.InstanceT } switch instanceType { - case "c5.large", "m3.medium", "m4.large", "m5.large", "m5.xlarge", "t2.micro", "t2.medium", "t3.medium", "t3.large", "c4.large": + case "c5.large", "m3.medium", "m4.large", "m5.large", "m5.xlarge", "t3.micro", "t3.medium", "t3.large", "c4.large": info.ProcessorInfo = &ec2.ProcessorInfo{ SupportedArchitectures: []*string{ - aws.String("x86_64"), + aws.String(ec2.ArchitectureTypeX8664), }, } case "a1.large": info.ProcessorInfo = &ec2.ProcessorInfo{ SupportedArchitectures: []*string{ - aws.String("arm64"), + aws.String(ec2.ArchitectureTypeArm64), + }, + } + case "t2.micro", "t2.medium": + info.ProcessorInfo = &ec2.ProcessorInfo{ + SupportedArchitectures: []*string{ + aws.String(ec2.ArchitectureTypeI386), + aws.String(ec2.ArchitectureTypeX8664), }, } } diff --git a/upup/pkg/fi/cloudup/populatecluster_test.go b/upup/pkg/fi/cloudup/populate_cluster_spec_test.go similarity index 100% rename from upup/pkg/fi/cloudup/populatecluster_test.go rename to upup/pkg/fi/cloudup/populate_cluster_spec_test.go diff --git a/upup/pkg/fi/cloudup/populate_instancegroup_spec.go b/upup/pkg/fi/cloudup/populate_instancegroup_spec.go index e3f5361472..91cc91f902 100644 --- a/upup/pkg/fi/cloudup/populate_instancegroup_spec.go +++ b/upup/pkg/fi/cloudup/populate_instancegroup_spec.go @@ -272,15 +272,19 @@ func MachineArchitecture(cloud fi.Cloud, machineType string) (architectures.Arch if info.ProcessorInfo == nil || len(info.ProcessorInfo.SupportedArchitectures) == 0 { return "", fmt.Errorf("error finding architecture info for instance type %q", machineType) } - arch := fi.StringValue(info.ProcessorInfo.SupportedArchitectures[0]) - switch arch { - case ec2.ArchitectureTypeX8664: - return architectures.ArchitectureAmd64, nil - case ec2.ArchitectureTypeArm64: - return architectures.ArchitectureArm64, nil - default: - return "", fmt.Errorf("unsupported architecture for instance type %q: %s", machineType, arch) + var unsupported []string + for _, arch := range info.ProcessorInfo.SupportedArchitectures { + // Return the first found supported architecture, in order of popularity + switch fi.StringValue(arch) { + case ec2.ArchitectureTypeX8664: + return architectures.ArchitectureAmd64, nil + case ec2.ArchitectureTypeArm64: + return architectures.ArchitectureArm64, nil + default: + unsupported = append(unsupported, fi.StringValue(arch)) + } } + return "", fmt.Errorf("unsupported architecture for instance type %q: %v", machineType, unsupported) default: // No other clouds are known to support any other architectures at this time return architectures.ArchitectureAmd64, nil diff --git a/upup/pkg/fi/cloudup/populateinstancegroup_test.go b/upup/pkg/fi/cloudup/populate_instancegroup_spec_test.go similarity index 66% rename from upup/pkg/fi/cloudup/populateinstancegroup_test.go rename to upup/pkg/fi/cloudup/populate_instancegroup_spec_test.go index 1cdd78fafa..5b1b1c261c 100644 --- a/upup/pkg/fi/cloudup/populateinstancegroup_test.go +++ b/upup/pkg/fi/cloudup/populate_instancegroup_spec_test.go @@ -22,6 +22,7 @@ import ( "testing" kopsapi "k8s.io/kops/pkg/apis/kops" + "k8s.io/kops/util/pkg/architectures" ) func buildMinimalNodeInstanceGroup(subnets ...string) *kopsapi.InstanceGroup { @@ -77,3 +78,47 @@ func expectErrorFromPopulateInstanceGroup(t *testing.T, cluster *kopsapi.Cluster t.Fatalf("Expected error %q, got %q", message, actualMessage) } } + +func TestMachineArchitecture(t *testing.T) { + tests := []struct { + machineType string + arch architectures.Architecture + err error + }{ + { + machineType: "t2.micro", + arch: architectures.ArchitectureAmd64, + err: nil, + }, + { + machineType: "t3.micro", + arch: architectures.ArchitectureAmd64, + err: nil, + }, + { + machineType: "a1.large", + arch: architectures.ArchitectureArm64, + err: nil, + }, + } + for _, test := range tests { + t.Run(fmt.Sprintf("%s-%s", test.machineType, test.arch), func(t *testing.T) { + _, cluster := buildMinimalCluster() + cloud, err := BuildCloud(cluster) + if err != nil { + t.Fatalf("error from BuildCloud: %v", err) + } + + arch, err := MachineArchitecture(cloud, test.machineType) + if err != test.err { + t.Errorf("actual error %q differs from expected error %q", err, test.err) + return + } + + if arch != test.arch { + t.Errorf("actual architecture %q differs from expected architecture %q", arch, test.arch) + return + } + }) + } +} diff --git a/upup/pkg/fi/cloudup/scratchpad b/upup/pkg/fi/cloudup/scratchpad deleted file mode 100644 index 0db8674bf9..0000000000 --- a/upup/pkg/fi/cloudup/scratchpad +++ /dev/null @@ -1,13 +0,0 @@ -> kops create cluster mixed.awsdata.com --zones us-east-1c --topology private --networking weave - -> kops edit cluster mixed.awsdata.com - -# Changed nodes to public here: - -``` - topology: - dns: - type: Public - masters: private - nodes: public -```