Skip to content
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.

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

dbus: Introduce GetAllProperties() for fetching all systemd unit properties #306

Merged
merged 1 commit into from
Jun 18, 2019

Conversation

martynasb
Copy link
Contributor

Hi,

I would like to fetch a bunch of systemd service unit properties (e.g. NRestarts, ActiveState, SubState, etc.) in a single call, similar to what I get by running systemctl show <service>.

Currently, dbus/methods.go exposes two methods for retrieving unit properties:

GetUnitTypeProperties(unit string, unitType string)
|-> return c.getProperties(path, "org.freedesktop.systemd1."+unitType) // unitType = Service

GetUnitProperties(unit string)
|-> return c.getProperties(path, "org.freedesktop.systemd1.Unit")

Both of these methods call the getProperties() helper method:

getProperties(path dbus.ObjectPath, dbusInterface string)
|   ...
|-> err = obj.Call("org.freedesktop.DBus.Properties.GetAll", 0, dbusInterface).Store(&props)

getProperties() will return a different set of properties depending on the dbusInterface parameter value (i.e. org.freedesktop.systemd1.Unit vs org.freedesktop.systemd1.Service). See the dbus-send examples below that illustrate this:

$ dbus-send --system --dest=org.freedesktop.systemd1 --type=method_call /org/freedesktop/systemd1/unit/<MYSERVICE>_2eservice --print-reply org.freedesktop.DBus.Properties.GetAll string:"org.freedesktop.systemd1.Service" | grep -A 1 -e NRestarts -e ActiveState -e SubState
         string "NRestarts"
         variant             uint32 0
$ dbus-send --system --dest=org.freedesktop.systemd1 --type=method_call /org/freedesktop/systemd1/unit/<MYSERVICE>_2eservice --print-reply org.freedesktop.DBus.Properties.GetAll string:"org.freedesktop.systemd1.Unit" | grep -A 1 -e NRestarts -e ActiveState -e SubState
         string "ActiveState"
         variant             string "active"
--
         string "SubState"
         variant             string "running"

However, if we set the interface argument to an empty string, we get all of these properties in a single call:

$ dbus-send --system --dest=org.freedesktop.systemd1 --type=method_call /org/freedesktop/systemd1/unit/<MYSERVICE>_2eservice --print-reply org.freedesktop.DBus.Properties.GetAll string:"" | grep -A 1 -e NRestarts -e ActiveState -e SubState
         string "NRestarts"
         variant             uint32 0
--
         string "ActiveState"
         variant             string "inactive"
--
         string "SubState"
         variant             string "dead"

I would like to be able to do the same using this lib by invoking something along the lines of dbus.GetProperties("<MYSERVICE>.service", "")

@lucab
Copy link
Contributor

lucab commented May 7, 2019

Thanks for the PR! I'm currently away from my usual workstation; I'll give a look at this once I'm back
(with the rest of the backlog), likely in a few days.

@martynasb
Copy link
Contributor Author

ping

@lucab
Copy link
Contributor

lucab commented Jun 13, 2019

Sorry for the long delay. I'm fine with adding this feature to the library.

The only comment I have is that if the actual goal of the PR is to expose all-interfaces properties, I'd directly call this GetAllProperties and get rid of the last string parameter. @martynasb do you have a usecase where you need to pass a non-empty arbitrary string, which is not already covered by the other methods?

@martynasb
Copy link
Contributor Author

martynasb commented Jun 18, 2019

The only comment I have is that if the actual goal of the PR is to expose all-interfaces properties, I'd directly call this GetAllProperties and get rid of the last string parameter.

I've updated my PR accordingly. I hope I'm not missing anything obvious ;)

@martynasb do you have a usecase where you need to pass a non-empty arbitrary string, which is not already covered by the other methods?

Nope. GetAllProperties() should cover all of our use cases.

@lucab
Copy link
Contributor

lucab commented Jun 18, 2019

Great, thanks! Please just squash the commits, and I'll approve and merge.

@martynasb martynasb changed the title dbus: add GetProperties() to allow callers specify dbus interface dbus: Introduce GetAllProperties() for fetching all systemd unit properties Jun 18, 2019
Copy link
Contributor

@lucab lucab left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@lucab lucab merged commit ff7011e into coreos:master Jun 18, 2019
@martynasb
Copy link
Contributor Author

Thanks. Can you cut a new release?

@lucab
Copy link
Contributor

lucab commented Jun 18, 2019

Yep, but I'd like to get #310 in first. It shouldn't take too long, I'll ping back here once done.

@lucab lucab modified the milestones: v19, v20 Jun 20, 2019
@lucab
Copy link
Contributor

lucab commented Jun 20, 2019

@martynasb
Copy link
Contributor Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants