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.

Sign up for GitHub

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

Jump to bottom

OADP 2943 Rewrite of Restic Backup CR to include Kopia #67067

Merged
merged 1 commit into from
Nov 21, 2023
Merged

OADP 2943 Rewrite of Restic Backup CR to include Kopia #67067

merged 1 commit into from
Nov 21, 2023

Conversation

CarmiWisemon
Copy link
Contributor

@CarmiWisemon CarmiWisemon commented Oct 29, 2023 •
edited

@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 29, 2023
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Oct 29, 2023 •
edited

🤖 Updated build preview is available at:
https://67067--docspreview.netlify.app

Build log: https://circleci.com/gh/ocpdocs-previewbot/openshift-docs/34189

@openshift-ci openshift-ci bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 29, 2023
@openshift-ci openshift-ci bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 29, 2023
@CarmiWisemon CarmiWisemon force-pushed the oadp2943backuprewrite branch 4 times, most recently from 21945ff to d822719 Compare November 1, 2023 16:36
@weshayutin
Copy link

weshayutin commented Nov 1, 2023 •
edited

Looks good to me /lgtm
reviewed https://67067--docspreview.netlify.app/openshift-enterprise/latest/backup_and_restore/application_backup_and_restore/backing_up_and_restoring/oadp-backing-up-applications-restic-doc

@CarmiWisemon
Copy link
Contributor Author

@stillalearner

Can you please review the PR.
Thanks!

@openshift-ci openshift-ci bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 5, 2023
stillalearner
stillalearner approved these changes Nov 6, 2023
View reviewed changes
Copy link

@stillalearner stillalearner left a comment

Choose a reason for hiding this comment

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

lgtm

@CarmiWisemon CarmiWisemon force-pushed the oadp2943backuprewrite branch 2 times, most recently from 1e83aec to aeabcd6 Compare November 7, 2023 16:10
adellape
adellape reviewed Nov 20, 2023
View reviewed changes
Copy link
Contributor

@adellape adellape left a comment

Choose a reason for hiding this comment

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

It looks like there are a couple :_mod-docs-content-type: attributes that are being changed back to the older :_content-type. See https://github.com/openshift/openshift-docs/blob/main/contributing_to_docs/doc_guidelines.adoc#content-type-attributes.

Dropping merge-review labels for now; please re-queue when issues have been addressed. Thank you!

...ion_backup_and_restore/backing_up_and_restoring/oadp-backing-up-applications-restic-doc.adoc Outdated
:_mod-docs-content-type: PROCEDURE
:_content-type: PROCEDURE
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert, also it seems like this type should be ASSEMBLY instead of PROCEDURE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adellape
I changed the attribute, but based on the other modules in the assembly this module is a procedure.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think what Alex is saying is that this is an assembly file based on the metadata and the fact that it's not in the modules folder. Assembly files use the type of ASSEMBLY:

The content type for the file. For assemblies, always use :_mod-docs-content-type: ASSEMBLY. Place this attribute before the anchor ID or, if present, the conditional that contains the anchor ID.

https://github.com/openshift/openshift-docs/blob/main/contributing_to_docs/doc_guidelines.adoc#assembly-file-metadata

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kcarmichael08
I made the change that you and Alex requested. I would appreciate if you could merge this PR.
Thank you for your help!
Carmi

modules/oadp-backing-up-applications-restic.adoc Outdated
:_mod-docs-content-type: PROCEDURE
:_content-type: PROCEDURE
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert

@adellape adellape removed merge-review-in-progress Signifies that the merge review team is reviewing this PR merge-review-needed Signifies that the merge review team needs to review this PR labels Nov 20, 2023
@CarmiWisemon CarmiWisemon force-pushed the oadp2943backuprewrite branch 2 times, most recently from cf8484e to 4c82364 Compare November 21, 2023 10:04
@CarmiWisemon
Copy link
Contributor Author

/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 Nov 21, 2023
@kcarmichael08 kcarmichael08 added merge-review-in-progress Signifies that the merge review team is reviewing this PR and removed merge-review-needed Signifies that the merge review team needs to review this PR labels Nov 21, 2023
@kcarmichael08
Copy link
Contributor

