openshift-docs
openshift-docs copied to clipboard
openshift
OADP-566 Document data mover workflow
OADP-566 Document data mover workflow
Resolves: https://issues.redhat.com/browse/OADP-566
OADP 1.1.0
Preview:
@sbeskin-redhat few additional points:
- is it mentioned anywhere else that the DataMover is a tech-preview feature? if not, we should mention it somewhere
- please also mention that currently GCS bucket is not supported for DataMover
- @shubham-pampattiwar, @savitharaghunathan should the docs also address ReplicationSource/ReplicationDestination?
- @shubham-pampattiwar, @savitharaghunathan should the docs also address ReplicationSource/ReplicationDestination?
@mperetzred : IMO since these CRs are created and the lifecycle is managed by the volsnap controller, I dont think we need to mention ReplicationSource/ReplicationDestination CRs in the doc. @shubham-pampattiwar @eemcmullan WDYT?
@sbeskin-redhat A few small comments -- also try to figure out why your Travis build failed. Look at the Checks tab of the PR. If that doesn't help, ask someone on the peer review squad for assistance.
@RichardHoch Thank you! I know why the build failed, I just haven't figured out what to do about it. It's not in my part.
@RichardHoch I've made the corrections you requested. Please take a look now.
@sbeskin-redhat There's one 1.1 that needs to be changed to 1.1.0, aside from that this is ready for OCP peer review. LGTM.
This is not a full review, but a few things jump out at me in an initial pass. I recommend fixing these issues while you investigate your Travis build errors and before you reqeust a review. Please reach out if you have questions.
- Specify the version(s) of OpenShift your PR applies to in the first comment of the PR.
- Line breaks:
- You need a line break between lines 27 and 28
- Delete the extra line here and here
- Add a line break here and here
- Check for other instances of inconsistencies in the mark up in regards to line breaks
- Refer to the repo guidelines on code blocks for terminal commands. In particular:
- start CLI command blocks with a
$ [source,terminal]- Use proper lead-in sentences for commands
- start CLI command blocks with a
- You can use a tech preview snippet for you TP admonition
- Please don't stack admonitions: ISG
Thanks for making the updates. I have added it to my queue, and will give it a full peer review today.
@michaelryanpeter Thank you! There is some urgency to this PR as the OADP 1.1.0 GA is scheduled for tomorrow.
Heard.
Reviews still take time. I assure you I am working on your PR.
Friendly reminder to squash your commits if you want this merged tomorrow
Given the number of known mistakes in the CLI commands, we need QE reapproval before we can move forwrad on the merge.
The build looks good. There are still some typos in the oc get commands, where there is no space between -ojsonpath and no closing quotes to the end.
I will lift the hold once the PR is reapproved by QE. Every instance I could find in the docs repo for oc get ... -o jsonpath= uses single quotes instead of double quotes. Perhaps it does not matter, but with so many typos in commands, it is important to verify.
Please squash your commits.
After GA, please make a follow up PR to fix the user supplied variables that use a dash instead of an underscore.
@michaelryanpeter I've fixed the typos and underscores and squashed the commits.
@mperetzred The OCP reviewer found some errors (typos?) in commands. Please verify the corrections.
@sbeskin-redhat Thanks for asking for a re-approval from QE.
I am removing the hold, since I am not on the peer review squad this week and won't be checking GitHub as regularly. I don't want to block a merge once the PR has been verified.
/unhold
@sbeskin-redhat @michaelryanpeter could you please explain what requires QE re-approval? The thread is too long, and I can't check the changes since my last review since the commits where squashed...
Here are the changes. They are all in commands:
[image: image.png]
[image: image.png]
[image: image.png]
[image: image.png]
On Tue, Sep 13, 2022 at 9:42 AM Maya Peretz @.***> wrote:
@sbeskin-redhat https://github.com/sbeskin-redhat @michaelryanpeter https://github.com/michaelryanpeter could you please explain what requires QE re-approval? The thread is too long, and I can't check the changes since my last review since the commits where squashed...
— Reply to this email directly, view it on GitHub https://github.com/openshift/openshift-docs/pull/48619#issuecomment-1244968083, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATJEI5KTHAA7OGPJXVONFMLV6AO3JANCNFSM55HDXK4A . You are receiving this because you were mentioned.Message ID: @.***>
-- *Sasha Beskin (*סשה בסקין) He/His/Him Technical Writer Red Hat Migration and Modernization Solutions Red Hat http://redhat.com Israel
https://static.redhat.com/libs/redhat/brand-assets/2/corp/logo--200.png https://static.redhat.com/libs/redhat/brand-assets/2/corp/logo--200.png @.***
@sbeskin-redhat I don't see what are these images. Anyway, it seems to me from @michaelryanpeter's comments that the changes are semantics/format/typos changes; these kind of issues don't require QE verification and should be as part of peer review.
The typos were in the CLI commands, so they were not about semantics or formatting. That is why QE verification was requested.
For example, $ oc get route <route_name> -n <app_ns> -ojsonpath="{.spec.host} was used several times in the doc. I made a comment on what I suspect the command should be: $ oc get route <route_name> -n <app_ns> -o jsonpath="{.spec.host}". Since I am not certain that is the correct command (examples in other parts of the documentation use single quotes instead of double quotes) it seemed prudent to request re-verification.
@kalexand-rh: Looping you in for awareness.
@mperetzred: If you would please verify that $ oc get route <route_name> -n <app_ns> -o jsonpath="{.spec.host}" is correct, I think that we can move forward.
@sbeskin-redhat: Please squash your commits when you are ready to request a merge.
@mperetzred: If you would please verify that
$ oc get route <route_name> -n <app_ns> -o jsonpath="{.spec.host}"is correct, I think that we can move forward.
Hi @michaelryanpeter I executed this command, it worked fine.
$ oc get route console -n openshift-console -o jsonpath="{.spec.host}"
console-openshift-console.apps.oadp-19420.0923-i99.qe.rhcloud.com
