nvmeof: Move listener discovery from CreateVolume to ControllerPublishVolume by gadididi · Pull Request #6483 · ceph/ceph-csi

gadididi · 2026-08-18T14:14:25Z

Listeners are now fetched at ControllerPublishVolume time instead of at CreateVolume time.

Previously, auto-created listeners were queried once in createNVMeoFResources (during CreateVolume) and stored in volumeContext. Since volumeContext is immutable for the life of the volume, this list could go stale if gateways are scale down\up after provisioning..

The query itself (list listener) was racing the gateway's beacon sync on first creation (it comes from the monitor client).
There was first approach to fix it with retry- #6038 - but that PR was not merged, because it was just workaround.

This moves the query to ControllerPublishVolume, via a new reconcileListeners function, so listeners are fetched fresh on every publish/attach rather than once at creation.
publishResources now returns both HostNQN and Listeners (via a new PublishResult struct), and populatePublishContext writes the reconciled listeners into publishContext instead of leaving them in volumeContext.

The node plugin (getNvmeConnection) now reads listeners from publishContext instead of volumeContext to match.

If the fresh listener query fails during publish, publishResources falls back to the listeners stored in volumeContext at creation time rather than failing the call outright.

Depends-on: #6483

nixpanic

please add a note to PendingReleaseNotes.md and update the documentation/examples, mentioning that the listeners in the StorageClass is optional.

gadididi · 2026-08-19T11:32:59Z

some nit, @nixpanic in PendingReleaseNotes.md the linter failed before when I added paragraph with the number 2 - because according to https://github.com/markdownlint/markdownlint/blob/main/docs/RULES.md#md029---ordered-list-item-prefix
this is not ordered list by default.
possible to change it in scripts/mdl-style.rb by adding the line rule 'MD029', :style => "ordered" and this will allow natural 1. 2. 3. numbering I think..

Madhu-1 · 2026-08-24T07:32:04Z

-      "port": "4420",
-      "hostname": "cephnvme-vm9"
-    }]
+  # Listener discovery: specify EITHER listeners OR networkMask (XOR),not both.


can we have check to ensure if both are set we are rejecting the request in CreateVolume?

I already have it here:

ceph-csi/internal/nvmeof/controller/controllerserver.go

Lines 532 to 556 in f4ce351

func validateCreateVolumeRequest(req *csi.CreateVolumeRequest) error {

// Validate required parameters

params := req.GetParameters()

if params["nvmeofGatewayAddress"] == "" {

return errors.New("missing required parameter nvmeofGatewayAddress")

}

// Validate listeners JSON if provided

countOfListeners, err := validateListenersParameter(params["listeners"])

if err != nil {

return err

}

// Validate network mask if provided

err = validateNetworkMask(params["networkMask"])

if err != nil {

return fmt.Errorf("invalid network mask parameter: %w", err)

}

networkMask := params["networkMask"]

// Must have EITHER listeners XOR networkMask

if countOfListeners == 0 && networkMask == "" {

return errors.New("must specify either 'listeners' xor 'networkMask', but got neither")

}

if countOfListeners > 0 && networkMask != "" {

return errors.New("must specify either 'listeners' xor 'networkMask', but got both")

}

Madhu-1 · 2026-08-24T07:32:57Z

+		return nil, fmt.Errorf("failed to list auto-created listeners: %w", err)
+	}
+	currentListeners := nvmeof.ConvertListenersFromProto(autoListeners.GetListeners())
+	log.DebugLog(ctx, "Retrieved %d auto-created listeners", len(currentListeners))


we are already logging this in line 1042, can we remove this debug log?

gadididi · 2026-08-24T10:39:26Z

this https://github.com/ceph/ceph-csi/compare/419aaf18e99c74b12bd2b7a01699b3b32be86fd5..d1b922a667cce7f320b203ddae33a14de69c12f5 - is rebased onto devel

gadididi · 2026-08-26T11:49:11Z

@mergify rebase

mergify · 2026-08-26T11:49:16Z

rebase

🛑 The pull request rule doesn't match anymore

Details

This action has been cancelled.

nixpanic · 2026-08-26T12:29:58Z

Use the /queue command to start testing+merging this when #6483 is finished.

Pull request has been modified.

gadididi · 2026-08-31T09:46:03Z

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

gadididi · 2026-08-31T09:55:14Z

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

gadididi · 2026-08-31T11:04:07Z

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

gadididi · 2026-08-31T17:13:49Z

@mergify rebase

mergify · 2026-08-31T17:13:57Z

rebase

🛑 The pull request rule doesn't match anymore

Details

This action has been cancelled.

gadididi · 2026-08-31T17:15:02Z

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

gadididi · 2026-08-31T17:46:02Z

