[Piuparts-devel] [Git][debian/piuparts][phahn/docker-fixes] 15 commits: Start 1.2.1 development

Philipp Hahn (@pmhahn) gitlab at salsa.debian.org
Wed Dec 6 06:56:06 GMT 2023



Philipp Hahn pushed to branch phahn/docker-fixes at Debian / piuparts


Commits:
6d601ebe by Nicolas Dandrimont at 2023-10-29T12:50:52+01:00
Start 1.2.1 development

Gbp-Dch: ignore

- - - - -
4d436fc3 by Helmut Grohne at 2023-11-08T08:55:34+01:00
improve compatibility of mount_proc with unprivileged namespaces

When run in an unprivileged namespace, both the mknod and the mount
operation may be denied and piuparts may fail here. Looking deeper, this
branch happens when /dev/ptmx is not already a symlink and what is bind
mounted can be expressed as a symlink, so opt for implementing this
device as a symlink when it is missing entirely. If nothing else, one
less mount invocation is a minor speedup.

- - - - -
b7f4db99 by Helmut Grohne at 2023-11-08T09:00:43+01:00
make --bindmount recursive

When performing a bind mount, Linux will not do so recursively by
default. So if a subdirectory of the provided bind mount is a mount
point itself, this mount point will not be propagated and the underlying
hierarchy will be exposed inside piuparts. This may be intentional, but
likely is not.

Since a bind mount may be used to access a hierarchy that was hidden by
another mount, bind mounts are denied in unprivileged namespaces. On the
other hand, recursive bind mounts just work there.

- - - - -
13d93cee by Helmut Grohne at 2023-11-08T09:07:14+01:00
allow using --bindmount with things that are not directories

While typically think of directories when it comes to mounting, one can
also mount regular files or devices. Doing so only works when the mount
target is not a directory though and piuparts kindly creates such
directories. So when the mount options reveal that we're doing a bind
mount and the source is not a directory, we create an empty file
instead.

- - - - -
eb94e796 by Helmut Grohne at 2023-11-08T12:03:18+01:00
allow using tarballs compressed with non-gzip

The extension of the filename given to --basetgz with or without --save
will be checked for known compression formats by tar. Therefore gzip
tarballs must now have a suffix of .gz or .tgz.

- - - - -
d78f6457 by Helmut Grohne at 2023-11-08T14:01:24+01:00
refactor the opts parameter to mount into a list

Reported-by: Nicolas Dandrimont <nicolas at dandrimont.eu>

- - - - -
cdd25f5e by Nicolas Dandrimont at 2023-11-08T13:35:20+00:00
Explain why we bind mount ptmx when it exists
- - - - -
bd46522f by Nicolas Dandrimont at 2023-11-08T22:11:54+01:00
Merge branch 'helmutg/feature-tar-compression' into develop

- - - - -
4c861838 by Nicolas Dandrimont at 2023-11-08T22:12:09+01:00
Merge branch 'helmutg/improve-mount_proc' into develop

- - - - -
9e1fc883 by Nicolas Dandrimont at 2023-11-08T22:12:22+01:00
Merge branch 'helmutg/feature-recursive-bindmounts' into develop

- - - - -
929b4a4a by Nicolas Dandrimont at 2023-11-08T22:14:19+01:00
Merge branch 'helmutg/feature-bindmount-file' into develop

- - - - -
aa916c1e by Helmut Grohne at 2023-11-23T12:18:07+01:00
provide a basic /dev tree even in unprivileged namespaces

Most piuparts environments will provide a working /dev tree. For
instance, a pbuilder base.tgz contains them. As does a schroot tree.
Likewise, docker will set up devices. Even debootstrap will create them.
However when running piuparts in an unprivileged namespace, debootstrap
cannot create them (and can be made to work by exporting container=lxc).
Similarly, when passing a device-less base.tgz for an unprivileged
namespace they are missing. While piuparts previously created /dev/null,
this is bound to fail with -EPERM. In that case, individual device nodes
need to be bind mounted. Since such bind mounting is not preferred for
the other variants, we try creating missing devices first.

