Skip to content

rbd: skip flatten for ROX snapshot restores - #6534

Merged
mergify[bot] merged 1 commit into
ceph:develfrom
Rakshith-R:rox-snapshot-skip-flatten
Sep 9, 2026
Merged

rbd: skip flatten for ROX snapshot restores#6534
mergify[bot] merged 1 commit into
ceph:develfrom
Rakshith-R:rox-snapshot-skip-flatten

Conversation

@Rakshith-R

@Rakshith-R Rakshith-R commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Preserve the RBD parent chain when restoring snapshots as read-only volumes. This allows rbd diff changed-block tracking to work for incremental backups.

CreateVolume passes the all-capabilities reader-only state to flattenParentImage. ROX snapshot restores return before flattening, while RW and mixed-capability restores retain the existing flattening behavior.

The clone-depth acceptance coverage now verifies both behaviors with trashed parents: ROX restores retain a parent, and RW restores flatten the image.

Validation

  • internal/csi-common unit tests
  • internal/rbd unit tests
  • go vet for touched packages and e2e
  • E2E compile check
  • PR acceptance E2E passed

Assisted-by: Claude Code noreply@anthropic.com


CI resource ordering.

Depends-on: #6490

Copilot AI lite review requested due to automatic review settings September 3, 2026 11:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

It changes core RBD snapshot-restore behavior in the CSI controller (flattening semantics), which can have broad functional impact and should be validated by a maintainer with real-world upgrade/backup workflows in mind.

Pull request overview

This PR updates the RBD CSI controller snapshot-restore path to preserve deep clone chains for read-only (ROX) restores by skipping parent flattening, enabling tools like rbd diff to leverage the parent chain for incremental backup workflows. It also extends the RBD E2E coverage to assert ROX retention behavior while keeping existing RW restore flattening behavior.

Changes:

  • Skip flattenParentImage() during CreateVolume when restoring from a snapshot with exclusively reader-only capabilities.
  • Add AreAllCapabilitiesReaderOnly() helper (with unit tests) to distinguish “all-RO” requests from mixed capability sets.
  • Extend the “trashed parents + clone depth” RBD E2E to verify ROX restore retains the parent chain and RW restore still triggers flattening (and label it for acceptance).
File summaries
File Description
internal/rbd/controllerserver.go Skips snapshot-parent flattening for ROX snapshot restores while preserving existing behavior for RW restores.
internal/csi-common/utils.go Adds AreAllCapabilitiesReaderOnly() to detect exclusively reader-only capability sets.
internal/csi-common/utils_test.go Adds unit tests for the new AreAllCapabilitiesReaderOnly() helper.
e2e/rbd.go Updates/renames the clone-depth test and adds acceptance label.
e2e/rbd_helper.go Refactors parent-detection logic and adds ensureRBDImageHasParent() plus getRBDImageParent() helper.
e2e/rbd_clone_depth.go Extends the trashed-parent clone-depth scenario to cover ROX retention and RW flattening expectations.
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread e2e/rbd_helper.go

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused behavior change is validated by comprehensive unit and end-to-end coverage.

Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@Rakshith-R
Rakshith-R force-pushed the rox-snapshot-skip-flatten branch 2 times, most recently from 917177d to 9e38256 Compare September 7, 2026 10:25
@Rakshith-R
Rakshith-R requested a balanced review from Copilot September 7, 2026 10:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The implementation is focused, handles malformed capabilities safely, and includes appropriate unit and E2E coverage.

Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@Rakshith-R
Rakshith-R force-pushed the rox-snapshot-skip-flatten branch from 9e38256 to 3cd412a Compare September 7, 2026 11:07
@Rakshith-R Rakshith-R changed the title Rox snapshot skip flatten rbd: skip flatten for ROX snapshot restores Sep 7, 2026
@mergify mergify Bot added the component/rbd Issues related to RBD label Sep 7, 2026
@Rakshith-R
Rakshith-R force-pushed the rox-snapshot-skip-flatten branch 2 times, most recently from 3890922 to d1d5bbb Compare September 7, 2026 11:58
@Rakshith-R
Rakshith-R marked this pull request as ready for review September 7, 2026 12:00
@Rakshith-R
Rakshith-R requested review from a team as code owners September 7, 2026 12:00
Comment thread internal/rbd/controllerserver.go
@Rakshith-R
Rakshith-R force-pushed the rox-snapshot-skip-flatten branch 2 times, most recently from 3c85645 to ad22352 Compare September 8, 2026 09:38
@Rakshith-R
Rakshith-R force-pushed the rox-snapshot-skip-flatten branch from ad22352 to 3617669 Compare September 8, 2026 09:43
@nixpanic

