From 8011d45cdafc97967c71304a76bbfebb07f2c83e Mon Sep 17 00:00:00 2001 From: JulienBalestra Date: Fri, 8 Jun 2018 17:14:17 +0200 Subject: [PATCH 1/2] systemd: use a more portable version of listUnits --- pkg/job/systemd.go | 3 ++- pkg/run/probe.go | 5 +++-- pkg/setup/manifests.go | 7 ++++++- pkg/setup/systemd.go | 7 +++---- pkg/util/systemd_action.go | 28 ++++++++++++++++++++++++++++ pkg/wait/wait.go | 4 +++- 6 files changed, 45 insertions(+), 9 deletions(-) diff --git a/pkg/job/systemd.go b/pkg/job/systemd.go index 2b1d18d..8aeabad 100644 --- a/pkg/job/systemd.go +++ b/pkg/job/systemd.go @@ -16,6 +16,7 @@ import ( "github.com/DataDog/pupernetes/pkg/config" "github.com/DataDog/pupernetes/pkg/logging" + "github.com/DataDog/pupernetes/pkg/util" ) const ( @@ -65,7 +66,7 @@ func RunSystemdJob(givenRootPath string) error { if !strings.HasSuffix(unitName, ".service") { unitName = unitName + ".service" } - units, err := dbus.ListUnitsByNames([]string{unitName}) + units, err := util.GetUnitStates(dbus, []string{unitName}) if err != nil { glog.Errorf("Cannot get the status of %s: %v", unitName, err) return err diff --git a/pkg/run/probe.go b/pkg/run/probe.go index cbfedc7..5000070 100644 --- a/pkg/run/probe.go +++ b/pkg/run/probe.go @@ -2,6 +2,7 @@ package run import ( "fmt" + "github.com/DataDog/pupernetes/pkg/util" "github.com/golang/glog" "io/ioutil" "net/http" @@ -31,7 +32,7 @@ func (r *Runtime) httpProbe(url string) error { } func (r *Runtime) probeUnitStatuses() ([]string, error) { - units, err := r.env.GetDBUSClient().ListUnitsByNames(r.env.GetSystemdUnits()) + units, err := util.GetUnitStates(r.env.GetDBUSClient(), r.env.GetSystemdUnits()) if err != nil { glog.Errorf("Unexpected error: %v", err) return nil, err @@ -39,7 +40,7 @@ func (r *Runtime) probeUnitStatuses() ([]string, error) { var failed []string for _, u := range units { s := fmt.Sprintf("unit %q with load state %q is %q", u.Name, u.LoadState, u.SubState) - glog.V(3).Infof("%s", s) + glog.V(3).Infof("Current state: %s", s) switch u.SubState { case "running": continue diff --git a/pkg/setup/manifests.go b/pkg/setup/manifests.go index de67e46..aaecaff 100644 --- a/pkg/setup/manifests.go +++ b/pkg/setup/manifests.go @@ -59,6 +59,11 @@ func (e *Environment) renderTemplates(category string) error { return err } glog.V(4).Infof("Rendering templates with the following metadata: %v", string(b)) + prefix := "" + if category == defaultTemplates.ManifestSystemdUnit { + prefix = e.systemdUnitPrefix + glog.V(4).Infof("Currently rendering %s with file prefix %q", category, prefix) + } for _, f := range files { p := path.Join(sourceDir, f.Name()) @@ -73,7 +78,7 @@ func (e *Environment) renderTemplates(category string) error { return err } - destPath := path.Join(e.rootABSPath, category, f.Name()) + destPath := path.Join(e.rootABSPath, category, prefix+f.Name()) glog.V(4).Infof("Rendering manifest %s to %s", f.Name(), destPath) dest, err := os.OpenFile(destPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0444) if err != nil { diff --git a/pkg/setup/systemd.go b/pkg/setup/systemd.go index 6913043..d2ad799 100644 --- a/pkg/setup/systemd.go +++ b/pkg/setup/systemd.go @@ -178,17 +178,16 @@ func (e *Environment) createEnd2EndSection() []*unit2.UnitOption { } func (e *Environment) createUnitFromTemplate(unitName string) error { - unitNameNoPrefix := strings.TrimPrefix(unitName, e.systemdUnitPrefix) - manifestUnitName := path.Join(e.manifestSystemdUnit, unitNameNoPrefix) + manifestUnitName := path.Join(e.manifestSystemdUnit, unitName) fd, err := os.OpenFile(manifestUnitName, os.O_RDONLY, 0) if err != nil { - glog.Errorf("Cannot read %s: %v", unitNameNoPrefix, err) + glog.Errorf("Cannot read %s: %v", manifestUnitName, err) return err } defer fd.Close() unitOptions, err := unit2.Deserialize(fd) if err != nil { - glog.Errorf("Unexpected error during parsing s: %v", unitNameNoPrefix, err) + glog.Errorf("Unexpected error during parsing s: %v", manifestUnitName, err) return err } // TODO see how to insert e.systemdEnd2EndSection diff --git a/pkg/util/systemd_action.go b/pkg/util/systemd_action.go index b0a9cdc..4e3b03e 100644 --- a/pkg/util/systemd_action.go +++ b/pkg/util/systemd_action.go @@ -11,6 +11,7 @@ import ( "github.com/coreos/go-systemd/dbus" "github.com/golang/glog" + "strings" ) func executeSystemdAction(unitName string, systemdAction func(string, string, chan<- string) (int, error)) error { @@ -46,3 +47,30 @@ func StopUnit(d *dbus.Conn, unitName string) error { glog.Infof("Stopping systemd unit: %s ...", unitName) return executeSystemdAction(unitName, d.StopUnit) } + +func GetUnitStates(d *dbus.Conn, unitNames []string) ([]dbus.UnitStatus, error) { + var units []dbus.UnitStatus + + allUnits, err := d.ListUnits() + if err != nil { + glog.Errorf("Cannot ListUnits: %v", err) + return nil, err + } + intersect := make(map[string]dbus.UnitStatus) + for _, elt := range allUnits { + if !strings.HasSuffix(elt.Name, ".service") { + continue + } + intersect[elt.Name] = elt + } + for _, wantedUnit := range unitNames { + unit, ok := intersect[wantedUnit] + if !ok { + glog.V(2).Infof("cannot find %s in actual running units", wantedUnit) + continue + } + glog.V(4).Infof("Found wanted unit %s", wantedUnit) + units = append(units, unit) + } + return units, nil +} diff --git a/pkg/wait/wait.go b/pkg/wait/wait.go index d3d18b4..ced5665 100644 --- a/pkg/wait/wait.go +++ b/pkg/wait/wait.go @@ -3,6 +3,7 @@ package wait import ( "fmt" "github.com/DataDog/pupernetes/pkg/logging" + "github.com/DataDog/pupernetes/pkg/util" "github.com/coreos/go-systemd/dbus" "github.com/golang/glog" "strings" @@ -26,7 +27,7 @@ func NewWaiter(systemdUnitName string, timeout, loggingSince time.Duration) *Wai } func getSystemdUnitState(conn *dbus.Conn, unitName string) (string, error) { - units, err := conn.ListUnitsByNames([]string{unitName}) + units, err := util.GetUnitStates(conn, []string{unitName}) if err != nil { glog.Errorf("Cannot list units: %v", err) return "", err @@ -82,6 +83,7 @@ func (w *Wait) Wait() error { if err != nil { return err } + glog.V(4).Infof("Systemd unit %s is %s", w.systemdUnitName, state) if state == "running" { glog.Infof("Systemd unit %s is %s", w.systemdUnitName, state) return nil From 8a3836e90d8d4dbf50abdd67690af72eee51d7dd Mon Sep 17 00:00:00 2001 From: JulienBalestra Date: Mon, 11 Jun 2018 11:59:01 +0200 Subject: [PATCH 2/2] systemd: add a safety for List units --- pkg/util/systemd_action.go | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/pkg/util/systemd_action.go b/pkg/util/systemd_action.go index 4e3b03e..cbe6197 100644 --- a/pkg/util/systemd_action.go +++ b/pkg/util/systemd_action.go @@ -38,16 +38,19 @@ func executeSystemdAction(unitName string, systemdAction func(string, string, ch } } +// StartUnit call dbus to start the given unit name func StartUnit(d *dbus.Conn, unitName string) error { glog.Infof("Starting systemd unit: %s ...", unitName) return executeSystemdAction(unitName, d.StartUnit) } +// StopUnit call dbus to stop the given unit name func StopUnit(d *dbus.Conn, unitName string) error { glog.Infof("Stopping systemd unit: %s ...", unitName) return executeSystemdAction(unitName, d.StopUnit) } +// GetUnitStates returns the dbus UnitStates of unit names passed in parameter func GetUnitStates(d *dbus.Conn, unitNames []string) ([]dbus.UnitStatus, error) { var units []dbus.UnitStatus @@ -56,21 +59,30 @@ func GetUnitStates(d *dbus.Conn, unitNames []string) ([]dbus.UnitStatus, error) glog.Errorf("Cannot ListUnits: %v", err) return nil, err } - intersect := make(map[string]dbus.UnitStatus) + intersectName := make(map[string][]dbus.UnitStatus) for _, elt := range allUnits { if !strings.HasSuffix(elt.Name, ".service") { continue } - intersect[elt.Name] = elt + // Note that units may be known by multiple names at the same time + intersectName[elt.Name] = append(intersectName[elt.Name], elt) } for _, wantedUnit := range unitNames { - unit, ok := intersect[wantedUnit] + unitStatuses, ok := intersectName[wantedUnit] if !ok { glog.V(2).Infof("cannot find %s in actual running units", wantedUnit) continue } - glog.V(4).Infof("Found wanted unit %s", wantedUnit) - units = append(units, unit) + if len(unitStatuses) != 1 { + err := fmt.Errorf("invalid number of unitStatuses %s: %d", wantedUnit, len(unitStatuses)) + glog.Errorf("Cannot select unit: %v", err) + for _, elt := range unitStatuses { + glog.Errorf("Dbus yield for %s: %s %d %s %s %s %s", wantedUnit, elt.Name, elt.JobId, elt.LoadState, elt.SubState) + } + return nil, err + } + glog.V(4).Infof("Found wanted unitStatus %s", wantedUnit) + units = append(units, unitStatuses[0]) } return units, nil }