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

CNV#33978-1: ROSA support and general storage update #67365

Merged
merged 1 commit into from
Nov 6, 2023

Conversation

ousleyp
Copy link
Member

@ousleyp ousleyp commented Nov 3, 2023

Version(s): 4.14+

Issue: CNV-33978

Link to docs preview: https://67365--docspreview.netlify.app/openshift-enterprise/latest/virt/install/preparing-cluster-for-virt#virt-aws-bm_preparing-cluster-for-virt

QE review:

  • QE has approved this change.
  • seeking additional approval from @orenc1 before merge

Additional information:

@ousleyp ousleyp added this to the Continuous Release milestone Nov 3, 2023
@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 3, 2023
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Nov 3, 2023

🤖 Updated build preview is available at:
https://67365--docspreview.netlify.app

Build log: https://circleci.com/gh/ocpdocs-previewbot/openshift-docs/31830

@ousleyp ousleyp requested a review from orenc1 November 3, 2023 15:23
@dbasunag
Copy link

dbasunag commented Nov 3, 2023

/lgtm

@ousleyp ousleyp added the peer-review-needed Signifies that the peer review team needs to review this PR label Nov 3, 2023
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 3, 2023
@cbippley
Copy link
Contributor

cbippley commented Nov 3, 2023

/label peer-review-in-progress
/remove-label peer-review-needed

@openshift-ci openshift-ci bot added peer-review-in-progress Signifies that the peer review team is reviewing this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Nov 3, 2023
Copy link
Contributor

@cbippley cbippley left a comment

Choose a reason for hiding this comment

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

LGTM!

@cbippley
Copy link
Contributor

cbippley commented Nov 3, 2023

/label peer-review-done
/remove-label peer-review-in-progress

@openshift-ci openshift-ci bot added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-in-progress Signifies that the peer review team is reviewing this PR labels Nov 3, 2023
@orenc1
Copy link

orenc1 commented Nov 5, 2023

lgtm in general, but I would like to comment about:

  • You can install the cluster by using installer-provisioned infrastructure, ensuring that you specify bare-metal node instance types in the install-config.yaml file. For example, you can use the c5n.metal type value for a machine based on x86_64 architecture.

I think we can mention that at least the worker nodes should be bare metal instances (e.g. c5n.metal), since they are running the VirtualMachines. At least two of them in order to support live migration.
There is no hard requirements for the master/control-plane nodes to be also bare metals.
This distinction has a considerable influence on the running costs of the AWS/ROSA cluster. Metal instance is 10x more expensive than a virtual EC2 that can serve as a master node.

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 6, 2023
Copy link

openshift-ci bot commented Nov 6, 2023

New changes are detected. LGTM label has been removed.

@ousleyp
Copy link
Member Author

ousleyp commented Nov 6, 2023

lgtm in general, but I would like to comment about:

  • You can install the cluster by using installer-provisioned infrastructure, ensuring that you specify bare-metal node instance types in the install-config.yaml file. For example, you can use the c5n.metal type value for a machine based on x86_64 architecture.

I think we can mention that at least the worker nodes should be bare metal instances (e.g. c5n.metal), since they are running the VirtualMachines. At least two of them in order to support live migration. There is no hard requirements for the master/control-plane nodes to be also bare metals. This distinction has a considerable influence on the running costs of the AWS/ROSA cluster. Metal instance is 10x more expensive than a virtual EC2 that can serve as a master node.

Changed to:

* You can install the cluster by using installer-provisioned infrastructure, ensuring that you specify bare-metal instance types for the worker nodes by editing the `install-config.yaml` file. For example, you can use the `c5n.metal` type value for a machine based on x86_64 architecture.

@ousleyp ousleyp merged commit 718b75c into openshift:main Nov 6, 2023
1 check passed
@ousleyp
Copy link
Member Author

ousleyp commented Nov 6, 2023

/cherrypick enterprise-4.15

@ousleyp
Copy link
Member Author

ousleyp commented Nov 6, 2023

/cherrypick enterprise-4.14

@openshift-cherrypick-robot

@ousleyp: new pull request created: #67422

In response to this:

/cherrypick enterprise-4.15

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.

@openshift-cherrypick-robot

@ousleyp: new pull request created: #67423

In response to this:

/cherrypick enterprise-4.14

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.14 branch/enterprise-4.15 CNV Label for all CNV PRs peer-review-done Signifies that the peer review team has reviewed this PR 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.

None yet

6 participants