Skip to content

Conversation

stoobie
Copy link
Contributor

@stoobie stoobie commented Feb 9, 2022

This PR addresses https://issues.redhat.com/browse/MIG-1017.

This applies to:
enterprise-4.10

Preview:
https://deploy-preview-41620--osdocs.netlify.app/openshift-enterprise/latest/migrating_from_ocp_3_to_4/advanced-migration-options-3-4.html#migration-migrating-applications-on-prem-to-cloud_advanced-migration-options-3-4

Added a new module:
modules/migration-migrating-on-prem-to-cloud.adoc

Minor fix in existing module (sentence was incomplete)

@openshift-ci openshift-ci bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 9, 2022
@stoobie
Copy link
Contributor Author

stoobie commented Feb 9, 2022

@jmontleon Please review for accuracy and completeness, and there are several questions that need answers.

Also, please make sure that I correctly fixed the incomplete sentence in modules/migration-adding-cluster-to-cam.adoc.

@netlify
Copy link

netlify bot commented Feb 9, 2022

Deploy Preview for osdocs ready!

Name Link
🔨 Latest commit b040dea
🔍 Latest deploy log https://app.netlify.com/sites/osdocs/deploys/623cb78f676855000886bed7
😎 Deploy Preview https://deploy-preview-41620--osdocs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Contributor

Choose a reason for hiding this comment

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

For the UI, you create a MigCluster CR by adding a cluster. Here's the documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I added links in the parent assembly under .Additional resources, following this module.

Copy link
Contributor

@apinnick apinnick Feb 13, 2022

Choose a reason for hiding this comment

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

Looks like you get it by running $ oc config view. See docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks.

Copy link
Contributor

@apinnick apinnick Feb 13, 2022

Choose a reason for hiding this comment

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

The logs for the OpenVPN pods.

oc get po is short for oc get pods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

= token of the migration controller service account.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Copy link
Contributor

Choose a reason for hiding this comment

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

A list of pods in the namespace, with their status, restarts, and age,

Copy link
Contributor

Choose a reason for hiding this comment

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

The video demo shows the result. Source cluster at 2:12.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I added that, but I'm not sure how to refer to the string following openvpn. I referred to is <log-hash>. @jmontleon Can you weigh in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Jason posted the command to install it in one of the comments on your Jira.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this as a step in the procedure.

@stoobie stoobie force-pushed the MIG-1017 branch 3 times, most recently from 5fdf51a to 69bcc86 Compare February 17, 2022 10:35
Copy link
Contributor

Choose a reason for hiding this comment

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

$ oc logs -f <openvpn> -n <namespace> -c openvpn

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@stoobie stoobie force-pushed the MIG-1017 branch 6 times, most recently from 5fffee3 to d09fc74 Compare February 17, 2022 16:42
@stoobie stoobie force-pushed the MIG-1017 branch 2 times, most recently from f81d37a to a67157d Compare March 7, 2022 17:12
Copy link

Choose a reason for hiding this comment

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

Why is Jason's name mentioned here ? is it relevant to the customers ?

Choose a reason for hiding this comment

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

Thanks for catching this @midays - the entire bit in parenthesis should be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Copy link

Choose a reason for hiding this comment

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

at the end of the url it's also requried to specify the port as follows:
https://proxied-cluster.${namespace}.svc.cluster.local:8443

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@stoobie
Copy link
Contributor Author

stoobie commented Mar 9, 2022

@midays Please review my changes.

Choose a reason for hiding this comment

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

The source cluster needs to be added to the mtc console, not the destination.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Choose a reason for hiding this comment

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

Cluster name: Source cluster name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@stoobie stoobie force-pushed the MIG-1017 branch 2 times, most recently from e272611 to e998a20 Compare March 10, 2022 17:31
@michaelryanpeter michaelryanpeter added peer-review-needed Signifies that the peer review team needs to review this PR branch/enterprise-4.10 branch/enterprise-4.11 labels Mar 11, 2022
@michaelryanpeter michaelryanpeter added this to the Next Release milestone Mar 11, 2022
Copy link
Contributor

@michaelryanpeter michaelryanpeter left a comment

