From 87da7cd3c14591e7c41667a9f726fe94e7dac44d Mon Sep 17 00:00:00 2001 From: Gabriel Cocenza Date: Fri, 19 Apr 2024 16:51:41 -0300 Subject: [PATCH] Fix support for latest/stable for channel based apps (#372) - removed channel based and subordinates from calculating OpenStack release when not using releases channel --- cou/apps/auxiliary.py | 26 ++--- cou/apps/auxiliary_subordinate.py | 18 +--- cou/apps/base.py | 72 ++++++++++---- cou/apps/channel_based.py | 3 + cou/apps/subordinate.py | 42 ++++---- cou/steps/analyze.py | 14 ++- cou/steps/plan.py | 8 +- cou/utils/app_utils.py | 16 +--- cou/utils/juju_utils.py | 8 ++ cou/utils/nova_compute.py | 7 +- tests/unit/apps/test_auxiliary.py | 34 ++++++- tests/unit/apps/test_auxiliary_subordinate.py | 34 ++++++- tests/unit/apps/test_base.py | 75 +++++++++++++-- tests/unit/apps/test_channel_based.py | 96 ++++++++++++++++++- tests/unit/apps/test_subordinate.py | 44 ++++++++- tests/unit/steps/test_analyze.py | 62 ++++++++++++ tests/unit/utils/test_juju_utils.py | 5 + tests/unit/utils/test_nova_compute.py | 18 ++-- 18 files changed, 469 insertions(+), 113 deletions(-) diff --git a/cou/apps/auxiliary.py b/cou/apps/auxiliary.py index c17dbdc0..272ed896 100644 --- a/cou/apps/auxiliary.py +++ b/cou/apps/auxiliary.py @@ -63,19 +63,26 @@ def is_valid_track(self, charm_channel: str) -> bool: current_track, ) in TRACK_TO_OPENSTACK_MAPPING and len(possible_tracks) > 0 - @property - def expected_current_channel(self) -> str: + def expected_current_channel(self, target: OpenStackRelease) -> str: """Return the expected current channel. - Expected current channel is the channel that the application is suppose to be using based + Expected current channel is the channel that the application is supposed to be using based on the current series, workload version and, by consequence, the OpenStack release identified. + + :param target: OpenStack release as target to upgrade. + :type target: OpenStackRelease :return: The expected current channel of the application. E.g: "3.9/stable" :rtype: str """ - *_, track = OPENSTACK_TO_TRACK_MAPPING[ - (self.charm, self.series, self.current_os_release.codename) - ] + if self.need_crossgrade and self.based_on_channel: + *_, track = OPENSTACK_TO_TRACK_MAPPING[ + (self.charm, self.series, f"{target.previous_release}") + ] + else: + *_, track = OPENSTACK_TO_TRACK_MAPPING[ + (self.charm, self.series, self.current_os_release.codename) + ] return f"{track}/stable" @@ -112,12 +119,9 @@ def channel_codename(self) -> OpenStackRelease: :raises ApplicationError: When cannot identify suitable OpenStack release codename based on the track of the charm channel. """ - if self.is_from_charm_store: + if self.need_crossgrade: logger.debug( - ( - "'Application %s' installed from charm store; assuming Ussuri as the " - "underlying version." - ), + "Cannot determine the OpenStack release of '%s' via its channel. Assuming Ussuri", self.name, ) return OpenStackRelease("ussuri") diff --git a/cou/apps/auxiliary_subordinate.py b/cou/apps/auxiliary_subordinate.py index d5821500..4d7bf21a 100644 --- a/cou/apps/auxiliary_subordinate.py +++ b/cou/apps/auxiliary_subordinate.py @@ -14,26 +14,14 @@ """Auxiliary subordinate application class.""" from cou.apps.auxiliary import OVN, AuxiliaryApplication from cou.apps.factory import AppFactory -from cou.apps.subordinate import SubordinateBase -from cou.utils.openstack import AUXILIARY_SUBORDINATES, OpenStackRelease +from cou.apps.subordinate import SubordinateApplication +from cou.utils.openstack import AUXILIARY_SUBORDINATES @AppFactory.register_application(AUXILIARY_SUBORDINATES) -class AuxiliarySubordinateApplication(SubordinateBase, AuxiliaryApplication): +class AuxiliarySubordinateApplication(SubordinateApplication, AuxiliaryApplication): """Auxiliary subordinate application class.""" - @property - def current_os_release(self) -> OpenStackRelease: - """Infer the OpenStack release from subordinate charm's channel. - - We cannot determine the OpenStack release base on workload packages because the principal - charm has already upgraded the packages. - - :return: OpenStackRelease object. - :rtype: OpenStackRelease - """ - return self.channel_codename - @AppFactory.register_application(["ovn-chassis"]) class OVNSubordinate(OVN, AuxiliarySubordinateApplication): diff --git a/cou/apps/base.py b/cou/apps/base.py index 5ad6cafa..ddb0da63 100644 --- a/cou/apps/base.py +++ b/cou/apps/base.py @@ -67,6 +67,9 @@ class OpenStackApplication(Application): packages_to_hold: Optional[list] = field(default=None, init=False) wait_timeout: int = field(default=STANDARD_IDLE_TIMEOUT, init=False) wait_for_model: bool = field(default=False, init=False) # waiting only for application itself + # OpenStack apps rely on the workload version of the packages to evaluate current OpenStack + # release + based_on_channel = False def __hash__(self) -> int: """Hash magic method for Application. @@ -131,6 +134,14 @@ def __str__(self) -> str: return yaml.dump(summary, sort_keys=False) + def __repr__(self) -> str: + """App representation. + + :return: Name of the application + :rtype: str + """ + return self.name + @property def apt_source_codename(self) -> OpenStackRelease: """Identify the OpenStack release set on "openstack-origin" or "source" config. @@ -178,6 +189,14 @@ def channel_codename(self) -> OpenStackRelease: :return: OpenStackRelease object :rtype: OpenStackRelease """ + if self.need_crossgrade: + logger.debug( + "Cannot determine the OpenStack release of '%s' " + "via its channel. Assuming Ussuri", + self.name, + ) + return OpenStackRelease("ussuri") + # get the OpenStack release from the channel track of the application. return OpenStackRelease(self._get_track_from_channel(self.channel)) @@ -217,18 +236,21 @@ def origin_setting(self) -> Optional[str]: logger.debug("%s has no origin setting config", self.name) return None - @property - def expected_current_channel(self) -> str: + def expected_current_channel(self, target: OpenStackRelease) -> str: """Return the expected current channel. - Expected current channel is the channel that the application is suppose to be using based + Expected current channel is the channel that the application is supposed to be using based on the current series, workload version and, by consequence, the OpenStack release identified. + :param target: OpenStack release as target to upgrade. + :type target: OpenStackRelease :return: The expected current channel of the application. E.g: "ussuri/stable" :rtype: str """ - return f"{self.current_os_release.codename}/stable" + if self.need_crossgrade and self.based_on_channel: + return f"{target.previous_release}/stable" + return f"{self.current_os_release}/stable" @property def os_release_units(self) -> dict[OpenStackRelease, list[str]]: @@ -244,6 +266,15 @@ def os_release_units(self) -> dict[OpenStackRelease, list[str]]: return dict(os_versions) + @property + def need_crossgrade(self) -> bool: + """Check if need a charm crossgrade. + + :return: True if necessary, False otherwise + :rtype: bool + """ + return self.is_from_charm_store or self.channel == LATEST_STABLE + def is_valid_track(self, charm_channel: str) -> bool: """Check if the channel track is valid. @@ -256,7 +287,7 @@ def is_valid_track(self, charm_channel: str) -> bool: OpenStackRelease(self._get_track_from_channel(charm_channel)) return True except ValueError: - return self.is_from_charm_store + return False def get_latest_os_version(self, unit: Unit) -> OpenStackRelease: """Get the latest compatible OpenStack release based on the unit workload version. @@ -515,9 +546,9 @@ def _get_refresh_charm_step(self, target: OpenStackRelease) -> PreUpgradeStep: :rtype: PreUpgradeStep """ if self.is_from_charm_store: - return self._get_charmhub_migration_step() + return self._get_charmhub_migration_step(target) if self.channel == LATEST_STABLE: - return self._get_change_to_openstack_channels_step() + return self._get_change_to_openstack_channels_step(target) if self._need_current_channel_refresh(target): return self._get_refresh_current_channel_step() logger.info( @@ -525,37 +556,42 @@ def _get_refresh_charm_step(self, target: OpenStackRelease) -> PreUpgradeStep: ) return PreUpgradeStep() - def _get_charmhub_migration_step(self) -> PreUpgradeStep: + def _get_charmhub_migration_step(self, target: OpenStackRelease) -> PreUpgradeStep: """Get the step for charm hub migration from charm store. + :param target: OpenStack release as target to upgrade. + :type target: OpenStackRelease :return: Step for charmhub migration :rtype: PreUpgradeStep """ return PreUpgradeStep( f"Migrate '{self.name}' from charmstore to charmhub", coro=self.model.upgrade_charm( - self.name, self.expected_current_channel, switch=f"ch:{self.charm}" + self.name, self.expected_current_channel(target), switch=f"ch:{self.charm}" ), ) - def _get_change_to_openstack_channels_step(self) -> PreUpgradeStep: - """Get the step for changing to OpenStack channels. + def _get_change_to_openstack_channels_step(self, target: OpenStackRelease) -> PreUpgradeStep: + """Get the step for changing to an OpenStack channel. + :param target: OpenStack release as target to upgrade. + :type target: OpenStackRelease :return: Step for changing to OpenStack channels :rtype: PreUpgradeStep """ logger.warning( - "Changing '%s' channel from %s to %s. This may be a charm downgrade, " + "Changing '%s' channel from %s to %s to upgrade to %s. This may be a charm downgrade, " "which is generally not supported.", self.name, self.channel, - self.expected_current_channel, + self.expected_current_channel(target), + target, ) return PreUpgradeStep( f"WARNING: Changing '{self.name}' channel from {self.channel} to " - f"{self.expected_current_channel}. This may be a charm downgrade, " + f"{self.expected_current_channel(target)}. This may be a charm downgrade, " "which is generally not supported.", - coro=self.model.upgrade_charm(self.name, self.expected_current_channel), + coro=self.model.upgrade_charm(self.name, self.expected_current_channel(target)), ) def _get_refresh_current_channel_step(self) -> PreUpgradeStep: @@ -747,11 +783,7 @@ def _check_channel(self) -> None: :raises ApplicationError: Exception raised when channel is not a valid OpenStack channel. """ - if ( - self.is_from_charm_store - or self.channel == LATEST_STABLE - or self.is_valid_track(self.channel) - ): + if self.need_crossgrade or self.is_valid_track(self.channel): logger.debug("%s app has proper channel %s", self.name, self.channel) return diff --git a/cou/apps/channel_based.py b/cou/apps/channel_based.py index 58413cc1..2292ef28 100644 --- a/cou/apps/channel_based.py +++ b/cou/apps/channel_based.py @@ -28,6 +28,9 @@ class ChannelBasedApplication(OpenStackApplication): """Application for charms that are channel based.""" + # rely on the channel to evaluate current OpenStack release + based_on_channel = True + def get_latest_os_version(self, unit: Unit) -> OpenStackRelease: """Get the latest compatible OpenStack release based on the channel. diff --git a/cou/apps/subordinate.py b/cou/apps/subordinate.py index 52c002be..da6420a1 100644 --- a/cou/apps/subordinate.py +++ b/cou/apps/subordinate.py @@ -25,9 +25,25 @@ logger = logging.getLogger(__name__) -class SubordinateBase(OpenStackApplication): +@AppFactory.register_application(SUBORDINATES) +class SubordinateApplication(OpenStackApplication): """Subordinate base class.""" + # subordinate apps rely on the channel to evaluate current OpenStack release + based_on_channel = True + + @property + def current_os_release(self) -> OpenStackRelease: + """Infer the OpenStack release from subordinate charm's channel. + + We cannot determine the OpenStack release base on workload packages because the principal + charm has already upgraded the packages. + + :return: OpenStackRelease object. + :rtype: OpenStackRelease + """ + return self.channel_codename + def _check_application_target(self, target: OpenStackRelease) -> None: """Check if the application is already upgraded. @@ -87,27 +103,3 @@ def post_upgrade_steps( :rtype: list[PostUpgradeStep] """ return [] - - -@AppFactory.register_application(SUBORDINATES) -class SubordinateApplication(SubordinateBase): - """Subordinate application class.""" - - @property - def current_os_release(self) -> OpenStackRelease: - """Infer the OpenStack release from subordinate charm's channel. - - We cannot determine the OpenStack release base on workload packages because the principal - charm has already upgraded the packages. - - :return: OpenStackRelease object. - :rtype: OpenStackRelease - """ - if self.is_from_charm_store: - logger.debug( - "'%s' is from charm store and will be considered with channel codename as ussuri", - self.name, - ) - return OpenStackRelease("ussuri") - - return OpenStackRelease(self._get_track_from_channel(self.channel)) diff --git a/cou/steps/analyze.py b/cou/steps/analyze.py index ec200db4..d4b7cf0b 100644 --- a/cou/steps/analyze.py +++ b/cou/steps/analyze.py @@ -156,12 +156,24 @@ def __str__(self) -> str: def min_os_release_apps(apps: list[OpenStackApplication]) -> Optional[OpenStackRelease]: """Get the minimal OpenStack release from a list of applications. + - subordinates or channel based apps are not considered if not using release channels + - other apps are considered even if not using release channels + :param apps: Applications. :type apps: list[OpenStackApplication] :return: OpenStack release. :rtype: Optional[OpenStackRelease] """ - return min((app.current_os_release for app in apps), default=None) + # NOTE(gabrielcocenza) Apps based on channels to identify OpenStack release cannot + # be considered when on 'latest/stable' or from Charmstore because it's not reliable and + # will be considered as Ussuri. + apps_skipped = {app for app in apps if app.based_on_channel and app.need_crossgrade} + if apps_skipped: + logger.debug( + "%s were skipped from calculating cloud OpenStack release", + sorted(apps_skipped, key=lambda app: app.name), + ) + return min((app.current_os_release for app in set(apps) - apps_skipped), default=None) def _get_minimum_cloud_os_release(self) -> Optional[OpenStackRelease]: """Get the current minimum OpenStack release in the cloud. diff --git a/cou/steps/plan.py b/cou/steps/plan.py index 46f6ed45..dabd622f 100644 --- a/cou/steps/plan.py +++ b/cou/steps/plan.py @@ -35,7 +35,7 @@ from cou.apps.base import OpenStackApplication from cou.apps.channel_based import ChannelBasedApplication # noqa: F401 from cou.apps.core import Keystone, Octavia, Swift # noqa: F401 -from cou.apps.subordinate import SubordinateApplication, SubordinateBase # noqa: F401 +from cou.apps.subordinate import SubordinateApplication # noqa: F401 from cou.commands import CONTROL_PLANE, DATA_PLANE, HYPERVISORS, CLIargs from cou.exceptions import ( COUException, @@ -50,7 +50,7 @@ from cou.steps.analyze import Analysis from cou.steps.backup import backup from cou.steps.hypervisor import HypervisorUpgradePlanner -from cou.utils.app_utils import set_require_osd_release_option, stringify_units +from cou.utils.app_utils import set_require_osd_release_option from cou.utils.juju_utils import DEFAULT_TIMEOUT, Machine, Unit from cou.utils.nova_compute import get_empty_hypervisors from cou.utils.openstack import LTS_TO_OS_RELEASE, OpenStackRelease @@ -594,7 +594,9 @@ async def _get_upgradable_hypervisors_machines( ) if cli_force: - logger.info("Selected all hypervisors: %s", stringify_units(nova_compute_units)) + logger.info( + "Selected all hypervisors: %s", sorted(nova_compute_units, key=lambda unit: unit.name) + ) return nova_compute_machines return await get_empty_hypervisors(nova_compute_units, analysis_result.model) diff --git a/cou/utils/app_utils.py b/cou/utils/app_utils.py index 67656423..5acc3867 100644 --- a/cou/utils/app_utils.py +++ b/cou/utils/app_utils.py @@ -15,10 +15,10 @@ """Application utilities.""" import json import logging -from typing import Iterable, Optional +from typing import Optional from cou.exceptions import RunUpgradeError -from cou.utils.juju_utils import Model, Unit +from cou.utils.juju_utils import Model from cou.utils.openstack import CEPH_RELEASES logger = logging.getLogger(__name__) @@ -133,15 +133,3 @@ async def _get_current_osd_release(unit: str, model: Model) -> str: logger.debug("Currently OSDs are on the '%s' release", current_osd_release) return current_osd_release - - -def stringify_units(units: Iterable[Unit]) -> str: - """Convert Units into a comma-separated string of unit names, sorted alphabetically. - - :param units: An iterable of Unit objects to be converted. - :type units: Iterable[Unit] - :return: A comma-separated string of sorted unit names. - :rtype: str - """ - sorted_unit_names = sorted([unit.name for unit in units]) - return ", ".join(sorted_unit_names) diff --git a/cou/utils/juju_utils.py b/cou/utils/juju_utils.py index 38cffb30..6635bc35 100644 --- a/cou/utils/juju_utils.py +++ b/cou/utils/juju_utils.py @@ -150,6 +150,14 @@ class Unit: machine: Machine workload_version: str + def __repr__(self) -> str: + """App representation. + + :return: Name of the application + :rtype: str + """ + return self.name + @dataclass(frozen=True) class Application: diff --git a/cou/utils/nova_compute.py b/cou/utils/nova_compute.py index dfc6e38c..b0d8e386 100644 --- a/cou/utils/nova_compute.py +++ b/cou/utils/nova_compute.py @@ -18,7 +18,6 @@ import logging from cou.exceptions import HaltUpgradeExecution -from cou.utils.app_utils import stringify_units from cou.utils.juju_utils import Machine, Model, Unit logger = logging.getLogger(__name__) @@ -41,8 +40,10 @@ async def get_empty_hypervisors(units: list[Unit], model: Model) -> list[Machine skipped_units = set(units) - empty_units if skipped_units: - logger.warning("Found non-empty hypervisors: %s", stringify_units(skipped_units)) - logger.info("Selected hypervisors: %s", stringify_units(empty_units)) + logger.warning( + "Found non-empty hypervisors: %s", sorted(skipped_units, key=lambda unit: unit.name) + ) + logger.info("Selected hypervisors: %s", sorted(empty_units, key=lambda unit: unit.name)) return [unit.machine for unit in empty_units] diff --git a/tests/unit/apps/test_auxiliary.py b/tests/unit/apps/test_auxiliary.py index 8d6bee41..80d62ab5 100644 --- a/tests/unit/apps/test_auxiliary.py +++ b/tests/unit/apps/test_auxiliary.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. """Auxiliary application class.""" -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, PropertyMock, patch import pytest @@ -1415,3 +1415,35 @@ def test_need_current_channel_refresh_auxiliary( app_name, can_upgrade_to, app_name, "3.9/stable", {}, {}, model, "ch", "focal", [], {}, "1" ) assert app._need_current_channel_refresh(target) is exp_result + + +@pytest.mark.parametrize( + "channel, origin", + [ + ("latest/stable", "ch"), + ("latest", "cs"), + ("octopus/stable", "ch"), + ("pacific/stable", "ch"), + ], +) +@patch("cou.apps.base.OpenStackApplication.current_os_release", new_callable=PropertyMock) +def test_expected_current_channel_auxiliary(mock_os_release, model, channel, origin): + """Expected current channel is based on the OpenStack release of the workload version.""" + target = OpenStackRelease("wallaby") + mock_os_release.return_value = OpenStackRelease("victoria") + ceph_osd = CephOsd( + name="ceph-osd", + can_upgrade_to="octopus/stable", + charm="ceph-osd", + channel=channel, + config={}, + machines={}, + model=model, + origin=origin, + series="focal", + subordinate_to=[], + units={}, + workload_version="15.2.0", + ) + # expected_current_channel is indifferent if the charm needs crossgrade + assert ceph_osd.expected_current_channel(target) == "octopus/stable" diff --git a/tests/unit/apps/test_auxiliary_subordinate.py b/tests/unit/apps/test_auxiliary_subordinate.py index 09adcc95..b848f6e0 100644 --- a/tests/unit/apps/test_auxiliary_subordinate.py +++ b/tests/unit/apps/test_auxiliary_subordinate.py @@ -25,7 +25,7 @@ from cou.steps import ApplicationUpgradePlan, PreUpgradeStep, UpgradeStep from cou.utils.juju_utils import Machine from cou.utils.openstack import OpenStackRelease -from tests.unit.utils import assert_steps +from tests.unit.utils import assert_steps, dedent_plan def test_auxiliary_subordinate(model): @@ -312,3 +312,35 @@ def test_ceph_dashboard_upgrade_plan_xena_to_yoga(model): upgrade_plan = app.generate_upgrade_plan(target, False) assert_steps(upgrade_plan, expected_plan) + + +def test_auxiliary_subordinate_latest_stable(model): + target = OpenStackRelease("victoria") + + exp_plan = dedent_plan( + """\ + Upgrade plan for 'keystone-hacluster' to 'victoria' + WARNING: Changing 'keystone-hacluster' channel from latest/stable to 2.4/stable. \ +This may be a charm downgrade, which is generally not supported. + Upgrade 'keystone-hacluster' to the new channel: '2.4/stable' + """ + ) + + machines = {"0": MagicMock(spec_set=Machine)} + + app = AuxiliarySubordinateApplication( + name="keystone-hacluster", + can_upgrade_to="ch:amd64/focal/hacluster-131", + charm="hacluster", + channel="latest/stable", + config={}, + machines=machines, + model=model, + origin="ch", + series="focal", + subordinate_to=["nova-compute"], + units={}, + workload_version="", + ) + plan = app.generate_upgrade_plan(target, force=False) + assert str(plan) == exp_plan diff --git a/tests/unit/apps/test_base.py b/tests/unit/apps/test_base.py index 8333832a..e1f3da78 100644 --- a/tests/unit/apps/test_base.py +++ b/tests/unit/apps/test_base.py @@ -515,6 +515,7 @@ def test_check_mismatched_versions(mock_os_release_units, model): @patch("cou.apps.base.OpenStackApplication.current_os_release", new_callable=PropertyMock) def test_get_charmhub_migration_step(current_os_release, model): """Switch applications installed from charm store to a charmhub channel.""" + target = OpenStackRelease("victoria") current_os_release.return_value = OpenStackRelease("ussuri") app = OpenStackApplication( @@ -531,9 +532,11 @@ def test_get_charmhub_migration_step(current_os_release, model): units={}, workload_version="1", ) - assert app._get_charmhub_migration_step() == PreUpgradeStep( + assert app._get_charmhub_migration_step(target) == PreUpgradeStep( f"Migrate '{app.name}' from charmstore to charmhub", - coro=model.upgrade_charm(app.name, app.expected_current_channel, switch=f"ch:{app.charm}"), + coro=model.upgrade_charm( + app.name, app.expected_current_channel(target), switch=f"ch:{app.charm}" + ), ) @@ -541,6 +544,7 @@ def test_get_charmhub_migration_step(current_os_release, model): def test_get_change_to_openstack_channels_step(current_os_release, model): """Applications using latest/stable should be switched to a release-specific channel.""" current_os_release.return_value = OpenStackRelease("ussuri") + target = OpenStackRelease("victoria") app = OpenStackApplication( name="app", @@ -556,11 +560,10 @@ def test_get_change_to_openstack_channels_step(current_os_release, model): units={}, workload_version="1", ) - assert app._get_change_to_openstack_channels_step() == PreUpgradeStep( + assert app._get_change_to_openstack_channels_step(target) == PreUpgradeStep( f"WARNING: Changing '{app.name}' channel from {app.channel} to " - f"{app.expected_current_channel}. This may be a charm downgrade, " - "which is generally not supported.", - coro=model.upgrade_charm(app.name, app.expected_current_channel), + "ussuri/stable. This may be a charm downgrade, which is generally not supported.", + coro=model.upgrade_charm(app.name, app.expected_current_channel(target)), ) @@ -696,7 +699,7 @@ def test_get_refresh_charm_step_change_to_openstack_channels( assert app._get_refresh_charm_step(target) == expected_result mock_ch_migration.assert_not_called() - mock_change_os_channels.assert_called_once() + mock_change_os_channels.assert_called_once_with(target) mock_refresh_current_channel.assert_not_called() @@ -897,3 +900,61 @@ def test_apt_source_codename_unknown_source(source_value, model): with pytest.raises(ApplicationError, match=exp_msg): app.apt_source_codename + + +@pytest.mark.parametrize( + "channel, origin, exp_result", + [ + ("latest/stable", "ch", True), + ("ussuri/stable", "ch", False), + ("stable", "cs", True), + ("latest/edge", "ch", False), + ("foo/stable", "ch", False), + ], +) +def test_need_crossgrade(model, channel, origin, exp_result): + """Test if application need a crossgrade.""" + app = OpenStackApplication( + name="app", + can_upgrade_to="", + charm="app", + channel=channel, + config={}, + machines={}, + model=model, + origin=origin, + series="focal", + subordinate_to=[], + units={}, + workload_version="1", + ) + + assert app.need_crossgrade is exp_result + + +@pytest.mark.parametrize( + "channel, origin", [("latest/stable", "ch"), ("latest", "cs"), ("victoria/stable", "ch")] +) +@patch("cou.apps.base.OpenStackApplication.current_os_release", new_callable=PropertyMock) +def test_expected_current_channel(mock_os_release, model, channel, origin): + """Expected current channel is based on the OpenStack release of the workload version.""" + mock_os_release.return_value = OpenStackRelease("victoria") + target = OpenStackRelease("wallaby") + + app = OpenStackApplication( + name="app", + can_upgrade_to="", + charm="app", + channel=channel, + config={}, + machines={}, + model=model, + origin=origin, + series="focal", + subordinate_to=[], + units={}, + workload_version="1", + ) + + # expected_current_channel is indifferent if the charm needs crossgrade + assert app.expected_current_channel(target) == "victoria/stable" diff --git a/tests/unit/apps/test_channel_based.py b/tests/unit/apps/test_channel_based.py index e603c795..75edfdfd 100644 --- a/tests/unit/apps/test_channel_based.py +++ b/tests/unit/apps/test_channel_based.py @@ -12,6 +12,8 @@ from unittest.mock import MagicMock +import pytest + from cou.apps.channel_based import ChannelBasedApplication from cou.steps import ( ApplicationUpgradePlan, @@ -23,7 +25,7 @@ from cou.utils import app_utils from cou.utils.juju_utils import Machine, Unit from cou.utils.openstack import OpenStackRelease -from tests.unit.utils import assert_steps +from tests.unit.utils import assert_steps, dedent_plan def test_application_versionless(model): @@ -59,6 +61,54 @@ def test_application_versionless(model): assert app.get_latest_os_version(units["glance-simplestreams-sync/0"]) == app.channel_codename +def test_channel_based_application_latest_stable(model): + """Test channel based application using latest/stable channel.""" + target = OpenStackRelease("wallaby") + + exp_plan = dedent_plan( + """\ + Upgrade plan for 'glance-simplestreams-sync' to 'wallaby' + Upgrade software packages of 'glance-simplestreams-sync' from the current APT repositories + Upgrade software packages on unit 'glance-simplestreams-sync/0' + WARNING: Changing 'glance-simplestreams-sync' channel from latest/stable to victoria/stable. \ +This may be a charm downgrade, which is generally not supported. + Upgrade 'glance-simplestreams-sync' to the new channel: 'wallaby/stable' + Change charm config of 'glance-simplestreams-sync' 'openstack-origin' to 'cloud:focal-wallaby' + """ # noqa: E501 line too long + ) + + machines = {"0": MagicMock(spec_set=Machine)} + units = { + "glance-simplestreams-sync/0": Unit( + name="glance-simplestreams-sync/0", + workload_version="", + machine=machines["0"], + ) + } + app = ChannelBasedApplication( + name="glance-simplestreams-sync", + can_upgrade_to="", + charm="glance-simplestreams-sync", + channel="latest/stable", + config={ + "openstack-origin": {"value": "distro"}, + }, + machines=machines, + model=model, + origin="ch", + series="focal", + subordinate_to=[], + units=units, + workload_version="", + ) + # app is considered as ussuri because it's using latest/stable, but it won't be considered when + # calculating the cloud minimum OpenStack release. It will refresh the charm channel to + # whatever the minimum version of other components passed as a target. + assert app.current_os_release == "ussuri" + plan = app.generate_upgrade_plan(target, force=False) + assert str(plan) == exp_plan + + def test_application_gnocchi_ussuri(model): """Test the Gnocchi ChannelBasedApplication with Ussuri.""" machines = {"0": MagicMock(spec_set=Machine)} @@ -391,3 +441,47 @@ def test_application_designate_bind_upgrade_plan_ussuri_to_victoria(model): upgrade_plan = app.generate_upgrade_plan(target, False) assert_steps(upgrade_plan, expected_plan) + + +@pytest.mark.parametrize( + "channel, origin, release_target, exp_current_channel", + [ + # using latest/stable will always be N-1 from the target + ("latest/stable", "ch", "victoria", "ussuri/stable"), + ("latest/stable", "ch", "wallaby", "victoria/stable"), + ("latest/stable", "ch", "xena", "wallaby/stable"), + ("latest/stable", "ch", "yoga", "xena/stable"), + # from charmstore will always be N-1 from the target + ("latest", "cs", "victoria", "ussuri/stable"), + ("latest", "cs", "wallaby", "victoria/stable"), + ("latest", "cs", "xena", "wallaby/stable"), + ("latest", "cs", "yoga", "xena/stable"), + # when using release channel will always point to the channel track + ("ussuri/stable", "ch", "victoria", "ussuri/stable"), + ("victoria/stable", "ch", "wallaby", "victoria/stable"), + ("wallaby/stable", "ch", "xena", "wallaby/stable"), + ("xena/stable", "ch", "yoga", "xena/stable"), + ], +) +def test_expected_current_channel_channel_based( + model, channel, origin, release_target, exp_current_channel +): + """Test expected current channel for channel base apps.""" + target = OpenStackRelease(release_target) + app = ChannelBasedApplication( + name="app", + can_upgrade_to="", + charm="app", + channel=channel, + config={}, + machines={}, + model=model, + origin=origin, + series="focal", + subordinate_to=[], + units={}, + workload_version="1", + ) + + # expected_current_channel changes if the charm needs crossgrade + assert app.expected_current_channel(target) == exp_current_channel diff --git a/tests/unit/apps/test_subordinate.py b/tests/unit/apps/test_subordinate.py index 28678c39..eac6f66e 100644 --- a/tests/unit/apps/test_subordinate.py +++ b/tests/unit/apps/test_subordinate.py @@ -184,7 +184,7 @@ def test_generate_plan_ch_migration(model, channel): PreUpgradeStep( description=f"Migrate '{app.name}' from charmstore to charmhub", parallel=False, - coro=model.upgrade_charm(app.name, "ussuri/stable", switch="ch:keystone-ldap"), + coro=model.upgrade_charm(app.name, "victoria/stable", switch="ch:keystone-ldap"), ), UpgradeStep( description=f"Upgrade '{app.name}' to the new channel: 'wallaby/stable'", @@ -284,3 +284,45 @@ def test_generate_plan_in_same_version(model, from_to): upgrade_plan = app.generate_upgrade_plan(target, False) assert_steps(upgrade_plan, expected_plan) + + +@pytest.mark.parametrize( + "channel, origin, release_target, exp_current_channel", + [ + # using latest/stable will always be N-1 from the target + ("latest/stable", "ch", "victoria", "ussuri/stable"), + ("latest/stable", "ch", "wallaby", "victoria/stable"), + ("latest/stable", "ch", "xena", "wallaby/stable"), + ("latest/stable", "ch", "yoga", "xena/stable"), + # from charmstore will always be N-1 from the target + ("latest", "cs", "victoria", "ussuri/stable"), + ("latest", "cs", "wallaby", "victoria/stable"), + ("latest", "cs", "xena", "wallaby/stable"), + ("latest", "cs", "yoga", "xena/stable"), + # when using release channel will always point to the channel track + ("ussuri/stable", "ch", "victoria", "ussuri/stable"), + ("victoria/stable", "ch", "wallaby", "victoria/stable"), + ("wallaby/stable", "ch", "xena", "wallaby/stable"), + ("xena/stable", "ch", "yoga", "xena/stable"), + ], +) +def test_expected_current_channel_subordinate( + model, channel, origin, release_target, exp_current_channel +): + target = OpenStackRelease(release_target) + app = SubordinateApplication( + name="app", + can_upgrade_to="", + charm="app", + channel=channel, + config={}, + machines={}, + model=model, + origin=origin, + series="focal", + subordinate_to=["keystone"], + units={}, + workload_version="1", + ) + + assert app.expected_current_channel(target) == exp_current_channel diff --git a/tests/unit/steps/test_analyze.py b/tests/unit/steps/test_analyze.py index 035117ab..0a9f8849 100644 --- a/tests/unit/steps/test_analyze.py +++ b/tests/unit/steps/test_analyze.py @@ -18,6 +18,7 @@ from cou.apps.auxiliary import RabbitMQServer from cou.apps.base import OpenStackApplication +from cou.apps.channel_based import ChannelBasedApplication from cou.apps.core import Keystone from cou.apps.subordinate import SubordinateApplication from cou.steps import analyze @@ -536,3 +537,64 @@ def test_split_apps(exp_control_plane, exp_data_plane): control_plane, data_plane = Analysis._split_apps(all_apps) assert exp_control_plane == control_plane assert exp_data_plane == data_plane + + +@pytest.mark.parametrize( + "channel_keystone, channel_gnocchi, origin, exp_release", + [ + # when a channel based app (e.g: gnocchi) doesn't need to crossgrade, it's considered on + # the calculation of the cloud OpenStack release + ("wallaby/stable", "ussuri/stable", "ch", "ussuri"), + # when a channel based app (e.g: gnocchi, subordinates and etc) need to crossgrade, + # it's NOT considered on the calculation of the cloud OpenStack release + ("latest", "latest", "cs", "wallaby"), + ("latest/stable", "latest/stable", "ch", "wallaby"), + ], +) +def test_min_os_release_apps(model, channel_keystone, channel_gnocchi, origin, exp_release): + """Test to evaluate the Openstack release from a list of apps.""" + machines = {f"{i}": generate_cou_machine(f"{i}") for i in range(3)} + + keystone = Keystone( + name="keystone", + can_upgrade_to="", + charm="keystone", + channel=channel_keystone, + config={"source": {"value": "cloud:focal-wallaby"}}, + machines=machines, + model=model, + origin=origin, + series="focal", + subordinate_to=[], + units={ + "keystone/0": Unit( + name="keystone/0", + workload_version="19.1.0", + machine=machines["0"], + ) + }, + workload_version="19.1.0", + ) + + gnocchi = ChannelBasedApplication( + name="gnocchi", + can_upgrade_to="", + charm="gnocchi", + channel=channel_gnocchi, + config={}, + machines=machines, + model=model, + origin=origin, + series="focal", + subordinate_to=[], + units={ + "gnocchi": Unit( + name="gnocchi/0", + workload_version="4.3.0", + machine=machines["1"], + ) + }, + workload_version="4.3.0", + ) + + assert Analysis.min_os_release_apps([keystone, gnocchi]) == exp_release diff --git a/tests/unit/utils/test_juju_utils.py b/tests/unit/utils/test_juju_utils.py index 4d64552d..9702d6cb 100644 --- a/tests/unit/utils/test_juju_utils.py +++ b/tests/unit/utils/test_juju_utils.py @@ -725,3 +725,8 @@ async def test_get_applications(mock_get_machines, mock_get_status, mocked_model assert len(apps["app2"].machines) == 1 assert len(apps["app3"].machines) == 1 assert len(apps["app4"].machines) == 1 + + +def test_unit_repr(): + unit = juju_utils.Unit(name="foo/0", machine=MagicMock(), workload_version="1") + assert repr(unit) == "foo/0" diff --git a/tests/unit/utils/test_nova_compute.py b/tests/unit/utils/test_nova_compute.py index 77178f15..76114cfa 100644 --- a/tests/unit/utils/test_nova_compute.py +++ b/tests/unit/utils/test_nova_compute.py @@ -69,19 +69,17 @@ async def test_get_empty_hypervisors( mock_instance_count, mock_logger, hypervisors_count, expected_result, model ): mock_instance_count.side_effect = [count for _, count in hypervisors_count] - result = await nova_compute.get_empty_hypervisors( - [_mock_nova_unit(nova_unit) for nova_unit, _ in hypervisors_count], model - ) + selected_hypervisors = [ + _mock_nova_unit(nova_unit) for nova_unit, count in hypervisors_count if count == 0 + ] + non_empty_hypervisors = [ + _mock_nova_unit(nova_unit) for nova_unit, count in hypervisors_count if count != 0 + ] + units = sorted(selected_hypervisors + non_empty_hypervisors, key=lambda unit: unit.name) + result = await nova_compute.get_empty_hypervisors(units, model) assert {machine.machine_id for machine in result} == expected_result - selected_hypervisors = ", ".join( - [f"nova-compute/{i}" for i, count in hypervisors_count if count == 0] - ) - non_empty_hypervisors = ", ".join( - [f"nova-compute/{i}" for i, count in hypervisors_count if count != 0] - ) - mock_logger.info.assert_called_once_with("Selected hypervisors: %s", selected_hypervisors) if non_empty_hypervisors: mock_logger.warning.assert_called_once_with(