rbd: skip DiffIterate for block volume stats without object-map by iPraveenParihar · Pull Request #6442 · ceph/ceph-csi

iPraveenParihar · 2026-07-31T10:52:03Z

Describe what this PR does

DiffIterate with WholeObject relies on the object-map feature for fast used-bytes calculation.
Images created before object-map was part of the default feature set (e.g. older PVC clones)
cause DiffIterate to fall back to reading actual data blocks, making NodeGetVolumeStats extremely
slow.

Check for the object-map feature before calling DiffIterate and fall back to returning only the block
device size when it is absent.

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 · 2026-08-03T07:50:16Z

@Rakshith-R PTAL

Copilot

Pull request overview

This PR improves Ceph-CSI operational reliability by avoiding expensive RBD DiffIterate-based used-bytes calculation when object-map is missing in an image’s clone chain, and introduces a lightweight “acceptance” E2E smoke gate (minikube + Rook Ceph) that runs on every PR to catch basic deployment/provisioning regressions early.

Changes:

RBD: detect object-map across the full clone chain and fall back to capacity-only stats when absent (avoids very slow NodeGetVolumeStats on older/cloned images).
CI/E2E: add a new GitHub Actions minikube “acceptance” workflow plus supporting helper scripts and labeled smoke specs.
NVMe-oF + NFS: add clone capability for NVMe-oF and add mutable NFS clients support via ControllerModifyVolume, with examples/tests/docs updates.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
scripts/rook.sh	Adjust Rook deploy readiness checks for RBD pool (phase-based readiness).
scripts/minikube.sh	Update kubectl download URL and handle kubelet config update for `VM_DRIVER=none`.
scripts/github-action-helper.sh	Add GitHub Actions helper for minikube prereqs, disk prep, and failure log collection.
internal/util/log/log.go	Add stack-dump tracing for slow gRPC calls at trace verbosity.
internal/rbd/rbd_util.go	Add clone-chain feature walk helper to validate prerequisites for fast diff usage.
internal/rbd/nodeserver.go	Skip used-bytes diff calculation when `object-map` is missing; return capacity-only stats.
internal/nvmeof/driver/driver.go	Advertise CLONE_VOLUME capability for NVMe-oF controller service.
internal/nvmeof/controller/controllerserver.go	Add source-volume lock handling for clone requests and clarify cloning path.
internal/nfs/types/volume.go	Introduce mutable `clients` parameter and implement export update logic.
internal/nfs/controller/controllerserver.go	Wire mutable `clients` parameter into `ControllerModifyVolume`.
internal/csi-common/utils.go	Log goroutine stacks once for slow gRPC calls (trace level).
examples/nvmeof/pvc-clone.yaml	Provide NVMe-oF PVC clone example manifest.
examples/nfs/volumeattributesclass.yaml	Document mutable `clients` usage via VolumeAttributesClass.
examples/nfs/storageclass.yaml	Clarify that `clients` can be updated post-create via VolumeAttributesClass.
e2e/utils.go	Add helpers/flags for acceptance runs (skip Vault, empty KMS configmap creation).
e2e/README.md	Document acceptance E2E suite purpose, scope, and how to run/extend it.
e2e/rbd.go	Add acceptance spec for block volume stats behavior without `object-map`; support skip-vault.
e2e/pvc.go	Add kubelet metrics scraping helper to validate volume stats metrics.
e2e/nvmeof.go	Add an NVMe-oF PVC clone E2E flow.
e2e/nfs.go	Add VolumeAttributesClass-based NFS `clients` mutation test and SC creation adjustments.
e2e/e2e_test.go	Add `--skip-vault` flag to speed up acceptance gate execution.
e2e/cephfs.go	Honor `--skip-vault` for CephFS test setup/teardown.
build.env	Set default minikube Kubernetes version.
AGENTS.md	Document acceptance E2E workflow and how to add labeled specs.
.github/workflows/e2e-minikube-acceptance.yaml	New PR-triggered acceptance minikube workflow and log artifact upload.

