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

Make the vsc created by backup sync controller deletable #4832

Merged
merged 1 commit into from
Apr 20, 2022

Conversation

reasonerjt
Copy link
Contributor

@reasonerjt reasonerjt commented Apr 14, 2022

Fixes #4760

This commit make changes in 2 parts:

  1. When a volumesnapshotcontent is persisted during backup, velero will reset its
    Source field to remove the VolumeHandle, so that the
    csi-snapshotter will not try to call CreateSnapshot when its synced
    to another cluster with a backup.
  2. Make sure the referenced volumesnapshotclasses are persisted and
    synced with the backup, so that when the volumesnapshotcontent is
    deleted the storage snapshot is also removed.

Signed-off-by: Daniel Jiang jiangd@vmware.com

Thank you for contributing to Velero!

Please add a summary of your change

Does your change fix a particular issue?

Fixes #(issue)

Please indicate you've done the following:

  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Created a changelog file or added /kind changelog-not-required as a comment on this pull request.
  • Updated the corresponding documentation in site/content/docs/main.

@reasonerjt reasonerjt force-pushed the fix-rm-csi-bak-tmp branch 2 times, most recently from 8386835 to f429186 Compare April 14, 2022 09:01
@reasonerjt reasonerjt added this to the 1.9.0 milestone Apr 14, 2022
reasonerjt pushed a commit to reasonerjt/velero-plugin-for-csi that referenced this pull request Apr 18, 2022
This commit makes sure the behavior of the plugin is consistent with
velero, it reset the fields in VSC before it's persisted, to avoid
causing unexpected behavior in CSI snapshotter when the VSC is restored.

More details please see the description in
vmware-tanzu/velero#4832

Signed-off-by: Daniel Jiang <jiangd@vmware.com>
Fixes vmware-tanzu#4760

This commit make changes in 2 parts:
1) When a volumesnapshotcontent is persisted during backup, velero will reset its
   `Source` field to remove the VolumeHandle, so that the
   csi-snapshotter will not try to call `CreateSnapshot` when its synced
   to another cluster with a backup.
2) Make sure the referenced volumesnapshotclasses are persisted and
   synced with the backup, so that when the volumesnapshotcontent is
   deleted the storage snapshot is also removed.

Signed-off-by: Daniel Jiang <jiangd@vmware.com>
@codecov-commenter
Copy link

Codecov Report

Merging #4832 (7196dce) into main (c115a37) will increase coverage by 0.43%.
The diff coverage is 8.33%.

❗ Current head 7196dce differs from pull request most recent head 4f9e445. Consider uploading reports for the commit 4f9e445 to get more accurate results

@@            Coverage Diff             @@
##             main    #4832      +/-   ##
==========================================
+ Coverage   40.92%   41.36%   +0.43%     
==========================================
  Files         203      204       +1     
  Lines       17995    18059      +64     
==========================================
+ Hits         7365     7470     +105     
+ Misses      10104    10042      -62     
- Partials      526      547      +21     
Impacted Files Coverage Δ
pkg/cmd/server/server.go 6.75% <0.00%> (-0.04%) ⬇️
pkg/controller/backup_sync_controller.go 61.53% <0.00%> (-3.33%) ⬇️
pkg/persistence/object_store.go 57.77% <5.88%> (-2.73%) ⬇️
pkg/controller/backup_controller.go 57.37% <8.69%> (-2.63%) ⬇️
pkg/persistence/object_store_layout.go 91.83% <100.00%> (+0.34%) ⬆️
pkg/controller/pod_volume_restore_controller.go 23.34% <0.00%> (-1.25%) ⬇️
pkg/restic/exec_commands.go 15.38% <0.00%> (-0.81%) ⬇️
pkg/cmd/cli/backup/create.go 22.06% <0.00%> (-0.37%) ⬇️
pkg/cmd/cli/restic/server.go 15.05% <0.00%> (-0.25%) ⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c115a37...4f9e445. Read the comment docs.

@ywk253100 ywk253100 merged commit 9e786d6 into vmware-tanzu:main Apr 20, 2022
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.

The volumesnapshotcontent is not removed for the synced backup
4 participants