- - - - -
c60ee5e7 by Philipp Hahn at 2023-12-06T07:54:52+01:00
fix(docker) getting Container ID

in case the image does not exists locally docker will pull it from any
remote registry, but will include the progress information in the
output:

> Unable to find image 'docker-registry.XXX/YYY:ZZZ' locally
> ZZZ: Pulling from YYY
...
> Digest: sha256:4b9bfe7b0a6c970e3613c04f267ba6319cfceb8cc120b0435d9ee7b8037a1f06
> Status: Downloaded newer image for docker-registry.XXX/YYY:ZZZ
> 8be38c89d12937b98c8be5ab7466dd45b0e4a306862f282b58077ac7193251eb

The old code expected the output to be the container ID, which fails in that case.

- - - - -
717d64a9 by Philipp Hahn at 2023-12-06T07:55:46+01:00
refactor(docker): Let `docker inspect` return path

Directly use golang templating to only get the MergedDir path instead of
using JSON in Python.

- - - - -
9817cf0c by Philipp Hahn at 2023-12-06T07:55:47+01:00
refactor(docker): improve log output

Include path in debug output.

Let logging.debug() handle the variable substitution.

- - - - -


3 changed files:

- debian/changelog
- docs/piuparts/piuparts.1.txt
- piuparts.py


Changes:

=====================================
debian/changelog
=====================================
@@ -1,3 +1,10 @@
+piuparts (1.2.1) UNRELEASED; urgency=medium
+
+  * Start 1.2.1 development. Use `gbp dch --since=1.2` to generate a
+    changelog for this release.
+
+ -- Nicolas Dandrimont <olasd at debian.org>  Sun, 29 Oct 2023 12:49:51 +0100
+
 piuparts (1.2) unstable; urgency=medium
 
   [ David Steele ]


=====================================
docs/piuparts/piuparts.1.txt
=====================================
@@ -61,7 +61,7 @@ Options must come before the other command line arguments.
 
   Use :file:`tarball` as the contents of the initial chroot, instead of building a new one with :program:`debootstrap`.
 
-  The tarball can be created with the :option:`-s` option, or you can use one that :program:`pbuilder` has created (see :option:`-p`). If you create one manually, make sure the root of the chroot is the root of the tarball.
+  The tarball can be created with the :option:`-s` option, or you can use one that :program:`pbuilder` has created (see :option:`-p`). If you create one manually, make sure the root of the chroot is the root of the tarball. Despite the option name implying gzip compression, the compression scheme is deduced by :program:`tar` from the filename suffix.
 
 .. option:: --bindmount dir
 


=====================================
piuparts.py
=====================================
@@ -35,7 +35,7 @@ from __future__ import print_function
 VERSION = "__PIUPARTS_VERSION__"
 
 
-import json
+import errno
 import logging
 import optparse
 import os
@@ -893,7 +893,7 @@ class Chroot:
         cleanup_tmpfile = lambda: os.remove(tmpfile)
         panic_handler_id = do_on_panic(cleanup_tmpfile)
 
-        run(['tar', '-czf', tmpfile, '--one-file-system', '--exclude', 'tmp/scripts', '-C', self.name, './'])
+        run(['tar', '--auto-compress', '-cf', tmpfile, '--one-file-system', '--exclude', 'tmp/scripts', '-C', self.name, './'])
 
         os.chmod(tmpfile, 0o644)
         os.rename(tmpfile, result)
@@ -905,7 +905,7 @@ class Chroot:
         prefix = []
         if settings.eatmydata and os.path.isfile('/usr/bin/eatmydata'):
             prefix.append('eatmydata')
