diff --git a/cou/apps/auxiliary.py b/cou/apps/auxiliary.py index dbc07373..c4da6301 100644 --- a/cou/apps/auxiliary.py +++ b/cou/apps/auxiliary.py @@ -49,9 +49,6 @@ def is_valid_track(self, charm_channel: str) -> bool: :return: True if valid, False otherwise. :rtype: bool """ - if self.is_from_charm_store: - logger.debug("'%s' has been installed from the charm store", self.name) - return True current_track = self._get_track_from_channel(charm_channel) possible_tracks = OPENSTACK_TO_TRACK_MAPPING.get( @@ -107,26 +104,15 @@ def target_channel(self, target: OpenStackRelease) -> str: ) ) - @property - def channel_codename(self) -> OpenStackRelease: - """Identify the OpenStack release set in the charm channel. + def _get_os_release_from_channel(self, channel: str) -> OpenStackRelease: + """Get the OpenStack release from a channel - Auxiliary charms can have multiple compatible OpenStack releases. In - that case, return the latest compatible OpenStack version. - - :return: OpenStackRelease object + :param channel: channel to get the release + :type channel: str + :return: OpenStack release that the channel points to :rtype: OpenStackRelease - :raises ApplicationError: When cannot identify suitable OpenStack release codename - based on the track of the charm channel. """ - if self.need_crossgrade: - logger.debug( - "Cannot determine the OpenStack release of '%s' via its channel. Assuming Ussuri", - self.name, - ) - return OpenStackRelease("ussuri") - - track: str = self._get_track_from_channel(self.channel) + track: str = self._get_track_from_channel(channel) compatible_os_releases = TRACK_TO_OPENSTACK_MAPPING[(self.charm, self.series, track)] if not compatible_os_releases: diff --git a/cou/apps/base.py b/cou/apps/base.py index 05a4ff53..a3d47a01 100644 --- a/cou/apps/base.py +++ b/cou/apps/base.py @@ -20,7 +20,7 @@ import os from collections import defaultdict from dataclasses import dataclass, field -from typing import Any, Optional +from typing import Any, Optional, Union import yaml @@ -184,7 +184,7 @@ def _extract_from_uca_source(self) -> OpenStackRelease: ) from exc @property - def channel_codename(self) -> OpenStackRelease: + def current_channel_os_release(self) -> OpenStackRelease: """Identify the OpenStack release set in the charm channel. :return: OpenStackRelease object @@ -197,9 +197,17 @@ def channel_codename(self) -> OpenStackRelease: self.name, ) return OpenStackRelease("ussuri") + return self._get_os_release_from_channel(self.channel) - # get the OpenStack release from the channel track of the application. - return OpenStackRelease(self._get_track_from_channel(self.channel)) + def _get_os_release_from_channel(self, channel: str) -> OpenStackRelease: + """Get the OpenStack release from a channel + + :param channel: channel to get the release + :type channel: str + :return: OpenStack release that the channel points to + :rtype: OpenStackRelease + """ + return OpenStackRelease(self._get_track_from_channel(channel)) @property def current_os_release(self) -> OpenStackRelease: @@ -332,6 +340,9 @@ def target_channel(self, target: OpenStackRelease) -> str: """ return f"{target.codename}/stable" + def target_channel_os_release(self, target) -> OpenStackRelease: + return self._get_os_release_from_channel(self.target_channel(target)) + def new_origin(self, target: OpenStackRelease) -> str: """Return the new openstack-origin or source configuration. @@ -556,7 +567,9 @@ def _get_refresh_charm_step(self, target: OpenStackRelease) -> PreUpgradeStep: if self.is_from_charm_store: return self._get_charmhub_migration_step(target) if self.channel == LATEST_STABLE: - return self._get_change_to_openstack_channels_step(target) + return self._get_change_channel_possible_downgrade_step( + target, self.expected_current_channel(target), PreUpgradeStep + ) if self._need_current_channel_refresh(target): return self._get_refresh_current_channel_step() logger.info( @@ -579,12 +592,14 @@ def _get_charmhub_migration_step(self, target: OpenStackRelease) -> PreUpgradeSt ), ) - def _get_change_to_openstack_channels_step(self, target: OpenStackRelease) -> PreUpgradeStep: - """Get the step for changing to an OpenStack channel. + def _get_change_channel_possible_downgrade_step( + self, target: OpenStackRelease, channel: str, step: Union[PreUpgradeStep, UpgradeStep] + ) -> Union[PreUpgradeStep, UpgradeStep]: + """Get the step for changing to a channel that can be a downgrade. :param target: OpenStack release as target to upgrade. :type target: OpenStackRelease - :return: Step for changing to OpenStack channels + :return: Step for changing to a channel that can be a downgrade :rtype: PreUpgradeStep """ logger.warning( @@ -592,15 +607,14 @@ def _get_change_to_openstack_channels_step(self, target: OpenStackRelease) -> Pr "which is generally not supported.", self.name, self.channel, - self.expected_current_channel(target), + channel, target, ) - return PreUpgradeStep( + msg = ( f"WARNING: Changing '{self.name}' channel from {self.channel} to " - 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(target)), + f"{channel}. This may be a charm downgrade, which is generally not supported." ) + return step(msg, coro=self.model.upgrade_charm(self.name, channel)) def _get_refresh_current_channel_step(self) -> PreUpgradeStep: """Get step for refreshing the current channel. @@ -621,7 +635,7 @@ def _need_current_channel_refresh(self, target: OpenStackRelease) -> bool: :return: True if needs to refresh, False otherwise :rtype: bool """ - return bool(self.can_upgrade_to) and self.channel_codename <= target + return bool(self.can_upgrade_to) and self.current_channel_os_release <= target def _get_upgrade_charm_step(self, target: OpenStackRelease) -> UpgradeStep: """Get step for upgrading the charm. @@ -631,12 +645,19 @@ def _get_upgrade_charm_step(self, target: OpenStackRelease) -> UpgradeStep: :return: Step for upgrading the charm. :rtype: UpgradeStep """ - if self.channel != self.target_channel(target): + if self.current_channel_os_release <= self.target_channel_os_release( + target + ) and self.channel != self.target_channel(target): return UpgradeStep( - description=f"Upgrade '{self.name}' to the new channel: " + description=f"Upgrade '{self.name}' from '{self.channel}' to the new channel: " f"'{self.target_channel(target)}'", coro=self.model.upgrade_charm(self.name, self.target_channel(target)), ) + if self.current_channel_os_release > self.target_channel_os_release(target): + return self._get_change_channel_possible_downgrade_step( + target, self.target_channel(target), UpgradeStep + ) + logger.debug("%s does not need to upgrade the channel", self.name) return UpgradeStep() def _set_action_managed_upgrade(self, enable: bool) -> UpgradeStep: diff --git a/cou/apps/channel_based.py b/cou/apps/channel_based.py index 2292ef28..3fefb209 100644 --- a/cou/apps/channel_based.py +++ b/cou/apps/channel_based.py @@ -39,7 +39,7 @@ def get_latest_os_version(self, unit: Unit) -> OpenStackRelease: :return: The latest compatible OpenStack release. :rtype: OpenStackRelease """ - return self.channel_codename + return self.current_channel_os_release @property def current_os_release(self) -> OpenStackRelease: @@ -48,7 +48,7 @@ def current_os_release(self) -> OpenStackRelease: :return: OpenStackRelease object :rtype: OpenStackRelease """ - return self.channel_codename + return self.current_channel_os_release @property def is_versionless(self) -> bool: diff --git a/cou/apps/subordinate.py b/cou/apps/subordinate.py index da6420a1..50fe1373 100644 --- a/cou/apps/subordinate.py +++ b/cou/apps/subordinate.py @@ -42,7 +42,7 @@ def current_os_release(self) -> OpenStackRelease: :return: OpenStackRelease object. :rtype: OpenStackRelease """ - return self.channel_codename + return self.current_channel_os_release def _check_application_target(self, target: OpenStackRelease) -> None: """Check if the application is already upgraded. diff --git a/tests/functional/tests/smoke.py b/tests/functional/tests/smoke.py index 4e015256..731a9a68 100644 --- a/tests/functional/tests/smoke.py +++ b/tests/functional/tests/smoke.py @@ -141,7 +141,7 @@ def generate_expected_plan(self, backup: bool = True) -> str: "\t\t\tUpgrade software packages of 'designate-bind' " "from the current APT repositories\n" "\t\t\t\tΨ Upgrade software packages on unit 'designate-bind/0'\n" - "\t\t\tUpgrade 'designate-bind' to the new channel: 'victoria/stable'\n" + "\t\t\tUpgrade 'designate-bind' from 'ussuri/stable' to the new channel: 'victoria/stable'\n" "\t\t\tWait for up to 300s for app 'designate-bind' to reach the idle state\n" "\t\t\tVerify that the workload of 'designate-bind' has been upgraded on units:" " designate-bind/0\n" diff --git a/tests/unit/apps/test_auxiliary.py b/tests/unit/apps/test_auxiliary.py index e612c3d7..45189717 100644 --- a/tests/unit/apps/test_auxiliary.py +++ b/tests/unit/apps/test_auxiliary.py @@ -70,7 +70,7 @@ def test_auxiliary_app(model): assert app.is_valid_track(app.channel) is True assert app.os_origin == "distro" assert app.apt_source_codename == "ussuri" - assert app.channel_codename == "yoga" + assert app.current_channel_os_release == "yoga" assert app.is_subordinate is False assert app.current_os_release == "yoga" @@ -100,10 +100,10 @@ def test_auxiliary_app_cs(model): ) assert app.channel == "stable" - assert app.is_valid_track(app.channel) is True + assert app.is_valid_track(app.channel) is False assert app.os_origin == "distro" assert app.apt_source_codename == "ussuri" - assert app.channel_codename == "ussuri" + assert app.current_channel_os_release == "ussuri" assert app.current_os_release == "yoga" @@ -154,7 +154,7 @@ def test_auxiliary_upgrade_plan_ussuri_to_victoria_change_channel(model): coro=model.upgrade_charm(app.name, "3.8/stable"), ), UpgradeStep( - description=f"Upgrade '{app.name}' to the new channel: '3.9/stable'", + description=f"Upgrade '{app.name}' from '3.8/stable' to the new channel: '3.9/stable'", parallel=False, coro=model.upgrade_charm(app.name, "3.9/stable"), ), @@ -262,7 +262,7 @@ def test_auxiliary_upgrade_plan_ussuri_to_victoria_ch_migration(model): machines = {"0": MagicMock(spec_set=Machine)} app = RabbitMQServer( name="rabbitmq-server", - can_upgrade_to="3.9/stable", + can_upgrade_to="cs:rabbitmq-server", charm="rabbitmq-server", channel="stable", config={"source": {"value": "distro"}}, @@ -301,7 +301,7 @@ def test_auxiliary_upgrade_plan_ussuri_to_victoria_ch_migration(model): coro=model.upgrade_charm(app.name, "3.9/stable", switch="ch:rabbitmq-server"), ), UpgradeStep( - description=f"Upgrade '{app.name}' to the new channel: '3.9/stable'", + description=f"Upgrade '{app.name}' from 'stable' to the new channel: '3.9/stable'", parallel=False, coro=model.upgrade_charm(app.name, "3.9/stable"), ), @@ -624,7 +624,7 @@ def test_ceph_mon_app(model): assert app.os_origin == "cloud:focal-xena" assert app.get_latest_os_version(app.units[f"{charm}/0"]) == OpenStackRelease("xena") assert app.apt_source_codename == "xena" - assert app.channel_codename == "xena" + assert app.current_channel_os_release == "xena" assert app.is_subordinate is False @@ -681,7 +681,7 @@ def test_ceph_mon_upgrade_plan_xena_to_yoga(model): coro=app_utils.set_require_osd_release_option("ceph-mon/0", model), ), UpgradeStep( - description=f"Upgrade '{app.name}' to the new channel: 'quincy/stable'", + description=f"Upgrade '{app.name}' from 'pacific/stable' to the new channel: 'quincy/stable'", parallel=False, coro=model.upgrade_charm(app.name, "quincy/stable"), ), @@ -817,7 +817,7 @@ def test_ovn_principal(model): assert app.channel == "22.03/stable" assert app.os_origin == "distro" assert app.apt_source_codename == "ussuri" - assert app.channel_codename == "yoga" + assert app.current_channel_os_release == "yoga" assert app.current_os_release == "yoga" assert app.is_subordinate is False @@ -1447,3 +1447,33 @@ def test_expected_current_channel_auxiliary(mock_os_release, model, channel, ori ) # expected_current_channel is indifferent if the charm needs crossgrade assert ceph_osd.expected_current_channel(target) == "octopus/stable" + + +def test_auxiliary_wrong_channel(model): + """Test when an auxiliary charm is with a channel that doesn't match the workload version.""" + target = OpenStackRelease("victoria") + charm = "ceph-mon" + machines = {"0": MagicMock(spec_set=Machine)} + app = CephMon( + name=charm, + can_upgrade_to="asfd", + charm=charm, + channel="quincy/stable", + config={"source": {"value": "distro"}}, + machines=machines, + model=model, + origin="ch", + series="focal", + subordinate_to=[], + units={ + f"{charm}/0": Unit( + name=f"{charm}/0", + workload_version="15.2.0", + machine=machines["0"], + ) + }, + workload_version="15.2.0", + ) + plan = app.generate_upgrade_plan(target, force=False) + print(plan) + assert 0 diff --git a/tests/unit/apps/test_auxiliary_subordinate.py b/tests/unit/apps/test_auxiliary_subordinate.py index 5df4ebbf..a3f7897e 100644 --- a/tests/unit/apps/test_auxiliary_subordinate.py +++ b/tests/unit/apps/test_auxiliary_subordinate.py @@ -50,7 +50,7 @@ def test_auxiliary_subordinate(model): assert app.origin == "ch" assert app.os_origin == "" assert app.apt_source_codename == "yoga" - assert app.channel_codename == "yoga" + assert app.current_channel_os_release == "yoga" assert app.current_os_release == "yoga" assert app.is_subordinate is True @@ -109,7 +109,7 @@ def test_ovn_subordinate(model): assert app.channel == "22.03/stable" assert app.os_origin == "" assert app.apt_source_codename == "yoga" - assert app.channel_codename == "yoga" + assert app.current_channel_os_release == "yoga" assert app.current_os_release == "yoga" assert app.is_subordinate is True @@ -302,7 +302,7 @@ def test_ceph_dashboard_upgrade_plan_xena_to_yoga(model): coro=model.upgrade_charm(app.name, "pacific/stable"), ), UpgradeStep( - description=f"Upgrade '{app.name}' to the new channel: 'quincy/stable'", + description=f"Upgrade '{app.name} from 'pacific/stable' to the new channel: 'quincy/stable'", parallel=False, coro=model.upgrade_charm(app.name, "quincy/stable"), ), @@ -322,7 +322,7 @@ def test_auxiliary_subordinate_latest_stable(model): 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' + Upgrade 'keystone-hacluster' from 'latest/stable' to the new channel: '2.4/stable' """ ) @@ -346,7 +346,7 @@ def test_auxiliary_subordinate_latest_stable(model): assert str(plan) == exp_plan -def test_auxiliary_subordinate_channel_codename_raise(model): +def test_auxiliary_subordinate_current_channel_os_release_raise(model): app = AuxiliarySubordinateApplication( name="ceph-dashboard", can_upgrade_to="", @@ -370,7 +370,7 @@ def test_auxiliary_subordinate_channel_codename_raise(model): ) with pytest.raises(ApplicationError, match=exp_msg): - app.channel_codename + app.current_channel_os_release with pytest.raises(ApplicationError, match=exp_msg): app.current_os_release diff --git a/tests/unit/apps/test_base.py b/tests/unit/apps/test_base.py index 8302a637..3910cc87 100644 --- a/tests/unit/apps/test_base.py +++ b/tests/unit/apps/test_base.py @@ -541,7 +541,7 @@ def test_get_charmhub_migration_step(current_os_release, model): @patch("cou.apps.base.OpenStackApplication.current_os_release", new_callable=PropertyMock) -def test_get_change_to_openstack_channels_step(current_os_release, model): +def test_get_change_channel_possible_downgrade_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") @@ -560,10 +560,12 @@ def test_get_change_to_openstack_channels_step(current_os_release, model): units={}, workload_version="1", ) - assert app._get_change_to_openstack_channels_step(target) == PreUpgradeStep( - f"WARNING: Changing '{app.name}' channel from {app.channel} to " + assert app._get_change_channel_possible_downgrade_step( + target, app.expected_current_channel(target), PreUpgradeStep + ) == PreUpgradeStep( + f"WARNING: Changing '{app.name}' channel from latest/stable to " "ussuri/stable. This may be a charm downgrade, which is generally not supported.", - coro=model.upgrade_charm(app.name, app.expected_current_channel(target)), + coro=model.upgrade_charm(app.name, "ussuri/stable"), ) @@ -592,7 +594,7 @@ def test_get_refresh_current_channel_step(model): @patch("cou.apps.base.OpenStackApplication._get_refresh_current_channel_step") -@patch("cou.apps.base.OpenStackApplication._get_change_to_openstack_channels_step") +@patch("cou.apps.base.OpenStackApplication._get_change_channel_possible_downgrade_step") @patch("cou.apps.base.OpenStackApplication._get_charmhub_migration_step") def test_get_refresh_charm_step_skip( mock_ch_migration, mock_change_os_channels, mock_refresh_current_channel, model @@ -621,7 +623,7 @@ def test_get_refresh_charm_step_skip( @patch("cou.apps.base.OpenStackApplication._get_refresh_current_channel_step") -@patch("cou.apps.base.OpenStackApplication._get_change_to_openstack_channels_step") +@patch("cou.apps.base.OpenStackApplication._get_change_channel_possible_downgrade_step") @patch( "cou.apps.base.OpenStackApplication._get_charmhub_migration_step", ) @@ -659,7 +661,7 @@ def test_get_refresh_charm_step_refresh_current_channel( @patch("cou.apps.base.OpenStackApplication._get_refresh_current_channel_step") -@patch("cou.apps.base.OpenStackApplication._get_change_to_openstack_channels_step") +@patch("cou.apps.base.OpenStackApplication._get_change_channel_possible_downgrade_step") @patch("cou.apps.base.OpenStackApplication._get_charmhub_migration_step") @patch("cou.apps.base.OpenStackApplication.current_os_release", new_callable=PropertyMock) def test_get_refresh_charm_step_change_to_openstack_channels( @@ -704,7 +706,7 @@ def test_get_refresh_charm_step_change_to_openstack_channels( @patch("cou.apps.base.OpenStackApplication._get_refresh_current_channel_step") -@patch("cou.apps.base.OpenStackApplication._get_change_to_openstack_channels_step") +@patch("cou.apps.base.OpenStackApplication._get_change_channel_possible_downgrade_step") @patch("cou.apps.base.OpenStackApplication._get_charmhub_migration_step") @patch("cou.apps.base.OpenStackApplication.current_os_release", new_callable=PropertyMock) def test_get_refresh_charm_step_charmhub_migration( diff --git a/tests/unit/apps/test_channel_based.py b/tests/unit/apps/test_channel_based.py index 71473a47..33efdd37 100644 --- a/tests/unit/apps/test_channel_based.py +++ b/tests/unit/apps/test_channel_based.py @@ -58,7 +58,10 @@ def test_application_versionless(model): assert app.current_os_release == "ussuri" assert app.is_versionless is True - assert app.get_latest_os_version(units["glance-simplestreams-sync/0"]) == app.channel_codename + assert ( + app.get_latest_os_version(units["glance-simplestreams-sync/0"]) + == app.current_channel_os_release + ) def test_channel_based_application_latest_stable(model): @@ -72,7 +75,7 @@ def test_channel_based_application_latest_stable(model): Ψ 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' + Upgrade 'glance-simplestreams-sync' from 'ussuri/stable' to the new channel: 'wallaby/stable' Change charm config of 'glance-simplestreams-sync' 'openstack-origin' to 'cloud:focal-wallaby' """ # noqa: E501 line too long ) @@ -254,7 +257,7 @@ def test_application_versionless_upgrade_plan_ussuri_to_victoria(model): coro=model.upgrade_charm(app.name, "ussuri/stable"), ), UpgradeStep( - description=f"Upgrade '{app.name}' to the new channel: 'victoria/stable'", + description=f"Upgrade '{app.name}' from 'ussuri/stable' to the new channel: 'victoria/stable'", parallel=False, coro=model.upgrade_charm(app.name, "victoria/stable"), ), @@ -328,7 +331,7 @@ def test_application_gnocchi_upgrade_plan_ussuri_to_victoria(model): coro=model.upgrade_charm(app.name, "ussuri/stable"), ), UpgradeStep( - description=f"Upgrade '{app.name}' to the new channel: 'victoria/stable'", + description=f"Upgrade '{app.name}' from 'ussuri/stable' to the new channel: 'victoria/stable'", parallel=False, coro=model.upgrade_charm(app.name, "victoria/stable"), ), @@ -411,7 +414,7 @@ def test_application_designate_bind_upgrade_plan_ussuri_to_victoria(model): coro=model.upgrade_charm(app.name, "ussuri/stable"), ), UpgradeStep( - description=f"Upgrade '{app.name}' to the new channel: 'victoria/stable'", + description=f"Upgrade '{app.name}' from 'ussuri/stable' to the new channel: 'victoria/stable'", parallel=False, coro=model.upgrade_charm(app.name, "victoria/stable"), ),