From c4fb696f6702b46a1e51eb1036489c5031ce51de Mon Sep 17 00:00:00 2001 From: Paul Fischer Date: Mon, 20 Sep 2021 11:09:09 -0600 Subject: [PATCH 1/7] Added ListUnitProcessesContext --- dbus/methods.go | 35 +++++++++++++++++++++++++++++++++++ dbus/methods_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/dbus/methods.go b/dbus/methods.go index f577ab37..e8670f77 100644 --- a/dbus/methods.go +++ b/dbus/methods.go @@ -839,3 +839,38 @@ func (c *Conn) FreezeUnit(ctx context.Context, unit string) error { func (c *Conn) ThawUnit(ctx context.Context, unit string) error { return c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.ThawUnit", 0, unit).Store() } + +type Process struct { + Path string // The primary unit name as string + PID uint64 + Command string +} + +// GetUnitProcessesContext returns an array with all currently running processes in a unit +func (c *Conn) GetUnitProcessesContext(ctx context.Context, unit string) ([]Process, error) { + return c.getUnitProcessesInternal(ctx, unit) +} + +func (c *Conn) getUnitProcessesInternal(ctx context.Context, unit string) ([]Process, error) { + result := make([][]interface{}, 0) + if err := c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.GetUnitProcesses", 0, unit).Store(&result); err != nil { + return nil, err + } + + resultInterface := make([]interface{}, len(result)) + for i := range result { + resultInterface[i] = result[i] + } + + process := make([]Process, len(result)) + processInterface := make([]interface{}, len(process)) + for i := range process { + processInterface[i] = &process[i] + } + + if err := dbus.Store(resultInterface, processInterface...); err != nil { + return nil, err + } + + return process, nil +} diff --git a/dbus/methods_test.go b/dbus/methods_test.go index 55bc2c9c..1c2e2bc6 100644 --- a/dbus/methods_test.go +++ b/dbus/methods_test.go @@ -26,6 +26,7 @@ import ( "testing" "time" + "github.com/coreos/go-systemd/v22/util" "github.com/godbus/dbus/v5" ) @@ -1657,3 +1658,41 @@ func TestFreezer(t *testing.T) { runStopUnit(t, conn, TrUnitProp{target, nil}) } + +func TestListUnitProcesses(t *testing.T) { + target, err := util.CurrentUnitName() + if err != nil { + t.Fatal(err) + } + + conn := setupConn(t) + defer conn.Close() + + cmd := exec.Command("/bin/sleep", "400") + err = cmd.Start() + if err != nil { + t.Fatal(err) + } + defer cmd.Process.Kill() + + pid := uint64(cmd.Process.Pid) + + ctx := context.Background() + processes, err := conn.GetUnitProcessesContext(ctx, target) + + if err != nil { + t.Fatal(err) + } + + exists := false + for _, p := range processes { + if p.PID == pid && p.Command == cmd.String() { + exists = true + t.Logf("Found %v\n", p) + } + } + + if !exists { + t.Errorf("PID %d ('/bin/sleep 400') not found in current unit's process list", pid) + } +} From 5a40f4c559258c83169eb8b909e3e3b04c6b3891 Mon Sep 17 00:00:00 2001 From: Paul Fischer Date: Mon, 20 Sep 2021 11:28:11 -0600 Subject: [PATCH 2/7] Improve docs & test --- dbus/methods.go | 8 ++++---- dbus/methods_test.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/dbus/methods.go b/dbus/methods.go index e8670f77..39eea642 100644 --- a/dbus/methods.go +++ b/dbus/methods.go @@ -841,12 +841,12 @@ func (c *Conn) ThawUnit(ctx context.Context, unit string) error { } type Process struct { - Path string // The primary unit name as string - PID uint64 - Command string + Path string // Where this process exists in the unit/cgroup hierarchy + PID uint64 // The numeric process ID (PID) + Command string // The process command and arguments as a string } -// GetUnitProcessesContext returns an array with all currently running processes in a unit +// GetUnitProcessesContext returns an array with all currently running processes in a unit *including* its child units. func (c *Conn) GetUnitProcessesContext(ctx context.Context, unit string) ([]Process, error) { return c.getUnitProcessesInternal(ctx, unit) } diff --git a/dbus/methods_test.go b/dbus/methods_test.go index 1c2e2bc6..23ec8ac3 100644 --- a/dbus/methods_test.go +++ b/dbus/methods_test.go @@ -1660,7 +1660,7 @@ func TestFreezer(t *testing.T) { } func TestListUnitProcesses(t *testing.T) { - target, err := util.CurrentUnitName() + target, err := util.GetRunningSlice() // This test should still pass even if the cmd is spawned in a child unit (i.e. session Scope) of the current Slice if err != nil { t.Fatal(err) } @@ -1693,6 +1693,6 @@ func TestListUnitProcesses(t *testing.T) { } if !exists { - t.Errorf("PID %d ('/bin/sleep 400') not found in current unit's process list", pid) + t.Errorf("PID %d ('/bin/sleep 400') not found in current Slice unit's process list", pid) } } From cde39f9ee548f37cc63d62b0d687db7dffcb429c Mon Sep 17 00:00:00 2001 From: Paul Fischer Date: Mon, 20 Sep 2021 13:41:29 -0600 Subject: [PATCH 3/7] Fix to support go<1.13 --- dbus/methods_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dbus/methods_test.go b/dbus/methods_test.go index 23ec8ac3..5bb56797 100644 --- a/dbus/methods_test.go +++ b/dbus/methods_test.go @@ -22,6 +22,7 @@ import ( "path" "path/filepath" "reflect" + "strings" "syscall" "testing" "time" @@ -1686,7 +1687,7 @@ func TestListUnitProcesses(t *testing.T) { exists := false for _, p := range processes { - if p.PID == pid && p.Command == cmd.String() { + if p.PID == pid && strings.HasPrefix(p.Command, "/bin/sleep") { exists = true t.Logf("Found %v\n", p) } From a338cfb2b4c1fb13178ed90b853830ad380ce1c9 Mon Sep 17 00:00:00 2001 From: Paul Fischer Date: Tue, 21 Sep 2021 10:48:33 -0600 Subject: [PATCH 4/7] Rename GetUnitProcessesContext and remove redundant func --- dbus/methods.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/dbus/methods.go b/dbus/methods.go index 39eea642..b8df02ab 100644 --- a/dbus/methods.go +++ b/dbus/methods.go @@ -846,12 +846,8 @@ type Process struct { Command string // The process command and arguments as a string } -// GetUnitProcessesContext returns an array with all currently running processes in a unit *including* its child units. -func (c *Conn) GetUnitProcessesContext(ctx context.Context, unit string) ([]Process, error) { - return c.getUnitProcessesInternal(ctx, unit) -} - -func (c *Conn) getUnitProcessesInternal(ctx context.Context, unit string) ([]Process, error) { +// GetUnitProcesses returns an array with all currently running processes in a unit *including* its child units. +func (c *Conn) GetUnitProcesses(ctx context.Context, unit string) ([]Process, error) { result := make([][]interface{}, 0) if err := c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.GetUnitProcesses", 0, unit).Store(&result); err != nil { return nil, err From 020c50bd27d7f3262027da5e4dc28724157968df Mon Sep 17 00:00:00 2001 From: Paul Fischer Date: Tue, 21 Sep 2021 10:54:39 -0600 Subject: [PATCH 5/7] Add retry to address possible race with process spawning --- dbus/methods_test.go | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/dbus/methods_test.go b/dbus/methods_test.go index 5bb56797..8e2bb4e5 100644 --- a/dbus/methods_test.go +++ b/dbus/methods_test.go @@ -1679,21 +1679,28 @@ func TestListUnitProcesses(t *testing.T) { pid := uint64(cmd.Process.Pid) ctx := context.Background() - processes, err := conn.GetUnitProcessesContext(ctx, target) - - if err != nil { - t.Fatal(err) - } exists := false - for _, p := range processes { - if p.PID == pid && strings.HasPrefix(p.Command, "/bin/sleep") { - exists = true - t.Logf("Found %v\n", p) + retries := 3 + var attempt int + for attempt = 1; !exists && attempt <= retries; attempt++ { + time.Sleep(time.Millisecond * 50) + processes, err := conn.GetUnitProcesses(ctx, target) + + if err != nil { + t.Fatal(err) + } + + for _, p := range processes { + if p.PID == pid && strings.HasPrefix(p.Command, "/bin/sleep") { + exists = true + t.Logf("Found %v after %d attempts\n", p, attempt) + break + } } } if !exists { - t.Errorf("PID %d ('/bin/sleep 400') not found in current Slice unit's process list", pid) + t.Errorf("PID %d ('/bin/sleep 400') not found in current Slice unit's process list after %d attempts", pid, attempt) } } From 1acf322ad9aff7ffaba455431b93873fafd70ac3 Mon Sep 17 00:00:00 2001 From: Paul Fischer Date: Tue, 21 Sep 2021 11:37:57 -0600 Subject: [PATCH 6/7] Refactor TestListUnitProcesses to use test unit --- dbus/methods_test.go | 54 +++++++++++++++++++--------------------- fixtures/list-me.service | 5 ++++ 2 files changed, 30 insertions(+), 29 deletions(-) create mode 100644 fixtures/list-me.service diff --git a/dbus/methods_test.go b/dbus/methods_test.go index 8e2bb4e5..1c3c88f5 100644 --- a/dbus/methods_test.go +++ b/dbus/methods_test.go @@ -27,7 +27,6 @@ import ( "testing" "time" - "github.com/coreos/go-systemd/v22/util" "github.com/godbus/dbus/v5" ) @@ -1661,46 +1660,43 @@ func TestFreezer(t *testing.T) { } func TestListUnitProcesses(t *testing.T) { - target, err := util.GetRunningSlice() // This test should still pass even if the cmd is spawned in a child unit (i.e. session Scope) of the current Slice - if err != nil { - t.Fatal(err) - } + ctx := context.Background() + target := "list-me.service" conn := setupConn(t) defer conn.Close() - cmd := exec.Command("/bin/sleep", "400") - err = cmd.Start() + setupUnit(target, conn, t) + linkUnit(target, conn, t) + + reschan := make(chan string) + _, err := conn.StartUnitContext(ctx, target, "replace", reschan) if err != nil { t.Fatal(err) } - defer cmd.Process.Kill() - - pid := uint64(cmd.Process.Pid) - - ctx := context.Background() + defer runStopUnit(t, conn, TrUnitProp{target, nil}) - exists := false - retries := 3 - var attempt int - for attempt = 1; !exists && attempt <= retries; attempt++ { - time.Sleep(time.Millisecond * 50) - processes, err := conn.GetUnitProcesses(ctx, target) + job := <-reschan + if job != "done" { + t.Fatal("Job is not done:", job) + } - if err != nil { - t.Fatal(err) - } + processes, err := conn.GetUnitProcesses(ctx, target) - for _, p := range processes { - if p.PID == pid && strings.HasPrefix(p.Command, "/bin/sleep") { - exists = true - t.Logf("Found %v after %d attempts\n", p, attempt) - break - } + if err != nil { + e, ok := err.(dbus.Error) + if ok && (e.Name == "org.freedesktop.DBus.Error.UnknownMethod" || e.Name == "org.freedesktop.DBus.Error.NotSupported") { + t.SkipNow() } + t.Fatalf("failed to list processes of %s: %s", target, err) } - if !exists { - t.Errorf("PID %d ('/bin/sleep 400') not found in current Slice unit's process list after %d attempts", pid, attempt) + for _, p := range processes { + if strings.HasPrefix(p.Command, "/bin/sleep") { + t.Logf("Found %v.\n", p) + return + } } + t.Log("processes:", processes) + t.Error("The sleep process was not found in list-me.service unit's process list.") } diff --git a/fixtures/list-me.service b/fixtures/list-me.service new file mode 100644 index 00000000..7c9ead40 --- /dev/null +++ b/fixtures/list-me.service @@ -0,0 +1,5 @@ +[Unit] +Description=GetUnitProcesses test + +[Service] +ExecStart=/bin/sleep 400 From de54554541b54998254497f6bb13340ebb96d39f Mon Sep 17 00:00:00 2001 From: Paul Fischer Date: Tue, 21 Sep 2021 11:43:02 -0600 Subject: [PATCH 7/7] More robust process matching --- dbus/methods_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbus/methods_test.go b/dbus/methods_test.go index 1c3c88f5..f827f0af 100644 --- a/dbus/methods_test.go +++ b/dbus/methods_test.go @@ -1692,7 +1692,7 @@ func TestListUnitProcesses(t *testing.T) { } for _, p := range processes { - if strings.HasPrefix(p.Command, "/bin/sleep") { + if strings.Contains(p.Command, "sleep") { t.Logf("Found %v.\n", p) return }