upgrading docker-ce while the service is stopped leads to delayed/surprise service restart

[Tyrael]

we just recently discovered that if you upgrade the docker-ce package (at least on oracle9 with the rhel9 packages) while the docker service is stopped, then the docker service will be still marked as needs-restart after the upgrade
that is not neccessary as when the service will be started after the package upgrade, it will be running the new version, so there is no need for another restart

but because of the lingering mark, the next (random) execution of systemctl reload-or-restart --marked will trigger a restart on the docker service which is surprising to say the least.

here are the steps you can use to reproduce the issue:

dnf install docker-ce-29.1.5-1.el9
systemctl enable --now docker
systemctl stop docker docker.socket
dnf install docker-ce-29.2.0-1.el9
systemctl start docker
systemctl show docker --property=Markers
systemctl reload-or-restart --marked

[Tyrael]

cc @robmry @thaJeztah

[thaJeztah]

Interesting; not sure if we do something special here that would cause this. I have some recollection of an issue we had in the past where a restart was needed, because containerd shims were otherwise kept in memory (and some upgrades had incompatibilities), but I have to think hard what (and where) the fix for that was; I found some old hack that's no longer in the codebase, but not sure if that was the one related to it;

• restart docker service if needed for rpm upgrade #20

At least, I don't see that code in the current code anymore though, and the only bit about restart we define looks to be on %postun (after uninstalling?)

docker-ce-packaging/rpm/SPECS/docker-ce.spec

Lines 128 to 129 in 03b1035

	%postun
	%systemd_postun_with_restart docker.service

I did a bit more searching and asking ChatGPT (as I'm lazy), it mentioned that systemctl reload-or-restart --marked may be set if the update / upgrade made modifications to any of the systemd units (or overrides), and this may be set by the systemd-update-helper used by RPM.

If this option would be set because it detected changes in a systemd unit that was installed; I went looking for the last time we made changes, but that looks to have been 9 months ago, for docker 28.5.x, so that wouldn't explain an update from 29.1 to 29.2 to trigger that; moby/moby@7263ae7

[thaJeztah]

Ah! I think I found what may be relevant; %postun is ALSO triggered when upgrading; because it's called when uninstalling the old version after installing the new version;
https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/?utm_source=chatgpt.com#ordering

11. %postun of old package

And looking at the RPM systemd-update-helper, I found these bits;
https://github.com/systemd/systemd/blob/8d96a18d90afd7f5a631a28fa4562cf2b8bcf4d8/src/rpm/systemd-update-helper.in#L45-L61

https://github.com/systemd/systemd/blob/8d96a18d90afd7f5a631a28fa4562cf2b8bcf4d8/src/rpm/macros.systemd.in#L84-L99

So, yes, I suspect this line I mentioned earlier may be responsible;

At least, I don't see that code in the current code anymore though, and the only bit about restart we define looks to be on %postun (after uninstalling?)

docker-ce-packaging/rpm/SPECS/docker-ce.spec

Lines 128 to 129 in 03b1035

	%postun
	%systemd_postun_with_restart docker.service

Unfortunately, it looks like the RPM helper doesn't distinguish between "was the service running" or "was it stopped", so not sure if there's a default solution for this. The Fedora RPM docs mention some parts about not requiring a restart;

Some services do not support being restarted (e.g. D-Bus and various storage daemons). If your service should not be restarted upon upgrade, but should be reloaded instead, then use the following %postun scriptlet instead of the one shown above:

%postun
%systemd_postun_with_reload apache-httpd.service

If your service should not be restarted or reloaded, then use the following %postun scriptlet instead:

%postun
%systemd_postun apache-httpd.service

The alternative would be some (hacky) way to detect if it was running, and use that, which would be a bit similar to what #20 did;

%pre
if [ $1 -ge 2 ]; then # upgrade (per Fedora/RPM conventions)
    if systemctl -q is-active docker.service; then
        touch /run/docker.was-active
    else
        rm -f /run/docker.was-active
    fi
fi

%postun
if [ $1 -ge 1 ]; then # upgrade, not erase
    if [ -e /run/docker.was-active ] && [ -x /usr/lib/systemd/systemd-update-helper ]; then
        /usr/lib/systemd/systemd-update-helper mark-restart-system-units docker.service || :
    fi
    rm -f /run/docker.was-active
fi

[Tyrael]

yeah, postun is also executed on upgrade (for cleaning up the previous version), I think using the macro maces your manifest clean/neat, but introduces this issue.

I think it is also a bit easy to miss from package maintainer perspective as the docs say:

       reload-or-restart PATTERN...
           Reload one or more units if they support it. If not, stop and
           then start them instead. If the units are not running yet,
           they will be started.

           This has a slightly differing functionality when used in
           combination with --marked, see below.

and when you check the doscs for --marked it still doesn't mention stopped services:

       --marked
           Only allowed with reload-or-restart. Enqueues restart jobs for
           all units that have the "needs-restart" mark, and reload jobs
           for units that have the "needs-reload" mark. When a unit
           marked for reload does not support reload, restart will be
           queued. Those properties can be set using set-property
           Markers=....

           Unless --no-block is used, systemctl will wait for the queued
           jobs to finish.

           Added in version 248.

so I think this cornercase is easy to miss.
I'm not expert with systemd macros, but I couldn't find any alternatives or options which would allow you to keep using a simple macro without also introducing this issue. So you would need to either have custom postun which doesn't mark needs-restart if the service should not be running or have a custom code after the %postun macro to remove the needs-restart Marker if the service won't be started after the upgrade.