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

@@ -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")

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):

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,47 +546,52 @@ 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(
             "'%s' does not need to refresh the current channel: %s", self.name, self.channel
         )
         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
 

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.
 

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))

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.

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)

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)