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

Data mover wrong bsl after sync #6533

Merged

Conversation

Lyndon-Li
Copy link
Contributor

@Lyndon-Li Lyndon-Li commented Jul 21, 2023

This PR fixes two issues:

  1. The PVB CR's StorageLocation is not reset to the latest one after backup sync, as a result, when deleting the PVB, the backup deletion controller cannot delete the repo snapshot from the correct backup storage location, so there will be data leak in the repo
  2. During data mover restore, the DataUploadResult's StorageLocation is retrieved from the DataUpload CR, while the value in DataUpload CR is not up to date if the BSL's name has changed in the new cluster, as a result, the data mover restore will fail

As the fixes:

  • For 1, reset PVB CR's StorageLocation to the latest one during backup sync
  • For 2, retrieve DataUploadResult's StorageLocation from the latest backup CR whose StorageLocation has been reset during backup sync

Fix #6534

@Lyndon-Li Lyndon-Li added kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes and removed has-unit-tests labels Jul 21, 2023
@Lyndon-Li Lyndon-Li force-pushed the data-mover-wrong-bsl-after-sync branch from d6e1784 to d804b14 Compare July 21, 2023 10:06
@Lyndon-Li Lyndon-Li removed the kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes label Jul 21, 2023
@Lyndon-Li Lyndon-Li marked this pull request as ready for review July 21, 2023 10:08
@Lyndon-Li Lyndon-Li force-pushed the data-mover-wrong-bsl-after-sync branch from d804b14 to 6b8840b Compare July 21, 2023 10:10
@codecov-commenter
Copy link

codecov-commenter commented Jul 21, 2023

Codecov Report

Merging #6533 (74bf03b) into main (d2b5e90) will increase coverage by 0.07%.
The diff coverage is 55.55%.

@@            Coverage Diff             @@
##             main    #6533      +/-   ##
==========================================
+ Coverage   60.28%   60.36%   +0.07%     
==========================================
  Files         238      239       +1     
  Lines       25256    25498     +242     
==========================================
+ Hits        15226    15391     +165     
- Misses       8976     9036      +60     
- Partials     1054     1071      +17     
Files Changed Coverage Δ
pkg/cmd/cli/nodeagent/server.go 9.03% <0.00%> (-0.37%) ⬇️
pkg/controller/backup_sync_controller.go 45.18% <100.00%> (+0.18%) ⬆️
pkg/restore/dataupload_retrieve_action.go 75.75% <100.00%> (+3.82%) ⬆️

... and 13 files with indirect coverage changes

@@ -29,19 +29,22 @@ import (

velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
velerov2alpha1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v2alpha1"
velerov1client "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/typed/velero/v1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use the controller runtime client instead as we are moving away from the generated clients

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done the changes, thanks.

@Lyndon-Li Lyndon-Li force-pushed the data-mover-wrong-bsl-after-sync branch from 6b8840b to ceac873 Compare July 25, 2023 04:08
Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
@Lyndon-Li Lyndon-Li force-pushed the data-mover-wrong-bsl-after-sync branch from ceac873 to 74bf03b Compare July 25, 2023 04:38
@Lyndon-Li Lyndon-Li merged commit 9c8275e into vmware-tanzu:main Jul 25, 2023
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Backup repo snapshot is not deleted after backup sync for file system backup
4 participants