RBD: add snap delete function #5000
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6 tasks
iPraveenParihar
force-pushed
the
rbd/add-snap-delete-function
branch
from
35bcc53 to
edc9bb2
Compare
|
/test ci/centos/mini-e2e-helm/k8s-1.31 |
1 similar comment
|
/test ci/centos/mini-e2e-helm/k8s-1.31 |
Madhu-1
reviewed
Dec 4, 2024
| validateOmapCount(rvgs.framework, 0, rbdType, defaultRBDPool, volumesType) | ||
| validateOmapCount(rvgs.framework, 0, rbdType, defaultRBDPool, snapsType) | ||
| validateOmapCount(rvgs.framework, 0, rbdType, defaultRBDPool, groupSnapsType) | ||
|
|
There was a problem hiding this comment.
add a validation of no images in the pool itself. validateRBDImageCount
iPraveenParihar
force-pushed
the
rbd/add-snap-delete-function
branch
from
3ae9abb to
9b76924
Compare
|
/test ci/centos/mini-e2e/k8s-1.31 |
|
failed ci logs
/DeleteSnapshot was called for replicapool/csi-snap-8c1a0986-5ba1-4111-bcbb-5faac931bc14 And this image was actually part of VolumeGroupSnapshot When deleting VolumeGroupSnapshot, snapshotter calls /DeleteSnapshot for the snapshot that is part of the VolumeGroupSnapshot being deleted - will be fixed by kubernetes-csi/external-snapshotter#1231. |
iPraveenParihar
force-pushed
the
rbd/add-snap-delete-function
branch
from
9b76924 to
aaa79e8
Compare
|
/test ci/centos/mini-e2e/k8s-1.31 |
|
passed ci logs @Madhu-1 @Rakshith-R PTAL |
Madhu-1
requested changes
Dec 4, 2024
e2e/volumegroupsnapshot.go
Outdated
|
|
||
| err := waitToRemoveImagesFromTrash(rvgs.framework, defaultRBDPool, deployTimeout) | ||
| if err != nil { | ||
| framework.Failf("failed to validate rbd images in pool %s trash: %v", defaultRBDPool, err) |
There was a problem hiding this comment.
its already logged at the caller, lets remove the extra logging from here
There was a problem hiding this comment.
I am not sure if framework.Failf() really returns, does the test not get aborted when it is called?
internal/rbd/snapshot.go
Outdated
| rbdSnap.RbdImageName = rbdVol.RbdImageName | ||
| err = cleanUpSnapshot(ctx, rbdVol, rbdSnap, rbdVol) | ||
| if err != nil { | ||
| log.ErrorLog(ctx, "failed to delete image: %v", err) |
There was a problem hiding this comment.
log the details of what image and snapshot was tried for cleanup for better debugging.
|
Can you please increase the pvc count and put the vgs test in loop of 5 or 10 (as extra commit) to ensure the issue is fixed as we are close to release? |
iPraveenParihar
force-pushed
the
rbd/add-snap-delete-function
branch
from
aaa79e8 to
deaa370
Compare
|
/test ci/centos/mini-e2e/k8s-1.29 |
|
/test ci/centos/mini-e2e/k8s-1.31 |
iPraveenParihar
force-pushed
the
rbd/add-snap-delete-function
branch
2 times, most recently
from
fac97cc to
eadc4bc
Compare
|
/test ci/centos/mini-e2e/k8s-1.31 |
iPraveenParihar
force-pushed
the
rbd/add-snap-delete-function
branch
from
eadc4bc to
0764605
Compare
|
/test ci/centos/mini-e2e/k8s-1.31 |
iPraveenParihar
force-pushed
the
rbd/add-snap-delete-function
branch
from
0764605 to
b22b61d
Compare
|
/test ci/centos/mini-e2e/k8s-1.31 |
|
test volumeGroupSnapshot for both RBD and CephFS passed here. However, it failed due to test timeout of 150m. I'll try with 3 PVCs and loop 3 times. |
iPraveenParihar
force-pushed
the
rbd/add-snap-delete-function
branch
from
b22b61d to
95cfa33
Compare
|
/test ci/centos/mini-e2e/k8s-1.31 |
|
passed ci logs. But, OMAP from ceph-csi/internal/rbd/group/util.go Lines 303 to 307 in a32ba13 This is not getting captured in e2e as we don't have check for RBD groupSnap OMAP data - Lines 165 to 217 in a32ba13 I'll add fix for both in this PR. |
iPraveenParihar
force-pushed
the
rbd/add-snap-delete-function
branch
from
95cfa33 to
61a81b5
Compare
Madhu-1
reviewed
Dec 16, 2024
This function deletes rbd snap and rbd image backing k8s snapshot. The same function is used for deleting individual snapshots in group snapshot. Signed-off-by: Praveen M <[email protected]>
This commit fixes the VolumeGroup UndoReservation by using the correct RequestName of the VolumeGroup instead of the volumeGroupHandle. Signed-off-by: Praveen M <[email protected]>
Signed-off-by: Praveen M <[email protected]>
iPraveenParihar
force-pushed
the
rbd/add-snap-delete-function
branch
from
34f1a82 to
8fa6da2
Compare
Madhu-1
approved these changes
Dec 16, 2024
Rakshith-R
approved these changes
Dec 16, 2024
|
@Mergifyio queue |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at 88b7e0d |
|
/test ci/centos/k8s-e2e-external-storage/1.29 |
|
/test ci/centos/upgrade-tests-cephfs |
|
/test ci/centos/mini-e2e-helm/k8s-1.29 |
|
/test ci/centos/upgrade-tests-rbd |
|
/test ci/centos/k8s-e2e-external-storage/1.30 |
|
/test ci/centos/mini-e2e/k8s-1.29 |
|
/test ci/centos/k8s-e2e-external-storage/1.31 |
|
/test ci/centos/mini-e2e-helm/k8s-1.30 |
|
/test ci/centos/mini-e2e-helm/k8s-1.31 |
|
/test ci/centos/mini-e2e/k8s-1.30 |
|
/test ci/centos/mini-e2e/k8s-1.31 |
|
@Mergifyio backport release-v3.13 |
✅ Backports have been created
|
6 tasks
iPraveenParihar
added
the
backport-to-release-v3.13
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe what this PR does
This function deletes rbd snap and rbd image
backing k8s snapshot.
The same function is used for deleting
individual snapshots in group snapshot.
Checklist:
Show available bot commands
These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:
/retest ci/centos/<job-name>: retest the<job-name>after unrelatedfailure (please report the failure too!)