launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #21925
[Merge] lp:~cjwatson/launchpad-buildd/loop-mounted-dev into lp:launchpad-buildd
Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/loop-mounted-dev into lp:launchpad-buildd.
Commit message:
The previous patch was labouring under mistaken assumptions: it's actually the mounted-dev Upstart job that we race with in trusty containers, so neuter that instead.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/loop-mounted-dev/+merge/332409
This is nicer than the previous approach in that we get to use a proper override file and will cope with the /etc/init/ directory not existing, but more horrible in that we have to produce a mangled shell script.
This time, I've confirmed that if I start a container with the mounted-dev neutering but not with launchpad-buildd's own loop device creation, I get a container with no loop devices; so I think it actually works this time.
A few device nodes go missing in trusty containers as a result of this:
/dev/kmem
/dev/mem
/dev/port
/dev/ppp
/dev/ram*
We can add these back in if we need to, but it gets more complicated. Most of these are deprecated (not present in xenial) or fairly useless in this context, so I think it'll be OK.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/loop-mounted-dev into lp:launchpad-buildd.
=== modified file 'debian/changelog'
--- debian/changelog 2017-10-18 06:58:31 +0000
+++ debian/changelog 2017-10-18 12:02:06 +0000
@@ -1,3 +1,11 @@
+launchpad-buildd (154) UNRELEASED; urgency=medium
+
+ * The previous patch was labouring under mistaken assumptions: it's
+ actually the mounted-dev Upstart job that we race with in trusty
+ containers, so neuter that instead.
+
+ -- Colin Watson <cjwatson@xxxxxxxxxx> Wed, 18 Oct 2017 12:52:26 +0100
+
launchpad-buildd (153) xenial; urgency=medium
* Defend against racing with udev to create loop devices in trusty
=== modified file 'lpbuildd/target/lxd.py'
--- lpbuildd/target/lxd.py 2017-10-17 16:20:21 +0000
+++ lpbuildd/target/lxd.py 2017-10-18 12:02:06 +0000
@@ -9,6 +9,7 @@
import io
import json
import os
+import re
import stat
import subprocess
import tarfile
@@ -331,26 +332,38 @@
policy_rc_d_file.flush()
os.fchmod(policy_rc_d_file.fileno(), 0o755)
self.copy_in(policy_rc_d_file.name, "/usr/local/sbin/policy-rc.d")
- # Ensure that loop devices are not created, even if the target
- # system's udev rules would ordinarily do so. We can't do it the
- # other way round (ensure that udev always creates them) because not
- # all buildd chroots have udev.
- # Poking this into /lib is wrong, but /etc/udev/rules.d/ doesn't
- # exist in all buildd chroots, and xenial's LXD doesn't support
- # creating directories when pushing files. The containers won't be
- # upgraded in ways that make this be a problem anyway.
- with tempfile.NamedTemporaryFile(mode="w+") as udev_rules_file:
- # Copied from systemd 234.
- print(
- 'SUBSYSTEM=="block", KERNEL=="loop[0-9]*", '
- 'ENV{DEVTYPE}=="disk", TEST!="loop/backing_file", '
- 'ENV{SYSTEMD_READY}="0"',
- file=udev_rules_file)
- udev_rules_file.flush()
- os.fchmod(udev_rules_file.fileno(), 0o644)
- self.copy_in(
- udev_rules_file.name,
- "/lib/udev/rules.d/99-zz-buildd-loop.rules")
+ # For targets that use Upstart, prevent the mounted-dev job from
+ # creating devices. Most of the devices it creates are unnecessary
+ # in a container, and creating loop devices will race with our own
+ # code to do so.
+ with tempfile.NamedTemporaryFile(mode="w+") as mounted_dev_file:
+ try:
+ self.copy_out(
+ "/etc/init/mounted-dev.conf", mounted_dev_file.name)
+ except LXDAPIException:
+ pass
+ else:
+ mounted_dev_file.seek(0, os.SEEK_SET)
+ script = ""
+ in_script = False
+ for line in mounted_dev_file:
+ if in_script:
+ script += re.sub(
+ r"^(\s*)(.*MAKEDEV)", r"\1: # \2", line)
+ if line.strip() == "end script":
+ in_script = False
+ elif line.strip() == "script":
+ script += line
+ in_script = True
+ if script:
+ mounted_dev_file.seek(0, os.SEEK_SET)
+ mounted_dev_file.truncate()
+ mounted_dev_file.write(script)
+ mounted_dev_file.flush()
+ os.fchmod(mounted_dev_file.fileno(), 0o644)
+ self.copy_in(
+ mounted_dev_file.name,
+ "/etc/init/mounted-dev.override")
# Start the container and wait for it to start.
container.start(wait=True)
=== modified file 'lpbuildd/target/tests/test_lxd.py'
--- lpbuildd/target/tests/test_lxd.py 2017-10-17 17:48:48 +0000
+++ lpbuildd/target/tests/test_lxd.py 2017-10-18 12:02:06 +0000
@@ -56,6 +56,23 @@
return "Fake LXD exception"
+class FakeSessionGet:
+
+ def __init__(self, file_contents):
+ self.file_contents = file_contents
+
+ def __call__(self, *args, **kwargs):
+ params = kwargs["params"]
+ response = mock.MagicMock()
+ if params["path"] in self.file_contents:
+ response.status_code = 200
+ response.iter_content.return_value = iter(
+ self.file_contents[params["path"]])
+ else:
+ response.json.return_value = {"error": "not found"}
+ return response
+
+
class TestLXD(TestCase):
def make_chroot_tarball(self, output_path):
@@ -184,9 +201,9 @@
lambda wait=False: setattr(container, "status_code", LXD_RUNNING))
files_api = container.api.files
files_api._api_endpoint = "/1.0/containers/lp-xenial-amd64/files"
- files_api.session.get.return_value.status_code = 200
- files_api.session.get.return_value.iter_content.return_value = (
- iter([b"127.0.0.1\tlocalhost\n"]))
+ files_api.session.get.side_effect = FakeSessionGet({
+ "/etc/hosts": [b"127.0.0.1\tlocalhost\n"],
+ })
processes_fixture = self.useFixture(FakeProcesses())
processes_fixture.add(lambda _: {}, name="sudo")
processes_fixture.add(lambda _: {}, name="lxc")
@@ -248,7 +265,7 @@
"profiles": ["lpbuildd"],
"source": {"type": "image", "alias": "lp-xenial-amd64"},
}, wait=True)
- files_api.session.get.assert_called_once_with(
+ files_api.session.get.assert_any_call(
"/1.0/containers/lp-xenial-amd64/files",
params={"path": "/etc/hosts"}, stream=True)
files_api.post.assert_any_call(
@@ -267,13 +284,13 @@
params={"path": "/usr/local/sbin/policy-rc.d"},
data=policy_rc_d.encode("UTF-8"),
headers={"X-LXD-uid": 0, "X-LXD-gid": 0, "X-LXD-mode": "0755"})
- files_api.post.assert_any_call(
- params={"path": "/lib/udev/rules.d/99-zz-buildd-loop.rules"},
- data=(
- b'SUBSYSTEM=="block", KERNEL=="loop[0-9]*", '
- b'ENV{DEVTYPE}=="disk", TEST!="loop/backing_file", '
- b'ENV{SYSTEMD_READY}="0"\n'),
- headers={"X-LXD-uid": 0, "X-LXD-gid": 0, "X-LXD-mode": "0644"})
+ files_api.session.get.assert_any_call(
+ "/1.0/containers/lp-xenial-amd64/files",
+ params={"path": "/etc/init/mounted-dev.conf"}, stream=True)
+ self.assertNotIn(
+ "/etc/init/mounted-dev.override",
+ [kwargs["params"]["path"]
+ for _, kwargs in files_api.post.call_args_list])
files_api.post.assert_any_call(
params={"path": "/etc/systemd/system/snapd.service.d/no-cdn.conf"},
data=b"[Service]\nEnvironment=SNAPPY_STORE_NO_CDN=1\n",
@@ -300,16 +317,13 @@
lambda wait=False: setattr(container, "status_code", LXD_RUNNING))
files_api = container.api.files
files_api._api_endpoint = "/1.0/containers/lp-xenial-amd64/files"
- files_api.session.get.return_value.status_code = 404
- files_api.session.get.return_value.json.return_value = {
- "error": "not found",
- }
+ files_api.session.get.side_effect = FakeSessionGet({})
processes_fixture = self.useFixture(FakeProcesses())
processes_fixture.add(lambda _: {}, name="sudo")
processes_fixture.add(lambda _: {}, name="lxc")
LXD("1", "xenial", "amd64").start()
- files_api.session.get.assert_called_once_with(
+ files_api.session.get.assert_any_call(
"/1.0/containers/lp-xenial-amd64/files",
params={"path": "/etc/hosts"}, stream=True)
files_api.post.assert_any_call(
@@ -319,6 +333,52 @@
"\n127.0.1.1\tlp-xenial-amd64\n").encode("UTF-8"),
headers={"X-LXD-uid": 0, "X-LXD-gid": 0, "X-LXD-mode": "0644"})
+ def test_start_with_mounted_dev_conf(self):
+ fs_fixture = self.useFixture(FakeFilesystem())
+ fs_fixture.add("/sys")
+ fs_fixture.add("/run")
+ os.makedirs("/run/launchpad-buildd")
+ fs_fixture.add("/etc")
+ os.mkdir("/etc")
+ with open("/etc/resolv.conf", "w") as f:
+ print("host resolv.conf", file=f)
+ os.chmod("/etc/resolv.conf", 0o644)
+ self.useFixture(MockPatch("pylxd.Client"))
+ client = pylxd.Client()
+ client.profiles.get.side_effect = FakeLXDAPIException
+ container = client.containers.create.return_value
+ client.containers.get.return_value = container
+ container.start.side_effect = (
+ lambda wait=False: setattr(container, "status_code", LXD_RUNNING))
+ files_api = container.api.files
+ files_api._api_endpoint = "/1.0/containers/lp-trusty-amd64/files"
+ files_api.session.get.side_effect = FakeSessionGet({
+ "/etc/init/mounted-dev.conf": dedent("""\
+ start on mounted MOUNTPOINT=/dev
+ script
+ [ -e /dev/shm ] || ln -s /run/shm /dev/shm
+ /sbin/MAKEDEV std fd ppp tun
+ end script
+ task
+ """)})
+ processes_fixture = self.useFixture(FakeProcesses())
+ processes_fixture.add(lambda _: {}, name="sudo")
+ processes_fixture.add(lambda _: {}, name="lxc")
+ LXD("1", "trusty", "amd64").start()
+
+ files_api.session.get.assert_any_call(
+ "/1.0/containers/lp-trusty-amd64/files",
+ params={"path": "/etc/init/mounted-dev.conf"}, stream=True)
+ files_api.post.assert_any_call(
+ params={"path": "/etc/init/mounted-dev.override"},
+ data=dedent("""\
+ script
+ [ -e /dev/shm ] || ln -s /run/shm /dev/shm
+ : # /sbin/MAKEDEV std fd ppp tun
+ end script
+ """).encode("UTF-8"),
+ headers={"X-LXD-uid": 0, "X-LXD-gid": 0, "X-LXD-mode": "0644"})
+
def test_run(self):
processes_fixture = self.useFixture(FakeProcesses())
processes_fixture.add(lambda _: {}, name="lxc")
@@ -375,13 +435,13 @@
client = pylxd.Client()
container = mock.MagicMock()
client.containers.get.return_value = container
- files_api = container.api.files
- files_api._api_endpoint = "/1.0/containers/lp-xenial-amd64/files"
- files_api.session.get.return_value.status_code = 200
- files_api.session.get.return_value.iter_content.return_value = (
- iter([b"hello\n", b"world\n"]))
source_path = "/path/to/source"
target_path = os.path.join(target_dir, "target")
+ files_api = container.api.files
+ files_api._api_endpoint = "/1.0/containers/lp-xenial-amd64/files"
+ files_api.session.get.side_effect = FakeSessionGet({
+ source_path: [b"hello\n", b"world\n"],
+ })
LXD("1", "xenial", "amd64").copy_out(source_path, target_path)
client.containers.get.assert_called_once_with("lp-xenial-amd64")
