launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29376
[Merge] ~andrey-fedoseev/launchpad-buildd:backend-open into launchpad-buildd:master
Andrey Fedoseev has proposed merging ~andrey-fedoseev/launchpad-buildd:backend-open into launchpad-buildd:master.
Commit message:
Use `Backend.open` to modify files in target environments
instead of using a local temporary file and `copy_out` / `copy_in` methods
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~andrey-fedoseev/launchpad-buildd/+git/launchpad-buildd/+merge/432731
A new method `open` is added to the base `Backend` class
The method access to the files in the target environment via a file-like object.
Under the hood it uses a temporary file on the host system and `copy_in` / `copy_out` methods to transfer the file to / from the target environment.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~andrey-fedoseev/launchpad-buildd:backend-open into launchpad-buildd:master.
diff --git a/lpbuildd/binarypackage.py b/lpbuildd/binarypackage.py
index c3fef09..3498e58 100644
--- a/lpbuildd/binarypackage.py
+++ b/lpbuildd/binarypackage.py
@@ -156,11 +156,11 @@ class BinaryPackageBuildManager(DebianBuildManager):
self.archive_purpose))
if self.build_debug_symbols:
currently_building_contents += 'Build-Debug-Symbols: yes\n'
- with tempfile.NamedTemporaryFile(mode='w+') as currently_building:
+ with self.backend.open(
+ '/CurrentlyBuilding', mode='w+'
+ ) as currently_building:
currently_building.write(currently_building_contents)
- currently_building.flush()
os.fchmod(currently_building.fileno(), 0o644)
- self.backend.copy_in(currently_building.name, '/CurrentlyBuilding')
args = ["sbuild-package", self._buildid, self.arch_tag]
args.append(self.suite)
diff --git a/lpbuildd/ci.py b/lpbuildd/ci.py
index 04aa5f8..601d8b4 100644
--- a/lpbuildd/ci.py
+++ b/lpbuildd/ci.py
@@ -6,7 +6,6 @@ from configparser import (
NoSectionError,
)
import os
-import tempfile
import yaml
from twisted.internet import defer
@@ -168,14 +167,10 @@ class CIBuildManager(BuildManagerProxyMixin, DebianBuildManager):
["--plugin-setting", f"{key}={value}"])
if self.secrets is not None:
text = yaml.dump(self.secrets)
- with tempfile.NamedTemporaryFile(mode="w") as f:
+ with self.backend.open(
+ "/build/.launchpad-secrets.yaml", mode="w"
+ ) as f:
f.write(text)
- f.flush()
- path_to_secrets = f.name
- self.backend.copy_in(
- source_path=path_to_secrets,
- target_path="/build/.launchpad-secrets.yaml"
- )
args.extend(
["--secrets", "/build/.launchpad-secrets.yaml"])
if self.scan_malware:
diff --git a/lpbuildd/target/apt.py b/lpbuildd/target/apt.py
index ebd4683..a411806 100644
--- a/lpbuildd/target/apt.py
+++ b/lpbuildd/target/apt.py
@@ -5,7 +5,6 @@ import logging
import os
import subprocess
import sys
-import tempfile
from textwrap import dedent
import time
@@ -30,49 +29,44 @@ class OverrideSourcesList(Operation):
def run(self):
logger.info("Overriding sources.list in build-%s", self.args.build_id)
- with tempfile.NamedTemporaryFile(mode="w+") as sources_list:
+ with self.backend.open(
+ "/etc/apt/sources.list", mode="w+"
+ ) as sources_list:
for archive in self.args.archives:
print(archive, file=sources_list)
- sources_list.flush()
os.fchmod(sources_list.fileno(), 0o644)
- self.backend.copy_in(sources_list.name, "/etc/apt/sources.list")
- with tempfile.NamedTemporaryFile(mode="w+") as apt_retries_conf:
+ with self.backend.open(
+ "/etc/apt/apt.conf.d/99retries", mode="w+"
+ ) as apt_retries_conf:
print('Acquire::Retries "3";', file=apt_retries_conf)
- apt_retries_conf.flush()
os.fchmod(apt_retries_conf.fileno(), 0o644)
- self.backend.copy_in(
- apt_retries_conf.name, "/etc/apt/apt.conf.d/99retries")
# Versions of APT that support phased updates do this automatically
# if running in a chroot, but builds may be running in a LXD
# container instead.
- with tempfile.NamedTemporaryFile(mode="w+") as apt_phasing_conf:
+ with self.backend.open(
+ "/etc/apt/apt.conf.d/99phasing", mode="w+"
+ ) as apt_phasing_conf:
print('APT::Get::Always-Include-Phased-Updates "true";',
file=apt_phasing_conf)
- apt_phasing_conf.flush()
os.fchmod(apt_phasing_conf.fileno(), 0o644)
- self.backend.copy_in(
- apt_phasing_conf.name, "/etc/apt/apt.conf.d/99phasing")
if self.args.apt_proxy_url is not None:
- with tempfile.NamedTemporaryFile(mode="w+") as apt_proxy_conf:
+ with self.backend.open(
+ "/etc/apt/apt.conf.d/99proxy", mode="w+"
+ ) as apt_proxy_conf:
print(
f'Acquire::http::Proxy "{self.args.apt_proxy_url}";',
file=apt_proxy_conf)
- apt_proxy_conf.flush()
os.fchmod(apt_proxy_conf.fileno(), 0o644)
- self.backend.copy_in(
- apt_proxy_conf.name, "/etc/apt/apt.conf.d/99proxy")
for pocket in ("proposed", "backports"):
- with tempfile.NamedTemporaryFile(mode="w+") as preferences:
+ with self.backend.open(
+ f"/etc/apt/preferences.d/{pocket}.pref", mode="w+"
+ ) as preferences:
print(dedent(f"""\
Package: *
Pin: release a=*-{pocket}
Pin-Priority: 500
"""), file=preferences, end="")
- preferences.flush()
os.fchmod(preferences.fileno(), 0o644)
- self.backend.copy_in(
- preferences.name,
- f"/etc/apt/preferences.d/{pocket}.pref")
return 0
@@ -91,7 +85,9 @@ class AddTrustedKeys(Operation):
gpg_cmd = [
"gpg", "--ignore-time-conflict", "--no-options", "--no-keyring",
]
- with tempfile.NamedTemporaryFile(mode="wb+") as keyring:
+ with self.backend.open(
+ "/etc/apt/trusted.gpg.d/launchpad-buildd.gpg", mode="wb+"
+ ) as keyring:
subprocess.check_call(
gpg_cmd + ["--dearmor"], stdin=self.input_file, stdout=keyring)
keyring.seek(0)
@@ -100,8 +96,6 @@ class AddTrustedKeys(Operation):
["--show-keys", "--keyid-format", "long", "--fingerprint"],
stdin=keyring, stdout=self.show_keys_file)
os.fchmod(keyring.fileno(), 0o644)
- self.backend.copy_in(
- keyring.name, "/etc/apt/trusted.gpg.d/launchpad-buildd.gpg")
return 0
diff --git a/lpbuildd/target/backend.py b/lpbuildd/target/backend.py
index 69746bb..a920cf1 100644
--- a/lpbuildd/target/backend.py
+++ b/lpbuildd/target/backend.py
@@ -3,6 +3,10 @@
import os.path
import subprocess
+import tempfile
+from contextlib import contextmanager
+from pathlib import Path
+from shutil import rmtree
class BackendException(Exception):
@@ -189,6 +193,27 @@ class Backend:
"""Remove the backend."""
subprocess.check_call(["sudo", "rm", "-rf", self.build_path])
+ @contextmanager
+ def open(self, path: str, mode="r", **kwargs):
+ """
+ Provides access to the files in the target environment via a
+ file-like object.
+
+ The arguments are the same as those of the built-in `open` function.
+ """
+ tmp_dir = tempfile.mkdtemp()
+ tmp_path = os.path.join(tmp_dir, Path(path).name)
+ if self.path_exists(path):
+ self.copy_out(path, tmp_path)
+ tmp_file = open(tmp_path, mode=mode, **kwargs)
+ try:
+ yield tmp_file
+ finally:
+ tmp_file.close()
+ if mode not in ("r", "rb", "rt"):
+ self.copy_in(tmp_path, path)
+ rmtree(tmp_dir)
+
def make_backend(name, build_id, series=None, arch=None):
if name == "chroot":
diff --git a/lpbuildd/target/build_oci.py b/lpbuildd/target/build_oci.py
index 5241f9f..ecbda0d 100644
--- a/lpbuildd/target/build_oci.py
+++ b/lpbuildd/target/build_oci.py
@@ -3,7 +3,6 @@
import logging
import os.path
-import tempfile
from textwrap import dedent
from lpbuildd.target.backend import check_path_escape
@@ -56,10 +55,8 @@ class BuildOCI(BuilderProxyOperationMixin, VCSOperationMixin,
Environment="{setting.upper()}={self.args.proxy_url}"
""")
file_path = f"/etc/systemd/system/docker.service.d/{setting}.conf"
- with tempfile.NamedTemporaryFile(mode="w+") as systemd_file:
+ with self.backend.open(file_path, mode="w+") as systemd_file:
systemd_file.write(contents)
- systemd_file.flush()
- self.backend.copy_in(systemd_file.name, file_path)
def install(self):
logger.info("Running install phase...")
diff --git a/lpbuildd/target/build_snap.py b/lpbuildd/target/build_snap.py
index 30c5ecc..615c2d9 100644
--- a/lpbuildd/target/build_snap.py
+++ b/lpbuildd/target/build_snap.py
@@ -5,7 +5,6 @@ import argparse
import json
import logging
import os.path
-import tempfile
from textwrap import dedent
from urllib.parse import urlparse
@@ -92,13 +91,12 @@ class BuildSnap(BuilderProxyOperationMixin, VCSOperationMixin,
svn_servers += f"http-proxy-username = {proxy.username}\n"
if proxy.password:
svn_servers += f"http-proxy-password = {proxy.password}\n"
- with tempfile.NamedTemporaryFile(mode="w+") as svn_servers_file:
+ self.backend.run(["mkdir", "-p", "/root/.subversion"])
+ with self.backend.open(
+ "/root/.subversion/servers", mode="w+"
+ ) as svn_servers_file:
svn_servers_file.write(svn_servers)
- svn_servers_file.flush()
os.fchmod(svn_servers_file.fileno(), 0o644)
- self.backend.run(["mkdir", "-p", "/root/.subversion"])
- self.backend.copy_in(
- svn_servers_file.name, "/root/.subversion/servers")
def install(self):
logger.info("Running install phase...")
diff --git a/lpbuildd/target/lxd.py b/lpbuildd/target/lxd.py
index d58d9ab..63f36d5 100644
--- a/lpbuildd/target/lxd.py
+++ b/lpbuildd/target/lxd.py
@@ -9,7 +9,6 @@ import re
import stat
import subprocess
import tarfile
-import tempfile
from textwrap import dedent
import time
@@ -376,22 +375,16 @@ class LXD(Backend):
["hostname"], universal_newlines=True).rstrip("\n")
fqdn = subprocess.check_output(
["hostname", "--fqdn"], universal_newlines=True).rstrip("\n")
- with tempfile.NamedTemporaryFile(mode="w+") as hosts_file:
- try:
- self.copy_out("/etc/hosts", hosts_file.name)
- except LXDException:
- hosts_file.seek(0, os.SEEK_SET)
- hosts_file.write(fallback_hosts)
+ with self.open("/etc/hosts", mode="a") as hosts_file:
hosts_file.seek(0, os.SEEK_END)
+ if not hosts_file.tell():
+ # /etc/hosts is missing or empty
+ hosts_file.write(fallback_hosts)
print(f"\n127.0.1.1\t{fqdn} {hostname}", file=hosts_file)
- hosts_file.flush()
os.fchmod(hosts_file.fileno(), 0o644)
- self.copy_in(hosts_file.name, "/etc/hosts")
- with tempfile.NamedTemporaryFile(mode="w+") as hostname_file:
+ with self.open("/etc/hostname", mode="w+") as hostname_file:
print(hostname, file=hostname_file)
- hostname_file.flush()
os.fchmod(hostname_file.fileno(), 0o644)
- self.copy_in(hostname_file.name, "/etc/hostname")
resolv_conf = "/etc/resolv.conf"
@@ -403,23 +396,17 @@ class LXD(Backend):
self.copy_in(resolv_conf, "/etc/resolv.conf")
- with tempfile.NamedTemporaryFile(mode="w+") as policy_rc_d_file:
+ with self.open(
+ "/usr/local/sbin/policy-rc.d", mode="w+"
+ ) as policy_rc_d_file:
policy_rc_d_file.write(policy_rc_d)
- 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")
# 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 LXDException:
- pass
- else:
- mounted_dev_file.seek(0, os.SEEK_SET)
+ if self.path_exists("/etc/init/mounted-dev.conf"):
+ with self.open("/etc/init/mounted-dev.conf") as mounted_dev_file:
script = ""
in_script = False
for line in mounted_dev_file:
@@ -431,15 +418,13 @@ class LXD(Backend):
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")
+
+ if script:
+ with self.open(
+ "/etc/init/mounted-dev.override", mode="w"
+ ) as mounted_dev_override_file:
+ mounted_dev_override_file.write(script)
+ os.fchmod(mounted_dev_override_file.fileno(), 0o644)
# Start the container and wait for it to start.
container.start(wait=True)
@@ -481,16 +466,14 @@ class LXD(Backend):
# directory until the container has started. We can get away with
# this for the time being because snapd isn't in the buildd chroots.
self.run(["mkdir", "-p", "/etc/systemd/system/snapd.service.d"])
- with tempfile.NamedTemporaryFile(mode="w+") as no_cdn_file:
+ with self.open(
+ "/etc/systemd/system/snapd.service.d/no-cdn.conf", mode="w+"
+ ) as no_cdn_file:
print(dedent("""\
[Service]
Environment=SNAPPY_STORE_NO_CDN=1
"""), file=no_cdn_file, end="")
- no_cdn_file.flush()
os.fchmod(no_cdn_file.fileno(), 0o644)
- self.copy_in(
- no_cdn_file.name,
- "/etc/systemd/system/snapd.service.d/no-cdn.conf")
# Refreshing snaps from a timer unit during a build isn't
# appropriate. Mask this, but manually so that we don't depend on
diff --git a/lpbuildd/target/run_ci.py b/lpbuildd/target/run_ci.py
index 4ebf765..28dc2e7 100644
--- a/lpbuildd/target/run_ci.py
+++ b/lpbuildd/target/run_ci.py
@@ -3,7 +3,6 @@
import logging
import os
-import tempfile
from lpbuildd.target.build_snap import SnapChannelsAction
from lpbuildd.target.operation import Operation
@@ -77,16 +76,12 @@ class RunCIPrepare(BuilderProxyOperationMixin, VCSOperationMixin,
# services, which is convenient since it allows us to ensure
# that ClamAV's database is up to date before proceeding.
if self.args.clamav_database_url:
- freshclam_path = "/etc/clamav/freshclam.conf"
- with tempfile.NamedTemporaryFile(mode="w+") as freshclam_file:
- self.backend.copy_out(freshclam_path, freshclam_file.name)
- freshclam_file.seek(0, os.SEEK_END)
- print(
- f"PrivateMirror {self.args.clamav_database_url}",
- file=freshclam_file,
+ with self.backend.open(
+ "/etc/clamav/freshclam.conf", mode="a"
+ ) as freshclam_file:
+ freshclam_file.write(
+ f"PrivateMirror {self.args.clamav_database_url}\n"
)
- freshclam_file.flush()
- self.backend.copy_in(freshclam_file.name, freshclam_path)
kwargs = {}
env = self.build_proxy_environment(proxy_url=self.args.proxy_url)
if env:
diff --git a/lpbuildd/target/tests/test_backend.py b/lpbuildd/target/tests/test_backend.py
new file mode 100644
index 0000000..575f564
--- /dev/null
+++ b/lpbuildd/target/tests/test_backend.py
@@ -0,0 +1,37 @@
+from unittest.mock import patch, ANY
+
+from testtools import TestCase
+from fixtures import TempDir
+
+from lpbuildd.tests.fakebuilder import UncontainedBackend
+
+
+class TestBackend(TestCase):
+
+ def test_open(self):
+ backend = UncontainedBackend("1")
+ backend_root = self.useFixture(TempDir())
+ target_path = backend_root.join("test.txt")
+
+ with patch.object(
+ backend, "copy_in", wraps=backend.copy_in
+ ) as copy_in, patch.object(
+ backend, "copy_out", wraps=backend.copy_out
+ ) as copy_out:
+
+ with backend.open(target_path, "w") as f:
+ f.write("text")
+
+ copy_out.assert_not_called()
+ copy_in.assert_called_once_with(ANY, target_path)
+
+ self.assertTrue(backend.path_exists(target_path))
+
+ copy_in.reset_mock()
+ copy_out.reset_mock()
+
+ with backend.open(target_path, "r") as f:
+ self.assertEqual(f.read(), "text")
+
+ copy_in.assert_not_called()
+ copy_out.assert_called_once_with(target_path, ANY)
diff --git a/lpbuildd/target/tests/test_lxd.py b/lpbuildd/target/tests/test_lxd.py
index ecf3ddd..73f4d94 100644
--- a/lpbuildd/target/tests/test_lxd.py
+++ b/lpbuildd/target/tests/test_lxd.py
@@ -391,9 +391,10 @@ class TestLXD(TestCase):
lambda wait=False: setattr(container, "status_code", LXD_RUNNING))
files_api = container.api.files
files_api._api_endpoint = f"/1.0/containers/lp-xenial-{arch}/files"
- files_api.session.get.side_effect = FakeSessionGet({
+ existing_files = {
"/etc/hosts": [b"127.0.0.1\tlocalhost\n"],
- })
+ }
+ files_api.session.get.side_effect = FakeSessionGet(existing_files)
processes_fixture = self.useFixture(FakeProcesses())
def fake_sudo(args):
@@ -414,7 +415,13 @@ class TestLXD(TestCase):
processes_fixture.add(lambda _: {}, name="lxc")
processes_fixture.add(
FakeHostname("example", "example.buildd"), name="hostname")
- LXD("1", "xenial", arch).start()
+
+ with mock.patch.object(
+ LXD,
+ "path_exists",
+ side_effect=lambda path: path in existing_files
+ ):
+ LXD("1", "xenial", arch).start()
self.assert_correct_profile()
@@ -515,9 +522,6 @@ class TestLXD(TestCase):
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.session.get.assert_any_call(
- f"/1.0/containers/lp-xenial-{arch}/files",
- params={"path": "/etc/init/mounted-dev.conf"}, stream=True)
self.assertNotIn(
"/etc/init/mounted-dev.override",
[kwargs["params"]["path"]
@@ -551,11 +555,10 @@ class TestLXD(TestCase):
processes_fixture.add(lambda _: {}, name="lxc")
processes_fixture.add(
FakeHostname("example", "example.buildd"), name="hostname")
- LXD("1", "xenial", "amd64").start()
- files_api.session.get.assert_any_call(
- "/1.0/containers/lp-xenial-amd64/files",
- params={"path": "/etc/hosts"}, stream=True)
+ with mock.patch.object(LXD, "path_exists", return_value=False):
+ LXD("1", "xenial", "amd64").start()
+
files_api.post.assert_any_call(
params={"path": "/etc/hosts"},
data=(
@@ -576,7 +579,7 @@ class TestLXD(TestCase):
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({
+ existing_files = {
"/etc/init/mounted-dev.conf": [dedent("""\
start on mounted MOUNTPOINT=/dev
script
@@ -584,11 +587,18 @@ class TestLXD(TestCase):
/sbin/MAKEDEV std fd ppp tun
end script
task
- """).encode("UTF-8")]})
+ """).encode("UTF-8")]}
+ files_api.session.get.side_effect = FakeSessionGet(existing_files)
processes_fixture = self.useFixture(FakeProcesses())
processes_fixture.add(lambda _: {}, name="sudo")
processes_fixture.add(lambda _: {}, name="lxc")
- LXD("1", "trusty", "amd64").start()
+
+ with mock.patch.object(
+ LXD,
+ "path_exists",
+ side_effect=lambda path: path in existing_files
+ ):
+ LXD("1", "trusty", "amd64").start()
files_api.session.get.assert_any_call(
"/1.0/containers/lp-trusty-amd64/files",
