fleetctl: Adding restart command

[hhoover]

This allows users to easily do rolling restarts of units in the cluster.

This is the squash requested in #1082, as requested. I didn't want to wait.
This closes #1082 and closes #975

[hhoover]

@bcwaldon do you mind reviewing this PR

[bcwaldon]

Please move the restart-related documentation to the bottom of the "Start and stop units" block.

[bcwaldon]

This doesn't seem to work for global units. For an actual restart to happen, we need to guarantee a state transition from launched through loaded and back to launched. If we don't block for global units, they aren't actually guaranteed to restart, which this command is trying to provide.

Having said that, I don't think it is possible given the current data model to get what we want with global units, so I would just ignore them and warn the user that we can't restart globals.

[guilhem]

Hi all,
I have a huge problem with the way you are doing restart.

systemctl stop service && systemctl start service != systemctl restart service.

Why:
If service 2 Requires service 1, restarting service 1 will stop service 2, than restart (stop/start) service 1 BUT (and this is the important point), it will start again service 2.

With a simple stop/start... at the end of service 1 "restart" process, service 2 will be stopped...

[antrik]

@guilhem I hope I'm not saying something stupid here -- but it seems to me that if all you want to do is have systemctl --restart executed, you could just do fleetctl ssh <unit> systemctl restart <unit>?... (+/- syntax)

The point about dependencies is worth consideration though.

[antrik]

I believe the fact that this doesn't work for global units, is just a symptom of this approach -- imperatively launching start and stop commands (or using systemctl restart as in #961 ) -- being fundamentally incompatible with fleet's declare & reconcile model.

It seems to me the right way to handle this is to declare that we want the unit to have been started after a certain timestamp (defaulting to now()), and storing this intention in the registry; upon which the reconciler will check this condition, and restart the unit as necessary.

@jonboulle does that strike you as reasonable?

[jonboulle]

@antrik I think your suggestion is eminently reasonable, but it's a fairly significant change in API. I think we could punt that to a possible v1+ feature and close this particular request with your advice in #1238 (comment) (perhaps adding documentation to that effect). Thoughts?

[antrik]

@jonboulle well, the advice only applies to the specific use case when people just want systemctl restart behaviour. (And to https://github.com/coreos/fleet/pull/961/files , which just wanted to add a wrapper for that...) I still think a "proper" implementation following fleet semantics would be valuable -- especially considering how often this feature has been requested in one form or another...

You are certainly right though that this is not a trivial change; and while it sounds like an interesting task that I would like to work on, it's up to you to decide priorities.

Either way, I do agree that it's probably best to close all the outstanding PRs, as none of them use the "right" approach; and add a note to this effect in #975 .

[jonboulle]

@antrik +1 on your summary, mind doing that?

[antrik]

@jonboulle doing what exactly? Closing the PRs? I don't have permission for that...

[tixxdz]

@antrik

Either way, I do agree that it's probably best to close all the outstanding PRs, as none of them use the "right" approach; and add a note to this effect in #975 .

Could you please open a new issue or a feature request based on your comment #1238 (comment) which will be +v1 feature

Then please a PR to add documentation based on your comment: #1238 (comment) and yes systemd dependencies do matter when restarting services and even the order since some services may need a socket unit to be already running...

Ping @hhoover @guilhem any comments ?

[antrik]

@tixxdz I don't think we should open a separate wishlist issue, as it would essentially just be a duplicate of #975 . I will post a summary of the conclusions there; and the tags/milestones can be adjusted there as appropriate. (Not by me though, as I don't have the necessary permissions...)

[tixxdz]

@tixxdz I don't think we should open a separate wishlist issue, as it would essentially just be a duplicate of #975 . I will post a summary of the conclusions there; and the tags/milestones can be adjusted there as appropriate. (Not by me though, as I don't have the necessary permissions...)

Sounds fine, if you think that we should also add documentation based on your comment #1238 (comment) then please open a PR, thanks!

[hhoover]

Ping @hhoover any comments ?

This PR was opened quite a while ago. In that time I've changed companies/jobs/life. As such my toolset also changed and I no longer use fleet. At the time I just used a forked version that included this code. Use it, don't use it, do something else (like what @antrik is suggesting).

I'm not emotionally invested in this issue either way at this point and what I think doesn't really matter. It's up to the folks who maintain/use this today to decide how best to move forward. If this code helps the community, great. If it doesn't, trash it. Thanks for thinking of me though. 😄

[guilhem]

For my part I don't understand new points. To much information in so many PR for my brain ^^

My only point is that it's painful to manage unit change.
For example: have you ever try to manage an edit on a gobal .mount unit...
Quite tricky when many other services rely on it ;)

What I have in view is to be able to "load" a new version of a unit without stopping existing one. And then systemctl restart and converge with systemd dependency tree (systemd will automatically stop and restart any other unit who Requires it).
But from what I can see, fleet isn't made for this kind of purpose and I try to migrate to another solution / design (some part may rely on fleet).

So for this one, do your best as you always do and it will be good for me. :)

[antrik]

@guilhem so using fleetctl ssh with systemctl restart doesn't work for you? Could you please explain why?

[tixxdz]

Thanks all for the comments, more details on how the feature should be implemented used are here: #975 (comment)

Closing.