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

OADP-447: PVC Snapshot restore does not work if PVC requested size does not match disk capacity #45986

Merged
merged 1 commit into from
May 26, 2022

Conversation

RichardHoch
Copy link
Contributor

@RichardHoch RichardHoch commented May 24, 2022

OADP 1.0.3, OCP 4.6+

Resolves https://issues.redhat.com/browse/OADP-447 by adding a prerequisite to the procedure in section 4.3.1 of Backup and Restore

Preview:
Restore_CDR_Prereqs

Last prerequisite

@openshift-ci openshift-ci bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 24, 2022
@@ -14,6 +14,7 @@ You restore a `Backup` custom resource (CR) by creating a `Restore` CR.
* You must install the OpenShift API for Data Protection (OADP) Operator.
* The `DataProtectionApplication` CR must be in a `Ready` state.
* You must have a Velero `Backup` CR.
* Ensure that PVC capacity at backup time matches the requested size, adjusting the requested size to match capacity.

Choose a reason for hiding this comment

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

Ensure that the PV capacity maybe? PVC is the request...

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.

lgtm

@RichardHoch RichardHoch force-pushed the pvc_restore branch 2 times, most recently from e8bac25 to 04eb852 Compare May 25, 2022 10:14
@mperetzred
Copy link

lgtm

Copy link
Contributor

@opayne1 opayne1 left a comment

Choose a reason for hiding this comment

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

Small suggestion for you to consider.

@@ -14,6 +14,7 @@ You restore a `Backup` custom resource (CR) by creating a `Restore` CR.
* You must install the OpenShift API for Data Protection (OADP) Operator.
* The `DataProtectionApplication` CR must be in a `Ready` state.
* You must have a Velero `Backup` CR.
* Ensure that PV capacity at backup time matches the requested size, adjusting the requested size to match capacity.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest removing the bit after the comma. It seems repetitive of what you have in the first part of the sentence.

Suggested change
* Ensure that PV capacity at backup time matches the requested size, adjusting the requested size to match capacity.
* Ensure that persistent volume (PV) capacity at backup time matches the requested size.

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 raised this issue with the SME who said that "It may not be obvious to average user who maybe conscious of space requested."

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 keeping the clause as is still presents confusion. I have played with the wording a bit. what do you think of:

Suggested change
* Ensure that PV capacity at backup time matches the requested size, adjusting the requested size to match capacity.
* Ensure to adjust the requested size so the persistent volume (PV) capacity matches the requested size at backup time.

Copy link
Contributor

@opayne1 opayne1 left a comment

Choose a reason for hiding this comment

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

Added another suggestion to provide clarity. It is merely a suggestion, so feel free to reject if you think it messes with the meaning.

@@ -14,6 +14,7 @@ You restore a `Backup` custom resource (CR) by creating a `Restore` CR.
* You must install the OpenShift API for Data Protection (OADP) Operator.
* The `DataProtectionApplication` CR must be in a `Ready` state.
* You must have a Velero `Backup` CR.
* Ensure that PV capacity at backup time matches the requested size, adjusting the requested size to match capacity.
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 keeping the clause as is still presents confusion. I have played with the wording a bit. what do you think of:

Suggested change
* Ensure that PV capacity at backup time matches the requested size, adjusting the requested size to match capacity.
* Ensure to adjust the requested size so the persistent volume (PV) capacity matches the requested size at backup time.

@maxwelldb
Copy link
Contributor

/cherry-pick enterprise-4.6

@maxwelldb
Copy link
Contributor

/cherry-pick enterprise-4.7

@maxwelldb
Copy link
Contributor

/cherry-pick enterprise-4.8

@maxwelldb
Copy link
Contributor

/cherry-pick enterprise-4.9

@maxwelldb
Copy link
Contributor

/cherry-pick enterprise-4.10

@maxwelldb
Copy link
Contributor

/cherry-pick enterprise-4.11

@openshift-cherrypick-robot

@maxwelldb: new pull request created: #46050

In response to this:

/cherry-pick enterprise-4.6

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

@maxwelldb: new pull request created: #46051

In response to this:

/cherry-pick enterprise-4.7

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

@maxwelldb: new pull request created: #46052

In response to this:

/cherry-pick enterprise-4.8

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

@maxwelldb: new pull request created: #46054

In response to this:

/cherry-pick enterprise-4.9

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

@maxwelldb: new pull request created: #46055

In response to this:

/cherry-pick 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

@maxwelldb: new pull request could not be created: failed to create pull request against openshift/openshift-docs#enterprise-4.11 from head openshift-cherrypick-robot:cherry-pick-45986-to-enterprise-4.11: the GitHub API request returns a 403 error: {"message":"You have exceeded a secondary rate limit and have been temporarily blocked from content creation. Please retry your request again later.","documentation_url":"https://docs.github.com/rest/overview/resources-in-the-rest-api#secondary-rate-limits"}

In response to this:

/cherry-pick 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.

@maxwelldb
Copy link
Contributor

/cherry-pick enterprise-4.11

@openshift-cherrypick-robot

@maxwelldb: new pull request created: #46077

In response to this:

/cherry-pick 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.6 branch/enterprise-4.7 branch/enterprise-4.8 branch/enterprise-4.9 branch/enterprise-4.10 branch/enterprise-4.11 peer-review-done Signifies that the peer review team has reviewed this PR size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants