OADP-2144: Resize cloned pvc based on cmp between cloned vs size and source pvc size #251
Conversation
|
@shubham-pampattiwar: This pull request references OADP-2144 which is a valid jira issue. In response to this:
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-ci
bot
added
the
approved
shubham-pampattiwar
changed the title
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
|
@shubham-pampattiwar: This pull request references OADP-2144 which is a valid jira issue. In response to this: 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. |
shubham-pampattiwar
changed the title
|
@sseago Thank you ! Updated the PR title (fixed typo) |
|
@shubham-pampattiwar: This pull request references OADP-2144 which is a valid jira issue. In response to this:
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. |
controllers/pvc.go
Outdated
| @@ -143,6 +156,11 @@ func (r *VolumeSnapshotBackupReconciler) buildPVCClone(pvcClone *corev1.Persiste | |||
| } | |||
|
|
|||
| pvcClone.Spec.Resources = sourcePVC.Spec.Resources | |||
|
|
|||
| // use the clonedPVCSize that is computed earlier | |||
| if clonedPVCSize != sourcePVCRequestSize { | |||
There was a problem hiding this comment.
Why not just always set to clonedSize?
There was a problem hiding this comment.
@shawn-hurley will cloned size ever be smaller than source PVC size? This seems safer for "just in case the (almost) impossible happens"
There was a problem hiding this comment.
We are just doing the same logic twice I think
There was a problem hiding this comment.
@sseago @shawn-hurley updated the PR, PTAL, Thanks !
|
I am a little worried about the amount of log statements. You could probably use half of the logs and get the same info. |
|
@shawn-hurley @kaovilai This PR is not ready to be merged/final review yet, the log statements are part of the patch we are providing to customer for testing. |
|
@shubham-pampattiwar: This pull request references OADP-2144 which is a valid jira issue. In response to this:
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. |
|
/lgtm |
|
@kaovilai: once the present PR merges, I will cherry-pick it on top of oadp-1.2 in a new PR and assign it to you. In response to this:
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. |
|
/unlgtm |
|
/lgtm cancel |
|
This patch worked in the customer environment as well as for manual developer testing. |
|
PTAL @kaovilai @shawn-hurley @sseago Thank you ! |
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kaovilai, shawn-hurley, shubham-pampattiwar, sseago The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@kaovilai: new pull request created: #261 In response to this:
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. |
Fixes #57
Target 1.2.2