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

Use ec2.DescribeInstanceTypes for machine type info #8856

Merged
merged 8 commits into from
Jun 9, 2020
Merged

Use ec2.DescribeInstanceTypes for machine type info #8856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
01b0fc5
Remove hack/machine_types script
rifelpet Jun 3, 2020
64ef8c2
Add DescribeInstanceTypes to AWSCloud
rifelpet Jun 3, 2020
cebb708
Move MixedInstancesPolicy validation to aws.go
rifelpet Jun 3, 2020
bc074e8
Use ec2.DescribeInstanceTypes in awsup.GetMachineTypeInfo
rifelpet Jun 3, 2020
3739859
Add a release note documenting the change in behavior
rifelpet Jun 8, 2020
807cbcb
fix new validation unit test
rifelpet Jun 8, 2020
95b6a3f
Address feedback
rifelpet Jun 9, 2020
d923354
Address review comments
rifelpet Jun 9, 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
7 changes: 0 additions & 7 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -829,13 +829,6 @@ build-docs-netlify:
pip install -r ${KOPS_ROOT}/images/mkdocs/requirements.txt
mkdocs build

# Update machine_types.go
.PHONY: update-machine-types
update-machine-types:
go build -o hack/machine_types/machine_types ${KOPS_ROOT}/hack/machine_types/
hack/machine_types/machine_types --out upup/pkg/fi/cloudup/awsup/machine_types.go
go fmt upup/pkg/fi/cloudup/awsup/machine_types.go

#-----------------------------------------------------------
# development targets

Expand Down
5 changes: 5 additions & 0 deletions cloudmock/aws/mockec2/instances.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,8 @@ func (m *MockEC2) DescribeInstancesPages(request *ec2.DescribeInstancesInput, ca
func (m *MockEC2) DescribeInstancesPagesWithContext(aws.Context, *ec2.DescribeInstancesInput, func(*ec2.DescribeInstancesOutput, bool) bool, ...request.Option) error {
panic("Not implemented")
}

func (m *MockEC2) DescribeInstanceTypes(*ec2.DescribeInstanceTypesInput) (*ec2.DescribeInstanceTypesOutput, error) {
klog.Warningf("MockEc2::DescribeInstanceTypes is stub-implemented")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it helps, we can try to narrow the instance types we support in testing, or just support fake instance types.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function isn't actually used, it just needs to be defined in order to satisfy the mock interface. The actual stub we use in integration testing is in upup/pkg/fi/cloudup/awsup/mock_aws_cloud.go where I defined a reasonable mock behavior.

return &ec2.DescribeInstanceTypesOutput{}, nil
}
4 changes: 2 additions & 2 deletions cmd/kops/create_cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1231,7 +1231,7 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr
}

strict := false
err = validation.DeepValidate(cluster, instanceGroups, strict)
err = validation.DeepValidate(cluster, instanceGroups, strict, nil)
if err != nil {
return err
}
Expand All @@ -1255,7 +1255,7 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr
fullInstanceGroups = append(fullInstanceGroups, fullGroup)
}

err = validation.DeepValidate(fullCluster, fullInstanceGroups, true)
err = validation.DeepValidate(fullCluster, fullInstanceGroups, true, nil)
if err != nil {
return err
}
Expand Down
7 changes: 6 additions & 1 deletion cmd/kops/create_ig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,12 @@ func RunCreateInstanceGroup(ctx context.Context, f *util.Factory, cmd *cobra.Com
return fmt.Errorf("unexpected object type: %T", obj)
}

err = validation.CrossValidateInstanceGroup(group, cluster).ToAggregate()
cloud, err := cloudup.BuildCloud(cluster)
if err != nil {
return err
}

err = validation.CrossValidateInstanceGroup(group, cluster, cloud).ToAggregate()
if err != nil {
return err
}
Expand Down
7 changes: 6 additions & 1 deletion cmd/kops/edit_cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,12 @@ func RunEditCluster(ctx context.Context, f *util.Factory, cmd *cobra.Command, ar
continue
}

err = validation.DeepValidate(fullCluster, instanceGroups, true)
cloud, err := cloudup.BuildCloud(fullCluster)
if err != nil {
return err
}

err = validation.DeepValidate(fullCluster, instanceGroups, true, cloud)
if err != nil {
results = editResults{
file: file,
Expand Down
7 changes: 6 additions & 1 deletion cmd/kops/edit_instancegroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,12 @@ func RunEditInstanceGroup(ctx context.Context, f *util.Factory, cmd *cobra.Comma
return err
}

err = validation.CrossValidateInstanceGroup(fullGroup, fullCluster).ToAggregate()
cloud, err := cloudup.BuildCloud(fullCluster)
if err != nil {
return err
}

err = validation.CrossValidateInstanceGroup(fullGroup, fullCluster, cloud).ToAggregate()
if err != nil {
return err
}
Expand Down
2 changes: 2 additions & 0 deletions docs/releases/1.19-NOTES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

# Significant changes

* Clusters using the Amazon VPC CNI provider now perform an `ec2.DescribeInstanceTypes` call at instance launch time. In large clusters or AWS accounts this may lead to API throttling which could delay node readiness. If this becomes a problem please open a GitHub issue.

# Breaking changes

* Support for Kubernetes 1.9 and 1.10 has been removed.
Expand Down
1 change: 0 additions & 1 deletion hack/.packages
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ k8s.io/kops/dnsprovider/pkg/dnsprovider/providers/openstack/designate
k8s.io/kops/dnsprovider/pkg/dnsprovider/rrstype
k8s.io/kops/dnsprovider/pkg/dnsprovider/tests
k8s.io/kops/examples/kops-api-example
k8s.io/kops/hack/machine_types
k8s.io/kops/kube-discovery/cmd/kube-discovery
k8s.io/kops/node-authorizer/cmd/node-authorizer
k8s.io/kops/node-authorizer/pkg/authorizers/alwaysallow
Expand Down
1 change: 0 additions & 1 deletion hack/machine_types/.gitignore
View file
Open in desktop

This file was deleted.

21 changes: 0 additions & 21 deletions hack/machine_types/BUILD.bazel
View file
Open in desktop

This file was deleted.

19 changes: 0 additions & 19 deletions hack/machine_types/README.md
View file
Open in desktop

This file was deleted.

Loading