Bug#747909: WIP adt-virt-docker

Sat Apr 11 15:07:07 UTC 2015

Hey Mathieu,

thanks for working on this!

Note that I haven't used docker myself so far, so I'm still rather
ignorant about its properties; so please do correct me if I wrongly
moan about something below :-)

Mathieu Parent [2015-03-29 12:49 +0200]:
> new file mode 100755
> index 0000000..2442200
> --- /dev/null
> +++ b/virt-subproc/adt-virt-docker
> @@ -0,0 +1,141 @@
> +#!/usr/bin/python3
> +#
> +# adt-virt-docker is part of autopkgtest
> +# autopkgtest is a tool for testing Debian binary packages
> +#
> +# autopkgtest is Copyright (C) 2006-2015 Canonical Ltd.
> +#
> +# adt-virt-docker was derived from adt-virt-lxc and modified to suit Docker by
> +# Mathieu Parent <math.parent at gmail.com>
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> +#
> +# See the file CREDITS for a full list of credits information (often
> +# installed as /usr/share/doc/autopkgtest/CREDITS).
> +
> +import sys
> +import os
> +import subprocess
> +import tempfile
> +import shutil
> +import argparse
> +
> +try:
> +    our_base = os.environ['AUTOPKGTEST_BASE'] + '/lib'
> +except KeyError:
> +    our_base = '/usr/share/autopkgtest/python'
> +sys.path.insert(1, our_base)
> +
> +import VirtSubproc
> +import adtlog
> +
> +
> +capabilities = ['revert-full-system', 'root-on-testbed',
> +                'isolation-container']

You need 'revert' here as well; revert-full-system only specifies a
stricter behaviour of revert.

> +args = None
> +docker_container_id = None
> +shared_dir = None
> +
> +
> +def parse_args():
> +    global args
> +
> +    parser = argparse.ArgumentParser(fromfile_prefix_chars='@')
> +
> +    parser.add_argument('-d', '--debug', action='store_true',
> +                        help='Enable debugging output')
> +    parser.add_argument('image', help='Base image')
> +    parser.add_argument('dockerargs', nargs=argparse.REMAINDER,
> +                        help='Additional arguments to pass to docker run ')
> +    args = parser.parse_args()
> +    if args.debug:
> +        adtlog.verbosity = 2
> +
> +
> +def hook_open():
> +    global args, docker_container_id, shared_dir
> +
> +    if shared_dir is None:
> +        shared_dir = tempfile.mkdtemp(prefix='adt-virt-docker.shared.')
> +    else:
> +        # don't change the name between resets, to provide stable downtmp paths
> +        os.makedirs(shared_dir)
> +    os.chmod(shared_dir, 0o755)
> +    docker_container_id = VirtSubproc.check_exec(['docker', 'run', '--detach=true',

Is docker-run similar to lxc-attach in the guest, i. e. will that
always run the command as root?

Can you run adt-virt-docker as normal user, i. e. is it similar to LXC
user containers? Or always just as root? In the latter case this
should probably get a --sudo option like adt-virt-lxc, so that you
don't need to run the entire adt-run as root, just the virt-server?

> +        '--volume', "{0}:{0}".format(shared_dir)] + args.dockerargs + [args.image, 'sh', '-c', 'while true; do sleep 600; done'],

This shell sleep loop looks ugly -- isn't there a command to just
start a docker container and let it run?

Style issues: Please use '%' instead of format() for code consistency
(also in the other parts of the code below), and try to avoid such
overly long lines.

So this command always starts the guest with an ephemeral overlay, the
actual base image is never modified? OOI, what does that use to
implement this? (tmpfs and overlayfs or similar?)

> +        outp=True)
> +    adtlog.debug('hook_open: got docker container id %s' % docker_container_id)
> +    VirtSubproc.auxverb = [
> +        'docker', 'exec', docker_container_id
> +    ]

