search

coreos / fleet

fleet ties together systemd and etcd into a distributed init system
Apache License 2.0
2.42k stars 303 forks source link

fleetctl: check systemd active state via client API #1667

Closed dongsupark closed 7 years ago

dongsupark commented 7 years ago

It's not appropriate to check systemd active states with DBUS connection, as that will return correct states only on the same machine. So we should also introduce a check using cAPI.UnitStates(), to check states on other machines.

jonboulle commented 7 years ago

I don't think it's ever really sane to fall back to D-Bus, is it?

dongsupark commented 7 years ago

@jonboulle Good question. In the beginning I also tried to get it working only via cAPI. But it didn't work as expected. For some units it's reported to be not active, which made functional tests fail. So I had to fall back to checking via DBUS too, to get it working. I'd also like to know the reason.

jonboulle commented 7 years ago

@dongsupark could you link to an example of test failures in that case?

dongsupark commented 7 years ago

@jonboulle Reverted fallback, printed out more debug messages: https://gist.github.com/dongsupark/c444e10ad843090cc45750b85db51d7e

dongsupark commented 7 years ago

Finally I fixed a bug, and force-pushed.

It turns out, checking states via client API has been already working correctly, and "fleetctl list-units" has actually shown correct output. The only broken thing was the way of using loop around waitForSystemdActiveState(), where it didn't actually repeatedly try to retrieve states. So I fixed the logic to make it try to fetch the states repeatedly up to 15 seconds. The fallback routine to checking DBUS connection was removed.

Please have a look. Thanks.

jonboulle commented 7 years ago

Looks pretty good overall

dongsupark commented 7 years ago

@jonboulle Now I suppose everything was addressed. I'll merge it soon.