Merge pull request #474 from cjp256/lxd-checks

charmcraft: perform LXD checks before use (CRAFT-420)

charmcraft/providers.py

@@ -27,16 +27,10 @@
 
 from craft_providers import Executor, bases, lxd
 from craft_providers.actions import snap_installer
-from craft_providers.lxd import LXDInstallationError
-from craft_providers.lxd import installer as lxd_installer
-from craft_providers.lxd.remotes import configure_buildd_image_remote
 
 from charmcraft.cmdbase import CommandError
 from charmcraft.config import Base
-from charmcraft.env import (
-    get_managed_environment_log_path,
-    get_managed_environment_project_path,
-)
+from charmcraft.env import get_managed_environment_log_path, get_managed_environment_project_path
 from charmcraft.utils import confirm_with_user, get_host_architecture
 
 logger = logging.getLogger(__name__)
@@ -112,26 +106,29 @@ def ensure_provider_is_available() -> None:
 
     :raises CommandError: if provider is not available.
     """
-    if is_provider_available():
-        return
-
-    if confirm_with_user(
-        "LXD is required, but not installed. Do you wish to install LXD "
-        "and configure it with the defaults?",
-        default=False,
-    ):
-        try:
-            lxd_installer.install()
-        except LXDInstallationError as error:
+    if not is_provider_available():
+        if confirm_with_user(
+            "LXD is required, but not installed. Do you wish to install LXD "
+            "and configure it with the defaults?",
+            default=False,
+        ):
+            try:
+                lxd.install()
+            except lxd.LXDInstallationError as error:
+                raise CommandError(
+                    "Failed to install LXD. Visit https://snapcraft.io/lxd for "
+                    "instructions on how to install the LXD snap for your distribution"
+                ) from error
+        else:
             raise CommandError(
-                "Failed to install LXD. Please visit https://snapcraft.io/lxd for "
+                "LXD is required, but not installed. Visit https://snapcraft.io/lxd for "
                 "instructions on how to install the LXD snap for your distribution"
-            ) from error
-    else:
-        raise CommandError(
-            "LXD is required, but not installed. Please visit https://snapcraft.io/lxd for "
-            "instructions on how to install the LXD snap for your distribution"
-        )
+            )
+
+    try:
+        lxd.ensure_lxd_is_ready()
+    except lxd.LXDError as error:
+        raise CommandError(str(error))
 
 
 def is_base_providable(base: Base) -> Tuple[bool, Union[str, None]]:
@@ -216,7 +213,7 @@ def is_provider_available() -> bool:
 
     :returns: True if installed.
     """
-    return lxd_installer.is_installed()
+    return lxd.is_installed()
 
 
 @contextlib.contextmanager
@@ -250,7 +247,7 @@ def launched_environment(
     )
 
     environment = get_command_environment()
-    image_remote = configure_buildd_image_remote()
+    image_remote = lxd.configure_buildd_image_remote()
     base_configuration = CharmcraftBuilddBaseConfiguration(
         alias=alias, environment=environment, hostname=instance_name
     )

requirements-dev.txt

@@ -3,13 +3,13 @@ attrs==21.2.0
 black==21.7b0
 certifi==2021.5.30
 cffi==1.14.6
-charset-normalizer==2.0.3
+charset-normalizer==2.0.4
 click==8.0.1
 coverage==5.5
 craft-parts @ https://github.com/canonical/craft-parts/archive/refs/tags/v1.0-alpha.3.tar.gz
-craft-providers==1.0.1
+craft-providers==1.0.2
 flake8==3.9.2
-humanize==3.10.0
+humanize==3.11.0
 idna==3.2
 iniconfig==1.1.1
 Jinja2==3.0.1
@@ -51,6 +51,6 @@ six==1.16.0
 snowballstemmer==2.1.0
 tabulate==0.8.9
 toml==0.10.2
-tomli==1.1.0
+tomli==1.2.0
 typing-extensions==3.10.0.0
 urllib3==1.26.6

