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

feat: run apt upgrade in overlay #875

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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: 2 additions & 0 deletions craft_parts/executor/executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ def prologue(self) -> None:
self._project_info.overlay_mount_dir, self._project_info
)
ctx.refresh_packages_list()
logger.info("Upgrading packages in the overlay base")
ctx.upgrade_packages()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@linostar have you tested this with Rockcraft? I'm don't remember the details but I'm almost sure that the contents of this PackageCacheMount layer do not end up in the final lifecycle payload. Typically we update the packages list and then mount a layer on top of this one to install overlay-packages and execute the overlay-script.


callbacks.run_prologue(self._project_info)

Expand Down
4 changes: 4 additions & 0 deletions craft_parts/lifecycle_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,10 @@ def refresh_packages_list(self) -> None:
"""
packages.Repository.refresh_packages_list()

def upgrade_packages(self) -> None:
"""Upgrade all installed packages."""
packages.Repository.upgrade_packages()

def plan(
self,
target_step: Step,
Expand Down
12 changes: 12 additions & 0 deletions craft_parts/overlays/overlay_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,14 @@ def refresh_packages_list(self) -> None:
packages.Repository.refresh_packages_list.cache_clear() # type: ignore[attr-defined]
chroot.chroot(mount_dir, packages.Repository.refresh_packages_list)

def upgrade_packages(self) -> None:
"""Upgrade all packages in the overlay system."""
if not self._overlay_fs:
raise RuntimeError("overlay filesystem not mounted")

mount_dir = self._project_info.overlay_mount_dir
chroot.chroot(mount_dir, packages.Repository.upgrade_packages)

def download_packages(self, package_names: list[str]) -> None:
"""Download packages and populate the overlay package cache.

Expand Down Expand Up @@ -226,6 +234,10 @@ def refresh_packages_list(self) -> None:
"""Update the list of available packages in the overlay system."""
self._overlay_manager.refresh_packages_list()

def upgrade_packages(self) -> None:
"""Upgrade all packages in the overlay system."""
self._overlay_manager.upgrade_packages()

def download_packages(self, package_names: list[str]) -> None:
"""Download the specified packages to the local system.

Expand Down
13 changes: 13 additions & 0 deletions craft_parts/packages/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,15 @@ def refresh_packages_list(cls) -> None:
should be raised.
"""

@classmethod
@abc.abstractmethod
def upgrade_packages(cls) -> None:
"""Upgrade the packages in the repository.

If upgrade is not possible :class:`PackageUpgradeError`
should be raised.
"""

@classmethod
@abc.abstractmethod
def download_packages(cls, package_names: list[str]) -> None:
Expand Down Expand Up @@ -199,6 +208,10 @@ def get_packages_for_source_type(cls, source_type: str) -> set[str]: # noqa: AR
def refresh_packages_list(cls) -> None:
"""Refresh the build packages cache."""

@classmethod
def upgrade_packages(cls) -> None:
"""Upgrade the packages."""

@classmethod
def download_packages(cls, package_names: list[str]) -> None:
"""Download the specified packages to the local package cache."""
Expand Down
19 changes: 19 additions & 0 deletions craft_parts/packages/deb.py
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,25 @@ def refresh_packages_list( # pyright: ignore[reportIncompatibleMethodOverride]
"failed to run apt update"
) from call_error

@classmethod
def upgrade_packages( # pyright: ignore[reportIncompatibleMethodOverride]
cls,
) -> None:
"""Upgrade all packages."""
# Return early when testing.
if os.geteuid() != 0:
logger.warning("Packages not upgraded, not running as superuser.")
return

try:
cmd = ["apt-get", "upgrade", "-y"]
logger.debug("Executing: %s", cmd)
process_run(cmd)
except subprocess.CalledProcessError as call_error:
raise errors.PackageUpgradeError(
"failed to run apt upgrade"
) from call_error

@classmethod
@_apt_cache_wrapper
def _check_if_all_packages_installed(cls, package_names: list[str]) -> bool:
Expand Down
13 changes: 13 additions & 0 deletions craft_parts/packages/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,19 @@ def __init__(self, message: str) -> None:
super().__init__(brief=brief)


class PackageUpgradeError(PackagesError):
"""Failed to upgrade the available packages.

:param message: The error message.
"""

def __init__(self, message: str) -> None:
self.message = message
brief = f"Failed to upgrade the packages: {message}."

super().__init__(brief=brief)


class PackageBroken(PackagesError):
"""Package has unmet dependencies.

Expand Down
4 changes: 4 additions & 0 deletions craft_parts/packages/yum.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ def refresh_packages_list(cls) -> None:
itself, no separate previous action is needed.
"""

@classmethod
def upgrade_packages(cls) -> None:
"""Upgrade the packages in the repository."""

@classmethod
def _check_if_all_packages_installed(cls, package_names: list[str]) -> bool:
"""Check if all given packages are installed.
Expand Down
1 change: 1 addition & 0 deletions docs/common/craft-parts/craft-parts.wordlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ PackageCacheMount
PackageFetchError
PackageListRefreshError
PackageNotFound
PackageUpgradeError
PackagesDownloadError
PackagesError
PackagesNotFound
Expand Down
9 changes: 9 additions & 0 deletions tests/unit/features/overlay/test_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,20 @@ def configure_overlay(overlay_dir: Path, project_info: ProjectInfo) -> None:
def refresh_packages_list() -> None:
call_order.append("refresh_packages_list")

def upgrade_packages() -> None:
call_order.append("upgrade_packages")

callbacks.register_configure_overlay(configure_overlay)
mocker.patch.object(
overlays.PackageCacheMount,
"refresh_packages_list",
side_effect=refresh_packages_list,
)
mocker.patch.object(
overlays.PackageCacheMount,
"upgrade_packages",
side_effect=upgrade_packages,
)

p1 = Part("p1", {"plugin": "nil", "overlay-packages": ["fake-pkg"]})
info = ProjectInfo(application_name="test", cache_dir=new_dir, custom="custom")
Expand All @@ -90,4 +98,5 @@ def refresh_packages_list() -> None:
assert call_order == [
f"configure_overlay: {info.overlay_mount_dir} custom",
"refresh_packages_list",
"upgrade_packages",
]
1 change: 1 addition & 0 deletions tests/unit/packages/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ def test_abstract_methods(self):
"install_packages",
"fetch_stage_packages",
"refresh_packages_list",
"upgrade_packages",
"unpack_stage_packages",
}

Expand Down
17 changes: 17 additions & 0 deletions tests/unit/packages/test_deb.py
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,23 @@ def test_refresh_packages_list_fails(self, fake_deb_run, mocker):

assert fake_deb_run.mock_calls == [call(["apt-get", "update"])]

def test_upgrade_packages(self, fake_deb_run, mocker):
mocker.patch("os.geteuid", return_value=0)
deb.Ubuntu.upgrade_packages()

assert fake_deb_run.mock_calls == [call(["apt-get", "upgrade", "-y"])]

def test_upgrade_packages_fails(self, fake_deb_run, mocker):
mocker.patch("os.geteuid", return_value=0)
fake_deb_run.side_effect = CalledProcessError(
returncode=1, cmd=["apt-get", "upgrade", "-y"]
)

with pytest.raises(errors.PackageUpgradeError):
deb.Ubuntu.upgrade_packages()

assert fake_deb_run.mock_calls == [call(["apt-get", "upgrade", "-y"])]


@pytest.mark.parametrize(
("source_type", "pkgs"),
Expand Down
Loading