fix: return success when backend image/volume is missing on unpublish by iPraveenParihar · Pull Request #6589 · ceph/ceph-csi

iPraveenParihar · 2026-09-28T07:39:03Z

Describe what this PR does

When an RBD image or CephFS subvolume is removed manually,
its controller unpublish request can fail with a not-found error.
That can leave the external-attacher finalizer in place and prevent
detach from completing. Return success when the image or subvolume,
journal key, or pool is missing, so the attacher can finish cleanup.

Fixes: #6586

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

Madhu-1 · 2026-09-28T07:48:21Z

 	volOptions, _, err := store.NewVolumeOptionsFromVolID(ctx, volumeId, nil, secrets, cs.ClusterName)
 	if err != nil {
+		if errors.Is(err, cerrors.ErrVolumeNotFound) ||
+			errors.Is(err, util.ErrKeyNotFound) ||


do we need KeyNotFound error here? same for rbd as well

First we get the imageID from the journal and then getImageInfo(). If the journal object/key state is messed up (manually or other reason) there is no way we can getImageInfo. Similar for CephFS

we still need to consider new keys or few less important keys not required for Unpublish/delete might be present for new volumes not for old one.

Copilot

Copilot review overview

🟡 Changes recommended

Regression tests are needed to protect the new error-handling behavior in both drivers.

Review effort: Balanced
Findings: 2

Open (2)

Add CephFS unpublish regression tests for not-found cleanup errors · New
Test absorbed not-found errors while preserving Internal failures · New

What changed in this PR

Makes controller unpublish idempotent when RBD or CephFS backend resources are missing, allowing attachment cleanup to complete.

Changes:

Treat missing images, subvolumes, journal keys, and pools as successful unpublishes.
Log skipped cleanup for missing volumes.

File	Description
`internal/rbd/controllerserver.go`	Handles missing RBD resources successfully.
`internal/cephfs/controllerserver.go`	Handles missing CephFS resources successfully.

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

+		if errors.Is(err, cerrors.ErrVolumeNotFound) ||
+			errors.Is(err, util.ErrKeyNotFound) ||
+			errors.Is(err, util.ErrPoolNotFound) {


+		if errors.Is(err, rbderrors.ErrImageNotFound) ||
+			errors.Is(err, util.ErrKeyNotFound) ||
+			errors.Is(err, util.ErrPoolNotFound) {


nixpanic · 2026-09-28T10:46:59Z

@iPraveenParihar , please add the backport-to-... labels.

nixpanic · 2026-09-28T11:25:40Z

/queue

ceph-csi-bot · 2026-09-28T11:26:01Z

@Mergifyio rebase

mergify · 2026-09-28T11:26:22Z

rebase

❌ This pull request comes from a fork and cannot be rebased

Details

GitHub refuses an OAuth token on its rebase API for a fork, so rebasing one means impersonating a GitHub user to force-push the contributor's branch. Mergify does not do that.

Use the update action or the @mergifyio update command instead: it brings the pull request up to date by merging the base branch into it, and needs no impersonation. It only has something to do when the pull request is behind its base branch, so if what the branch needs is a linear history, its author has to rebase it themselves.

iPraveenParihar · 2026-09-28T11:42:25Z

@Mergifyio rebase

mergify · 2026-09-28T11:42:33Z

rebase

❌ This pull request comes from a fork and cannot be rebased

Details

GitHub refuses an OAuth token on its rebase API for a fork, so rebasing one means impersonating a GitHub user to force-push the contributor's branch. Mergify does not do that.

Use the update action or the @mergifyio update command instead: it brings the pull request up to date by merging the base branch into it, and needs no impersonation. It only has something to do when the pull request is behind its base branch, so if what the branch needs is a linear history, its author has to rebase it themselves.

iPraveenParihar · 2026-09-28T11:42:45Z

/queue

ceph-csi-bot · 2026-09-28T11:43:05Z

@Mergifyio rebase

mergify · 2026-09-28T11:43:09Z

rebase

❌ This pull request comes from a fork and cannot be rebased

Details

GitHub refuses an OAuth token on its rebase API for a fork, so rebasing one means impersonating a GitHub user to force-push the contributor's branch. Mergify does not do that.

Use the update action or the @mergifyio update command instead: it brings the pull request up to date by merging the base branch into it, and needs no impersonation. It only has something to do when the pull request is behind its base branch, so if what the branch needs is a linear history, its author has to rebase it themselves.

black-dragon74 · 2026-09-28T12:18:19Z

@Mergifyio queue

mergify · 2026-09-28T12:18:26Z

Merge Queue Status

✅ Entered queue — 2026-09-29 07:11 UTC · Rule: default · triggered by @black-dragon74 with the @mergifyio queue command
❌ Checks failed · in-place
🚫 Left the queue — 2026-09-29 07:12 UTC · at c8abd5961d37a6c669fb9d8b79b8cb7be0f9ef29

This pull request spent 50 seconds in the queue, with no time running CI.

Reason

The merge conditions cannot be satisfied due to failing checks

Mergify Merge Protections
DCO

Hint

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio queue comment.

Required conditions to merge

github-review-approved [🛡 GitHub branch protection]
- fix: return success when backend image/volume is missing on unpublish #6589

Requeued — the merge queue status continues in this comment ↓.

ceph-csi-bot · 2026-09-28T12:19:25Z

/test ci/centos/upgrade-tests-cephfs

ceph-csi-bot · 2026-09-28T12:19:26Z

/test ci/centos/mini-e2e/k8s-1.37/nfs

ceph-csi-bot · 2026-09-28T12:19:26Z

/test ci/centos/upgrade-tests-rbd

ceph-csi-bot · 2026-09-28T12:19:26Z

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

ceph-csi-bot · 2026-09-28T12:19:26Z

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

ceph-csi-bot · 2026-09-29T07:11:54Z

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

ceph-csi-bot · 2026-09-29T07:11:54Z

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

ceph-csi-bot · 2026-09-29T07:11:54Z

/test ci/centos/upgrade-tests-rbd

ceph-csi-bot · 2026-09-29T07:11:55Z

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

ceph-csi-bot · 2026-09-29T07:11:55Z

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

ceph-csi-bot · 2026-09-29T07:11:55Z

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

ceph-csi-bot · 2026-09-29T07:11:55Z

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

iPraveenParihar · 2026-09-29T07:36:36Z

😞 Why did Mergify add queue label in middle of ongoing CI runs? Now it runs all again.

nixpanic · 2026-09-29T08:49:58Z

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

Failed to install the VM to run the tests, logs.

black-dragon74 · 2026-09-29T09:19:40Z

/test ci/centos/mini-e2e/k8s-1.37/nfs

Flaked twice.

black-dragon74 · 2026-09-29T09:39:48Z

Both failing e2e report:

cico-workspace-q6p50 seems to be removed or offline (hudson.remoting.RequestAbortedException: java.nio.channels.ClosedChannelException); will wait for 5 min 0 sec for it to come back online

black-dragon74 · 2026-09-29T09:49:01Z

/retest ci/centos/k8s-e2e-external-storage/1.34
/retest ci/centos/mini-e2e/k8s-1.34
/retest ci/centos/mini-e2e/k8s-1.35
/retest ci/centos/mini-e2e/k8s-1.36
/retest ci/centos/mini-e2e/k8s-1.37/nfs

iPraveenParihar · 2026-09-29T13:55:34Z

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

iPraveenParihar · 2026-09-29T13:55:50Z

failed logs

iPraveenParihar · 2026-09-29T17:42:31Z

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

iPraveenParihar · 2026-09-29T17:42:50Z


  I0929 17:09:32.172738   87884 framework.go:487] Found DeleteNamespaceOnFailure=false and current test failed, skipping namespace deletion!
�[38;5;9m• [FAILED] [1246.236 seconds]�[0m
�[0mnvmeof �[38;5;243mTest NVMe CSI �[38;5;9m�[1m[It] create a PVC and delete it�[0m
�[38;5;243m/go/src/github.com/ceph/ceph-csi/e2e/nvmeof.go:136�[0m

  �[38;5;9m[FAILED] Unexpected error:
      <*fmt.wrapError | 0x30e7d1fd5060>: 
      pod with allowed service account "allowed-sa-nvmeof-6430" should have started but failed: failed to get app: client rate limiter Wait returned an error: context deadline exceeded
      {
          msg: "pod with allowed service account \"allowed-sa-nvmeof-6430\" should have started but failed: failed to get app: client rate limiter Wait returned an error: context deadline exceeded",
          err: <*fmt.wrapError | 0x30e7d2845840>{
              msg: "failed to get app: client rate limiter Wait returned an error: context deadline exceeded",
              err: <*fmt.wrapError | 0x30e7d2845820>{
                  msg: "client rate limiter Wait returned an error: context deadline exceeded",
                  err: <context.deadlineExceededError>{},
              },
          },
      }

logs

iPraveenParihar · 2026-09-30T04:10:14Z

log

rpc error: code = InvalidArgument desc = invalid volume context: no listeners found in publish context

😕 it passed on 1.35 and 1.36, and fails on 1.34

I0929 20:35:38.778504   84980 nvmeof-deploy.go:183] configuring StorageClass for gateway "ceph-nvmeof-gateway-75fdcff6dc-jqts8" at 10.244.1.5
I0929 20:35:38.782534   84980 nvmeof-gateway.go:145] gateway Pod is on node "minikube", using networkMask "10.244.0.0/24"

The IP 10.244.1.5 is not inside 10.244.0.0/24, @nixpanic @gadididi Is this flaky test we need to improve or error handling needed in core logic?

Madhu-1 · 2026-09-30T05:40:35Z

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

Madhu-1 · 2026-09-30T05:41:14Z

skipping e2e as e2e passed for other tests, this seems flaky

mergify · 2026-09-30T05:41:21Z

Merge Queue Status

✅ Entered queue — 2026-09-30 05:41 UTC · Rule: default · triggered by merge protections
✅ Checks skipped · PR is already up-to-date
✅ Merged — 2026-09-30 05:41 UTC · at 41bc8f2c2772a2d0e535ac749c5152680a5f0177 · rebase

This pull request spent 10 seconds in the queue, including 1 second running CI.

Required conditions to merge

github-review-approved [🛡 GitHub branch protection]
- fix: return success when backend image/volume is missing on unpublish #6589

gadididi · 2026-10-07T08:55:05Z

log
rpc error: code = InvalidArgument desc = invalid volume context: no listeners found in publish context
😕 it passed on 1.35 and 1.36, and fails on 1.34
I0929 20:35:38.778504   84980 nvmeof-deploy.go:183] configuring StorageClass for gateway "ceph-nvmeof-gateway-75fdcff6dc-jqts8" at 10.244.1.5
I0929 20:35:38.782534   84980 nvmeof-gateway.go:145] gateway Pod is on node "minikube", using networkMask "10.244.0.0/24"
The IP 10.244.1.5 is not inside 10.244.0.0/24, @nixpanic @gadididi Is this flaky test we need to improve or error handling needed in core logic?

Hi @iPraveenParihar sorry about that, I was on PTO.. it is something wrong I do in the nvmeof e2e test, I will fix it ASAP.
need to fix the networkMask value in the nvmeof StorageClass (in the e2e test)

mergify Bot added the bug Something isn't working label Sep 28, 2026

iPraveenParihar marked this pull request as ready for review September 28, 2026 07:46

iPraveenParihar requested review from a team as code owners September 28, 2026 07:46

black-dragon74 approved these changes Sep 28, 2026

View reviewed changes

Madhu-1 reviewed Sep 28, 2026

View reviewed changes

iPraveenParihar added component/cephfs Issues related to CephFS component/rbd Issues related to RBD labels Sep 28, 2026

Madhu-1 requested a balanced review from Copilot September 28, 2026 09:50

Copilot started reviewing on behalf of Madhu-1 September 28, 2026 09:50 View session

Copilot AI reviewed Sep 28, 2026

View reviewed changes

Madhu-1 approved these changes Sep 28, 2026

View reviewed changes

ceph-csi-bot added the queued/rebase label Sep 28, 2026

black-dragon74 added the ok-to-test Label to trigger E2E tests label Sep 28, 2026

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 29, 2026

mergify Bot added dequeued and removed ci/in-progress/e2e This label acts like a guard and prevents Mergify from adding the `ok-to-test` label again. queued labels Sep 29, 2026

Madhu-1 added the ci/skip/e2e skip running e2e CI jobs label Sep 30, 2026

mergify Bot merged commit 41bc8f2 into ceph:devel Sep 30, 2026
51 of 52 checks passed

This was referenced Sep 30, 2026

fix: return success when backend image/volume is missing on unpublish (backport #6589) #6607

Merged

fix: return success when backend image/volume is missing on unpublish (backport #6589) #6608

Merged

Conversation

iPraveenParihar commented Sep 28, 2026

Describe what this PR does

Uh oh!

Madhu-1 Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

iPraveenParihar Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

Madhu-1 Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

nixpanic commented Sep 28, 2026

Uh oh!

nixpanic commented Sep 28, 2026

Uh oh!

ceph-csi-bot commented Sep 28, 2026

Uh oh!

mergify Bot commented Sep 28, 2026

❌ This pull request comes from a fork and cannot be rebased

Uh oh!

iPraveenParihar commented Sep 28, 2026

Uh oh!

mergify Bot commented Sep 28, 2026

❌ This pull request comes from a fork and cannot be rebased

Uh oh!

iPraveenParihar commented Sep 28, 2026

Uh oh!

ceph-csi-bot commented Sep 28, 2026

Uh oh!

mergify Bot commented Sep 28, 2026

❌ This pull request comes from a fork and cannot be rebased

Uh oh!

black-dragon74 commented Sep 28, 2026

Uh oh!

mergify Bot commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge Queue Status

Reason

Hint

Uh oh!

ceph-csi-bot commented Sep 28, 2026

Uh oh!

ceph-csi-bot commented Sep 28, 2026

Uh oh!

ceph-csi-bot commented Sep 28, 2026

Uh oh!

ceph-csi-bot commented Sep 28, 2026

Uh oh!

ceph-csi-bot commented Sep 28, 2026

Uh oh!

ceph-csi-bot commented Sep 29, 2026

Uh oh!

ceph-csi-bot commented Sep 29, 2026

Uh oh!

ceph-csi-bot commented Sep 29, 2026

Uh oh!

ceph-csi-bot commented Sep 29, 2026

Uh oh!

ceph-csi-bot commented Sep 29, 2026

Uh oh!

ceph-csi-bot commented Sep 29, 2026

Uh oh!

ceph-csi-bot commented Sep 29, 2026

Uh oh!

iPraveenParihar commented Sep 29, 2026

Uh oh!

nixpanic commented Sep 29, 2026

Uh oh!

black-dragon74 commented Sep 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

black-dragon74 commented Sep 29, 2026

Uh oh!

mergify Bot commented Sep 28, 2026 •

edited

Loading

black-dragon74 commented Sep 29, 2026 •

edited

Loading

mergify Bot commented Sep 30, 2026 •

edited

Loading