Setting csi controller plugin hostNetwork for data replication seperation #498

rohan47 · 2025-11-26T05:49:03Z

client-operator will receive information for enabling host network for csi controller plugins if data replicaion seperation is enabled and will configure it for the csi drivers

leelavg

LGTM, let's not use "HostNet" we are using "HostNetwork" in rest of the places.

rewantsoni · 2025-11-26T07:58:35Z

internal/controller/operatorconfigmap_controller.go

 	for i := range storageClients.Items {
 		if storageClients.Items[i].Status.RbdDriverRequirements != nil {
 			enableRbdDriver = true
+			useHostNetForRbdCtrlPlugin = storageClients.Items[i].Status.RbdDriverRequirements.CtrlPluginHostNet


Could there be a situation where we have two storage clients, with one using hostNetwork and the other not? What would be the expected value for useHostNetForRbdCtrlPlugin value in that case?

Madhu-1 · 2025-11-26T08:22:28Z

bundle/manifests/ocs-client-operator-config_v1_configmap.yaml

 apiVersion: v1
 data:
  generateRbdOMapInfo: "true"
-  topologyFailureDomainLabels: ""


why is this removed?

This is automatically getting removed while doing make bundle

@leelavg dont we have CI job to catch this one?

looks like without removing this verify_generated_changes is failing

that line was added in https://github.com/red-hat-storage/ocs-client-operator/pull/495/files
and should also be added in https://github.com/red-hat-storage/ocs-client-operator/blob/main/config/manager/kustomization.yaml#L8

Shall I add the chage mentioned in the above comment in this PR or have a new one?
make bundle removes this line in main branch as well

we need to make the CI happy, please add it as part of this PR

Madhu-1 · 2025-11-26T08:24:56Z

internal/controller/operatorconfigmap_controller.go

 	for i := range storageClients.Items {
 		if storageClients.Items[i].Status.RbdDriverRequirements != nil {
 			enableRbdDriver = true
+			useHostNetForRbdCtrlPlugin = storageClients.Items[i].Status.RbdDriverRequirements.CtrlPluginHostNet


if storageClients.Items[i].Status.RbdDriverRequirements.CtrlPluginHostNet{ useHostNetForRbdCtrlPlugin=true } we should use host networking if one client is having this value set to true

Madhu-1 · 2025-11-26T13:08:58Z

internal/controller/operatorconfigmap_controller.go

 	enableCephFsDriver := c.shouldEnableDriver(enableCephFsDriverKey)
 	enableNfsDriver := c.shouldEnableDriver(enableNfsDriverKey)

+	var useHostNetForRbdCtrlPlugin, useHostNetForCephFsCtrlPlugin, useHostNetForNfsCtrlPlugin *bool


why do we need this as point to bool? lets keep it bool only

Madhu-1

/lgtm

Madhu-1 · 2025-11-26T14:35:18Z

/hold

Madhu-1 · 2025-11-26T14:35:44Z

adding hold label to have a chance for others to review. @leelavg @rewantsoni PTAL

rewantsoni · 2025-11-26T14:49:24Z

internal/controller/operatorconfigmap_controller.go

+			if storageClients.Items[i].Status.RbdDriverRequirements.CtrlPluginHostNetwork != nil &&
+				*storageClients.Items[i].Status.RbdDriverRequirements.CtrlPluginHostNetwork {
+				useHostNetForRbdCtrlPlugin = ptr.Deref(storageClients.Items[i].Status.RbdDriverRequirements.CtrlPluginHostNetwork, false)
+			}


Can this be simplified to this?

Suggested change

if storageClients.Items[i].Status.RbdDriverRequirements.CtrlPluginHostNetwork != nil &&

*storageClients.Items[i].Status.RbdDriverRequirements.CtrlPluginHostNetwork {

useHostNetForRbdCtrlPlugin = ptr.Deref(storageClients.Items[i].Status.RbdDriverRequirements.CtrlPluginHostNetwork, false)

}

if useHostNetwork:=storageClients.Items[i].Status.RbdDriverRequirements.CtrlPluginHostNetwork; useHostNetwork != nil {

useHostNetForRbdCtrlPlugin = ptr.Deref(useHostNetwork, false)

}

Or maybe something like to make sure if it is enabled for even one client it will be enabled for all clients?

Suggested change

if storageClients.Items[i].Status.RbdDriverRequirements.CtrlPluginHostNetwork != nil &&

*storageClients.Items[i].Status.RbdDriverRequirements.CtrlPluginHostNetwork {

useHostNetForRbdCtrlPlugin = ptr.Deref(storageClients.Items[i].Status.RbdDriverRequirements.CtrlPluginHostNetwork, false)

}

if useHostNetwork:=storageClients.Items[i].Status.RbdDriverRequirements.CtrlPluginHostNetwork; useHostNetwork != nil {

useHostNetForRbdCtrlPlugin = useHostNetForRbdCtrlPlugin || ptr.Deref(useHostNetwork, false)

}

second option looks clean, updated the changes

fetch the provider api changes required for csi controller plugin host network Signed-off-by: Rohan Gupta <[email protected]>

added csi ctrl plugin host network setting per driver in storage client status, based on this setting operator configmap controller will enable/disable host network for csi controllers Signed-off-by: Rohan Gupta <[email protected]>

csi controller plugin host network will be configured for drivers based on the information send via provider server Signed-off-by: Rohan Gupta <[email protected]>

Signed-off-by: Rohan Gupta <[email protected]>

rohan47 · 2025-11-26T15:59:36Z

/hold
Testing the changes again

rohan47 · 2025-11-26T16:28:42Z

Verified the changes

rewantsoni · 2025-11-26T16:46:14Z

/lgtm

openshift-ci · 2025-11-26T16:46:21Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Madhu-1, rewantsoni, rohan47

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [Madhu-1]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

rohan47 · 2025-11-27T06:08:39Z

/unhold

pull-request-size bot added the size/L label Nov 26, 2025

leelavg reviewed Nov 26, 2025

View reviewed changes

rewantsoni reviewed Nov 26, 2025

View reviewed changes

Madhu-1 requested changes Nov 26, 2025

View reviewed changes

openshift-ci bot assigned Madhu-1 Nov 26, 2025

rohan47 force-pushed the drs_hostnet branch 4 times, most recently from bb5fb24 to 86f1495 Compare November 26, 2025 12:33

Madhu-1 reviewed Nov 26, 2025

View reviewed changes

rohan47 force-pushed the drs_hostnet branch from 86f1495 to 90d0959 Compare November 26, 2025 14:22

Madhu-1 reviewed Nov 26, 2025

View reviewed changes

openshift-ci bot added lgtm approved labels Nov 26, 2025

openshift-ci bot added the do-not-merge/hold label Nov 26, 2025

rewantsoni reviewed Nov 26, 2025

View reviewed changes

rohan47 added 4 commits November 26, 2025 21:25

godeps: fetch the latest provider api changes for ocs-operator

14bcf99

fetch the provider api changes required for csi controller plugin host network Signed-off-by: Rohan Gupta <[email protected]>

api: added ctrlPluginHostNw setting in client status

d55ff37

added csi ctrl plugin host network setting per driver in storage client status, based on this setting operator configmap controller will enable/disable host network for csi controllers Signed-off-by: Rohan Gupta <[email protected]>

controllers: setting csi ctrl plugin hostNetwork per driver

96ebcea

csi controller plugin host network will be configured for drivers based on the information send via provider server Signed-off-by: Rohan Gupta <[email protected]>

bundle: added ctrlPluginHostNetwor in drivers requirements

cb1f938

Signed-off-by: Rohan Gupta <[email protected]>

rohan47 force-pushed the drs_hostnet branch from 90d0959 to cb1f938 Compare November 26, 2025 15:57

openshift-ci bot removed the lgtm label Nov 26, 2025

openshift-ci bot assigned rewantsoni Nov 26, 2025

openshift-ci bot added the lgtm label Nov 26, 2025

openshift-ci bot removed the do-not-merge/hold label Nov 27, 2025

openshift-merge-bot bot merged commit 68ce318 into red-hat-storage:main Nov 27, 2025
12 checks passed

Setting csi controller plugin hostNetwork for data replication seperation #498

Setting csi controller plugin hostNetwork for data replication seperation #498

Uh oh!

Conversation

rohan47 commented Nov 26, 2025

Uh oh!

leelavg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rohan47 Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Madhu-1 left a comment

Choose a reason for hiding this comment

Uh oh!

Madhu-1 commented Nov 26, 2025

Uh oh!

Madhu-1 commented Nov 26, 2025

Uh oh!

rewantsoni Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rohan47 commented Nov 26, 2025

Uh oh!

rohan47 commented Nov 26, 2025

Uh oh!

rewantsoni commented Nov 26, 2025

Uh oh!

openshift-ci bot commented Nov 26, 2025

Uh oh!

rohan47 commented Nov 27, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rohan47 Nov 26, 2025 •

edited

Loading

rewantsoni Nov 26, 2025 •

edited

Loading