CA: Update AWS instance type list #2550
Conversation
k8s-ci-robot
added
cncf-cla: yes
Mostly important due to existing Memory figures for some C5d instances being incorrect
gjtempleton
force-pushed
the
CA-AWS-Instance-List-Update
branch
from
15606c7 to
6bceab7
Compare
|
/assign @Jeffwan |
| @@ -223,7 +223,7 @@ var InstanceTypes = map[string]*InstanceType{ | |||
| "c5d.12xlarge": { | |||
| InstanceType: "c5d.12xlarge", | |||
| VCPU: 48, | |||
| MemoryMb: 131072, | |||
There was a problem hiding this comment.
Em..Interesting. I do use the generator to populate the specs. Do you change manually or use the gen.go?
There was a problem hiding this comment.
Yeah, I was confused as well when I noticed it, even weirder I changed it by running make generate again, so I can only assume the API was returning different results when you last generated it.
There was a problem hiding this comment.
Yeah. The new values you give match with values in aws docs. Great!
| "u-18tb1": { | ||
| InstanceType: "u-18tb1", | ||
| VCPU: 448, | ||
| MemoryMb: 0, |
There was a problem hiding this comment.
Any reason it get 0 MemoryMb?
There was a problem hiding this comment.
Ah, I failed to notice that! I think I'll need to dig into this and figure out why that's happening.
There was a problem hiding this comment.
Finally got around to spending more time on digging into this, and it looks like the Pricing API we're currently using can't be fully trusted. For one thing the correct API designation of these instances is u-18tb1.metal, not u-18tb1 as returned by the pricing API.
I'm going to raise a separate issue to discuss whether/how we move away from the Pricing API as it seems it can't be fully trusted to build a proper picture of these nodes, in the meantime any problem with merging this as is? (I've updated it with a couple more changes generated when running today and double checked them against the info returned by instance descriptions using a boto3 client.)
There was a problem hiding this comment.
Em.. u-18tb1.metal is the right name according to release note. I don't have concern merging this since this instances are available from dedicated hosts with 3 yrs reservation. Most users may not even have access to it. Feel free to open a new issue and let's resolve the pricing api issue there. Some other projects uses nodes template as well like CNI, I will also have a check on reliable API.
k8s-ci-robot
added
size/L
gjtempleton
force-pushed
the
CA-AWS-Instance-List-Update
branch
from
d5cddf7 to
9211366
Compare
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Jeffwan 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 |
k8s-ci-robot
added
the
approved
Mostly important due to existing Memory figures for some C5d instances being incorrect.
Despite the ability for the CA to dynamically generate this list added by #2240 users can still use this list and the memory figures for some of the C5d family currently are incorrect. These new generated figures match the published figures: https://aws.amazon.com/ec2/instance-types/c5/
This also pulls in the figures for the new super high memory instances: https://aws.amazon.com/blogs/aws/ec2-high-memory-update-new-18-tb-and-24-tb-instances/