I think the issue that Alex pointed out with the assembly file needs to be fixed still; otherwise, LGTM to merge.

@kcarmichael08 kcarmichael08 merged commit a915b30 into openshift:main Nov 21, 2023
1 check passed
@kcarmichael08
Copy link
Contributor

/cherrypick enterprise-4.13

@kcarmichael08
Copy link
Contributor

/cherrypick enterprise-4.14

@kcarmichael08
Copy link
Contributor

/cherrypick enterprise-4.15

@openshift-cherrypick-robot
Copy link

@kcarmichael08: #67067 failed to apply on top of branch "enterprise-4.13":

Applying: OADP 2943 Backup rewrite
Using index info to reconstruct a base tree...
M	_topic_maps/_topic_map.yml
M	backup_and_restore/application_backup_and_restore/backing_up_and_restoring/backing-up-applications.adoc
M	backup_and_restore/application_backup_and_restore/oadp-advanced-topics.adoc
M	backup_and_restore/index.adoc
M	modules/oadp-backing-up-pvs-csi.adoc
M	virt/backup_restore/virt-backing-up-vms.adoc
Falling back to patching base and 3-way merge...
Auto-merging virt/backup_restore/virt-backing-up-vms.adoc
CONFLICT (content): Merge conflict in virt/backup_restore/virt-backing-up-vms.adoc
Auto-merging modules/oadp-backing-up-pvs-csi.adoc
CONFLICT (content): Merge conflict in modules/oadp-backing-up-pvs-csi.adoc
Auto-merging backup_and_restore/index.adoc
Auto-merging backup_and_restore/application_backup_and_restore/oadp-advanced-topics.adoc
Auto-merging backup_and_restore/application_backup_and_restore/backing_up_and_restoring/backing-up-applications.adoc
Auto-merging _topic_maps/_topic_map.yml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 OADP 2943 Backup rewrite
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick enterprise-4.13

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
Copy link

@kcarmichael08: new pull request created: #68252

In response to this:

/cherrypick enterprise-4.14

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 openshift-cherrypick-robot mentioned this pull request Nov 21, 2023
Merged
@openshift-cherrypick-robot
Copy link

@kcarmichael08: new pull request created: #68253

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/test-infra repository.

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Nov 21, 2023
Merged
@kcarmichael08
Copy link
Contributor

Hi @CarmiWisemon , it looks like 4.13 will need a manual cherry pick.

Also, just confirming that it does NOT need to be cherry picked to 4.11 & 4.12 (I think those labels were erroneously added, maybe).

@CarmiWisemon
Copy link
Contributor Author

Hi @CarmiWisemon , it looks like 4.13 will need a manual cherry pick.

Also, just confirming that it does NOT need to be cherry picked to 4.11 & 4.12 (I think those labels were erroneously added, maybe).

@kcarmichael08
It does NOT need to be cherry picked for 4.11 & 4.12.
I initially asked for those by mistake.
Thank you for your help!

@kcarmichael08
Copy link
Contributor

I think we still need a manual cherry pick for 4.13.

@CarmiWisemon
Copy link
Contributor Author

@kcarmichael08

Thanks. What does that require?

@kcarmichael08
Copy link
Contributor

@kcarmichael08

Thanks. What does that require?

If you follow the instructions in the OpenShift docs manual, under "To manual cherry pick using the command line, in the event of merge errors:" then it should be fairly straightforward. But let me know if you run into any issues, and I can help!

@anarnold97 anarnold97 mentioned this pull request Nov 28, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adellape adellape adellape left review comments

@apinnick apinnick apinnick left review comments

@bburt-rh bburt-rh bburt-rh left review comments

@kcarmichael08 kcarmichael08 kcarmichael08 left review comments

@anarnold97 anarnold97 anarnold97 left review comments

@kaovilai kaovilai kaovilai approved these changes

@stillalearner stillalearner stillalearner approved these changes

Assignees

@adellape adellape

Labels
branch/enterprise-4.13 branch/enterprise-4.14 branch/enterprise-4.15 merge-review-in-progress Signifies that the merge review team is reviewing this PR OADP Label for all OADP 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
Milestone
Continuous Release
Development

Successfully merging this pull request may close these issues.

None yet