fix unit tests

cou/apps/auxiliary.py

@@ -49,7 +49,6 @@ def is_valid_track(self, charm_channel: str) -> bool:
         :return: True if valid, False otherwise.
         :rtype: bool
         """
-
         current_track = self._get_track_from_channel(charm_channel)
         possible_tracks = OPENSTACK_TO_TRACK_MAPPING.get(
             (self.charm, self.series, self.current_os_release.codename), []
@@ -105,12 +104,16 @@ def target_channel(self, target: OpenStackRelease) -> str:
         )
 
     def _get_os_release_from_channel(self, channel: str) -> OpenStackRelease:
-        """Get the OpenStack release from a channel
+        """Get the OpenStack release from a channel.
+
+        Auxiliary charms can have multiple compatible OpenStack releases. In that case, return the
+        latest compatible OpenStack version.
 
         :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
         """
         track: str = self._get_track_from_channel(channel)
         compatible_os_releases = TRACK_TO_OPENSTACK_MAPPING[(self.charm, self.series, track)]

cou/apps/base.py

@@ -20,7 +20,7 @@
 import os
 from collections import defaultdict
 from dataclasses import dataclass, field
-from typing import Any, Optional, Union
+from typing import Any, Coroutine, Optional
 
 import yaml
 
@@ -200,7 +200,7 @@ def current_channel_os_release(self) -> OpenStackRelease:
         return self._get_os_release_from_channel(self.channel)
 
     def _get_os_release_from_channel(self, channel: str) -> OpenStackRelease:
-        """Get the OpenStack release from a channel
+        """Get the OpenStack release from a channel.
 
         :param channel: channel to get the release
         :type channel: str
@@ -340,9 +340,6 @@ 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.
 
@@ -567,9 +564,11 @@ 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_channel_possible_downgrade_step(
-                target, self.expected_current_channel(target), PreUpgradeStep
+            description, coro = self._get_change_channel_possible_downgrade_step(
+                target, self.expected_current_channel(target)
             )
+            return PreUpgradeStep(description=description, coro=coro)
+
         if self._need_current_channel_refresh(target):
             return self._get_refresh_current_channel_step()
         logger.info(
@@ -593,14 +592,16 @@ def _get_charmhub_migration_step(self, target: OpenStackRelease) -> PreUpgradeSt
         )
 
     def _get_change_channel_possible_downgrade_step(
-        self, target: OpenStackRelease, channel: str, step: Union[PreUpgradeStep, UpgradeStep]
-    ) -> Union[PreUpgradeStep, UpgradeStep]:
+        self, target: OpenStackRelease, channel: str
+    ) -> tuple[str, Coroutine]:
         """Get the step for changing to a channel that can be a downgrade.
 
-        :param target: OpenStack release as target to upgrade.
+        :param target:  OpenStack release as target to upgrade
         :type target: OpenStackRelease
-        :return: Step for changing to a channel that can be a downgrade
-        :rtype: PreUpgradeStep
+        :param channel: channel to upgrade
+        :type channel: str
+        :return: tuple containing the message and the coroutine.
+        :rtype: tuple[str, Coroutine]
         """
         logger.warning(
             "Changing '%s' channel from %s to %s to upgrade to %s. This may be a charm downgrade, "
@@ -614,7 +615,7 @@ def _get_change_channel_possible_downgrade_step(
             f"WARNING: Changing '{self.name}' channel from {self.channel} to "
             f"{channel}. This may be a charm downgrade, which is generally not supported."
         )
-        return step(msg, coro=self.model.upgrade_charm(self.name, channel))
+        return msg, self.model.upgrade_charm(self.name, channel)
 
     def _get_refresh_current_channel_step(self) -> PreUpgradeStep:
         """Get step for refreshing the current channel.
@@ -645,18 +646,27 @@ def _get_upgrade_charm_step(self, target: OpenStackRelease) -> UpgradeStep:
         :return: Step for upgrading the charm.
         :rtype: UpgradeStep
         """
-        if self.current_channel_os_release <= self.target_channel_os_release(
-            target
-        ) and self.channel != self.target_channel(target):
+        channel = self.expected_current_channel(target) if self.need_crossgrade else self.channel
+
+        if (
+            self.current_channel_os_release <= self.current_os_release
+            and channel != self.target_channel(target)
+        ):
             return UpgradeStep(
-                description=f"Upgrade '{self.name}' from '{self.channel}' to the new channel: "
+                description=f"Upgrade '{self.name}' from '{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
+
+        if (
+            self.current_channel_os_release > self.current_os_release
+            and channel != self.target_channel(target)
+        ):
+            description, coro = self._get_change_channel_possible_downgrade_step(
+                target, self.target_channel(target)
             )
+            return UpgradeStep(description=description, coro=coro)
+
         logger.debug("%s does not need to upgrade the channel", self.name)
         return UpgradeStep()
 

tests/functional/tests/smoke.py

@@ -141,7 +141,8 @@ 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' from 'ussuri/stable' 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"

tests/unit/apps/test_auxiliary.py

@@ -300,11 +300,6 @@ def test_auxiliary_upgrade_plan_ussuri_to_victoria_ch_migration(model):
             f"Migrate '{app.name}' from charmstore to charmhub",
             coro=model.upgrade_charm(app.name, "3.9/stable", switch="ch:rabbitmq-server"),
         ),
-        UpgradeStep(
-            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"),
-        ),
         UpgradeStep(
             description=f"Change charm config of '{app.name}' "
             f"'{app.origin_setting}' to 'cloud:focal-victoria'",
@@ -681,7 +676,8 @@ 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}' from 'pacific/stable' 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"),
         ),
@@ -1456,7 +1452,7 @@ def test_auxiliary_wrong_channel(model):
     machines = {"0": MagicMock(spec_set=Machine)}
     app = CephMon(
         name=charm,
-        can_upgrade_to="asfd",
+        can_upgrade_to="",
         charm=charm,
         channel="quincy/stable",
         config={"source": {"value": "distro"}},
@@ -1474,6 +1470,20 @@ def test_auxiliary_wrong_channel(model):
         },
         workload_version="15.2.0",
     )
+
+    # plan will warn that channel will change from quincy to octopus to match the workload version,
+    # but this can be a downgrade. COU are adjusting the channel based
+    exp_plan = dedent_plan(
+        """\
+    Upgrade plan for 'ceph-mon' to 'victoria'
+        Upgrade software packages of 'ceph-mon' from the current APT repositories
+            Ψ Upgrade software packages on unit 'ceph-mon/0'
+        Ensure that the 'require-osd-release' option matches the 'ceph-osd' version
+        WARNING: Changing 'ceph-mon' channel from quincy/stable to octopus/stable. This may be a charm downgrade, which is generally not supported.
+        Change charm config of 'ceph-mon' 'source' to 'cloud:focal-victoria'
+        Wait for up to 2400s for model 'test_model' to reach the idle state
+        Verify that the workload of 'ceph-mon' has been upgraded on units: ceph-mon/0
+    """  # noqa: E501 line too long
+    )
     plan = app.generate_upgrade_plan(target, force=False)
-    print(plan)
-    assert 0
+    assert str(plan) == exp_plan

tests/unit/apps/test_auxiliary_subordinate.py

