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

MTV-1433 UI plans by provider #585

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

@RichardHoch RichardHoch changed the title UI plans by provider MTV-1433 UI plans by provider Nov 24, 2024
@RichardHoch
Copy link
Collaborator Author

@sgratch @mnecas Please review this PR.

Copy link
Contributor

@fabiand fabiand left a comment

Choose a reason for hiding this comment

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

Textual changes lgtm.

For the reordering a preview with all pending PRs would be appreciated.

Copy link
Contributor

@sgratch sgratch left a comment

Choose a reason for hiding this comment

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

@RichardHoch Few comments

* {virt}

You can add a source provider by using the {ocp} web console.
* Remote {virt} clusters
Copy link
Contributor

@sgratch sgratch Jan 20, 2025

Choose a reason for hiding this comment

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

AFAIK, migration can also be from a local ocp provider (to a remote one).
So a source OCP provider can be either local or remote.
@mnecas @fabiand please confirm


ifdef::vmware[]

. Check the following items related to VMware. All are optional except for *Warm migration*.
Copy link
Contributor

Choose a reason for hiding this comment

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

Following comments are relevant for all providers:

  1. "All are optional except for.." - WDYM by a non optional settings? It's a bit confusing since all options, including this setting, have a default value. So if not set by the user, a default cold value is used. A non optional setting is usually something that the user have to set for proceeding and include a red small asterisk next to it, e.g. plan name

  2. "Transfer Network" and "Target namepsace" settings are not detailed here. Is it in purpose?
    Screenshot from 2025-01-20 14-30-44

  3. Creating a plan via the plan wizard can be done also by going to Provider -> VMs tab, selecting VM(s) => and clicking the "Create migration plan" button. Is it mentioned anywhere?

@RichardHoch
Copy link
Collaborator Author

@sgratch I made the changes we talked about. Please review the PR again. Thanks.


[WARNING]
====
Do not include virtual machines with guest initiated storage connections, such as Internet Small Computer Systems Interface (iSCSI) connections or Network File System (NFS) mounts. These require either additional planning before a migration or reconfiguration after the migration
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Do not include virtual machines with guest initiated storage connections, such as Internet Small Computer Systems Interface (iSCSI) connections or Network File System (NFS) mounts. These require either additional planning before a migration or reconfiguration after the migration
Do not include virtual machines with guest-initiated storage connections, such as Internet Small Computer Systems Interface (iSCSI) connections or Network File System (NFS) mounts. These require either additional planning before migration or reconfiguration after the migration.

> To add either a *Network map* or a *Storage map*, click the *+* sign anf add a mapping.
. Click *Create migration plan*.
+
{project-short} validates the migration plan and the *Plan details* page opens, indicating whether the plan is ready for use or contains an error.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
{project-short} validates the migration plan and the *Plan details* page opens, indicating whether the plan is ready for use or contains an error.
{project-short} validates the migration plan, and the *Plan details* page opens, indicating whether the plan is ready for use or contains an error.

Conjunction joining two independent clauses means you need to use a comma


ifdef::vmware[]

* *Warm migration*: By default, all migrations are cold migrations. For a warm migration, click the *Edit* icon, and select *Warm migration*.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* *Warm migration*: By default, all migrations are cold migrations. For a warm migration, click the *Edit* icon, and select *Warm migration*.
* *Warm migration*: By default, all migrations are cold migrations. For a warm migration, click the *Edit* icon and select *Warm migration*.

Not two independent clauses, so you do not need to use a comma

ifdef::vmware[]

* *Warm migration*: By default, all migrations are cold migrations. For a warm migration, click the *Edit* icon, and select *Warm migration*.
* *Transfer Network*: The network used to transfer the VMs to {virt}. By default... To edit the transfer network, click the *Edit* icon and...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* *Transfer Network*: The network used to transfer the VMs to {virt}. By default... To edit the transfer network, click the *Edit* icon and...
* *Transfer Network*: The network used to transfer the VMs to {virt}. By default... To edit the transfer network, click the *Edit* icon and...

Please clarify here. I am unsure as to your usage of the ellipses
Should they be ... or should they be something else? I would certainly add a space between them and the preceding word.


* *Warm migration*: By default, all migrations are cold migrations. For a warm migration, click the *Edit* icon, and select *Warm migration*.
* *Transfer Network*: The network used to transfer the VMs to {virt}. By default... To edit the transfer network, click the *Edit* icon and...
* *Target namespace*: Destination namespace, By default, this is the {virt} cluster you chose as your destination provider. To edit the namespace, click the *Edit* icon and...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* *Target namespace*: Destination namespace, By default, this is the {virt} cluster you chose as your destination provider. To edit the namespace, click the *Edit* icon and...
* *Target namespace*: Destination namespace, by default, this is the {virt} cluster you chose as your destination provider. To edit the namespace, click the *Edit* icon and...

Same as previous comment and the ellipses


* *Specifying a root device*: Applies to multi-boot VM migrations only. By default, {project-short} uses the first bootable device detected as the root device.
+
To specify a different root device, in the *Settings* section, click the *Edit* icon next to *Root device* and choose a device from the list of commonly-used options, or enter a device in the text box.
Copy link
Collaborator

Choose a reason for hiding this comment

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

it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.

https://www.merriam-webster.com/grammar/6-common-hypercorrections-and-how-to-avoid-them


* *Warm migration* By default, all migrations are cold migrations. For a warm migration, click the *Edit* icon, and select *Warm migration*.
* *Transfer Network*: The network used to transfer the VMs to {virt}. By default... To edit the transfer network, click the *Edit* icon and...
* *Target namespace*: Destination namespace, By default, this is the {virt} cluster you chose as your destination provider. To edit the namespace, click the *Edit* icon and...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* *Target namespace*: Destination namespace, By default, this is the {virt} cluster you chose as your destination provider. To edit the namespace, click the *Edit* icon and...
* *Target namespace*: Destination namespace, by default, this is the {virt} cluster you chose as your destination provider. To edit the namespace, click the *Edit* icon and...

Again, clarify the ellipses

endif::[]

ifdef::ostack,ova,cnv[]
* *Transfer Network*: The network used to transfer the VMs to {virt}. By default... To edit the transfer network, click the *Edit* icon and...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Clarify ellipses

ifdef::rhv[]

* *Warm migration* By default, all migrations are cold migrations. For a warm migration, click the *Edit* icon, and select *Warm migration*.
* *Transfer Network*: The network used to transfer the VMs to {virt}. By default... To edit the transfer network, click the *Edit* icon and...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Clarify ellipses

ifdef::ostack,ova,cnv[]
* *Transfer Network*: The network used to transfer the VMs to {virt}. By default... To edit the transfer network, click the *Edit* icon and...

* *Target namespace*: Destination namespace, By default, this is the {virt} cluster you chose as your destination provider. To edit the namespace, click the *Edit* icon and...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* *Target namespace*: Destination namespace, By default, this is the {virt} cluster you chose as your destination provider. To edit the namespace, click the *Edit* icon and...
* *Target namespace*: Destination namespace, by default, this is the {virt} cluster you chose as your destination provider. To edit the namespace, click the *Edit* icon and...

Clarify ellipses

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants