Bug#747909: WIP adt-virt-docker

[Inaki Malerba]

Hi Martin

I just brought to salsa [1] the Mathieu's patch with Chris fixes.

I'll do my best to try answering the questions you did.


1_ https://salsa.debian.org/ci-team/autopkgtest/merge_requests/3

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

Fixed on new patch.

>> +    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?

Fixed on new patch.

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

This is fixed. The while+sleep command was turned into an sleep infinity.
While it doesn't look good either, its something necessary to keep the
container running on the background and then executing things in it via
`docker exec`.

Also, the style mismatch was fixed.

>
>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?)

Exactly.
Base images are not modified when running them. Layers are added on top,
but, if not tagged, the modifications are dropped on exit.

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

Docker uses all the locales and configs of the guest image. No problem
on that.

>> +    (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?
>

This is not in the patch anymore.

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

This is not in the patch anymore.
Anyway, apt-get update via setup-command is needed, because most (not to
say all) images come without sources cache.

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

By default, it runs everything as root.

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

Bug#747909: WIP adt-virt-docker

[Mathieu Parent]

2015-08-28 21:58 GMT+02:00 Chris Kuehl :
> Howdy there Mathieu and Martin,

Hello Chris and Martin,

> Thanks for your work on this! This is pretty cool.
>
> Out of curiosity, has any progress been made on this patch/bug?

Nope

> If you'd like some help, I'd be happy to lend a hand to addressing the
> comments in Martin's review -- would really like to use this feature
> myself :-)

Please go ahead, this feature is not my top priority currently (but I
will happily test your patch).

Regards

-- 
Mathieu

Bug#747909: WIP adt-virt-docker

[Chris Kuehl]

Howdy there Mathieu and Martin,

Thanks for your work on this! This is pretty cool.

Out of curiosity, has any progress been made on this patch/bug?

If you'd like some help, I'd be happy to lend a hand to addressing the
comments in Martin's review -- would really like to use this feature
myself :-)

Many thanks!
Chris

Bug#747909: WIP adt-virt-docker

[Martin Pitt]

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 000..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.par...@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)
 +

Bug#747909: WIP adt-virt-docker

[Mathieu Parent]

2015-03-28 11:28 GMT+01:00 Mathieu Parent math.par...@gmail.com:
 Hello,

 I have attached the current version.

 Martin, can you review it? The manpage is missing.

Here's an improved version with th manpage.

Regards
-- 
Mathieu
From e5b3a94f7808cfd180a0e57007d36d27633a90a5 Mon Sep 17 00:00:00 2001
From: Mathieu Parent math.par...@gmail.com
Date: Sat, 28 Mar 2015 11:25:32 +0100
Subject: [PATCH] Add adt-virt-docker (Closes: #747909)

---
 virt-subproc/adt-virt-docker   | 141 +
 virt-subproc/adt-virt-docker.1 |  81 +++
 2 files changed, 222 insertions(+)
 create mode 100755 virt-subproc/adt-virt-docker
 create mode 100644 virt-subproc/adt-virt-docker.1

diff --git a/virt-subproc/adt-virt-docker b/virt-subproc/adt-virt-docker
new file mode 100755
index 000..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.par...@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']
+
+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',
+'--volume', {0}:{0}.format(shared_dir)] + args.dockerargs + [args.image, 'sh', '-c', 'while true; do sleep 600; done'],
+outp=True)
+adtlog.debug('hook_open: got docker container id %s' % docker_container_id)
+VirtSubproc.auxverb = [
+'docker', 'exec', docker_container_id
+]
+(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'
+]
+(status, out, err) = VirtSubproc.execute_timeout(None, 0, VirtSubproc.auxverb + ['apt-get', 'update'], stdout=subprocess.PIPE)
+
+
+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)
+capabilities.append('downtmp-host=' + 

Bug#747909: WIP adt-virt-docker

[Mathieu Parent]

Hello,

I have attached the current version.

Martin, can you review it? The manpage is missing.

Cheers,

2015-03-24 21:16 GMT+01:00 Mathieu Parent math.par...@gmail.com:
 Hello,

 I've started to implement adt-virt-docker. I will propose a patch within a 
 week.

 Regards

 --
 Mathieu



-- 
Mathieu
From 58f01839cd7452c971bc21187ee83ef2562ab33c Mon Sep 17 00:00:00 2001
From: Mathieu Parent math.par...@gmail.com
Date: Sat, 28 Mar 2015 11:25:32 +0100
Subject: [PATCH] Add adt-virt-docker

---
 virt-subproc/adt-virt-docker | 141 +++
 1 file changed, 141 insertions(+)
 create mode 100755 virt-subproc/adt-virt-docker

diff --git a/virt-subproc/adt-virt-docker b/virt-subproc/adt-virt-docker
new file mode 100755
index 000..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.par...@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']
+
+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',
+'--volume', {0}:{0}.format(shared_dir)] + args.dockerargs + [args.image, 'sh', '-c', 'while true; do sleep 600; done'],
+outp=True)
+adtlog.debug('hook_open: got docker container id %s' % docker_container_id)
+VirtSubproc.auxverb = [
+'docker', 'exec', docker_container_id
+]
+(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'
+]
+(status, out, err) = VirtSubproc.execute_timeout(None, 0, VirtSubproc.auxverb + ['apt-get', 'update'], stdout=subprocess.PIPE)
+
+
+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)
+capabilities.append('downtmp-host=' + d)
+else:
+d = VirtSubproc.downtmp_mktemp(path)

Bug#747909: WIP adt-virt-docker

[Mathieu Parent]

Hello,

I've started to implement adt-virt-docker. I will propose a patch within a week.

Regards

-- 
Mathieu


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org