requirements.txt

@@ -2,10 +2,10 @@ appdirs==1.4.4
 attrs==21.2.0
 certifi==2021.5.30
 cffi==1.14.6
-charset-normalizer==2.0.3
+charset-normalizer==2.0.4
 craft-parts @ https://github.com/canonical/craft-parts/archive/refs/tags/v1.0-alpha.3.tar.gz
-craft-providers==1.0.1
-humanize==3.10.0
+craft-providers==1.0.2
+humanize==3.11.0
 idna==3.2
 Jinja2==3.0.1
 jsonschema==3.2.0

tests/test_providers.py

@@ -24,7 +24,7 @@
 import pytest
 from craft_providers import Executor, bases
 from craft_providers.actions import snap_installer
-from craft_providers.lxd import LXDInstallationError
+from craft_providers.lxd import LXDError, LXDInstallationError
 
 from charmcraft import providers
 from charmcraft.cmdbase import CommandError
@@ -40,7 +40,7 @@ def bypass_buildd_base_setup(monkeypatch):
 @pytest.fixture()
 def mock_configure_buildd_image_remote():
     with mock.patch(
-        "charmcraft.providers.configure_buildd_image_remote",
+        "charmcraft.providers.lxd.configure_buildd_image_remote",
         return_value="buildd-remote",
     ) as mock_remote:
         yield mock_remote
@@ -73,22 +73,30 @@ def mock_lxc(monkeypatch):
 
 
 @pytest.fixture
-def mock_lxd(monkeypatch):
-    with mock.patch("charmcraft.providers.lxd", autospec=True) as mock_lxd:
-        yield mock_lxd
+def mock_lxd_launch(monkeypatch):
+    with mock.patch("charmcraft.providers.lxd.launch", autospec=True) as mock_lxd_launch:
+        yield mock_lxd_launch
+
+
+@pytest.fixture(autouse=True)
+def mock_lxd_ensure_lxd_is_ready():
+    with mock.patch(
+        "charmcraft.providers.lxd.ensure_lxd_is_ready", return_value=None
+    ) as mock_is_ready:
+        yield mock_is_ready
 
 
 @pytest.fixture(autouse=True)
 def mock_lxd_is_installed():
     with mock.patch(
-        "charmcraft.providers.lxd_installer.is_installed", return_value=True
+        "charmcraft.providers.lxd.is_installed", return_value=True
     ) as mock_is_installed:
         yield mock_is_installed
 
 
 @pytest.fixture()
 def mock_lxd_install():
-    with mock.patch("charmcraft.providers.lxd_installer.install") as mock_install:
+    with mock.patch("charmcraft.providers.lxd.install") as mock_install:
         yield mock_install
 
 
