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.

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

Update machine_types.go to support T3 family #5681

Merged
merged 1 commit into from Aug 23, 2018
Merged

Update machine_types.go to support T3 family #5681

merged 1 commit into from Aug 23, 2018

Conversation

wanghanlin
Copy link
Contributor

@wanghanlin wanghanlin commented Aug 22, 2018

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 22, 2018
@wanghanlin
Copy link
Contributor Author

/assign @KashifSaadat

@mikesplain
Copy link
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 22, 2018
@wanghanlin wanghanlin changed the title Update machine_types.go Update machine_types.go to support T3 family Aug 23, 2018
@wingyplus wingyplus mentioned this pull request Aug 23, 2018
@mikesplain
Copy link
Contributor

Thanks @wanghanlin!

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 23, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mikesplain, wanghanlin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 23, 2018
@k8s-ci-robot k8s-ci-robot merged commit 3a598a7 into kubernetes:master Aug 23, 2018
@Globegitter
Copy link
Contributor

I think it would be really amazing if there could be a cherry-pick for kops 1.10.0 / a kops 1.10.1 release - it seems like a small thing but I think a lot of people would be happy to get the t2 unlimited functionality as well as the money savings.

@mitpwd
Copy link

mitpwd commented Sep 8, 2018

KOPS should not hardcode any AWS instance types in the first place.

@justinsb
Copy link
Member

You're right @mitpwd except that there isn't a convenient API for instance types. I think @mikesplain and @sethpollack are actually working on putting a file somewhere where all of kubernetes-on-aws components could consume it (i.e. creating an API for instance type attributes).

@dllz
Copy link

dllz commented Oct 14, 2018

Any eta on when this is being released?

@tomaszkiewicz
Copy link

You're right @mitpwd except that there isn't a convenient API for instance types. I think @mikesplain and @sethpollack are actually working on putting a file somewhere where all of kubernetes-on-aws components could consume it (i.e. creating an API for instance type attributes).

I've searched aws-sdk-go for new instance types and it looks like there's a file with API model:

https://github.com/aws/aws-sdk-go/blob/master/models/apis/ec2/2016-11-15/api-2.json

The file seems to be used to generate API with the generator included in the repo:

https://github.com/aws/aws-sdk-go/blob/f26268b77ea9619d95b1d04b6f3e3c5a03b61110/private/model/cli/gen-api/main.go

And the file is updated frequently, so probably it's very up-to-date. When you use JSON path .shapes.InstanceType.enum you can extract all possible instance types :)

I don't know how kops is up-to-date with dependency on aws-sdk-go but maybe it's a good point to start a discussion :)

Best regards

Łukasz Tomaszkiewicz

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support of new T3 family
9 participants