Skip to content

dbus: add support for new ListUnits* methods #150

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lucab merged 2 commits into from
Jun 7, 2016
Merged

dbus: add support for new ListUnits* methods #150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
124d391
dbus: added new dbus methods
kayrus Apr 21, 2016
c498a32
test: optimized dbus tests
kayrus Jun 7, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 45 additions & 8 deletions dbus/methods.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,12 +239,11 @@ type UnitStatus struct {
JobPath dbus.ObjectPath // The job object path
}

// 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.
func (c *Conn) ListUnits() ([]UnitStatus, error) {
type storeFunc func(retvalues ...interface{}) error

func (c *Conn) listUnitsInternal(f storeFunc) ([]UnitStatus, error) {
result := make([][]interface{}, 0)
err := c.sysobj.Call("org.freedesktop.systemd1.Manager.ListUnits", 0).Store(&result)
err := f(&result)
if err != nil {
return nil, err
}
Expand All @@ -268,15 +267,43 @@ func (c *Conn) ListUnits() ([]UnitStatus, error) {
return status, nil
}

// 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.
func (c *Conn) ListUnits() ([]UnitStatus, error) {
return c.listUnitsInternal(c.sysobj.Call("org.freedesktop.systemd1.Manager.ListUnits", 0).Store)
}

// ListUnitsFiltered returns an array with units filtered by state.
// It takes a list of units' statuses to filter.
func (c *Conn) ListUnitsFiltered(states []string) ([]UnitStatus, error) {
return c.listUnitsInternal(c.sysobj.Call("org.freedesktop.systemd1.Manager.ListUnitsFiltered", 0, states).Store)
}

// ListUnitsByPatterns returns an array with units.
// It takes a list of units' statuses and names to filter.
// 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.
func (c *Conn) ListUnitsByPatterns(states []string, patterns []string) ([]UnitStatus, error) {
return c.listUnitsInternal(c.sysobj.Call("org.freedesktop.systemd1.Manager.ListUnitsByPatterns", 0, states, patterns).Store)
}

// ListUnitsByNames returns an array with units. It takes a list of units'
// names and returns an UnitStatus array. Comparing to ListUnitsByPatterns
// method, this method returns statuses even for inactive or non-existing
// units. Input array should contain exact unit names, but not patterns.
func (c *Conn) ListUnitsByNames(units []string) ([]UnitStatus, error) {
return c.listUnitsInternal(c.sysobj.Call("org.freedesktop.systemd1.Manager.ListUnitsByNames", 0, units).Store)
}

type UnitFile struct {
Path string
Type string
}

// ListUnitFiles returns an array of all available units on disk.
func (c *Conn) ListUnitFiles() ([]UnitFile, error) {
func (c *Conn) listUnitFilesInternal(f storeFunc) ([]UnitFile, error) {
result := make([][]interface{}, 0)
err := c.sysobj.Call("org.freedesktop.systemd1.Manager.ListUnitFiles", 0).Store(&result)
err := f(&result)
if err != nil {
return nil, err
}
Expand All @@ -300,6 +327,16 @@ func (c *Conn) ListUnitFiles() ([]UnitFile, error) {
return files, nil
}

// ListUnitFiles returns an array of all available units on disk.
func (c *Conn) ListUnitFiles() ([]UnitFile, error) {
return c.listUnitFilesInternal(c.sysobj.Call("org.freedesktop.systemd1.Manager.ListUnitFiles", 0).Store)
}

// ListUnitFilesByPatterns returns an array of all available units on disk matched the patterns.
func (c *Conn) ListUnitFilesByPatterns(states []string, patterns []string) ([]UnitFile, error) {
return c.listUnitFilesInternal(c.sysobj.Call("org.freedesktop.systemd1.Manager.ListUnitFilesByPatterns", 0, states, patterns).Store)
}

type LinkUnitFileChange EnableUnitFileChange

// LinkUnitFiles() links unit files (that are located outside of the
Expand Down
211 changes: 181 additions & 30 deletions dbus/methods_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"fmt"
"math/rand"
"os"
"path"
"path/filepath"
"reflect"
"testing"
Expand Down Expand Up @@ -70,6 +71,24 @@ func linkUnit(target string, conn *Conn, t *testing.T) {
}
}

func getUnitStatus(units []UnitStatus, name string) *UnitStatus {
for _, u := range units {
if u.Name == name {
return &u
}
}
return nil
}

func getUnitFile(units []UnitFile, name string) *UnitFile {
for _, u := range units {
if path.Base(u.Path) == name {
return &u
}
}
return nil
}

// Ensure that basic unit starting and stopping works.
func TestStartStopUnit(t *testing.T) {
target := "start-stop.service"
Expand All @@ -92,18 +111,11 @@ func TestStartStopUnit(t *testing.T) {

units, err := conn.ListUnits()

var unit *UnitStatus
for _, u := range units {
if u.Name == target {
unit = &u
}
}
unit := getUnitStatus(units, target)

if unit == nil {
t.Fatalf("Test unit not found in list")
}

if unit.ActiveState != "active" {
} else if unit.ActiveState != "active" {
t.Fatalf("Test unit not active")
}

Expand All @@ -118,18 +130,169 @@ func TestStartStopUnit(t *testing.T) {

units, err = conn.ListUnits()

unit = nil
for _, u := range units {
if u.Name == target {
unit = &u
}
}
unit = getUnitStatus(units, target)

if unit != nil {
t.Fatalf("Test unit found in list, should be stopped")
}
}

// Ensure that ListUnitsByNames works.
func TestListUnitsByNames(t *testing.T) {
target1 := "systemd-journald.service"
target2 := "unexisting.service"

conn := setupConn(t)

units, err := conn.ListUnitsByNames([]string{target1, target2})

if err != nil {
t.Skip(err)
}

unit := getUnitStatus(units, target1)

if unit == nil {
t.Fatalf("%s unit not found in list", target1)
} else if unit.ActiveState != "active" {
t.Fatalf("%s unit should be active but it is %s", target1, unit.ActiveState)
}

unit = getUnitStatus(units, target2)

if unit == nil {
Copy link
Contributor

@jonboulle jonboulle May 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should it be found in the list? this is counterintuitive the way it's written

Copy link
Contributor Author

@kayrus kayrus May 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonboulle take a look on this comment https://github.com/coreos/go-systemd/pull/150/files#diff-b5b8384be79564335a3b50bcfed63e91R293
systemd should return unit states for non-existing units.

Copy link
Contributor

@jonboulle jonboulle May 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh... confusing

On Tue, May 31, 2016 at 5:34 PM, kayrus [email protected] wrote:

In dbus/methods_test.go
#150 (comment):

  •   t.Fatalf("%s unit not found in list", target1)
  • }
  • if unit.ActiveState != "active" {
  •   t.Fatalf("%s unit should be active but it is %s", target1, unit.ActiveState)
  • }
  • unit = nil
  • for _, u := range units {
  •   if u.Name == target2 {

  •       unit = &u

  •       break

  •   }
  • }
  • if unit == nil {

@jonboulle https://github.com/jonboulle take a look on this comment
https://github.com/coreos/go-systemd/pull/150/files#diff-b5b8384be79564335a3b50bcfed63e91R293
systemd should return unit states for non-existing units.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/coreos/go-systemd/pull/150/files/124d391a9338d39c19cd55de93b70a4e7ed649f4#r65206518,
or mute the thread
https://github.com/notifications/unsubscribe/ACewNzMPezWMqZoW3zMavEVcKRxk5tcbks5qHFUSgaJpZM4IMyF5
.

Copy link

@tixxdz tixxdz Jun 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonboulle yep I confirm it's a bit confusing, but that's how systemd works actually please check this comment systemd/systemd#3182 (comment) so basically the implementation doesn't bother it's just gives you back what you have requested setting up the right state of course.

Now this test can be updated to be more readable:
just create a target1 and target2 object which has {unit_name, activestate, a bolean found or not} , do the whole check inside the same loop, and check after the loop if the units have been found or not. The doc of ListUnitsByNames() clearly states that it may return results for even non-existent units which is good.

t.Fatalf("Unexisting test unit not found in list")
} else if unit.ActiveState != "inactive" {
t.Fatalf("Test unit should be inactive")
}
}

// Ensure that ListUnitsByPatterns works.
func TestListUnitsByPatterns(t *testing.T) {
target1 := "systemd-journald.service"
target2 := "unexisting.service"

conn := setupConn(t)

units, err := conn.ListUnitsByPatterns([]string{}, []string{"systemd-journald*", target2})

if err != nil {
t.Skip(err)
}

unit := getUnitStatus(units, target1)

if unit == nil {
t.Fatalf("%s unit not found in list", target1)
} else if unit.ActiveState != "active" {
t.Fatalf("Test unit should be active")
}

unit = getUnitStatus(units, target2)

Copy link

@tixxdz tixxdz Jun 2, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same thing with previous test ? have one loop maybe ?

if unit != nil {
t.Fatalf("Unexisting test unit found in list")
}
}

// Ensure that ListUnitsFiltered works.
func TestListUnitsFiltered(t *testing.T) {
target := "systemd-journald.service"

conn := setupConn(t)

units, err := conn.ListUnitsFiltered([]string{"active"})

if err != nil {
t.Fatal(err)
}

unit := getUnitStatus(units, target)

if unit == nil {
t.Fatalf("%s unit not found in list", target)
} else if unit.ActiveState != "active" {
t.Fatalf("Test unit should be active")
}

units, err = conn.ListUnitsFiltered([]string{"inactive"})

if err != nil {
t.Fatal(err)
}

unit = getUnitStatus(units, target)

if unit != nil {
t.Fatalf("Inactive unit should not be found in list")
}
}

// Ensure that ListUnitFilesByPatterns works.
func TestListUnitFilesByPatterns(t *testing.T) {
target1 := "systemd-journald.service"
target2 := "exit.target"

conn := setupConn(t)

units, err := conn.ListUnitFilesByPatterns([]string{"static"}, []string{"systemd-journald*", target2})

if err != nil {
t.Skip(err)
}

unit := getUnitFile(units, target1)

if unit == nil {
t.Fatalf("%s unit not found in list", target1)
} else if unit.Type != "static" {
t.Fatalf("Test unit file should be static")
}

units, err = conn.ListUnitFilesByPatterns([]string{"disabled"}, []string{"systemd-journald*", target2})

if err != nil {
t.Fatal(err)
}

unit = getUnitFile(units, target2)

if unit == nil {
t.Fatalf("%s unit not found in list", target2)
} else if unit.Type != "disabled" {
t.Fatalf("%s unit file should be disabled", target2)
}
}

func TestListUnitFiles(t *testing.T) {
target1 := "systemd-journald.service"
target2 := "exit.target"

conn := setupConn(t)

units, err := conn.ListUnitFiles()

if err != nil {
t.Fatal(err)
}

unit := getUnitFile(units, target1)

if unit == nil {
t.Fatalf("%s unit not found in list", target1)
} else if unit.Type != "static" {
t.Fatalf("Test unit file should be static")
}

unit = getUnitFile(units, target2)

if unit == nil {
t.Fatalf("%s unit not found in list", target2)
} else if unit.Type != "disabled" {
t.Fatalf("%s unit file should be disabled", target2)
}
}

// Enables a unit and then immediately tears it down
func TestEnableDisableUnit(t *testing.T) {
target := "enable-disable.service"
Expand Down Expand Up @@ -298,18 +461,11 @@ func TestStartStopTransientUnit(t *testing.T) {

units, err := conn.ListUnits()

var unit *UnitStatus
for _, u := range units {
if u.Name == target {
unit = &u
}
}
unit := getUnitStatus(units, target)

if unit == nil {
t.Fatalf("Test unit not found in list")
}

if unit.ActiveState != "active" {
} else if unit.ActiveState != "active" {
t.Fatalf("Test unit not active")
}

Expand All @@ -324,12 +480,7 @@ func TestStartStopTransientUnit(t *testing.T) {

units, err = conn.ListUnits()

unit = nil
for _, u := range units {
if u.Name == target {
unit = &u
}
}
unit = getUnitStatus(units, target)

if unit != nil {
t.Fatalf("Test unit found in list, should be stopped")
Expand Down
2 changes: 1 addition & 1 deletion test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ split=(${TEST// / })
TEST=${split[@]/#/${REPO_PATH}/}

echo "Running tests..."
go test ${COVER} $@ ${TEST}
go test -v ${COVER} $@ ${TEST}

echo "Checking gofmt..."
fmtRes=$(gofmt -l $FMT)
Expand Down