update: refactor config

- Explicitly specify all of the valid options for Update
- Seperate the config from File() and Units()
- Add YAML tags for the fields

config/config.go

@@ -0,0 +1,66 @@
+package config
+
+import (
+	"fmt"
+	"reflect"
+	"strings"
+)
+
+// IsZero returns whether or not the parameter is the zero value for its type.
+// If the parameter is a struct, only the exported fields are considered.
+func IsZero(c interface{}) bool {
+	return isZero(reflect.ValueOf(c))
+}
+
+// AssertValid checks the fields in the structure and makes sure that they
+// contain valid values as specified by the 'valid' flag. Empty fields are
+// implicitly valid.
+func AssertValid(c interface{}) error {
+	ct := reflect.TypeOf(c)
+	cv := reflect.ValueOf(c)
+	for i := 0; i < ct.NumField(); i++ {
+		ft := ct.Field(i)
+		if !isFieldExported(ft) {
+			continue
+		}
+
+		valid := ft.Tag.Get("valid")
+		val := cv.Field(i)
+		if !isValid(val, valid) {
+			return fmt.Errorf("invalid value \"%v\" for option %q (valid options: %q)", val.Interface(), ft.Name, valid)
+		}
+	}
+	return nil
+}
+
+func isZero(v reflect.Value) bool {
+	switch v.Kind() {
+	case reflect.Struct:
+		vt := v.Type()
+		for i := 0; i < v.NumField(); i++ {
+			if isFieldExported(vt.Field(i)) && !isZero(v.Field(i)) {
+				return false
+			}
+		}
+		return true
+	default:
+		return v.Interface() == reflect.Zero(v.Type()).Interface()
+	}
+}
+
+func isFieldExported(f reflect.StructField) bool {
+	return f.PkgPath == ""
+}
+
+func isValid(v reflect.Value, valid string) bool {
+	if valid == "" || isZero(v) {
+		return true
+	}
+	vs := fmt.Sprintf("%v", v.Interface())
+	for _, valid := range strings.Split(valid, ",") {
+		if vs == valid {
+			return true
+		}
+	}
+	return false
+}

config/config_test.go

@@ -0,0 +1,63 @@
+package config
+
+import (
+	"errors"
+	"reflect"
+	"testing"
+)
+
+func TestIsZero(t *testing.T) {
+	for _, tt := range []struct {
+		c     interface{}
+		empty bool
+	}{
+		{struct{}{}, true},
+		{struct{ a, b string }{}, true},
+		{struct{ A, b string }{}, true},
+		{struct{ A, B string }{}, true},
+		{struct{ A string }{A: "hello"}, false},
+		{struct{ A int }{}, true},
+		{struct{ A int }{A: 1}, false},
+	} {
+		if empty := IsZero(tt.c); tt.empty != empty {
+			t.Errorf("bad result (%q): want %q, got %q", tt.c, tt.empty, empty)
+		}
+	}
+}
+
+func TestAssertValid(t *testing.T) {
+	for _, tt := range []struct {
+		c   interface{}
+		err error
+	}{
+		{struct{}{}, nil},
+		{struct {
+			A, b string `valid:"1,2"`
+		}{}, nil},
+		{struct {
+			A, b string `valid:"1,2"`
+		}{A: "1", b: "2"}, nil},
+		{struct {
+			A, b string `valid:"1,2"`
+		}{A: "1", b: "hello"}, nil},
+		{struct {
+			A, b string `valid:"1,2"`
+		}{A: "hello", b: "2"}, errors.New("invalid value \"hello\" for option \"A\" (valid options: \"1,2\")")},
+		{struct {
+			A, b int `valid:"1,2"`
+		}{}, nil},
+		{struct {
+			A, b int `valid:"1,2"`
+		}{A: 1, b: 2}, nil},
+		{struct {
+			A, b int `valid:"1,2"`
+		}{A: 1, b: 9}, nil},
+		{struct {
+			A, b int `valid:"1,2"`
+		}{A: 9, b: 2}, errors.New("invalid value \"9\" for option \"A\" (valid options: \"1,2\")")},
+	} {
+		if err := AssertValid(tt.c); !reflect.DeepEqual(tt.err, err) {
+			t.Errorf("bad result (%q): want %q, got %q", tt.c, tt.err, err)
+		}
+	}
+}

config/etc_hosts.go

@@ -0,0 +1,3 @@
+package config
+
+type EtcHosts string

config/update.go

@@ -0,0 +1,7 @@
+package config
+
+type Update struct {
+	RebootStrategy string `yaml:"reboot-strategy" env:"REBOOT_STRATEGY" valid:"best-effort,etcd-lock,reboot,off"`
+	Group          string `yaml:"group"           env:"GROUP"`
+	Server         string `yaml:"server"          env:"SERVER"`
+}

