upgrade to 17.03.2-ce in 16.04+

[Michael Hudson-Doyle]

[Impact]
docker 17.03 came out over six months ago so we should clearly be at least that up to date...

[Test Case]
The autopkgtests per https://wiki.ubuntu.com/DockerUpdates. They should run on arm64 and s390x now in the scalingstack infrastructure but if they don't I'll run them by hand.

[Regression potential]
https://wiki.ubuntu.com/DockerUpdates again.

[Brian Murray (brian-murray)]

Hello Michael, or anyone else affected,

Accepted docker.io into artful-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/docker.io/17.03.2-0ubuntu1~17.10.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-artful to verification-done-artful. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-artful. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Brian Murray (brian-murray)]

Hello Michael, or anyone else affected,

Accepted docker.io into zesty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/docker.io/17.03.2-0ubuntu1~17.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-zesty to verification-done-zesty. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-zesty. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Brian Murray (brian-murray)]

Hello Michael, or anyone else affected,

Accepted docker.io into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/docker.io/17.03.2-0ubuntu1~16.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-xenial to verification-done-xenial. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-xenial. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[George Kraft (cynerva)]

This fails for me on xenial after upgrading from Docker v1.13.1 to 17.03.2-ce from xenial-proposed. Here's what I did:

1. Deploy a Canonical Distribution of Kubernetes cluster (which deploys worker instances with Xenial and gets Docker v1.13.1 via `apt install docker.io`)
2. On all worker units: add xenial-proposed line to sources.list.d, then `apt update && apt install docker.io`
3. Run `docker --version` on the worker units to confirm 17.03.2-ce was installed.

At this point, I observed that Kubernetes could no longer create containers, with errors coming from Docker along these lines:

Create container failed with error: invalid header field value \"oci runtime error: container_linux.go:247: starting container process caused \\\"process_linux.go:334: running prestart hook 0 caused \\\\\\\"fork/exec /usr/bin/dockerd (deleted): no such file or directory\\\\\\\"\\\"\\n\"

After observing this failure repeating for several minutes, I did the following:

4. On all worker units: `service docker restart`

This fixed the problem: the fork/exec errors from Docker stopped, and Kubernetes was able to create containers again.

[George Kraft (cynerva)]

Attached docker service logs where the fork/exec error can be seen. Some important times to consider:

~14:11:58 - docker v1.13.1 service starts
~14:36:00 - `apt install docker.io` upgrades docker to 17.03.2
~14:39:26 - Kubernetes starts trying to create new containers, fails
~14:52:18 - `service docker restart` - service restarts, fork/exec errors stop

[Łukasz Zemczak (sil2100)]

I see a re-upload of docker.io for xenial (17.03.2-0ubuntu3~16.04.1) probably to address the mentioned issues. But I don't see the same upload for artful - does this mean that artful doesn't require a re-release? I would prefer forward-series to also have the same fixes, if applicable.

[Michael Hudson-Doyle (mwhudson)]

I should have commented here before, I'm waiting for slangasek or infinity to review this one. There should be an artful upload yes but that can wait for a bit.

[Steve Langasek (vorlon)]

Comments on the debdiff:

diff -Nru docker.io-17.03.2/debian/docker.io.config docker.io-17.03.2/debian/docker.io.config
--- docker.io-17.03.2/debian/docker.io.config 1970-01-01 00:00:00.000000000 +0000
+++ docker.io-17.03.2/debian/docker.io.config 2018-03-23 02:26:26.000000000 +0000
@@ -0,0 +1,11 @@
+#!/bin/sh
+set -e
+
+. /usr/share/debconf/confmodule
+
+db_version 2.0
+
+db_input high docker.io/restart || :
+db_go
+
+db_stop

This config script has the properties that:
 - the first time the question is asked, it will (probably) be shown to the user.
 - on subsequent invocations of the package's maintainer scripts, the question will not be shown.

This means that, once the user has answered the question, this behavior will be sticky; every subsequent update of the package will run the risk of either leaving the tools unable to talk to the daemon, or unexpectedly stopping (and failing to restart) containers, resulting in a service interruption.

The reason this is being done as a debconf question is because neither option is good. And *since* we are forced to ask the user to pick their poison, I consider it insufficient to ask this question only once. The question should by default be re-asked, at each package upgrade, so that the user has an opportunity to make the decision that is correct for them at that moment.

The pattern for this is:
 - no .config script
 - call 'db_fset docker.io/restart seen false' in postinst
 - call 'db_input high docker.io/restart' in postinst only if a restart may be required

It would be ok to give the user an additional option to "always restart" (so, a tristate question instead of a boolean); but at minimum users must be able to select the behavior locally at the point of any given upgrade.

In addition, the current default for the debconf question is "true"; so by default, which includes on non-interactive upgrades (unattended upgrades or the like), the behavior the user gets here is that the service is restarted, stopping any un-supervised containers. If the docker.io package is not able to detect whether all running containers will be automatically restarted, then if this is to be a supported use case at all, the default should be to not restart: because it is less disruptive to have the docker service become unavailable, than to have the docker containers become unavailable.

I am therefore rejecting the upload of 17.03.2-0ubuntu3~16.04.1.

Additional observations about the package, which are not blockers for an SRU:

+shouldStartRestart=
+if [ "$1" != 'reconfigure' ] && [ -z "${DEBCONF_RECONFIGURE:-}" ]; then
+ # (if we're just doing "dpkg-reconfigure", there's no reason to actually [re]start Docker)
+ shouldStartRestart=1
+fi

