openshift / oadp-operator

OADP Operator
Apache License 2.0
77 stars 70 forks source link

datamover commits in 1.1 are on openshift/velero.. They were all dropped in master #905

Closed weshayutin closed 1 year ago

weshayutin commented 1 year ago

Here is the list of commits that went on OADP 1.1.x that were related to datamover (analysis comments are inline on whether we need then or not):

[Add volumesnapshotbackups to restore priority] https://github.com/openshift/velero/pull/172 : We need this for sure as we need vsb to be restored first so that we can create VSR via RIA plugin [OADP-598: Improve datamover backup performance] https://github.com/openshift/velero/pull/179: We might not need this as we are going to rely on async plugins for datamover [OADP-644: Make datamover timeout configurable] https://github.com/openshift/velero/pull/183: We might not need this as we are going to rely on async plugins for datamover [log volumesnapshotbackup status if is not complete.] https://github.com/openshift/velero/commit/a519b2fb066d1d09b95526b466b64dd348745e4c : We might not need this as we are going to rely on async plugins for datamover [OADP-645: Clean temp VSClass for data mover restore] https://github.com/openshift/velero/pull/197 : Datamover restore performance depends on a fakevsclass that we create so we might not need as we port of plugin to use async plugin API [OADP-612: partiallyFail Backup if VSB fails] https://github.com/openshift/velero/pull/200 : backup state monitoring will be part of the async operations controller hence we might need this commit as well [Add nil check before execution of csi snapshot delete] https://github.com/openshift/velero/commit/5dc2c98bfaea787b3e62f243b60aad26f442566a : This fix got merged upstream already so we will have this in our regular rebase [OADP-927: Remove VSClass name in Get error] https://github.com/openshift/velero/pull/212 : Similar reasoning as that of point number 5 [OADP-1002: Ignore not found err for temp vsclass in cases of data mover without PVC]https://github.com/openshift/velero/pull/215 : Similar reasoning as that of point number 5

As of the current understanding we might need only 1 commit that is point 1 but other changes might depend on how the async controller changes get implemented upstream.

weshayutin commented 1 year ago

@shubham-pampattiwar instead of using Jira.. where are with untangling the spaghetti? Probably still holding for upstream to merge BIAV2

shubham-pampattiwar commented 1 year ago

@weshayutin Yes we need the upstream BiA v2 controller changes to get merged. Once that is done, I will be creating a PR to add changes related to deferring the deletion of CSI VolumeSnapshots in Backup finalize phase, the deletion will be removed from backup_controller and delegated to backup_finalizer_conrtoller (new in BIA V2 changes)

sseago commented 1 year ago

@shubham-pampattiwar What about the rest of the 1.1 fork commits? Those weren't related to the async workflow changes -- is that stuff needed or no longer relevant for 1.2?

weshayutin commented 1 year ago

The oadp-operator needs to be updated before we can proceed here

shubham-pampattiwar commented 1 year ago

Openshift/Velero is updated as of March 27th 2023 with the latest upstream changes. Might need another rebase after the v1.11 branching is done later this week. Regarding the datamover changes needed, will be doing a PRs for 2 things as of now:

shubham-pampattiwar commented 1 year ago

Adding VSB to Restore Priority: https://github.com/openshift/velero/pull/252

weshayutin commented 1 year ago

This ticket 905 closes whn working on https://github.com/openshift/velero/pull/255 is complete