machines: Set defaults for machine instance types by rna-afk · Pull Request #5841 · openshift/installer

rna-afk · 2022-04-25T17:28:29Z

Setting all compute nodes to use the instance types that provide
4 VPUs and 16 GB RAM for AWS, GCP and VSphere platforms.

patrickdillon

Nice. So it looks like you identified aws, gcp, nutanix, and vsphere. gcp I think is close enough so let's leave it alone.

For vsphere and nutanix let's get owner opinions: @thunderboltsid, @rvanderp3, @jcpowermac. I suspect resources might be tighter on these platforms.

For context, it has been pointed out to us that aws has fewer resources on compute nodes that gcp/azure. While we're fixing aws we're also checking other platforms.

barbacbd · 2022-05-02T12:45:43Z

@rna-afk I saw that the aws default instance type was set to gp3 but would other "generic" or general purpose instances make sense? I don't know if there are other conditions that need to be met.

rna-afk · 2022-05-02T14:14:26Z

We need a lot of throughput for all our operations on the cluster. The general guideline is to provide a premium disk type for the control-plane nodes and any for the compute nodes but we set the default to premium for better performance. I think gp3 for the root volume type of the VMs is fine.

thunderboltsid · 2022-05-02T14:30:32Z

This implies 16 CPU units (CPUs x Cores per CPU). Is that what we want? I actually have an open PR that was reducing these even further.. For 4 CPU units, I'd recommend setting NumCoresPerSocket to 1.

I just moved the code from [1] and the minimum requirement is 2 and 2 but the vsphere folks set it to 4 and 4 as part of the performance recommendation for vsphere. I'll remove the nutanix change though to let you change it to the ideal value, thanks!

Ah I think I understand where I made a mistake. I'll remove those changes then. Thanks!

@rna-afk I can also take care of bumping up the RAM to 16GB in my PR.

jcpowermac · 2022-05-02T18:13:03Z

For vsphere and nutanix let's get owner opinions: @thunderboltsid, @rvanderp3, @jcpowermac. I suspect resources might be tighter on these platforms.

In CI we have to up to this value anyway to pass jobs. So lgtm to me. We should update documentation right?

patrickdillon · 2022-05-02T20:24:10Z

We should update documentation right?

Yeah this is a good point. This may need more docs than the release note I originally mentioned. Although when I'm looking at the docs:
AWS
[GCP]https://docs.openshift.com/container-platform/4.9/installing/installing_gcp/installing-gcp-account.html#installation-gcp-limits_installing-gcp-account
Hm.. not finding the equivalent for vSphere?

It looks like AWS/GCP don't mention memory in those sections, so it doesn't look like they need updates. Not sure if anywhere else would need to be updated for AWS/GCP. @rna-afk can you take a quick look?

patrickdillon · 2022-05-02T20:24:34Z

/approve

Updating aws, gcp, & vsphere looks good to me.

openshift-ci · 2022-05-02T20:25:00Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: patrickdillon

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [patrickdillon]

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

rna-afk · 2022-05-02T20:50:05Z

Almost all docs need changing specifically in the compute section where the minimum requirements are 2 VCPUs and 8 GB RAM.
AWS
Azure

rna-afk · 2022-05-02T21:02:48Z

Almost all docs need changing specifically in the compute section where the minimum requirements are 2 VCPUs and 8 GB RAM.

This actually depends on if we want to increase our minimum requirements spec. Other than this I don't see any other docs change.

mjpytlak · 2022-05-03T12:51:16Z

@patrickdillon @rna-afk Please let @bscott-rh and I know if doc updates beyond the release note are required and to which platforms. Based on comments, not sure which platforms this PR will ultimately land on.

sadasu · 2022-05-03T16:49:11Z

@rna-afk it appears that we are changing the CPU and memory defaults just for AWS, GCP and vSphere. If that is the case, could you please update the commit message to reflect that (and not all platforms)? Also, #5841 (comment) needs to updated to reflect that too. I'll defer to the other reviewers with more knowledge of the platforms to decide if the defaults are the right ones.

patrickdillon · 2022-05-03T21:05:23Z

Almost all docs need changing specifically in the compute section where the minimum requirements are 2 VCPUs and 8 GB RAM.

This actually depends on if we want to increase our minimum requirements spec. Other than this I don't see any other docs change.

No, this wouldn't change minimum requirements.

patrickdillon · 2022-05-05T00:04:04Z

/lgtm

@rna-afk would you add a note in the jira card for the docs team that says what the new default instance type is for each platform updated in this pr is?

patrickdillon · 2022-05-05T00:06:06Z

/hold
/lgtm cancel

patrickdillon · 2022-05-05T00:07:03Z

Don't we need to bump CPUs too?
/cc @jcpowermac

patrickdillon · 2022-05-05T00:08:01Z

/hold cancel

patrickdillon · 2022-05-09T01:43:25Z

Above it is 4 NumCoresPerSocket and here it is 1. Which is right?

4 cpus and 1 cores per socket is right. I'll fix it

Changed it to set masters to 4 vcpus and 4 numcores and workers to 4 vcpus and 1 numcores

Setting compute nodes to use the instance types that provide 4 VPUs and 16 GB RAM for AWS, GCP and VSphere.

patrickdillon · 2022-05-10T19:37:09Z

/lgtm

We could update https://github.com/openshift/release/blob/master/ci-operator/step-registry/ipi/conf/vsphere/ipi-conf-vsphere-commands.sh to no longer explicitly set the parameters that are being updated here

openshift-bot · 2022-05-10T20:15:52Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-10T21:19:53Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

rvanderp3 · 2022-05-10T21:24:42Z

/lgtm

We could update https://github.com/openshift/release/blob/master/ci-operator/step-registry/ipi/conf/vsphere/ipi-conf-vsphere-commands.sh to no longer explicitly set the parameters that are being updated here

@patrickdillon sure, good idea. I'll follow up on that.

openshift-bot · 2022-05-10T21:32:52Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-10T22:32:47Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-10T23:23:17Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-10T23:33:48Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-ci · 2022-05-11T01:35:25Z

@rna-afk: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-azure	`16f2827`	link	false	`/test e2e-azure`
ci/prow/e2e-vsphere	`16f2827`	link	false	`/test e2e-vsphere`
ci/prow/e2e-ovirt	`16f2827`	link	false	`/test e2e-ovirt`
ci/prow/okd-e2e-aws	`16f2827`	link	false	`/test okd-e2e-aws`
ci/prow/e2e-metal-assisted	`16f2827`	link	false	`/test e2e-metal-assisted`
ci/prow/e2e-libvirt	`16f2827`	link	false	`/test e2e-libvirt`
ci/prow/e2e-metal-ipi	`16f2827`	link	false	`/test e2e-metal-ipi`
ci/prow/e2e-ibmcloud	`16f2827`	link	false	`/test e2e-ibmcloud`
ci/prow/e2e-gcp	`16f2827`	link	false	`/test e2e-gcp`
ci/prow/e2e-crc	`16f2827`	link	false	`/test e2e-crc`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-bot · 2022-05-11T02:21:48Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-11T02:34:47Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-11T03:26:47Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-05-11T03:39:47Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

Virtual machines running on vSphere should be configured for cores over sockets for NUMA. The change in openshift#5841 missed that. Changing vSphere back to using a single socket and multiple cores.

rvanderp3 · 2022-06-07T21:50:28Z

/lgtm
We could update https://github.com/openshift/release/blob/master/ci-operator/step-registry/ipi/conf/vsphere/ipi-conf-vsphere-commands.sh to no longer explicitly set the parameters that are being updated here

@patrickdillon sure, good idea. I'll follow up on that.

started a PR to address this: openshift/release#29234

openshift-ci Bot requested review from barbacbd and r4f4 April 25, 2022 17:29

patrickdillon reviewed Apr 25, 2022

View reviewed changes

Comment thread pkg/asset/machines/worker.go Outdated

rna-afk force-pushed the fix_instance_types branch 2 times, most recently from 500e9d3 to cf9e527 Compare April 25, 2022 22:20

