zincati.service: periodically restart zincati daemon #1121
Conversation
We've seen some issues in the past where a simple restart of the zincati daemon would have allowed systems to continue updating. Let's periodically restart the zincati daemon to handle cases like this in the future, which we can't always foresee. The most recent example being: coreos/fedora-coreos-tracker#1608
| @@ -22,6 +22,7 @@ Type=notify | |||
| ExecStart=/usr/libexec/zincati agent ${ZINCATI_VERBOSITY} | |||
| Restart=on-failure | |||
| RestartSec=10s | |||
| RuntimeMaxSec=3w | |||
There was a problem hiding this comment.
This seems to implicitly rely on the default FCOS release schedule meaning we'll have restarted before we hit this timeout by default. Now, zincati is pretty FCOS specific but still, this seems...slightly unclean at best.
I think the most obvious bigger fix here is to not run as a persistent daemon at all, but to run as a timer.
There was a problem hiding this comment.
(That said, I'm OK with this...but it does seem probably better done via a drop-in in fedora-coreos-config instead of hardcoded in source here)
There was a problem hiding this comment.
This seems to implicitly rely on the default FCOS release schedule meaning we'll have restarted before we hit this timeout by default. Now, zincati is pretty FCOS specific but still, this seems...slightly unclean at best.
The chosen period is FCOS-informed, but isn't tightly coupled at all. Whether for a given environment updates come in faster or slower than that, it shouldn't hurt to restart the service. I suggested making it longer so that at least in the context of FCOS (by far the most prevalent, if not only, use of Zincati), it's more of a last resort than a primary line of defense. But yes, also fine with having it be a dropin override in the FCOS overlays.
I think the most obvious bigger fix here is to not run as a persistent daemon at all, but to run as a timer.
Yes, @travier suggested this as well, but it requires a larger rework.
There was a problem hiding this comment.
I had originally suggested 2w but @jlebon suggested 3w. TBH it could be 1 day, though I'm not sure how well that would work with our periodic update strategy that people can set.
I'd be OK setting this in FCOS configs if we think it would be better placed there. Theoretically if zincati had other consumers this would be useful to them too, though those users might prefer a different cadence for restart.
There was a problem hiding this comment.
it shouldn't hurt to restart the service.
Well, it will be racy with DBus today.
But my larger (yet still minor) concern is this restart will appear as an error, when it's not.
There was a problem hiding this comment.
it shouldn't hurt to restart the service.
Well, it will be racy with DBus today.
That's fair. I actually was unaware that Zincati had a D-Bus interface until debugging #1119 (comment) today. It still seems pretty hidden currently at least, but indeed if we were to develop it more down the line as part of #498, having it be seemingly randomly force restarted would be odd and we'd probably want to rework/drop this directive.
according to the docs it should, but I can run some local tests. |
|
closing per #1121 (comment) let's re-open and discuss further if this is still needed. |
We've seen some issues in the past where a simple restart of the zincati daemon would have allowed systems to continue updating. Let's periodically restart the zincati daemon to handle cases like this in the future, which we can't always foresee.
The most recent example being: coreos/fedora-coreos-tracker#1608