diff --git a/osism/commands/apply.py b/osism/commands/apply.py index 12d946d62..d12ac317e 100644 --- a/osism/commands/apply.py +++ b/osism/commands/apply.py @@ -11,6 +11,7 @@ from osism import utils from osism.data import enums from osism.data.enums import Role +from osism.data.releases import format_release def _collect_result(result): @@ -106,6 +107,19 @@ def get_parser(self, prog_name): help="Dry run, do not initiate tasks (for collections only)", action="store_true", ) + parser.add_argument( + "--openstack-version", + default=None, + type=str, + help=( + "OpenStack release the collection is applied to, e.g. 2026.1. " + "Collections deploy a few roles only on certain releases (redis " + "up to 2025.1, valkey from 2025.2). Read from openstack_version " + "in /interface/versions/kolla-ansible.yml when not given. Must " + "be given before the collection name, or it is swallowed as an " + "Ansible argument. (env: OPENSTACK_VERSION)" + ), + ) parser.add_argument( "--show-tree", dest="show_tree", @@ -165,6 +179,7 @@ def _handle_collection( retry, dry_run, show_tree, + release=None, ): from celery import chain, group from osism.tasks import ansible @@ -182,27 +197,47 @@ def _handle_collection( role_name = item.name dependencies = item.dependencies - logger.info(f"A [{counter}] {'-' * (counter + 1)} {role_name}") - - if show_tree: - # Only show the tree, don't create tasks + if release is not None and not item.deployed_in(release): + # Not a warning: on a release where the role does not belong, + # this is correct output on every run, and a warning that always + # fires teaches operators to ignore warnings. + logger.info( + f"Skipping {role_name}: not deployed by this collection " + f"on OpenStack {format_release(release)} " + f"(deployed {item.bound_description()})" + ) pt = None - elif dry_run: - pt = ansible.noop.si() else: - pt = self._prepare_task( - arguments, - environment, - overwrite, - sub, - role_name, - action, - wait, - format, - timeout, - task_timeout, - ) + if release is None and item.release_bounded: + logger.warning( + f"{role_name} has release bounds " + f"({item.bound_description()}) but no OpenStack " + f"release was resolved; its bounds are being ignored " + f"and it will be scheduled unconditionally." + ) + + logger.info(f"A [{counter}] {'-' * (counter + 1)} {role_name}") + + if show_tree: + # Only show the tree, don't create tasks + pt = None + elif dry_run: + pt = ansible.noop.si() + else: + pt = self._prepare_task( + arguments, + environment, + overwrite, + sub, + role_name, + action, + wait, + format, + timeout, + task_timeout, + ) + st = None if dependencies: logger.debug(f"X [{counter + 1}] --> {dependencies}") st = self._handle_collection( @@ -221,12 +256,33 @@ def _handle_collection( retry, dry_run, show_tree, + release, ) - if not show_tree: - g.append(chain(pt, st)) - else: - if not show_tree: - g.append(pt) + + if show_tree: + continue + + # Both halves can be absent: pt when the role is excluded, st when + # its dependencies all were. Appending either unconditionally would + # build chain(pt, None) or group([None]). + if pt is not None and st is not None: + g.append(chain(pt, st)) + elif pt is not None: + g.append(pt) + elif st is not None: + # Promotion keeps the subtree and its position among retained + # siblings, but not what the excluded role supplied to it: + # chain(pt, st) ran the role BEFORE its dependencies, so they + # are its dependents and it is their prerequisite. Dropping it + # leaves them with no predecessor, and nothing here can know + # which retained or replacement role belongs in that place. + # + # Nothing checks that at runtime, deliberately. The catalog + # cannot grow a bounded role with dependencies without turning + # test_no_bounded_role_has_dependencies red first, so by the + # time this branch runs the ordering has already been settled + # by whoever made that test pass. + g.append(st) if g: return group(g) @@ -246,6 +302,7 @@ def handle_collection( retry, dry_run, show_tree, + release=None, ): if dry_run: logger.info(f"Dry run for collection {collection}. No tasks are scheduled.") @@ -270,6 +327,7 @@ def handle_collection( retry, dry_run, show_tree, + release, ) # Only apply tasks if not in show_tree mode @@ -284,6 +342,49 @@ def handle_collection( return 0 + def _resolve_release(self, override, bounded): + """Return the deployed release, or exit reporting why it is unknown. + + ``bounded`` maps collection name to the release-bounded roles it + contains; it is used only to name what needed the release. + """ + from osism.data.releases import ( + ReleaseUndetermined, + ReleaseUnparseable, + openstack_release, + ) + + try: + return openstack_release(override) + except ReleaseUnparseable as exc: + # Already a complete sentence. + logger.error(str(exc)) + except ReleaseUndetermined as exc: + names = ", ".join(sorted(bounded)) + noun = "Collection" if len(bounded) == 1 else "Collections" + verb = "contains" if len(bounded) == 1 else "contain" + logger.error( + f"{noun} {names} {verb} roles that depend on the OpenStack " + f"release, but the release could not be determined: {exc}" + ) + + # One role can appear in several collections; report it once. + descriptions = {} + for roles in bounded.values(): + for role in roles: + descriptions[role.name] = role.bound_description() + + affected = ", ".join( + f"{name} ({descriptions[name]})" for name in sorted(descriptions) + ) + logger.error(f"Affected roles: {affected}") + logger.error( + "Supply the release with OPENSTACK_VERSION= osism apply " + ", or with --openstack-version before the " + "collection name." + ) + exit(1) + def _prepare_task( self, arguments, @@ -457,6 +558,21 @@ def take_action(self, parsed_args): rc = 0 + # Resolve the release before the dispatch loop below, not inside it: each + # iteration ends in apply_async(), so resolving per entry would let an + # earlier collection reach the cluster before a later one failed. + release = None + if role: + bounded = {} + for entry in role.split("//"): + if entry in enums.MAP_ROLE2ROLE: + found = list(enums.bounded_roles(enums.MAP_ROLE2ROLE[entry])) + if found: + bounded[entry] = found + + if bounded: + release = self._resolve_release(parsed_args.openstack_version, bounded) + if not role: table = [] for role in MAP_ROLE2ENVIRONMENT: @@ -484,6 +600,7 @@ def take_action(self, parsed_args): retry, dry_run, show_tree, + release=release, ) if rc != 0: outer_break = True diff --git a/osism/data/enums.py b/osism/data/enums.py index b9562c4d7..b9e188931 100644 --- a/osism/data/enums.py +++ b/osism/data/enums.py @@ -1,5 +1,7 @@ # SPDX-License-Identifier: Apache-2.0 +from osism.data.releases import format_release, parse_release + class Role: """ @@ -8,6 +10,20 @@ class Role: Args: name: The name of the role (string) dependencies: Optional list of dependent Role objects + since: Earliest OpenStack release, inclusive, on which a collection + should deploy this role (string, e.g. "2025.2") + until: Latest such release, inclusive (string, e.g. "2025.1") + + ``since`` and ``until`` govern **collection membership only**: whether a + collection should deploy this role on a given release. They are NOT a + statement that the role's playbook exists, or that the role may be applied, + on those releases. ``valkey`` is the illustration -- its play ships from + OpenStack 2025.1, but collections deploy it only from 2025.2, because + ``osism/defaults`` leaves ``enable_valkey`` off until then. Reading the + bound as availability would wrongly reject ``osism apply valkey`` on 2025.1. + + Only collection expansion consults these bounds. ``osism apply `` + never does. Example: >>> role = Role("keystone", dependencies=[Role("glance"), Role("cinder")]) @@ -17,10 +33,64 @@ class Role: 2 """ - def __init__(self, name, dependencies=None): - """Initialize a Role with a name and optional dependencies.""" + def __init__(self, name, dependencies=None, since=None, until=None): + """Initialize a Role with a name, optional dependencies and bounds.""" self.name = name self.dependencies = dependencies or [] + # Parsed eagerly, unlike the deployed release: these are literals in + # this file, so a bad value is a bug here that should surface at once + # rather than on the one deployment whose release happens to test it. + self.since = parse_release(since) if since else None + self.until = parse_release(until) if until else None + + @property + def release_bounded(self): + """Whether collection membership depends on the OpenStack release.""" + return self.since is not None or self.until is not None + + def deployed_in(self, release): + """Whether a collection should deploy this role on ``release``. + + ``release`` is a ``(year, minor)`` tuple from ``osism.data.releases``. + """ + if self.since is not None and release < self.since: + return False + + if self.until is not None and release > self.until: + return False + + return True + + def bound_description(self): + """Render the bounds for log and error text, e.g. "from 2025.2". + + Only meaningful for a release-bounded role; raises ``ValueError`` on + one with neither bound, rather than silently rendering nonsense. + """ + if self.since is not None and self.until is not None: + return f"from {format_release(self.since)} to {format_release(self.until)}" + + if self.since is not None: + return f"from {format_release(self.since)}" + + if self.until is not None: + return f"up to {format_release(self.until)}" + + raise ValueError(f"{self.name!r} has no release bounds to describe") + + +def bounded_roles(roles): + """Yield every release-bounded Role reachable from ``roles``. + + Used to decide whether expanding a collection needs the release at all: a + collection with no bounded roles never triggers the lookup, and so can never + fail on it. + """ + for role in roles: + if role.release_bounded: + yield role + + yield from bounded_roles(role.dependencies) VALIDATE_PLAYBOOKS = { @@ -158,7 +228,10 @@ def __init__(self, name, dependencies=None): ), Role("openvswitch", dependencies=[Role("ovn")]), Role("memcached"), - Role("redis"), + # kolla replaced redis with valkey at OpenStack 2025.2; on 2025.1 + # both plays exist but osism/defaults leaves enable_valkey off. + Role("redis", until="2025.1"), + Role("valkey", since="2025.2"), Role("rabbitmq"), ], ), @@ -219,7 +292,8 @@ def __init__(self, name, dependencies=None): ), Role("openvswitch", dependencies=[Role("ovn")]), Role("memcached"), - Role("redis"), + Role("redis", until="2025.1"), + Role("valkey", since="2025.2"), Role("rabbitmq"), ], ), @@ -363,7 +437,8 @@ def __init__(self, name, dependencies=None): ), Role("openvswitch", dependencies=[Role("ovn")]), Role("memcached"), - Role("redis"), + Role("redis", until="2025.1"), + Role("valkey", since="2025.2"), Role("rabbitmq"), ], ), diff --git a/osism/data/releases.py b/osism/data/releases.py new file mode 100644 index 000000000..94eb2f3a0 --- /dev/null +++ b/osism/data/releases.py @@ -0,0 +1,129 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Resolution of the OpenStack release a deployment runs. + +Collections deploy a few roles only on some releases -- see the ``since`` and +``until`` bounds on ``osism.data.enums.Role`` -- so expanding one needs to know +which release is deployed. + +The value is read at call time, never at import. ``osism/settings.py`` and +``osism/utils/__init__.py`` parse environment variables during module import, +which turns a single typo into a CLI that cannot start at all; this module does +not repeat that. +""" + +import os +import re + +import yaml + +# The kolla-ansible container copies its own group_vars/all/versions.yml here +# when it starts, so the file describes the containers that are actually +# deployed and is present for a stable release as well as for latest. Reading +# /interface/versions has precedent in tasks/__init__.py, which takes container +# versions from the same directory. +# +# The manager configuration is deliberately not a source. It carries +# openstack_version only on a latest deployment: for a stable release the +# OpenStack and Ceph versions follow manager_version, and the key is stripped +# from environments/manager/configuration.yml -- so it is missing on exactly +# the deployments that pin a release. +VERSIONS_FILE = "/interface/versions/kolla-ansible.yml" + +# OpenStack releases are YYYY.N. Note YAML parses an unquoted 2024.2 as a float, +# so parse_release stringifies before matching. +_RELEASE_RE = re.compile(r"^(\d{4})\.(\d+)$") + + +class ReleaseError(Exception): + """Base class for failures to establish the OpenStack release.""" + + +class ReleaseUndetermined(ReleaseError): + """No source supplied a release. + + The message is a *cause fragment* ("... not found."), meant to be appended + to a sentence naming what needed the release. + """ + + +class ReleaseUnparseable(ReleaseError): + """A source supplied something that is not a release. + + The message is a complete sentence and is printed as-is. + """ + + +def parse_release(value): + """Return ``value`` as a ``(year, minor)`` tuple, e.g. ``(2025, 1)``. + + Tuples compare in release order, so callers can use ``<`` and ``>``. + """ + match = _RELEASE_RE.match(str(value).strip()) + if not match: + raise ReleaseUnparseable( + f"Could not parse OpenStack release {value!r} " + f"(expected a release like 2025.1)." + ) + + return int(match.group(1)), int(match.group(2)) + + +def format_release(release): + """Return ``release`` (a ``(year, minor)`` tuple) as ``"year.minor"``. + + Inverse of ``parse_release``. + """ + return f"{release[0]}.{release[1]}" + + +def openstack_release(override=None): + """Return the deployed OpenStack release as a ``(year, minor)`` tuple. + + Sources, in order: ``override`` (the ``--openstack-version`` argument), the + ``OPENSTACK_VERSION`` environment variable, and ``openstack_version`` in + ``VERSIONS_FILE``, which the kolla-ansible container publishes for every + deployment. + + Raises ``ReleaseUnparseable`` if a source supplied a value that is not a + release, and ``ReleaseUndetermined`` if no source supplied one at all. + """ + if override: + return parse_release(override) + + from_environment = os.environ.get("OPENSTACK_VERSION") + if from_environment: + return parse_release(from_environment) + + # Opened without testing for existence first: a stat that says the file is + # there is no promise that the open succeeds, and every way it can fail -- + # gone since, unreadable, a directory, /interface not mounted -- has to + # reach the caller as a ReleaseError anyway. + try: + with open(VERSIONS_FILE) as fp: + versions = yaml.safe_load(fp) + except FileNotFoundError: + raise ReleaseUndetermined(f"{VERSIONS_FILE} not found.") + except OSError as exc: + raise ReleaseUndetermined(f"{VERSIONS_FILE} could not be read: {exc.strerror}.") + except yaml.YAMLError: + raise ReleaseUndetermined(f"{VERSIONS_FILE} is not valid YAML.") + + # An empty file parses to None, which is the one non-mapping that means + # nothing is published rather than that the file is malformed. + if versions is None: + versions = {} + + # Well-formed YAML is not necessarily a mapping: a list or a bare scalar + # parses fine and would turn the lookup below into an AttributeError, + # escaping the diagnostic this function exists to produce. Falsy ones + # ([], false, 0, "") are caught here too, so they are reported as the + # malformed file they are and not as an unset key. + if not isinstance(versions, dict): + raise ReleaseUndetermined(f"{VERSIONS_FILE} does not contain a YAML mapping.") + + value = versions.get("openstack_version") + if not value: + raise ReleaseUndetermined(f"openstack_version not set in {VERSIONS_FILE}.") + + return parse_release(value) diff --git a/tests/unit/commands/test_apply.py b/tests/unit/commands/test_apply.py index 1245079b9..ffa476f59 100644 --- a/tests/unit/commands/test_apply.py +++ b/tests/unit/commands/test_apply.py @@ -35,7 +35,7 @@ from osism.data import enums, playbooks from osism.data.enums import Role -from ._helpers import assert_not_called_before_lock_check, make_command +from ._helpers import assert_not_called_before_lock_check, make_command, parse_args @pytest.fixture(autouse=True) @@ -111,6 +111,7 @@ def _public_collection_kwargs(**overrides): retry=0, dry_run=False, show_tree=False, + release=None, ) params.update(overrides) return params @@ -397,6 +398,247 @@ def test_handle_collection_show_tree_only_logs(task_mocks, loguru_logs): assert "A [1] -- child" in messages +# _handle_collection release bounds + + +def test_handle_collection_keeps_role_inside_its_bounds(mocker): + group_mock = mocker.patch("celery.group") + cmd = make_command(apply.Run) + pt = MagicMock(name="pt-redis") + cmd._prepare_task = MagicMock(return_value=pt) + + result = cmd._handle_collection( + [Role("redis", until="2025.1")], **_collection_kwargs(release=(2025, 1)) + ) + + group_mock.assert_called_once_with([pt]) + assert result is group_mock.return_value + + +def test_handle_collection_drops_role_outside_its_bounds(mocker, loguru_logs): + group_mock = mocker.patch("celery.group") + cmd = make_command(apply.Run) + cmd._prepare_task = MagicMock() + + result = cmd._handle_collection( + [Role("redis", until="2025.1")], **_collection_kwargs(release=(2026, 1)) + ) + + cmd._prepare_task.assert_not_called() + group_mock.assert_not_called() + assert result is None + assert any( + r["level"] == "INFO" + and r["message"] + == "Skipping redis: not deployed by this collection on OpenStack 2026.1 (deployed up to 2025.1)" + for r in loguru_logs + ) + + +def test_handle_collection_selects_between_two_bounded_roles(mocker): + group_mock = mocker.patch("celery.group") + cmd = make_command(apply.Run) + valkey_pt = MagicMock(name="pt-valkey") + cmd._prepare_task = MagicMock(return_value=valkey_pt) + roles = [Role("redis", until="2025.1"), Role("valkey", since="2025.2")] + + cmd._handle_collection(roles, **_collection_kwargs(release=(2026, 1))) + + assert [c.args[4] for c in cmd._prepare_task.call_args_list] == ["valkey"] + group_mock.assert_called_once_with([valkey_pt]) + + +def test_handle_collection_without_release_ignores_bounds(mocker): + """``release=None`` means no bounded roles were requested; filter nothing.""" + group_mock = mocker.patch("celery.group") + cmd = make_command(apply.Run) + prepared = [MagicMock(name="pt-redis"), MagicMock(name="pt-valkey")] + cmd._prepare_task = MagicMock(side_effect=prepared) + roles = [Role("redis", until="2025.1"), Role("valkey", since="2025.2")] + + cmd._handle_collection(roles, **_collection_kwargs(release=None)) + + group_mock.assert_called_once_with(prepared) + + +def test_handle_collection_without_release_warns_for_bounded_roles(mocker, loguru_logs): + """Diagnostic only: today the preflight guarantees this never happens, but + + if a future caller ever expands a collection with ``release=None`` while + it still contains bounded roles, that silent fail-open must be visible. + """ + mocker.patch("celery.group") + cmd = make_command(apply.Run) + cmd._prepare_task = MagicMock() + roles = [Role("redis", until="2025.1"), Role("valkey", since="2025.2")] + + cmd._handle_collection(roles, **_collection_kwargs(release=None)) + + warnings = [r["message"] for r in loguru_logs if r["level"] == "WARNING"] + assert any( + "redis" in m and "no OpenStack release was resolved" in m for m in warnings + ) + assert any( + "valkey" in m and "no OpenStack release was resolved" in m for m in warnings + ) + + +def test_handle_collection_with_release_does_not_warn_for_bounded_roles( + mocker, loguru_logs +): + """The warning is only for the ``release is None`` fail-open case.""" + mocker.patch("celery.group") + cmd = make_command(apply.Run) + cmd._prepare_task = MagicMock() + roles = [Role("valkey", since="2025.2")] + + cmd._handle_collection(roles, **_collection_kwargs(release=(2026, 1))) + + warnings = [r["message"] for r in loguru_logs if r["level"] == "WARNING"] + assert warnings == [] + + +# _handle_collection tree structure under exclusion + + +def test_handle_collection_excluded_parent_promotes_subtree(mocker): + """pt None, st a task: the subtree replaces the parent, unchained.""" + group_mock = mocker.patch("celery.group") + chain_mock = mocker.patch("celery.chain") + cmd = make_command(apply.Run) + child_pt = MagicMock(name="pt-child") + cmd._prepare_task = MagicMock(return_value=child_pt) + + result = cmd._handle_collection( + [Role("parent", until="2025.1", dependencies=[Role("child")])], + **_collection_kwargs(release=(2026, 1)), + ) + + assert [c.args[4] for c in cmd._prepare_task.call_args_list] == ["child"] + chain_mock.assert_not_called() + assert group_mock.call_args_list == [ + call([child_pt]), + call([group_mock.return_value]), + ] + assert result is group_mock.return_value + + +def test_handle_collection_excluded_parent_preserves_sibling_order(mocker): + """Promotion must not reorder the surrounding group.""" + group_mock = mocker.patch("celery.group") + mocker.patch("celery.chain") + cmd = make_command(apply.Run) + cmd._prepare_task = MagicMock(side_effect=lambda *args, **kwargs: args[4]) + roles = [ + Role("first"), + Role("gone", until="2025.1", dependencies=[Role("promoted")]), + Role("last"), + ] + + cmd._handle_collection(roles, **_collection_kwargs(release=(2026, 1))) + + outer = group_mock.call_args_list[-1].args[0] + assert outer == ["first", group_mock.return_value, "last"] + + +def test_handle_collection_retained_parent_all_children_excluded(mocker): + """pt a task, st None: schedule the parent alone, never chain(pt, None).""" + group_mock = mocker.patch("celery.group") + chain_mock = mocker.patch("celery.chain") + cmd = make_command(apply.Run) + parent_pt = MagicMock(name="pt-parent") + cmd._prepare_task = MagicMock(return_value=parent_pt) + + result = cmd._handle_collection( + [Role("parent", dependencies=[Role("child", until="2025.1")])], + **_collection_kwargs(release=(2026, 1)), + ) + + assert [c.args[4] for c in cmd._prepare_task.call_args_list] == ["parent"] + chain_mock.assert_not_called() + group_mock.assert_called_once_with([parent_pt]) + assert result is group_mock.return_value + + +def test_handle_collection_nested_fully_excluded_subtree(mocker): + """pt None, st None, nested: append nothing, never group([None]). + + The emptiness has to propagate all the way out as ``None`` rather than as an + empty group, through however many excluded levels there are. + """ + group_mock = mocker.patch("celery.group") + chain_mock = mocker.patch("celery.chain") + cmd = make_command(apply.Run) + cmd._prepare_task = MagicMock() + + result = cmd._handle_collection( + [ + Role( + "outer", + until="2025.1", + dependencies=[ + Role( + "middle", + until="2025.1", + dependencies=[Role("inner", until="2025.1")], + ) + ], + ) + ], + **_collection_kwargs(release=(2026, 1)), + ) + + cmd._prepare_task.assert_not_called() + chain_mock.assert_not_called() + group_mock.assert_not_called() + assert result is None + + +def test_handle_collection_all_roles_excluded_yields_nothing(mocker): + group_mock = mocker.patch("celery.group") + cmd = make_command(apply.Run) + cmd._prepare_task = MagicMock() + + result = cmd._handle_collection( + [Role("redis", until="2025.1"), Role("other", until="2024.2")], + **_collection_kwargs(release=(2026, 1)), + ) + + group_mock.assert_not_called() + assert result is None + + +def test_handle_collection_excluded_role_not_logged_as_applied(loguru_logs, mocker): + mocker.patch("celery.group") + cmd = make_command(apply.Run) + cmd._prepare_task = MagicMock() + + cmd._handle_collection( + [Role("redis", until="2025.1")], **_collection_kwargs(release=(2026, 1)) + ) + + assert not any(r["message"].startswith("A [0]") for r in loguru_logs) + + +def test_handle_collection_show_tree_still_reports_exclusions(loguru_logs): + cmd = make_command(apply.Run) + cmd._prepare_task = MagicMock() + + result = cmd._handle_collection( + [Role("redis", until="2025.1"), Role("valkey", since="2025.2")], + **_collection_kwargs(release=(2026, 1), show_tree=True), + ) + + assert result is None + cmd._prepare_task.assert_not_called() + messages = [r["message"] for r in loguru_logs] + assert ( + "Skipping redis: not deployed by this collection on OpenStack 2026.1 (deployed up to 2025.1)" + in messages + ) + assert "A [0] - valkey" in messages + + # handle_collection @@ -755,3 +997,291 @@ def test_take_action_collection_chain_continues_after_success(take_action_mocks) assert rc == 0 cmd.handle_role.assert_called_once() assert cmd.handle_role.call_args.args[4] == "other" + + +# take_action release preflight + + +def _bounded_collection(): + return [Role("valkey", since="2025.2"), Role("plain")] + + +def test_apply_accepts_openstack_version_argument(): + _, parsed = parse_args(apply.Run, ["--openstack-version", "2026.1", "nutshell"]) + + assert parsed.openstack_version == "2026.1" + + +def test_apply_openstack_version_defaults_to_none(): + _, parsed = parse_args(apply.Run, ["nutshell"]) + + assert parsed.openstack_version is None + + +def test_apply_openstack_version_help_warns_about_ordering(): + """``arguments`` is ``REMAINDER``: a flag after the role is swallowed.""" + cmd = make_command(apply.Run) + parser = cmd.get_parser("test") + action = next( + a for a in parser._actions if "--openstack-version" in a.option_strings + ) + + assert "before the collection name" in action.help + + +def test_apply_openstack_version_help_names_the_versions_file(): + """The fallback source has to be the one the CLI actually reads. + + The manager configuration is not it: openstack_version is stripped from + that file for a stable release, so help text pointing an operator there + would describe a source that is absent on every pinned deployment. + """ + from osism.data import releases + + cmd = make_command(apply.Run) + parser = cmd.get_parser("test") + action = next( + a for a in parser._actions if "--openstack-version" in a.option_strings + ) + + assert releases.VERSIONS_FILE in action.help + assert "/opt/configuration" not in action.help + + +def test_take_action_passes_resolved_release_to_collection(take_action_mocks): + cmd, parsed = parse_args( + apply.Run, ["--openstack-version", "2026.1", "testcollection"] + ) + cmd.handle_collection = MagicMock(return_value=0) + + with patch.dict(enums.MAP_ROLE2ROLE, {"testcollection": _bounded_collection()}): + cmd.take_action(parsed) + + assert cmd.handle_collection.call_args.kwargs["release"] == (2026, 1) + + +def test_take_action_unbounded_collection_never_resolves_release( + take_action_mocks, mocker +): + resolve = mocker.patch.object(apply.Run, "_resolve_release") + cmd, parsed = parse_args(apply.Run, ["testcollection"]) + cmd.handle_collection = MagicMock(return_value=0) + + with patch.dict(enums.MAP_ROLE2ROLE, {"testcollection": [Role("plain")]}): + cmd.take_action(parsed) + + resolve.assert_not_called() + assert cmd.handle_collection.call_args.kwargs["release"] is None + + +def test_take_action_undeterminable_release_exits( + take_action_mocks, mocker, loguru_logs +): + from osism.data import releases + + mocker.patch.object( + releases, + "openstack_release", + side_effect=releases.ReleaseUndetermined("/some/versions.yml not found."), + ) + cmd, parsed = parse_args(apply.Run, ["testcollection"]) + cmd.handle_collection = MagicMock(return_value=0) + + with patch.dict(enums.MAP_ROLE2ROLE, {"testcollection": _bounded_collection()}): + with pytest.raises(SystemExit) as excinfo: + cmd.take_action(parsed) + + assert excinfo.value.code == 1 + cmd.handle_collection.assert_not_called() + messages = [r["message"] for r in loguru_logs if r["level"] == "ERROR"] + assert any( + "Collection testcollection contains roles that depend on the OpenStack " + "release, but the release could not be determined: " + "/some/versions.yml not found." == m + for m in messages + ) + assert any("Affected roles: valkey (from 2025.2)" == m for m in messages) + assert any( + m.startswith("Supply the release with OPENSTACK_VERSION=") + and "--openstack-version" in m + for m in messages + ) + + +def test_take_action_undeterminable_release_advice_is_order_independent( + take_action_mocks, mocker, loguru_logs +): + """The advice must not recommend a flag placement that silently fails. + + ``arguments`` is ``nargs=argparse.REMAINDER``, so a flag placed after the + collection name is swallowed as an Ansible argument rather than parsed. + The advice therefore leads with the env-var form and, if it mentions the + flag at all, says where it must go. It names no file to edit either: the + versions file it failed to read belongs to the kolla-ansible container. + """ + from osism.data import releases + + mocker.patch.object( + releases, + "openstack_release", + side_effect=releases.ReleaseUndetermined("/some/versions.yml not found."), + ) + cmd, parsed = parse_args(apply.Run, ["testcollection"]) + cmd.handle_collection = MagicMock(return_value=0) + + with patch.dict(enums.MAP_ROLE2ROLE, {"testcollection": _bounded_collection()}): + with pytest.raises(SystemExit): + cmd.take_action(parsed) + + messages = [r["message"] for r in loguru_logs if r["level"] == "ERROR"] + assert any( + m + == ( + "Supply the release with OPENSTACK_VERSION= osism apply " + ", or with --openstack-version before the " + "collection name." + ) + for m in messages + ) + + +def test_take_action_unparseable_release_prints_bare_message( + take_action_mocks, mocker, loguru_logs +): + cmd, parsed = parse_args( + apply.Run, ["--openstack-version", "master", "testcollection"] + ) + cmd.handle_collection = MagicMock(return_value=0) + + with patch.dict(enums.MAP_ROLE2ROLE, {"testcollection": _bounded_collection()}): + with pytest.raises(SystemExit): + cmd.take_action(parsed) + + messages = [r["message"] for r in loguru_logs if r["level"] == "ERROR"] + assert any( + m == "Could not parse OpenStack release 'master' " + "(expected a release like 2025.1)." + for m in messages + ) + assert not any("could not be determined" in m for m in messages) + + +def test_take_action_preflight_schedules_nothing_across_slashes( + take_action_mocks, mocker +): + """The guarantee is command-wide, not collection-wide. + + Resolving inside the dispatch loop would let the first entry reach + apply_async before the second one failed. + """ + from osism.data import releases + + mocker.patch.object( + releases, + "openstack_release", + side_effect=releases.ReleaseUndetermined("/some/versions.yml not found."), + ) + cmd, parsed = parse_args(apply.Run, ["unbounded//testcollection"]) + cmd._handle_collection = MagicMock() + + with patch.dict( + enums.MAP_ROLE2ROLE, + {"unbounded": [Role("plain")], "testcollection": _bounded_collection()}, + ): + with pytest.raises(SystemExit): + cmd.take_action(parsed) + + cmd._handle_collection.assert_not_called() + + +def test_take_action_resolves_the_release_once_per_invocation( + take_action_mocks, mocker +): + from osism.data import releases + + resolve = mocker.patch.object(releases, "openstack_release", return_value=(2026, 1)) + cmd, parsed = parse_args(apply.Run, ["testcollection//othercollection"]) + cmd.handle_collection = MagicMock(return_value=0) + + with patch.dict( + enums.MAP_ROLE2ROLE, + { + "testcollection": _bounded_collection(), + "othercollection": [Role("redis", until="2025.1")], + }, + ): + cmd.take_action(parsed) + + resolve.assert_called_once() + assert [c.kwargs["release"] for c in cmd.handle_collection.call_args_list] == [ + (2026, 1), + (2026, 1), + ] + + +def test_take_action_names_every_affected_collection( + take_action_mocks, mocker, loguru_logs +): + from osism.data import releases + + mocker.patch.object( + releases, + "openstack_release", + side_effect=releases.ReleaseUndetermined("/some/versions.yml not found."), + ) + cmd, parsed = parse_args(apply.Run, ["testcollection//othercollection"]) + + with patch.dict( + enums.MAP_ROLE2ROLE, + { + "testcollection": _bounded_collection(), + "othercollection": [Role("redis", until="2025.1")], + }, + ): + with pytest.raises(SystemExit): + cmd.take_action(parsed) + + messages = [r["message"] for r in loguru_logs if r["level"] == "ERROR"] + assert any( + m.startswith("Collections othercollection, testcollection contain roles") + for m in messages + ) + assert any( + m == "Affected roles: redis (up to 2025.1), valkey (from 2025.2)" + for m in messages + ) + + +# bounds are collection membership, never role availability + + +def test_take_action_explicit_bounded_role_ignores_bounds(take_action_mocks, mocker): + """``osism apply valkey`` on 2025.1 must dispatch. + + valkey's play exists on 2025.1; the ``since="2025.2"`` bound says only that + collections should not deploy it there. Reading the bound as availability + would reject a legitimate command. + """ + resolve = mocker.patch.object(apply.Run, "_resolve_release") + cmd, parsed = parse_args(apply.Run, ["--openstack-version", "2025.1", "valkey"]) + cmd.handle_role = MagicMock(return_value=0) + + cmd.take_action(parsed) + + resolve.assert_not_called() + # handle_role is called positionally: (arguments, environment, overwrite, + # sub, role, ...), so the role is args[4]. + assert cmd.handle_role.call_args.args[4] == "valkey" + + +def test_take_action_explicit_redis_still_dispatches_on_2026_1( + take_action_mocks, mocker +): + resolve = mocker.patch.object(apply.Run, "_resolve_release") + cmd, parsed = parse_args(apply.Run, ["--openstack-version", "2026.1", "redis"]) + cmd.handle_role = MagicMock(return_value=0) + + cmd.take_action(parsed) + + resolve.assert_not_called() + assert cmd.handle_role.call_args.args[4] == "redis" diff --git a/tests/unit/data/test_enums.py b/tests/unit/data/test_enums.py index f1706248d..aceb2a3c4 100644 --- a/tests/unit/data/test_enums.py +++ b/tests/unit/data/test_enums.py @@ -1,9 +1,12 @@ # SPDX-License-Identifier: Apache-2.0 +import pytest + from osism.data.enums import ( MAP_ROLE2ROLE, VALIDATE_PLAYBOOKS, Role, + bounded_roles, ) @@ -284,3 +287,213 @@ def test_map_role2role_walk_handles_cycles(): assert {role.name for role in visited} == {"a", "b"} assert len(visited) == 2 + + +# --------------------------------------------------------------------------- +# Role release bounds +# --------------------------------------------------------------------------- + + +def test_role_bounds_default_to_none(): + role = Role("keystone") + + assert role.since is None + assert role.until is None + assert role.release_bounded is False + + +def test_role_bounds_parsed_to_tuples(): + role = Role("valkey", since="2025.2", until="2026.1") + + assert role.since == (2025, 2) + assert role.until == (2026, 1) + assert role.release_bounded is True + + +def test_role_rejects_unparseable_bound(): + """Catalog literals are parsed eagerly so a typo fails loudly, at once.""" + from osism.data.releases import ReleaseUnparseable + + with pytest.raises(ReleaseUnparseable): + Role("valkey", since="master") + + +def test_role_without_bounds_is_in_every_release(): + role = Role("keystone") + + assert role.deployed_in((2024, 1)) is True + assert role.deployed_in((2026, 1)) is True + + +def test_role_since_is_inclusive(): + role = Role("valkey", since="2025.2") + + assert role.deployed_in((2025, 1)) is False + assert role.deployed_in((2025, 2)) is True + assert role.deployed_in((2026, 1)) is True + + +def test_role_until_is_inclusive(): + role = Role("redis", until="2025.1") + + assert role.deployed_in((2024, 2)) is True + assert role.deployed_in((2025, 1)) is True + assert role.deployed_in((2025, 2)) is False + + +def test_role_both_bounds_form_a_closed_range(): + role = Role("interim", since="2025.2", until="2026.1") + + assert role.deployed_in((2025, 1)) is False + assert role.deployed_in((2025, 2)) is True + assert role.deployed_in((2026, 1)) is True + assert role.deployed_in((2026, 2)) is False + + +@pytest.mark.parametrize( + "kwargs,expected", + [ + ({"since": "2025.2"}, "from 2025.2"), + ({"until": "2025.1"}, "up to 2025.1"), + ({"since": "2025.2", "until": "2026.1"}, "from 2025.2 to 2026.1"), + ], +) +def test_role_bound_description(kwargs, expected): + assert Role("role", **kwargs).bound_description() == expected + + +def test_role_bound_description_raises_on_unbounded_role(): + """Nothing to describe on an unbounded Role; fail loud, not with nonsense.""" + with pytest.raises(ValueError): + Role("keystone").bound_description() + + +# --------------------------------------------------------------------------- +# bounded_roles +# --------------------------------------------------------------------------- + + +def test_bounded_roles_finds_nothing_when_unbounded(): + roles = [Role("a", dependencies=[Role("b")]), Role("c")] + + assert list(bounded_roles(roles)) == [] + + +def test_bounded_roles_finds_nested_bounds(): + valkey = Role("valkey", since="2025.2") + roles = [Role("a", dependencies=[Role("b", dependencies=[valkey])]), Role("c")] + + assert list(bounded_roles(roles)) == [valkey] + + +def test_bounded_roles_includes_a_bounded_parent_and_its_bounded_child(): + child = Role("child", since="2026.1") + parent = Role("parent", until="2025.1", dependencies=[child]) + + assert list(bounded_roles([parent])) == [parent, child] + + +# --------------------------------------------------------------------------- +# The key-value store cut-over +# --------------------------------------------------------------------------- + + +KVS_COLLECTIONS = ["nutshell", "collection-infrastructure", "cloudpod-infrastructure"] + + +@pytest.mark.parametrize("collection", KVS_COLLECTIONS) +def test_kvs_collections_carry_both_backends(collection): + roles = MAP_ROLE2ROLE[collection] + + assert find_role(roles, "redis") is not None + assert find_role(roles, "valkey") is not None + + +@pytest.mark.parametrize("collection", KVS_COLLECTIONS) +def test_kvs_backends_are_bounded_and_disjoint(collection): + roles = MAP_ROLE2ROLE[collection] + redis = find_role(roles, "redis") + valkey = find_role(roles, "valkey") + + assert redis.until == (2025, 1) + assert redis.since is None + assert valkey.since == (2025, 2) + assert valkey.until is None + + +@pytest.mark.parametrize("collection", KVS_COLLECTIONS) +@pytest.mark.parametrize( + "release,expected", + [ + ((2024, 2), "redis"), + ((2025, 1), "redis"), + ((2025, 2), "valkey"), + ((2026, 1), "valkey"), + ], +) +def test_exactly_one_backend_per_release(collection, release, expected): + """Never both, never neither -- on any release, past or future.""" + roles = MAP_ROLE2ROLE[collection] + selected = [ + name + for name in ("redis", "valkey") + if find_role(roles, name).deployed_in(release) + ] + + assert selected == [expected] + + +def test_only_the_kvs_roles_carry_bounds(): + """A bound anywhere else is new and wants its own test above.""" + bounded = { + role.name for roles in MAP_ROLE2ROLE.values() for role in bounded_roles(roles) + } + + assert bounded == {"redis", "valkey"} + + +def test_no_bounded_role_has_dependencies(): + """A release-bounded role must be a leaf, until someone settles ordering. + + Excluding a role promotes its dependencies into the surrounding group so + the subtree survives. That keeps the subtree and its position among + retained siblings, but not what the excluded role supplied to it: + ``chain(pt, st)`` runs a role BEFORE its ``dependencies``, so they are its + dependents and it is their prerequisite. Promote them past an excluded + parent and they run with no predecessor -- a well-formed task graph that + may be ordered wrongly, which is worse than an error because it looks + intentional. Nothing at expansion time can know which retained or + replacement role belongs in that place. + + So the decision is forced here, in CI, in the change that introduces such + a role -- not at deploy time, and not only for whoever reads the design + document. The case this exists for is the kolla 2026.1 split of ``common`` + into ``logs``, ``kolla_toolbox``, ``cron`` and ``fluentd``: ``common`` has + six dependents, so bounding it trips this test. + + If you are here because you added one: decide what runs in the excluded + role's place on each release, encode that, and replace this test with one + that pins the ordering you chose. + """ + offenders = { + role.name: [dependency.name for dependency in role.dependencies] + for roles in MAP_ROLE2ROLE.values() + for role in bounded_roles(roles) + if role.dependencies + } + + assert not offenders, ( + f"release-bounded roles with dependencies: {offenders}. " + "Excluding one promotes its dependents but not the prerequisite it " + "was; settle the ordering and replace this test. See the docstring." + ) + + +def test_valkey_absent_from_collections_that_never_had_redis(): + """The cut-over touches exactly the three collections that listed redis.""" + for name, roles in MAP_ROLE2ROLE.items(): + if name in KVS_COLLECTIONS: + continue + + assert find_role(roles, "valkey") is None, name + assert find_role(roles, "redis") is None, name diff --git a/tests/unit/data/test_releases.py b/tests/unit/data/test_releases.py new file mode 100644 index 000000000..6e7cda1d9 --- /dev/null +++ b/tests/unit/data/test_releases.py @@ -0,0 +1,322 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Tests for OpenStack release resolution. + +``osism.data.releases`` answers one question -- which OpenStack release is +deployed -- from three sources in a fixed order, the last of them the versions +file the kolla-ansible container publishes under /interface. These tests +characterize the parser, the resolution order, and the distinct failures the +caller has to render differently (missing file, unreadable file, missing key, +malformed file, unparseable value). +""" + +import os + +import pytest + +from osism.data import releases + +# parse_release + + +@pytest.mark.parametrize( + "value,expected", + [ + ("2025.1", (2025, 1)), + ("2026.1", (2026, 1)), + ("2024.2", (2024, 2)), + (" 2025.1 ", (2025, 1)), + ], +) +def test_parse_release_accepts_releases(value, expected): + assert releases.parse_release(value) == expected + + +def test_parse_release_accepts_yaml_float(): + """YAML parses an unquoted ``openstack_version: 2024.2`` as a float.""" + assert releases.parse_release(2024.2) == (2024, 2) + + +@pytest.mark.parametrize("value", ["master", "2025", "", "stable/2025.1", "v2025.1"]) +def test_parse_release_rejects_non_releases(value): + with pytest.raises(releases.ReleaseUnparseable) as excinfo: + releases.parse_release(value) + + assert "expected a release like 2025.1" in str(excinfo.value) + assert repr(value) in str(excinfo.value) + + +def test_parse_release_orders_as_tuples(): + assert releases.parse_release("2025.2") > releases.parse_release("2025.1") + assert releases.parse_release("2026.1") > releases.parse_release("2025.2") + + +# format_release + + +@pytest.mark.parametrize( + "release,expected", + [ + ((2025, 1), "2025.1"), + ((2026, 1), "2026.1"), + ((2024, 2), "2024.2"), + ], +) +def test_format_release_renders_year_dot_minor(release, expected): + assert releases.format_release(release) == expected + + +@pytest.mark.parametrize( + "value", + ["2025.1", "2026.1", "2024.2", 2024.2], +) +def test_format_release_round_trips_with_parse_release(value): + release = releases.parse_release(value) + + assert releases.parse_release(releases.format_release(release)) == release + + +# openstack_release resolution order + + +def test_openstack_release_override_wins(monkeypatch, tmp_path): + monkeypatch.setenv("OPENSTACK_VERSION", "2024.1") + monkeypatch.setattr(releases, "VERSIONS_FILE", str(tmp_path / "absent.yml")) + + assert releases.openstack_release("2026.1") == (2026, 1) + + +def test_openstack_release_env_beats_file(monkeypatch, tmp_path): + versions = tmp_path / "kolla-ansible.yml" + versions.write_text("openstack_version: 2024.1\n") + monkeypatch.setenv("OPENSTACK_VERSION", "2026.1") + monkeypatch.setattr(releases, "VERSIONS_FILE", str(versions)) + + assert releases.openstack_release() == (2026, 1) + + +def test_openstack_release_reads_versions_file(monkeypatch, tmp_path): + versions = tmp_path / "kolla-ansible.yml" + versions.write_text("openstack_version: 2026.1\nother_key: value\n") + monkeypatch.delenv("OPENSTACK_VERSION", raising=False) + monkeypatch.setattr(releases, "VERSIONS_FILE", str(versions)) + + assert releases.openstack_release() == (2026, 1) + + +def test_openstack_release_reads_versions_file_as_published(monkeypatch, tmp_path): + """The file as the kolla-ansible container publishes it. + + Two shapes in it would break a naive reader: openstack_version is quoted, + so it arrives as a string and not as the float an unquoted 2024.1 would + become, and the keys beside it hold un-rendered Jinja that must not be + mistaken for anything but opaque text. + """ + versions = tmp_path / "kolla-ansible.yml" + versions.write_text( + 'openstack_version: "2024.1"\n' + 'kolla_ansible_version: "{{ openstack_version }}"\n' + 'kolla_image_version: "{{ openstack_version }}"\n' + 'openstack_release: "{{ openstack_version }}"\n' + ) + monkeypatch.delenv("OPENSTACK_VERSION", raising=False) + monkeypatch.setattr(releases, "VERSIONS_FILE", str(versions)) + + assert releases.openstack_release() == (2024, 1) + + +def test_openstack_release_empty_env_falls_through(monkeypatch, tmp_path): + versions = tmp_path / "kolla-ansible.yml" + versions.write_text("openstack_version: 2026.1\n") + monkeypatch.setenv("OPENSTACK_VERSION", "") + monkeypatch.setattr(releases, "VERSIONS_FILE", str(versions)) + + assert releases.openstack_release() == (2026, 1) + + +# openstack_release failures + + +def test_openstack_release_missing_file(monkeypatch, tmp_path): + missing = tmp_path / "absent.yml" + monkeypatch.delenv("OPENSTACK_VERSION", raising=False) + monkeypatch.setattr(releases, "VERSIONS_FILE", str(missing)) + + with pytest.raises(releases.ReleaseUndetermined) as excinfo: + releases.openstack_release() + + assert str(missing) in str(excinfo.value) + assert "not found" in str(excinfo.value) + + +def test_openstack_release_unreadable_file(monkeypatch, tmp_path): + """A file that exists but cannot be opened is still a ReleaseError. + + Testing for existence and then opening leaves a window -- the file can be + unreadable, a directory, or gone by the time the open runs -- and an + OSError escaping here would reach the operator as a traceback instead of + the release-resolution diagnostic the caller renders. + """ + versions = tmp_path / "kolla-ansible.yml" + versions.write_text("openstack_version: 2025.1\n") + versions.chmod(0o000) + monkeypatch.delenv("OPENSTACK_VERSION", raising=False) + monkeypatch.setattr(releases, "VERSIONS_FILE", str(versions)) + + try: + if os.access(str(versions), os.R_OK): + pytest.skip("running with privileges that ignore file permissions") + + with pytest.raises(releases.ReleaseUndetermined) as excinfo: + releases.openstack_release() + finally: + versions.chmod(0o644) + + assert str(versions) in str(excinfo.value) + assert "could not be read" in str(excinfo.value) + + +def test_openstack_release_versions_path_is_a_directory(monkeypatch, tmp_path): + """The same window, in the form that does not depend on the test's uid.""" + versions = tmp_path / "kolla-ansible.yml" + versions.mkdir() + monkeypatch.delenv("OPENSTACK_VERSION", raising=False) + monkeypatch.setattr(releases, "VERSIONS_FILE", str(versions)) + + with pytest.raises(releases.ReleaseUndetermined) as excinfo: + releases.openstack_release() + + assert str(versions) in str(excinfo.value) + assert "could not be read" in str(excinfo.value) + + +def test_openstack_release_missing_key(monkeypatch, tmp_path): + versions = tmp_path / "kolla-ansible.yml" + versions.write_text("other_key: value\n") + monkeypatch.delenv("OPENSTACK_VERSION", raising=False) + monkeypatch.setattr(releases, "VERSIONS_FILE", str(versions)) + + with pytest.raises(releases.ReleaseUndetermined) as excinfo: + releases.openstack_release() + + assert "openstack_version not set" in str(excinfo.value) + assert str(versions) in str(excinfo.value) + + +def test_openstack_release_empty_file(monkeypatch, tmp_path): + versions = tmp_path / "kolla-ansible.yml" + versions.write_text("") + monkeypatch.delenv("OPENSTACK_VERSION", raising=False) + monkeypatch.setattr(releases, "VERSIONS_FILE", str(versions)) + + with pytest.raises(releases.ReleaseUndetermined): + releases.openstack_release() + + +def test_openstack_release_invalid_yaml(monkeypatch, tmp_path): + versions = tmp_path / "kolla-ansible.yml" + versions.write_text("openstack_version: [unclosed\n") + monkeypatch.delenv("OPENSTACK_VERSION", raising=False) + monkeypatch.setattr(releases, "VERSIONS_FILE", str(versions)) + + with pytest.raises(releases.ReleaseUndetermined) as excinfo: + releases.openstack_release() + + assert "not valid YAML" in str(excinfo.value) + + +@pytest.mark.parametrize( + "content,label", + [ + ("- a\n- b\n", "a list"), + ("just a scalar\n", "a bare scalar"), + # Falsy non-mappings. An "or {}" on the load turns each of these into + # an empty mapping, which passes the isinstance check below and is + # then reported as an unset key rather than as a malformed file. + ("[]\n", "an empty list"), + ("false\n", "a false scalar"), + ("0\n", "a zero scalar"), + ("''\n", "an empty string"), + ], +) +def test_openstack_release_yaml_that_is_not_a_mapping( + monkeypatch, tmp_path, content, label +): + """Well-formed YAML need not be a mapping. + + A list or a bare scalar parses without error, and looking a key up on it + would raise AttributeError -- escaping the ReleaseError diagnostic that + callers render. + """ + versions = tmp_path / "kolla-ansible.yml" + versions.write_text(content) + monkeypatch.delenv("OPENSTACK_VERSION", raising=False) + monkeypatch.setattr(releases, "VERSIONS_FILE", str(versions)) + + with pytest.raises(releases.ReleaseUndetermined) as excinfo: + releases.openstack_release() + + assert "does not contain a YAML mapping" in str(excinfo.value) + + +def test_openstack_release_unparseable_value(monkeypatch, tmp_path): + versions = tmp_path / "kolla-ansible.yml" + versions.write_text("openstack_version: master\n") + monkeypatch.delenv("OPENSTACK_VERSION", raising=False) + monkeypatch.setattr(releases, "VERSIONS_FILE", str(versions)) + + with pytest.raises(releases.ReleaseUnparseable): + releases.openstack_release() + + +def test_undetermined_and_unparseable_are_both_release_errors(): + assert issubclass(releases.ReleaseUndetermined, releases.ReleaseError) + assert issubclass(releases.ReleaseUnparseable, releases.ReleaseError) + + +# import-time safety + + +def test_import_does_not_read_environment(): + """A bad OPENSTACK_VERSION must break one command, not every import. + + ``osism/settings.py`` and ``osism/utils/__init__.py`` parse env vars at + import, so a typo there takes down the whole CLI. This module must not. + + This characterizes the environment half only: a bad ``OPENSTACK_VERSION`` + must not raise on import. It says nothing about filesystem access at + import time -- there is no environment variable to poison the way + ``OPENSTACK_VERSION`` poisons the environment half, so that half is not + exercised here. + + The subprocess also prints ``__file__`` so the assertion below pins down + which copy of the module was actually imported: run from a different + working directory, ``python -c`` can silently resolve an installed + package instead of this worktree's source, and a stale copy would make + this test pass without proving anything about the code under review. + """ + import os + import subprocess + import sys + + environment = dict(os.environ, OPENSTACK_VERSION="not-a-release") + result = subprocess.run( + [ + sys.executable, + "-c", + "import osism.data.releases as r; print(r.__file__)", + ], + env=environment, + capture_output=True, + text=True, + ) + + assert result.returncode == 0, result.stderr + + expected_file = os.path.abspath( + os.path.join( + os.path.dirname(__file__), "..", "..", "..", "osism", "data", "releases.py" + ) + ) + imported_file = os.path.abspath(result.stdout.strip()) + assert imported_file == expected_file