Skip to content

Conversation

bgaydosrh
Copy link

@bgaydosrh bgaydosrh commented Mar 10, 2021

Label for enterprise-4.5, 4.6, 4.7, 4.8

Added this bullet to the prereqs listed in the Configuring your cluster for OpenShift Virtualization assembly:

Before you migrate virtual machines to a cluster, ensure that all nodes in a single pool use the same type of CPU. For example, place nodes that use AMD processors in one pool and place nodes that use Intel processors in a different pool. Do not mix different CPU types in the same pool.

Jira: https://issues.redhat.com/browse/CNV-10445
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1926199
Test Build: https://deploy-preview-30340--osdocs.netlify.app/openshift-enterprise/latest/virt/install/preparing-cluster-for-virt.html

Tagging @kobi86 for Code Review
Tagging Israel Pinto for QE Review in Jira record.

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 10, 2021
@netlify
Copy link

netlify bot commented Mar 10, 2021

Deploy preview for osdocs ready!

Built with commit 0990a18

https://deploy-preview-30340--osdocs.netlify.app

@ILpinto
Copy link

ILpinto commented Mar 11, 2021

/lgtm

@openshift-ci-robot
Copy link

@ILpinto: changing LGTM is restricted to collaborators

In response to this:

/lgtm

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.

Copy link

@kobi86 kobi86 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 that we should change the sentence to :
To be able to migrate virtual machines into the cluster, ensure that all nodes in a single pool use the same type of CPU. For example, nodes in one pool can use AMD processors and nodes in another pool can use Intel processors, and so on. Avoid mixing different CPU types in the same pool.

@bgaydosrh
Copy link
Author

I think that we should change the sentence to :
To be able to migrate virtual machines into the cluster, ensure that all nodes in a single pool use the same type of CPU. For example, nodes in one pool can use AMD processors and nodes in another pool can use Intel processors, and so on. Avoid mixing different CPU types in the same pool.

Thanks for the suggestion. I'll see what the peer reviewers prefer in terms using "into the" vs. "to a" --- our style guidelines are pretty specific in cases like this.

@bgaydosrh bgaydosrh force-pushed the CNV-10445 branch 2 times, most recently from 5d73438 to 5d7fca7 Compare March 11, 2021 16:03
Copy link
Member

@ousleyp ousleyp left a comment

Choose a reason for hiding this comment

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

I've suggested a rewrite for clarity. Please let me know if you have any questions. Thanks!

Which versions does this apply to?

@ousleyp ousleyp added CNV Label for all CNV PRs peer-review-done Signifies that the peer review team has reviewed this PR labels Mar 11, 2021
@bgaydosrh bgaydosrh force-pushed the CNV-10445 branch 2 times, most recently from 2dc1f74 to 3a8516d Compare March 15, 2021 15:09
@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 18, 2021
@bgaydosrh bgaydosrh force-pushed the CNV-10445 branch 2 times, most recently from 4496d8f to 3beff99 Compare April 1, 2021 05:21
@bgaydosrh bgaydosrh force-pushed the CNV-10445 branch 2 times, most recently from 05bcf5d to 1273f0d Compare April 2, 2021 03:24
@bgaydosrh bgaydosrh force-pushed the CNV-10445 branch 2 times, most recently from cbf98bd to a034d34 Compare April 8, 2021 20:44
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 11, 2021
@kobi86
Copy link

kobi86 commented Apr 13, 2021

@bgaydosrh, I think we are ok to move on , aren't we?

@dankenigsberg
Copy link
Contributor

dankenigsberg commented Apr 13, 2021

@bgaydosrh, I think we are ok to move on , aren't we?

yes, I dropped my "hold". If the text is good, you can /lgtm it, but I would like to see the "CPU pool" term dropped from this PR title.

@bgaydosrh
Copy link
Author

@bgaydosrh, I think we are ok to move on , aren't we?

yes, I dropped my "hold". If the text is good, you can /lgtm it, but I would like to see the "CPU pool" term dropped from this PR title.

Moving this to Peer Review for final check and merge.

@bgaydosrh bgaydosrh changed the title CNV-10445 [1926199] VM Migration to mixed CPU pool (AMD,Intel) CNV-10445 [1926199] VM Migration of mixed CPU (AMD,Intel) Apr 15, 2021
Copy link
Contributor

@sjhala-ccs sjhala-ccs left a comment

Choose a reason for hiding this comment

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

One nit, otherwise LGTM

@sjhala-ccs sjhala-ccs merged commit 0eb4da3 into openshift:master Apr 15, 2021
@sjhala-ccs
Copy link
Contributor

/cherrypick enterprise-4.8

@sjhala-ccs
Copy link
Contributor

/cherrypick enterprise-4.7

@openshift-cherrypick-robot

@sjhala-ccs: new pull request created: #31624

In response to this:

/cherrypick enterprise-4.8

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

@sjhala-ccs: new pull request created: #31625

In response to this:

/cherrypick enterprise-4.7

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.

@sjhala-ccs
Copy link
Contributor

/cherrypick enterprise-4.6

@openshift-cherrypick-robot

@sjhala-ccs: new pull request created: #31626

In response to this:

/cherrypick enterprise-4.6

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.

@sjhala-ccs
Copy link
Contributor

/cherrypick enterprise-4.5

@openshift-cherrypick-robot

@sjhala-ccs: new pull request created: #31627

In response to this:

/cherrypick enterprise-4.5

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.

@kobi86 kobi86 mentioned this pull request May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.5 branch/enterprise-4.6 branch/enterprise-4.7 branch/enterprise-4.8 CNV Label for all CNV PRs peer-review-done Signifies that the peer review team has reviewed this PR size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants