etcd: refactor config

- Explicitly specify all of the valid options for etcd
- Remove the default name generation (ETCD_NAME is set by its unit file now)
- Seperate the etcd config from Units()
- Remove support for DISCOVERY_URL
- Add YAML tags for the fields

config/etcd.go

@@ -0,0 +1,32 @@
+package config
+
+type Etcd struct {
+	Addr                string `yaml:"addr"                  env:"ETCD_ADDR"`
+	BindAddr            string `yaml:"bind-addr"             env:"ETCD_BIND_ADDR"`
+	CAFile              string `yaml:"ca-file"               env:"ETCD_CA_FILE"`
+	CertFile            string `yaml:"cert-file"             env:"ETCD_CERT_FILE"`
+	ClusterActiveSize   string `yaml:"cluster-active-size"   env:"ETCD_CLUSTER_ACTIVE_SIZE"`
+	ClusterRemoveDelay  string `yaml:"cluster-remove-delay"  env:"ETCD_CLUSTER_REMOVE_DELAY"`
+	ClusterSyncInterval string `yaml:"cluster-sync-interval" env:"ETCD_CLUSTER_SYNC_INTERVAL"`
+	Cors                string `yaml:"cors"                  env:"ETCD_CORS"`
+	CPUProfileFile      string `yaml:"cpu-profile-file"      env:"ETCD_CPU_PROFILE_FILE"`
+	DataDir             string `yaml:"data-dir"              env:"ETCD_DATA_DIR"`
+	Discovery           string `yaml:"discovery"             env:"ETCD_DISCOVERY"`
+	HTTPReadTimeout     string `yaml:"http-read-timeout"     env:"ETCD_HTTP_READ_TIMEOUT"`
+	HTTPWriteTimeout    string `yaml:"http-write-timeout"    env:"ETCD_HTTP_WRITE_TIMEOUT"`
+	KeyFile             string `yaml:"key-file"              env:"ETCD_KEY_FILE"`
+	MaxClusterSize      string `yaml:"max-cluster-size"      env:"ETCD_MAX_CLUSTER_SIZE"`
+	MaxResultBuffer     string `yaml:"max-result-buffer"     env:"ETCD_MAX_RESULT_BUFFER"`
+	MaxRetryAttempts    string `yaml:"max-retry-attempts"    env:"ETCD_MAX_RETRY_ATTEMPTS"`
+	Name                string `yaml:"name"                  env:"ETCD_NAME"`
+	PeerAddr            string `yaml:"peer-addr"             env:"ETCD_PEER_ADDR"`
+	PeerBindAddr        string `yaml:"peer-bind-addr"        env:"ETCD_PEER_BIND_ADDR"`
+	PeerCAFile          string `yaml:"peer-ca-file"          env:"ETCD_PEER_CA_FILE"`
+	PeerCertFile        string `yaml:"peer-cert-file"        env:"ETCD_PEER_CERT_FILE"`
+	PeerKeyFile         string `yaml:"peer-key-file"         env:"ETCD_PEER_KEY_FILE"`
+	Peers               string `yaml:"peers"                 env:"ETCD_PEERS"`
+	PeersFile           string `yaml:"peers-file"            env:"ETCD_PEERS_FILE"`
+	Snapshot            string `yaml:"snapshot"              env:"ETCD_SNAPSHOT"`
+	Verbose             string `yaml:"verbose"               env:"ETCD_VERBOSE"`
+	VeryVerbose         string `yaml:"very-verbose"          env:"ETCD_VERY_VERBOSE"`
+}

initialize/config.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/coreos/coreos-cloudinit/third_party/gopkg.in/yaml.v1"
 
+	"github.com/coreos/coreos-cloudinit/config"
 	"github.com/coreos/coreos-cloudinit/network"
 	"github.com/coreos/coreos-cloudinit/system"
 )