@@ -302,7 +302,8 @@ def test_ceph_dashboard_upgrade_plan_xena_to_yoga(model):
             coro=model.upgrade_charm(app.name, "pacific/stable"),
         ),
         UpgradeStep(
-            description=f"Upgrade '{app.name} from 'pacific/stable' 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 +323,6 @@ 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' from 'latest/stable' to the new channel: '2.4/stable'
     """
     )
 

tests/unit/apps/test_base.py

@@ -49,6 +49,7 @@ def test_openstack_application_magic_functions(model):
     assert app == app
     assert app is not None
     assert app != "test-app"
+    assert repr(app) == "test-app"
 
 
 @patch("cou.utils.openstack.OpenStackCodenameLookup.find_compatible_versions")
@@ -560,13 +561,16 @@ def test_get_change_channel_possible_downgrade_step(current_os_release, model):
         units={},
         workload_version="1",
     )
-    assert app._get_change_channel_possible_downgrade_step(
-        target, app.expected_current_channel(target), PreUpgradeStep
-    ) == PreUpgradeStep(
+    description, _ = app._get_change_channel_possible_downgrade_step(
+        target, app.expected_current_channel(target)
+    )
+
+    exp_description = (
         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, "ussuri/stable"),
+        "ussuri/stable. This may be a charm downgrade, which is generally not supported."
     )
+    assert description == exp_description
+    model.upgrade_charm.assert_called_once_with(app.name, "ussuri/stable")
 
 
 def test_get_refresh_current_channel_step(model):
@@ -597,7 +601,7 @@ def test_get_refresh_current_channel_step(model):
 @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
+    mock_ch_migration, mock_possible_downgrade_step, mock_refresh_current_channel, model
 ):
     """Expect an empty pre-upgrade step for application that does not need to refresh."""
     target = OpenStackRelease("victoria")
@@ -618,7 +622,7 @@ def test_get_refresh_charm_step_skip(
     )
     assert app._get_refresh_charm_step(target) == PreUpgradeStep()
     mock_ch_migration.assert_not_called()
-    mock_change_os_channels.assert_not_called()
+    mock_possible_downgrade_step.assert_not_called()
     mock_refresh_current_channel.assert_not_called()
 
 
@@ -628,7 +632,7 @@ def test_get_refresh_charm_step_skip(
     "cou.apps.base.OpenStackApplication._get_charmhub_migration_step",
 )
 def test_get_refresh_charm_step_refresh_current_channel(
-    mock_ch_migration, mock_change_os_channels, mock_refresh_current_channel, model
+    mock_ch_migration, mock_possible_downgrade_step, mock_refresh_current_channel, model
 ):
     """Expect a pre-upgrade step for application that needs to refresh current channel."""
     target = OpenStackRelease("victoria")
@@ -656,7 +660,7 @@ def test_get_refresh_charm_step_refresh_current_channel(
     assert app._get_refresh_charm_step(target) == expected_result
 
     mock_ch_migration.assert_not_called()
-    mock_change_os_channels.assert_not_called()
+    mock_possible_downgrade_step.assert_not_called()
     mock_refresh_current_channel.assert_called_once()
 
 
@@ -667,7 +671,7 @@ def test_get_refresh_charm_step_refresh_current_channel(
 def test_get_refresh_charm_step_change_to_openstack_channels(
     current_os_release,
     mock_ch_migration,
-    mock_change_os_channels,
+    mock_possible_downgrade_step,
     mock_refresh_current_channel,
     model,
 ):
@@ -689,19 +693,22 @@ def test_get_refresh_charm_step_change_to_openstack_channels(
         units={},
         workload_version="1",
     )
-    expected_result = 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),
+
+    description = (
+        "WARNING: Changing 'app' channel from 'latest/stable' to 'ussuri/stable'. "
+        "This may be a charm downgrade, which is generally not supported.",
     )
 
-    mock_change_os_channels.return_value = expected_result
+    coro = model.upgrade_charm(app.name, app.expected_current_channel)
+
+    mock_possible_downgrade_step.return_value = (description, coro)
+
+    expected_result = PreUpgradeStep(description=description, coro=coro)
 
     assert app._get_refresh_charm_step(target) == expected_result
 
     mock_ch_migration.assert_not_called()
-    mock_change_os_channels.assert_called_once_with(target)
+    mock_possible_downgrade_step.assert_called_once_with(target, "ussuri/stable")
     mock_refresh_current_channel.assert_not_called()
 
 
@@ -712,7 +719,7 @@ def test_get_refresh_charm_step_change_to_openstack_channels(
 def test_get_refresh_charm_step_charmhub_migration(
     current_os_release,
     mock_ch_migration,
-    mock_change_os_channels,
+    mock_possible_downgrade_step,
     mock_refresh_current_channel,
     model,
 ):
@@ -743,7 +750,7 @@ def test_get_refresh_charm_step_charmhub_migration(
     assert app._get_refresh_charm_step(target) == expected_result
 
     mock_ch_migration.assert_called_once()
-    mock_change_os_channels.assert_not_called()
+    mock_possible_downgrade_step.assert_not_called()
     mock_refresh_current_channel.assert_not_called()
 
 

tests/unit/apps/test_channel_based.py

@@ -75,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' from 'ussuri/stable' to the new channel: 'wallaby/stable'
+            Upgrade 'glance-simplestreams-sync' from 'victoria/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
     )
@@ -257,7 +257,8 @@ def test_application_versionless_upgrade_plan_ussuri_to_victoria(model):
             coro=model.upgrade_charm(app.name, "ussuri/stable"),
         ),
         UpgradeStep(
-            description=f"Upgrade '{app.name}' from 'ussuri/stable' 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"),
         ),
@@ -331,7 +332,8 @@ def test_application_gnocchi_upgrade_plan_ussuri_to_victoria(model):
             coro=model.upgrade_charm(app.name, "ussuri/stable"),
         ),
         UpgradeStep(
-            description=f"Upgrade '{app.name}' from 'ussuri/stable' 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"),
         ),
@@ -414,7 +416,8 @@ 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}' from 'ussuri/stable' 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"),
         ),