thunderboltsid reviewed May 2, 2022

View reviewed changes

thunderboltsid mentioned this pull request May 2, 2022

Change the default machine pool config for Nutanix #5857

Merged

rna-afk force-pushed the fix_instance_types branch 3 times, most recently from 2be3979 to 70c6afe Compare May 2, 2022 18:07

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 2, 2022

rna-afk force-pushed the fix_instance_types branch from 70c6afe to 40dbc3c Compare May 3, 2022 16:56

barbacbd mentioned this pull request May 3, 2022

libvirt: bump memory of machines to 16G in the provider config #5230

Closed

openshift-ci Bot assigned patrickdillon May 5, 2022

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label May 5, 2022

openshift-ci Bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed lgtm Indicates that a PR is ready to be merged. labels May 5, 2022

patrickdillon reviewed May 5, 2022

View reviewed changes

openshift-ci Bot requested a review from jcpowermac May 5, 2022 00:07

openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 5, 2022

rna-afk force-pushed the fix_instance_types branch from 40dbc3c to daeb04b Compare May 5, 2022 13:18

patrickdillon reviewed May 9, 2022

View reviewed changes

machines: Set defaults for machine instance types

16f2827

Setting compute nodes to use the instance types that provide 4 VPUs and 16 GB RAM for AWS, GCP and VSphere.

rna-afk force-pushed the fix_instance_types branch from daeb04b to 16f2827 Compare May 9, 2022 14:55

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label May 10, 2022

openshift-merge-robot merged commit 8d15c81 into openshift:master May 11, 2022

jcpowermac mentioned this pull request May 16, 2022

Bug 2086936: vSphere: change socket back to cores #5914

Merged

jinyunma mentioned this pull request May 26, 2022

OSDOCS-3638: Release note for AWS/vSphere default compute node resources openshift/openshift-docs#45819

Merged

Conversation

rna-afk commented Apr 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

patrickdillon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

barbacbd commented May 2, 2022

Uh oh!

rna-afk commented May 2, 2022

Uh oh!

thunderboltsid May 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rna-afk May 2, 2022

Choose a reason for hiding this comment

Uh oh!

rna-afk May 2, 2022

Choose a reason for hiding this comment

Uh oh!

thunderboltsid May 2, 2022

Choose a reason for hiding this comment

Uh oh!

jcpowermac commented May 2, 2022

Uh oh!

patrickdillon commented May 2, 2022

Uh oh!

patrickdillon commented May 2, 2022

Uh oh!

openshift-ci Bot commented May 2, 2022

Uh oh!

rna-afk commented May 2, 2022

Uh oh!

rna-afk commented May 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mjpytlak commented May 3, 2022

Uh oh!

sadasu commented May 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

patrickdillon commented May 3, 2022

Uh oh!

patrickdillon commented May 5, 2022

Uh oh!

patrickdillon commented May 5, 2022

Uh oh!

patrickdillon May 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patrickdillon commented May 5, 2022

Uh oh!

patrickdillon May 9, 2022

Choose a reason for hiding this comment

Uh oh!

rna-afk May 9, 2022

Choose a reason for hiding this comment

Uh oh!

rna-afk May 9, 2022

Choose a reason for hiding this comment

Uh oh!

patrickdillon commented May 10, 2022

Uh oh!

openshift-bot commented May 10, 2022

Uh oh!

openshift-bot commented May 10, 2022

Uh oh!

rvanderp3 commented May 10, 2022

Uh oh!

openshift-bot commented May 10, 2022

Uh oh!

openshift-bot commented May 10, 2022

Uh oh!

openshift-bot commented May 10, 2022

Uh oh!

openshift-bot commented May 10, 2022

Uh oh!

rna-afk commented Apr 25, 2022 •

edited

Loading

thunderboltsid May 2, 2022 •

edited

Loading

rna-afk commented May 2, 2022 •

edited

Loading

sadasu commented May 3, 2022 •

edited

Loading

patrickdillon May 5, 2022 •

edited

Loading

openshift-ci Bot commented May 11, 2022 •

edited

Loading