Skip to content

Commit

Permalink
Merge pull request #10885 from hakman/automated-cherry-pick-of-#10747-#…
Browse files Browse the repository at this point in the history 
…10884-upstream-release-1.20

Automated cherry pick of #10747: Add validation for ami arch to instance type arch #10884: Improve machine type and image validation
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Feb 19, 2021
2 parents 0301976 + a811bcc commit 3f49145
Show file tree
Hide file tree
Showing 5 changed files with 158 additions and 11 deletions.
3 changes: 3 additions & 0 deletions pkg/apis/kops/validation/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,13 @@ go_test(
],
embed = [":go_default_library"],
deps = [
"//cloudmock/aws/mockec2:go_default_library",
"//pkg/apis/kops:go_default_library",
"//pkg/nodeidentity/aws:go_default_library",
"//upup/pkg/fi:go_default_library",
"//upup/pkg/fi/cloudup/awsup:go_default_library",
"//vendor/github.com/aws/aws-sdk-go/aws:go_default_library",
"//vendor/github.com/aws/aws-sdk-go/service/ec2:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/intstr:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
Expand Down
38 changes: 30 additions & 8 deletions pkg/apis/kops/validation/aws.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func awsValidateInstanceGroup(ig *kops.InstanceGroup, cloud awsup.AWSCloud) fiel

allErrs = append(allErrs, awsValidateAdditionalSecurityGroups(field.NewPath("spec", "additionalSecurityGroups"), ig.Spec.AdditionalSecurityGroups)...)

allErrs = append(allErrs, awsValidateInstanceType(field.NewPath(ig.GetName(), "spec", "machineType"), ig.Spec.MachineType, cloud)...)
allErrs = append(allErrs, awsValidateInstanceTypeAndImage(field.NewPath(ig.GetName(), "spec", "machineType"), field.NewPath(ig.GetName(), "spec", "image"), ig.Spec.MachineType, ig.Spec.Image, cloud)...)

allErrs = append(allErrs, awsValidateSpotDurationInMinute(field.NewPath(ig.GetName(), "spec", "spotDurationInMinutes"), ig)...)

Expand Down Expand Up @@ -121,14 +121,36 @@ func awsValidateAdditionalSecurityGroups(fieldPath *field.Path, groups []string)
return allErrs
}

func awsValidateInstanceType(fieldPath *field.Path, instanceType string, cloud awsup.AWSCloud) field.ErrorList {
func awsValidateInstanceTypeAndImage(instanceTypeFieldPath *field.Path, imageFieldPath *field.Path, instanceType string, image string, cloud awsup.AWSCloud) field.ErrorList {
allErrs := field.ErrorList{}
if instanceType != "" && cloud != nil {
for _, typ := range strings.Split(instanceType, ",") {
if _, err := cloud.DescribeInstanceType(typ); err != nil {
allErrs = append(allErrs, field.Invalid(fieldPath, typ, "machine type specified is invalid"))

if cloud != nil && instanceType != "" {
imageInfo, err := cloud.ResolveImage(image)
if err != nil {
return append(allErrs, field.Invalid(imageFieldPath, image,
fmt.Sprintf("image specified is invalid: %q", image)))
}

imageArch := fi.StringValue(imageInfo.Architecture)

machineInfo, err := cloud.DescribeInstanceType(instanceType)
if err != nil {
return append(allErrs, field.Invalid(instanceTypeFieldPath, instanceType,
fmt.Sprintf("machine type specified is invalid: %q", instanceType)))
}

found := false
if machineInfo.ProcessorInfo != nil {
for _, machineArch := range machineInfo.ProcessorInfo.SupportedArchitectures {
if imageArch == fi.StringValue(machineArch) {
found = true
}
}
}
if !found {
allErrs = append(allErrs, field.Invalid(instanceTypeFieldPath, instanceType,
fmt.Sprintf("machine type architecture does not match image architecture: %+v - %q", machineInfo.ProcessorInfo, imageArch)))
}
}

return allErrs
Expand Down Expand Up @@ -158,8 +180,8 @@ func awsValidateMixedInstancesPolicy(path *field.Path, spec *kops.MixedInstances
var errs field.ErrorList

// @step: check the instance types are valid
for i, x := range spec.Instances {
errs = append(errs, awsValidateInstanceType(path.Child("instances").Index(i), x, cloud)...)
for i, instanceType := range spec.Instances {
errs = append(errs, awsValidateInstanceTypeAndImage(path.Child("instances").Index(i), path.Child("image"), instanceType, ig.Spec.Image, cloud)...)
}

if spec.OnDemandBase != nil {
Expand Down
109 changes: 106 additions & 3 deletions pkg/apis/kops/validation/aws_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ package validation
import (
"testing"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"k8s.io/kops/cloudmock/aws/mockec2"

"k8s.io/kops/upup/pkg/fi"
"k8s.io/kops/upup/pkg/fi/cloudup/awsup"

Expand Down Expand Up @@ -102,28 +106,39 @@ func TestValidateInstanceGroupSpec(t *testing.T) {
{
Input: kops.InstanceGroupSpec{
MachineType: "t2.micro",
Image: "ami-073c8c0760395aab8",
},
},
{
Input: kops.InstanceGroupSpec{
MachineType: "t2.invalidType",
Image: "ami-073c8c0760395aab8",
},
ExpectedErrors: []string{"Invalid value::test-nodes.spec.machineType"},
},
{
Input: kops.InstanceGroupSpec{
MachineType: "m5.large",
Image: "k8s-1.9-debian-stretch-amd64-hvm-ebs-2018-03-11",
MachineType: "m4.large",
Image: "ami-073c8c0760395aab8",
},
ExpectedErrors: []string{},
},
{
Input: kops.InstanceGroupSpec{
MachineType: "c5.large",
Image: "k8s-1.9-debian-stretch-amd64-hvm-ebs-2018-03-11",
Image: "ami-073c8c0760395aab8",
},
ExpectedErrors: []string{},
},
{
Input: kops.InstanceGroupSpec{
MachineType: "a1.large",
Image: "ami-073c8c0760395aab8",
},
ExpectedErrors: []string{
"Invalid value::test-nodes.spec.machineType",
},
},
{
Input: kops.InstanceGroupSpec{
SpotDurationInMinutes: fi.Int64(55),
Expand Down Expand Up @@ -182,6 +197,18 @@ func TestValidateInstanceGroupSpec(t *testing.T) {
},
}
cloud := awsup.BuildMockAWSCloud("us-east-1", "abc")
mockEC2 := &mockec2.MockEC2{}
cloud.MockEC2 = mockEC2

mockEC2.Images = append(mockEC2.Images, &ec2.Image{
CreationDate: aws.String("2016-10-21T20:07:19.000Z"),
ImageId: aws.String("ami-073c8c0760395aab8"),
Name: aws.String("focal"),
OwnerId: aws.String(awsup.WellKnownAccountUbuntu),
RootDeviceName: aws.String("/dev/xvda"),
Architecture: aws.String("x86_64"),
})

for _, g := range grid {
ig := &kops.InstanceGroup{
ObjectMeta: v1.ObjectMeta{
Expand All @@ -195,6 +222,82 @@ func TestValidateInstanceGroupSpec(t *testing.T) {
}
}

func TestMixedInstancePolicies(t *testing.T) {
grid := []struct {
Input kops.InstanceGroupSpec
ExpectedErrors []string
}{
{
Input: kops.InstanceGroupSpec{
MachineType: "m4.large",
Image: "ami-073c8c0760395aab8",
MixedInstancesPolicy: &kops.MixedInstancesPolicySpec{
Instances: []string{
"m4.large",
"t3.medium",
"c5.large",
},
},
},
ExpectedErrors: nil,
},
{
Input: kops.InstanceGroupSpec{
MachineType: "m4.large",
Image: "ami-073c8c0760395aab8",
MixedInstancesPolicy: &kops.MixedInstancesPolicySpec{
Instances: []string{
"a1.large",
"c4.large",
"c5.large",
},
},
},
ExpectedErrors: []string{"Invalid value::spec.mixedInstancesPolicy.instances[0]"},
},
{
Input: kops.InstanceGroupSpec{
MachineType: "m4.large",
Image: "ami-073c8c0760395aab8",
MixedInstancesPolicy: &kops.MixedInstancesPolicySpec{
Instances: []string{
"t3.medium",
"c4.large",
"c5.large",
},
OnDemandAboveBase: fi.Int64(231),
},
},
ExpectedErrors: []string{"Invalid value::spec.mixedInstancesPolicy.onDemandAboveBase"},
},
}
cloud := awsup.BuildMockAWSCloud("us-east-1", "abc")
mockEC2 := &mockec2.MockEC2{}
cloud.MockEC2 = mockEC2

mockEC2.Images = append(mockEC2.Images, &ec2.Image{
CreationDate: aws.String("2016-10-21T20:07:19.000Z"),
ImageId: aws.String("ami-073c8c0760395aab8"),
Name: aws.String("focal"),
OwnerId: aws.String(awsup.WellKnownAccountUbuntu),
RootDeviceName: aws.String("/dev/xvda"),
Architecture: aws.String("x86_64"),
})

for _, g := range grid {
ig := &kops.InstanceGroup{
ObjectMeta: v1.ObjectMeta{
Name: "test-nodes",
},
Spec: g.Input,
}
errs := awsValidateInstanceGroup(ig, cloud)

testErrors(t, g.Input, errs, g.ExpectedErrors)
}

}

func TestInstanceMetadataOptions(t *testing.T) {
cloud := awsup.BuildMockAWSCloud("us-east-1", "abc")

Expand Down
3 changes: 3 additions & 0 deletions pkg/testutils/integrationtestharness.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ func (h *IntegrationTestHarness) SetupMockAWS() *awsup.MockAWSCloud {
Name: aws.String("k8s-1.4-debian-jessie-amd64-hvm-ebs-2016-10-21"),
OwnerId: aws.String(awsup.WellKnownAccountKopeio),
RootDeviceName: aws.String("/dev/xvda"),
Architecture: aws.String("x86_64"),
})

mockEC2.Images = append(mockEC2.Images, &ec2.Image{
Expand All @@ -181,6 +182,7 @@ func (h *IntegrationTestHarness) SetupMockAWS() *awsup.MockAWSCloud {
Name: aws.String("k8s-1.5-debian-jessie-amd64-hvm-ebs-2017-01-09"),
OwnerId: aws.String(awsup.WellKnownAccountKopeio),
RootDeviceName: aws.String("/dev/xvda"),
Architecture: aws.String("x86_64"),
})

mockEC2.Images = append(mockEC2.Images, &ec2.Image{
Expand All @@ -189,6 +191,7 @@ func (h *IntegrationTestHarness) SetupMockAWS() *awsup.MockAWSCloud {
Name: aws.String("k8s-1.14-debian-stretch-amd64-hvm-ebs-2019-08-16"),
OwnerId: aws.String(awsup.WellKnownAccountKopeio),
RootDeviceName: aws.String("/dev/xvda"),
Architecture: aws.String("x86_64"),
})

mockEC2.CreateVpcWithId(&ec2.CreateVpcInput{
Expand Down
16 changes: 16 additions & 0 deletions upup/pkg/fi/cloudup/awsup/mock_aws_cloud.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,22 @@ 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":
info.ProcessorInfo = &ec2.ProcessorInfo{
SupportedArchitectures: []*string{
aws.String("x86_64"),
},
}
case "a1.large":
info.ProcessorInfo = &ec2.ProcessorInfo{
SupportedArchitectures: []*string{
aws.String("arm64"),
},
}
}

return info, nil
}

Expand Down

0 comments on commit 3f49145

Please sign in to comment.