@nixpanic @Madhu-1
nvmeof test succeeded to run : https://jenkins-ceph-csi.apps.ocp.cloud.ci.centos.org/blue/organizations/jenkins/mini-e2e_k8s-1.37-nvmeof/detail/mini-e2e_k8s-1.37-nvmeof/12/pipeline/

I think it is ready for rereview.
I fixed in the 2 last commits:

the e2e failed before because the StorageClass we created in the e2e test contained networkmask and listener (after I changed the StorageClass.yaml example.. ) so I handle it here - c6f69bc
bump to GW version 1.6( == tentacle 9.1) instead of 1.5 (== tentacle 9.0) because networkmask feature is not supported in 1.5 (the commit - c39c863 )

thanks both for your help 🙂

gadididi · 2026-09-02T07:00:17Z

@mergify rebase

mergify · 2026-09-02T07:00:26Z

rebase

🛑 The pull request rule doesn't match anymore

Details

This action has been cancelled.

Copilot

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

gadididi · 2026-09-09T08:38:33Z

@mergify rebase

mergify · 2026-09-09T08:38:39Z

rebase

🛑 The pull request rule doesn't match anymore

Details

This action has been cancelled.

nixpanic · 2026-09-10T07:39:17Z

            - name: CEPH_ARGS
              value: --mon-host $(CEPH_MON_HOST) --keyring /etc/ceph/keyring
-          image: quay.io/ceph/nvmeof:1.5
+          image: quay.io/ceph/nvmeof:1.6


"tentacle 9.1+" and "9.0" are not upstream Ceph versions, please correct the commit message.

network mask feature was added in nvmeof gw version 1.6+. 1.5 does not include it. Signed-off-by: gadi-didi <gadi.didi@ibm.com>

gadididi · 2026-09-10T11:46:54Z

@mergify rebase

mergify · 2026-09-10T11:47: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.

gadididi · 2026-09-10T11:48:05Z

@mergify rebase

mergify · 2026-09-10T11:48: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.

gadididi · 2026-09-10T11:48:29Z

@Mergifyio update

mergify · 2026-09-10T11:48:44Z

update

❌ Pull request can't be updated with latest base branch changes

Details

This pull request seems to come from a fork, and Mergify needs the author's permission to update its branch.
The author needs to enable "Allow edits from maintainers" on this pull request, or update the branch manually.

nixpanic · 2026-09-11T10:17:59Z

+			nmParams := map[string]string{"pool": nvmeofPool}
+			nmPolicy := v1.PersistentVolumeReclaimDelete
+			createNVMeoFStorageClass(f, nmSCName, nmOptions, nmParams, nmPolicy, true)
+			ginkgo.DeferCleanup(deleteNVMeofStorageClass, f, nmSCName)


should we use ginkgo.DeferCleanup() in more places instead of just defer?

mergify Bot added the component/nvme-of Issues and PRs related to NVMe-oF. label Aug 18, 2026

gadididi changed the title ~~[WIP] nvmeof: move list listener to publish volume~~ [WIP] nvmeof: Move listener discovery from CreateVolume to ControllerPublishVolume Aug 19, 2026

gadididi requested a review from nixpanic August 19, 2026 06:45

gadididi changed the title ~~[WIP] nvmeof: Move listener discovery from CreateVolume to ControllerPublishVolume~~ nvmeof: Move listener discovery from CreateVolume to ControllerPublishVolume Aug 19, 2026

gadididi marked this pull request as ready for review August 19, 2026 06:45

gadididi requested review from a team as code owners August 19, 2026 06:45

gadididi self-assigned this Aug 19, 2026

nixpanic added this to the release-v3.18 milestone Aug 19, 2026

nixpanic reviewed Aug 19, 2026

View reviewed changes

Comment thread internal/nvmeof/controller/controllerserver.go Outdated

Comment thread internal/nvmeof/nodeserver/nodeserver.go

gadididi force-pushed the nvmeof/move_listlistener_to_publishvolume branch from efc2b0b to f92a7e1 Compare August 19, 2026 10:49

gadididi mentioned this pull request Aug 19, 2026

nvmeof: Remove the fetching listener from volumeContext #6485

Open

gadididi force-pushed the nvmeof/move_listlistener_to_publishvolume branch 3 times, most recently from d0cbbff to 08b1451 Compare August 19, 2026 11:29

gadididi requested review from Madhu-1 and nixpanic August 19, 2026 11:37

Madhu-1 reviewed Aug 24, 2026

View reviewed changes

gadididi force-pushed the nvmeof/move_listlistener_to_publishvolume branch from 08b1451 to 419aaf1 Compare August 24, 2026 09:23

gadididi requested a review from Madhu-1 August 24, 2026 10:36