-        run(prefix + ["tar", "-C", self.name, "-zxf", tarball])
+        run(prefix + ["tar", "-C", self.name, "--auto-compress", "-xf", tarball])
 
     def setup_from_schroot(self, schroot):
         self.schroot_session = schroot.split(":", 1)[-1] + "-" + str(uuid.uuid1()) + "-piuparts"
@@ -923,15 +923,17 @@ class Chroot:
 
     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)
+        with tempfile.NamedTemporaryFile("r") as cidfile:
+            ret_code, output = run(['docker', 'run', '-d', '-it', '--cidfile', cidfile.name, docker_image, 'bash'])
+            if ret_code != 0:
+                logging.error("Couldn't start the container from '%s'" % docker_image)
+                panic()
+
+            self.docker_container = cidfile.read().strip()
+
+        ret_code, output = run(['docker', 'inspect', '-f', '{{ .GraphDriver.Data.MergedDir }}', self.docker_container])
+        self.name = output.strip()
+        logging.info("New container created %r at %r", self.docker_container, self.name)
 
     def setup_from_lvm(self, lvm_volume):
         """Create a chroot by creating an LVM snapshot."""
@@ -1173,10 +1175,53 @@ class Chroot:
         self.create_dpkg_conf()
         self.create_policy_rc_d()
         self.create_resolv_conf()
+
+        # Most environments will have devices set up. Unprivileged namespaces do
+        # not. Ensure presence of devices while not clobbering existing ones.
+        # Set of devices taken from https://systemd.io/CONTAINER_INTERFACE/
+        chardevices = {
+            "null": os.makedev(1, 3),
+            "zero": os.makedev(1, 5),
+            "full": os.makedev(1, 7),
+            "random": os.makedev(1, 8),
+            "urandom": os.makedev(1, 9),
+            "tty": os.makedev(5, 0),
+        }
+        for devname, devnum in chardevices.items():
+            devname = "/dev/" + devname
+            isdevice = False
+            try:
+                isdevice = stat.S_ISCHR(os.stat(self.name + devname).st_mode)
+            except FileNotFoundError:
+                # Try creating missing devices. If that fails with -EPERM, we
+                # likely are in an unprivileged namespace and resort to bind
+                # mounting them individually.
+                try:
+                    os.mknod(self.name + devname, stat.S_IFCHR | 0o666, devnum)
+                    isdevice = True
+                except OSError as err:
+                    if err.errno != errno.EPERM:
+                        raise
+                    # Create a regular file to serve as a mount point.
+                    os.mknod(self.name + devname, stat.S_IFREG)
+            if not isdevice:
+                self.mount(devname, devname, opts=["bind"])
+
+        symlinks = {
+            "fd": "/proc/self/fd",
+            "stdin": "/proc/self/fd/0",
+            "stdout": "/proc/self/fd/1",
+            "stderr": "/proc/self/fd/2",
+        }
+        for linkname, linktarget in symlinks.items():
+            linkname = self.name + "/dev/" + linkname
+            try:
+                os.lstat(linkname)
+            except FileNotFoundError:
+                os.symlink(linktarget, linkname)
+
         for bindmount in settings.bindmounts:
