Skip to content

Conversation

@danielclowers
Copy link
Contributor

@danielclowers danielclowers commented Sep 10, 2025

@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 10, 2025
@ousleyp ousleyp added CNV Label for all CNV PRs branch/enterprise-4.20 labels Oct 8, 2025
@ousleyp ousleyp added this to the Planned for 4.20 GA milestone Oct 8, 2025
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.

Good work overall, but I have a couple questions and some nits/suggestions. Let me know if you have questions about my comments.

@openshift-ci openshift-ci bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 10, 2025
@danielclowers
Copy link
Contributor Author

@ousleyp i applied all your suggestions. and just reworded a few things. also swapped steps 2 and 3. can you take a look again?

also, do you think i should leave this or get rid of it

<1> As of {VirtProductName} 4.20, the `KubeVirtRelieveAndMigrate` profile replaces the `DevKubeVirtRelieveAndMigrate` profile.  

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.

Thanks for making the changes! I think it's looking good but just a few more comments re: the callouts. Feel free to ping me for merge when you resolve those.

@tiraboschi
Copy link
Contributor

I'd also add another note.
In section https://docs.redhat.com/en/documentation/openshift_container_platform/4.19/html/virtualization/managing-vms#virt-enabling-descheduler-evictions
we are explaining ho to enable PSI on the worker nodes and in the example snippet we are correctly naming the machine config object as 99-openshift-machineconfig-worker-psi-karg but I'd also add an explicit note mentioning that the name of the machine config object is relevant. This because the machine config objects are processed in lexicographical order and by default a machine config starting with 98-... is disabling PSI so, in order to really enable it, the machine config should be named 99-....

Copy link
Contributor

@tiraboschi tiraboschi left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-ci
Copy link

openshift-ci bot commented Oct 15, 2025

@danielclowers: all tests passed!

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

@ousleyp
Copy link
Member

ousleyp commented Oct 16, 2025

Excellent work, lgtm :)

@ousleyp ousleyp merged commit e19553a into openshift:main Oct 16, 2025
2 checks passed
@ousleyp
Copy link
Member

ousleyp commented Oct 16, 2025

/cherrypick enterprise-4.20

@openshift-cherrypick-robot

@ousleyp: new pull request created: #100681

In response to this:

/cherrypick enterprise-4.20

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-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.20 CNV Label for all CNV PRs size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants