From be51f4eba0887b0894d40c6857396a7243c6ef25 Mon Sep 17 00:00:00 2001 From: Jonathan Boulle Date: Thu, 5 Jun 2014 17:40:53 -0700 Subject: [PATCH] chore(*): split out unit processing from config.Apply --- initialize/config.go | 26 ++++-- initialize/config_test.go | 108 ++++++++++++++++++++++++ initialize/etcd_test.go | 8 +- system/networkd.go | 2 +- system/systemd.go | 168 ++++++++++++++------------------------ system/systemd_test.go | 22 +++-- system/unit.go | 67 +++++++++++++++ 7 files changed, 278 insertions(+), 123 deletions(-) create mode 100644 system/unit.go diff --git a/initialize/config.go b/initialize/config.go index 8c08e6e..cddace7 100644 --- a/initialize/config.go +++ b/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 { - dst := unit.Destination(env.Root()) + for _, unit := range units { + 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 } diff --git a/initialize/config_test.go b/initialize/config_test.go index 6db2949..f2c5b73 100644 --- a/initialize/config_test.go +++ b/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) + } +} diff --git a/initialize/etcd_test.go b/initialize/etcd_test.go index 101cd68..4e349d9 100644 --- a/initialize/etcd_test.go +++ b/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) } diff --git a/system/networkd.go b/system/networkd.go index a982869..e5d88aa 100644 --- a/system/networkd.go +++ b/system/networkd.go @@ -60,7 +60,7 @@ func probe8012q() error { } func restartNetworkd() error { - _, err := RunUnitCommand("restart", "systemd-networkd.service") + _, err := NewUnitManager("").RunUnitCommand("restart", "systemd-networkd.service") return err } diff --git a/system/systemd.go b/system/systemd.go index 1b478fd..73e90ad 100644 --- a/system/systemd.go +++ b/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 -} diff --git a/system/systemd_test.go b/system/systemd_test.go index e715396..de6d4e8 100644 --- a/system/systemd_test.go +++ b/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) { diff --git a/system/unit.go b/system/unit.go new file mode 100644 index 0000000..df4d384 --- /dev/null +++ b/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) + } +}