-            self.mount(bindmount, bindmount, opts="bind")
-        if not os.path.exists(self.name + '/dev/null'):
-            run(['mknod', '-m' ,'666', self.name + '/dev/null', 'c', '1', '3'])
+            self.mount(bindmount, bindmount, opts=["rbind"])
 
     def remember_available_md5(self):
         """Keep a history of 'apt-cache dumpavail | md5sum' after initial
@@ -1764,15 +1809,21 @@ class Chroot:
 
     def mount(self, source, path, fstype=None, opts=None, no_mkdir=False):
         """Mount something into the chroot and remember it for unmount_all()."""
+        if opts is None:
+            opts = []
         path = canonicalize_path(self.name, path)
-        if not no_mkdir:
-            self.mkdir_p(path)
         fullpath = self.relative(path)
+        if not no_mkdir:
+            if not ("bind" in opts or "rbind" in opts) or os.path.isdir(source):
+                self.mkdir_p(path)
+            elif not os.path.exists(fullpath):
+                self.mkdir_p(os.path.dirname(path))
+                os.mknod(fullpath, stat.S_IFREG)
         command = ["mount"]
         if fstype is not None:
             command.extend(["-t", fstype])
-        if opts is not None:
-            command.extend(["-o", opts])
+        if opts:
+            command.extend(["-o", ",".join(opts)])
         command.extend([source, fullpath])
         run(command)
         self.mounts.append(fullpath)
@@ -1796,12 +1847,16 @@ class Chroot:
         etcmtab = self.relative("etc/mtab")
         if not os.path.lexists(etcmtab):
             os.symlink("../proc/mounts", etcmtab)
-        self.mount("devpts", "/dev/pts", fstype="devpts", opts="newinstance,noexec,nosuid,gid=5,mode=0620,ptmxmode=0666")
+        self.mount("devpts", "/dev/pts", fstype="devpts", opts=["newinstance", "noexec", "nosuid", "gid=5", "mode=0620", "ptmxmode=0666"])
         dev_ptmx_rel_path = self.relative("dev/ptmx")
         if not os.path.islink(dev_ptmx_rel_path):
             if not os.path.exists(dev_ptmx_rel_path):
-                os.mknod(dev_ptmx_rel_path, 0o0666 | stat.S_IFCHR, os.makedev(5, 2))
-            self.mount(self.relative("dev/pts/ptmx"), "/dev/ptmx", opts="bind", no_mkdir=True)
+                # /dev/pts/ptmx has been created by the previous self.mount("devpts", ...), we can symlink /dev/ptmx to it.
+                os.symlink("pts/ptmx", dev_ptmx_rel_path)
+            else:
+                # /dev/ptmx is an unknown entity. Override it with a safe bind mount of /dev/pts/ptmx.
+                # Unlinking the existing /dev/ptmx could fail, this is safer.
+                self.mount(self.relative("dev/pts/ptmx"), "/dev/ptmx", opts=["bind"], no_mkdir=True)
         p = subprocess.Popen(["tty"], stdout=subprocess.PIPE,
                              universal_newlines=True)
         stdout, _ = p.communicate()
@@ -1810,10 +1865,10 @@ class Chroot:
             dev_console = self.relative("/dev/console")
             if not os.path.exists(dev_console):
                 os.mknod(dev_console, 0o0600, os.makedev(5, 1))
-            self.mount(current_tty, "/dev/console", opts="bind", no_mkdir=True)
-        self.mount("tmpfs", "/dev/shm", fstype="tmpfs", opts="size=65536k")
+            self.mount(current_tty, "/dev/console", opts=["bind"], no_mkdir=True)
+        self.mount("tmpfs", "/dev/shm", fstype="tmpfs", opts=["size=65536k"])
         if selinux_enabled():
-            self.mount("/sys/fs/selinux", self.selinuxfs_path(), opts="bind,ro")
+            self.mount("/sys/fs/selinux", self.selinuxfs_path(), opts=["bind", "ro"])
 
     def is_ignored(self, pathname, info="PATH"):
         """Is a file (or dir or whatever) to be ignored?"""



View it on GitLab: https://salsa.debian.org/debian/piuparts/-/compare/0dfa4c0b93a52d890834a223d71d0646b4a0f441...9817cf0cb411777f69cb07ef6a9d9fe839cfa341

-- 
View it on GitLab: https://salsa.debian.org/debian/piuparts/-/compare/0dfa4c0b93a52d890834a223d71d0646b4a0f441...9817cf0cb411777f69cb07ef6a9d9fe839cfa341
You're receiving this email because of your account on salsa.debian.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://alioth-lists.debian.net/pipermail/piuparts-devel/attachments/20231206/07dc6b27/attachment-0001.htm>


More information about the Piuparts-devel mailing list