-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Issue 7068: add a finalizer to protect retained VSC #7102
Conversation
2c67937
to
3883e7b
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #7102 +/- ##
==========================================
- Coverage 61.24% 61.16% -0.08%
==========================================
Files 257 258 +1
Lines 27273 27300 +27
==========================================
- Hits 16702 16699 -3
- Misses 9388 9419 +31
+ Partials 1183 1182 -1 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Lyndon-Li <[email protected]>
3883e7b
to
067984b
Compare
|
||
vsc.Finalizers = stringslice.Except(vsc.Finalizers, volumeSnapshotContentProtectFinalizer) | ||
|
||
_, err = snapshotClient.VolumeSnapshotContents().Update(ctx, vsc, metav1.UpdateOptions{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use update()
rather than patch()
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a contest when updating the VSC, the external snapshotter is also manipulating the VSC, specifically, a finalizer has been removed. Therefore, we use update+retry to make sure that what we are updating is with the latest version.
The problem we've seen if we use patch:
error to remove protect finalizer from vsc snapcontent-85024560-ca99-4f46-8378-5439bdb11a15: error patching VSC: VolumeSnapshotContent.snapshot.storage.k8s.io "snapcontent-85024560-ca99-4f46-8378-5439bdb11a15" is invalid: metadata.finalizers: Forbidden: no new finalizers can be added if the object is being deleted, found new finalizers []string{"snapshot.storage.kubernetes.io/volumesnapshotcontent-bound-protection"
Fix issue #7068, due to a behavior of CSI external snapshotter, manipulations of VS and VSC may not be handled in the same order inside external snapshotter as the API is called. So add a protection finalizer to ensure the order