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()
+


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. Walk the entire parent clone chain and check for the object-map feature before calling DiffIterate. Fall back to returning only the block device size when any image in the chain lacks object-map. Use a lightweight read-only open with only GetFeatures and GetParent calls instead of the heavy getImageInfo path. When a parent image is in trash, openReadOnly() fails with ErrImageNotFound. Return false (feature absent) instead of true so that DiffIterate is skipped and getBlockMetrics is used. Signed-off-by: Praveen M <m.praveen@ibm.com>

Squashed cherry-pick of Rakshith-R#303: - nvmeof: add volume cloning capability - e2e: add clone test for NVMeoF driver - nfs: make `clients` a mutable-parameter - e2e: add NFS test for modifying clients via VolumeAttributesClass - e2e: log clients and exports if match is not found - e2e: delete NFS StorageClass in case it already exists - util: log stacktrace once on slow operation when tracing is enabled - e2e: add acceptance minikube quick e2e smoke gate Signed-off-by: Praveen M <m.praveen@ibm.com> (cherry picked from commit b33dac6)

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)

- imageChainHasFeature: defer Destroy on the copied connection and destroy ioctx after each loop iteration - checkImageChainHasFeature: defer Destroy on the copied connection - getParent: destroy copied connection when getImageInfo fails Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

NodeGetVolumeStats is polled by kubelet every ~2 minutes. Cache the result of imageChainHasFeature 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. Also fix leaked connection and ioctx resources in imageChainHasFeature, checkImageChainHasFeature, and getParent. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

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? 🙏

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 added 3 commits August 7, 2026 13:54

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

iPraveenParihar and others added 2 commits August 7, 2026 16:25

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants