Skip to content

OSDOCS#39419: Fixing procedure for creating live migration policies #78982

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

Merged
merged 1 commit into from
Aug 1, 2024

Conversation

@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 17, 2024
@dshchedr
Copy link

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 30, 2024
@jherrman
Copy link
Contributor Author

/label peer-review-needed

@openshift-ci openshift-ci bot added the peer-review-needed Signifies that the peer review team needs to review this PR label Jul 31, 2024
@dfitzmau
Copy link
Contributor

/remove-label peer-review-needed

/label peer-review-in-progress

@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 Jul 31, 2024
Copy link
Contributor

@dfitzmau dfitzmau left a comment

Choose a reason for hiding this comment

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

Hi @jherrman . Please squash the commits. I see 5 commits lists on the Commits tab.

/remove-label peer-review-in-progress

/label peer-review-done

@@ -6,7 +6,7 @@
[id="virt-configuring-a-live-migration-policy_{context}"]
= Creating a live migration policy by using the command line

You can create a live migration policy by using the command line. A live migration policy is applied to selected virtual machines (VMs) by using any combination of labels:
You can create a live migration policy by using the command line. KubeVirt applies live migration policy to selected virtual machines (VMs) by using any combination of labels:
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
You can create a live migration policy by using the command line. KubeVirt applies live migration policy to selected virtual machines (VMs) by using any combination of labels:
You can create a live migration policy by using the command line. KubeVirt applies the live migration policy to selected virtual machines (VMs) by using any combination of labels:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, will fix!

labels:
kubevirt.io/domain: <vm-name>
kubevirt.io/size: large
kubevirt.io/environment: production
Copy link
Contributor

Choose a reason for hiding this comment

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

This might not be a complete YAML example, so consider ending the YAML with:

...

This syntax aligns with the following guideline:

https://github.com/openshift/openshift-docs/blob/main/contributing_to_docs/doc_guidelines.adoc#yaml-formatting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, it's not complete, will indicate as much in the codeblock

template:
metadata:
labels:
kubevirt.io/domain: <vm-name>
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
kubevirt.io/domain: <vm-name>
kubevirt.io/domain: <vm_name>

Copy link
Contributor

Choose a reason for hiding this comment

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

In two locations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -36,8 +68,7 @@ spec:
hpc-workloads: "True"
xyz-workloads-type: ""
virtualMachineInstanceSelector: <2>
workload-type: "db"
operating-system: ""
"kubevirt.io/environment": "production"
Copy link
Contributor

Choose a reason for hiding this comment

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

Parentheses required here? I do not see them used in the other example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIK it should work either way, but you're right that for consistency, it'd be better if they weren't there.

@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 Jul 31, 2024
@jherrman
Copy link
Contributor Author

Hi @jherrman . Please squash the commits. I see 5 commits lists on the Commits tab.

Thanks for the quick review Darragh! I was planning to squash the commits before submitting for merge review (since it seems rather counter-productive at any point before that).

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 31, 2024
Copy link

openshift-ci bot commented Jul 31, 2024

New changes are detected. LGTM label has been removed.

Copy link

openshift-ci bot commented Jul 31, 2024

@jherrman: 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.

@jherrman
Copy link
Contributor Author

jherrman commented Aug 1, 2024

/label merge-review-needed

@openshift-ci openshift-ci bot added the merge-review-needed Signifies that the merge review team needs to review this PR label Aug 1, 2024
@kcarmichael08 kcarmichael08 added merge-review-in-progress Signifies that the merge review team is reviewing this PR branch/enterprise-4.15 branch/enterprise-4.16 branch/enterprise-4.17 and removed merge-review-needed Signifies that the merge review team needs to review this PR labels Aug 1, 2024
@kcarmichael08 kcarmichael08 added this to the Continuous Release milestone Aug 1, 2024
@kcarmichael08 kcarmichael08 merged commit 3e6ac62 into openshift:main Aug 1, 2024
2 checks passed
@kcarmichael08 kcarmichael08 added the CNV Label for all CNV PRs label Aug 1, 2024
@kcarmichael08
Copy link
Contributor

/cherrypick enterprise-4.15

@openshift-cherrypick-robot

@kcarmichael08: new pull request created: #79854

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

@kcarmichael08
Copy link
Contributor

/cherrypick enterprise-4.16

@kcarmichael08
Copy link
Contributor

/cherrypick enterprise-4.17

@openshift-cherrypick-robot

@kcarmichael08: new pull request created: #79856

In response to this:

/cherrypick enterprise-4.16

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.

@openshift-cherrypick-robot

@kcarmichael08: new pull request created: #79857

In response to this:

/cherrypick enterprise-4.17

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.15 branch/enterprise-4.16 branch/enterprise-4.17 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.

6 participants