containerd / docker.io LP: #1870514

Fri Dec 4 09:41:15 UTC 2020

On Fri, Dec 4, 2020 at 12:49 AM Bryce Harrington
<bryce.harrington at canonical.com> wrote:
>
> I'm about to EOD, so in hopes of keeping activity on the bug cross
> timezones Rick suggested identifying some specific tasks that'd help

Awesome - this is exactly what I meant on standup and missed the other day.
Thanks!

> move the football down the field on Proposal X.
>
> Robie:
>
> The maintscripts I implemented are in the following patch:
>
>   https://git.launchpad.net/~bryce/ubuntu/+source/containerd/commit/?id=604b72a3543a4e25c8acde692a7ecd6874bd699c
>
> Obviously this could benefit from refactoring since I was copy and
> pasting chunks of logic, but the specific question I want your input on
> is if you are aware of any dh_* commands or deb-systemd-* commands that
> would either condense the code or make it more robust.
>
> One other more philosophical design question you could help answer.  The
> debhelper generated stuff provided a chunk of code for handling service
> restart via invoke-rc.d.  The question is are there any scenarios where
> we care about this legacy init system?  If there is, then we may need
> further logic beyond what I've included; if there isn't, then can we
> omit the update-rc.d / invoke-rc.d logic entirely?
>
>
> Christian:
>
> In the above script you can see I am registering reboot-required in
> certain circumstances.  In today's call you mentioned about the
> possibility of instead of rebooting just requesting a service restart.
> I was not able to find guidance on exactly how to do that, so wonder if
> you know more?  Can you investigate if that is a viable approach we can
> do for X/B/F/G/H, or if it is more of a future possibility.

I've checked the communication about it. The service-granularity
functionality is in src:needrestart.
AFAIK Matt/Xnox consider it as an optional item for 21.04. It needs
cleanup, MIR, ...
So it would be a future possibility, but not important for our current case.

> While you're at it, you can see I included logic for using the older
> update-notifier interface.  Can you check if that is needed for Xenial?
> If it isn't we can perhaps drop that.

AFAICS the "new code" is the one using /run/reboot-required directly.
The "old" according to your comments would be via
"/usr/share/update-notifier/notify-reboot-required"

The version we have in Xenial already has the "new" interface.
And the call to "old" is just a wrapper setting the same files/content as "new".

I have downgraded a Xenial system to the level it has in -release and
checked these scripts.
Even there the "new" interface is present. And also things like motd
probe from the path that you use.

Note: the "old" interface also still is present on the most recent
releases - is there any place that deprecates the old call?

---

But - there is one difference those two calls that you have do.

Calling notify-reboot-required does not just "touch"
/run/reboot-required + append-pkg to /run/reboot-required.pkgs"
But instead "set text to present + append-pkg to /run/reboot-required.pkgs"
Example:
echo "*** $(eval_gettext "System restart required") ***" >
/var/run/reboot-required
echo "$DPKG_MAINTSCRIPT_PACKAGE" >> /var/run/reboot-required.pkg

This text is what is presented on e.g. motd login.
So only touching it might trigger the update-notifier app, but be
invisible to ssh-motd notifications.

Also calling /usr/share/update-notifier/notify-reboot-required would
set "containerd" as the PKG causing it as it is (and was) based on
$DPKG_MAINTSCRIPT_PACKAGE.
Your code sets this to docker.io instead.

---

The above shows that it isn't a matter of old-vs-new they will both
work in all still maintained releases of Ubuntu.

IMHO we have two choices from here:
a) we are ok that containerd is listed in reboot-required.pkgs
    - in that case please only keep the call to notify-reboot-required
b) we want/need docker.io to be listed in reboot-required.pkgs
    - in that case you can drop the call to notify-reboot-required and
keep the rest
    - You'd need to change your "touch" to also echo eval_gettext
"System restart required to /var/run/reboot-required

Since - as far as I found - all that reboot-required.pkg is needed for
is https://bugs.launchpad.net/landscape/+bug/538253 which is defined
as
"data/notify-reboot-required: write out what scripts requested the
reboot in /var/run/reboot-required.pkgs" I think we are ok to pick (a)
which also means som fewer lines of new code.

>
> Sergio:
>
> Thanks again for demoing the docker commands you were using to test.
> I've put them in the [Test Case] section of the doc.  Unfortunately I
> get an error running the `docker run` command gives me a permission
> denied error for OCI runtime create.  Obviously my non-docker-fu is
> making me do something wrong.  Can you look at the test case and adjust
> it to give a more accurately paint-by-number set of docker commands?
>
>
> ------------------------------------------------------------------------
>
> Sergio demoed to me what he and Paride discovered while examining
> docker.io's prerm file (/var/lib/dpkg/info/docker.io.prerm).  Debhelper
> automatically adds a command to stop the docker service
> unconditionally:
>
>     # Automatically added by dh_systemd_start/13.2.1ubuntu1
>     if [ -d /run/systemd/system ]; then
>             deb-systemd-invoke stop 'docker.service' 'docker.socket' >/dev/null || true
>     fi
>     # End automatically added section
>
> This means three things.  1) Proposal A can be crossed off, 2) we might
> potentially be able to address the problem in docker.io's maintscripts
> better than in containerd's maintscripts by replacing this debhelper
> logic with some conditionals like done for Proposal X, and 3) since the
> user's installed docker.io's prerm gets run before any new package's
> maintscripts, this means all our proposals suffer the same problem that
> all of them will result in docker.service getting this 'stop' command at
> least one time.
>
> This third item I'm not sure how we're going to get around, or if we
> even can.  There was an idea expressed early on to introduce a new
> package with the simple purpose of hackily shimming out this
> `deb-systemd-invoke stop` call from docker's prerm before doing anything
> at all with the docker.io or containerd packages.  Should we revisit
> this idea?  Or should we simply accept that we're going to trigger the
> bug for all users one more time in order to get the long term fix in
> place?
>
> Bryce
>
> --
> ubuntu-server mailing list
> ubuntu-server at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/ubuntu-server
> More info: https://wiki.ubuntu.com/ServerTeam

-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd