Skip to content

Conversation

@iPraveenParihar
Copy link
Contributor

Describe what this PR does

Currently, Ceph-CSI deletes RBD snapshot on temporary
cloned image (csi-vol-xxxx-temp@csi-vol-xxxx) which is
the parent of the final clone image.

The parent-child mirroring requires both the parent and child
images to be present (i.e, not in trash).

This commit makes enhancement to createRBDClone function by
introducing deleteSnap parameter. If deleteSnap is true,
the snapshot is deleted after the clone is created.

This is required to support mirroring of child image with its
parent image.

Checklist:

  • Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide.
  • Reviewed the developer guide on Submitting a Pull Request
  • Pending release notes updated with breaking and/or notable changes for the next major release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

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 unrelated
    failure (please report the failure too!)

@iPraveenParihar
Copy link
Contributor Author

/test ci/centos/mini-e2e/k8s-1.30

@iPraveenParihar
Copy link
Contributor Author

/test ci/centos/mini-e2e/k8s-1.30

@iPraveenParihar
Copy link
Contributor Author

/test ci/centos/mini-e2e/k8s-1.30

@iPraveenParihar
Copy link
Contributor Author

/test ci/centos/mini-e2e/k8s-1.30

@iPraveenParihar
Copy link
Contributor Author

/test ci/centos/mini-e2e/k8s-1.30

@iPraveenParihar
Copy link
Contributor Author

/test ci/centos/mini-e2e/k8s-1.30

@iPraveenParihar iPraveenParihar marked this pull request as ready for review March 11, 2025 07:38
@iPraveenParihar iPraveenParihar changed the title [WIP] RBD: retain temp snap RBD: retain temp snap Mar 11, 2025
Copy link
Contributor

@Rakshith-R Rakshith-R left a comment

Choose a reason for hiding this comment

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

Can you remove the go mod changes from this pr ?

@iPraveenParihar
Copy link
Contributor Author

Can you remove the go mod changes from this pr ?

🤔Upon syncing go.mod with devel branch and doing go mod tidy updates these packages.

Copy link
Member

@nixpanic nixpanic left a comment

Choose a reason for hiding this comment

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

Definitely need to drop the go.mod and related change. Or put them in a separate commit.

@iPraveenParihar
Copy link
Contributor Author

Can you remove the go mod changes from this pr ?

🤔Upon syncing go.mod with devel branch and doing go mod tidy updates these packages.

Also, now the go.sum seems to be changed when performing go mod tidy. Module github.com/aws/smithy-go was updated from v1.22.2 to v1.22.3 in PR. This is some weird issue am seeing recently.

@iPraveenParihar iPraveenParihar force-pushed the rbd/retain-temp-snap branch 3 times, most recently from 1f16a8a to 6b3b710 Compare March 14, 2025 10:44
Rakshith-R
Rakshith-R previously approved these changes Mar 14, 2025
@Rakshith-R Rakshith-R requested a review from nixpanic March 14, 2025 10:45
@iPraveenParihar
Copy link
Contributor Author

@nixpanic PTAL

@iPraveenParihar
Copy link
Contributor Author

Thanks @Madhu-1 @Rakshith-R

@iPraveenParihar
Copy link
Contributor Author

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Mar 18, 2025

queue

🛑 The pull request has been removed from the queue default

The pull request can't be updated.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

@mergify
Copy link
Contributor

mergify bot commented Mar 18, 2025

This pull request has been removed from the queue for the following reason: pull request branch update failed.

The pull request can't be updated

You should update or rebase your pull request manually.

If you want to requeue this pull request, you can post a @mergifyio requeue comment.

@iPraveenParihar
Copy link
Contributor Author

@Mergifyio rebase

Currently, Ceph-CSI deletes intermediate RBD snapshot on
temporary cloned images (`csi-vol-xxxx-temp@csi-vol-xxxx`)
which is the parent of the final clone image.

The parent-child mirroring requires both the parent and child
images to be present (i.e, not in trash).

This commit makes enhancement to `createRBDClone` function by
introducing `deleteSnap` parameter. If `deleteSnap` is true,
the snapshot is deleted after the clone is created.

This is required to support mirroring of child image with its
parent image.

Signed-off-by: Praveen M <[email protected]>
@mergify
Copy link
Contributor

mergify bot commented Mar 18, 2025

rebase

✅ Branch has been successfully rebased

@iPraveenParihar
Copy link
Contributor Author

@Mergifyio requeue

@mergify
Copy link
Contributor

mergify bot commented Mar 18, 2025

requeue

❌ This pull request head commit has not been previously disembarked from queue.

@iPraveenParihar
Copy link
Contributor Author

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Mar 18, 2025

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 2af7269

@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Mar 18, 2025
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.32

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.32

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.32

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Mar 18, 2025
@mergify mergify bot merged commit 2af7269 into ceph:devel Mar 18, 2025
36 of 37 checks passed
@mergify
Copy link
Contributor

mergify bot commented Mar 18, 2025

This pull request has been removed from the queue for the following reason: pull request branch update failed.

The pull request can't be updated

You should update or rebase your pull request manually.

If you want to requeue this pull request, you can post a @mergifyio requeue comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants