fix: cleanup UI for fetch-service-related commands (#544)

Add some steps under the "Configuring fetch-service integration :: " progress,
to give some feedback that things are happening without all of the noisy output
from Apt.

Fixes #536

craft_application/application.py

@@ -53,7 +53,7 @@
         "craft_parts",
         "craft_providers",
         "craft_store",
-        "craft_application.remote",
+        "craft_application",
     }
 )
 

craft_application/fetch.py

@@ -16,6 +16,7 @@
 """Utilities to interact with the fetch-service."""
 import contextlib
 import io
+import logging
 import os
 import pathlib
 import shlex
@@ -35,6 +36,8 @@
 from craft_application.models import CraftBaseModel
 from craft_application.util import retry
 
+logger = logging.getLogger(__name__)
+
 
 @dataclass(frozen=True)
 class FetchServiceConfig:
@@ -163,6 +166,7 @@ def start_service() -> subprocess.Popen[str] | None:
             archive_key_id,
         ],
         text=True,
+        stderr=subprocess.PIPE,
     )
     env["FETCH_APT_RELEASE_PUBLIC_KEY"] = archive_key
 
@@ -272,7 +276,7 @@ def create_session(*, strict: bool) -> SessionData:
 def teardown_session(session_data: SessionData) -> dict[str, Any]:
     """Stop and cleanup a running fetch-service session.
 
-    :param SessionData: the data of a previously-created session.
+    :param session_data: the data of a previously-created session.
     :return: A dict containing the session's report (the contents and format
       of this dict are still subject to change).
     """
@@ -345,23 +349,23 @@ def _get_service_base_dir() -> pathlib.Path:
 
 def _install_certificate(instance: craft_providers.Executor) -> None:
 
+    logger.info("Installing certificate")
     # Push the local certificate
     cert, _key = _obtain_certificate()
     instance.push_file(
         source=cert,
         destination=_FETCH_CERT_INSTANCE_PATH,
     )
     # Update the certificates db
