From d0d4b1378b95d87e9115d94eb098667eb5b0a5df Mon Sep 17 00:00:00 2001 From: Jonathan Sokolowski Date: Tue, 28 Mar 2017 16:26:02 +1100 Subject: [PATCH 1/2] embed: Delay setting initial cluster for YAML NewConfig() sets an initial cluster (potentially using a default name) but we should clear it in the event another discovery option has been specified. PR #7517 attempted to address this however it only worked if the name was left as "default". (Completely) Fixes #7516 --- embed/config.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/embed/config.go b/embed/config.go index 56fd3f35c..aaf6d1be5 100644 --- a/embed/config.go +++ b/embed/config.go @@ -203,6 +203,8 @@ func (cfg *configYAML) configFromFile(path string) error { return err } + defaultInitialCluster := cfg.InitialCluster + err = yaml.Unmarshal(b, cfg) if err != nil { return err @@ -246,7 +248,8 @@ func (cfg *configYAML) configFromFile(path string) error { cfg.ACUrls = []url.URL(u) } - if (cfg.Durl != "" || cfg.DNSCluster != "") && cfg.InitialCluster == cfg.InitialClusterFromName(cfg.Name) { + // If a discovery flag is set, clear default initial cluster set by InitialClusterFromName + if (cfg.Durl != "" || cfg.DNSCluster != "") && cfg.InitialCluster == defaultInitialCluster { cfg.InitialCluster = "" } if cfg.ClusterState == "" { From 0472b2dc9f9750b97995f792a47cccc575f38f04 Mon Sep 17 00:00:00 2001 From: Jonathan Sokolowski Date: Thu, 30 Mar 2017 12:19:47 +1100 Subject: [PATCH 2/2] etcdmain: test config file clustering flags A test to ensure that when clustering flags are correctly and independently specified no errors are raised. --- etcdmain/config_test.go | 74 ++++++++++++++++++++++++++++++++--------- 1 file changed, 59 insertions(+), 15 deletions(-) diff --git a/etcdmain/config_test.go b/etcdmain/config_test.go index 1acbc1f46..03c0bce20 100644 --- a/etcdmain/config_test.go +++ b/etcdmain/config_test.go @@ -78,9 +78,7 @@ func TestConfigFileMemberFields(t *testing.T) { tmpfile := mustCreateCfgFile(t, b) defer os.Remove(tmpfile.Name()) - args := []string{ - fmt.Sprintf("--config-file=%s", tmpfile.Name()), - } + args := []string{fmt.Sprintf("--config-file=%s", tmpfile.Name())} cfg := newConfig() if err = cfg.parse(args); err != nil { @@ -133,9 +131,7 @@ func TestConfigFileClusteringFields(t *testing.T) { tmpfile := mustCreateCfgFile(t, b) defer os.Remove(tmpfile.Name()) - args := []string{ - fmt.Sprintf("--config-file=%s", tmpfile.Name()), - } + args := []string{fmt.Sprintf("--config-file=%s", tmpfile.Name())} cfg := newConfig() err = cfg.parse(args) if err != nil { @@ -145,6 +141,60 @@ func TestConfigFileClusteringFields(t *testing.T) { validateClusteringFlags(t, cfg) } +func TestConfigFileClusteringFlags(t *testing.T) { + tests := []struct { + Name string `json:"name"` + InitialCluster string `json:"initial-cluster"` + DNSCluster string `json:"discovery-srv"` + Durl string `json:"discovery"` + }{ + { + // Use default name and generate a default inital-cluster + }, + { + Name: "non-default", + }, + { + InitialCluster: "0=localhost:8000", + }, + { + Name: "non-default", + InitialCluster: "0=localhost:8000", + }, + { + DNSCluster: "example.com", + }, + { + Name: "non-default", + DNSCluster: "example.com", + }, + { + Durl: "http://example.com/abc", + }, + { + Name: "non-default", + Durl: "http://example.com/abc", + }, + } + + for i, tt := range tests { + b, err := yaml.Marshal(&tt) + if err != nil { + t.Fatal(err) + } + + tmpfile := mustCreateCfgFile(t, b) + defer os.Remove(tmpfile.Name()) + + args := []string{fmt.Sprintf("--config-file=%s", tmpfile.Name())} + + cfg := newConfig() + if err := cfg.parse(args); err != nil { + t.Errorf("%d: err = %v", i, err) + } + } +} + func TestConfigParsingOtherFlags(t *testing.T) { args := []string{"-proxy=readonly"} @@ -172,9 +222,7 @@ func TestConfigFileOtherFields(t *testing.T) { tmpfile := mustCreateCfgFile(t, b) defer os.Remove(tmpfile.Name()) - args := []string{ - fmt.Sprintf("--config-file=%s", tmpfile.Name()), - } + args := []string{fmt.Sprintf("--config-file=%s", tmpfile.Name())} cfg := newConfig() err = cfg.parse(args) @@ -248,9 +296,7 @@ func TestConfigFileConflictClusteringFlags(t *testing.T) { tmpfile := mustCreateCfgFile(t, b) defer os.Remove(tmpfile.Name()) - args := []string{ - fmt.Sprintf("--config-file=%s", tmpfile.Name()), - } + args := []string{fmt.Sprintf("--config-file=%s", tmpfile.Name())} cfg := newConfig() if err := cfg.parse(args); err != embed.ErrConflictBootstrapFlags { @@ -428,9 +474,7 @@ func TestConfigFileElectionTimeout(t *testing.T) { tmpfile := mustCreateCfgFile(t, b) defer os.Remove(tmpfile.Name()) - args := []string{ - fmt.Sprintf("--config-file=%s", tmpfile.Name()), - } + args := []string{fmt.Sprintf("--config-file=%s", tmpfile.Name())} cfg := newConfig() if err := cfg.parse(args); err == nil || !strings.Contains(err.Error(), tt.errStr) {