From 477ae29135c79e77a0a03b720c4c4ec8ded57793 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Tue, 25 Mar 2014 18:50:05 -0700 Subject: [PATCH 1/3] fix(systemd): Fail if daemon-reload returns error --- initialize/config.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/initialize/config.go b/initialize/config.go index b81170b..4239e6d 100644 --- a/initialize/config.go +++ b/initialize/config.go @@ -153,7 +153,9 @@ func Apply(cfg CloudConfig, env *Environment) error { } } - system.DaemonReload() + if err := system.DaemonReload(); err != nil { + log.Fatalf("Failed systemd daemon-reload: %v", err) + } for unit, command := range commands { log.Printf("Calling unit command '%s %s'", command, unit) From f5f9a0a6a9028446a6a4feacf13ef2888dbbe0e3 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Tue, 25 Mar 2014 19:35:20 -0700 Subject: [PATCH 2/3] bump(github.com/coreos/go-systemd/dbus): 4fbc5060a317b142e6c7bfbedb65596d5f0ab99b --- .../github.com/coreos/go-systemd/dbus/dbus.go | 9 +- .../coreos/go-systemd/dbus/methods.go | 104 ++++++++++++++++- .../coreos/go-systemd/dbus/methods_test.go | 105 +++++++++++++++++- .../coreos/go-systemd/dbus/properties.go | 9 ++ 4 files changed, 219 insertions(+), 8 deletions(-) diff --git a/third_party/github.com/coreos/go-systemd/dbus/dbus.go b/third_party/github.com/coreos/go-systemd/dbus/dbus.go index 384e671..a6f8793 100644 --- a/third_party/github.com/coreos/go-systemd/dbus/dbus.go +++ b/third_party/github.com/coreos/go-systemd/dbus/dbus.go @@ -18,6 +18,8 @@ limitations under the License. package dbus import ( + "os" + "strconv" "strings" "sync" @@ -73,7 +75,12 @@ func (c *Conn) initConnection() error { return err } - err = c.sysconn.Auth(nil) + // Only use EXTERNAL method, and hardcode the uid (not username) + // to avoid a username lookup (which requires a dynamically linked + // libc) + methods := []dbus.Auth{dbus.AuthExternal(strconv.Itoa(os.Getuid()))} + + err = c.sysconn.Auth(methods) if err != nil { c.sysconn.Close() return err diff --git a/third_party/github.com/coreos/go-systemd/dbus/methods.go b/third_party/github.com/coreos/go-systemd/dbus/methods.go index 1dd19ad..2020106 100644 --- a/third_party/github.com/coreos/go-systemd/dbus/methods.go +++ b/third_party/github.com/coreos/go-systemd/dbus/methods.go @@ -35,6 +35,7 @@ func (c *Conn) jobComplete(signal *dbus.Signal) { out, ok := c.jobListener.jobs[job] if ok { out <- result + delete(c.jobListener.jobs, job) } c.jobListener.Unlock() } @@ -137,8 +138,8 @@ func (c *Conn) KillUnit(name string, signal int32) { c.sysobj.Call("org.freedesktop.systemd1.Manager.KillUnit", 0, name, "all", signal).Store() } -// GetUnitProperties takes the unit name and returns all of its dbus object properties. -func (c *Conn) GetUnitProperties(unit string) (map[string]interface{}, error) { +// getProperties takes the unit name and returns all of its dbus object properties, for the given dbus interface +func (c *Conn) getProperties(unit string, dbusInterface string) (map[string]interface{}, error) { var err error var props map[string]dbus.Variant @@ -148,7 +149,7 @@ func (c *Conn) GetUnitProperties(unit string) (map[string]interface{}, error) { } obj := c.sysconn.Object("org.freedesktop.systemd1", path) - err = obj.Call("org.freedesktop.DBus.Properties.GetAll", 0, "org.freedesktop.systemd1.Unit").Store(&props) + err = obj.Call("org.freedesktop.DBus.Properties.GetAll", 0, dbusInterface).Store(&props) if err != nil { return nil, err } @@ -161,6 +162,55 @@ func (c *Conn) GetUnitProperties(unit string) (map[string]interface{}, error) { return out, nil } +// GetUnitProperties takes the unit name and returns all of its dbus object properties. +func (c *Conn) GetUnitProperties(unit string) (map[string]interface{}, error) { + return c.getProperties(unit, "org.freedesktop.systemd1.Unit") +} + +func (c *Conn) getProperty(unit string, dbusInterface string, propertyName string) (*Property, error) { + var err error + var prop dbus.Variant + + path := ObjectPath("/org/freedesktop/systemd1/unit/" + unit) + if !path.IsValid() { + return nil, errors.New("invalid unit name: " + unit) + } + + obj := c.sysconn.Object("org.freedesktop.systemd1", path) + err = obj.Call("org.freedesktop.DBus.Properties.Get", 0, dbusInterface, propertyName).Store(&prop) + if err != nil { + return nil, err + } + + return &Property{Name: propertyName, Value: prop}, nil +} + +func (c *Conn) GetUnitProperty(unit string, propertyName string) (*Property, error) { + return c.getProperty(unit, "org.freedesktop.systemd1.Unit", propertyName) +} + +// GetUnitTypeProperties returns the extra properties for a unit, specific to the unit type. +// Valid values for unitType: Service, Socket, Target, Device, Mount, Automount, Snapshot, Timer, Swap, Path, Slice, Scope +// return "dbus.Error: Unknown interface" if the unitType is not the correct type of the unit +func (c *Conn) GetUnitTypeProperties(unit string, unitType string) (map[string]interface{}, error) { + return c.getProperties(unit, "org.freedesktop.systemd1."+unitType) +} + +// SetUnitProperties() may be used to modify certain unit properties at runtime. +// Not all properties may be changed at runtime, but many resource management +// settings (primarily those in systemd.cgroup(5)) may. The changes are applied +// instantly, and stored on disk for future boots, unless runtime is true, in which +// case the settings only apply until the next reboot. name is the name of the unit +// to modify. properties are the settings to set, encoded as an array of property +// name and value pairs. +func (c *Conn) SetUnitProperties(name string, runtime bool, properties ...Property) error { + return c.sysobj.Call("SetUnitProperties", 0, name, runtime, properties).Store() +} + +func (c *Conn) GetUnitTypeProperty(unit string, unitType string, propertyName string) (*Property, error) { + return c.getProperty(unit, "org.freedesktop.systemd1." + unitType, propertyName) +} + // ListUnits returns an array with all currently loaded units. Note that // units may be known by multiple names at the same time, and hence there might // be more unit names loaded than actual units behind them. @@ -253,8 +303,52 @@ type EnableUnitFileChange struct { Destination string // Destination of the symlink } +// DisableUnitFiles() may be used to disable one or more units in the system (by +// removing symlinks to them from /etc or /run). +// +// It takes a list of unit files to disable (either just file names or full +// absolute paths if the unit files are residing outside the usual unit +// search paths), and one boolean: whether the unit was enabled for runtime +// only (true, /run), or persistently (false, /etc). +// +// This call returns an array with the changes made. The changes list +// consists of structures with three strings: the type of the change (one of +// symlink or unlink), the file name of the symlink and the destination of the +// symlink. +func (c *Conn) DisableUnitFiles(files []string, runtime bool) ([]DisableUnitFileChange, error) { + result := make([][]interface{}, 0) + err := c.sysobj.Call("DisableUnitFiles", 0, files, runtime).Store(&result) + if err != nil { + return nil, err + } + + resultInterface := make([]interface{}, len(result)) + for i := range result { + resultInterface[i] = result[i] + } + + changes := make([]DisableUnitFileChange, len(result)) + changesInterface := make([]interface{}, len(changes)) + for i := range changes { + changesInterface[i] = &changes[i] + } + + err = dbus.Store(resultInterface, changesInterface...) + if err != nil { + return nil, err + } + + return changes, nil +} + +type DisableUnitFileChange struct { + Type string // Type of the change (one of symlink or unlink) + Filename string // File name of the symlink + Destination string // Destination of the symlink +} + // Reload instructs systemd to scan for and reload unit files. This is // equivalent to a 'systemctl daemon-reload'. -func (c *Conn) Reload() (string, error) { - return c.runJob("org.freedesktop.systemd1.Manager.Reload") +func (c *Conn) Reload() error { + return c.sysobj.Call("org.freedesktop.systemd1.Manager.Reload", 0).Store() } diff --git a/third_party/github.com/coreos/go-systemd/dbus/methods_test.go b/third_party/github.com/coreos/go-systemd/dbus/methods_test.go index 64d4b8e..2c92b33 100644 --- a/third_party/github.com/coreos/go-systemd/dbus/methods_test.go +++ b/third_party/github.com/coreos/go-systemd/dbus/methods_test.go @@ -18,9 +18,11 @@ package dbus import ( "fmt" + "github.com/coreos/coreos-cloudinit/third_party/github.com/guelfey/go.dbus" "math/rand" "os" "path/filepath" + "reflect" "testing" ) @@ -50,13 +52,16 @@ func setupUnit(target string, conn *Conn, t *testing.T) { fixture := []string{abs} install, changes, err := conn.EnableUnitFiles(fixture, true, true) + if err != nil { + t.Fatal(err) + } if install != false { t.Fatal("Install was true") } if len(changes) < 1 { - t.Fatal("Expected one change, got %v", changes) + t.Fatalf("Expected one change, got %v", changes) } if changes[0].Filename != targetRun { @@ -118,6 +123,37 @@ func TestStartStopUnit(t *testing.T) { } } +// Enables a unit and then immediately tears it down +func TestEnableDisableUnit(t *testing.T) { + target := "enable-disable.service" + conn := setupConn(t) + + setupUnit(target, conn, t) + + abs, err := filepath.Abs("../fixtures/" + target) + if err != nil { + t.Fatal(err) + } + + path := filepath.Join("/run/systemd/system/", target) + + // 2. Disable the unit + changes, err := conn.DisableUnitFiles([]string{abs}, true) + if err != nil { + t.Fatal(err) + } + + if len(changes) != 1 { + t.Fatalf("Changes should include the path, %v", changes) + } + if changes[0].Filename != path { + t.Fatalf("Change should include correct filename, %+v", changes[0]) + } + if changes[0].Destination != "" { + t.Fatalf("Change destination should be empty, %+v", changes[0]) + } +} + // TestGetUnitProperties reads the `-.mount` which should exist on all systemd // systems and ensures that one of its properties is valid. func TestGetUnitProperties(t *testing.T) { @@ -139,6 +175,20 @@ func TestGetUnitProperties(t *testing.T) { if names[0] != "system.slice" { t.Fatal("unexpected wants for /") } + + prop, err := conn.GetUnitProperty(unit, "Wants") + if err != nil { + t.Fatal(err) + } + + if prop.Name != "Wants" { + t.Fatal("unexpected property name") + } + + val := prop.Value.Value().([]string) + if !reflect.DeepEqual(val, names) { + t.Fatal("unexpected property value") + } } // TestGetUnitPropertiesRejectsInvalidName attempts to get the properties for a @@ -150,10 +200,37 @@ func TestGetUnitPropertiesRejectsInvalidName(t *testing.T) { unit := "//invalid#$^/" _, err := conn.GetUnitProperties(unit) - if err == nil { t.Fatal("Expected an error, got nil") } + + _, err = conn.GetUnitProperty(unit, "Wants") + if err == nil { + t.Fatal("Expected an error, got nil") + } +} + +// TestSetUnitProperties changes a cgroup setting on the `tmp.mount` +// which should exist on all systemd systems and ensures that the +// property was set. +func TestSetUnitProperties(t *testing.T) { + conn := setupConn(t) + + unit := "tmp.mount" + + if err := conn.SetUnitProperties(unit, true, Property{"CPUShares", dbus.MakeVariant(uint64(1023))}); err != nil { + t.Fatal(err) + } + + info, err := conn.GetUnitTypeProperties(unit, "Mount") + if err != nil { + t.Fatal(err) + } + + value := info["CPUShares"].(uint64) + if value != 1023 { + t.Fatal("CPUShares of unit is not 1023, %s", value) + } } // Ensure that basic transient unit starting and stopping works. @@ -211,3 +288,27 @@ func TestStartStopTransientUnit(t *testing.T) { t.Fatalf("Test unit found in list, should be stopped") } } + +func TestConnJobListener(t *testing.T) { + target := "start-stop.service" + conn := setupConn(t) + + setupUnit(target, conn, t) + + jobSize := len(conn.jobListener.jobs) + + _, err := conn.StartUnit(target, "replace") + if err != nil { + t.Fatal(err) + } + + _, err = conn.StopUnit(target, "replace") + if err != nil { + t.Fatal(err) + } + + currentJobSize := len(conn.jobListener.jobs) + if jobSize != currentJobSize { + t.Fatal("JobListener jobs leaked") + } +} diff --git a/third_party/github.com/coreos/go-systemd/dbus/properties.go b/third_party/github.com/coreos/go-systemd/dbus/properties.go index cf9d6bd..fcd1d88 100644 --- a/third_party/github.com/coreos/go-systemd/dbus/properties.go +++ b/third_party/github.com/coreos/go-systemd/dbus/properties.go @@ -209,3 +209,12 @@ func PropPropagatesReloadTo(units ...string) Property { func PropRequiresMountsFor(units ...string) Property { return propDependency("RequiresMountsFor", units) } + +// PropSlice sets the Slice unit property. See +// http://www.freedesktop.org/software/systemd/man/systemd.resource-control.html#Slice= +func PropSlice(slice string) Property { + return Property{ + Name: "Slice", + Value: dbus.MakeVariant(slice), + } +} From 58ae8989487a4f45486104364860fdca6260226b Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Tue, 25 Mar 2014 18:50:38 -0700 Subject: [PATCH 3/3] fix(systemd): Update usage of dbus.Reload --- system/systemd.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/system/systemd.go b/system/systemd.go index a96fb29..dda28b2 100644 --- a/system/systemd.go +++ b/system/systemd.go @@ -116,8 +116,7 @@ func DaemonReload() error { return err } - _, err = conn.Reload() - return err + return conn.Reload() } func ExecuteScript(scriptPath string) (string, error) {