fleetctl: check also systemd states after starting units

[dongsupark]

To resolve inconsistency between "systemctl start" and "fleetctl start", fleetctl needs to check systemd states after starting units. If the systemd state is not a desired one, wait until it gets an active state.

Fixes: #1025

[dongsupark]

Updated. (after several hiccups)

• Handle return value of tryWaitForSystemdActiveState carefully. It should not return 1 only because of a missing systemd dbus connection. Otherwise it will break unit tests based on Travis, as that environment cannot correctly set up a systemd dbus connection.
• Added a missing nil checking before conn.Close(). Also call conn.Close() after checking for nil.
• Wrote comments etc.

[dongsupark]

I'm going to merge it soon, unless anyone has other opinions.

[jonboulle]

imho the ret value is pretty unidiomatic; rather, return an error, and have the callers decide to log the error or not

[dongsupark]

Agreed. I'll try to make it return an error.

[jonboulle]

unnecessary

[dongsupark]

Correct. I'll just remove err = nil.

[jonboulle]

I think this is wrong; it's checking against a local systemd instance over dbus, but that will ONLY be right if fleetctl happens to be invoked on the same system on which the units are scheduled, right? Surely we need to check against the API, as we do with the other states?

[dongsupark]

@jonboulle Ok. I'll try to take a different approach as you suggested. From my understanding, schema.UnitState has already its own fields like SystemdActiveState, SystemdLoadState, and SystemdSubState. So we should make use of cAPI.UnitStates() to retrieve these states, to make sure that these are the expected states. Is that correct?

[jonboulle]

Yep, that sounds about right!

On 19 August 2016 at 10:28, Dongsu Park notifications@github.com wrote:

@jonboulle https://github.com/jonboulle Ok. I'll try to take a
different approach as you suggested. From my understanding,
schema.UnitState has already its own fields like SystemdActiveState,
SystemdLoadState, and SystemdSubState. So we should make use of
cAPI.UnitStates() to retrieve these states, to make sure that these are
the expected states. Is that correct?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1566 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ACewN-tGx7Yv86yZzL7X0TJt8xhP4t9Uks5qhWkrgaJpZM4IPDSK
.