Skip to content

Commit

Permalink
fix: don't mount cache directories in lxd base instances (#473)
Browse files Browse the repository at this point in the history
  • Loading branch information
lengau authored Dec 4, 2023
1 parent b885aea commit 0089540
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 15 deletions.
5 changes: 4 additions & 1 deletion craft_providers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -979,6 +979,7 @@ def setup(
*,
executor: Executor,
timeout: Optional[float] = TIMEOUT_UNPREDICTABLE,
mount_cache: bool = True,
) -> None:
"""Prepare base instance for use by the application.
Expand All @@ -994,6 +995,7 @@ def setup(
:param executor: Executor for target container.
:param timeout: Timeout in seconds.
:param mount_cache: If true, mount the cache directories.
:raises BaseCompatibilityError: if instance is incompatible.
:raises BaseConfigurationError: on other unexpected error.
Expand All @@ -1015,7 +1017,8 @@ def setup(

self._update_compatibility_tag(executor=executor)

self._mount_shared_cache_dirs(executor=executor)
if mount_cache:
self._mount_shared_cache_dirs(executor=executor)

self._pre_setup_os(executor=executor)
self._setup_os(executor=executor)
Expand Down
10 changes: 8 additions & 2 deletions craft_providers/lxd/launcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,10 @@ def _create_instance(
)
config_timer = InstanceTimer(base_instance)
config_timer.start()
base_configuration.setup(executor=base_instance)

# The base configuration shouldn't mount cache directories because if
# they get deleted, copying the base instance will fail.
base_configuration.setup(executor=base_instance, mount_cache=False)
_set_timezone(
base_instance,
base_instance.project,
Expand Down Expand Up @@ -219,7 +222,10 @@ def _create_instance(
instance.restart()
else:
instance.start()
base_configuration.wait_until_ready(executor=instance)
# Warmup the instance to ensure we have everything set up.
# There are cases where setup won't do everything to the base instance that
# warmup does to the final instance (such as mounting cache directories).
base_configuration.warmup(executor=instance)


def _ensure_project_exists(
Expand Down
9 changes: 8 additions & 1 deletion tests/integration/lxd/test_launcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,12 @@ def _base_instance(


@pytest.fixture()
def base_configuration():
def base_configuration(tmp_path):
"""Returns a simple base configuration."""
return ubuntu.BuilddBase(
alias=ubuntu.BuilddBaseAlias.JAMMY,
hostname="test-hostname",
cache_path=tmp_path / "cache",
)


Expand Down Expand Up @@ -224,6 +225,10 @@ def test_launch_use_base_instance(
# fingerprint the base instance
base_instance.start()
base_instance.execute_run(["touch", "/base-instance"])
assert (
base_instance.execute_run(["ls", "/root/.cache/pip/"], check=False).returncode
== 2
)
base_instance.stop()

# delete the instance so a new instance is created from the base instance
Expand Down Expand Up @@ -270,6 +275,8 @@ def test_launch_use_base_instance(

assert instance_hostname == instance.instance_name

instance.execute_run(["ls", "/root/.cache/pip/"], check=True)


def test_launch_create_base_instance_with_correct_image_description(
base_configuration, get_base_instance, instance_name
Expand Down
22 changes: 11 additions & 11 deletions tests/unit/lxd/test_launcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ def test_launch_no_base_instance(
assert mock_base_configuration.mock_calls == [
call.get_command_environment(),
call.setup(executor=fake_instance),
call.wait_until_ready(executor=fake_instance),
call.warmup(executor=fake_instance),
]


Expand Down Expand Up @@ -247,8 +247,8 @@ def test_launch_use_base_instance(
assert mock_base_configuration.mock_calls == [
call.get_command_environment(),
call.get_command_environment(),
call.setup(executor=fake_base_instance),
call.wait_until_ready(executor=fake_instance),
call.setup(executor=fake_base_instance, mount_cache=False),
call.warmup(executor=fake_instance),
]


Expand Down Expand Up @@ -527,8 +527,8 @@ def test_launch_existing_base_instance_invalid(
assert mock_base_configuration.mock_calls == [
call.get_command_environment(),
call.get_command_environment(),
call.setup(executor=fake_base_instance),
call.wait_until_ready(executor=fake_instance),
call.setup(executor=fake_base_instance, mount_cache=False),
call.warmup(executor=fake_instance),
]


Expand Down Expand Up @@ -609,7 +609,7 @@ def test_launch_all_opts(
assert mock_base_configuration.mock_calls == [
call.get_command_environment(),
call.setup(executor=fake_instance),
call.wait_until_ready(executor=fake_instance),
call.warmup(executor=fake_instance),
]


Expand Down Expand Up @@ -703,7 +703,7 @@ def test_launch_create_project(
assert mock_base_configuration.mock_calls == [
call.get_command_environment(),
call.setup(executor=fake_instance),
call.wait_until_ready(executor=fake_instance),
call.warmup(executor=fake_instance),
]


Expand Down Expand Up @@ -846,7 +846,7 @@ def test_launch_with_existing_instance_incompatible_with_auto_clean(
call.get_command_environment(),
call.warmup(executor=fake_instance),
call.setup(executor=fake_instance),
call.wait_until_ready(executor=fake_instance),
call.warmup(executor=fake_instance),
]


Expand Down Expand Up @@ -940,7 +940,7 @@ def test_launch_with_existing_ephemeral_instance(
assert mock_base_configuration.mock_calls == [
call.get_command_environment(),
call.setup(executor=fake_instance),
call.wait_until_ready(executor=fake_instance),
call.warmup(executor=fake_instance),
]


Expand Down Expand Up @@ -1133,8 +1133,8 @@ def test_use_snapshots_deprecated(
assert mock_base_configuration.mock_calls == [
call.get_command_environment(),
call.get_command_environment(),
call.setup(executor=fake_base_instance),
call.wait_until_ready(executor=fake_instance),
call.setup(executor=fake_base_instance, mount_cache=False),
call.warmup(executor=fake_instance),
]


Expand Down

0 comments on commit 0089540

Please sign in to comment.