Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion craft_application/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
"craft_parts",
"craft_providers",
"craft_store",
"craft_application.remote",
"craft_application",
tigarmo marked this conversation as resolved.
Show resolved Hide resolved
}
)

Expand Down
52 changes: 31 additions & 21 deletions craft_application/fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"""Utilities to interact with the fetch-service."""
import contextlib
import io
import logging
import os
import pathlib
import shlex
Expand All @@ -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:
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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).
"""
Expand Down Expand Up @@ -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"),
Expand All @@ -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'

Expand All @@ -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:
Expand Down Expand Up @@ -461,6 +460,7 @@ def _obtain_certificate() -> tuple[pathlib.Path, pathlib.Path]:
"4096",
],
check=True,
capture_output=True,
)

subprocess.run(
Expand All @@ -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
Expand All @@ -497,6 +498,7 @@ def _obtain_certificate() -> tuple[pathlib.Path, pathlib.Path]:
cert_tmp,
],
check=True,
capture_output=True,
)

cert_tmp.rename(cert)
Expand All @@ -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
)
1 change: 1 addition & 0 deletions craft_application/services/fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down
1 change: 1 addition & 0 deletions docs/reference/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
tigarmo marked this conversation as resolved.
Show resolved Hide resolved

Commands
========
Expand Down
28 changes: 27 additions & 1 deletion tests/integration/services/test_fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion tests/unit/test_application.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 21 additions & 10 deletions tests/unit/test_fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ def test_start_service(mocker, tmp_path):
"F6ECB3762474EDA9D21B7022871920D1991BC93C",
],
text=True,
stderr=subprocess.PIPE,
)

assert mock_obtain_certificate.called
Expand Down Expand Up @@ -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,
),
]

Expand Down
Loading