Deploy csi-addons sidecar with pod network by black-dragon74 · Pull Request #253 · ceph/ceph-csi-operator

black-dragon74 · 2025-06-16T08:24:11Z

This patch introduces a set of changes that allows the csi-addons sidecars to run without host networking. Here's a brief of the changes (separated into their own commits):

Create a new DaemonSet for csi-addons sidecar that communicates with CSI nodeplugin.
- The UDS is already on a hostpath which is shared across these two daemon sets.
Modify the provisioner deployments to use hostpath for UDS located at:kubelet_dir/plugin/driverid/ctrl-plugin
Create a separate deployment for CSI Addons to pair with provisioner deployments
- The csi-addons to provisioner pairing is ensured using pod affinities.

This patch introduces a new DaemonSet for csi-addons operations that uses pod network instead of the host networking. Signed-off-by: Niraj Yadav <niryadav@redhat.com>

This patch modifies the provisioner deployments to use hostpath for CSI sockets. This is done to (later) enable CSI Addons sidecar to run as a separate deployment. Signed-off-by: Niraj Yadav <niryadav@redhat.com>

This patch deploys csi addons sidecar container into its own deployment that shares the hostpath for UDS with the CSI provisioner. This model enables CSI Addons operation without its dependence on host networking. Signed-off-by: Niraj Yadav <niryadav@redhat.com>

nixpanic · 2025-06-17T15:11:03Z

Deploying fails because these images can not be pulled:

registry.k8s.io/sig-storage/csi-resizer:v1.14.0
registry.k8s.io/sig-storage/csi-node-driver-registrar:v2.14.1

These versions come from ceph-csi-operator main branch:

https://github.com/ceph/ceph-csi-operator/blob/main/internal/controller/defaults.go#L30-L38

That was merged with #252 yesterday... Not sure how it passed CI if those images are not available yet, it requires a PR to promote the versions before the image can be pulled.

Madhu-1 · 2025-06-19T09:32:56Z

+
+	// Reconcile daemonset and deployment for CSI Addons
+	if ptr.Deref(r.driver.Spec.DeployCsiAddons, false) {
+		// CSI Addons deployment


Update the comment to mention this is only for cephfs and rbd, i would suggest to use isRbDDriver and isCephFSDriver to avoid future problem when we add new csi driver

Madhu-1 · 2025-06-19T09:33:44Z

 	}
 }

