rename socket directory to a common name

[Madhu-1]

as the socket directory will be created inside the container no need to follow the plugin name in for the directory creation, this will also reduce the code changes if we want to change driver name.

Fixes: #223 (comment)
Signed-off-by: Madhu Rajanna madhupr007@gmail.com

[ShyamsundarR]

Current changes look fine. Are we not handling the helm change for the same in this PR?

[Madhu-1]

@ShyamsundarR if I change that value I need to make a lot of changes in helm folder, so I kept it as it is, changing this value should be enough for the users to deploy helm chats

most of the fields are derived from the values so changing value should be sufficient,

@kfox1111 any comments related to this one?

[ShyamsundarR]

@ShyamsundarR if I change that value I need to make a lot of changes in helm folder, so I kept it as it is, changing this value should be enough for the users to deploy helm chats

most of the fields are derived from the values so changing value should be sufficient,

Just for clarification, I was suggesting to just change that default value in the variable to what is now the default, and NOT make it static and remove it from the variables yaml for Helm. Does that require changes beyond the change to the variable in the file in Helm?

[humblec]

@Madhu-1 eventhough I see the intention of this PR, I doubt this is really necessary. At times , the container or mount path 'at same location' gives more clarity. If its causing confusion to someone/developer, I think thats still fine/ok to have as they will eventually find out the change should be necessary or not :).

[Madhu-1]

@ShyamsundarR if I change that value I need to make a lot of changes in helm folder, so I kept it as it is, changing this value should be enough for the users to deploy helm chats
most of the fields are derived from the values so changing value should be sufficient,

Just for clarification, I was suggesting to just change that default value in the variable to what is now the default, and NOT make it static and remove it from the variables yaml for Helm. Does that require changes beyond the change to the variable in the file in Helm?

yes, this is the variable used for binding to the hostpath also. https://github.com/ceph/ceph-csi/blob/csi-v1.0/deploy/rbd/helm/templates/nodeplugin-daemonset.yaml#L109

[ShyamsundarR]

Looks fine.

[humblec]

@Madhu-1 eventhough I see the intention of this PR, I doubt this is really necessary. At times , the container or mount path 'at same location' gives more clarity. If its causing confusion to someone/developer, I think thats still fine/ok to have as they will eventually find out the change should be necessary or not :).

[ShyamsundarR]

@Madhu-1 eventhough I see the intention of this PR, I doubt this is really necessary. At times , the container or mount path 'at same location' gives more clarity. If its causing confusion to someone/developer, I think that's still fine/ok to have as they will eventually find out the change should be necessary or not :).

Considering I raised this issue, here is what I think,

• This mount path at the same location is actually not required, and is adding verbosity without any need
• The socket dir need not take in the CSI name at all, so why add that and maintain it for any revision of the CSI name (thinking multi instance CSI if any in the future)
• The current scheme just makes the socket look special, whereas it can just be at a well known path

The existing scheme does not provide any clarity or is not required, IMO simplifying it is better than retaining it with these long names.

[j-griffith]

Seems like a resonable simplification to me without any real negative side effects

[Madhu-1]

@gman0 @rootfs PTAL

[Madhu-1]

@rootfs @gman0 can you guys merge this one if it looks good

[rootfs]

@Madhu-1 what about attacher in rbd plugin?

[rootfs]

cc @kfox1111

[Madhu-1]

@Madhu-1 what about attacher in rbd plugin?

@rootfs Thanks, updated