gadididi force-pushed the nvmeof/move_listlistener_to_publishvolume branch from 419aaf1 to d1b922a Compare August 24, 2026 10:38

nixpanic previously approved these changes Aug 25, 2026

View reviewed changes

nixpanic requested a review from a team August 25, 2026 16:08

Madhu-1 previously approved these changes Aug 26, 2026

View reviewed changes

ceph-csi-bot force-pushed the nvmeof/move_listlistener_to_publishvolume branch from d1b922a to 4caecb6 Compare August 26, 2026 11:49

ceph-csi-bot force-pushed the nvmeof/move_listlistener_to_publishvolume branch from b014111 to c39c863 Compare August 31, 2026 17:14

gadididi requested a review from Madhu-1 August 31, 2026 17:41

Copilot AI lite review requested due to automatic review settings September 2, 2026 07:00

ceph-csi-bot force-pushed the nvmeof/move_listlistener_to_publishvolume branch from c39c863 to 92c1a7e Compare September 2, 2026 07:00

Copilot AI reviewed Sep 2, 2026

ceph-csi-bot force-pushed the nvmeof/move_listlistener_to_publishvolume branch from 92c1a7e to e0ff853 Compare September 9, 2026 08:38

nixpanic reviewed Sep 10, 2026

View reviewed changes

e2e: bump nvmeof gw version to 1.6

9550aa8

network mask feature was added in nvmeof gw version 1.6+. 1.5 does not include it. Signed-off-by: gadi-didi <gadi.didi@ibm.com>

gadididi force-pushed the nvmeof/move_listlistener_to_publishvolume branch from e0ff853 to 9550aa8 Compare September 10, 2026 11:46

gadididi requested a review from nixpanic September 10, 2026 11:47

nixpanic approved these changes Sep 11, 2026

View reviewed changes

nixpanic requested a review from a team September 11, 2026 12:24

	func validateCreateVolumeRequest(req *csi.CreateVolumeRequest) error {
	// Validate required parameters
	params := req.GetParameters()
	if params["nvmeofGatewayAddress"] == "" {
	return errors.New("missing required parameter nvmeofGatewayAddress")
	}

	// Validate listeners JSON if provided
	countOfListeners, err := validateListenersParameter(params["listeners"])
	if err != nil {
	return err
	}
	// Validate network mask if provided
	err = validateNetworkMask(params["networkMask"])
	if err != nil {
	return fmt.Errorf("invalid network mask parameter: %w", err)
	}
	networkMask := params["networkMask"]
	// Must have EITHER listeners XOR networkMask
	if countOfListeners == 0 && networkMask == "" {
	return errors.New("must specify either 'listeners' xor 'networkMask', but got neither")
	}
	if countOfListeners > 0 && networkMask != "" {
	return errors.New("must specify either 'listeners' xor 'networkMask', but got both")
	}

Uh oh!

Conversation

gadididi commented Aug 18, 2026 • edited by nixpanic Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nixpanic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gadididi commented Aug 19, 2026

Uh oh!

Madhu-1 Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

gadididi Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

Madhu-1 Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

gadididi Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

gadididi commented Aug 24, 2026

Uh oh!

gadididi commented Aug 26, 2026

Uh oh!

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

Uh oh!

🛑 The pull request rule doesn't match anymore

Uh oh!

nixpanic commented Aug 26, 2026

Uh oh!

gadididi commented Aug 31, 2026

Uh oh!

gadididi commented Aug 31, 2026

Uh oh!

gadididi commented Aug 31, 2026

Uh oh!

gadididi commented Aug 31, 2026

Uh oh!

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

Uh oh!

🛑 The pull request rule doesn't match anymore

Uh oh!

gadididi commented Aug 31, 2026

Uh oh!

gadididi commented Aug 31, 2026

Uh oh!

gadididi commented Sep 2, 2026

Uh oh!

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

Uh oh!

🛑 The pull request rule doesn't match anymore

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

gadididi commented Sep 9, 2026

Uh oh!

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

Uh oh!

🛑 The pull request rule doesn't match anymore

Uh oh!

nixpanic Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

gadididi Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

gadididi commented Sep 10, 2026

Uh oh!

mergify Bot commented Sep 10, 2026

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

Uh oh!

gadididi commented Sep 10, 2026

Uh oh!

mergify Bot commented Sep 10, 2026

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

gadididi commented Aug 18, 2026 •

edited by nixpanic

Loading

mergify Bot commented Aug 26, 2026 •

edited

Loading

mergify Bot commented Aug 31, 2026 •

edited

Loading

mergify Bot commented Sep 2, 2026 •

edited

Loading

mergify Bot commented Sep 9, 2026 •

edited

Loading