+func (r *driverReconcile) reconcileCsiAddonsDeployment() error {


if i set the flag to false, i dont see the code where we are removing the deployment and deamonset.

Madhu-1 · 2025-06-19T09:38:00Z

+		Name: pluginDirHostVolumeName,
+		VolumeSource: corev1.VolumeSource{
+			HostPath: &corev1.HostPathVolumeSource{
+				Path: fmt.Sprintf("%s/plugins/%s/provisioner", kubeletDirPath, driverNamePrefix),


Will there be any problem when both deployment and deamonset runs on the same node, Are we using different socket names?

if not lets use different name and remove provisioner from the path and use csiaddons

Yes. We are using different sockets based on the driver type, e.g:

For RBD:
DS: /var/lib/kubelet/plugins/rook-ceph.rbd.csi.ceph.com/csi.sock
Deploy: /var/lib/kubelet/plugins/rook-ceph.rbd.csi.ceph.com/provisioner/csi.sock

For CephFS:
Deploy: /var/lib/kubelet/plugins/rook-ceph.cephfs.csi.ceph.com/provisioner/csi.sock

CSIAddons doesn't make much sense as a name here?

lets rename it to ctrl-plugin we can use this for other future sockets as well. for DS can you please make it node-plugin?

Madhu-1 · 2025-06-19T09:38:38Z

+							},
+						}
+						// Liveness Sidecar Container
+						if r.driver.Spec.Liveness != nil {


we dont need this container, can we remove it

Madhu-1

Can you please share the results from multiple testing to ensure we are good?

Madhu-1 · 2025-06-19T10:50:16Z

+				Spec: corev1.PodSpec{
+					ServiceAccountName: serviceAccountName,
+					PriorityClassName:  ptr.Deref(pluginSpec.PrioritylClassName, ""),
+					HostPID:            r.isRdbDriver(),


why this is required?

This is just ported over from existing code as-is. Pretty sure it was there for a reason in cases of RBD.

we should remove it, it was required for RBD as its does many operations like map. we dont required it for simple csi-addons

Madhu-1 · 2025-06-19T10:53:03Z

+		Name: pluginDirHostVolumeName,
+		VolumeSource: corev1.VolumeSource{
+			HostPath: &corev1.HostPathVolumeSource{
+				Path: fmt.Sprintf("%s/plugins/%s/provisioner", kubeletDirPath, driverNamePrefix),


lets rename it to ctrl-plugin we can use this for other future sockets as well. for DS can you please make it node-plugin?

Madhu-1 · 2025-06-19T10:54:03Z

+								Image:           r.images["addons"],
+								ImagePullPolicy: imagePullPolicy,
+								SecurityContext: &corev1.SecurityContext{
+									Privileged: ptr.To(true),


This need to be privileged? can you please verify for other containers as well, we need to keep it very minimum permission

Yes. It is required in order to access UDS created by privileged CSI provisioner container. Not having this will cause issues on systems with enforcing selinux.

sounds good :+1 can you please add a comment to it?

Signed-off-by: Niraj Yadav <niryadav@redhat.com>

This patch adds functionality to delete CSI Addons pods when their matching CSI Ctrlplugin pod is deleted by the user. This is done so that the kube-scheduler can reevaluate pod placement which is crucial when host-path is shared between the two pods. Signed-off-by: Niraj Yadav <niryadav@redhat.com>

black-dragon74 · 2025-07-01T11:11:14Z

Closing this one as #269 is the updated implementation. This PR/branch is kept as is to preserve the work done (if needed down the line).

black-dragon74 force-pushed the csi-addons-podnet branch from 2da1778 to e4075eb Compare June 16, 2025 12:06

black-dragon74 added 3 commits June 17, 2025 15:35

driver: deploy csi-addons sidecar as a DaemonSet with pod network

5dc97ca

This patch introduces a new DaemonSet for csi-addons operations that uses pod network instead of the host networking. Signed-off-by: Niraj Yadav <niryadav@redhat.com>

driver: use hostpath for csi provisioner sockets

d10c8dc

This patch modifies the provisioner deployments to use hostpath for CSI sockets. This is done to (later) enable CSI Addons sidecar to run as a separate deployment. Signed-off-by: Niraj Yadav <niryadav@redhat.com>

black-dragon74 force-pushed the csi-addons-podnet branch from e4075eb to 5206417 Compare June 17, 2025 10:05

black-dragon74 marked this pull request as ready for review June 17, 2025 10:22

Madhu-1 requested changes Jun 19, 2025

View reviewed changes

black-dragon74 force-pushed the csi-addons-podnet branch from 5290204 to e549046 Compare June 19, 2025 10:36

Madhu-1 reviewed Jun 19, 2025

View reviewed changes

black-dragon74 force-pushed the csi-addons-podnet branch from e549046 to 64f5765 Compare June 19, 2025 11:18

driver: Remove liveliness sidecar container

0f8b9e3

Signed-off-by: Niraj Yadav <niryadav@redhat.com>

black-dragon74 force-pushed the csi-addons-podnet branch from 64f5765 to 0f8b9e3 Compare June 19, 2025 11:33

black-dragon74 force-pushed the csi-addons-podnet branch from 78730d4 to 6c775e7 Compare June 24, 2025 08:23

black-dragon74 closed this Jul 1, 2025

black-dragon74 mentioned this pull request Jul 14, 2025

driver: deploy csi-addons sidecar in its own DaemonSet #269

Merged

Conversation

black-dragon74 commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nixpanic commented Jun 17, 2025

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!

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!

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!

black-dragon74 commented Jul 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

black-dragon74 commented Jun 16, 2025 •

edited

Loading