Skip to content

Conversation

@odoucet
Copy link
Contributor

@odoucet odoucet commented Jun 24, 2025

Describe what this PR does

PrepareVolumeForSnapshot() or flattenParentImage() requires rbd image flattening, that is performed in function flattenClonedRbdImages(). This function is lacking namespace support. This was undetected before because ceph local installs, used as tests, provides a default namespace and it's not common behaviour to create new ones.

The underlying bug is very critical : snapshots are performed but flattening are never performed, leading to accumulation of data.

Is there anything that requires special attention

Do you have any questions? no

Is the change backward compatible? yes

Are there concerns around backward compatibility? this code was tested with non-default namespaces but should be tested proactively in all situations.

Future concerns

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.

This is my first PR on this project, let me know if I missed something.

Copy link
Collaborator

@Madhu-1 Madhu-1 left a comment

Choose a reason for hiding this comment

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

LGTM.

@odoucet Do you have any reproducer steps to verify the fix?

@Madhu-1 Madhu-1 requested a review from Rakshith-R June 24, 2025 12:20
@odoucet
Copy link
Contributor Author

odoucet commented Jun 24, 2025

@odoucet Do you have any reproducer steps to verify the fix?

You can start following this guide to create namespace in ceph : https://access.redhat.com/solutions/4872331
Then create specific auth keys to that namespace and use it in csi-ceph.

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.

LGTM,
Thanks !

@Madhu-1
Copy link
Collaborator

Madhu-1 commented Jun 24, 2025

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Jun 24, 2025

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at b0eb428

@Madhu-1 Madhu-1 added the backport-to-release-v3.14 Label to backport from devel to release-v3.14 branch label Jun 24, 2025
flattenClonedRbdImages() requires namespace if it not the default one.

Signed-off-by: Olivier Doucet <[email protected]>
@mergify mergify bot force-pushed the radosnamespace-support branch from 4183efc to c628b6d Compare June 24, 2025 14:04
@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Jun 24, 2025
@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@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.33

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@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-helm/k8s-1.32

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@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 Jun 24, 2025
mergify bot added a commit that referenced this pull request Jun 24, 2025
@mergify mergify bot merged commit b0eb428 into ceph:devel Jun 24, 2025
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-to-release-v3.14 Label to backport from devel to release-v3.14 branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants