fixes for "improve description of --initial-cluster-state flag" #15743 #15752
Conversation
server/etcdmain/config.go
Outdated
| @@ -189,8 +189,7 @@ func newConfig() *config { | |||
| fs.StringVar(&cfg.ec.DNSClusterServiceName, "discovery-srv-name", cfg.ec.DNSClusterServiceName, "Service name to query when using DNS discovery.") | |||
| fs.StringVar(&cfg.ec.InitialCluster, "initial-cluster", cfg.ec.InitialCluster, "Initial cluster configuration for bootstrapping.") | |||
| fs.StringVar(&cfg.ec.InitialClusterToken, "initial-cluster-token", cfg.ec.InitialClusterToken, "Initial cluster token for the etcd cluster during bootstrap.") | |||
| fs.Var(cfg.cf.clusterState, "initial-cluster-state", "Initial cluster state ('new' or 'existing').") | |||
|
|
|||
| fs.Var(cfg.cf.clusterState, "initial-cluster-state", "Initial cluster state ('new' when bootstrapping a new cluster or 'existing' when adding new members to an existing cluster).Initial cluster state is based on the availability of a populated data/WAL directory.") | |||
There was a problem hiding this comment.
Suggest to remove "Initial cluster state is based on the availability of a populated data/WAL directory.". Note that there shouldn't have local data no matter bootstrapping a new cluster or adding new members into an existing cluster.
After successfully bootstrapping the new cluster or adding any new members, then it doesn't matter what's the value for the flag.
There was a problem hiding this comment.
How about we also add After successful initialization (bootstrapping or adding), flag is ignored on restarts
|
Please also squash the commits. |
There was a problem hiding this comment.
Overall this looks good to me, thanks @sharathsivakumar.
Just one minor nitpick about a potentially missing space after a sentence finishes.
server/etcdmain/config.go
Outdated
| @@ -189,8 +189,7 @@ func newConfig() *config { | |||
| fs.StringVar(&cfg.ec.DNSClusterServiceName, "discovery-srv-name", cfg.ec.DNSClusterServiceName, "Service name to query when using DNS discovery.") | |||
| fs.StringVar(&cfg.ec.InitialCluster, "initial-cluster", cfg.ec.InitialCluster, "Initial cluster configuration for bootstrapping.") | |||
| fs.StringVar(&cfg.ec.InitialClusterToken, "initial-cluster-token", cfg.ec.InitialClusterToken, "Initial cluster token for the etcd cluster during bootstrap.") | |||
| fs.Var(cfg.cf.clusterState, "initial-cluster-state", "Initial cluster state ('new' or 'existing').") | |||
|
|
|||
| fs.Var(cfg.cf.clusterState, "initial-cluster-state", "Initial cluster state ('new' when bootstrapping a new cluster or 'existing' when adding new members to an existing cluster).After successful initialization (bootstrapping or adding), flag is ignored on restarts.") | |||
There was a problem hiding this comment.
Nitpick, I think there is a space missing here at .After .
| fs.Var(cfg.cf.clusterState, "initial-cluster-state", "Initial cluster state ('new' when bootstrapping a new cluster or 'existing' when adding new members to an existing cluster).After successful initialization (bootstrapping or adding), flag is ignored on restarts.") | |
| fs.Var(cfg.cf.clusterState, "initial-cluster-state", "Initial cluster state ('new' when bootstrapping a new cluster or 'existing' when adding new members to an existing cluster). After successful initialization (bootstrapping or adding), flag is ignored on restarts.") |
There was a problem hiding this comment.
Thanks for that. Fixed it.
Signed-off-by: sharathsivakumar <mailssr9@gmail.com>
|
@sharathsivakumar could you please also backport this PR to 3.5 and 3.4? thx |
Improves the description for --initial-clsuter-state provided in the help. Details of the issue can be found here