"reconfigure" is not a valid argument to a postinst script. (And dpkg-reconfigure does not invoke it this way - it invokes it as 'postinst configure'.) So the first part of this test is always true and can be omitted.

+if \
+ [ "$RET" = 'true' ] \
+ && [ -n "$2" ] && [ "$1" = 'configure' ] \
+ && [ -n "$shouldStartRestart" ] \
+ && [ -x /etc/init.d/docker ] \
+ && invoke-rc.d --disclose-deny docker status > /dev/null 2>&1 \

The la...

[Steve Langasek (vorlon)]

An upload of docker.io to xenial-proposed has been rejected from the upload queue for the following reason: "See https://bugs.launchpad.net/ubuntu/+source/docker.io/+bug/1732368/comments/8".

[Tianon Gravi (tianon)]

> The reason this is being done as a debconf question is because neither option is good. And *since* we are forced to ask the user to pick their poison, I consider it insufficient to ask this question only once. The question should by default be re-asked, at each package upgrade, so that the user has an opportunity to make the decision that is correct for them at that moment.

> In addition, the current default for the debconf question is "true"; so by default, which includes on non-interactive upgrades (unattended upgrades or the like), the behavior the user gets here is that the service is restarted, stopping any un-supervised containers. If the docker.io package is not able to detect whether all running containers will be automatically restarted, then if this is to be a supported use case at all, the default should be to not restart: because it is less disruptive to have the docker service become unavailable, than to have the docker containers become unavailable.

Fair enough, will update Git and poke Michael when it's ready (thanks for noting the exact pattern we ought to be using to accomplish this). :)

> "reconfigure" is not a valid argument to a postinst script. (And dpkg-reconfigure does not invoke it this way - it invokes it as 'postinst configure'.) So the first part of this test is always true and can be omitted.

I included that because I read on the lists that it was intended to be supported _eventually_, but I guess that was long enough ago (and the transition wouldn't be fun to implement across the archive) that it's probably safe to ignore that ever happened; fair enough. O:)

> And the check for status is not consistent with expected behavior of packages: upgrading the package should normally start the service even if it was currently not running, unless the administrator has explicitly disabled it through policy-rc.d.

So, this block of code is _only_ responsible for performing the daemon restart, hence why this bit was included. Starting the daemon happens in the following block.

To put it another way, if Docker isn't running, there's not really a good reason to even prompt the user whether we're OK to restart it -- we should just fire it up.

Would it make more sense to have this block just stop the daemon instead and let the following block start it up?

> This code includes a check for "$shouldStartRestart"; but this is the code to start the daemon on package initial installation, and the debconf question doesn't ask about starting, only about restarting. So I think this check shouldn't be there.

The "$shouldStartRestart" variable basically amounts to "Are we running this script because of package install or because of 'dpkg-reconfigure'?" -- if the user is simply running "dpkg-reconfigure", I don't think it's appropriate or necessary for us to fire up the Docker daemon (or bother restarting it -- we only care about doing that when upgrading or installing the package).

Since we're going to ditch the check for `[ "$1" != 'reconfigure' ]`, we can just use "$DEBCONF_RECONFIGURE" which will make this more clear.

So to summarize, the current code, as written, will restart the daemon based on the user's input, but only d...

[Łukasz Zemczak (sil2100)]

Is this being tested? The package has been waiting in the -proposed pockets for 219 days now. Please perform verification in the nearest week or I will be removing it from -proposed as per SRU policy.

[Michael Hudson-Doyle (mwhudson)]

There is a new upload in UNAPPROVED that is waiting for review from Steve.
This version will not migrate so may as well remove it.

On Sat, 30 Jun 2018 at 03:32, Łukasz Zemczak <email address hidden>
wrote:

> Is this being tested? The package has been waiting in the -proposed
> pockets for 219 days now. Please perform verification in the nearest
> week or I will be removing it from -proposed as per SRU policy.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1732368
>
> Title:
> upgrade to 17.03.2-ce in 16.04+
>
> To manage notifications about this bug go to:
>
> https://bugs.launchpad.net/ubuntu/+source/docker.io/+bug/1732368/+subscriptions
>

[Steve Langasek (vorlon)]

zesty is now EOL.

[Steve Langasek (vorlon)]

Hello Michael, or anyone else affected,

Accepted docker.io into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/docker.io/17.03.2-0ubuntu2~16.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-xenial to verification-done-xenial. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-xenial. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Michael Hudson-Doyle (mwhudson)]

Per https://wiki.ubuntu.com/DockerUpdates, the passing autopkgtests are considered sufficient QA for docker updates and they all passed.

[Launchpad Janitor (janitor)]

This bug was fixed in the package docker.io - 17.03.2-0ubuntu2~16.04.1

---------------
docker.io (17.03.2-0ubuntu2~16.04.1) xenial; urgency=medium

  [ Michael Hudson-Doyle ]
  * Backport to Xenial. (LP: #1732368)
  * Install the service file with .install again, fixing service activation
    on install.
  * Use golang.org/x/net/context instead of stdlib context to enable building
    with Go 1.6.
  * Adjust debconf template to match change in behavior.

  [ Tianon Gravi ]
  * Adjust debconf prompt:
    - ask during _every_ upgrade
    - default to not restarting

 -- Michael Hudson-Doyle <email address hidden> Fri, 23 Mar 2018 15:34:08 +1300

[Robie Basak (racb)]

The verification of the Stable Release Update for docker.io has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.