On 03/23/18 15:03, Holger Levsen wrote: [snip] > > I've done some review... > >> *--schroot*='SCHROOT-NAME':: >> - Use schroot session named SCHROOT-NAME for the chroot, instead of >> building a new one with debootstrap. >> + Use schroot session named SCHROOT-NAME for the testing environment. > > why drop the second half here?
Not really sure, maybe I thought it was more clear. But now you mention it, I
prefer to keep it hehe. I've re-added it
>
>> parser.add_option("--schroot", metavar="SCHROOT-NAME", action="store",
>> - help="Use schroot session named SCHROOT-NAME for the
>> chroot, instead of building " +
>> - "a new one with debootstrap.")
>> + help="Use schroot session named SCHROOT-NAME for the
>> testing environment.")
>
done
> same
>
>> *-k*, *--keep-tmpdir*::
>> - Don't remove the temporary directory for the chroot when the program ends.
>> + Depending on which option is passed, it keeps the environment used for
>> testing after the program ends::
>> + * By default it doesn't remove the temporary directory for the chroot
>> + * if --schroot is used, the schroot session is not terminated
>> + * or if --docker-image is used, the container created is not destroyed.
>
> I think I prefer:
>
> Depending on which option is passed, it keeps the environment used for
> testing after the program ends::
> * By default it doesn't remove the temporary directory for the chroot,
> * or if --schroot is used, the schroot session is not terminated,
> * or if --docker-image is used, the container created is not destroyed.
Done
> (twice "or if" at the beginning and always commas at the end)
>
>> - help="Don't remove the temporary directory for the " +
>> - "chroot when the program ends.")
>> + help="It keeps the environment used for testing after
>> "
>> + "the program ends.")
>
> -> "Keep the environment used for testing after the programm ends."
Done
>> --- a/piuparts.py
>> +++ b/piuparts.py
> [...]
>> +import json
>
> doesn't this need a new dependency?
Noup, it's part of the stdlib
libpython3.6-stdlib:amd64: /usr/lib/python3.6/json/__init__.py
libpython2.7-stdlib:amd64: /usr/lib/python2.7/json/__init__.py
>
>> def create_resolv_conf(self):
>> + if settings.docker_image:
>> + # Do nothing, docker already takes care of this
>> + return
>
> and
>
>> def terminate_running_processes(self):
>> + if settings.docker_image:
>> + # docker takes care of this
>> + return
>
> here I wonder: is it really cleaner to return if docker is used or
> wouldnt it be better to not call it in the first place?
Maybe, I'm not totally sure, but if you feel it's better, I prefer the way you
feel more comfortable :). After all, it's your little creature hehe
>
> The rest looks fine to me and I'll happily merge once you fixed up those
> little things. Thanks again!
Great! Please take a look at it again. Also I've made little fixes that Andreas
Beckman and Iñaki Malerva point me on the PR ¯\_(ツ)_/¯. Basically add docker.io
as suggested package, edit debian/changelog and fix a typo in doc.
Thanks,
--
TiN
From c40f18d64bf6c1149b3538cd01b826cd50b80cc0 Mon Sep 17 00:00:00 2001
From: Agustin Henze <t...@aayy.com.ar>
Date: Wed, 21 Mar 2018 16:54:50 -0300
Subject: [PATCH 1/6] Add docker support, new param is introduced
`--docker-image`
e.g. piuparts --docker-image debian:unstable package.deb
It only supports overlay2 for now and it uses the `MergedDir` layer
where piuparts can access, add, edit and remove files easily.
Signed-off-by: Agustin Henze <t...@aayy.com.ar>
---
piuparts.py | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 60 insertions(+), 9 deletions(-)
diff --git a/piuparts.py b/piuparts.py
index 3daba797..fc0530d3 100644
--- a/piuparts.py
+++ b/piuparts.py
@@ -47,6 +47,7 @@ import os
import tarfile
import stat
import re
+import json
import pickle
import subprocess
import traceback
@@ -191,6 +192,7 @@ class Settings:
self.skip_minimize = True
self.minimize = False
self.debfoster_options = None
+ self.docker_image = None
# tests and checks
self.no_install_purge_test = False
self.no_upgrade_test = False
@@ -769,7 +771,7 @@ class Chroot:
def create(self, temp_tgz=None):
"""Create a chroot according to user's wishes."""
self.panic_handler_id = do_on_panic(self.remove)
- if not settings.schroot:
+ if not settings.schroot and not settings.docker_image:
self.create_temp_dir()
if temp_tgz:
@@ -782,10 +784,12 @@ class Chroot:
self.setup_from_dir(settings.existing_chroot)
elif settings.schroot:
self.setup_from_schroot(settings.schroot)
+ elif settings.docker_image:
+ self.setup_from_docker(settings.docker_image)
else:
self.setup_minimal_chroot()
- if not settings.schroot:
+ if not settings.schroot and not settings.docker_image:
self.mount_proc()
self.configure_chroot()
@@ -807,7 +811,7 @@ class Chroot:
self.run_scripts("post_chroot_unpack")
self.run(["apt-get", "update"])
- if settings.basetgz or settings.schroot or settings.existing_chroot:
+ if settings.basetgz or settings.docker_image or settings.schroot or settings.existing_chroot:
self.run(["apt-get", "-yf", "dist-upgrade"])
self.minimize()
self.remember_available_md5()
@@ -823,7 +827,8 @@ class Chroot:
def remove(self):
"""Remove a chroot and all its contents."""
if not settings.keep_tmpdir and os.path.exists(self.name):
- self.terminate_running_processes()
+ if not settings.docker_image:
+ self.terminate_running_processes()
self.unmount_all()
if settings.lvm_volume:
logging.debug('Unmounting and removing LVM snapshot %s' % self.lvm_snapshot_name)
@@ -832,7 +837,10 @@ class Chroot:
if settings.schroot:
logging.debug("Terminate schroot session '%s'" % self.name)
run(['schroot', '--end-session', '--chroot', "session:" + self.schroot_session])
- if not settings.schroot:
+ if settings.docker_image:
+ logging.debug("Destroy docker container '%s'" % self.docker_container)
+ run(['docker', 'rm', '-f', self.docker_container])
+ if not settings.schroot and not settings.docker_image:
run(['rm', '-rf', '--one-file-system', self.name])
if os.path.exists(self.name):
create_file(os.path.join(self.name, ".piuparts.tmpdir"), "removal failed")
@@ -840,6 +848,8 @@ class Chroot:
elif settings.keep_tmpdir:
if settings.schroot:
logging.debug("Keeping schroot session %s at %s" % (self.schroot_session, self.name))
+ elif settings.docker_image:
+ logging.debug("Keeping container %s" % self.docker_container)
else:
logging.debug("Keeping directory tree at %s" % self.name)
dont_do_on_panic(self.panic_handler_id)
@@ -892,6 +902,25 @@ class Chroot:
self.name = output.strip()
logging.info("New schroot session in '%s'" % self.name)
+ @staticmethod
+ def check_if_docker_storage_driver_is_supported():
+ ret_code, output = run(['docker', 'info'])
+ if 'overlay2' not in output:
+ logging.error('Only overlay2 storage driver is supported')
+ panic()
+
+ def setup_from_docker(self, docker_image):
+ self.check_if_docker_storage_driver_is_supported()
+ ret_code, output = run(['docker', 'run', '-d', '-it', docker_image, 'bash'])
+ if ret_code != 0:
+ logging.error("Couldn't start the container from '%s'" % docker_image)
+ panic()
+ self.docker_container = output.strip()
+ ret_code, output = run(['docker', 'inspect', self.docker_container])
+ container_data = json.loads(output)[0]
+ self.name = container_data['GraphDriver']['Data']['MergedDir']
+ logging.info("New container created '%s'" % self.docker_container)
+
def setup_from_lvm(self, lvm_volume):
"""Create a chroot by creating an LVM snapshot."""
self.lvm_base = os.path.dirname(lvm_volume)
@@ -938,6 +967,12 @@ class Chroot:
["schroot", "--preserve-environment", "--run-session", "--chroot", "session:" +
self.schroot_session, "--directory", "/", "-u", "root", "--"] + prefix + command,
ignore_errors=ignore_errors, timeout=settings.max_command_runtime)
+ elif settings.docker_image:
+ return run(
+ ['docker', 'exec', self.docker_container,] + prefix + command,
+ ignore_errors=ignore_errors,
+ timeout=settings.max_command_runtime,
+ )
else:
return run(["chroot", self.name] + prefix + command,
ignore_errors=ignore_errors, timeout=settings.max_command_runtime)
@@ -1094,7 +1129,9 @@ class Chroot:
self.create_apt_conf()
self.create_dpkg_conf()
self.create_policy_rc_d()
- self.create_resolv_conf()
+ # Docker already takes care of this
+ if not settings.docker_image:
+ self.create_resolv_conf()
for bindmount in settings.bindmounts:
self.mount(bindmount, bindmount, opts="bind")
@@ -1379,6 +1416,9 @@ class Chroot:
'broken-symlink',
]
ignored_tags = []
+ if not os.path.exists(self.name + '/dev/null'):
+ device = os.makedev(1, 3)
+ os.mknod(self.name + '/dev/null', 0o666, device)
(status, output) = run(["adequate", "--root", self.name] + packages, ignore_errors=True)
for tag in ignored_tags:
# ignore some tags
@@ -1624,15 +1664,20 @@ class Chroot:
def check_for_no_processes(self, fail=None):
"""Check there are no processes running inside the chroot."""
- (status, output) = run(["lsof", "-w", "+D", self.name], ignore_errors=True)
- count = len(output.split("\n")) - 1
+ if settings.docker_image:
+ (status, output) = run(["docker", "top", self.docker_container])
+ count = len(output.strip().split("\n")) - 2 # header + bash launched on container creation
+ else:
+ (status, output) = run(["lsof", "-w", "+D", self.name], ignore_errors=True)
+ count = len(output.split("\n")) - 1
if count > 0:
if fail is None:
fail = not settings.allow_database
logging.error("%s: Processes are running inside chroot:\n%s" %
("FAIL" if fail else "WARN", indent_string(output)))
if fail:
- self.terminate_running_processes()
+ if not settings.docker_image:
+ self.terminate_running_processes()
panic()
def terminate_running_processes(self):
@@ -2726,6 +2771,11 @@ def parse_command_line():
help="Use schroot session named SCHROOT-NAME for the chroot, instead of building " +
"a new one with debootstrap.")
+ parser.add_option("--docker-image", metavar="DOCKER-IMAGE", action="store",
+ help="Use a container created from the docker image "
+ "DOCKER-IMAGE for the testing environment, instead of "
+ "building a new one with debootstrap.")
+
parser.add_option("-m", "--mirror", action="append", metavar="URL",
default=[],
help="Which Debian mirror to use.")
@@ -2948,6 +2998,7 @@ def parse_command_line():
if settings.minimize:
settings.skip_minimize = False
settings.debfoster_options = opts.debfoster_options.split()
+ settings.docker_image = opts.docker_image
# tests and checks
settings.no_install_purge_test = opts.no_install_purge_test
settings.no_upgrade_test = opts.no_upgrade_test
--
2.16.2
From 928119530426c37cff11d1d4380e5b4ee400b38c Mon Sep 17 00:00:00 2001 From: Agustin Henze <t...@aayy.com.ar> Date: Thu, 22 Mar 2018 09:06:16 -0300 Subject: [PATCH 2/6] Add `--docker-image` param doc in manpage Signed-off-by: Agustin Henze <t...@aayy.com.ar> --- piuparts.1.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/piuparts.1.txt b/piuparts.1.txt index 94403240..6fe4e584 100644 --- a/piuparts.1.txt +++ b/piuparts.1.txt @@ -232,6 +232,9 @@ Note that file: addresses works if the directories are made accessible from with *--schroot*='SCHROOT-NAME':: Use schroot session named SCHROOT-NAME for the chroot, instead of building a new one with debootstrap. +*--docker-image*='DOCKER-IMAGE':: + Use a container created from the docker image DOCKER-IMAGE for the testing environment, instead of building a new one with debootstrap. + *--single-changes-list*:: When processing changes files, piuparts will process the packages in each individual changes file seperately. This option will set piuparts to scan the packages of all changes files together along with any individual package files that may have been given on the command line. -- 2.16.2
From 5e3a28a24a8a3ab59a438784a43bec84c6519671 Mon Sep 17 00:00:00 2001
From: Agustin Henze <t...@aayy.com.ar>
Date: Thu, 22 Mar 2018 09:11:28 -0300
Subject: [PATCH 3/6] Rewrite `--keep-tmpdir` doc
Signed-off-by: Agustin Henze <t...@aayy.com.ar>
---
piuparts.1.txt | 5 ++++-
piuparts.py | 4 ++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/piuparts.1.txt b/piuparts.1.txt
index 6fe4e584..ba2a9ce5 100644
--- a/piuparts.1.txt
+++ b/piuparts.1.txt
@@ -144,7 +144,10 @@ The tarball can be created with the '-s' option, or you can use one that *pbuild
Remove package after installation and reinstall. For testing installation in config-files-remaining state.
*-k*, *--keep-tmpdir*::
- Don't remove the temporary directory for the chroot when the program ends.
+ Depending on which option is passed, it keeps the environment used for testing after the program ends::
+ * By default it doesn't remove the temporary directory for the chroot,
+ * or if --schroot is used, the schroot session is not terminated,
+ * or if --docker-image is used, the container created is not destroyed
*-K*, *--keyring*='filename'::
Use FILE as the keyring to use with debootstrap when creating chroots.
diff --git a/piuparts.py b/piuparts.py
index fc0530d3..a895cd21 100644
--- a/piuparts.py
+++ b/piuparts.py
@@ -2738,8 +2738,8 @@ def parse_command_line():
parser.add_option("-k", "--keep-tmpdir",
action="store_true", default=False,
- help="Don't remove the temporary directory for the " +
- "chroot when the program ends.")
+ help="Keep the environment used for testing after the "
+ "programm ends.")
parser.add_option("-K", "--keyring", action="store", metavar="FILE",
help="Use FILE as the keyring to use with debootstrap when creating chroots.")
--
2.16.2
From ed7a5a32f22dcf56ce82bd5be9000aaa9cffd63f Mon Sep 17 00:00:00 2001
From: Agustin Henze <t...@aayy.com.ar>
Date: Thu, 22 Mar 2018 09:17:36 -0300
Subject: [PATCH 4/6] Rephrase `--schroot` param doc
Signed-off-by: Agustin Henze <t...@aayy.com.ar>
---
piuparts.1.txt | 2 +-
piuparts.py | 5 +++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/piuparts.1.txt b/piuparts.1.txt
index ba2a9ce5..9855b8d6 100644
--- a/piuparts.1.txt
+++ b/piuparts.1.txt
@@ -233,7 +233,7 @@ Note that file: addresses works if the directories are made accessible from with
Directory where are custom scripts are placed. By default, this is not set. For more information about this, read README_server.txt
*--schroot*='SCHROOT-NAME'::
- Use schroot session named SCHROOT-NAME for the chroot, instead of building a new one with debootstrap.
+ Use schroot session named SCHROOT-NAME for the testing environment, instead of building a new one with debootstrap.
*--docker-image*='DOCKER-IMAGE'::
Use a container created from the docker image DOCKER-IMAGE for the testing environment, instead of building a new one with debootstrap.
diff --git a/piuparts.py b/piuparts.py
index a895cd21..6832793f 100644
--- a/piuparts.py
+++ b/piuparts.py
@@ -2768,8 +2768,9 @@ def parse_command_line():
"a new LVM snapshot (default: 1G)")
parser.add_option("--schroot", metavar="SCHROOT-NAME", action="store",
- help="Use schroot session named SCHROOT-NAME for the chroot, instead of building " +
- "a new one with debootstrap.")
+ help="Use schroot session named SCHROOT-NAME for the "
+ "testing environment, instead of building a new one "
+ "with debootstrap.")
parser.add_option("--docker-image", metavar="DOCKER-IMAGE", action="store",
help="Use a container created from the docker image "
--
2.16.2
From ed49dba2a80bb82d824e4877edf236d3a3f8cc30 Mon Sep 17 00:00:00 2001 From: Agustin Henze <t...@aayy.com.ar> Date: Thu, 22 Mar 2018 11:46:18 -0300 Subject: [PATCH 5/6] Add `docker.io` as suggested package Signed-off-by: Agustin Henze <t...@aayy.com.ar> --- debian/control | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/debian/control b/debian/control index e9f90237..898db7bf 100644 --- a/debian/control +++ b/debian/control @@ -45,7 +45,8 @@ Depends: Recommends: adequate Suggests: - schroot + schroot, + docker.io, Description: .deb package installation, upgrading, and removal testing tool piuparts tests that .deb packages (as used by Debian) handle installation, upgrading, and removal correctly. It does this by -- 2.16.2
From 9800785b2ea5dc46ce82df6f989e904f1babb458 Mon Sep 17 00:00:00 2001 From: Agustin Henze <t...@aayy.com.ar> Date: Thu, 22 Mar 2018 12:38:16 -0300 Subject: [PATCH 6/6] Update debian/changelog Signed-off-by: Agustin Henze <t...@aayy.com.ar> --- debian/changelog | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/debian/changelog b/debian/changelog index c9705418..3e66bb2e 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,8 +1,15 @@ piuparts (0.85) UNRELEASED; urgency=medium + [ Holger Levsen ] * dwke.py: change logging to make relevant information more visible. * detect_*_issues.in: improve output. + [ Agustin Henze ] + * Add docker support, new param is introduced `--docker-image`. + (Closes: #893731) + * Add `docker.io` as suggested package + * Reworded documentation for `--keep-tmpdir` and `--schroot` parameters + -- Holger Levsen <hol...@debian.org> Fri, 16 Feb 2018 12:47:08 +0000 piuparts (0.84) unstable; urgency=medium -- 2.16.2
signature.asc
Description: OpenPGP digital signature