coreos-cloudinit_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/coreos/coreos-cloudinit/initialize"
+	"github.com/coreos/coreos-cloudinit/config"
 )
 
 func TestMergeCloudConfig(t *testing.T) {
@@ -81,7 +82,7 @@ func TestMergeCloudConfig(t *testing.T) {
 			// Non-mergeable settings in user-data should not be affected
 			initialize.CloudConfig{
 				Hostname:       "mememe",
-				ManageEtcHosts: initialize.EtcHosts("lolz"),
+				ManageEtcHosts: config.EtcHosts("lolz"),
 			},
 			initialize.CloudConfig{
 				Hostname:          "youyouyou",
@@ -90,7 +91,7 @@ func TestMergeCloudConfig(t *testing.T) {
 			},
 			initialize.CloudConfig{
 				Hostname:          "mememe",
-				ManageEtcHosts:    initialize.EtcHosts("lolz"),
+				ManageEtcHosts:    config.EtcHosts("lolz"),
 				NetworkConfigPath: "meta-meta-yo",
 				NetworkConfig:     `{"hostname":"test"}`,
 			},
@@ -101,7 +102,7 @@ func TestMergeCloudConfig(t *testing.T) {
 				Hostname: "mememe",
 			},
 			initialize.CloudConfig{
-				ManageEtcHosts:    initialize.EtcHosts("lolz"),
+				ManageEtcHosts:    config.EtcHosts("lolz"),
 				NetworkConfigPath: "meta-meta-yo",
 				NetworkConfig:     `{"hostname":"test"}`,
 			},

initialize/config.go

@@ -18,13 +18,13 @@ import (
 type CloudConfigFile interface {
 	// File should either return (*system.File, error), or (nil, nil) if nothing
 	// needs to be done for this configuration option.
-	File(root string) (*system.File, error)
+	File() (*system.File, error)
 }
 
 // CloudConfigUnit represents a CoreOS specific configuration option that can generate
 // associated system.Units to be created/enabled appropriately
 type CloudConfigUnit interface {
-	Units(root string) ([]system.Unit, error)
+	Units() ([]system.Unit, error)
 }
 
 // CloudConfig encapsulates the entire cloud-config configuration file and maps directly to YAML
@@ -34,13 +34,13 @@ type CloudConfig struct {
 		Etcd   config.Etcd
 		Fleet  config.Fleet
 		OEM    config.OEM
-		Update UpdateConfig
+		Update config.Update
 		Units  []system.Unit
 	}
 	WriteFiles        []system.File `yaml:"write_files"`
 	Hostname          string
 	Users             []system.User
-	ManageEtcHosts    EtcHosts `yaml:"manage_etc_hosts"`
+	ManageEtcHosts    config.EtcHosts `yaml:"manage_etc_hosts"`
 	NetworkConfigPath string
 	NetworkConfig     string
 }
@@ -217,8 +217,12 @@ func Apply(cfg CloudConfig, env *Environment) error {
 		}
 	}
 
-	for _, ccf := range []CloudConfigFile{system.OEM{cfg.Coreos.OEM}, cfg.Coreos.Update, cfg.ManageEtcHosts} {
+	for _, ccf := range []CloudConfigFile{
-		f, err := ccf.File(env.Root())
+		system.OEM{cfg.Coreos.OEM},
+		system.Update{cfg.Coreos.Update, system.DefaultReadConfig},
+		system.EtcHosts{cfg.ManageEtcHosts},
+	} {
+		f, err := ccf.File()
 		if err != nil {
 			return err
 		}
@@ -227,8 +231,12 @@ func Apply(cfg CloudConfig, env *Environment) error {
 		}
 	}
 
-	for _, ccu := range []CloudConfigUnit{system.Etcd{cfg.Coreos.Etcd}, system.Fleet{cfg.Coreos.Fleet}, cfg.Coreos.Update} {
+	for _, ccu := range []CloudConfigUnit{
-		u, err := ccu.Units(env.Root())
+		system.Etcd{cfg.Coreos.Etcd},
+		system.Fleet{cfg.Coreos.Fleet},
+		system.Update{cfg.Coreos.Update, system.DefaultReadConfig},
+	} {
+		u, err := ccu.Units()
 		if err != nil {
 			return err
 		}

initialize/config_test.go

@@ -226,7 +226,7 @@ Address=10.209.171.177/19
 	if cfg.Hostname != "trontastic" {
 		t.Errorf("Failed to parse hostname")
 	}
-	if cfg.Coreos.Update["reboot-strategy"] != "reboot" {
+	if cfg.Coreos.Update.RebootStrategy != "reboot" {
 		t.Errorf("Failed to parse locksmith strategy")
 	}
 }

initialize/manage_etc_hosts_test.go

@@ -1,83 +0,0 @@
-package initialize
-
-import (
-	"fmt"
-	"io/ioutil"
-	"os"
-	"path"
-	"testing"
-
-	"github.com/coreos/coreos-cloudinit/system"
-)
-
-func TestCloudConfigManageEtcHosts(t *testing.T) {
-	contents := `
-manage_etc_hosts: localhost
-`
-	cfg, err := NewCloudConfig(contents)
-	if err != nil {
-		t.Fatalf("Encountered unexpected error: %v", err)
-	}
-
-	manageEtcHosts := cfg.ManageEtcHosts
-
-	if manageEtcHosts != "localhost" {
-		t.Errorf("ManageEtcHosts value is %q, expected 'localhost'", manageEtcHosts)
-	}
-}
-
-func TestManageEtcHostsInvalidValue(t *testing.T) {
-	eh := EtcHosts("invalid")
-	if f, err := eh.File(""); err == nil || f != nil {
-		t.Fatalf("EtcHosts File succeeded with invalid value!")
-	}
-}
-
-func TestEtcHostsWrittenToDisk(t *testing.T) {
-	dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-")
-	if err != nil {
-		t.Fatalf("Unable to create tempdir: %v", err)
-	}
-	defer os.RemoveAll(dir)
-
-	eh := EtcHosts("localhost")
-
-	f, err := eh.File(dir)
-	if err != nil {
-		t.Fatalf("Error calling File on EtcHosts: %v", err)
-	}
-	if f == nil {
-		t.Fatalf("manageEtcHosts returned nil file unexpectedly")
-	}
-
-	if _, err := system.WriteFile(f, dir); err != nil {
-		t.Fatalf("Error writing EtcHosts: %v", err)
-	}
-
-	fullPath := path.Join(dir, "etc", "hosts")
-
-	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)
-	}
-
-	hostname, err := os.Hostname()
-	if err != nil {
-		t.Fatalf("Unable to read OS hostname: %v", err)
-	}
-
-	expect := fmt.Sprintf("%s %s\n", DefaultIpv4Address, hostname)
-
-	if string(contents) != expect {
-		t.Fatalf("File has incorrect contents")
-	}
-}

initialize/update.go

@@ -1,165 +0,0 @@
-package initialize
-
-import (
-	"bufio"
-	"errors"
-	"fmt"
-	"os"
-	"path"
-	"strings"
-
-	"github.com/coreos/coreos-cloudinit/system"
-)
-
-const (
-	locksmithUnit    = "locksmithd.service"
-	updateEngineUnit = "update-engine.service"
-)
-
-// updateOption represents a configurable update option, which, if set, will be
-// written into update.conf, replacing any existing value for the option
-type updateOption struct {
-	key    string   // key used to configure this option in cloud-config
-	valid  []string // valid values for the option
-	prefix string   // prefix for the option in the update.conf file
-	value  string   // used to store the new value in update.conf (including prefix)
-	seen   bool     // whether the option has been seen in any existing update.conf
-}
-
-// updateOptions defines the update options understood by cloud-config.
-// The keys represent the string used in cloud-config to configure the option.
-var updateOptions = []*updateOption{
-	&updateOption{
-		key:    "reboot-strategy",
-		prefix: "REBOOT_STRATEGY=",
-		valid:  []string{"best-effort", "etcd-lock", "reboot", "off"},
-	},
-	&updateOption{
-		key:    "group",
-		prefix: "GROUP=",
-	},
-	&updateOption{
-		key:    "server",
-		prefix: "SERVER=",
-	},
-}
-
-// isValid checks whether a supplied value is valid for this option
-func (uo updateOption) isValid(val string) bool {
-	if len(uo.valid) == 0 {
-		return true
-	}
-	for _, v := range uo.valid {
-		if val == v {
-			return true
-		}
-	}
-	return false
-}
-
-type UpdateConfig map[string]string
-
-// File generates an `/etc/coreos/update.conf` file (if any update
-// configuration options are set in cloud-config) by either rewriting the
-// existing file on disk, or starting from `/usr/share/coreos/update.conf`
-func (uc UpdateConfig) File(root string) (*system.File, error) {
-	if len(uc) < 1 {
-		return nil, nil
-	}
-
-	var out string
-
-	// Generate the list of possible substitutions to be performed based on the options that are configured
-	subs := make([]*updateOption, 0)
-	for _, uo := range updateOptions {
-		val, ok := uc[uo.key]
-		if !ok {
-			continue
-		}
-		if !uo.isValid(val) {
-			return nil, errors.New(fmt.Sprintf("invalid value %v for option %v (valid options: %v)", val, uo.key, uo.valid))
-		}
-		uo.value = uo.prefix + val
-		subs = append(subs, uo)
-	}
-
-	etcUpdate := path.Join(root, "etc", "coreos", "update.conf")
-	usrUpdate := path.Join(root, "usr", "share", "coreos", "update.conf")
-
-	conf, err := os.Open(etcUpdate)
-	if os.IsNotExist(err) {
-		conf, err = os.Open(usrUpdate)
-	}
-	if err != nil {
-		return nil, err
-	}
-
-	scanner := bufio.NewScanner(conf)
-
-	for scanner.Scan() {
-		line := scanner.Text()
-		for _, s := range subs {
-			if strings.HasPrefix(line, s.prefix) {
-				line = s.value
-				s.seen = true
-				break
-			}
-		}
-		out += line
-		out += "\n"
-		if err := scanner.Err(); err != nil {
-			return nil, err
-		}
-	}
-
-	for _, s := range subs {
-		if !s.seen {
-			out += s.value
-			out += "\n"
-		}
-	}
-
-	return &system.File{
-		Path:               path.Join("etc", "coreos", "update.conf"),
-		RawFilePermissions: "0644",
-		Content:            out,
-	}, nil
-}
-
-// Units generates units for the cloud-init initializer to act on:
-// - a locksmith system.Unit, if "reboot-strategy" was set in cloud-config
-// - an update_engine system.Unit, if "group" was set in cloud-config
-func (uc UpdateConfig) Units(root string) ([]system.Unit, error) {
-	var units []system.Unit
-	if strategy, ok := uc["reboot-strategy"]; ok {
-		ls := &system.Unit{
-			Name:    locksmithUnit,
-			Command: "restart",
-			Mask:    false,
-			Runtime: true,
-		}
-
-		if strategy == "off" {
-			ls.Command = "stop"
-			ls.Mask = true
-		}
-		units = append(units, *ls)
-	}
-
-	rue := false
-	if _, ok := uc["group"]; ok {
-		rue = true
-	}
-	if _, ok := uc["server"]; ok {
-		rue = true
-	}
-	if rue {
-		ue := system.Unit{
-			Name:    updateEngineUnit,
-			Command: "restart",
-		}
-		units = append(units, ue)
-	}
-
-	return units, nil
-}

initialize/update_test.go

@@ -1,232 +0,0 @@
-package initialize
-
-import (
-	"io/ioutil"
-	"os"
-	"path"
-	"sort"
-	"strings"
-	"testing"
-
-	"github.com/coreos/coreos-cloudinit/system"
-)
-
-const (
-	base = `SERVER=https://example.com
-GROUP=thegroupc`
-	configured = base + `
-REBOOT_STRATEGY=awesome
-`
-	expected = base + `
-REBOOT_STRATEGY=etcd-lock
-`
-)
-
-func setupFixtures(dir string) {
-	os.MkdirAll(path.Join(dir, "usr", "share", "coreos"), 0755)
-	os.MkdirAll(path.Join(dir, "run", "systemd", "system"), 0755)
-
-	ioutil.WriteFile(path.Join(dir, "usr", "share", "coreos", "update.conf"), []byte(base), 0644)
-}
-
-func TestEmptyUpdateConfig(t *testing.T) {
-	uc := &UpdateConfig{}
-	f, err := uc.File("")
-	if err != nil {
-		t.Error("unexpected error getting file from empty UpdateConfig")
-	}
-	if f != nil {
-		t.Errorf("getting file from empty UpdateConfig should have returned nil, got %v", f)
-	}
-	uu, err := uc.Units("")
-	if err != nil {
-		t.Error("unexpected error getting unit from empty UpdateConfig")
-	}
-	if len(uu) != 0 {
-		t.Errorf("getting unit from empty UpdateConfig should have returned zero units, got %d", len(uu))
-	}
-}
-
-func TestInvalidUpdateOptions(t *testing.T) {
-	uon := &updateOption{
-		key:    "numbers",
-		prefix: "numero_",
-		valid:  []string{"one", "two"},
-	}
-	uoa := &updateOption{
-		key:    "any_will_do",
-		prefix: "any_",
-	}
-
-	if !uon.isValid("one") {
-		t.Error("update option did not accept valid option \"one\"")
-	}
-	if uon.isValid("three") {
-		t.Error("update option accepted invalid option \"three\"")
-	}
-	for _, s := range []string{"one", "asdf", "foobarbaz"} {
-		if !uoa.isValid(s) {
-			t.Errorf("update option with no \"valid\" field did not accept %q", s)
-		}
-	}
-
-	uc := &UpdateConfig{"reboot-strategy": "wizzlewazzle"}
-	f, err := uc.File("")
-	if err == nil {
-		t.Errorf("File did not give an error on invalid UpdateOption")
-	}
-	if f != nil {
-		t.Errorf("File did not return a nil file on invalid UpdateOption")
-	}
-}
-
-func TestServerGroupOptions(t *testing.T) {
-	dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-")
-	if err != nil {
-		t.Fatalf("unable to create tempdir: %v", err)
-	}
-	defer os.RemoveAll(dir)
-	setupFixtures(dir)
-	u := &UpdateConfig{"group": "master", "server": "http://foo.com"}
-
-	want := `
-GROUP=master
-SERVER=http://foo.com`
-
-	f, err := u.File(dir)
-	if err != nil {
-		t.Errorf("unexpected error getting file from UpdateConfig: %v", err)
-	} else if f == nil {
-		t.Error("unexpectedly got empty file from UpdateConfig")
-	} else {
-		out := strings.Split(f.Content, "\n")
-		sort.Strings(out)
-		got := strings.Join(out, "\n")
-		if got != want {
-			t.Errorf("File has incorrect contents, got %v, want %v", got, want)
-		}
-	}
-
-	uu, err := u.Units(dir)
-	if err != nil {
-		t.Errorf("unexpected error getting units from UpdateConfig: %v", err)
-	} else if len(uu) != 1 {
-		t.Errorf("unexpected number of files returned from UpdateConfig: want 1, got %d", len(uu))
-	} else {
-		unit := uu[0]
-		if unit.Name != "update-engine.service" {
-			t.Errorf("bad name for generated unit: want update-engine.service, got %s", unit.Name)
-		}
-		if unit.Command != "restart" {
-			t.Errorf("bad command for generated unit: want restart, got %s", unit.Command)
-		}
-	}
-}
-
-func TestRebootStrategies(t *testing.T) {
-	dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-")
-	if err != nil {
-		t.Fatalf("Unable to create tempdir: %v", err)
-	}
-	defer os.RemoveAll(dir)
-	setupFixtures(dir)
-	strategies := []struct {
-		name     string
-		line     string
-		uMask    bool
-		uCommand string
-	}{
-		{"best-effort", "REBOOT_STRATEGY=best-effort", false, "restart"},
-		{"etcd-lock", "REBOOT_STRATEGY=etcd-lock", false, "restart"},
-		{"reboot", "REBOOT_STRATEGY=reboot", false, "restart"},
-		{"off", "REBOOT_STRATEGY=off", true, "stop"},
-	}
-	for _, s := range strategies {
-		uc := &UpdateConfig{"reboot-strategy": s.name}
-		f, err := uc.File(dir)
-		if err != nil {
-			t.Errorf("update failed to generate file for reboot-strategy=%v: %v", s.name, err)
-		} else if f == nil {
-			t.Errorf("generated empty file for reboot-strategy=%v", s.name)
-		} else {
-			seen := false
-			for _, line := range strings.Split(f.Content, "\n") {
-				if line == s.line {
-					seen = true
-					break
-				}
-			}
-			if !seen {
-				t.Errorf("couldn't find expected line %v for reboot-strategy=%v", s.line)
-			}
-		}
-		uu, err := uc.Units(dir)
-		if err != nil {
-			t.Errorf("failed to generate unit for reboot-strategy=%v!", s.name)
-		} else if len(uu) != 1 {
-			t.Errorf("unexpected number of units for reboot-strategy=%v: %d", s.name, len(uu))
-		} else {
-			u := uu[0]
-			if u.Name != locksmithUnit {
-				t.Errorf("unit generated for reboot strategy=%v had bad name: %v", s.name, u.Name)
-			}
-			if u.Mask != s.uMask {
-				t.Errorf("unit generated for reboot strategy=%v had bad mask: %t", s.name, u.Mask)
-			}
-			if u.Command != s.uCommand {
-				t.Errorf("unit generated for reboot strategy=%v had bad command: %v", s.name, u.Command)
-			}
-		}
-	}
-
-}
-
-func TestUpdateConfWrittenToDisk(t *testing.T) {
-	dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-")
-	if err != nil {
-		t.Fatalf("Unable to create tempdir: %v", err)
-	}
-	defer os.RemoveAll(dir)
-	setupFixtures(dir)
-
-	for i := 0; i < 2; i++ {
-		if i == 1 {
-			err = ioutil.WriteFile(path.Join(dir, "etc", "coreos", "update.conf"), []byte(configured), 0644)
-			if err != nil {
-				t.Fatal(err)
-			}
-		}
-		uc := &UpdateConfig{"reboot-strategy": "etcd-lock"}
-
-		f, err := uc.File(dir)
-		if err != nil {
-			t.Fatalf("Processing UpdateConfig failed: %v", err)
-		} else if f == nil {
-			t.Fatal("Unexpectedly got nil updateconfig file")
-		}
-
-		if _, err := system.WriteFile(f, dir); err != nil {
-			t.Fatalf("Error writing update config: %v", err)
-		}
-
-		fullPath := path.Join(dir, "etc", "coreos", "update.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)
-		}
-
-		if string(contents) != expected {
-			t.Fatalf("File has incorrect contents, got %v, wanted %v", string(contents), expected)
-		}
-	}
-}

system/etc_hosts.go

@@ -1,4 +1,4 @@
-package initialize
+package system
 
 import (
 	"errors"
@@ -6,15 +6,17 @@ import (
 	"os"
 	"path"
 
-	"github.com/coreos/coreos-cloudinit/system"
+	"github.com/coreos/coreos-cloudinit/config"
 )
 
 const DefaultIpv4Address = "127.0.0.1"
 
-type EtcHosts string
+type EtcHosts struct {
+	Config config.EtcHosts
+}
 
 func (eh EtcHosts) generateEtcHosts() (out string, err error) {
-	if eh != "localhost" {
+	if eh.Config != "localhost" {
 		return "", errors.New("Invalid option to manage_etc_hosts")
 	}
 
@@ -28,8 +30,8 @@ func (eh EtcHosts) generateEtcHosts() (out string, err error) {
 
 }
 
-func (eh EtcHosts) File(root string) (*system.File, error) {
+func (eh EtcHosts) File() (*File, error) {
-	if eh == "" {
+	if eh.Config == "" {
 		return nil, nil
 	}
 
@@ -38,7 +40,7 @@ func (eh EtcHosts) File(root string) (*system.File, error) {
 		return nil, err
 	}
 
-	return &system.File{
+	return &File{
 		Path:               path.Join("etc", "hosts"),
 		RawFilePermissions: "0644",
 		Content:            etcHosts,

system/etc_hosts_test.go

@@ -0,0 +1,46 @@
+package system
+
+import (
+	"fmt"
+	"os"
+	"reflect"
+	"testing"
+
+	"github.com/coreos/coreos-cloudinit/config"
+)
+
+func TestEtcdHostsFile(t *testing.T) {
+	hostname, err := os.Hostname()
+	if err != nil {
+		panic(err)
+	}
+
+	for _, tt := range []struct {
+		config config.EtcHosts
+		file   *File
+		err    error
+	}{
+		{
+			"invalid",
+			nil,
+			fmt.Errorf("Invalid option to manage_etc_hosts"),
+		},
+		{
+			"localhost",
+			&File{
+				Content:            fmt.Sprintf("127.0.0.1 %s\n", hostname),
+				Path:               "etc/hosts",
+				RawFilePermissions: "0644",
+			},
+			nil,
+		},
+	} {
+		file, err := EtcHosts{tt.config}.File()
+		if !reflect.DeepEqual(tt.err, err) {
+			t.Errorf("bad error (%q): want %q, got %q", tt.config, tt.err, err)
+		}
+		if !reflect.DeepEqual(tt.file, file) {
+			t.Errorf("bad units (%q): want %#v, got %#v", tt.config, tt.file, file)
+		}
+	}
+}

system/etcd.go

@@ -11,7 +11,7 @@ type Etcd struct {
 }
 
 // Units creates a Unit file drop-in for etcd, using any configured options.
-func (ee Etcd) Units(_ string) ([]Unit, error) {
+func (ee Etcd) Units() ([]Unit, error) {
 	content := dropinContents(ee.Etcd)
 	if content == "" {
 		return nil, nil

system/etcd_test.go

@@ -49,7 +49,7 @@ Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002"
 			}},
 		},
 	} {
-		units, err := Etcd{tt.config}.Units("")
+		units, err := Etcd{tt.config}.Units()
 		if err != nil {
 			t.Errorf("bad error (%q): want %q, got %q", tt.config, nil, err)
 		}

system/fleet.go

@@ -12,7 +12,7 @@ type Fleet struct {
 
 // Units generates a Unit file drop-in for fleet, if any fleet options were
 // configured in cloud-config
-func (fe Fleet) Units(_ string) ([]Unit, error) {
+func (fe Fleet) Units() ([]Unit, error) {
 	content := dropinContents(fe.Fleet)
 	if content == "" {
 		return nil, nil

system/fleet_test.go

@@ -30,7 +30,7 @@ Environment="FLEET_PUBLIC_IP=12.34.56.78"
 			}},
 		},
 	} {
-		units, err := Fleet{tt.config}.Units("")
+		units, err := Fleet{tt.config}.Units()
 		if err != nil {
 			t.Errorf("bad error (%q): want %q, got %q", tt.config, nil, err)
 		}

system/oem.go

@@ -13,7 +13,7 @@ type OEM struct {
 	config.OEM
 }
 
-func (oem OEM) File(_ string) (*File, error) {
+func (oem OEM) File() (*File, error) {
 	if oem.ID == "" {
 		return nil, nil
 	}

system/oem_test.go

@@ -36,7 +36,7 @@ BUG_REPORT_URL="https://github.com/coreos/coreos-overlay"
 			},
 		},
 	} {
-		file, err := OEM{tt.config}.File("")
+		file, err := OEM{tt.config}.File()
 		if err != nil {
 			t.Errorf("bad error (%q): want %q, got %q", tt.config, nil, err)
 		}

system/update.go

@@ -0,0 +1,137 @@
+package system
+
+import (
+	"bufio"
+	"fmt"
+	"io"
+	"os"
+	"path"
+	"reflect"
+	"sort"
+	"strings"
+
+	"github.com/coreos/coreos-cloudinit/config"
+)
+
+const (
+	locksmithUnit    = "locksmithd.service"
+	updateEngineUnit = "update-engine.service"
+)
+
+// Update is a top-level structure which contains its underlying configuration,
+// config.Update, a function for reading the configuration (the default
+// implementation reading from the filesystem), and provides the system-specific
+// File() and Unit().
+type Update struct {
+	Config     config.Update
+	ReadConfig func() (io.Reader, error)
+}
+
+func DefaultReadConfig() (io.Reader, error) {
+	etcUpdate := path.Join("/etc", "coreos", "update.conf")
+	usrUpdate := path.Join("/usr", "share", "coreos", "update.conf")
+
+	f, err := os.Open(etcUpdate)
+	if os.IsNotExist(err) {
+		f, err = os.Open(usrUpdate)
+	}
+	return f, err
+}
+
+// File generates an `/etc/coreos/update.conf` file (if any update
+// configuration options are set in cloud-config) by either rewriting the
+// existing file on disk, or starting from `/usr/share/coreos/update.conf`
+func (uc Update) File() (*File, error) {
+	if config.IsZero(uc.Config) {
+		return nil, nil
+	}
+	if err := config.AssertValid(uc.Config); err != nil {
+		return nil, err
+	}
+
+	// Generate the list of possible substitutions to be performed based on the options that are configured
+	subs := map[string]string{}
+	uct := reflect.TypeOf(uc.Config)
+	ucv := reflect.ValueOf(uc.Config)
+	for i := 0; i < uct.NumField(); i++ {
+		val := ucv.Field(i).String()
+		if val == "" {
+			continue
+		}
+		env := uct.Field(i).Tag.Get("env")
+		subs[env] = fmt.Sprintf("%s=%s", env, val)
+	}
+
+	conf, err := uc.ReadConfig()
+	if err != nil {
+		return nil, err
+	}
+	scanner := bufio.NewScanner(conf)
+
+	var out string
+	for scanner.Scan() {
+		line := scanner.Text()
+		for env, value := range subs {
+			if strings.HasPrefix(line, env) {
+				line = value
+				delete(subs, env)
+				break
+			}
+		}
+		out += line
+		out += "\n"
+		if err := scanner.Err(); err != nil {
+			return nil, err
+		}
+	}
+
+	for _, key := range sortedKeys(subs) {
+		out += subs[key]
+		out += "\n"
+	}
+
+	return &File{
+		Path:               path.Join("etc", "coreos", "update.conf"),
+		RawFilePermissions: "0644",
+		Content:            out,
+	}, nil
+}
+
+// Units generates units for the cloud-init initializer to act on:
+// - a locksmith Unit, if "reboot-strategy" was set in cloud-config
+// - an update_engine Unit, if "group" or "server" was set in cloud-config
+func (uc Update) Units() ([]Unit, error) {
+	var units []Unit
+	if uc.Config.RebootStrategy != "" {
+		ls := &Unit{
+			Name:    locksmithUnit,
+			Command: "restart",
+			Mask:    false,
+			Runtime: true,
+		}
+
+		if uc.Config.RebootStrategy == "off" {
+			ls.Command = "stop"
+			ls.Mask = true
+		}
+		units = append(units, *ls)
+	}
+
+	if uc.Config.Group != "" || uc.Config.Server != "" {
+		ue := Unit{
+			Name:    updateEngineUnit,
+			Command: "restart",
+		}
+		units = append(units, ue)
+	}
+
+	return units, nil
+}
+
+func sortedKeys(m map[string]string) (keys []string) {
+	for key := range m {
+		keys = append(keys, key)
+	}
+	sort.Strings(keys)
+	return
+}

system/update_test.go

@@ -0,0 +1,151 @@
+package system
+
+import (
+	"errors"
+	"io"
+	"reflect"
+	"strings"
+	"testing"
+
+	"github.com/coreos/coreos-cloudinit/config"
+)
+
+func testReadConfig(config string) func() (io.Reader, error) {
+	return func() (io.Reader, error) {
+		return strings.NewReader(config), nil
+	}
+}
+
+func TestUpdateUnits(t *testing.T) {
+	for _, tt := range []struct {
+		config config.Update
+		units  []Unit
+		err    error
+	}{
+		{
+			config: config.Update{},
+		},
+		{
+			config: config.Update{Group: "master", Server: "http://foo.com"},
+			units: []Unit{{
+				Name:    "update-engine.service",
+				Command: "restart",
+			}},
+		},
+		{
+			config: config.Update{RebootStrategy: "best-effort"},
+			units: []Unit{{
+				Name:    "locksmithd.service",
+				Command: "restart",
+				Runtime: true,
+			}},
+		},
+		{
+			config: config.Update{RebootStrategy: "etcd-lock"},
+			units: []Unit{{
+				Name:    "locksmithd.service",
+				Command: "restart",
+				Runtime: true,
+			}},
+		},
+		{
+			config: config.Update{RebootStrategy: "reboot"},
+			units: []Unit{{
+				Name:    "locksmithd.service",
+				Command: "restart",
+				Runtime: true,
+			}},
+		},
+		{
+			config: config.Update{RebootStrategy: "off"},
+			units: []Unit{{
+				Name:    "locksmithd.service",
+				Command: "stop",
+				Runtime: true,
+				Mask:    true,
+			}},
+		},
+	} {
+		units, err := Update{tt.config, testReadConfig("")}.Units()
+		if !reflect.DeepEqual(tt.err, err) {
+			t.Errorf("bad error (%q): want %q, got %q", tt.config, tt.err, err)
+		}
+		if !reflect.DeepEqual(tt.units, units) {
+			t.Errorf("bad units (%q): want %#v, got %#v", tt.config, tt.units, units)
+		}
+	}
+}
+
+func TestUpdateFile(t *testing.T) {
+	for _, tt := range []struct {
+		config config.Update
+		orig   string
+		file   *File
+		err    error
+	}{
+		{
+			config: config.Update{},
+		},
+		{
+			config: config.Update{RebootStrategy: "wizzlewazzle"},
+			err:    errors.New("invalid value \"wizzlewazzle\" for option \"RebootStrategy\" (valid options: \"best-effort,etcd-lock,reboot,off\")"),
+		},
+		{
+			config: config.Update{Group: "master", Server: "http://foo.com"},
+			file: &File{
+				Content:            "GROUP=master\nSERVER=http://foo.com\n",
+				Path:               "etc/coreos/update.conf",
+				RawFilePermissions: "0644",
+			},
+		},
+		{
+			config: config.Update{RebootStrategy: "best-effort"},
+			file: &File{
+				Content:            "REBOOT_STRATEGY=best-effort\n",
+				Path:               "etc/coreos/update.conf",
+				RawFilePermissions: "0644",
+			},
+		},
+		{
+			config: config.Update{RebootStrategy: "etcd-lock"},
+			file: &File{
+				Content:            "REBOOT_STRATEGY=etcd-lock\n",
+				Path:               "etc/coreos/update.conf",
+				RawFilePermissions: "0644",
+			},
+		},
+		{
+			config: config.Update{RebootStrategy: "reboot"},
+			file: &File{
+				Content:            "REBOOT_STRATEGY=reboot\n",
+				Path:               "etc/coreos/update.conf",
+				RawFilePermissions: "0644",
+			},
+		},
+		{
+			config: config.Update{RebootStrategy: "off"},
+			file: &File{
+				Content:            "REBOOT_STRATEGY=off\n",
+				Path:               "etc/coreos/update.conf",
+				RawFilePermissions: "0644",
+			},
+		},
+		{
+			config: config.Update{RebootStrategy: "etcd-lock"},
+			orig:   "SERVER=https://example.com\nGROUP=thegroupc\nREBOOT_STRATEGY=awesome",
+			file: &File{
+				Content:            "SERVER=https://example.com\nGROUP=thegroupc\nREBOOT_STRATEGY=etcd-lock\n",
+				Path:               "etc/coreos/update.conf",
+				RawFilePermissions: "0644",
+			},
+		},
+	} {
+		file, err := Update{tt.config, testReadConfig(tt.orig)}.File()
+		if !reflect.DeepEqual(tt.err, err) {
+			t.Errorf("bad error (%q): want %q, got %q", tt.config, tt.err, err)
+		}
+		if !reflect.DeepEqual(tt.file, file) {
+			t.Errorf("bad units (%q): want %#v, got %#v", tt.config, tt.file, file)
+		}
+	}
+}

test

@@ -13,19 +13,22 @@ COVER=${COVER:-"-cover"}
 
 source ./build
 
-declare -a TESTPKGS=(initialize
+declare -a TESTPKGS=(
-                     system
+	config
-                     datasource
+	datasource
-                     datasource/configdrive
+	datasource/configdrive
-                     datasource/file
+	datasource/file
-                     datasource/metadata
+	datasource/metadata
-                     datasource/metadata/cloudsigma
+	datasource/metadata/cloudsigma
-                     datasource/metadata/digitalocean
+	datasource/metadata/digitalocean
-                     datasource/metadata/ec2
+	datasource/metadata/ec2
-                     datasource/proc_cmdline
+	datasource/proc_cmdline
-                     datasource/url
+	datasource/url
-                     pkg
+	initialize
-                     network)
+	network
+	pkg
+	system
+)
 
 if [ -z "$PKG" ]; then
 	GOFMTPATH="${TESTPKGS[*]} coreos-cloudinit.go"