From dcaabe4d4a8589e1bd73c506dc6f03c8302787fc Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Mon, 24 Nov 2014 16:42:31 -0800 Subject: [PATCH 1/5] system: clean up UnitManager interface --- initialize/config.go | 38 ++++++++++++++++---------------- initialize/config_test.go | 20 ++++++++--------- system/file.go | 9 +++++--- system/networkd.go | 3 ++- system/systemd.go | 46 +++++++++++++++------------------------ system/systemd_test.go | 14 ++++++------ system/unit.go | 18 +++++++-------- 7 files changed, 70 insertions(+), 78 deletions(-) diff --git a/initialize/config.go b/initialize/config.go index a2024ce..9e169ce 100644 --- a/initialize/config.go +++ b/initialize/config.go @@ -195,66 +195,66 @@ func Apply(cfg config.CloudConfig, env *Environment) error { // commands against units. It returns any error encountered. func processUnits(units []system.Unit, root string, um system.UnitManager) error { type action struct { - unit string + unit system.Unit command string } actions := make([]action, 0, len(units)) reload := false 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 := um.PlaceUnit(&unit, dst); err != nil { + log.Printf("Writing unit %q to filesystem", unit.Name) + if err := um.PlaceUnit(unit); err != nil { return err } - log.Printf("Placed unit %s at %s", unit.Name, dst) + log.Printf("Wrote unit %q", unit.Name) reload = true } if unit.Mask { - log.Printf("Masking unit file %s", unit.Name) - if err := um.MaskUnit(&unit); err != nil { + log.Printf("Masking unit file %q", unit.Name) + 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 := um.UnmaskUnit(&unit); err != nil { + log.Printf("Ensuring runtime unit file %q is unmasked", unit.Name) + if err := um.UnmaskUnit(unit); err != nil { return err } } if unit.Enable { if unit.Group() != "network" { - log.Printf("Enabling unit file %s", unit.Name) - if err := um.EnableUnitFile(unit.Name, unit.Runtime); err != nil { + log.Printf("Enabling unit file %q", unit.Name) + if err := um.EnableUnitFile(unit); err != nil { return err } - log.Printf("Enabled unit %s", unit.Name) + log.Printf("Enabled unit %q", unit.Name) } else { - log.Printf("Skipping enable for network-like unit %s", unit.Name) + log.Printf("Skipping enable for network-like unit %q", unit.Name) } } if unit.Group() == "network" { - actions = append(actions, action{"systemd-networkd.service", "restart"}) + networkd := system.Unit{Unit: config.Unit{Name: "systemd-networkd.service"}} + actions = append(actions, action{networkd, "restart"}) } else if unit.Command != "" { - actions = append(actions, action{unit.Name, unit.Command}) + actions = append(actions, action{unit, unit.Command}) } } if reload { if err := um.DaemonReload(); err != nil { - return errors.New(fmt.Sprintf("failed systemd daemon-reload: %v", err)) + return errors.New(fmt.Sprintf("failed systemd daemon-reload: %s", err)) } } for _, action := range actions { - log.Printf("Calling unit command '%s %s'", action.command, action.unit) - res, err := um.RunUnitCommand(action.command, action.unit) + log.Printf("Calling unit command %q on %q'", action.command, action.unit.Name) + res, err := um.RunUnitCommand(action.unit, action.command) if err != nil { return err } - log.Printf("Result of '%s %s': %s", action.command, action.unit, res) + log.Printf("Result of %q on %q': %s", action.command, action.unit.Name, res) } return nil diff --git a/initialize/config_test.go b/initialize/config_test.go index fd67d7e..10a5ae3 100644 --- a/initialize/config_test.go +++ b/initialize/config_test.go @@ -32,29 +32,29 @@ type TestUnitManager struct { reload bool } -func (tum *TestUnitManager) PlaceUnit(unit *system.Unit, dst string) error { - tum.placed = append(tum.placed, unit.Name) +func (tum *TestUnitManager) PlaceUnit(u system.Unit) error { + tum.placed = append(tum.placed, u.Name) return nil } -func (tum *TestUnitManager) EnableUnitFile(unit string, runtime bool) error { - tum.enabled = append(tum.enabled, unit) +func (tum *TestUnitManager) EnableUnitFile(u system.Unit) error { + tum.enabled = append(tum.enabled, u.Name) return nil } -func (tum *TestUnitManager) RunUnitCommand(command, unit string) (string, error) { +func (tum *TestUnitManager) RunUnitCommand(u system.Unit, c string) (string, error) { tum.commands = make(map[string]string) - tum.commands[unit] = command + tum.commands[u.Name] = c 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) +func (tum *TestUnitManager) MaskUnit(u system.Unit) error { + tum.masked = append(tum.masked, u.Name) return nil } -func (tum *TestUnitManager) UnmaskUnit(unit *system.Unit) error { - tum.unmasked = append(tum.unmasked, unit.Name) +func (tum *TestUnitManager) UnmaskUnit(u system.Unit) error { + tum.unmasked = append(tum.unmasked, u.Name) return nil } diff --git a/system/file.go b/system/file.go index f56ab10..ad8673f 100644 --- a/system/file.go +++ b/system/file.go @@ -19,6 +19,7 @@ package system import ( "fmt" "io/ioutil" + "log" "os" "os/exec" "path" @@ -47,13 +48,14 @@ func (f *File) Permissions() (os.FileMode, error) { } func WriteFile(f *File, root string) (string, error) { + fullpath := path.Join(root, f.Path) + dir := path.Dir(fullpath) + log.Printf("Writing file to %q", fullpath) + if f.Encoding != "" { return "", fmt.Errorf("Unable to write file with encoding %s", f.Encoding) } - fullpath := path.Join(root, f.Path) - dir := path.Dir(fullpath) - if err := EnsureDirectoryExists(dir); err != nil { return "", err } @@ -94,6 +96,7 @@ func WriteFile(f *File, root string) (string, error) { return "", err } + log.Printf("Wrote file to %q", fullpath) return fullpath, nil } diff --git a/system/networkd.go b/system/networkd.go index 528aaa2..056eb6a 100644 --- a/system/networkd.go +++ b/system/networkd.go @@ -96,7 +96,8 @@ func maybeProbeBonding(interfaces []network.InterfaceGenerator) error { func restartNetworkd() error { log.Printf("Restarting networkd.service\n") - _, err := NewUnitManager("").RunUnitCommand("restart", "systemd-networkd.service") + networkd := Unit{config.Unit{Name: "systemd-networkd.service"}} + _, err := NewUnitManager("").RunUnitCommand(networkd, "restart") return err } diff --git a/system/systemd.go b/system/systemd.go index 8200333..2c346a5 100644 --- a/system/systemd.go +++ b/system/systemd.go @@ -23,7 +23,6 @@ import ( "os" "os/exec" "path" - "path/filepath" "strings" "github.com/coreos/coreos-cloudinit/Godeps/_workspace/src/github.com/coreos/go-systemd/dbus" @@ -42,49 +41,38 @@ type systemd struct { // never be used as a true MachineID const fakeMachineID = "42000000000000000000000000000042" -// PlaceUnit writes a unit file at the provided destination, creating -// parent directories as necessary. -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 { - return err - } - } - +// PlaceUnit writes a unit file at its desired destination, creating parent +// directories as necessary. +func (s *systemd) PlaceUnit(u Unit) error { file := File{config.File{ - Path: filepath.Base(dst), + Path: u.Destination(s.root), Content: u.Content, RawFilePermissions: "0644", }} - _, err := WriteFile(&file, dir) - if err != nil { - return err - } - - return nil + _, err := WriteFile(&file, "/") + return err } -func (s *systemd) EnableUnitFile(unit string, runtime bool) error { +func (s *systemd) EnableUnitFile(u Unit) error { conn, err := dbus.New() if err != nil { return err } - units := []string{unit} - _, _, err = conn.EnableUnitFiles(units, runtime, true) + units := []string{u.Name} + _, _, err = conn.EnableUnitFiles(units, u.Runtime, true) return err } -func (s *systemd) RunUnitCommand(command, unit string) (string, error) { +func (s *systemd) RunUnitCommand(u Unit, c string) (string, error) { conn, err := dbus.New() if err != nil { return "", err } var fn func(string, string) (string, error) - switch command { + switch c { case "start": fn = conn.StartUnit case "stop": @@ -100,10 +88,10 @@ func (s *systemd) RunUnitCommand(command, unit string) (string, error) { case "reload-or-try-restart": fn = conn.ReloadOrTryRestartUnit default: - return "", fmt.Errorf("Unsupported systemd command %q", command) + return "", fmt.Errorf("Unsupported systemd command %q", c) } - return fn(unit, "replace") + return fn(u.Name, "replace") } func (s *systemd) DaemonReload() error { @@ -119,8 +107,8 @@ func (s *systemd) DaemonReload() error { // /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) +func (s *systemd) MaskUnit(u Unit) error { + masked := u.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 @@ -134,8 +122,8 @@ func (s *systemd) MaskUnit(unit *Unit) error { // 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) +func (s *systemd) UnmaskUnit(u Unit) error { + masked := u.Destination(s.root) ne, err := nullOrEmpty(masked) if os.IsNotExist(err) { return nil diff --git a/system/systemd_test.go b/system/systemd_test.go index 2d4f1d5..1a728f6 100644 --- a/system/systemd_test.go +++ b/system/systemd_test.go @@ -51,7 +51,7 @@ Address=10.209.171.177/19 t.Fatalf("unit.Destination returned %s, expected %s", dst, expectDst) } - if err := sd.PlaceUnit(&u, dst); err != nil { + if err := sd.PlaceUnit(u); err != nil { t.Fatalf("PlaceUnit failed: %v", err) } @@ -128,7 +128,7 @@ Where=/media/state t.Fatalf("unit.Destination returned %s, expected %s", dst, expectDst) } - if err := sd.PlaceUnit(&u, dst); err != nil { + if err := sd.PlaceUnit(u); err != nil { t.Fatalf("PlaceUnit failed: %v", err) } @@ -180,7 +180,7 @@ func TestMaskUnit(t *testing.T) { sd := &systemd{dir} // Ensure mask works with units that do not currently exist - uf := &Unit{config.Unit{Name: "foo.service"}} + uf := Unit{config.Unit{Name: "foo.service"}} if err := sd.MaskUnit(uf); err != nil { t.Fatalf("Unable to mask new unit: %v", err) } @@ -194,7 +194,7 @@ func TestMaskUnit(t *testing.T) { } // Ensure mask works with unit files that already exist - ub := &Unit{config.Unit{Name: "bar.service"}} + ub := Unit{config.Unit{Name: "bar.service"}} barPath := path.Join(dir, "etc", "systemd", "system", "bar.service") if _, err := os.Create(barPath); err != nil { t.Fatalf("Error creating new unit file: %v", err) @@ -220,12 +220,12 @@ func TestUnmaskUnit(t *testing.T) { sd := &systemd{dir} - nilUnit := &Unit{config.Unit{Name: "null.service"}} + nilUnit := Unit{config.Unit{Name: "null.service"}} if err := sd.UnmaskUnit(nilUnit); err != nil { t.Errorf("unexpected error from unmasking nonexistent unit: %v", err) } - uf := &Unit{config.Unit{Name: "foo.service", Content: "[Service]\nExecStart=/bin/true"}} + uf := Unit{config.Unit{Name: "foo.service", Content: "[Service]\nExecStart=/bin/true"}} dst := uf.Destination(dir) if err := os.MkdirAll(path.Dir(dst), os.FileMode(0755)); err != nil { t.Fatalf("Unable to create unit directory: %v", err) @@ -245,7 +245,7 @@ func TestUnmaskUnit(t *testing.T) { t.Errorf("unmask of non-empty unit mutated unit contents unexpectedly") } - ub := &Unit{config.Unit{Name: "bar.service"}} + ub := Unit{config.Unit{Name: "bar.service"}} dst = ub.Destination(dir) if err := os.Symlink("/dev/null", dst); err != nil { t.Fatalf("Unable to create masked unit: %v", err) diff --git a/system/unit.go b/system/unit.go index 26e9bbc..b48db7b 100644 --- a/system/unit.go +++ b/system/unit.go @@ -27,12 +27,12 @@ import ( 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) + PlaceUnit(unit Unit) error + EnableUnitFile(unit Unit) error + RunUnitCommand(unit Unit, command string) (string, error) + MaskUnit(unit Unit) error + UnmaskUnit(unit Unit) error DaemonReload() error - MaskUnit(unit *Unit) error - UnmaskUnit(unit *Unit) error } // Unit is a top-level structure which embeds its underlying configuration, @@ -41,10 +41,10 @@ type Unit struct { config.Unit } -// 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 { +// 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" From 75ed8dacf997220248e261306f1dfe07fede5650 Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Mon, 24 Nov 2014 17:05:36 -0800 Subject: [PATCH 2/5] initialize: clean up TestProcessUnits() --- initialize/config_test.go | 136 ++++++++++++++++++++------------------ 1 file changed, 71 insertions(+), 65 deletions(-) diff --git a/initialize/config_test.go b/initialize/config_test.go index 10a5ae3..cf8f80e 100644 --- a/initialize/config_test.go +++ b/initialize/config_test.go @@ -17,6 +17,7 @@ package initialize import ( + "reflect" "testing" "github.com/coreos/coreos-cloudinit/config" @@ -59,72 +60,77 @@ func (tum *TestUnitManager) UnmaskUnit(u system.Unit) error { } func TestProcessUnits(t *testing.T) { - tum := &TestUnitManager{} - units := []system.Unit{ - system.Unit{Unit: config.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) + tests := []struct { + units []system.Unit + + result TestUnitManager + }{ + { + units: []system.Unit{ + system.Unit{Unit: config.Unit{ + Name: "foo", + Mask: true, + }}, + }, + result: TestUnitManager{ + masked: []string{"foo"}, + }, + }, + { + units: []system.Unit{ + system.Unit{Unit: config.Unit{ + Name: "bar.network", + }}, + }, + result: TestUnitManager{ + commands: map[string]string{ + "systemd-networkd.service": "restart", + }, + }, + }, + { + units: []system.Unit{ + system.Unit{Unit: config.Unit{ + Name: "baz.service", + Content: "[Service]\nExecStart=/bin/true", + }}, + }, + result: TestUnitManager{ + placed: []string{"baz.service"}, + reload: true, + }, + }, + { + units: []system.Unit{ + system.Unit{Unit: config.Unit{ + Name: "locksmithd.service", + Runtime: true, + }}, + }, + result: TestUnitManager{ + unmasked: []string{"locksmithd.service"}, + }, + }, + { + units: []system.Unit{ + system.Unit{Unit: config.Unit{ + Name: "woof", + Enable: true, + }}, + }, + result: TestUnitManager{ + enabled: []string{"woof"}, + }, + }, } - tum = &TestUnitManager{} - units = []system.Unit{ - system.Unit{Unit: config.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{Unit: config.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{Unit: config.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{Unit: config.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) + for _, tt := range tests { + tum := &TestUnitManager{} + if err := processUnits(tt.units, "", tum); err != nil { + t.Errorf("bad error (%+v): want nil, got %s", tt.units, err) + } + if !reflect.DeepEqual(tt.result, *tum) { + t.Errorf("bad result (%+v): want %+v, got %+v", tt.units, tt.result, tum) + } } } From 624df676d06e5f1a54d671a2667ffb07f05f87c2 Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Tue, 25 Nov 2014 17:41:26 -0800 Subject: [PATCH 3/5] config/unit: move Type() and Group() out of config --- config/config_test.go | 3 -- config/unit.go | 19 -------- system/systemd_test.go | 32 ------------- system/unit.go | 23 +++++++++- system/unit_test.go | 100 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 122 insertions(+), 55 deletions(-) create mode 100644 system/unit_test.go diff --git a/config/config_test.go b/config/config_test.go index e4c322d..d858cfa 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -261,9 +261,6 @@ Address=10.209.171.177/19 if u.Name != "50-eth0.network" { t.Errorf("Unit has incorrect name %s", u.Name) } - if u.Type() != "network" { - t.Errorf("Unit has incorrect type '%s'", u.Type()) - } } if cfg.Coreos.OEM.ID != "rackspace" { diff --git a/config/unit.go b/config/unit.go index 1f56a1e..93ed3f1 100644 --- a/config/unit.go +++ b/config/unit.go @@ -16,11 +16,6 @@ package config -import ( - "path/filepath" - "strings" -) - type Unit struct { Name string `yaml:"name"` Mask bool `yaml:"mask"` @@ -34,17 +29,3 @@ type Unit struct { // 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() string { - switch u.Type() { - case "network", "netdev", "link": - return "network" - default: - return "system" - } -} diff --git a/system/systemd_test.go b/system/systemd_test.go index 1a728f6..a728efd 100644 --- a/system/systemd_test.go +++ b/system/systemd_test.go @@ -46,10 +46,6 @@ Address=10.209.171.177/19 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 := sd.PlaceUnit(u); err != nil { t.Fatalf("PlaceUnit failed: %v", err) @@ -80,30 +76,6 @@ Address=10.209.171.177/19 } } -func TestUnitDestination(t *testing.T) { - dir := "/some/dir" - name := "foobar.service" - - u := Unit{config.Unit{ - Name: name, - DropIn: false, - }} - - dst := u.Destination(dir) - expectDst := path.Join(dir, "etc", "systemd", "system", "foobar.service") - if dst != expectDst { - t.Errorf("unit.Destination returned %s, expected %s", dst, expectDst) - } - - u.DropIn = true - - dst = u.Destination(dir) - expectDst = path.Join(dir, "etc", "systemd", "system", "foobar.service.d", cloudConfigDropIn) - if dst != expectDst { - t.Errorf("unit.Destination returned %s, expected %s", dst, expectDst) - } -} - func TestPlaceMountUnit(t *testing.T) { u := Unit{config.Unit{ Name: "media-state.mount", @@ -123,10 +95,6 @@ Where=/media/state 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 := sd.PlaceUnit(u); err != nil { t.Fatalf("PlaceUnit failed: %v", err) diff --git a/system/unit.go b/system/unit.go index b48db7b..897a853 100644 --- a/system/unit.go +++ b/system/unit.go @@ -19,6 +19,8 @@ package system import ( "fmt" "path" + "path/filepath" + "strings" "github.com/coreos/coreos-cloudinit/config" ) @@ -36,11 +38,30 @@ type UnitManager interface { } // Unit is a top-level structure which embeds its underlying configuration, -// config.Unit, and provides the system-specific Destination(). +// config.Unit, and provides the system-specific Destination(), Type(), and +// Group(). type Unit struct { config.Unit } +// Type returns the extension of the unit (everything that follows the final +// period). +func (u Unit) Type() string { + ext := filepath.Ext(u.Name) + return strings.TrimLeft(ext, ".") +} + +// Group returns "network" or "system" depending on whether or not the unit is +// a network unit or otherwise. +func (u Unit) Group() string { + switch u.Type() { + case "network", "netdev", "link": + return "network" + default: + return "system" + } +} + // 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). diff --git a/system/unit_test.go b/system/unit_test.go new file mode 100644 index 0000000..0303694 --- /dev/null +++ b/system/unit_test.go @@ -0,0 +1,100 @@ +/* + Copyright 2014 CoreOS, Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package system + +import ( + "testing" + + "github.com/coreos/coreos-cloudinit/config" +) + +func TestType(t *testing.T) { + tests := []struct { + name string + + typ string + }{ + {}, + {"test.service", "service"}, + {"hello", ""}, + {"lots.of.dots", "dots"}, + } + + for _, tt := range tests { + u := Unit{config.Unit{ + Name: tt.name, + }} + if typ := u.Type(); tt.typ != typ { + t.Errorf("bad type (%+v): want %q, got %q", tt, tt.typ, typ) + } + } +} + +func TestGroup(t *testing.T) { + tests := []struct { + name string + + group string + }{ + {"test.service", "system"}, + {"test.link", "network"}, + {"test.network", "network"}, + {"test.netdev", "network"}, + {"test.conf", "system"}, + } + + for _, tt := range tests { + u := Unit{config.Unit{ + Name: tt.name, + }} + if group := u.Group(); tt.group != group { + t.Errorf("bad group (%+v): want %q, got %q", tt, tt.group, group) + } + } +} + +func TestDestination(t *testing.T) { + tests := []struct { + root string + name string + runtime bool + + destination string + }{ + { + root: "/some/dir", + name: "foobar.service", + destination: "/some/dir/etc/systemd/system/foobar.service", + }, + { + root: "/some/dir", + name: "foobar.service", + runtime: true, + destination: "/some/dir/run/systemd/system/foobar.service", + }, + } + + for _, tt := range tests { + u := Unit{config.Unit{ + Name: tt.name, + Runtime: tt.runtime, + }} + if d := u.Destination(tt.root); tt.destination != d { + t.Errorf("bad destination (%+v): want %q, got %q", tt, tt.destination, d) + } + } +} From 420f7cf2025d3d881711848acf7839ccd81267cf Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Tue, 25 Nov 2014 17:42:37 -0800 Subject: [PATCH 4/5] system: clean up TestPlaceUnit() --- system/systemd_test.go | 123 +++++++++++++---------------------------- 1 file changed, 37 insertions(+), 86 deletions(-) diff --git a/system/systemd_test.go b/system/systemd_test.go index a728efd..ba0be4d 100644 --- a/system/systemd_test.go +++ b/system/systemd_test.go @@ -17,6 +17,7 @@ package system import ( + "fmt" "io/ioutil" "os" "path" @@ -25,101 +26,51 @@ import ( "github.com/coreos/coreos-cloudinit/config" ) -func TestPlaceNetworkUnit(t *testing.T) { - u := Unit{config.Unit{ - Name: "50-eth0.network", - Runtime: true, - Content: `[Match] -Name=eth47 - -[Network] -Address=10.209.171.177/19 -`, - }} - - dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-") - if err != nil { - t.Fatalf("Unable to create tempdir: %v", err) - } - defer os.RemoveAll(dir) - - sd := &systemd{dir} - - dst := u.Destination(dir) - - if err := sd.PlaceUnit(u); err != nil { - t.Fatalf("PlaceUnit failed: %v", err) +func TestPlaceUnit(t *testing.T) { + tests := []config.Unit{ + { + Name: "50-eth0.network", + Runtime: true, + Content: "[Match]\nName=eth47\n\n[Network]\nAddress=10.209.171.177/19\n", + }, + { + Name: "media-state.mount", + Content: "[Mount]\nWhat=/dev/sdb1\nWhere=/media/state\n", + }, } - fi, err := os.Stat(dst) - if err != nil { - t.Fatalf("Unable to stat file: %v", err) - } + for _, tt := range tests { + dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-") + if err != nil { + panic(fmt.Sprintf("Unable to create tempdir: %v", err)) + } - if fi.Mode() != os.FileMode(0644) { - t.Errorf("File has incorrect mode: %v", fi.Mode()) - } + u := Unit{tt} + sd := &systemd{dir} - contents, err := ioutil.ReadFile(dst) - if err != nil { - t.Fatalf("Unable to read expected file: %v", err) - } + if err := sd.PlaceUnit(u); err != nil { + t.Fatalf("PlaceUnit(): bad error (%+v): want nil, got %s", tt, err) + } - expectContents := `[Match] -Name=eth47 + fi, err := os.Stat(u.Destination(dir)) + if err != nil { + t.Fatalf("Stat(): bad error (%+v): want nil, got %s", tt, err) + } -[Network] -Address=10.209.171.177/19 -` - if string(contents) != expectContents { - t.Fatalf("File has incorrect contents '%s'.\nExpected '%s'", string(contents), expectContents) - } -} + if mode := fi.Mode(); mode != os.FileMode(0644) { + t.Errorf("bad filemode (%+v): want %v, got %v", tt, os.FileMode(0644), mode) + } -func TestPlaceMountUnit(t *testing.T) { - u := Unit{config.Unit{ - Name: "media-state.mount", - Runtime: false, - Content: `[Mount] -What=/dev/sdb1 -Where=/media/state -`, - }} + c, err := ioutil.ReadFile(u.Destination(dir)) + if err != nil { + t.Fatalf("ReadFile(): bad error (%+v): want nil, got %s", tt, err) + } - dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-") - if err != nil { - t.Fatalf("Unable to create tempdir: %v", err) - } - defer os.RemoveAll(dir) + if string(c) != tt.Content { + t.Errorf("bad contents (%+v): want %q, got %q", tt, tt.Content, string(c)) + } - sd := &systemd{dir} - - dst := u.Destination(dir) - - if err := sd.PlaceUnit(u); err != nil { - t.Fatalf("PlaceUnit failed: %v", err) - } - - fi, err := os.Stat(dst) - 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(dst) - if err != nil { - t.Fatalf("Unable to read expected file: %v", err) - } - - expectContents := `[Mount] -What=/dev/sdb1 -Where=/media/state -` - if string(contents) != expectContents { - t.Fatalf("File has incorrect contents '%s'.\nExpected '%s'", string(contents), expectContents) + os.RemoveAll(dir) } } From ffc54b028cb9ebe60a2353ea322b3f7b5caad6f9 Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Tue, 25 Nov 2014 16:57:15 -0800 Subject: [PATCH 5/5] drop-in: add support for drop-ins This allows a list of drop-ins for a unit to be declared inline within a cloud-config. For example: #cloud-config coreos: units: - name: docker.service drop-ins: - name: 50-insecure-registry.conf content: | [Service] Environment=DOCKER_OPTS='--insecure-registry="10.0.1.0/24"' --- Documentation/cloud-config.md | 80 +++++++++++++++++++++-------------- config/config_test.go | 20 --------- config/unit.go | 23 +++++----- initialize/config.go | 16 +++++++ initialize/config_test.go | 67 +++++++++++++++++++++++++++++ system/env.go | 15 +------ system/env_test.go | 4 +- system/etcd.go | 9 +++- system/etcd_test.go | 18 +++++--- system/flannel.go | 9 +++- system/flannel_test.go | 16 ++++--- system/fleet.go | 9 +++- system/fleet_test.go | 16 ++++--- system/systemd.go | 13 ++++++ system/systemd_test.go | 58 +++++++++++++++++++++++++ system/unit.go | 23 ++++++---- system/unit_test.go | 38 +++++++++++++++++ 17 files changed, 329 insertions(+), 105 deletions(-) diff --git a/Documentation/cloud-config.md b/Documentation/cloud-config.md index e36778c..aab6746 100644 --- a/Documentation/cloud-config.md +++ b/Documentation/cloud-config.md @@ -16,7 +16,7 @@ We've designed our implementation to allow the same cloud-config file to work ac The cloud-config file uses the [YAML][yaml] file format, which uses whitespace and new-lines to delimit lists, associative arrays, and values. -A cloud-config file should contain `#cloud-config`, followed by an associative array which has zero or more of the following keys: +A cloud-config file must contain `#cloud-config`, followed by an associative array which has zero or more of the following keys: - `coreos` - `ssh_authorized_keys` @@ -46,13 +46,13 @@ If the platform environment supports the templating feature of coreos-cloudinit #cloud-config coreos: - etcd: - name: node001 - # generate a new token for each unique cluster from https://discovery.etcd.io/new - discovery: https://discovery.etcd.io/ - # multi-region and multi-cloud deployments need to use $public_ipv4 - addr: $public_ipv4:4001 - peer-addr: $private_ipv4:7001 + etcd: + name: node001 + # generate a new token for each unique cluster from https://discovery.etcd.io/new + discovery: https://discovery.etcd.io/ + # multi-region and multi-cloud deployments need to use $public_ipv4 + addr: $public_ipv4:4001 + peer-addr: $private_ipv4:7001 ``` ...will generate a systemd unit drop-in like this: @@ -66,7 +66,6 @@ Environment="ETCD_PEER_ADDR=192.0.2.13:7001" ``` For more information about the available configuration parameters, see the [etcd documentation][etcd-config]. -Note that hyphens in the coreos.etcd.* keys are mapped to underscores. _Note: The `$private_ipv4` and `$public_ipv4` substitution variables referenced in other documents are only supported on Amazon EC2, Google Compute Engine, OpenStack, Rackspace, DigitalOcean, and Vagrant._ @@ -80,9 +79,9 @@ The `coreos.fleet.*` parameters work very similarly to `coreos.etcd.*`, and allo #cloud-config coreos: - fleet: - public-ip: $public_ipv4 - metadata: region=us-west + fleet: + public-ip: $public_ipv4 + metadata: region=us-west ``` ...will generate a systemd unit drop-in like this: @@ -105,8 +104,8 @@ The `coreos.flannel.*` parameters also work very similarly to `coreos.etcd.*` an #cloud-config coreos: - flannel: - etcd-prefix: /coreos.com/network2 + flannel: + etcd-prefix: /coreos.com/network2 ``` ...will generate systemd unit drop-in like so: @@ -158,6 +157,10 @@ Each item is an object with the following fields: - **content**: Plaintext string representing entire unit file. If no value is provided, the unit is assumed to exist already. - **command**: Command to execute on unit: start, stop, reload, restart, try-restart, reload-or-restart, reload-or-try-restart. The default behavior is to not execute any commands. - **mask**: Whether to mask the unit file by symlinking it to `/dev/null` (analogous to `systemctl mask `). Note that unlike `systemctl mask`, **this will destructively remove any existing unit file** located at `/etc/systemd/system/`, to ensure that the mask succeeds. The default value is false. +- **drop-ins**: A list of unit drop-ins with the following fields: + - **name**: String representing unit's name. Required. + - **content**: Plaintext string representing entire file. Required. + **NOTE:** The command field is ignored for all network, netdev, and link units. The systemd-networkd.service unit will be restarted in their place. @@ -169,19 +172,34 @@ Write a unit to disk, automatically starting it. #cloud-config coreos: - units: - - name: docker-redis.service - command: start - content: | - [Unit] - Description=Redis container - Author=Me - After=docker.service + units: + - name: docker-redis.service + command: start + content: | + [Unit] + Description=Redis container + Author=Me + After=docker.service - [Service] - Restart=always - ExecStart=/usr/bin/docker start -a redis_server - ExecStop=/usr/bin/docker stop -t 2 redis_server + [Service] + Restart=always + ExecStart=/usr/bin/docker start -a redis_server + ExecStop=/usr/bin/docker stop -t 2 redis_server +``` + +Add the DOCKER_OPTS environment variable to docker.service. + +```yaml +#cloud-config + +coreos: + units: + - name: docker.service + drop-ins: + - name: 50-insecure-registry.conf + content: | + [Service] + Environment=DOCKER_OPTS='--insecure-registry="10.0.1.0/24"' ``` Start the built-in `etcd` and `fleet` services: @@ -190,11 +208,11 @@ Start the built-in `etcd` and `fleet` services: #cloud-config coreos: - units: - - name: etcd.service - command: start - - name: fleet.service - command: start + units: + - name: etcd.service + command: start + - name: fleet.service + command: start ``` ### ssh_authorized_keys diff --git a/config/config_test.go b/config/config_test.go index d858cfa..29171ad 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -335,26 +335,6 @@ func TestCloudConfigSerializationHeader(t *testing.T) { } } -// TestDropInIgnored asserts that users are unable to set DropIn=True on units -func TestDropInIgnored(t *testing.T) { - contents := ` -coreos: - units: - - name: test - dropin: true -` - cfg, err := NewCloudConfig(contents) - if err != nil || len(cfg.Coreos.Units) != 1 { - t.Fatalf("Encountered unexpected error: %v", err) - } - if len(cfg.Coreos.Units) != 1 || cfg.Coreos.Units[0].Name != "test" { - t.Fatalf("Expected 1 unit, but got %d: %v", len(cfg.Coreos.Units), cfg.Coreos.Units) - } - if cfg.Coreos.Units[0].DropIn { - t.Errorf("dropin option on unit in cloud-config was not ignored!") - } -} - func TestCloudConfigUsers(t *testing.T) { contents := ` users: diff --git a/config/unit.go b/config/unit.go index 93ed3f1..3a09bbb 100644 --- a/config/unit.go +++ b/config/unit.go @@ -17,15 +17,16 @@ package config type Unit struct { - Name string `yaml:"name"` - Mask bool `yaml:"mask"` - Enable bool `yaml:"enable"` - Runtime bool `yaml:"runtime"` - Content string `yaml:"content"` - Command string `yaml:"command" valid:"start,stop,restart,reload,try-restart,reload-or-restart,reload-or-try-restart"` - - // 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:"-"` + Name string `yaml:"name"` + Mask bool `yaml:"mask"` + Enable bool `yaml:"enable"` + Runtime bool `yaml:"runtime"` + Content string `yaml:"content"` + Command string `yaml:"command" valid:"start,stop,restart,reload,try-restart,reload-or-restart,reload-or-try-restart"` + DropIns []UnitDropIn `yaml:"drop_ins"` +} + +type UnitDropIn struct { + Name string `yaml:"name"` + Content string `yaml:"content"` } diff --git a/initialize/config.go b/initialize/config.go index 9e169ce..660f00b 100644 --- a/initialize/config.go +++ b/initialize/config.go @@ -201,6 +201,11 @@ func processUnits(units []system.Unit, root string, um system.UnitManager) error actions := make([]action, 0, len(units)) reload := false for _, unit := range units { + if unit.Name == "" { + log.Printf("Skipping unit without name") + continue + } + if unit.Content != "" { log.Printf("Writing unit %q to filesystem", unit.Name) if err := um.PlaceUnit(unit); err != nil { @@ -210,6 +215,17 @@ func processUnits(units []system.Unit, root string, um system.UnitManager) error reload = true } + for _, dropin := range unit.DropIns { + if dropin.Name != "" && dropin.Content != "" { + log.Printf("Writing drop-in unit %q to filesystem", dropin.Name) + if err := um.PlaceUnitDropIn(unit, dropin); err != nil { + return err + } + log.Printf("Wrote drop-in unit %q", dropin.Name) + reload = true + } + } + if unit.Mask { log.Printf("Masking unit file %q", unit.Name) if err := um.MaskUnit(unit); err != nil { diff --git a/initialize/config_test.go b/initialize/config_test.go index cf8f80e..ed95d58 100644 --- a/initialize/config_test.go +++ b/initialize/config_test.go @@ -37,6 +37,10 @@ func (tum *TestUnitManager) PlaceUnit(u system.Unit) error { tum.placed = append(tum.placed, u.Name) return nil } +func (tum *TestUnitManager) PlaceUnitDropIn(u system.Unit, d config.UnitDropIn) error { + tum.placed = append(tum.placed, u.Name+".d/"+d.Name) + return nil +} func (tum *TestUnitManager) EnableUnitFile(u system.Unit) error { tum.enabled = append(tum.enabled, u.Name) return nil @@ -122,6 +126,69 @@ func TestProcessUnits(t *testing.T) { enabled: []string{"woof"}, }, }, + { + units: []system.Unit{ + system.Unit{Unit: config.Unit{ + Name: "hi.service", + Runtime: true, + Content: "[Service]\nExecStart=/bin/echo hi", + DropIns: []config.UnitDropIn{ + { + Name: "lo.conf", + Content: "[Service]\nExecStart=/bin/echo lo", + }, + { + Name: "bye.conf", + Content: "[Service]\nExecStart=/bin/echo bye", + }, + }, + }}, + }, + result: TestUnitManager{ + placed: []string{"hi.service", "hi.service.d/lo.conf", "hi.service.d/bye.conf"}, + unmasked: []string{"hi.service"}, + reload: true, + }, + }, + { + units: []system.Unit{ + system.Unit{Unit: config.Unit{ + DropIns: []config.UnitDropIn{ + { + Name: "lo.conf", + Content: "[Service]\nExecStart=/bin/echo lo", + }, + }, + }}, + }, + result: TestUnitManager{}, + }, + { + units: []system.Unit{ + system.Unit{Unit: config.Unit{ + Name: "hi.service", + DropIns: []config.UnitDropIn{ + { + Content: "[Service]\nExecStart=/bin/echo lo", + }, + }, + }}, + }, + result: TestUnitManager{}, + }, + { + units: []system.Unit{ + system.Unit{Unit: config.Unit{ + Name: "hi.service", + DropIns: []config.UnitDropIn{ + { + Name: "lo.conf", + }, + }, + }}, + }, + result: TestUnitManager{}, + }, } for _, tt := range tests { diff --git a/system/env.go b/system/env.go index 152b977..993d583 100644 --- a/system/env.go +++ b/system/env.go @@ -25,7 +25,7 @@ import ( // 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 { +func serviceContents(e interface{}) string { et := reflect.TypeOf(e) ev := reflect.ValueOf(e) @@ -42,16 +42,3 @@ func dropinContents(e interface{}) string { } return "[Service]\n" + out } - -func dropinFromConfig(cfg interface{}, name string) []Unit { - content := dropinContents(cfg) - if content == "" { - return nil - } - return []Unit{{config.Unit{ - Name: name, - Runtime: true, - DropIn: true, - Content: content, - }}} -} diff --git a/system/env_test.go b/system/env_test.go index 0623186..7a02ca2 100644 --- a/system/env_test.go +++ b/system/env_test.go @@ -4,7 +4,7 @@ import ( "testing" ) -func TestDropinContents(t *testing.T) { +func TestServiceContents(t *testing.T) { tests := []struct { Config interface{} Contents string @@ -48,7 +48,7 @@ Environment="D=0.1" } for _, tt := range tests { - if c := dropinContents(tt.Config); c != tt.Contents { + if c := serviceContents(tt.Config); c != tt.Contents { t.Errorf("bad contents (%+v): want %q, got %q", tt, tt.Contents, c) } } diff --git a/system/etcd.go b/system/etcd.go index a80b091..0e898b1 100644 --- a/system/etcd.go +++ b/system/etcd.go @@ -28,5 +28,12 @@ type Etcd struct { // Units creates a Unit file drop-in for etcd, using any configured options. func (ee Etcd) Units() []Unit { - return dropinFromConfig(ee.Etcd, "etcd.service") + return []Unit{{config.Unit{ + Name: "etcd.service", + Runtime: true, + DropIns: []config.UnitDropIn{{ + Name: "20-cloudinit.conf", + Content: serviceContents(ee.Etcd), + }}, + }}} } diff --git a/system/etcd_test.go b/system/etcd_test.go index 879eb8f..a161abc 100644 --- a/system/etcd_test.go +++ b/system/etcd_test.go @@ -30,7 +30,11 @@ func TestEtcdUnits(t *testing.T) { }{ { config.Etcd{}, - nil, + []Unit{{config.Unit{ + Name: "etcd.service", + Runtime: true, + DropIns: []config.UnitDropIn{{Name: "20-cloudinit.conf"}}, + }}}, }, { config.Etcd{ @@ -40,11 +44,13 @@ func TestEtcdUnits(t *testing.T) { []Unit{{config.Unit{ Name: "etcd.service", Runtime: true, - DropIn: true, - Content: `[Service] + DropIns: []config.UnitDropIn{{ + Name: "20-cloudinit.conf", + Content: `[Service] Environment="ETCD_DISCOVERY=http://disco.example.com/foobar" Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002" `, + }}, }}}, }, { @@ -56,12 +62,14 @@ Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002" []Unit{{config.Unit{ Name: "etcd.service", Runtime: true, - DropIn: true, - Content: `[Service] + DropIns: []config.UnitDropIn{{ + Name: "20-cloudinit.conf", + Content: `[Service] Environment="ETCD_DISCOVERY=http://disco.example.com/foobar" Environment="ETCD_NAME=node001" Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002" `, + }}, }}}, }, } { diff --git a/system/flannel.go b/system/flannel.go index d6559a5..3e97f18 100644 --- a/system/flannel.go +++ b/system/flannel.go @@ -13,5 +13,12 @@ type Flannel struct { // Units generates a Unit file drop-in for flannel, if any flannel options were // configured in cloud-config func (fl Flannel) Units() []Unit { - return dropinFromConfig(fl.Flannel, "flanneld.service") + return []Unit{{config.Unit{ + Name: "flanneld.service", + Runtime: true, + DropIns: []config.UnitDropIn{{ + Name: "20-cloudinit.conf", + Content: serviceContents(fl.Flannel), + }}, + }}} } diff --git a/system/flannel_test.go b/system/flannel_test.go index 943382e..2e20c46 100644 --- a/system/flannel_test.go +++ b/system/flannel_test.go @@ -14,7 +14,11 @@ func TestFlannelUnits(t *testing.T) { }{ { config.Flannel{}, - nil, + []Unit{{config.Unit{ + Name: "flanneld.service", + Runtime: true, + DropIns: []config.UnitDropIn{{Name: "20-cloudinit.conf"}}, + }}}, }, { config.Flannel{ @@ -22,13 +26,15 @@ func TestFlannelUnits(t *testing.T) { EtcdPrefix: "/coreos.com/network/tenant1", }, []Unit{{config.Unit{ - Name: "flanneld.service", - Content: `[Service] + Name: "flanneld.service", + Runtime: true, + DropIns: []config.UnitDropIn{{ + Name: "20-cloudinit.conf", + Content: `[Service] Environment="FLANNELD_ETCD_ENDPOINT=http://12.34.56.78:4001" Environment="FLANNELD_ETCD_PREFIX=/coreos.com/network/tenant1" `, - Runtime: true, - DropIn: true, + }}, }}}, }, } { diff --git a/system/fleet.go b/system/fleet.go index 45def39..84cb378 100644 --- a/system/fleet.go +++ b/system/fleet.go @@ -29,5 +29,12 @@ 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() []Unit { - return dropinFromConfig(fe.Fleet, "fleet.service") + return []Unit{{config.Unit{ + Name: "fleet.service", + Runtime: true, + DropIns: []config.UnitDropIn{{ + Name: "20-cloudinit.conf", + Content: serviceContents(fe.Fleet), + }}, + }}} } diff --git a/system/fleet_test.go b/system/fleet_test.go index bf3c77e..f3e3284 100644 --- a/system/fleet_test.go +++ b/system/fleet_test.go @@ -30,19 +30,25 @@ func TestFleetUnits(t *testing.T) { }{ { config.Fleet{}, - nil, + []Unit{{config.Unit{ + Name: "fleet.service", + Runtime: true, + DropIns: []config.UnitDropIn{{Name: "20-cloudinit.conf"}}, + }}}, }, { config.Fleet{ PublicIP: "12.34.56.78", }, []Unit{{config.Unit{ - Name: "fleet.service", - Content: `[Service] + Name: "fleet.service", + Runtime: true, + DropIns: []config.UnitDropIn{{ + Name: "20-cloudinit.conf", + Content: `[Service] Environment="FLEET_PUBLIC_IP=12.34.56.78" `, - Runtime: true, - DropIn: true, + }}, }}}, }, } { diff --git a/system/systemd.go b/system/systemd.go index 2c346a5..f9651b1 100644 --- a/system/systemd.go +++ b/system/systemd.go @@ -54,6 +54,19 @@ func (s *systemd) PlaceUnit(u Unit) error { return err } +// PlaceUnitDropIn writes a unit drop-in file at its desired destination, +// creating parent directories as necessary. +func (s *systemd) PlaceUnitDropIn(u Unit, d config.UnitDropIn) error { + file := File{config.File{ + Path: u.DropInDestination(s.root, d), + Content: d.Content, + RawFilePermissions: "0644", + }} + + _, err := WriteFile(&file, "/") + return err +} + func (s *systemd) EnableUnitFile(u Unit) error { conn, err := dbus.New() if err != nil { diff --git a/system/systemd_test.go b/system/systemd_test.go index ba0be4d..2794c88 100644 --- a/system/systemd_test.go +++ b/system/systemd_test.go @@ -74,6 +74,64 @@ func TestPlaceUnit(t *testing.T) { } } +func TestPlaceUnitDropIn(t *testing.T) { + tests := []config.Unit{ + { + Name: "false.service", + Runtime: true, + DropIns: []config.UnitDropIn{ + { + Name: "00-true.conf", + Content: "[Service]\nExecStart=\nExecStart=/usr/bin/true\n", + }, + }, + }, + { + Name: "true.service", + DropIns: []config.UnitDropIn{ + { + Name: "00-false.conf", + Content: "[Service]\nExecStart=\nExecStart=/usr/bin/false\n", + }, + }, + }, + } + + for _, tt := range tests { + dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-") + if err != nil { + panic(fmt.Sprintf("Unable to create tempdir: %v", err)) + } + + u := Unit{tt} + sd := &systemd{dir} + + if err := sd.PlaceUnitDropIn(u, u.DropIns[0]); err != nil { + t.Fatalf("PlaceUnit(): bad error (%+v): want nil, got %s", tt, err) + } + + fi, err := os.Stat(u.DropInDestination(dir, u.DropIns[0])) + if err != nil { + t.Fatalf("Stat(): bad error (%+v): want nil, got %s", tt, err) + } + + if mode := fi.Mode(); mode != os.FileMode(0644) { + t.Errorf("bad filemode (%+v): want %v, got %v", tt, os.FileMode(0644), mode) + } + + c, err := ioutil.ReadFile(u.DropInDestination(dir, u.DropIns[0])) + if err != nil { + t.Fatalf("ReadFile(): bad error (%+v): want nil, got %s", tt, err) + } + + if string(c) != u.DropIns[0].Content { + t.Errorf("bad contents (%+v): want %q, got %q", tt, u.DropIns[0].Content, string(c)) + } + + os.RemoveAll(dir) + } +} + func TestMachineID(t *testing.T) { dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-") if err != nil { diff --git a/system/unit.go b/system/unit.go index 897a853..e7152ec 100644 --- a/system/unit.go +++ b/system/unit.go @@ -25,11 +25,9 @@ import ( "github.com/coreos/coreos-cloudinit/config" ) -// Name for drop-in service configuration files created by cloudconfig -const cloudConfigDropIn = "20-cloudinit.conf" - type UnitManager interface { PlaceUnit(unit Unit) error + PlaceUnitDropIn(unit Unit, dropIn config.UnitDropIn) error EnableUnitFile(unit Unit) error RunUnitCommand(unit Unit, command string) (string, error) MaskUnit(unit Unit) error @@ -66,14 +64,21 @@ func (u Unit) Group() string { // argument indicates the effective base directory of the system (similar to a // chroot). func (u Unit) Destination(root string) string { + return path.Join(u.prefix(root), u.Name) +} + +// DropInDestination builds the appropriate absolute file path for the +// UnitDropIn. The root argument indicates the effective base directory of the +// system (similar to a chroot) and the dropIn argument is the UnitDropIn for +// which the destination is being calculated. +func (u Unit) DropInDestination(root string, dropIn config.UnitDropIn) string { + return path.Join(u.prefix(root), fmt.Sprintf("%s.d", u.Name), dropIn.Name) +} + +func (u Unit) prefix(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) - } + return path.Join(root, dir, "systemd", u.Group()) } diff --git a/system/unit_test.go b/system/unit_test.go index 0303694..10cb883 100644 --- a/system/unit_test.go +++ b/system/unit_test.go @@ -98,3 +98,41 @@ func TestDestination(t *testing.T) { } } } + +func TestDropInDestination(t *testing.T) { + tests := []struct { + root string + unitName string + dropInName string + runtime bool + + destination string + }{ + { + root: "/some/dir", + unitName: "foo.service", + dropInName: "bar.conf", + destination: "/some/dir/etc/systemd/system/foo.service.d/bar.conf", + }, + { + root: "/some/dir", + unitName: "foo.service", + dropInName: "bar.conf", + runtime: true, + destination: "/some/dir/run/systemd/system/foo.service.d/bar.conf", + }, + } + + for _, tt := range tests { + u := Unit{config.Unit{ + Name: tt.unitName, + Runtime: tt.runtime, + DropIns: []config.UnitDropIn{{ + Name: tt.dropInName, + }}, + }} + if d := u.DropInDestination(tt.root, u.DropIns[0]); tt.destination != d { + t.Errorf("bad destination (%+v): want %q, got %q", tt, tt.destination, d) + } + } +}