add a hostNetwork setting to the Driver and ControllerPlugin Specs #194
Conversation
This bool setting can be added to the controllerPlugin section of the Driver Spec. It will be propagated to all controller plugin pods. This implements the following design: https://github.com/ceph/ceph-csi-operator/blob/main/docs/design/hostNetwork.md Signed-off-by: Michael Adam <[email protected]>
|
PR updated to address the port collisions that @rohan47 observed in testing. I have built an image and pushed it to quay: $ docker pull quay.io/madam/ceph-csi-operator:host-net`or: $ podman pull quay.io/madam/ceph-csi-operator:host-net |
| @@ -771,15 +772,15 @@ func (r *driverReconcile) reconcileControllerPluginDeployment() error { | |||
| utils.PodContainerArg, | |||
| utils.PodUidContainerArg, | |||
| utils.CsiAddonsAddressContainerArg, | |||
| utils.ControllerPortContainerArg, | |||
| utils.ControllerPort2ContainerArg, | |||
There was a problem hiding this comment.
A better name for the variable is required, we have 2 deployments where csi-addons is enabled (cephfs and rbd), don't we going to get a port conflict when both deployments(pods) run on the same node?
There was a problem hiding this comment.
for better name, I could do something like PortDeployment and PortDeamonSet. @Madhu-1 , wdyt?
internal/utils/csi.go
Outdated
| // CSI Addons container port definition | ||
| var CsiAddonsContainerPort = corev1.ContainerPort{ | ||
| // CSI Addons container port definitions | ||
| var CsiAddonsContainerPort1 = corev1.ContainerPort{ |
There was a problem hiding this comment.
I have few question regarding the port in use here @Madhu-1
- What is the use of this port?
- Is this port information required anywhere else?
- Since we know that we need to mitigate port collision when we use hostNetwork, can we hard code new ports?
There was a problem hiding this comment.
@rohan47 , the ports are in fact hard-coded in internal/utils/csi.go
There was a problem hiding this comment.
I have few question regarding the port in use here @Madhu-1
- What is the use of this port?
- Is this port information required anywhere else?
These ports are used for csi-addons server
- Since we know that we need to mitigate port collision when we use hostNetwork, can we hard code new ports?
Yes, it can be hardcoded in this PR but we need to have a new PR to make it configurable as we support creating multiple instances of the csi drives. For 2 instances of driver will get in to the same problem of port (This bug we have today as well for daemonset)
There was a problem hiding this comment.
@Madhu-1 : do you mean a PR as preparation for this one?
There was a problem hiding this comment.
Yes to make the port configurable in CR in a new PR
Usubg host network produces port collisions. So we use different ports for controller plugin deployments and node plugin deamonsets to avoid collisions. Signed-off-by: Michael Adam <[email protected]>
|
@Madhu-1 : updated with names |
|
CI failures seem to be mostly due to problems with |
This bool setting can be added to the controllerPlugin section of the Driver Spec.
It will be propagated to all controller plugin pods.
This implements the following design:
https://github.com/ceph/ceph-csi-operator/blob/main/docs/design/hostNetwork.md
see issue #157 for background and context.
Describe what this PR does
This work in progress is a draft PR for the implementation of host Networking for controller plugin pods as described in https://github.com/ceph/ceph-csi-operator/blob/main/docs/design/hostNetwork.md
Is there anything that requires special attention
This needs thorough testing.
Is the change backward compatible?
should be but this needs testing.
Are there concerns around backward compatibility?
none right now
For example:
Related issues
Fixes: #157
Future concerns
none.
Checklist:
guidelines in the developer
guide.
Request
notes
updated with breaking and/or notable changes for the next major release.