@@ -313,7 +321,7 @@ def test_ensure_provider_is_available_errors_when_user_declines(
     with pytest.raises(
         CommandError,
         match=re.escape(
-            "LXD is required, but not installed. Please visit https://snapcraft.io/lxd for "
+            "LXD is required, but not installed. Visit https://snapcraft.io/lxd for "
             "instructions on how to install the LXD snap for your distribution"
         ),
     ):
@@ -338,7 +346,7 @@ def test_ensure_provider_is_available_errors_when_lxd_install_fails(
     with pytest.raises(
         CommandError,
         match=re.escape(
-            "Failed to install LXD. Please visit https://snapcraft.io/lxd for "
+            "Failed to install LXD. Visit https://snapcraft.io/lxd for "
             "instructions on how to install the LXD snap for your distribution"
         ),
     ) as exc_info:
@@ -354,6 +362,24 @@ def test_ensure_provider_is_available_errors_when_lxd_install_fails(
     assert exc_info.value.__cause__ == mock_lxd_install.side_effect
 
 
+def test_ensure_provider_is_available_errors_when_lxd_not_ready(
+    mock_confirm_with_user, mock_lxd_is_installed, mock_lxd_install, mock_lxd_ensure_lxd_is_ready
+):
+    mock_confirm_with_user.return_value = True
+    mock_lxd_is_installed.return_value = True
+    mock_lxd_ensure_lxd_is_ready.side_effect = LXDError(
+        brief="some error", details="some details", resolution="some resolution"
+    )
+
+    with pytest.raises(
+        CommandError,
+        match=re.escape("some error\nsome details\nsome resolution"),
+    ) as exc_info:
+        providers.ensure_provider_is_available()
+
+    assert exc_info.value.__cause__ == mock_lxd_install.side_effect
+
+
 def test_get_command_environment_minimal(monkeypatch):
     monkeypatch.setenv("IGNORE_ME", "or-im-failing")
     monkeypatch.setenv("PATH", "not-using-host-path")
@@ -453,9 +479,10 @@ def test_is_base_providable(
 
 
 @pytest.mark.parametrize("is_installed", [True, False])
-def test_is_provider_available(is_installed):
-    with mock.patch("charmcraft.providers.lxd_installer.is_installed", return_value=is_installed):
-        assert providers.is_provider_available() == is_installed
+def test_is_provider_available(is_installed, mock_lxd_is_installed):
+    mock_lxd_is_installed.return_value = is_installed
+
+    assert providers.is_provider_available() == is_installed
 
 
 @pytest.mark.parametrize(
@@ -466,7 +493,7 @@ def test_launched_environment(
     channel,
     alias,
     mock_configure_buildd_image_remote,
-    mock_lxd,
+    mock_lxd_launch,
     monkeypatch,
     tmp_path,
     mock_path,
@@ -491,8 +518,8 @@ def test_launched_environment(
         ) as instance:
             assert instance is not None
             assert mock_configure_buildd_image_remote.mock_calls == [mock.call()]
-            assert mock_lxd.mock_calls == [
-                mock.call.launch(
+            assert mock_lxd_launch.mock_calls == [
+                mock.call(
                     name="charmcraft-test-charm-445566-1-2-host-arch",
                     base_configuration=mock_base_config.return_value,
                     image_name=channel,
@@ -504,9 +531,7 @@ def test_launched_environment(
                     project="charmcraft",
                     remote="local",
                 ),
-                mock.call.launch().mount(
-                    host_source=mock_path, target=pathlib.Path("/root/project")
-                ),
+                mock.call().mount(host_source=mock_path, target=pathlib.Path("/root/project")),
             ]
             assert mock_base_config.mock_calls == [
                 call(
@@ -516,16 +541,16 @@ def test_launched_environment(
                 )
             ]
 
-            mock_lxd.reset_mock()
+            mock_lxd_launch.reset_mock()
 
-        assert mock_lxd.mock_calls == [
-            mock.call.launch().unmount_all(),
-            mock.call.launch().stop(),
+        assert mock_lxd_launch.mock_calls == [
+            mock.call().unmount_all(),
+            mock.call().stop(),
         ]
 
 
 def test_launched_environment_unmounts_and_stops_after_error(
-    mock_configure_buildd_image_remote, mock_lxd, monkeypatch, tmp_path
+    mock_configure_buildd_image_remote, mock_lxd_launch, monkeypatch, tmp_path
 ):
     monkeypatch.setattr(providers, "get_host_architecture", lambda: "host-arch")
     base = Base(name="ubuntu", channel="20.04", architectures=["host-arch"])
@@ -541,10 +566,10 @@ def test_launched_environment_unmounts_and_stops_after_error(
                 lxd_project="charmcraft",
                 lxd_remote="local",
             ):
-                mock_lxd.reset_mock()
+                mock_lxd_launch.reset_mock()
                 raise RuntimeError("this is a test")
 
-    assert mock_lxd.mock_calls == [
-        mock.call.launch().unmount_all(),
-        mock.call.launch().stop(),
+    assert mock_lxd_launch.mock_calls == [
+        mock.call().unmount_all(),
+        mock.call().stop(),
     ]