-    instance.execute_run(  # pyright: ignore[reportUnknownMemberType]
-        ["/bin/sh", "-c", "/usr/sbin/update-ca-certificates > /dev/null"],
-        check=True,
+    _execute_run(
+        instance, ["/bin/sh", "-c", "/usr/sbin/update-ca-certificates > /dev/null"]
     )
 
 
 def _configure_pip(instance: craft_providers.Executor) -> None:
-    instance.execute_run(  # pyright: ignore[reportUnknownMemberType]
-        ["mkdir", "-p", "/root/.pip"]
-    )
+    logger.info("Configuring pip")
+
+    _execute_run(instance, ["mkdir", "-p", "/root/.pip"])
     pip_config = b"[global]\ncert=/usr/local/share/ca-certificates/local-ca.crt"
     instance.push_file_io(
         destination=pathlib.Path("/root/.pip/pip.conf"),
@@ -376,16 +380,16 @@ def _configure_snapd(instance: craft_providers.Executor, net_info: NetInfo) -> N
     Note: This *must* be called *after* _install_certificate(), to ensure that
     when the snapd restart happens the new cert is there.
     """
-    instance.execute_run(  # pyright: ignore[reportUnknownMemberType]
-        ["systemctl", "restart", "snapd"]
-    )
+    logger.info("Configuring snapd")
+    _execute_run(instance, ["systemctl", "restart", "snapd"])
     for config in ("proxy.http", "proxy.https"):
-        instance.execute_run(  # pyright: ignore[reportUnknownMemberType]
-            ["snap", "set", "system", f"{config}={net_info.http_proxy}"]
+        _execute_run(
+            instance, ["snap", "set", "system", f"{config}={net_info.http_proxy}"]
         )
 
 
 def _configure_apt(instance: craft_providers.Executor, net_info: NetInfo) -> None:
+    logger.info("Configuring Apt")
     apt_config = f'Acquire::http::Proxy "{net_info.http_proxy}";\n'
     apt_config += f'Acquire::https::Proxy "{net_info.http_proxy}";\n'
 
@@ -394,15 +398,10 @@ def _configure_apt(instance: craft_providers.Executor, net_info: NetInfo) -> Non
         content=io.BytesIO(apt_config.encode("utf-8")),
         file_mode="0644",
     )
-    instance.execute_run(  # pyright: ignore[reportUnknownMemberType]
-        ["/bin/rm", "-Rf", "/var/lib/apt/lists"],
-        check=True,
-    )
-    env = cast(dict[str, str | None], net_info.env)
-    with emit.open_stream() as fd:
-        instance.execute_run(  # pyright: ignore[reportUnknownMemberType]
-            ["apt", "update"], env=env, check=True, stdout=fd, stderr=fd
-        )
+    _execute_run(instance, ["/bin/rm", "-Rf", "/var/lib/apt/lists"])
+
+    logger.info("Refreshing Apt package listings")
+    _execute_run(instance, ["apt", "update"])
 
 
 def _get_gateway(instance: craft_providers.Executor) -> str:
@@ -461,6 +460,7 @@ def _obtain_certificate() -> tuple[pathlib.Path, pathlib.Path]:
             "4096",
         ],
         check=True,
+        capture_output=True,
     )
 
     subprocess.run(
@@ -475,6 +475,7 @@ def _obtain_certificate() -> tuple[pathlib.Path, pathlib.Path]:
             key_tmp,
         ],
         check=True,
+        capture_output=True,
     )
 
     # Create a certificate with the key
@@ -497,6 +498,7 @@ def _obtain_certificate() -> tuple[pathlib.Path, pathlib.Path]:
             cert_tmp,
         ],
         check=True,
+        capture_output=True,
     )
 
     cert_tmp.rename(cert)
@@ -521,3 +523,11 @@ def _get_log_filepath() -> pathlib.Path:
     base_dir = _get_service_base_dir()
 
     return base_dir / "craft/fetch-log.txt"
+
+
+def _execute_run(
+    instance: craft_providers.Executor, cmd: list[str]
+) -> subprocess.CompletedProcess[str]:
+    return instance.execute_run(  # pyright: ignore[reportUnknownMemberType, reportUnknownVariableType]
+        cmd, check=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE
+    )

craft_application/services/fetch.py

@@ -106,6 +106,7 @@ def create_session(self, instance: craft_providers.Executor) -> dict[str, str]:
         strict_session = self._session_policy == "strict"
         self._session_data = fetch.create_session(strict=strict_session)
         self._instance = instance
+        emit.progress("Configuring fetch-service integration")
         return fetch.configure_instance(instance, self._session_data)
 
     def teardown_session(self) -> dict[str, typing.Any]:

docs/reference/changelog.rst

@@ -14,6 +14,7 @@ Application
   dict.
 - Fixes a bug where the fetch-service integration would try to spawn the
   fetch-service process when running in managed mode.
+- Cleans up the output from the fetch-service integration.
 
 Commands
 ========

tests/integration/services/test_fetch.py

@@ -27,8 +27,10 @@
 import craft_providers
 import pytest
 from craft_application import errors, fetch, services, util
+from craft_application.application import DEFAULT_CLI_LOGGERS
 from craft_application.models import BuildInfo
 from craft_application.services.fetch import _PROJECT_MANIFEST_MANAGED_PATH
+from craft_cli import EmitterMode, emit
 from craft_providers import bases
 
 
@@ -259,8 +261,16 @@ def lxd_instance(snap_safe_tmp_path, provider_service):
 
 
 def test_build_instance_integration(
-    app_service, lxd_instance, tmp_path, monkeypatch, fake_project, manifest_data_dir
+    app_service,
+    lxd_instance,
+    tmp_path,
+    monkeypatch,
+    fake_project,
+    manifest_data_dir,
+    capsys,
 ):
+    emit.init(EmitterMode.BRIEF, "testcraft", "hi", streaming_brief=True)
+    util.setup_loggers(*DEFAULT_CLI_LOGGERS)
     monkeypatch.chdir(tmp_path)
 
     app_service.setup()
@@ -329,3 +339,19 @@ def test_build_instance_integration(
     assert dependencies["hello"]["type"] == "application/vnd.debian.binary-package"
     assert dependencies["craft-application"]["type"] == "application/x.python.wheel"
     assert dependencies["craft-application"]["component-version"] == "3.0.0"
+
+    # Note: the messages don't show up as
+    # 'Configuring fetch-service integration :: Installing certificate' noqa: ERA001 (commented-out code)
+    # because streaming-brief is disabled in non-terminal runs.
+    expected_err = textwrap.dedent(
+        """\
+        Configuring fetch-service integration
+        Installing certificate
+        Configuring pip
+        Configuring snapd
+        Configuring Apt
+        Refreshing Apt package listings
+        """
+    )
+    _, captured_err = capsys.readouterr()
+    assert expected_err in captured_err

tests/unit/test_application.py

@@ -670,7 +670,7 @@ def test_craft_lib_log_level(app_metadata, fake_services):
         "craft_parts",
         "craft_providers",
         "craft_store",
-        "craft_application.remote",
+        "craft_application",
     ]
 
     # The logging module is stateful and global, so first lets clear the logging level

tests/unit/test_fetch.py

@@ -117,6 +117,7 @@ def test_start_service(mocker, tmp_path):
             "F6ECB3762474EDA9D21B7022871920D1991BC93C",
         ],
         text=True,
+        stderr=subprocess.PIPE,
     )
 
     assert mock_obtain_certificate.called
@@ -243,37 +244,47 @@ def test_configure_build_instance(mocker):
     env = fetch.configure_instance(instance, session_data)
     assert env == expected_env
 
+    default_args = {"check": True, "stdout": subprocess.PIPE, "stderr": subprocess.PIPE}
+
     # Execution calls on the instance
     assert instance.execute_run.mock_calls == [
         call(
             ["/bin/sh", "-c", "/usr/sbin/update-ca-certificates > /dev/null"],
-            check=True,
+            **default_args,
+        ),
+        call(
+            ["mkdir", "-p", "/root/.pip"],
+            **default_args,
+        ),
+        call(
+            ["systemctl", "restart", "snapd"],
+            **default_args,
         ),
-        call(["mkdir", "-p", "/root/.pip"]),
-        call(["systemctl", "restart", "snapd"]),
         call(
             [
                 "snap",
                 "set",
                 "system",
                 f"proxy.http={expected_proxy}",
-            ]
+            ],
+            **default_args,
         ),
         call(
             [
                 "snap",
                 "set",
                 "system",
                 f"proxy.https={expected_proxy}",
-            ]
+            ],
+            **default_args,
+        ),
+        call(
+            ["/bin/rm", "-Rf", "/var/lib/apt/lists"],
+            **default_args,
         ),
-        call(["/bin/rm", "-Rf", "/var/lib/apt/lists"], check=True),
         call(
             ["apt", "update"],
-            env=expected_env,
-            check=True,
-            stdout=mocker.ANY,
-            stderr=mocker.ANY,
+            **default_args,
         ),
     ]