Merge pull request #180 from marineam/systemd_testing

chore(*): split out unit processing from config.Apply

initialize/config.go

@@ -272,13 +272,23 @@ func Apply(cfg CloudConfig, env *Environment) error {
 		}
 	}
 
+	um := system.NewUnitManager(env.Root())
+	return processUnits(cfg.Coreos.Units, env.Root(), um)
+
+}
+
+// processUnits takes a set of Units and applies them to the given root using
+// the given UnitManager. This can involve things like writing unit files to
+// disk, masking/unmasking units, or invoking systemd
+// commands against units. It returns any error encountered.
+func processUnits(units []system.Unit, root string, um system.UnitManager) error {
 	commands := make(map[string]string, 0)
 	reload := false
-	for _, unit := range cfg.Coreos.Units {
+	for _, unit := range units {
-		dst := unit.Destination(env.Root())
+		dst := unit.Destination(root)
 		if unit.Content != "" {
 			log.Printf("Writing unit %s to filesystem at path %s", unit.Name, dst)
-			if err := system.PlaceUnit(&unit, dst); err != nil {
+			if err := um.PlaceUnit(&unit, dst); err != nil {
 				return err
 			}
 			log.Printf("Placed unit %s at %s", unit.Name, dst)
@@ -287,12 +297,12 @@ func Apply(cfg CloudConfig, env *Environment) error {
 
 		if unit.Mask {
 			log.Printf("Masking unit file %s", unit.Name)
-			if err := system.MaskUnit(&unit, env.Root()); err != nil {
+			if err := um.MaskUnit(&unit); err != nil {
 				return err
 			}
 		} else if unit.Runtime {
 			log.Printf("Ensuring runtime unit file %s is unmasked", unit.Name)
-			if err := system.UnmaskUnit(&unit, env.Root()); err != nil {
+			if err := um.UnmaskUnit(&unit); err != nil {
 				return err
 			}
 		}
@@ -300,7 +310,7 @@ func Apply(cfg CloudConfig, env *Environment) error {
 		if unit.Enable {
 			if unit.Group() != "network" {
 				log.Printf("Enabling unit file %s", unit.Name)
-				if err := system.EnableUnitFile(unit.Name, unit.Runtime); err != nil {
+				if err := um.EnableUnitFile(unit.Name, unit.Runtime); err != nil {
 					return err
 				}
 				log.Printf("Enabled unit %s", unit.Name)
@@ -317,14 +327,14 @@ func Apply(cfg CloudConfig, env *Environment) error {
 	}
 
 	if reload {
-		if err := system.DaemonReload(); err != nil {
+		if err := um.DaemonReload(); err != nil {
 			return errors.New(fmt.Sprintf("failed systemd daemon-reload: %v", err))
 		}
 	}
 
 	for unit, command := range commands {
 		log.Printf("Calling unit command '%s %s'", command, unit)
-		res, err := system.RunUnitCommand(command, unit)
+		res, err := um.RunUnitCommand(command, unit)
 		if err != nil {
 			return err
 		}

initialize/config_test.go

@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"strings"
 	"testing"
+
+	"github.com/coreos/coreos-cloudinit/system"
 )
 
 func TestCloudConfigUnknownKeys(t *testing.T) {
@@ -332,3 +334,109 @@ users:
 		t.Errorf("Failed to parse no-log-init field")
 	}
 }
+
+type TestUnitManager struct {
+	placed   []string
+	enabled  []string
+	masked   []string
+	unmasked []string
+	commands map[string]string
+	reload   bool
+}
+
+func (tum *TestUnitManager) PlaceUnit(unit *system.Unit, dst string) error {
+	tum.placed = append(tum.placed, unit.Name)
+	return nil
+}
+func (tum *TestUnitManager) EnableUnitFile(unit string, runtime bool) error {
+	tum.enabled = append(tum.enabled, unit)
+	return nil
+}
+func (tum *TestUnitManager) RunUnitCommand(command, unit string) (string, error) {
+	tum.commands = make(map[string]string)
+	tum.commands[unit] = command
+	return "", nil
+}
+func (tum *TestUnitManager) DaemonReload() error {
+	tum.reload = true
+	return nil
+}
+func (tum *TestUnitManager) MaskUnit(unit *system.Unit) error {
+	tum.masked = append(tum.masked, unit.Name)
+	return nil
+}
+func (tum *TestUnitManager) UnmaskUnit(unit *system.Unit) error {
+	tum.unmasked = append(tum.unmasked, unit.Name)
+	return nil
+}
+
+func TestProcessUnits(t *testing.T) {
+	tum := &TestUnitManager{}
+	units := []system.Unit{
+		system.Unit{
+			Name: "foo",
+			Mask: true,
+		},
+	}
+	if err := processUnits(units, "", tum); err != nil {
+		t.Fatalf("unexpected error calling processUnits: %v", err)
+	}
+	if len(tum.masked) != 1 || tum.masked[0] != "foo" {
+		t.Errorf("expected foo to be masked, but found %v", tum.masked)
+	}
+
+	tum = &TestUnitManager{}
+	units = []system.Unit{
+		system.Unit{
+			Name: "bar.network",
+		},
+	}
+	if err := processUnits(units, "", tum); err != nil {
+		t.Fatalf("unexpected error calling processUnits: %v", err)
+	}
+	if _, ok := tum.commands["systemd-networkd.service"]; !ok {
+		t.Errorf("expected systemd-networkd.service to be reloaded!")
+	}
+
+	tum = &TestUnitManager{}
+	units = []system.Unit{
+		system.Unit{
+			Name:    "baz.service",
+			Content: "[Service]\nExecStart=/bin/true",
+		},
+	}
+	if err := processUnits(units, "", tum); err != nil {
+		t.Fatalf("unexpected error calling processUnits: %v", err)
+	}
+	if len(tum.placed) != 1 || tum.placed[0] != "baz.service" {
+		t.Fatalf("expected baz.service to be written, but got %v", tum.placed)
+	}
+
+	tum = &TestUnitManager{}
+	units = []system.Unit{
+		system.Unit{
+			Name:    "locksmithd.service",
+			Runtime: true,
+		},
+	}
+	if err := processUnits(units, "", tum); err != nil {
+		t.Fatalf("unexpected error calling processUnits: %v", err)
+	}
+	if len(tum.unmasked) != 1 || tum.unmasked[0] != "locksmithd.service" {
+		t.Fatalf("expected locksmithd.service to be unmasked, but got %v", tum.unmasked)
+	}
+
+	tum = &TestUnitManager{}
+	units = []system.Unit{
+		system.Unit{
+			Name:   "woof",
+			Enable: true,
+		},
+	}
+	if err := processUnits(units, "", tum); err != nil {
+		t.Fatalf("unexpected error calling processUnits: %v", err)
+	}
+	if len(tum.enabled) != 1 || tum.enabled[0] != "woof" {
+		t.Fatalf("expected woof to be enabled, but got %v", tum.enabled)
+	}
+}

initialize/etcd_test.go

@@ -70,6 +70,8 @@ func TestEtcdEnvironmentWrittenToDisk(t *testing.T) {
 	}
 	defer os.RemoveAll(dir)
 
+	sd := system.NewUnitManager(dir)
+
 	uu, err := ee.Units(dir)
 	if err != nil {
 		t.Fatalf("Generating etcd unit failed: %v", err)
@@ -81,7 +83,7 @@ func TestEtcdEnvironmentWrittenToDisk(t *testing.T) {
 
 	dst := u.Destination(dir)
 	os.Stderr.WriteString("writing to " + dir + "\n")
-	if err := system.PlaceUnit(&u, dst); err != nil {
+	if err := sd.PlaceUnit(&u, dst); err != nil {
 		t.Fatalf("Writing of EtcdEnvironment failed: %v", err)
 	}
 
@@ -119,6 +121,8 @@ func TestEtcdEnvironmentWrittenToDiskDefaultToMachineID(t *testing.T) {
 	}
 	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 {
@@ -136,7 +140,7 @@ func TestEtcdEnvironmentWrittenToDiskDefaultToMachineID(t *testing.T) {
 
 	dst := u.Destination(dir)
 	os.Stderr.WriteString("writing to " + dir + "\n")
-	if err := system.PlaceUnit(&u, dst); err != nil {
+	if err := sd.PlaceUnit(&u, dst); err != nil {
 		t.Fatalf("Writing of EtcdEnvironment failed: %v", err)
 	}
 

system/networkd.go

@@ -86,7 +86,7 @@ func maybeProbeBonding(interfaces []network.InterfaceGenerator) error {
 }
 
 func restartNetworkd() error {
-	_, err := RunUnitCommand("restart", "systemd-networkd.service")
+	_, err := NewUnitManager("").RunUnitCommand("restart", "systemd-networkd.service")
 	return err
 }
 

system/systemd.go

@@ -13,63 +13,21 @@ import (
 	"github.com/coreos/coreos-cloudinit/third_party/github.com/coreos/go-systemd/dbus"
 )
 
+func NewUnitManager(root string) UnitManager {
+	return &systemd{root}
+}
+
+type systemd struct {
+	root string
+}
+
 // fakeMachineID is placed on non-usr CoreOS images and should
 // never be used as a true MachineID
 const fakeMachineID = "42000000000000000000000000000042"
 
-// Name for drop-in service configuration files created by cloudconfig
-const cloudConfigDropIn = "20-cloudinit.conf"
-
-type Unit struct {
-	Name    string
-	Mask    bool
-	Enable  bool
-	Runtime bool
-	Content string
-	Command string
-
-	// For drop-in units, a cloudinit.conf is generated.
-	// This is currently unbound in YAML (and hence unsettable in cloud-config files)
-	// until the correct behaviour for multiple drop-in units is determined.
-	DropIn bool `yaml:"-"`
-}
-
-func (u *Unit) Type() string {
-	ext := filepath.Ext(u.Name)
-	return strings.TrimLeft(ext, ".")
-}
-
-func (u *Unit) Group() (group string) {
-	t := u.Type()
-	if t == "network" || t == "netdev" || t == "link" {
-		group = "network"
-	} else {
-		group = "system"
-	}
-	return
-}
-
-type Script []byte
-
-// Destination builds the appropriate absolute file path for
-// the Unit. The root argument indicates the effective base
-// directory of the system (similar to a chroot).
-func (u *Unit) Destination(root string) string {
-	dir := "etc"
-	if u.Runtime {
-		dir = "run"
-	}
-
-	if u.DropIn {
-		return path.Join(root, dir, "systemd", u.Group(), fmt.Sprintf("%s.d", u.Name), cloudConfigDropIn)
-	} else {
-		return path.Join(root, dir, "systemd", u.Group(), u.Name)
-	}
-}
-
 // PlaceUnit writes a unit file at the provided destination, creating
 // parent directories as necessary.
-func PlaceUnit(u *Unit, dst string) error {
+func (s *systemd) PlaceUnit(u *Unit, dst string) error {
 	dir := filepath.Dir(dst)
 	if _, err := os.Stat(dir); os.IsNotExist(err) {
 		if err := os.MkdirAll(dir, os.FileMode(0755)); err != nil {
@@ -91,7 +49,7 @@ func PlaceUnit(u *Unit, dst string) error {
 	return nil
 }
 
-func EnableUnitFile(unit string, runtime bool) error {
+func (s *systemd) EnableUnitFile(unit string, runtime bool) error {
 	conn, err := dbus.New()
 	if err != nil {
 		return err
@@ -102,7 +60,7 @@ func EnableUnitFile(unit string, runtime bool) error {
 	return err
 }
 
-func RunUnitCommand(command, unit string) (string, error) {
+func (s *systemd) RunUnitCommand(command, unit string) (string, error) {
 	conn, err := dbus.New()
 	if err != nil {
 		return "", err
@@ -131,7 +89,7 @@ func RunUnitCommand(command, unit string) (string, error) {
 	return fn(unit, "replace")
 }
 
-func DaemonReload() error {
+func (s *systemd) DaemonReload() error {
 	conn, err := dbus.New()
 	if err != nil {
 		return err
@@ -140,6 +98,57 @@ func DaemonReload() error {
 	return conn.Reload()
 }
 
+// MaskUnit masks the given Unit by symlinking its unit file to
+// /dev/null, analogous to `systemctl mask`.
+// N.B.: Unlike `systemctl mask`, this function will *remove any existing unit
+// file at the location*, to ensure that the mask will succeed.
+func (s *systemd) MaskUnit(unit *Unit) error {
+	masked := unit.Destination(s.root)
+	if _, err := os.Stat(masked); os.IsNotExist(err) {
+		if err := os.MkdirAll(path.Dir(masked), os.FileMode(0755)); err != nil {
+			return err
+		}
+	} else if err := os.Remove(masked); err != nil {
+		return err
+	}
+	return os.Symlink("/dev/null", masked)
+}
+
+// UnmaskUnit is analogous to systemd's unit_file_unmask. If the file
+// associated with the given Unit is empty or appears to be a symlink to
+// /dev/null, it is removed.
+func (s *systemd) UnmaskUnit(unit *Unit) error {
+	masked := unit.Destination(s.root)
+	ne, err := nullOrEmpty(masked)
+	if os.IsNotExist(err) {
+		return nil
+	} else if err != nil {
+		return err
+	}
+	if !ne {
+		log.Printf("%s is not null or empty, refusing to unmask", masked)
+		return nil
+	}
+	return os.Remove(masked)
+}
+
+// nullOrEmpty checks whether a given path appears to be an empty regular file
+// or a symlink to /dev/null
+func nullOrEmpty(path string) (bool, error) {
+	fi, err := os.Stat(path)
+	if err != nil {
+		return false, err
+	}
+	m := fi.Mode()
+	if m.IsRegular() && fi.Size() <= 0 {
+		return true, nil
+	}
+	if m&os.ModeCharDevice > 0 {
+		return true, nil
+	}
+	return false, nil
+}
+
 func ExecuteScript(scriptPath string) (string, error) {
 	props := []dbus.Property{
 		dbus.PropDescription("Unit generated and executed by coreos-cloudinit on behalf of user"),
@@ -178,54 +187,3 @@ func MachineID(root string) string {
 
 	return id
 }
-
-// MaskUnit masks the given Unit by symlinking its unit file to
-// /dev/null, analogous to `systemctl mask`.
-// N.B.: Unlike `systemctl mask`, this function will *remove any existing unit
-// file at the location*, to ensure that the mask will succeed.
-func MaskUnit(unit *Unit, root string) error {
-	masked := unit.Destination(root)
-	if _, err := os.Stat(masked); os.IsNotExist(err) {
-		if err := os.MkdirAll(path.Dir(masked), os.FileMode(0755)); err != nil {
-			return err
-		}
-	} else if err := os.Remove(masked); err != nil {
-		return err
-	}
-	return os.Symlink("/dev/null", masked)
-}
-
-// UnmaskUnit is analogous to systemd's unit_file_unmask. If the file
-// associated with the given Unit is empty or appears to be a symlink to
-// /dev/null, it is removed.
-func UnmaskUnit(unit *Unit, root string) error {
-	masked := unit.Destination(root)
-	ne, err := nullOrEmpty(masked)
-	if os.IsNotExist(err) {
-		return nil
-	} else if err != nil {
-		return err
-	}
-	if !ne {
-		log.Printf("%s is not null or empty, refusing to unmask", masked)
-		return nil
-	}
-	return os.Remove(masked)
-}
-
-// nullOrEmpty checks whether a given path appears to be an empty regular file
-// or a symlink to /dev/null
-func nullOrEmpty(path string) (bool, error) {
-	fi, err := os.Stat(path)
-	if err != nil {
-		return false, err
-	}
-	m := fi.Mode()
-	if m.IsRegular() && fi.Size() <= 0 {
-		return true, nil
-	}
-	if m&os.ModeCharDevice > 0 {
-		return true, nil
-	}
-	return false, nil
-}

system/systemd_test.go

@@ -25,13 +25,15 @@ Address=10.209.171.177/19
 	}
 	defer os.RemoveAll(dir)
 
+	sd := &systemd{dir}
+
 	dst := u.Destination(dir)
 	expectDst := path.Join(dir, "run", "systemd", "network", "50-eth0.network")
 	if dst != expectDst {
 		t.Fatalf("unit.Destination returned %s, expected %s", dst, expectDst)
 	}
 
-	if err := PlaceUnit(&u, dst); err != nil {
+	if err := sd.PlaceUnit(&u, dst); err != nil {
 		t.Fatalf("PlaceUnit failed: %v", err)
 	}
 
@@ -100,13 +102,15 @@ Where=/media/state
 	}
 	defer os.RemoveAll(dir)
 
+	sd := &systemd{dir}
+
 	dst := u.Destination(dir)
 	expectDst := path.Join(dir, "etc", "systemd", "system", "media-state.mount")
 	if dst != expectDst {
 		t.Fatalf("unit.Destination returned %s, expected %s", dst, expectDst)
 	}
 
-	if err := PlaceUnit(&u, dst); err != nil {
+	if err := sd.PlaceUnit(&u, dst); err != nil {
 		t.Fatalf("PlaceUnit failed: %v", err)
 	}
 
@@ -155,9 +159,11 @@ func TestMaskUnit(t *testing.T) {
 	}
 	defer os.RemoveAll(dir)
 
+	sd := &systemd{dir}
+
 	// Ensure mask works with units that do not currently exist
 	uf := &Unit{Name: "foo.service"}
-	if err := MaskUnit(uf, dir); err != nil {
+	if err := sd.MaskUnit(uf); err != nil {
 		t.Fatalf("Unable to mask new unit: %v", err)
 	}
 	fooPath := path.Join(dir, "etc", "systemd", "system", "foo.service")
@@ -175,7 +181,7 @@ func TestMaskUnit(t *testing.T) {
 	if _, err := os.Create(barPath); err != nil {
 		t.Fatalf("Error creating new unit file: %v", err)
 	}
-	if err := MaskUnit(ub, dir); err != nil {
+	if err := sd.MaskUnit(ub); err != nil {
 		t.Fatalf("Unable to mask existing unit: %v", err)
 	}
 	barTgt, err := os.Readlink(barPath)
@@ -194,8 +200,10 @@ func TestUnmaskUnit(t *testing.T) {
 	}
 	defer os.RemoveAll(dir)
 
+	sd := &systemd{dir}
+
 	nilUnit := &Unit{Name: "null.service"}
-	if err := UnmaskUnit(nilUnit, dir); err != nil {
+	if err := sd.UnmaskUnit(nilUnit); err != nil {
 		t.Errorf("unexpected error from unmasking nonexistent unit: %v", err)
 	}
 
@@ -211,7 +219,7 @@ func TestUnmaskUnit(t *testing.T) {
 	if err := ioutil.WriteFile(dst, []byte(uf.Content), 700); err != nil {
 		t.Fatalf("Unable to write unit file: %v", err)
 	}
-	if err := UnmaskUnit(uf, dir); err != nil {
+	if err := sd.UnmaskUnit(uf); err != nil {
 		t.Errorf("unmask of non-empty unit returned unexpected error: %v", err)
 	}
 	got, _ := ioutil.ReadFile(dst)
@@ -224,7 +232,7 @@ func TestUnmaskUnit(t *testing.T) {
 	if err := os.Symlink("/dev/null", dst); err != nil {
 		t.Fatalf("Unable to create masked unit: %v", err)
 	}
-	if err := UnmaskUnit(ub, dir); err != nil {
+	if err := sd.UnmaskUnit(ub); err != nil {
 		t.Errorf("unmask of unit returned unexpected error: %v", err)
 	}
 	if _, err := os.Stat(dst); !os.IsNotExist(err) {

system/unit.go

@@ -0,0 +1,67 @@
+package system
+
+import (
+	"fmt"
+	"path"
+	"path/filepath"
+	"strings"
+)
+
+// Name for drop-in service configuration files created by cloudconfig
+const cloudConfigDropIn = "20-cloudinit.conf"
+
+type UnitManager interface {
+	PlaceUnit(unit *Unit, dst string) error
+	EnableUnitFile(unit string, runtime bool) error
+	RunUnitCommand(command, unit string) (string, error)
+	DaemonReload() error
+	MaskUnit(unit *Unit) error
+	UnmaskUnit(unit *Unit) error
+}
+
+type Unit struct {
+	Name    string
+	Mask    bool
+	Enable  bool
+	Runtime bool
+	Content string
+	Command string
+
+	// For drop-in units, a cloudinit.conf is generated.
+	// This is currently unbound in YAML (and hence unsettable in cloud-config files)
+	// until the correct behaviour for multiple drop-in units is determined.
+	DropIn bool `yaml:"-"`
+}
+
+func (u *Unit) Type() string {
+	ext := filepath.Ext(u.Name)
+	return strings.TrimLeft(ext, ".")
+}
+
+func (u *Unit) Group() (group string) {
+	t := u.Type()
+	if t == "network" || t == "netdev" || t == "link" {
+		group = "network"
+	} else {
+		group = "system"
+	}
+	return
+}
+
+type Script []byte
+
+// Destination builds the appropriate absolute file path for
+// the Unit. The root argument indicates the effective base
+// directory of the system (similar to a chroot).
+func (u *Unit) Destination(root string) string {
+	dir := "etc"
+	if u.Runtime {
+		dir = "run"
+	}
+
+	if u.DropIn {
+		return path.Join(root, dir, "systemd", u.Group(), fmt.Sprintf("%s.d", u.Name), cloudConfigDropIn)
+	} else {
+		return path.Join(root, dir, "systemd", u.Group(), u.Name)
+	}
+}