Check possible charm downgrade when upgrading the channel

Closes: canonical#376

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:

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,28 +592,29 @@ 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(
             "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(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:

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:

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.

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"

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

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