What does this to the environment of the command in the docker
container? a-v-lxc needs to do quite some extra effort to ensure that
the guest's /etc/environment, /etc/default/locale, and /etc/profile
are respected.

> +    (status, out, err) = VirtSubproc.execute_timeout(None, 0, VirtSubproc.auxverb + ['false'], stdout=subprocess.PIPE)
> +    if status == 0:
> +        # In Docker < 1.4, docker exec doesn't pass the return value
> +        # We use nsenter which pass return value
> +        # See https://github.com/duglin/docker/commit/90928eb1140fc0394e2
> +        adtlog.debug('hook_open: using nsenter workaround')
> +        docker_container_pid = VirtSubproc.check_exec(['docker', 'inspect',
> +            '--format', '{{.State.Pid}}', docker_container_id], outp=True)
> +        VirtSubproc.auxverb = [
> +            'sudo', 'nsenter', '--target', docker_container_pid, '--mount', '--uts', '--ipc', '--net',
> +            '--pid', '--root', '--wd' # '--user'
> +        ]

Eww :) Given that jessie has docker 1.5, this code path would be
exercised rather seldomly, and also couldn't easily be covered by
automatic tests. Would you mind if this would just spit out an error
message in the case that the exit code is not passed correctly?

> +    (status, out, err) = VirtSubproc.execute_timeout(None, 0, VirtSubproc.auxverb + ['apt-get', 'update'], stdout=subprocess.PIPE)

Please drop this. It's not the virt-server's business to call apt-get
update, it's a *very* expensive and also brittle operation (argh the
dreaded "hash sum mismatch"), and might not even always work. This gets
controlled by adt-run, possibly after modifying the guest's apt
sources with --setup-command or --apt-pocket.

Do you have a concept of "Unix user" in the docker world, or does
everything just run as root? In the former case, it might make sense
to adapt the suggested-normal-user= auto-detection logic from LXC?

> +def hook_downtmp(path):
> +    global capabilities, shared_dir
> +
> +    if shared_dir:
> +        d = os.path.join(shared_dir, 'downtmp')
> +        # these permissions are ugly, but otherwise we can't clean up files
> +        # written by the testbed when running as user
> +        VirtSubproc.check_exec(['mkdir', '-m', '777', d], downp=True)

Whether this is necessary depends on the answer from above whether you
can run docker as a user.

> +        capabilities.append('downtmp-host=' + d)
> +    else:
> +        d = VirtSubproc.downtmp_mktemp(path)
> +    return d
> +
> +
> +def hook_revert():
> +    hook_cleanup()
> +    hook_open()
> +
> +
> +def hook_cleanup():
> +    global capabilities, docker_container_id, shared_dir
> +
> +    VirtSubproc.downtmp_remove()
> +    capabilities = [c for c in capabilities if not c.startswith('downtmp-host')]
> +    if shared_dir:
> +        shutil.rmtree(shared_dir)
> +
> +    stop_outp = VirtSubproc.check_exec(['docker', 'stop', docker_container_id], outp=True)
> +    adtlog.debug('hook_cleanup: %s stopped' % stop_outp)

So I take it that "stop" is the counterpart to "run".

> +    rm_outp = VirtSubproc.check_exec(['docker', 'rm', docker_container_id], outp=True)
> +    adtlog.debug('hook_cleanup: %s removed' % rm_outp)

What does "rm" do? There is no "create" or similar counterpart in the
setup, and this looks suspiciously asymmetric?