nixpanic commented Sep 8, 2026

Copy link
Copy Markdown
Member

Wait for #6490 to be merged before adding /queue here.

@mergify

mergify Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Merge Protections

🟢 All 2 merge protections satisfied — ready to merge.

Show 2 satisfied protections

🟢 ⛓️ Depends-On Requirements

Requirement based on the presence of Depends-On in the body of the pull request

🟢 🚦 Auto-queue

When all merge protections are satisfied, this pull request will be queued automatically.

@nixpanic

nixpanic commented Sep 9, 2026

Copy link
Copy Markdown
Member

/queue

@ceph-csi-bot

Copy link
Copy Markdown
Collaborator

@Mergifyio rebase

@mergify

mergify Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

rebase

🛑 The pull request rule doesn't match anymore

Details

This action has been cancelled.

Avoid flattening snapshot parents when restoring read-only volumes so
deep clone chains remain intact. Extend the trashed-parent E2E to verify
ROX retention and RW flattening.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Rakshith R <rar@redhat.com>
@ceph-csi-bot
ceph-csi-bot force-pushed the rox-snapshot-skip-flatten branch from 3617669 to 144534c Compare September 9, 2026 07:23
@ceph-csi-bot ceph-csi-bot added ok-to-test Label to trigger E2E tests and removed queued/rebase labels Sep 9, 2026
@ceph-csi-bot

Copy link
Copy Markdown
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot

Copy link
Copy Markdown
Collaborator

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

@ceph-csi-bot

Copy link
Copy Markdown
Collaborator

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

@ceph-csi-bot

Copy link
Copy Markdown
Collaborator

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

@ceph-csi-bot

Copy link
Copy Markdown
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot

Copy link
Copy Markdown
Collaborator

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

@ceph-csi-bot

Copy link
Copy Markdown
Collaborator

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

@ceph-csi-bot

Copy link
Copy Markdown
Collaborator

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

@ceph-csi-bot

Copy link
Copy Markdown
Collaborator

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

@ceph-csi-bot

Copy link
Copy Markdown
Collaborator

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

@ceph-csi-bot

Copy link
Copy Markdown
Collaborator

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

@ceph-csi-bot ceph-csi-bot added ci/in-progress/e2e This label acts like a guard and prevents Mergify from adding the `ok-to-test` label again. and removed ok-to-test Label to trigger E2E tests labels Sep 9, 2026
@mergify mergify Bot removed the ci/in-progress/e2e This label acts like a guard and prevents Mergify from adding the `ok-to-test` label again. label Sep 9, 2026
@iPraveenParihar

Copy link
Copy Markdown
Contributor

/retest ci/centos/mini-e2e/k8s-1.34

@mergify

mergify Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Deprecation notice: This pull request comes from a fork and was queued with update_method=rebase and update_bot_account impersonation. This capability will be removed on July 1, 2026. After this date, the merge queue will no longer be able to rebase fork pull requests with this configuration. To avoid disruption, switch to update_method=merge in your queue rule.

@mergify

mergify Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • Entered queue2026-09-09 12:14 UTC · Rule: default · triggered by merge protections
  • Checks skipped · PR is already up-to-date
  • Merged2026-09-09 12:15 UTC · at c2be926f46d436f7c8870ef01d3853554544ec81 · rebase

This pull request spent 11 seconds in the queue, including 2 seconds running CI.

Required conditions to merge

@mergify
mergify Bot merged commit c2be926 into ceph:devel Sep 9, 2026
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/rbd Issues related to RBD

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants