launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #28019
[Merge] ~cjwatson/lpcraft:isolate-from-project-directory into lpcraft:main
Colin Watson has proposed merging ~cjwatson/lpcraft:isolate-from-project-directory into lpcraft:main.
Commit message:
Isolate jobs from the project directory
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/lpcraft/+git/lpcraft/+merge/414585
Simply mounting the project directory in the container encourages jobs to write temporary files to the project directory, which makes it far too easy to leak files between jobs unintentionally or to corrupt the project directory. It's better to isolate jobs as cleanly as possible, so give each job a fresh copy of the project directory instead.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/lpcraft:isolate-from-project-directory into lpcraft:main.
diff --git a/.launchpad.yaml b/.launchpad.yaml
index 290429c..1f1397b 100644
--- a/.launchpad.yaml
+++ b/.launchpad.yaml
@@ -12,3 +12,5 @@ jobs:
series: focal
architectures: amd64
plugin: pyproject-build
+ output:
+ paths: ["dist/*.whl"]
diff --git a/lpcraft/providers/_lxd.py b/lpcraft/providers/_lxd.py
index 677f97b..51fd266 100644
--- a/lpcraft/providers/_lxd.py
+++ b/lpcraft/providers/_lxd.py
@@ -8,14 +8,18 @@ __all__ = [
]
import re
+import subprocess
from contextlib import contextmanager
from pathlib import Path
-from typing import Generator, List, Protocol
+from typing import Any, Generator, List, Protocol
from craft_cli import emit
from craft_providers import Base, bases, lxd
-from lpcraft.env import get_managed_environment_project_path
+from lpcraft.env import (
+ get_managed_environment_home_path,
+ get_managed_environment_project_path,
+)
from lpcraft.errors import CommandError
from lpcraft.providers._base import Provider
from lpcraft.providers._buildd import (
@@ -189,6 +193,40 @@ class LXDProvider(Provider):
"""
return self.lxd_installer.is_installed()
+ def _internal_execute_run(
+ self,
+ instance: lxd.LXDInstance,
+ instance_name: str,
+ command: List[str],
+ **kwargs: Any,
+ ) -> Any: # LXC.exec has no return type annotation
+ """Execute an internal command using subprocess.run().
+
+ This is like LXDInstance.execute_run(), but we drop down to
+ LXC.exec() for easier testability: this approach means that we don't
+ have to cause all our (many) tests that look at
+ LXDInstance.execute_run's call list to make assertions about
+ internal details of the provider.
+
+ :param instance: Instance to execute in.
+ :param instance_name: Name of instance to execute in.
+ :param command: Command to execute.
+ :param kwargs: Keyword args to pass to subprocess.run().
+
+ :returns: Completed process.
+
+ :raises subprocess.CalledProcessError: if command fails and check is
+ True.
+ """
+ return instance.lxc.exec(
+ instance_name=instance_name,
+ command=command,
+ project=self.lxd_project,
+ remote=self.lxd_remote,
+ runner=subprocess.run,
+ **kwargs,
+ )
+
@contextmanager
def launched_environment(
self,
@@ -238,15 +276,51 @@ class LXDProvider(Provider):
except (bases.BaseConfigurationError, lxd.LXDError) as error:
raise CommandError(str(error)) from error
- instance.mount(
- host_source=project_path,
- target=get_managed_environment_project_path(),
- )
-
try:
+ tmp_project_path = (
+ get_managed_environment_home_path() / "tmp-project"
+ )
+ instance.mount(host_source=project_path, target=tmp_project_path)
+ try:
+ self._internal_execute_run(
+ instance,
+ instance_name,
+ [
+ "rm",
+ "-rf",
+ get_managed_environment_project_path().as_posix(),
+ ],
+ check=True,
+ )
+ self._internal_execute_run(
+ instance,
+ instance_name,
+ [
+ "cp",
+ "-a",
+ tmp_project_path.as_posix(),
+ get_managed_environment_project_path().as_posix(),
+ ],
+ check=True,
+ )
+ except subprocess.CalledProcessError as error:
+ raise CommandError(str(error)) from error
+ finally:
+ instance.unmount(target=tmp_project_path)
+
yield instance
finally:
try:
+ self._internal_execute_run(
+ instance,
+ instance_name,
+ [
+ "rm",
+ "-rf",
+ get_managed_environment_project_path().as_posix(),
+ ],
+ check=True,
+ )
instance.unmount_all()
instance.stop()
except lxd.LXDError as error:
diff --git a/lpcraft/providers/tests/test_lxd.py b/lpcraft/providers/tests/test_lxd.py
index dcc586a..627e85c 100644
--- a/lpcraft/providers/tests/test_lxd.py
+++ b/lpcraft/providers/tests/test_lxd.py
@@ -2,9 +2,10 @@
# GNU General Public License version 3 (see the file LICENSE).
import re
+import subprocess
from pathlib import Path
-from typing import Optional
-from unittest.mock import Mock, call, patch
+from typing import Any, AnyStr, List, Optional
+from unittest.mock import ANY, Mock, call, patch
from craft_providers.bases import BaseConfigurationError, BuilddBaseAlias
from craft_providers.lxd import LXC, LXDError, launch
@@ -320,15 +321,48 @@ class TestLXDProvider(TestCase):
),
call().mount(
host_source=self.mock_path,
- target=Path("/root/project"),
+ target=Path("/root/tmp-project"),
),
+ call().lxc.exec(
+ instance_name=expected_instance_name,
+ command=["rm", "-rf", "/root/project"],
+ project="test-project",
+ remote="test-remote",
+ runner=subprocess.run,
+ check=True,
+ ),
+ call().lxc.exec(
+ instance_name=expected_instance_name,
+ command=[
+ "cp",
+ "-a",
+ "/root/tmp-project",
+ "/root/project",
+ ],
+ project="test-project",
+ remote="test-remote",
+ runner=subprocess.run,
+ check=True,
+ ),
+ call().unmount(target=Path("/root/tmp-project")),
],
mock_launcher.mock_calls,
)
mock_launcher.reset_mock()
self.assertEqual(
- [call().unmount_all(), call().stop()],
+ [
+ call().lxc.exec(
+ instance_name=expected_instance_name,
+ command=["rm", "-rf", "/root/project"],
+ project="test-project",
+ remote="test-remote",
+ runner=subprocess.run,
+ check=True,
+ ),
+ call().unmount_all(),
+ call().stop(),
+ ],
mock_launcher.mock_calls,
)
@@ -367,7 +401,70 @@ class TestLXDProvider(TestCase):
self.assertIs(error, raised.exception.__cause__)
+ def test_launched_environment_unmounts_and_stops_after_copy_error(self):
+ def execute(
+ command: List[str], **kwargs: Any
+ ) -> "subprocess.CompletedProcess[AnyStr]":
+ if command[0] == "cp":
+ raise subprocess.CalledProcessError(1, command)
+ else:
+ return subprocess.CompletedProcess([], 0)
+
+ expected_instance_name = "lpcraft-my-project-12345-focal-amd64"
+ mock_launcher = Mock(spec=launch)
+ provider = self.makeLXDProvider(lxd_launcher=mock_launcher)
+ mock_launcher.return_value.lxc.exec.side_effect = execute
+ with self.assertRaisesRegex(
+ CommandError, r"returned non-zero exit status 1"
+ ):
+ with provider.launched_environment(
+ project_name="my-project",
+ project_path=self.mock_path,
+ series="focal",
+ architecture="amd64",
+ ):
+ pass # pragma: no cover
+
+ self.assertEqual(
+ [
+ ANY,
+ call().mount(
+ host_source=self.mock_path,
+ target=Path("/root/tmp-project"),
+ ),
+ call().lxc.exec(
+ instance_name=expected_instance_name,
+ command=["rm", "-rf", "/root/project"],
+ project="test-project",
+ remote="test-remote",
+ runner=subprocess.run,
+ check=True,
+ ),
+ call().lxc.exec(
+ instance_name=expected_instance_name,
+ command=["cp", "-a", "/root/tmp-project", "/root/project"],
+ project="test-project",
+ remote="test-remote",
+ runner=subprocess.run,
+ check=True,
+ ),
+ call().unmount(target=Path("/root/tmp-project")),
+ call().lxc.exec(
+ instance_name=expected_instance_name,
+ command=["rm", "-rf", "/root/project"],
+ project="test-project",
+ remote="test-remote",
+ runner=subprocess.run,
+ check=True,
+ ),
+ call().unmount_all(),
+ call().stop(),
+ ],
+ mock_launcher.mock_calls,
+ )
+
def test_launched_environment_unmounts_and_stops_after_error(self):
+ expected_instance_name = "lpcraft-my-project-12345-focal-amd64"
mock_launcher = Mock(spec=launch)
provider = self.makeLXDProvider(lxd_launcher=mock_launcher)
with self.assertRaisesRegex(RuntimeError, r"Boom"):
@@ -381,7 +478,18 @@ class TestLXDProvider(TestCase):
raise RuntimeError("Boom")
self.assertEqual(
- [call().unmount_all(), call().stop()],
+ [
+ call().lxc.exec(
+ instance_name=expected_instance_name,
+ command=["rm", "-rf", "/root/project"],
+ project="test-project",
+ remote="test-remote",
+ runner=subprocess.run,
+ check=True,
+ ),
+ call().unmount_all(),
+ call().stop(),
+ ],
mock_launcher.mock_calls,
)