Choose a reason for hiding this comment

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

Please let me know if you have questions. I tried to provide references to the style rules where I could find them.

Copy link
Contributor

Choose a reason for hiding this comment

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

Additional resources are limited to an unordered list of links or xrefs. (See the peer review checklist

Also, this is duplicated at the end of the modules/migration-migrating-on-prem-to-cloud.adoc file.

I think that this would work better at the end of the procedure module as a [TIP] admonition. This is how I have most commonly seen these examples presented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added it as a tip close to where I first mentioned the command.

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
* For information on creating a MigCluster CR manifest for each remote cluster, see
* For information about creating a MigCluster CR manifest for each remote cluster, see

Re: CCS IBM style guide quick reference

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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
.Additional resources
[role="_additional-resources"]
.Additional resources

Re: Jupiter guidelines from the OSDOCS peer review checklist

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

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
* For information on adding a cluster using the web console, see xref:../migrating_from_ocp_3_to_4/migrating-applications-3-4.adoc#migrating-applications-mtc-web-console_migrating-applications-3-4[Migrating your applications by using the {mtc-short} web console]
* For information about adding a cluster using the web console, see xref:../migrating_from_ocp_3_to_4/migrating-applications-3-4.adoc#migrating-applications-mtc-web-console_migrating-applications-3-4[Migrating your applications by using the {mtc-short} web console]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove the hard wrap/line break here. It shows up inline in the preview, but we don't want to have the line break in the .adoc.

Re: repo guidelines

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

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
$ oc get service -n _<namespace>_
$ oc get service -n <namespace>

Copy link
Contributor Author

@stoobie stoobie Mar 15, 2022

Choose a reason for hiding this comment

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

FIxed.

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
. On the source node, get the SA token for the migration controller:
. On the source node, get the service account (SA) token for the migration controller:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is duplicated in the assembly file. Please see that note for a comment on replacing the example --help command as a [TIP] admonition in this procedure module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

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
. Launch the {mtc-short} web console and add the source cluster, using the following values:
. Open the {mtc-short} web console and add the source cluster, using the following values:

ISG recommends using start or open instead of launch unless it is part of the product terminology.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

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
For example:
.Example command

@michaelryanpeter michaelryanpeter added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Mar 11, 2022
@stoobie
Copy link
Contributor Author

stoobie commented Mar 16, 2022

@michaelryanpeter Please do not merge this until the MTC 1.7 release. I don't believe there is any way to automatically indicate this, so I or someone else on the team will give the go-ahead.

@stoobie
Copy link
Contributor Author

stoobie commented Mar 24, 2022

@michaelryanpeter Please merge. MTC 1.7 is now live.

Comment on lines 83 to 95
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
.Example output
[source,terminal]
----
NAME READY STATUS RESTARTS AGE
<pod_name> 2/2 Running 0 44s
[source,terminal,subs="+quotes"]
----
# oc logs -f -n <namespace> <pod_name> -c openvpn
----
When the address of the load balancer is resolved, the message `Initialization Sequence Completed` appears at the end of the log.
====
.Example output
[source,terminal]
----
NAME READY STATUS RESTARTS AGE
<pod_name> 2/2 Running 0 44s
----
[source,terminal,subs="+quotes"]
----
# oc logs -f -n <namespace> <pod_name> -c openvpn
----
When the address of the load balancer is resolved, the message `Initialization Sequence Completed` appears at the end of the log.
====

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@michaelryanpeter michaelryanpeter merged commit 1502b7c into openshift:main Mar 24, 2022
@michaelryanpeter
Copy link
Contributor

/cherrypick enterprise-4.10

@michaelryanpeter
Copy link
Contributor

/cherrypick enterprise-4.11

@openshift-cherrypick-robot

@michaelryanpeter: new pull request created: #43793

In response to this:

/cherrypick enterprise-4.10

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

@michaelryanpeter: new pull request created: #43794

In response to this:

/cherrypick enterprise-4.11

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.

@stoobie stoobie deleted the MIG-1017 branch March 31, 2022 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.10 branch/enterprise-4.11 peer-review-done Signifies that the peer review team has reviewed this PR 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.

7 participants