fix cache persistent default #120
Conversation
Signed-off-by: Huamin Chen <hchen@redhat.com>
|
Yep, haven't thought about this issue. Seems better to default cachepersister to node. |
| @@ -35,7 +35,7 @@ var ( | |||
| driverName = flag.String("drivername", "csi-rbdplugin", "name of the driver") | |||
| nodeID = flag.String("nodeid", "", "node id") | |||
| containerized = flag.Bool("containerized", true, "whether run as containerized") | |||
| metadataStorage = flag.String("metadatastorage", "", "metadata persistence method [node|k8s_configmap]") | |||
| metadataStorage = flag.String("metadatastorage", "node", "metadata persistence method [node|k8s_configmap]") | |||
There was a problem hiding this comment.
For consistency, this should then also go in the cephfs driver
| @@ -50,6 +50,10 @@ func main() { | |||
| os.Exit(1) | |||
| } | |||
|
|
|||
| if *metadataStorage != "node" && *metadataStorage != "k8s_configmap" { | |||
There was a problem hiding this comment.
The validation is already done here:
https://github.com/ceph/ceph-csi/blob/master/pkg/util/cachepersister.go#L38-L50
|
I realise I'm late to this as it's already been merged, but what it the logic behind choosing "node" for the default in code, but leaving the command-line arg in |
|
We don't want to have k8s-specific defaults in the code, but it's ok to default to configmaps for k8s deployments. Also, this way it will have consistent behavior with older versions. |
|
Makes perfect sense, thanks for the fast response. |
fix cache persistent default
…ck-118-to-release-4.12 [release-4.12] sync downstream devel with upstream devel
fix #119
cc @mickymiek