Suppressed comments (2)

.github/workflows/e2e-minikube-acceptance.yaml:54

The line continuation inside the quoted echo for CSI_IMAGE_VERSION inserts leading spaces into the value when written to $GITHUB_ENV, which can cause consumers of CSI_IMAGE_VERSION to receive a value with unexpected whitespace.

            echo "CSI_IMAGE_VERSION=\
          ${CSI_IMAGE_VERSION}"

scripts/github-action-helper.sh:39

This second grep has the same "(loop|${boot_dev})" pattern issue as above: when boot_dev is empty, it becomes "(loop|)" and filters out all candidates, potentially leaving extra_dev empty even after iSCSI login.

    extra_dev="$(sudo lsblk --noheading --list \
      --nodeps --output KNAME \
      | grep -Ev "(loop|${boot_dev})" | head -1)"

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+			vacName := "updated-parameters"
+			patchData := []byte(fmt.Sprintf(`{"spec":{"volumeAttributesClassName":"%s"}}`, vacName))
+			_, err = f.ClientSet.CoreV1().PersistentVolumeClaims(pvc.Namespace).Patch(
+				context.TODO(), pvc.Name, "application/strategic-merge-patch+json", patchData, metav1.PatchOptions{})
+			if err != nil {


+	buf := make([]byte, 1<<16)
+	size := runtime.Stack(buf, true)
+
+	klog.InfoDepth(1, Log(ctx, string(buf[:size])))


+  boot_dev="$(sudo lsblk --noheading --list --output MOUNTPOINT,PKNAME | grep boot | awk '{print $2}' | sort -u)"
+  extra_dev="$(sudo lsblk --noheading --list --nodeps --output KNAME | grep -Ev "(loop|${boot_dev})" | head -1)"


+	// Update the export with new clients list
+	if clients != "" {
+		clientAddrs := strings.Split(clients, ",")
+		exportInfo.Clients = []nfs.ClientInfo{
+			{
+				Addresses:  clientAddrs,
+				AccessType: "rw",
+				Squash:     nfs.NoneSquash,
+			},
+		}
+	}


+---
+# Acceptance quick e2e: minikube + Rook Ceph smoke on every PR.
+# Complements (does NOT replace) CentOS mini-e2e via ok-to-test.
+name: e2e-minikube-acceptance


+            echo "ROOK_CEPH_CLUSTER_IMAGE=\
+          ${ROOK_CEPH_CLUSTER_IMAGE}"


+	rbdImg.Pool = ri.Pool
+	rbdImg.RadosNamespace = ri.RadosNamespace
+	rbdImg.Monitors = ri.Monitors
+	rbdImg.RbdImageName = ri.RbdImageName
+	rbdImg.conn = ri.conn.Copy()
+


mergify · 2026-08-07T11:08:34Z

This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏

getReqID() had no case for *csi.NodeGetVolumeStatsRequest, so the gRPC interceptor could not inject a ReqID into the logger for NodeGetVolumeStats calls. Add the request type so its VolumeId is used as the ReqID, making those log lines traceable like the other Node RPCs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Praveen M <m.praveen@ibm.com>

black-dragon74 · 2026-09-09T10:37:22Z

Should rebase on top of: #6536

DiffIterate with WholeObject relies on the object-map, fast-diff and exclusive-lock features for fast used-bytes calculation. Images created before these features were part of the default feature set (e.g. older PVC clones) cause DiffIterate to fall back to reading actual data blocks, making NodeGetVolumeStats extremely slow. Walk the entire parent clone chain and check for the object-map, fast-diff and exclusive-lock features before calling DiffIterate. Fall back to returning only the block device size when any image in the chain lacks these features. Signed-off-by: Praveen M <m.praveen@ibm.com>

NodeGetVolumeStats is polled by kubelet every ~2 minutes. Cache the result of allImageInChainHasFeature in a sync.Map on NodeServer keyed by volume ID so that subsequent calls skip credential fetch, connection setup, and the full parent chain walk. The cache is evicted on NodeUnstageVolume. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Praveen M <m.praveen@ibm.com>

Add an e2e test that creates a block PVC with only the layering feature (no object-map) and verifies that kubelet volume stats metrics are still reported. This validates that DiffIterate is correctly skipped for images without object-map, falling back to block device size only. Signed-off-by: Praveen M <m.praveen@ibm.com> (cherry picked from commit d22b222)

Add the missing deferred Destroy() in each. Signed-off-by: Praveen M <m.praveen@ibm.com>

mergify Bot added the component/rbd Issues related to RBD label Jul 31, 2026

iPraveenParihar force-pushed the fix/rbd/skip-diffiterate-without-objectmap branch 4 times, most recently from 92db718 to d22b222 Compare August 3, 2026 07:11

iPraveenParihar force-pushed the fix/rbd/skip-diffiterate-without-objectmap branch from f4d4081 to 7d58aa9 Compare August 6, 2026 08:04

iPraveenParihar marked this pull request as ready for review August 6, 2026 08:15

iPraveenParihar requested review from a team as code owners August 6, 2026 08:15

Rakshith-R requested a lite review from Copilot August 6, 2026 08:18

Copilot started reviewing on behalf of Rakshith-R August 6, 2026 08:19 View session

Copilot AI reviewed Aug 6, 2026

View reviewed changes

iPraveenParihar force-pushed the fix/rbd/skip-diffiterate-without-objectmap branch from 7d58aa9 to fd4c0ce Compare August 7, 2026 08:25

iPraveenParihar force-pushed the fix/rbd/skip-diffiterate-without-objectmap branch from 501910f to ee70248 Compare August 17, 2026 09:51

iPraveenParihar marked this pull request as draft August 24, 2026 09:50

iPraveenParihar force-pushed the fix/rbd/skip-diffiterate-without-objectmap branch 5 times, most recently from 7710baa to 14f2e4b Compare August 27, 2026 04:51

iPraveenParihar force-pushed the fix/rbd/skip-diffiterate-without-objectmap branch 2 times, most recently from 535506f to bdd38b4 Compare September 1, 2026 07:37

iPraveenParihar marked this pull request as ready for review September 2, 2026 04:49

iPraveenParihar force-pushed the fix/rbd/skip-diffiterate-without-objectmap branch from bdd38b4 to 2500675 Compare September 9, 2026 11:55

iPraveenParihar and others added 2 commits September 9, 2026 17:37

iPraveenParihar added 2 commits September 9, 2026 17:40

cleanup: release copied rbd connections to avoid leaks

6feb1ba

Add the missing deferred Destroy() in each. Signed-off-by: Praveen M <m.praveen@ibm.com>

iPraveenParihar force-pushed the fix/rbd/skip-diffiterate-without-objectmap branch from 2500675 to 6feb1ba Compare September 9, 2026 12:10

Navigation Menu

Uh oh!

rbd: skip DiffIterate for block volume stats without object-map - #6442

rbd: skip DiffIterate for block volume stats without object-map#6442
iPraveenParihar wants to merge 5 commits into
ceph:develfrom
iPraveenParihar:fix/rbd/skip-diffiterate-without-objectmap

iPraveenParihar commented Jul 31, 2026

Uh oh!

iPraveenParihar commented Aug 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

mergify Bot commented Aug 7, 2026

Uh oh!

black-dragon74 commented Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		boot_dev="$(sudo lsblk --noheading --list --output MOUNTPOINT,PKNAME \| grep boot \| awk '{print $2}' \| sort -u)"
		extra_dev="$(sudo lsblk --noheading --list --nodeps --output KNAME \| grep -Ev "(loop\|${boot_dev})" \| head -1)"

Uh oh!

Conversation

iPraveenParihar commented Jul 31, 2026

Describe what this PR does

Uh oh!

iPraveenParihar commented Aug 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

mergify Bot commented Aug 7, 2026

Uh oh!

black-dragon74 commented Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants