dbus: add GetUnitProcessesContext to list any unit's running processes #379
dbus: add GetUnitProcessesContext to list any unit's running processes #379pphysch wants to merge 16 commits intocoreos:mainfrom
Conversation
dbus/methods_test.go
Outdated
| } | ||
|
|
||
| if !exists { | ||
| t.Errorf("PID %d ('/bin/sleep 400') not found in current Slice unit's process list", pid) |
There was a problem hiding this comment.
It looks like this test is failing on some CI runs. I suspect there is a race between spawning the child sleep and getting unit processes. It could be useful to retry the GetUnitProcessesContext() a few times until we get a non-empty set with /bin/sleep, or eventually timeout if we never get that.
dbus/methods.go
Outdated
| } | ||
|
|
||
| // 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) { |
There was a problem hiding this comment.
I think we can directly call this GetUnitProcesses, plus avoid the separate *Internal helper.
|
Thanks for the PR! It looks like the new test is frequently failing in CI, I think because it is internally racing (see review). |
|
Looks like the test is still failing even with delayed retries; I'll rewrite it so that it does more mocking and has less reliance on the system/container. |
|
Any chance this PR can get reviewed? Tests were passing as of the last commit. |
dbus/methods.go
Outdated
|
|
||
| // 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) |
There was a problem hiding this comment.
Please use any instead of interface{}.
There was a problem hiding this comment.
In fact this should now use the new storeSlice[Process], being added by #493
3 participants
This PR adds a binding for
org.freedesktop.systemd1.Manager.GetUnitProcesses, taking a unit string and returning a list of all processes currently running under that unit (including child units).This is convenient for getting a lot of information about a Slice's running processes, as it descends into child Slices/Scopes and includes those processes in the list. For example, if we target
user-1000.slicewe can learn about the differentsession-*Scopes under that user and all the processes therein.My test (which may need adjustments to be more idiomatic/isolated) passes on systemd v239 (RHEL 8), but this Manager method doesn't exist on v219 (RHEL 7). I dug for a bit and can't find the exact version that implemented the GetProcesses API, but it seems to have been around 5 years ago.
This is my first PR here so please let me know how I can fix it! I used existing Manager method bindings as a template, so the code itself should be conformant even if the test needs work.