> +def hook_forked_inchild():
> +    pass
> +
> +
> +def hook_capabilities():
> +    return capabilities
> +
> +
> +parse_args()
> +VirtSubproc.main()
> diff --git a/virt-subproc/adt-virt-docker.1 b/virt-subproc/adt-virt-docker.1
> new file mode 100644
> index 0000000..3391213
> --- /dev/null
> +++ b/virt-subproc/adt-virt-docker.1
> @@ -0,0 +1,81 @@
> +.TH adt\-virt-docker 1 2015 "Linux Programmer's Manual"
> +.SH NAME
> +adt\-virt\-docker \- autopkgtest virtualisation server using Docker
> +
> +.SH SYNOPSYS
> +.B adt\-virt\-docker
> +.RI [ options ]
> +.I docker\-image
> +.RI [ "-- extra docker-run args..." ]
> +
> +.SH DESCRIPTION
> +.B adt-virt-docker
> +provides an autopkgtest virtualisation server using Docker. It adapts the
> +functionality provided by the
> +.BR docker
> +command line for use by autopkgtest.
> +
> +Normally
> +.B adt-virt-docker
> +will be invoked by
> +.BR adt-run .
> +
> +.SH REQUIREMENTS
> +.B adt-virt-docker
> +needs the docker client (in the docker.io package).
> +
> +.SH OPTIONS
> +
> +.TP
> +.BR \-d " | " \-\-debug
> +Enables debugging output.
> +
> +.PP
> +You can pass additional options to Docker: Anything after a
> +.B --
> +gets passed verbatim to \fBdocker-run\fR(1).
> +
> +.SH INPUT, OUTPUT AND EXIT STATUS
> +The behaviour of
> +.B adt-virt-docker
> +is as described by the AutomatedTesting virtualisation regime
> +specification.
> +
> +.SH NOTES
> +
> +Read \fB/usr/share/doc/docker.io/README.Debian\fR first to communicate to the
> +daemon.

What does that sentence mean? As an user of autopkgtest I don't know
what "communicate to the daemon" means or why I'd need to care about
it.

> +
> +\fBadt-virt-docker\fR try to use \fBdocker-exec\fR(1), but fall back to
> +\fBsudo\fR(1) \fBnsenter\fR(1) on Docker < 1.4 which doesn't return status.

See above.

> +\fBadt-virt-docker\fR does run \fBapt-get update\fR at the start of a package
> +build, as most images needs it.

See above, please don't do this.

> +.SH EXAMPLE
> +
> +You can use "(Semi) Official Debian base images" like \fBdebian:jessie\fR,
> +or "Official Ubuntu base images", like \fBubuntu:trusty\fR. 
> +
> +Run tests against \fIhello_2.8\-4.dsc\fR, using the Docker image \fIdebian:jessie\fR:
> +
> +.RS
> +.EX
> +adt-run \fIhello_2.8\-4.dsc\fR --- adt-virt-docker \fIdebian:jessie\fR

Ah nice, so you don't even need to create a docker image locally, but
this will download the base image from the network and run it right
away? Will this be cached, so that in the next run it doesn't have to
download everything again? (This is quite important for larger scale
deployment).

Can you also give an example how to create (and possibly modify) a
custom docker image and use that with a-v-docker?

> +.EE
> +.RE
> +
> +.SH SEE ALSO
> +\fBadt\-run\fR(1),
> +\fBdocker\-run\fR(1),
> +\fB/usr/share/doc/autopkgtest/\fR.
> +
> +.SH AUTHORS AND COPYRIGHT
> +.B adt-virt-docker
> +was written by Mathieu Parent <math.parent at gmail.com>.
> +
> +This manpage is part of autopkgtest, a tool for testing Debian binary
> +packages.  autopkgtest is Copyright (C) 2006-2015 Canonical Ltd and others.
> +
> +See \fB/usr/share/doc/autopkgtest/CREDITS\fR for the list of
> +contributors and full copying conditions.
> -- 
> 2.1.4

Finally, can you please add a set of test cases similar to LxcRunner
in tests/adt-run, to make sure this keeps working in the future? This
is a strict requirement of mine in order to land this in the
autopkgtest project. This will most probably also uncover a few corner
cases (environment handling, cleanup between reverts, running tests as
user, etc.).

Thanks,

Martin

-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)