feat: fetch logs from inside the build instance (#87)

* feat: fetch logs from inside the build instance

One could argue that this a bugfix since this was missing behavior from
the existing craft tools. Regardless, this commit updates the instance()
method in the default ProviderService to fetch the logfile from inside
the build instance once its done. The contents of this file are then
emitted so that they can end up in the logs of the "outer" instance.

Fixes #59

craft_application/application.py

@@ -106,16 +106,9 @@ def command_groups(self) -> list[craft_cli.CommandGroup]:
     def log_path(self) -> pathlib.Path | None:
         """Get the path to this process's log file, if any."""
         if self.services.ProviderClass.is_managed():
-            return self._managed_log_path
+            return util.get_managed_logpath(self.app)
         return None
 
-    @property
-    def _managed_log_path(self) -> pathlib.Path:
-        """Get the location of the managed instance's log file."""
-        return pathlib.Path(
-            f"/tmp/{self.app.name}.log"  # noqa: S108 - only applies inside managed instance.
-        )
-
     def add_global_argument(self, argument: craft_cli.GlobalArgument) -> None:
         """Add a global argument to the Application."""
         self._global_arguments.append(argument)

craft_application/services/provider.py

@@ -18,6 +18,7 @@
 
 import contextlib
 import os
+import pathlib
 import sys
 from typing import TYPE_CHECKING
 
@@ -27,10 +28,10 @@
 from craft_providers.lxd import LXDProvider
 from craft_providers.multipass import MultipassProvider
 
+from craft_application import util
 from craft_application.services import base
 
 if TYPE_CHECKING:  # pragma: no cover
-    import pathlib
     from collections.abc import Generator
 
     import craft_providers
@@ -112,8 +113,11 @@ def instance(
                 target=self._app.managed_instance_project_path,  # type: ignore[arg-type]
             )
             emit.debug("Instance launched and working directory mounted")
-            with emit.pause():
-                yield instance
+            try:
+                with emit.pause():
+                    yield instance
+            finally:
+                self._capture_logs_from_instance(instance)
 
     def get_base(
         self,
@@ -187,3 +191,19 @@ def _get_lxd_provider(self) -> LXDProvider:
     def _get_multipass_provider(self) -> MultipassProvider:
         """Get the Multipass provider for this manager."""
         return MultipassProvider()
+
+    def _capture_logs_from_instance(self, instance: craft_providers.Executor) -> None:
+        """Fetch the logfile from inside `instance` and emit its contents."""
+        source_log_path = util.get_managed_logpath(self._app)
+        with instance.temporarily_pull_file(
+            source=source_log_path, missing_ok=True
+        ) as log_path:
+            if log_path:
+                emit.debug("Logs retrieved from managed instance:")
+                with log_path.open() as log_file:
+                    for line in log_file:
+                        emit.debug(":: " + line.rstrip())
+            else:
+                emit.debug(
+                    f"Could not find log file {source_log_path.as_posix()} in instance."
+                )

craft_application/util/__init__.py

@@ -16,6 +16,7 @@
 """Utilities for craft-application."""
 
 from craft_application.util.yaml import safe_yaml_load
+from craft_application.util.paths import get_managed_logpath
 from craft_application.util.platforms import (
     get_host_architecture,
     convert_architecture_deb_to_platform,
@@ -25,4 +26,5 @@
     "safe_yaml_load",
     "get_host_architecture",
     "convert_architecture_deb_to_platform",
+    "get_managed_logpath",
 ]

craft_application/util/paths.py

@@ -0,0 +1,34 @@
+# This file is part of craft_application.
+#
+# Copyright 2023 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU Lesser General Public License version 3, as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
+# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License along
+# with this program.  If not, see <http://www.gnu.org/licenses/>.
+"""Utility functions and helpers related to path handling."""
+from __future__ import annotations
+
+import pathlib
+from typing import TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from craft_application import AppMetadata
+
+
+def get_managed_logpath(app: AppMetadata) -> pathlib.PosixPath:
+    """Get the path to the logfile inside a build instance.
+
+    Note that this always returns a PosixPath, as it refers to a path inside of
+    a Linux-based build instance.
+    """
+    return pathlib.PosixPath(
+        f"/tmp/{app.name}.log"  # noqa: S108 - only applies inside managed instance.
+    )

tests/unit/services/test_provider.py

@@ -14,6 +14,7 @@
 # You should have received a copy of the GNU Lesser General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 """Unit tests for provider service"""
+import pathlib
 from unittest import mock
 
 import craft_providers
@@ -197,3 +198,135 @@ def test_instance(
         emitter.assert_progress("Launching managed .+ instance...", regex=True)
     with check:
         assert spy_pause.call_count == 1
+
+
+@pytest.fixture()
+def setup_fetch_logs_provider(monkeypatch, provider_service, tmp_path):
+    """Return a function that, when called, mocks the provider_service's instance()."""
+
+    def _setup(*, should_have_logfile: bool):
+        """
+        param should_have_logfile: Whether the logfile in the fake "build instance"
+          should exist (True) or not (False).
+        """
+        mock_provider = mock.MagicMock(spec=craft_providers.Provider)
+        monkeypatch.setattr(provider_service, "get_provider", lambda: mock_provider)
+
+        # This ugly call is to mock the "instance" returned by the "launched_environment"
+        # context manager.
+        mock_instance = (
+            mock_provider.launched_environment.return_value.__enter__.return_value
+        )
+        mock_instance.temporarily_pull_file = mock.MagicMock()
+
+        if should_have_logfile:
+            fake_log = tmp_path / "fake.file"
+            fake_log_data = "some\nlog data\nhere"
+            fake_log.write_text(fake_log_data, encoding="utf-8")
+            mock_instance.temporarily_pull_file.return_value.__enter__.return_value = (
+                fake_log
+            )
+        else:
+            mock_instance.temporarily_pull_file.return_value.__enter__.return_value = (
+                None
+            )
+
+        return provider_service
+
+    return _setup
+
+
+def _get_build_info() -> models.BuildInfo:
+    arch = util.get_host_architecture()
+    return models.BuildInfo(
+        platform=arch,
+        build_on=arch,
+        build_for=arch,
+        base=bases.BaseName("ubuntu", "22.04"),
+    )
+
+
+def test_instance_fetch_logs(
+    provider_service, setup_fetch_logs_provider, check, emitter
+):
+    """Test that logs from the build instance are fetched in case of success."""
+
+    # Setup the build instance and pretend the command inside it finished successfully.
+    provider_service = setup_fetch_logs_provider(should_have_logfile=True)
+    with provider_service.instance(
+        build_info=_get_build_info(),
+        work_dir=pathlib.Path(),
+    ) as mock_instance:
+        pass
+
+    # Now check that the logs from the build instance were collected.
+    with check:
+        mock_instance.temporarily_pull_file.assert_called_once_with(
+            source=pathlib.PosixPath("/tmp/testcraft.log"), missing_ok=True
+        )
+
+    expected = [
+        mock.call("debug", "Logs retrieved from managed instance:"),
+        mock.call("debug", ":: some"),
+        mock.call("debug", ":: log data"),
+        mock.call("debug", ":: here"),
+    ]
+
+    with check:
+        emitter.assert_interactions(expected)
+
+
+def test_instance_fetch_logs_error(
+    provider_service, setup_fetch_logs_provider, check, emitter
+):
+    """Test that logs from the build instance are fetched in case of errors."""
+
+    # Setup the build instance and pretend the command inside it finished with error.
+    provider_service = setup_fetch_logs_provider(should_have_logfile=True)
+    with pytest.raises(RuntimeError), provider_service.instance(
+        build_info=_get_build_info(),
+        work_dir=pathlib.Path(),
+    ) as mock_instance:
+        raise RuntimeError("Faking an error in the build instance!")
+
+    # Now check that the logs from the build instance were collected.
+    with check:
+        mock_instance.temporarily_pull_file.assert_called_once_with(
+            source=pathlib.PosixPath("/tmp/testcraft.log"), missing_ok=True
+        )
+
+    expected = [
+        mock.call("debug", "Logs retrieved from managed instance:"),
+        mock.call("debug", ":: some"),
+        mock.call("debug", ":: log data"),
+        mock.call("debug", ":: here"),
+    ]
+
+    with check:
+        emitter.assert_interactions(expected)
+
+
+def test_instance_fetch_logs_missing_file(
+    provider_service, setup_fetch_logs_provider, check, emitter
+):
+    """Test that we handle the case where the logfile is missing."""
+
+    # Setup the build instance and pretend the command inside it finished successfully.
+    provider_service = setup_fetch_logs_provider(should_have_logfile=False)
+    with provider_service.instance(
+        build_info=_get_build_info(),
+        work_dir=pathlib.Path(),
+    ) as mock_instance:
+        pass
+
+    # Now check that the logs from the build instance were *attempted* to be collected.
+    with check:
+        mock_instance.temporarily_pull_file.assert_called_once_with(
+            source=pathlib.PosixPath("/tmp/testcraft.log"), missing_ok=True
+        )
+    expected = [
+        mock.call("debug", "Could not find log file /tmp/testcraft.log in instance."),
+    ]
+
+    with check:
+        emitter.assert_interactions(expected)

tests/unit/util/test_paths.py

@@ -0,0 +1,26 @@
+# This file is part of craft-application.
+#
+# Copyright 2023 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU Lesser General Public License version 3, as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
+# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.
+# See the GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+"""Tests for internal path utilities."""
+import pathlib
+
+from craft_application import util
+
+
+def test_get_managed_logpath(app_metadata):
+    logpath = util.get_managed_logpath(app_metadata)
+
+    assert isinstance(logpath, pathlib.PosixPath)
+    assert str(logpath) == "/tmp/testcraft.log"