@@ -30,7 +31,7 @@ type CloudConfigUnit interface {
 type CloudConfig struct {
 	SSHAuthorizedKeys []string `yaml:"ssh_authorized_keys"`
 	Coreos            struct {
-		Etcd   EtcdEnvironment
+		Etcd   config.Etcd
 		Fleet  FleetEnvironment
 		OEM    OEMRelease
 		Update UpdateConfig
@@ -226,7 +227,7 @@ func Apply(cfg CloudConfig, env *Environment) error {
 		}
 	}
 
-	for _, ccu := range []CloudConfigUnit{cfg.Coreos.Etcd, cfg.Coreos.Fleet, cfg.Coreos.Update} {
+	for _, ccu := range []CloudConfigUnit{system.Etcd{cfg.Coreos.Etcd}, cfg.Coreos.Fleet, cfg.Coreos.Update} {
 		u, err := ccu.Units(env.Root())
 		if err != nil {
 			return err

initialize/config_test.go

@@ -66,7 +66,7 @@ hostname:
 	if cfg.Hostname != "foo" {
 		t.Fatalf("hostname not correctly set when invalid keys are present")
 	}
-	if len(cfg.Coreos.Etcd) < 1 {
+	if cfg.Coreos.Etcd.Discovery != "https://discovery.etcd.io/827c73219eeb2fa5530027c37bf18877" {
 		t.Fatalf("etcd section not correctly set when invalid keys are present")
 	}
 	if len(cfg.WriteFiles) < 1 || cfg.WriteFiles[0].Content != "fun" || cfg.WriteFiles[0].Path != "/var/party" {

initialize/etcd.go

@@ -1,63 +0,0 @@
-package initialize
-
-import (
-	"errors"
-	"fmt"
-	"sort"
-
-	"github.com/coreos/coreos-cloudinit/system"
-)
-
-type EtcdEnvironment map[string]string
-
-func (ee EtcdEnvironment) String() (out string) {
-	norm := normalizeSvcEnv(ee)
-
-	if val, ok := norm["DISCOVERY_URL"]; ok {
-		delete(norm, "DISCOVERY_URL")
-		if _, ok := norm["DISCOVERY"]; !ok {
-			norm["DISCOVERY"] = val
-		}
-	}
-
-	var sorted sort.StringSlice
-	for k, _ := range norm {
-		sorted = append(sorted, k)
-	}
-	sorted.Sort()
-
-	out += "[Service]\n"
-
-	for _, key := range sorted {
-		val := norm[key]
-		out += fmt.Sprintf("Environment=\"ETCD_%s=%s\"\n", key, val)
-	}
-
-	return
-}
-
-// Units creates a Unit file drop-in for etcd, using any configured
-// options and adding a default MachineID if unset.
-func (ee EtcdEnvironment) Units(root string) ([]system.Unit, error) {
-	if len(ee) < 1 {
-		return nil, nil
-	}
-
-	if _, ok := ee["name"]; !ok {
-		if machineID := system.MachineID(root); machineID != "" {
-			ee["name"] = machineID
-		} else if hostname, err := system.Hostname(); err == nil {
-			ee["name"] = hostname
-		} else {
-			return nil, errors.New("Unable to determine default etcd name")
-		}
-	}
-
-	etcd := system.Unit{
-		Name:    "etcd.service",
-		Runtime: true,
-		DropIn:  true,
-		Content: ee.String(),
-	}
-	return []system.Unit{etcd}, nil
-}

initialize/etcd_test.go

@@ -1,184 +0,0 @@
-package initialize
-
-import (
-	"io/ioutil"
-	"os"
-	"path"
-	"testing"
-
-	"github.com/coreos/coreos-cloudinit/system"
-)
-
-func TestEtcdEnvironment(t *testing.T) {
-	cfg := make(EtcdEnvironment, 0)
-	cfg["discovery"] = "http://disco.example.com/foobar"
-	cfg["peer-bind-addr"] = "127.0.0.1:7002"
-
-	env := cfg.String()
-	expect := `[Service]
-Environment="ETCD_DISCOVERY=http://disco.example.com/foobar"
-Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002"
-`
-
-	if env != expect {
-		t.Errorf("Generated environment:\n%s\nExpected environment:\n%s", env, expect)
-	}
-}
-
-func TestEtcdEnvironmentDiscoveryURLTranslated(t *testing.T) {
-	cfg := make(EtcdEnvironment, 0)
-	cfg["discovery_url"] = "http://disco.example.com/foobar"
-	cfg["peer-bind-addr"] = "127.0.0.1:7002"
-
-	env := cfg.String()
-	expect := `[Service]
-Environment="ETCD_DISCOVERY=http://disco.example.com/foobar"
-Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002"
-`
-
-	if env != expect {
-		t.Errorf("Generated environment:\n%s\nExpected environment:\n%s", env, expect)
-	}
-}
-
-func TestEtcdEnvironmentDiscoveryOverridesDiscoveryURL(t *testing.T) {
-	cfg := make(EtcdEnvironment, 0)
-	cfg["discovery_url"] = "ping"
-	cfg["discovery"] = "pong"
-	cfg["peer-bind-addr"] = "127.0.0.1:7002"
-
-	env := cfg.String()
-	expect := `[Service]
-Environment="ETCD_DISCOVERY=pong"
-Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002"
-`
-
-	if env != expect {
-		t.Errorf("Generated environment:\n%s\nExpected environment:\n%s", env, expect)
-	}
-}
-
-func TestEtcdEnvironmentWrittenToDisk(t *testing.T) {
-	ee := EtcdEnvironment{
-		"name":           "node001",
-		"discovery":      "http://disco.example.com/foobar",
-		"peer-bind-addr": "127.0.0.1:7002",
-	}
-	dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-")
-	if err != nil {
-		t.Fatalf("Unable to create tempdir: %v", err)
-	}
-	defer os.RemoveAll(dir)
-
-	sd := system.NewUnitManager(dir)
-
-	uu, err := ee.Units(dir)
-	if err != nil {
-		t.Fatalf("Generating etcd unit failed: %v", err)
-	}
-	if len(uu) != 1 {
-		t.Fatalf("Expected 1 unit to be returned, got %d", len(uu))
-	}
-	u := uu[0]
-
-	dst := u.Destination(dir)
-	os.Stderr.WriteString("writing to " + dir + "\n")
-	if err := sd.PlaceUnit(&u, dst); err != nil {
-		t.Fatalf("Writing of EtcdEnvironment failed: %v", err)
-	}
-
-	fullPath := path.Join(dir, "run", "systemd", "system", "etcd.service.d", "20-cloudinit.conf")
-
-	fi, err := os.Stat(fullPath)
-	if err != nil {
-		t.Fatalf("Unable to stat file: %v", err)
-	}
-
-	if fi.Mode() != os.FileMode(0644) {
-		t.Errorf("File has incorrect mode: %v", fi.Mode())
-	}
-
-	contents, err := ioutil.ReadFile(fullPath)
-	if err != nil {
-		t.Fatalf("Unable to read expected file: %v", err)
-	}
-
-	expect := `[Service]
-Environment="ETCD_DISCOVERY=http://disco.example.com/foobar"
-Environment="ETCD_NAME=node001"
-Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002"
-`
-	if string(contents) != expect {
-		t.Fatalf("File has incorrect contents")
-	}
-}
-
-func TestEtcdEnvironmentEmptyNoOp(t *testing.T) {
-	ee := EtcdEnvironment{}
-	uu, err := ee.Units("")
-	if err != nil {
-		t.Fatalf("Unexpected error: %v", err)
-	}
-	if len(uu) > 0 {
-		t.Fatalf("Generated etcd units unexpectedly: %v")
-	}
-}
-
-func TestEtcdEnvironmentWrittenToDiskDefaultToMachineID(t *testing.T) {
-	ee := EtcdEnvironment{"foo": "bar"}
-	dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-")
-	if err != nil {
-		t.Fatalf("Unable to create tempdir: %v", err)
-	}
-	defer os.RemoveAll(dir)
-
-	sd := system.NewUnitManager(dir)
-
-	os.Mkdir(path.Join(dir, "etc"), os.FileMode(0755))
-	err = ioutil.WriteFile(path.Join(dir, "etc", "machine-id"), []byte("node007"), os.FileMode(0444))
-	if err != nil {
-		t.Fatalf("Failed writing out /etc/machine-id: %v", err)
-	}
-
-	uu, err := ee.Units(dir)
-	if err != nil {
-		t.Fatalf("Generating etcd unit failed: %v", err)
-	}
-	if len(uu) == 0 {
-		t.Fatalf("Returned empty etcd units unexpectedly")
-	}
-	u := uu[0]
-
-	dst := u.Destination(dir)
-	os.Stderr.WriteString("writing to " + dir + "\n")
-	if err := sd.PlaceUnit(&u, dst); err != nil {
-		t.Fatalf("Writing of EtcdEnvironment failed: %v", err)
-	}
-
-	fullPath := path.Join(dir, "run", "systemd", "system", "etcd.service.d", "20-cloudinit.conf")
-
-	contents, err := ioutil.ReadFile(fullPath)
-	if err != nil {
-		t.Fatalf("Unable to read expected file: %v", err)
-	}
-
-	expect := `[Service]
-Environment="ETCD_FOO=bar"
-Environment="ETCD_NAME=node007"
-`
-	if string(contents) != expect {
-		t.Fatalf("File has incorrect contents")
-	}
-}
-
-func TestEtcdEnvironmentWhenNil(t *testing.T) {
-	// EtcdEnvironment will be a nil map if it wasn't in the yaml
-	var ee EtcdEnvironment
-	if ee != nil {
-		t.Fatalf("EtcdEnvironment is not nil")
-	}
-	uu, err := ee.Units("")
-	if len(uu) != 0 || err != nil {
-		t.Fatalf("Units returned value for nil input")
-	}
-}

system/env.go

@@ -0,0 +1,26 @@
+package system
+
+import (
+	"fmt"
+	"reflect"
+)
+
+// dropinContents generates the contents for a drop-in unit given the config.
+// The argument must be a struct from the 'config' package.
+func dropinContents(e interface{}) string {
+	et := reflect.TypeOf(e)
+	ev := reflect.ValueOf(e)
+
+	var out string
+	for i := 0; i < et.NumField(); i++ {
+		if val := ev.Field(i).String(); val != "" {
+			key := et.Field(i).Tag.Get("env")
+			out += fmt.Sprintf("Environment=\"%s=%s\"\n", key, val)
+		}
+	}
+
+	if out == "" {
+		return ""
+	}
+	return "[Service]\n" + out
+}

system/etcd.go

@@ -0,0 +1,25 @@
+package system
+
+import (
+	"github.com/coreos/coreos-cloudinit/config"
+)
+
+// Etcd is a top-level structure which embeds its underlying configuration,
+// config.Etcd, and provides the system-specific Unit().
+type Etcd struct {
+	config.Etcd
+}
+
+// Units creates a Unit file drop-in for etcd, using any configured options.
+func (ee Etcd) Units(_ string) ([]Unit, error) {
+	content := dropinContents(ee.Etcd)
+	if content == "" {
+		return nil, nil
+	}
+	return []Unit{{
+		Name:    "etcd.service",
+		Runtime: true,
+		DropIn:  true,
+		Content: content,
+	}}, nil
+}

system/etcd_test.go

@@ -0,0 +1,60 @@
+package system
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/coreos/coreos-cloudinit/config"
+)
+
+func TestEtcdUnits(t *testing.T) {
+	for _, tt := range []struct {
+		config config.Etcd
+		units  []Unit
+	}{
+		{
+			config.Etcd{},
+			nil,
+		},
+		{
+			config.Etcd{
+				Discovery:    "http://disco.example.com/foobar",
+				PeerBindAddr: "127.0.0.1:7002",
+			},
+			[]Unit{{
+				Name:    "etcd.service",
+				Runtime: true,
+				DropIn:  true,
+				Content: `[Service]
+Environment="ETCD_DISCOVERY=http://disco.example.com/foobar"
+Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002"
+`,
+			}},
+		},
+		{
+			config.Etcd{
+				Name:         "node001",
+				Discovery:    "http://disco.example.com/foobar",
+				PeerBindAddr: "127.0.0.1:7002",
+			},
+			[]Unit{{
+				Name:    "etcd.service",
+				Runtime: true,
+				DropIn:  true,
+				Content: `[Service]
+Environment="ETCD_DISCOVERY=http://disco.example.com/foobar"
+Environment="ETCD_NAME=node001"
+Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002"
+`,
+			}},
+		},
+	} {
+		units, err := Etcd{tt.config}.Units("")
+		if err != nil {
+			t.Errorf("bad error (%q): want %q, got %q", tt.config, nil, err)
+		}
+		if !reflect.DeepEqual(tt.units, units) {
+			t.Errorf("bad units (%q): want %#v, got %#v", tt.config, tt.units, units)
+		}
+	}
+}