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

fix(config): adding some missing instances block device config #4087

Merged

Conversation

marchello2000
Copy link
Contributor

No description provided.

@marchello2000
Copy link
Contributor Author

@abelzberg please let me know if you have feedback

@marchello2000
Copy link
Contributor Author

Added offline feedback from @abelzberg

Copy link
Contributor

@cfieber cfieber left a comment

Choose a reason for hiding this comment

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

I think some of the values were off based on the instance-type pages

@@ -99,6 +99,11 @@ class BlockDeviceConfig {
"i3.16xlarge" : enumeratedBlockDevicesWithVirtualName(8),
"i3.metal" : enumeratedBlockDevicesWithVirtualName(8),

"i3en.large" : enumeratedBlockDevicesWithVirtualName(1),
"i3en.xlarge" : enumeratedBlockDevicesWithVirtualName(1),
"i3en.2xlarge": enumeratedBlockDevicesWithVirtualName(1),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"i3en.2xlarge": enumeratedBlockDevicesWithVirtualName(1),
"i3en.2xlarge": enumeratedBlockDevicesWithVirtualName(2),

Copy link
Contributor

Choose a reason for hiding this comment

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

I think, based on this table: https://aws.amazon.com/ec2/instance-types/i3en/

Copy link
Contributor

Choose a reason for hiding this comment

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

worth adding the rest of the i3en types from that page while you are in here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

"m5d.12xlarge" : enumeratedBlockDevicesWithVirtualName(2),
"m5d.16xlarge" : enumeratedBlockDevicesWithVirtualName(2),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"m5d.16xlarge" : enumeratedBlockDevicesWithVirtualName(2),
"m5d.16xlarge" : enumeratedBlockDevicesWithVirtualName(4),

https://aws.amazon.com/ec2/instance-types/m5/

"r5d.12xlarge" : enumeratedBlockDevicesWithVirtualName(2),
"r5d.16xlarge" : enumeratedBlockDevicesWithVirtualName(2),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"r5d.16xlarge" : enumeratedBlockDevicesWithVirtualName(2),
"r5d.16xlarge" : enumeratedBlockDevicesWithVirtualName(4),

https://aws.amazon.com/ec2/instance-types/r5/

"m5.12xlarge" : sizedBlockDevicesForEbs(120),
"m5.16xlarge" : sizedBlockDevicesForEbs(120),
"m5.24xlarge" : sizedBlockDevicesForEbs(120),

"m5d.large" : enumeratedBlockDevicesWithVirtualName(1),
"m5d.xlarge" : enumeratedBlockDevicesWithVirtualName(1),
"m5d.2xlarge" : enumeratedBlockDevicesWithVirtualName(1),
"m5d.4xlarge" : enumeratedBlockDevicesWithVirtualName(1),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"m5d.4xlarge" : enumeratedBlockDevicesWithVirtualName(1),
"m5d.4xlarge" : enumeratedBlockDevicesWithVirtualName(2),

@marchello2000 marchello2000 merged commit cde0faf into spinnaker:master Oct 14, 2019
@marchello2000 marchello2000 deleted the mark/block_device_defaults branch October 14, 2019 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants