From edae6b2b8450019c9921c5bb6d92ec99fb7c6791 Mon Sep 17 00:00:00 2001 From: James Falcon Date: Mon, 25 Sep 2023 09:05:43 -0500 Subject: [PATCH] Allow installing snaps via package_update_upgrade_install module (#4202) This also includes a major refactoring in how the packagement management code is handled. Changes include: * Backwards compatible change to cc_package_update_upgrade_install schema to allow explicitly specifying the package manager to use * Create PackageManager base class that new package manager classes can inherit from * Allow distros to specify the package managers they support with generic install code to install from any of the supported package managers * Create new snap.py and apt.py classes for anything snap and APT related respectively * Move all APT functionality out of debian.py and into apt.py and update callers appropriately * Pull the packaging related calls out of child distro classes and into `distros/__init__.py` so distro code continues to work once package management code is factored out. * Add and update tests Note that this currently only affects debian and ubuntu, along with apt and snap. Migrating other package managers should be straightforward enough, but can be done in later PRs. --- .../cc_package_update_upgrade_install.py | 8 +- .../schemas/schema-cloud-config-v1.json | 39 ++- cloudinit/distros/__init__.py | 110 ++++++++- cloudinit/distros/debian.py | 174 +------------- .../distros/package_management/__init__.py | 1 + cloudinit/distros/package_management/apt.py | 227 ++++++++++++++++++ .../package_management/package_manager.py | 32 +++ cloudinit/distros/package_management/snap.py | 35 +++ cloudinit/distros/package_management/utils.py | 10 + cloudinit/distros/ubuntu.py | 9 +- cloudinit/subp.py | 2 +- doc/rtd/reference/base_config_reference.rst | 16 +- ...{test_apt.py => test_apt_functionality.py} | 1 + .../test_package_update_upgrade_install.py | 38 +-- tests/unittests/config/test_cc_ntp.py | 9 +- .../test_cc_package_update_upgrade_install.py | 169 +++++++++++++ .../distros/package_management/test_apt.py | 88 +++++++ tests/unittests/distros/test_debian.py | 82 +------ tests/unittests/distros/test_init.py | 70 ++++++ tools/run-container | 2 +- 20 files changed, 841 insertions(+), 281 deletions(-) create mode 100644 cloudinit/distros/package_management/__init__.py create mode 100644 cloudinit/distros/package_management/apt.py create mode 100644 cloudinit/distros/package_management/package_manager.py create mode 100644 cloudinit/distros/package_management/snap.py create mode 100644 cloudinit/distros/package_management/utils.py rename tests/integration_tests/modules/{test_apt.py => test_apt_functionality.py} (99%) create mode 100644 tests/unittests/distros/package_management/test_apt.py diff --git a/cloudinit/config/cc_package_update_upgrade_install.py b/cloudinit/config/cc_package_update_upgrade_install.py index 1c04840a3ebe..a26e001d3115 100644 --- a/cloudinit/config/cc_package_update_upgrade_install.py +++ b/cloudinit/config/cc_package_update_upgrade_install.py @@ -44,6 +44,12 @@ - pwgen - pastebinit - [libpython3.8, 3.8.10-0ubuntu1~20.04.2] + - snap: + - certbot + - [juju, --edge] + - [lxd, --channel=5.15/stable] + - apt: + - mg package_update: true package_upgrade: true package_reboot_if_required: true @@ -96,7 +102,7 @@ def handle(name: str, cfg: Config, cloud: Cloud, args: list) -> None: pkglist = util.get_cfg_option_list(cfg, "packages", []) errors = [] - if update or len(pkglist) or upgrade: + if update or upgrade: try: cloud.distro.update_package_sources() except Exception as e: diff --git a/cloudinit/config/schemas/schema-cloud-config-v1.json b/cloudinit/config/schemas/schema-cloud-config-v1.json index e830808db53a..38e24175b6c9 100644 --- a/cloudinit/config/schemas/schema-cloud-config-v1.json +++ b/cloudinit/config/schemas/schema-cloud-config-v1.json @@ -453,6 +453,21 @@ }, "additionalProperties": true }, + "package_item_definition": { + "oneOf": [ + { + "type": "array", + "items": { + "type": "string" + }, + "minItems": 2, + "maxItems": 2 + }, + { + "type": "string" + } + ] + }, "cc_ansible": { "type": "object", "properties": { @@ -1924,19 +1939,29 @@ "properties": { "packages": { "type": "array", - "description": "A list of packages to install. Each entry in the list can be either a package name or a list with two entries, the first being the package name and the second being the specific package version to install.", + "description": "An array containing either a package specification, or an object consisting of a package manager key having a package specification value . A package specification can be either a package name or a list with two entries, the first being the package name and the second being the specific package version to install.", "items": { "oneOf": [ { - "type": "array", - "items": { - "type": "string" + "type": "object", + "properties": { + "apt": { + "type": "array", + "items": { + "$ref": "#/$defs/package_item_definition" + } + }, + "snap": { + "type": "array", + "items": { + "$ref": "#/$defs/package_item_definition" + } + } }, - "minItems": 2, - "maxItems": 2 + "additionalProperties": false }, { - "type": "string" + "$ref": "#/$defs/package_item_definition" } ] }, diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index d17ceca83d52..67b66940b83d 100644 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -16,11 +16,24 @@ import stat import string import urllib.parse +from collections import defaultdict from io import StringIO -from typing import Any, Mapping, MutableMapping, Optional, Type +from typing import ( + Any, + Dict, + Iterable, + List, + Mapping, + MutableMapping, + Optional, + Set, + Tuple, + Type, +) import cloudinit.net.netops.iproute2 as iproute2 from cloudinit import ( + helpers, importer, net, persistence, @@ -31,6 +44,8 @@ util, ) from cloudinit.distros.networking import LinuxNetworking, Networking +from cloudinit.distros.package_management.package_manager import PackageManager +from cloudinit.distros.package_management.utils import known_package_managers from cloudinit.distros.parsers import hosts from cloudinit.features import ALLOW_EC2_MIRRORS_ON_NON_AWS_INSTANCE_TYPES from cloudinit.net import activators, dhcp, eni, network_state, renderers @@ -126,6 +141,8 @@ def __init__(self, name, cfg, paths): dhcp.Udhcpc, ] self.net_ops = iproute2.Iproute2 + self._runner = helpers.Runners(paths) + self.package_managers: List[PackageManager] = [] def _unpickle(self, ci_pkl_version: int) -> None: """Perform deserialization fixes for Distro.""" @@ -139,9 +156,84 @@ def _unpickle(self, ci_pkl_version: int) -> None: # missing expected instance state otherwise. self.networking = self.networking_cls() - @abc.abstractmethod + def _extract_package_by_manager( + self, pkglist: Iterable + ) -> Tuple[Dict[Type[PackageManager], Set], Set]: + """Transform the generic package list to package by package manager. + + Additionally, include list of generic packages + """ + packages_by_manager = defaultdict(set) + generic_packages: Set = set() + for entry in pkglist: + if isinstance(entry, dict): + for package_manager, package_list in entry.items(): + for definition in package_list: + if isinstance(definition, list): + definition = tuple(definition) + try: + packages_by_manager[ + known_package_managers[package_manager] + ].add(definition) + except KeyError: + LOG.error( + "Cannot install packages under '%s' as it is " + "not a supported package manager!", + package_manager, + ) + elif isinstance(entry, str): + generic_packages.add(entry) + else: + raise ValueError( + "Invalid 'packages' yaml specification. " + "Check schema definition." + ) + return dict(packages_by_manager), generic_packages + def install_packages(self, pkglist): - raise NotImplementedError() + error_message = ( + "Failed to install the following packages: %s. " + "See associated package manager logs for more details." + ) + # If an entry hasn't been included with an explicit package name, + # add it to a 'generic' list of packages + ( + packages_by_manager, + generic_packages, + ) = self._extract_package_by_manager(pkglist) + + # First install packages using package manager(s) + # supported by the distro + uninstalled = [] + for manager in self.package_managers: + to_try = ( + packages_by_manager.get(manager.__class__, set()) + | generic_packages + ) + if not to_try: + continue + uninstalled = manager.install_packages(to_try) + failed = { + pkg for pkg in uninstalled if pkg not in generic_packages + } + if failed: + LOG.error(error_message, failed) + generic_packages = set(uninstalled) + + # Now attempt any specified package managers not explicitly supported + # by distro + for manager, packages in packages_by_manager.items(): + if manager.name in [p.name for p in self.package_managers]: + # We already installed/attempted these; don't try again + continue + uninstalled.extend( + manager.from_config(self._runner, self._cfg).install_packages( + pkglist=packages + ) + ) + + if uninstalled: + LOG.error(error_message, uninstalled) def _write_network(self, settings): """Deprecated. Remove if/when arch and gentoo support renderers.""" @@ -202,11 +294,19 @@ def uses_systemd(): @abc.abstractmethod def package_command(self, command, args=None, pkgs=None): + # Long-term, this method should be removed and callers refactored. + # Very few commands are going to be consistent across all package + # managers. raise NotImplementedError() - @abc.abstractmethod def update_package_sources(self): - raise NotImplementedError() + for manager in self.package_managers: + try: + manager.update_package_sources() + except Exception as e: + LOG.error( + "Failed to update package using %s: %s", manager.name, e + ) def get_primary_arch(self): arch = os.uname()[4] diff --git a/cloudinit/distros/debian.py b/cloudinit/distros/debian.py index bef60743d86b..05eca6f9c695 100644 --- a/cloudinit/distros/debian.py +++ b/cloudinit/distros/debian.py @@ -7,30 +7,17 @@ # Author: Joshua Harlow # # This file is part of cloud-init. See LICENSE file for license information. -import fcntl import logging import os -import time +from typing import List -from cloudinit import distros, helpers, subp, util +from cloudinit import distros, subp, util +from cloudinit.distros.package_management.apt import Apt +from cloudinit.distros.package_management.package_manager import PackageManager from cloudinit.distros.parsers.hostname import HostnameConf -from cloudinit.settings import PER_INSTANCE LOG = logging.getLogger(__name__) -APT_LOCK_WAIT_TIMEOUT = 30 -APT_GET_COMMAND = ( - "apt-get", - "--option=Dpkg::Options::=--force-confold", - "--option=Dpkg::options::=--force-unsafe-io", - "--assume-yes", - "--quiet", -) -APT_GET_WRAPPER = { - "command": "eatmydata", - "enabled": "auto", -} - NETWORK_FILE_HEADER = """\ # This file is generated from information provided by the datasource. Changes # to it will not persist across an instance reboot. To disable cloud-init's @@ -42,19 +29,6 @@ NETWORK_CONF_FN = "/etc/network/interfaces.d/50-cloud-init" LOCALE_CONF_FN = "/etc/default/locale" -# The frontend lock needs to be acquired first followed by the order that -# apt uses. /var/lib/apt/lists is locked independently of that install chain, -# and only locked during update, so you can acquire it either order. -# Also update does not acquire the dpkg frontend lock. -# More context: -# https://github.com/canonical/cloud-init/pull/1034#issuecomment-986971376 -APT_LOCK_FILES = [ - "/var/lib/dpkg/lock-frontend", - "/var/lib/dpkg/lock", - "/var/cache/apt/archives/lock", - "/var/lib/apt/lists/lock", -] - class Distro(distros.Distro): hostname_conf_fn = "/etc/hostname" @@ -75,14 +49,15 @@ class Distro(distros.Distro): } def __init__(self, name, cfg, paths): - distros.Distro.__init__(self, name, cfg, paths) + super().__init__(name, cfg, paths) # This will be used to restrict certain # calls from repeatly happening (when they # should only happen say once per instance...) - self._runner = helpers.Runners(paths) self.osfamily = "debian" self.default_locale = "en_US.UTF-8" self.system_locale = None + self.apt = Apt.from_config(self._runner, cfg) + self.package_managers: List[PackageManager] = [self.apt] def get_locale(self): """Return the default locale if set, else use default locale""" @@ -132,10 +107,6 @@ def apply_locale(self, locale, out_fn=None, keyname="LANG"): # once we've updated the system config, invalidate cache self.system_locale = None - def install_packages(self, pkglist): - self.update_package_sources() - self.package_command("install", pkgs=pkglist) - def _write_network_state(self, *args, **kwargs): _maybe_remove_legacy_eth0() return super()._write_network_state(*args, **kwargs) @@ -186,121 +157,12 @@ def _get_localhost_ip(self): def set_timezone(self, tz): distros.set_etc_timezone(tz=tz, tz_file=self._find_tz_file(tz)) - def _apt_lock_available(self, lock_files=None): - """Determines if another process holds any apt locks. - - If all locks are clear, return True else False. - """ - if lock_files is None: - lock_files = APT_LOCK_FILES - for lock in lock_files: - if not os.path.exists(lock): - # Only wait for lock files that already exist - continue - with open(lock, "w") as handle: - try: - fcntl.lockf(handle, fcntl.LOCK_EX | fcntl.LOCK_NB) - except OSError: - return False - return True - - def _wait_for_apt_command( - self, short_cmd, subp_kwargs, timeout=APT_LOCK_WAIT_TIMEOUT - ): - """Wait for apt install to complete. - - short_cmd: Name of command like "upgrade" or "install" - subp_kwargs: kwargs to pass to subp - """ - start_time = time.time() - LOG.debug("Waiting for apt lock") - while time.time() - start_time < timeout: - if not self._apt_lock_available(): - time.sleep(1) - continue - LOG.debug("apt lock available") - try: - # Allow the output of this to flow outwards (not be captured) - log_msg = "apt-%s [%s]" % ( - short_cmd, - " ".join(subp_kwargs["args"]), - ) - return util.log_time( - logfunc=LOG.debug, - msg=log_msg, - func=subp.subp, - kwargs=subp_kwargs, - ) - except subp.ProcessExecutionError: - # Even though we have already waited for the apt lock to be - # available, it is possible that the lock was acquired by - # another process since the check. Since apt doesn't provide - # a meaningful error code to check and checking the error - # text is fragile and subject to internationalization, we - # can instead check the apt lock again. If the apt lock is - # still available, given the length of an average apt - # transaction, it is extremely unlikely that another process - # raced us when we tried to acquire it, so raise the apt - # error received. If the lock is unavailable, just keep waiting - if self._apt_lock_available(): - raise - LOG.debug("Another process holds apt lock. Waiting...") - time.sleep(1) - raise TimeoutError("Could not get apt lock") - def package_command(self, command, args=None, pkgs=None): - """Run the given package command. - - On Debian, this will run apt-get (unless APT_GET_COMMAND is set). - - command: The command to run, like "upgrade" or "install" - args: Arguments passed to apt itself in addition to - any specified in APT_GET_COMMAND - pkgs: Apt packages that the command will apply to - """ - if pkgs is None: - pkgs = [] - - e = os.environ.copy() - # See: http://manpages.ubuntu.com/manpages/bionic/man7/debconf.7.html - e["DEBIAN_FRONTEND"] = "noninteractive" - - wcfg = self.get_option("apt_get_wrapper", APT_GET_WRAPPER) - cmd = _get_wrapper_prefix( - wcfg.get("command", APT_GET_WRAPPER["command"]), - wcfg.get("enabled", APT_GET_WRAPPER["enabled"]), - ) - - cmd.extend(list(self.get_option("apt_get_command", APT_GET_COMMAND))) - - if args and isinstance(args, str): - cmd.append(args) - elif args and isinstance(args, list): - cmd.extend(args) - - subcmd = command - if command == "upgrade": - subcmd = self.get_option( - "apt_get_upgrade_subcommand", "dist-upgrade" - ) - - cmd.append(subcmd) - - pkglist = util.expand_package_list("%s=%s", pkgs) - cmd.extend(pkglist) - - self._wait_for_apt_command( - short_cmd=command, - subp_kwargs={"args": cmd, "env": e, "capture": False}, - ) - - def update_package_sources(self): - self._runner.run( - "update-sources", - self.package_command, - ["update"], - freq=PER_INSTANCE, - ) + # As of this writing, the only use of `package_command` outside of + # distros calling it within their own classes is calling "upgrade" + if command != "upgrade": + raise RuntimeError(f"Unable to handle {command} command") + self.apt.run_package_command("upgrade") def get_primary_arch(self): return util.get_dpkg_architecture() @@ -340,18 +202,6 @@ def set_keymap(self, layout: str, model: str, variant: str, options: str): self.manage_service("restart", "console-setup") -def _get_wrapper_prefix(cmd, mode): - if isinstance(cmd, str): - cmd = [str(cmd)] - - if util.is_true(mode) or ( - str(mode).lower() == "auto" and cmd[0] and subp.which(cmd[0]) - ): - return cmd - else: - return [] - - def _maybe_remove_legacy_eth0(path="/etc/network/interfaces.d/eth0.cfg"): """Ubuntu cloud images previously included a 'eth0.cfg' that had hard coded content. That file would interfere with the rendered diff --git a/cloudinit/distros/package_management/__init__.py b/cloudinit/distros/package_management/__init__.py new file mode 100644 index 000000000000..da6365a59415 --- /dev/null +++ b/cloudinit/distros/package_management/__init__.py @@ -0,0 +1 @@ +# This file is part of cloud-init. See LICENSE file for license information. diff --git a/cloudinit/distros/package_management/apt.py b/cloudinit/distros/package_management/apt.py new file mode 100644 index 000000000000..3c4164cd2dad --- /dev/null +++ b/cloudinit/distros/package_management/apt.py @@ -0,0 +1,227 @@ +# This file is part of cloud-init. See LICENSE file for license information. +import fcntl +import functools +import logging +import os +import time +from typing import Any, Iterable, List, Mapping, Optional, Sequence, cast + +from cloudinit import helpers, subp, util +from cloudinit.distros.package_management.package_manager import ( + PackageManager, + UninstalledPackages, +) +from cloudinit.settings import PER_INSTANCE + +LOG = logging.getLogger(__name__) + +APT_GET_COMMAND = ( + "apt-get", + "--option=Dpkg::Options::=--force-confold", + "--option=Dpkg::options::=--force-unsafe-io", + "--assume-yes", + "--quiet", +) +# The frontend lock needs to be acquired first followed by the order that +# apt uses. /var/lib/apt/lists is locked independently of that install chain, +# and only locked during update, so you can acquire it either order. +# Also update does not acquire the dpkg frontend lock. +# More context: +# https://github.com/canonical/cloud-init/pull/1034#issuecomment-986971376 +APT_LOCK_FILES = [ + "/var/lib/dpkg/lock-frontend", + "/var/lib/dpkg/lock", + "/var/cache/apt/archives/lock", + "/var/lib/apt/lists/lock", +] +APT_LOCK_WAIT_TIMEOUT = 30 + + +def get_apt_wrapper(cfg: Optional[dict]) -> List[str]: + """Parse the 'apt_get_wrapper' section of cloud-config. + + apt_get_wrapper may be defined in cloud-config: + apt_get_wrapper: + enabled: true + command: ["eatmydata"] + + The function takes the value of "apt_get_wrapper" and returns the list + of arguments to prefix to the apt-get command. + """ + enabled: Optional[str] + command: Optional[Any] + if not cfg: + enabled = "auto" + command = ["eatmydata"] + else: + enabled = cfg.get("enabled") + command = cfg.get("command") + + if isinstance(command, str): + command = [command] + elif not isinstance(command, list): + raise TypeError("apt_wrapper command must be a string or list") + + if util.is_true(enabled) or ( + str(enabled).lower() == "auto" and command and subp.which(command[0]) + ): + return cast(List[str], command) + else: + return [] + + +class Apt(PackageManager): + name = "apt" + + def __init__( + self, + runner: helpers.Runners, + *, + apt_get_wrapper_command: Sequence[str] = (), + apt_get_command: Optional[Sequence[str]] = None, + apt_get_upgrade_subcommand: Optional[str] = None, + ): + super().__init__(runner) + if apt_get_command is None: + apt_get_command = APT_GET_COMMAND + if apt_get_upgrade_subcommand is None: + apt_get_upgrade_subcommand = "dist-upgrade" + self.apt_command = tuple(apt_get_wrapper_command) + tuple( + apt_get_command + ) + + self.apt_get_upgrade_subcommand = apt_get_upgrade_subcommand + self.environment = os.environ.copy() + self.environment["DEBIAN_FRONTEND"] = "noninteractive" + + @classmethod + def from_config(cls, runner: helpers.Runners, cfg: Mapping) -> "Apt": + return Apt( + runner, + apt_get_wrapper_command=get_apt_wrapper( + cfg.get("apt_get_wrapper") + ), + apt_get_command=cfg.get("apt_get_command"), + apt_get_upgrade_subcommand=cfg.get("apt_get_upgrade_subcommand"), + ) + + def update_package_sources(self): + self.runner.run( + "update-sources", + self.run_package_command, + ["update"], + freq=PER_INSTANCE, + ) + + @functools.lru_cache(maxsize=1) + def get_all_packages(self): + resp: str = subp.subp(["apt-cache", "pkgnames"]).stdout + + # Searching the string directly and searching a list are both + # linear searches. Converting to a set takes some extra up front + # time, but resulting searches become binary searches and are much + # faster + return set(resp.splitlines()) + + def get_unavailable_packages(self, pkglist: Iterable[str]): + return [pkg for pkg in pkglist if pkg not in self.get_all_packages()] + + def install_packages(self, pkglist: Iterable) -> UninstalledPackages: + self.update_package_sources() + pkglist = util.expand_package_list("%s=%s", list(pkglist)) + unavailable = self.get_unavailable_packages( + [x.split("=")[0] for x in pkglist] + ) + LOG.debug( + "The following packages were not found by APT so APT will " + "not attempt to install them: %s", + unavailable, + ) + to_install = [p for p in pkglist if p not in unavailable] + if to_install: + self.run_package_command("install", pkgs=to_install) + return unavailable + + def run_package_command(self, command, args=None, pkgs=None): + if pkgs is None: + pkgs = [] + full_command = list(self.apt_command) + + if args and isinstance(args, str): + full_command.append(args) + elif args and isinstance(args, list): + full_command.extend(args) + + if command == "upgrade": + command = self.apt_get_upgrade_subcommand + full_command.append(command) + pkglist = util.expand_package_list("%s=%s", pkgs) + full_command.extend(pkglist) + + self._wait_for_apt_command( + short_cmd=command, + subp_kwargs={ + "args": full_command, + "env": self.environment, + "capture": False, + }, + ) + + def _apt_lock_available(self, lock_files=None): + """Determines if another process holds any apt locks. + + If all locks are clear, return True else False. + """ + if lock_files is None: + lock_files = APT_LOCK_FILES + for lock in lock_files: + if not os.path.exists(lock): + # Only wait for lock files that already exist + continue + with open(lock, "w") as handle: + try: + fcntl.lockf(handle, fcntl.LOCK_EX | fcntl.LOCK_NB) + except OSError: + return False + return True + + def _wait_for_apt_command( + self, short_cmd, subp_kwargs, timeout=APT_LOCK_WAIT_TIMEOUT + ): + """Wait for apt install to complete. + + short_cmd: Name of command like "upgrade" or "install" + subp_kwargs: kwargs to pass to subp + """ + start_time = time.time() + LOG.debug("Waiting for APT lock") + while time.time() - start_time < timeout: + if not self._apt_lock_available(): + time.sleep(1) + continue + LOG.debug("APT lock available") + try: + # Allow the output of this to flow outwards (not be captured) + log_msg = f'apt-{short_cmd} [{" ".join(subp_kwargs["args"])}]' + return util.log_time( + logfunc=LOG.debug, + msg=log_msg, + func=subp.subp, + kwargs=subp_kwargs, + ) + except subp.ProcessExecutionError: + # Even though we have already waited for the apt lock to be + # available, it is possible that the lock was acquired by + # another process since the check. Since apt doesn't provide + # a meaningful error code to check and checking the error + # text is fragile and subject to internationalization, we + # can instead check the apt lock again. If the apt lock is + # still available, given the length of an average apt + # transaction, it is extremely unlikely that another process + # raced us when we tried to acquire it, so raise the apt + # error received. If the lock is unavailable, just keep waiting + if self._apt_lock_available(): + raise + LOG.debug("Another process holds APT lock. Waiting...") + time.sleep(1) + raise TimeoutError("Could not get APT lock") diff --git a/cloudinit/distros/package_management/package_manager.py b/cloudinit/distros/package_management/package_manager.py new file mode 100644 index 000000000000..864555f6a4df --- /dev/null +++ b/cloudinit/distros/package_management/package_manager.py @@ -0,0 +1,32 @@ +# This file is part of cloud-init. See LICENSE file for license information. +from abc import ABC, abstractmethod +from typing import Iterable, List + +from cloudinit import helpers + +UninstalledPackages = List[str] + + +class PackageManager(ABC): + name: str + + def __init__(self, runner: helpers.Runners, **kwargs): + self.runner = runner + + @classmethod + def from_config(cls, runner: helpers.Runners, cfg) -> "PackageManager": + return cls(runner) + + @abstractmethod + def update_package_sources(self): + ... + + @abstractmethod + def install_packages(self, pkglist: Iterable) -> UninstalledPackages: + """Install the given packages. + + Return a list of packages that failed to install. + Overriding classes should NOT raise an exception if packages failed + to install. Instead, log the error and return what couldn't be + installed so other installed package managers may be attempted. + """ diff --git a/cloudinit/distros/package_management/snap.py b/cloudinit/distros/package_management/snap.py new file mode 100644 index 000000000000..92eb1af8f5f7 --- /dev/null +++ b/cloudinit/distros/package_management/snap.py @@ -0,0 +1,35 @@ +# This file is part of cloud-init. See LICENSE file for license information. +import logging +from typing import Iterable, List + +from cloudinit import subp, util +from cloudinit.distros.package_management.package_manager import ( + PackageManager, + UninstalledPackages, +) + +LOG = logging.getLogger(__name__) + + +class Snap(PackageManager): + name = "snap" + + def update_package_sources(self): + pass + + def install_packages(self, pkglist: Iterable) -> UninstalledPackages: + # Snap doesn't provide us with a mechanism to know which packages + # are available or have failed, so install one at a time + pkglist = util.expand_package_list("%s=%s", list(pkglist)) + failed: List[str] = [] + for pkg in pkglist: + try: + subp.subp(["snap", "install"] + pkg.split("=", 1)) + except subp.ProcessExecutionError: + failed.append(pkg) + LOG.info("Failed to 'snap install %s'!", pkg) + return failed + + @staticmethod + def upgrade_packages(): + subp.subp(["snap", "refresh"]) diff --git a/cloudinit/distros/package_management/utils.py b/cloudinit/distros/package_management/utils.py new file mode 100644 index 000000000000..f7a44654bcab --- /dev/null +++ b/cloudinit/distros/package_management/utils.py @@ -0,0 +1,10 @@ +from typing import Dict, Type + +from cloudinit.distros.package_management.apt import Apt +from cloudinit.distros.package_management.package_manager import PackageManager +from cloudinit.distros.package_management.snap import Snap + +known_package_managers: Dict[str, Type[PackageManager]] = { + "apt": Apt, + "snap": Snap, +} diff --git a/cloudinit/distros/ubuntu.py b/cloudinit/distros/ubuntu.py index 2f5c7110e290..f7b1e7b281d6 100644 --- a/cloudinit/distros/ubuntu.py +++ b/cloudinit/distros/ubuntu.py @@ -12,11 +12,12 @@ import copy from cloudinit.distros import PREFERRED_NTP_CLIENTS, debian +from cloudinit.distros.package_management.snap import Snap class Distro(debian.Distro): def __init__(self, name, cfg, paths): - super(Distro, self).__init__(name, cfg, paths) + super().__init__(name, cfg, paths) # Ubuntu specific network cfg locations self.network_conf_fn = { "eni": "/etc/network/interfaces.d/50-cloud-init.cfg", @@ -33,6 +34,12 @@ def __init__(self, name, cfg, paths): "postcmds": True, }, } + self.snap = Snap(self._runner) + self.package_managers.append(self.snap) + + def package_command(self, command, args=None, pkgs=None): + super().package_command(command, args, pkgs) + self.snap.upgrade_packages() @property def preferred_ntp_clients(self): diff --git a/cloudinit/subp.py b/cloudinit/subp.py index 7daedff3fe1a..7624d9f39a46 100644 --- a/cloudinit/subp.py +++ b/cloudinit/subp.py @@ -158,7 +158,7 @@ def subp( update_env=None, status_cb=None, cwd=None, -) -> SubpResult: +): """Run a subprocess. :param args: command to run in a list. [cmd, arg1, arg2...] diff --git a/doc/rtd/reference/base_config_reference.rst b/doc/rtd/reference/base_config_reference.rst index f9b602aabe09..953fc2188e1e 100644 --- a/doc/rtd/reference/base_config_reference.rst +++ b/doc/rtd/reference/base_config_reference.rst @@ -111,8 +111,8 @@ Both keys will be processed independently. - ``paths``: Definitions of common paths used by ``cloud-init``. - + ``cloud_dir``: Defaults to :file:`/var/lib/cloud`. - + ``templates_dir``: Defaults to :file:`/etc/cloud/templates`. + + ``cloud_dir``: Default: :file:`/var/lib/cloud`. + + ``templates_dir``: Default: :file:`/etc/cloud/templates`. - ``distro``: Name of distro being used. - ``default_user``: Defines the default user for the system using the same @@ -147,6 +147,18 @@ Both keys will be processed independently. * ``network-manager``: For ``nmcli connection load``/ ``nmcli connection up``. * ``networkd``: For ``ip link set up``/``ip link set down``. + - ``apt_get_command``: Command used to interact with APT repositories. + Default: ``apt-get``. + - ``apt_get_upgrade_subcommand``: APT subcommand used to upgrade system. + Default: ``dist-upgrade``. + - ``apt_get_wrapper``: Command used to wrap the apt-get command. + + + ``enabled``: Whether to use the specified ``apt_wrapper`` command. + If set to ``auto``, use the command if it exists on the ``PATH``. + Default: ``true``. + + + ``command``: Command used to wrap any ``apt-get`` calls. + Default: ``eatmydata``. Logging keys ------------ diff --git a/tests/integration_tests/modules/test_apt.py b/tests/integration_tests/modules/test_apt_functionality.py similarity index 99% rename from tests/integration_tests/modules/test_apt.py rename to tests/integration_tests/modules/test_apt_functionality.py index f550d310a3c3..7eb6e32d378d 100644 --- a/tests/integration_tests/modules/test_apt.py +++ b/tests/integration_tests/modules/test_apt_functionality.py @@ -1,3 +1,4 @@ +# This file is part of cloud-init. See LICENSE file for license information. """Series of integration tests covering apt functionality.""" import re diff --git a/tests/integration_tests/modules/test_package_update_upgrade_install.py b/tests/integration_tests/modules/test_package_update_upgrade_install.py index c54365b81f4f..1fcc02911230 100644 --- a/tests/integration_tests/modules/test_package_update_upgrade_install.py +++ b/tests/integration_tests/modules/test_package_update_upgrade_install.py @@ -1,15 +1,9 @@ +# This file is part of cloud-init. See LICENSE file for license information. """Integration test for the package update upgrade install module. This test module asserts that packages are upgraded/updated during boot with the ``package_update_upgrade_install`` module. We are also testing if we can install new packages during boot too. - -(This is ported from -``tests/cloud_tests/testcases/modules/package_update_upgrade_install.yaml``.) - -NOTE: the testcase for this looks for the command in history.log as - /usr/bin/apt-get..., which is not how it always appears. it should - instead look for just apt-get... """ import re @@ -22,7 +16,11 @@ #cloud-config packages: - sl - - tree + - apt: + - tree + - snap: + - curl + - postman package_update: true package_upgrade: true """ @@ -31,7 +29,7 @@ @pytest.mark.skipif(not IS_UBUNTU, reason="Uses Apt") @pytest.mark.user_data(USER_DATA) class TestPackageUpdateUpgradeInstall: - def assert_package_installed(self, pkg_out, name, version=None): + def assert_apt_package_installed(self, pkg_out, name, version=None): """Check dpkg-query --show output for matching package name. @param name: package base name @@ -52,25 +50,26 @@ def assert_package_installed(self, pkg_out, name, version=None): version, installed_version, ) - raise AssertionError("Package not installed: %s" % name) + raise AssertionError(f"Package not installed: {name}") - def test_new_packages_are_installed(self, class_client): + def test_apt_packages_are_installed(self, class_client): pkg_out = class_client.execute("dpkg-query --show") - self.assert_package_installed(pkg_out, "sl") - self.assert_package_installed(pkg_out, "tree") + self.assert_apt_package_installed(pkg_out, "sl") + self.assert_apt_package_installed(pkg_out, "tree") - def test_packages_were_updated(self, class_client): + def test_apt_packages_were_updated(self, class_client): out = class_client.execute( "grep ^Commandline: /var/log/apt/history.log" ) - assert ( + assert re.search( "Commandline: /usr/bin/apt-get --option=Dpkg::Options" "::=--force-confold --option=Dpkg::options::=--force-unsafe-io " - "--assume-yes --quiet install sl tree" in out + r"--assume-yes --quiet install (sl|tree) (tree|sl)", + out, ) - def test_packages_were_upgraded(self, class_client): + def test_apt_packages_were_upgraded(self, class_client): """Test cloud-init-output for install & upgrade stuff.""" out = class_client.read_from_file("/var/log/cloud-init-output.log") assert "Setting up tree (" in out @@ -79,3 +78,8 @@ def test_packages_were_upgraded(self, class_client): assert "Building dependency tree..." in out assert "Reading state information..." in out assert "Calculating upgrade..." in out + + def test_snap_packages_are_installed(self, class_client): + output = class_client.execute("snap list") + assert "curl" in output + assert "postman" in output diff --git a/tests/unittests/config/test_cc_ntp.py b/tests/unittests/config/test_cc_ntp.py index a4c8d7891c9c..a9444ec508d2 100644 --- a/tests/unittests/config/test_cc_ntp.py +++ b/tests/unittests/config/test_cc_ntp.py @@ -540,9 +540,12 @@ def test_snappy_system_picks_timesyncd(self, m_which): # we are on ubuntu-core here self.m_snappy.return_value = True - # ubuntu core systems will have timesyncd installed + # ubuntu core systems will have timesyncd installed, so simulate that. + # First None is for the 'eatmydata' check when initializing apt + # when initializing the distro class. The rest represent possible + # finds the various npt services m_which.side_effect = iter( - [None, "/lib/systemd/systemd-timesyncd", None, None, None] + [None, None, "/lib/systemd/systemd-timesyncd", None, None, None] ) distro = "ubuntu" mycloud = self._get_cloud(distro) @@ -606,7 +609,7 @@ def test_ntp_custom_client_overrides_installed_clients( cfg = {"ntp": {"ntp_client": client}} for distro in cc_ntp.distros: # client is not installed - m_which.side_effect = iter([None]) + m_which.return_value = None mycloud = self._get_cloud(distro) with mock.patch.object( mycloud.distro, "install_packages" diff --git a/tests/unittests/config/test_cc_package_update_upgrade_install.py b/tests/unittests/config/test_cc_package_update_upgrade_install.py index 07c5b93224c8..74f11ffe8bb1 100644 --- a/tests/unittests/config/test_cc_package_update_upgrade_install.py +++ b/tests/unittests/config/test_cc_package_update_upgrade_install.py @@ -1,11 +1,180 @@ +# This file is part of cloud-init. See LICENSE file for license information. +from unittest import mock + import pytest +from cloudinit import subp +from cloudinit.config.cc_package_update_upgrade_install import handle from cloudinit.config.schema import ( SchemaValidationError, get_schema, validate_cloudconfig_schema, ) +from cloudinit.subp import SubpResult from tests.unittests.helpers import skipUnlessJsonSchema +from tests.unittests.util import get_cloud + + +@pytest.fixture +def common_mocks(mocker): + mocker.patch("os.path.isfile", return_value=False) + mocker.patch("cloudinit.distros.Distro.update_package_sources") + mocker.patch( + "cloudinit.distros.package_management.apt.Apt.update_package_sources" + ) + mocker.patch( + "cloudinit.distros.package_management.apt.Apt._apt_lock_available", + return_value=True, + ) + + +class TestMultiplePackageManagers: + def test_explicit_apt(self, common_mocks): + def _new_subp(*args, **kwargs): + if args and "apt-cache" in args[0]: + return SubpResult("pkg1\npkg2\npkg3", None) + + cloud = get_cloud("ubuntu") + cfg = {"packages": [{"apt": ["pkg1", "pkg2"]}]} + with mock.patch( + "cloudinit.subp.subp", side_effect=_new_subp + ) as m_subp: + handle("", cfg, cloud, []) + + assert len(m_subp.call_args_list) == 2 + assert m_subp.call_args_list[0] == mock.call(["apt-cache", "pkgnames"]) + + for arg in ["apt-get", "install", "pkg1", "pkg2"]: + assert arg in m_subp.call_args_list[1][1]["args"] + + def test_explicit_apt_version(self, common_mocks): + def _new_subp(*args, **kwargs): + if args and "apt-cache" in args[0]: + return SubpResult("pkg1\npkg2\npkg3", None) + + cloud = get_cloud("ubuntu") + cfg = {"packages": [{"apt": ["pkg1", ["pkg2", "1.2.3"]]}]} + with mock.patch( + "cloudinit.subp.subp", side_effect=_new_subp + ) as m_subp: + handle("", cfg, cloud, []) + + assert len(m_subp.call_args_list) == 2 + assert m_subp.call_args_list[0] == mock.call(["apt-cache", "pkgnames"]) + + for arg in ["apt-get", "install", "pkg1", "pkg2=1.2.3"]: + assert arg in m_subp.call_args_list[1][1]["args"] + + @mock.patch("cloudinit.subp.subp") + def test_explicit_snap(self, m_subp, common_mocks): + cloud = get_cloud("ubuntu") + cfg = {"packages": [{"snap": ["pkg1", "pkg2"]}]} + handle("", cfg, cloud, []) + + assert len(m_subp.call_args_list) == 2 + assert mock.call(["snap", "install", "pkg1"]) in m_subp.call_args_list + assert mock.call(["snap", "install", "pkg2"]) in m_subp.call_args_list + + @mock.patch("cloudinit.subp.subp") + def test_explicit_snap_version(self, m_subp, common_mocks): + cloud = get_cloud("ubuntu") + cfg = {"packages": [{"snap": ["pkg1", ["pkg2", "--edge"]]}]} + handle("", cfg, cloud, []) + + assert len(m_subp.call_args_list) == 2 + assert mock.call(["snap", "install", "pkg1"]) in m_subp.call_args_list + assert ( + mock.call(["snap", "install", "pkg2", "--edge"]) + in m_subp.call_args_list + ) + + def test_combined(self, common_mocks): + """Ensure that pkg1 is installed by snap since it isn't available + in the apt cache, and ensure pkg5 is installed by snap even though it + is available under apt because it is explicitly listed under snap.""" + + def _new_subp(*args, **kwargs): + if args and "apt-cache" in args[0]: + return SubpResult("pkg2\npkg3\npkg5\npkg6", None) + + cloud = get_cloud("ubuntu") + cfg = { + "packages": [ + "pkg1", + {"apt": ["pkg2", ["pkg3", "1.2.3"]]}, + {"snap": ["pkg4", ["pkg5", "--edge"]]}, + "pkg6", + ], + } + with mock.patch( + "cloudinit.subp.subp", side_effect=_new_subp + ) as m_subp: + handle("", cfg, cloud, []) + + assert len(m_subp.call_args_list) == 5 + assert m_subp.call_args_list[0] == mock.call(["apt-cache", "pkgnames"]) + for arg in ["apt-get", "install", "pkg2", "pkg3=1.2.3", "pkg6"]: + assert arg in m_subp.call_args_list[1][1]["args"] + + assert mock.call(["snap", "install", "pkg1"]) in m_subp.call_args_list + assert ( + mock.call(["snap", "install", "pkg5", "--edge"]) + in m_subp.call_args_list + ) + assert mock.call(["snap", "install", "pkg4"]) in m_subp.call_args_list + + def test_error_apt(self, common_mocks): + """Since we have already checked that the package(s) exists in apt, + if we have an error in apt, we don't want to fall through to + additional package installs. + """ + + def _new_subp(*args, **kwargs): + if args and "apt-cache" in args[0]: + return SubpResult("pkg1", None) + if "args" in kwargs and "install" in kwargs["args"]: + raise subp.ProcessExecutionError( + cmd=kwargs["args"], + stdout="dontcare", + stderr="E: Unable to locate package pkg1", + exit_code=100, + ) + + cloud = get_cloud("ubuntu") + cfg = {"packages": ["pkg1"]} + with mock.patch("cloudinit.subp.subp", side_effect=_new_subp): + with pytest.raises(subp.ProcessExecutionError): + handle("", cfg, cloud, []) + + def test_error_snap(self, common_mocks, caplog): + """Since we haven't checked the package(s) existence, we should fall + through to additional package installs. + """ + + def _new_subp(*args, **kwargs): + if args: + if "apt-cache" in args[0]: + return SubpResult("", None) + if "install" in args[0]: + raise subp.ProcessExecutionError( + cmd=args[0], + stdout="dontcare", + stderr='error: snap "pkg1" not found', + exit_code=1, + ) + + cloud = get_cloud("ubuntu") + cfg = {"packages": ["pkg1"]} + with mock.patch("cloudinit.subp.subp", side_effect=_new_subp): + handle("", cfg, cloud, []) + + assert caplog.records[1].levelname == "INFO" + assert caplog.records[1].message == "Failed to 'snap install pkg1'!" + + assert caplog.records[2].levelname == "ERROR" + assert caplog.records[2].message.startswith( + "Failed to install the following packages: ['pkg1']" + ) class TestPackageUpdateUpgradeSchema: diff --git a/tests/unittests/distros/package_management/test_apt.py b/tests/unittests/distros/package_management/test_apt.py new file mode 100644 index 000000000000..6af3ad8ed640 --- /dev/null +++ b/tests/unittests/distros/package_management/test_apt.py @@ -0,0 +1,88 @@ +# This file is part of cloud-init. See LICENSE file for license information. +from itertools import count, cycle +from unittest import mock + +import pytest + +from cloudinit import subp +from cloudinit.distros.package_management.apt import APT_GET_COMMAND, Apt + + +@mock.patch.dict("os.environ", {}, clear=True) +@mock.patch("cloudinit.distros.debian.subp.which", return_value=True) +@mock.patch("cloudinit.distros.debian.subp.subp") +class TestPackageCommand: + @mock.patch( + "cloudinit.distros.package_management.apt.Apt._apt_lock_available", + return_value=True, + ) + def test_simple_command(self, m_apt_avail, m_subp, m_which): + apt = Apt(runner=mock.Mock(), apt_get_wrapper_command=["eatmydata"]) + apt.run_package_command("update") + expected_call = { + "args": ["eatmydata"] + list(APT_GET_COMMAND) + ["update"], + "capture": False, + "env": {"DEBIAN_FRONTEND": "noninteractive"}, + } + assert m_subp.call_args == mock.call(**expected_call) + + @mock.patch( + "cloudinit.distros.package_management.apt.Apt._apt_lock_available", + side_effect=[False, False, True], + ) + @mock.patch("cloudinit.distros.package_management.apt.time.sleep") + def test_wait_for_lock(self, m_sleep, m_apt_avail, m_subp, m_which): + apt = Apt(runner=mock.Mock(), apt_get_wrapper_command=("dontcare",)) + apt._wait_for_apt_command("stub", {"args": "stub2"}) + assert m_sleep.call_args_list == [mock.call(1), mock.call(1)] + assert m_subp.call_args_list == [mock.call(args="stub2")] + + @mock.patch( + "cloudinit.distros.package_management.apt.Apt._apt_lock_available", + return_value=False, + ) + @mock.patch("cloudinit.distros.package_management.apt.time.sleep") + @mock.patch( + "cloudinit.distros.package_management.apt.time.time", + side_effect=count(), + ) + def test_lock_wait_timeout( + self, m_time, m_sleep, m_apt_avail, m_subp, m_which + ): + apt = Apt(runner=mock.Mock(), apt_get_wrapper_command=("dontcare",)) + with pytest.raises(TimeoutError): + apt._wait_for_apt_command("stub", "stub2", timeout=5) + assert m_subp.call_args_list == [] + + @mock.patch( + "cloudinit.distros.package_management.apt.Apt._apt_lock_available", + side_effect=cycle([True, False]), + ) + @mock.patch("cloudinit.distros.package_management.apt.time.sleep") + def test_lock_exception_wait(self, m_sleep, m_apt_avail, m_subp, m_which): + apt = Apt(runner=mock.Mock(), apt_get_wrapper_command=("dontcare",)) + exception = subp.ProcessExecutionError( + exit_code=100, stderr="Could not get apt lock" + ) + m_subp.side_effect = [exception, exception, "return_thing"] + ret = apt._wait_for_apt_command("stub", {"args": "stub2"}) + assert ret == "return_thing" + + @mock.patch( + "cloudinit.distros.package_management.apt.Apt._apt_lock_available", + side_effect=cycle([True, False]), + ) + @mock.patch("cloudinit.distros.package_management.apt.time.sleep") + @mock.patch( + "cloudinit.distros.package_management.apt.time.time", + side_effect=count(), + ) + def test_lock_exception_timeout( + self, m_time, m_sleep, m_apt_avail, m_subp, m_which + ): + apt = Apt(runner=mock.Mock(), apt_get_wrapper_command=("dontcare",)) + m_subp.side_effect = subp.ProcessExecutionError( + exit_code=100, stderr="Could not get apt lock" + ) + with pytest.raises(TimeoutError): + apt._wait_for_apt_command("stub", {"args": "stub2"}, timeout=5) diff --git a/tests/unittests/distros/test_debian.py b/tests/unittests/distros/test_debian.py index c7c5932e7461..85de06d33fdb 100644 --- a/tests/unittests/distros/test_debian.py +++ b/tests/unittests/distros/test_debian.py @@ -1,11 +1,7 @@ # This file is part of cloud-init. See LICENSE file for license information. -from itertools import count, cycle from unittest import mock -import pytest - -from cloudinit import distros, subp, util -from cloudinit.distros.debian import APT_GET_COMMAND, APT_GET_WRAPPER +from cloudinit import distros, util from tests.unittests.helpers import FilesystemMockingTestCase @@ -133,79 +129,3 @@ def test_falseish_locale_raises_valueerror(self, m_subp): self.assertEqual( "Failed to provide locale value.", str(ctext_m.exception) ) - - -@mock.patch.dict("os.environ", {}, clear=True) -@mock.patch("cloudinit.distros.debian.subp.which", return_value=True) -@mock.patch("cloudinit.distros.debian.subp.subp") -class TestPackageCommand: - distro = distros.fetch("debian")("debian", {}, None) - - @mock.patch( - "cloudinit.distros.debian.Distro._apt_lock_available", - return_value=True, - ) - def test_simple_command(self, m_apt_avail, m_subp, m_which): - self.distro.package_command("update") - apt_args = [APT_GET_WRAPPER["command"]] - apt_args.extend(APT_GET_COMMAND) - apt_args.append("update") - expected_call = { - "args": apt_args, - "capture": False, - "env": {"DEBIAN_FRONTEND": "noninteractive"}, - } - assert m_subp.call_args == mock.call(**expected_call) - - @mock.patch( - "cloudinit.distros.debian.Distro._apt_lock_available", - side_effect=[False, False, True], - ) - @mock.patch("cloudinit.distros.debian.time.sleep") - def test_wait_for_lock(self, m_sleep, m_apt_avail, m_subp, m_which): - self.distro._wait_for_apt_command("stub", {"args": "stub2"}) - assert m_sleep.call_args_list == [mock.call(1), mock.call(1)] - assert m_subp.call_args_list == [mock.call(args="stub2")] - - @mock.patch( - "cloudinit.distros.debian.Distro._apt_lock_available", - return_value=False, - ) - @mock.patch("cloudinit.distros.debian.time.sleep") - @mock.patch("cloudinit.distros.debian.time.time", side_effect=count()) - def test_lock_wait_timeout( - self, m_time, m_sleep, m_apt_avail, m_subp, m_which - ): - with pytest.raises(TimeoutError): - self.distro._wait_for_apt_command("stub", "stub2", timeout=5) - assert m_subp.call_args_list == [] - - @mock.patch( - "cloudinit.distros.debian.Distro._apt_lock_available", - side_effect=cycle([True, False]), - ) - @mock.patch("cloudinit.distros.debian.time.sleep") - def test_lock_exception_wait(self, m_sleep, m_apt_avail, m_subp, m_which): - exception = subp.ProcessExecutionError( - exit_code=100, stderr="Could not get apt lock" - ) - m_subp.side_effect = [exception, exception, "return_thing"] - ret = self.distro._wait_for_apt_command("stub", {"args": "stub2"}) - assert ret == "return_thing" - - @mock.patch( - "cloudinit.distros.debian.Distro._apt_lock_available", - side_effect=cycle([True, False]), - ) - @mock.patch("cloudinit.distros.debian.time.sleep") - @mock.patch("cloudinit.distros.debian.time.time", side_effect=count()) - def test_lock_exception_timeout( - self, m_time, m_sleep, m_apt_avail, m_subp, m_which - ): - m_subp.side_effect = subp.ProcessExecutionError( - exit_code=100, stderr="Could not get apt lock" - ) - with pytest.raises(TimeoutError): - self.distro._wait_for_apt_command( - "stub", {"args": "stub2"}, timeout=5 - ) diff --git a/tests/unittests/distros/test_init.py b/tests/unittests/distros/test_init.py index 8f3c89780186..85424a0d31bc 100644 --- a/tests/unittests/distros/test_init.py +++ b/tests/unittests/distros/test_init.py @@ -10,6 +10,7 @@ import pytest from cloudinit.distros import LDH_ASCII_CHARS, _get_package_mirror_info +from tests.unittests.distros import _get_distro # In newer versions of Python, these characters will be omitted instead # of substituted because of security concerns. @@ -246,3 +247,72 @@ def test_valid_substitution( print(patterns) print(expected) assert {"primary": expected} == ret + + +class TestInstall: + """Tests for cloudinit.distros.Distro.install_packages.""" + + @pytest.fixture + def m_apt_install(self, mocker): + return mocker.patch( + "cloudinit.distros.package_management.apt.Apt.install_packages", + return_value=[], + ) + + @pytest.fixture + def m_snap_install(self, mocker): + return mocker.patch( + "cloudinit.distros.package_management.snap.Snap.install_packages", + return_value=[], + ) + + def test_invalid_yaml(self, m_apt_install): + """Test that an invalid YAML raises an exception.""" + with pytest.raises(ValueError): + _get_distro("debian").install_packages([["invalid"]]) + m_apt_install.assert_not_called() + + def test_unknown_package_manager(self, m_apt_install, caplog): + """Test that an unknown package manager raises an exception.""" + _get_distro("debian").install_packages( + [{"apt": ["pkg1"]}, "pkg2", {"invalid": ["pkg3"]}] + ) + assert ( + "Cannot install packages under 'invalid' as it is not a supported " + "package manager!" in caplog.text + ) + install_args = m_apt_install.call_args_list[0][0][0] + assert "pkg1" in install_args + assert "pkg2" in install_args + assert "pkg3" not in install_args + + def test_non_default_package_manager(self, m_apt_install, m_snap_install): + """Test success from package manager not supported by distro.""" + _get_distro("debian").install_packages( + [{"apt": ["pkg1"]}, "pkg2", {"snap": ["pkg3"]}] + ) + apt_install_args = m_apt_install.call_args_list[0][0][0] + assert "pkg1" in apt_install_args + assert "pkg2" in apt_install_args + assert "pkg3" not in apt_install_args + + assert "pkg3" in m_snap_install.call_args_list[0][1]["pkglist"] + + def test_non_default_package_manager_fail( + self, m_apt_install, mocker, caplog + ): + """Test fail from package manager not supported by distro.""" + m_snap_install = mocker.patch( + "cloudinit.distros.package_management.snap.Snap.install_packages", + return_value=["pkg3"], + ) + _get_distro("debian").install_packages( + [{"apt": ["pkg1"]}, "pkg2", {"snap": ["pkg3"]}] + ) + + assert "pkg3" in m_snap_install.call_args_list[0][1]["pkglist"] + assert ( + "Failed to install the following packages: ['pkg3']. " + "See associated package manager logs for more details" + in caplog.text + ) diff --git a/tools/run-container b/tools/run-container index d8499ed8ddb3..f27c02137d0d 100755 --- a/tools/run-container +++ b/tools/run-container @@ -251,7 +251,7 @@ install_packages() { case "$OS_NAME" in centos|rocky*) yum_install "$@";; opensuse*) zypper_install "$@";; - debian|ubuntu) apt_install "$@";; + debian|ubuntu) apt_install "$@" -y;; *) error "Do not know how to install packages on ${OS_NAME}"; return 1;; esac