containerd / docker.io LP: #1870514

[Bryce Harrington]

On Fri, Dec 04, 2020 at 10:41:15AM +0100, Christian Ehrhardt wrote:
> 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.

Ok thanks for investigating.  Too bad it's not available on the LTS's,
that'd be handy.

> > 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.

Great, I'm glad the legacy code can be omitted without issue.

> > 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.

On the other branch of this thread, #2 appears to be panning out (thanks
sergio and paride) and since would be self-contained to docker.io
can let us avoid the reboot tracking, so will be cleaner all around.
But I'll hang onto these notes in case we need to come back to proposal X.

Bryce