- move check_mismatched to the application

- removed units from SubordinateApplication

cou/apps/base.py

@@ -23,7 +23,11 @@
 import yaml
 
 from cou.apps import STANDARD_IDLE_TIMEOUT
-from cou.exceptions import ApplicationError, HaltUpgradePlanGeneration
+from cou.exceptions import (
+    ApplicationError,
+    HaltUpgradePlanGeneration,
+    MismatchedOpenStackVersions,
+)
 from cou.steps import (
     ApplicationUpgradePlan,
     PostUpgradeStep,
@@ -239,7 +243,7 @@ def target_channel(self, target: OpenStackRelease) -> str:
         return f"{target.codename}/stable"
 
     @property
-    def os_release_units(self) -> defaultdict[OpenStackRelease, list[str]]:
+    def os_release_units(self) -> dict[OpenStackRelease, list[str]]:
         """Get the OpenStack release versions from the units.
 
         :return: OpenStack release versions from the units.
@@ -249,7 +253,7 @@ def os_release_units(self) -> defaultdict[OpenStackRelease, list[str]]:
         for unit in self.units.values():
             os_version = self._get_latest_os_version(unit)
             os_versions[os_version].append(unit.name)
-        return os_versions
+        return dict(os_versions)
 
     @property
     def current_os_release(self) -> OpenStackRelease:
@@ -386,15 +390,20 @@ def post_upgrade_steps(
             self._get_reached_expected_target_step(target, units),
         ]
 
-    def upgrade_plan_sanity_checks(self, target: OpenStackRelease) -> None:
+    def upgrade_plan_sanity_checks(
+        self, target: OpenStackRelease, units: Optional[list[COUUnit]]
+    ) -> None:
         """Run sanity checks before generating upgrade plan.
 
         :param target: OpenStack release as target to upgrade.
         :type target: OpenStackRelease
+        :param units: Units to generate upgrade plan, defaults to None
+        :type units: Optional[list[COUUnit]], optional
         :raises ApplicationError: When enable-auto-restarts is not enabled.
         :raises HaltUpgradePlanGeneration: When the application halt the upgrade plan generation.
         """
         self._check_application_target(target)
+        self._check_mismatched_versions(units)
         self._check_auto_restarts()
         logger.info(
             "%s application met all the necessary prerequisites to generate the upgrade plan",
@@ -420,7 +429,7 @@ def generate_upgrade_plan(
         :return: Full upgrade plan if the Application is able to generate it.
         :rtype: ApplicationUpgradePlan
         """
-        self.upgrade_plan_sanity_checks(target)
+        self.upgrade_plan_sanity_checks(target, units)
 
         upgrade_plan = ApplicationUpgradePlan(
             description=f"Upgrade plan for '{self.name}' to {target}",
@@ -782,3 +791,28 @@ def _check_application_target(self, target: OpenStackRelease) -> None:
                 f"Application '{self.name}' already configured for release equal to or greater "
                 f"than {target}. Ignoring."
             )
+
+    def _check_mismatched_versions(self, units: Optional[list[COUUnit]]) -> None:
+        """Check that there are no mismatched versions on app units.
+
+        If units are passed that means that the application will upgrade unit by unit,
+        so there are no mismatch.
+
+        :param units: Units to generate upgrade plan
+        :type units: Optional[list[COUUnit]]
+        :raises MismatchedOpenStackVersions:  When the units of the app are running
+            different OpenStack versions
+        """
+        if units:
+            return
+        os_versions = self.os_release_units
+        if len(os_versions.keys()) > 1:
+            mismatched_repr = [
+                f"'{openstack_release.codename}': {units}"
+                for openstack_release, units in os_versions.items()
+            ]
+
+            raise MismatchedOpenStackVersions(
+                f"Units of application {self.name} are running mismatched OpenStack versions: "
+                f"{', '.join(mismatched_repr)}. This is not currently handled."
+            )

cou/steps/plan.py

@@ -40,7 +40,6 @@
     DataPlaneMachineFilterError,
     HaltUpgradePlanGeneration,
     HighestReleaseAchieved,
-    MismatchedOpenStackVersions,
     NoTargetError,
     OutOfSupportRange,
 )
@@ -64,7 +63,6 @@ def pre_plan_sanity_checks(args: CLIargs, analysis_result: Analysis) -> None:
     :type analysis_result: Analysis
     """
     verify_supported_series(analysis_result)
-    verify_os_versions_control_plane(analysis_result.apps_control_plane)
     verify_highest_release_achieved(analysis_result)
     verify_data_plane_ready_to_upgrade(args, analysis_result)
     verify_hypervisors_cli_input(args, analysis_result)
@@ -86,33 +84,6 @@ def verify_supported_series(analysis_result: Analysis) -> None:
         )
 
 
-def verify_os_versions_control_plane(apps: list[OpenStackApplication]) -> None:
-    """Verify that there are no mismatched versions on control-plane apps.
-
-    Control-plane apps are upgraded using the all-in-one strategy, so it's not
-    suppose to have units running different OpenStack versions.
-
-    :param apps: Control plane apps
-    :type apps: list[OpenStackApplication]
-    :raises MismatchedOpenStackVersions: When the units of a control-plane app are running
-        different OpenStack versions
-    """
-    for app in apps:
-        if isinstance(app, SubordinateBase):
-            continue
-        os_versions = app.os_release_units
-        if len(os_versions.keys()) > 1:
-            mismatched_repr = [
-                f"'{openstack_release.codename}': {units}"
-                for openstack_release, units in os_versions.items()
-            ]
-
-            raise MismatchedOpenStackVersions(
-                f"Units of application {app.name} are running mismatched OpenStack versions: "
-                f"{', '.join(mismatched_repr)}. This is not currently handled."
-            )
-
-
 def verify_highest_release_achieved(analysis_result: Analysis) -> None:
     """Verify if the highest OpenStack release is reached for the current Ubuntu series.
 

tests/unit/apps/test_base.py

@@ -17,7 +17,11 @@
 import pytest
 
 from cou.apps.base import OpenStackApplication
-from cou.exceptions import ApplicationError, HaltUpgradePlanGeneration
+from cou.exceptions import (
+    ApplicationError,
+    HaltUpgradePlanGeneration,
+    MismatchedOpenStackVersions,
+)
 from cou.steps import UnitUpgradeStep, UpgradeStep
 from cou.utils.juju_utils import COUMachine, COUUnit
 from cou.utils.openstack import OpenStackRelease
@@ -362,3 +366,110 @@ def test_check_application_target_error(current_os_release, apt_source_codename,
 
     with pytest.raises(HaltUpgradePlanGeneration, match=exp_error_msg):
         app._check_application_target(target)
+
+
+@patch("cou.apps.base.OpenStackApplication.os_release_units", new_callable=PropertyMock)
+def test_check_mismatched_versions_exception(mock_os_release_units, model):
+    """Raise exception if workload version is different on units of a control-plane application."""
+    exp_error_msg = (
+        "Units of application my-app are running mismatched OpenStack versions: "
+        r"'ussuri': \['my-app\/0', 'my-app\/1'\], 'victoria': \['my-app\/2'\]. "
+        "This is not currently handled."
+    )
+
+    machines = {
+        "0": MagicMock(spec_set=COUMachine),
+        "1": MagicMock(spec_set=COUMachine),
+        "2": MagicMock(spec_set=COUMachine),
+    }
+    units = {
+        "my-app/0": COUUnit(
+            name="my-app/0",
+            workload_version="17.0.1",
+            machine=machines["0"],
+        ),
+        "my-app/1": COUUnit(
+            name="my-app/1",
+            workload_version="17.0.1",
+            machine=machines["1"],
+        ),
+        "my-app/2": COUUnit(
+            name="my-app/2",
+            workload_version="18.1.0",
+            machine=machines["2"],
+        ),
+    }
+
+    mock_os_release_units.return_value = {
+        OpenStackRelease("ussuri"): ["my-app/0", "my-app/1"],
+        OpenStackRelease("victoria"): ["my-app/2"],
+    }
+
+    app = OpenStackApplication(
+        name="my-app",
+        can_upgrade_to="ussuri/stable",
+        charm="my-app",
+        channel="ussuri/stable",
+        config={"source": {"value": "distro"}},
+        machines=machines,
+        model=model,
+        origin="ch",
+        series="focal",
+        subordinate_to=[],
+        units=units,
+        workload_version="18.1.0",
+    )
+
+    with pytest.raises(MismatchedOpenStackVersions, match=exp_error_msg):
+        app._check_mismatched_versions(None)
+
+    # if units are passed, it doesn't raise exception
+    assert app._check_mismatched_versions([units["my-app/0"]]) is None
+
+
+@patch("cou.apps.base.OpenStackApplication.os_release_units", new_callable=PropertyMock)
+def test_check_mismatched_versions(mock_os_release_units, model):
+    """Test that no exceptions is raised if units of the app have the same OpenStack version."""
+    machines = {
+        "0": MagicMock(spec_set=COUMachine),
+        "1": MagicMock(spec_set=COUMachine),
+        "2": MagicMock(spec_set=COUMachine),
+    }
+    units = {
+        "my-app/0": COUUnit(
+            name="my-app/0",
+            workload_version="17.0.1",
+            machine=machines["0"],
+        ),
+        "my-app/1": COUUnit(
+            name="my-app/1",
+            workload_version="17.0.1",
+            machine=machines["1"],
+        ),
+        "my-app/2": COUUnit(
+            name="my-app/2",
+            workload_version="18.1.0",
+            machine=machines["2"],
+        ),
+    }
+
+    mock_os_release_units.return_value = {
+        OpenStackRelease("ussuri"): ["my-app/0", "my-app/1", "my-app/2"],
+    }
+
+    app = OpenStackApplication(
+        name="my-app",
+        can_upgrade_to="ussuri/stable",
+        charm="my-app",
+        channel="ussuri/stable",
+        config={"source": {"value": "distro"}},
+        machines=machines,
+        model=model,
+        origin="ch",
+        series="focal",
+        subordinate_to=[],
+        units=units,
+        workload_version="18.1.0",
+    )
+
+    assert app._check_mismatched_versions([units["my-app/0"]]) is None

tests/unit/apps/test_subordinate.py

@@ -70,13 +70,7 @@ def test_generate_upgrade_plan(model):
         origin="ch",
         series="focal",
         subordinate_to=["nova-compute"],
-        units={
-            "keystone-ldap/0": COUUnit(
-                name="keystone-ldap/0",
-                workload_version="18.1.0",
-                machine=machines["0"],
-            )
-        },
+        units={},
         workload_version="18.1.0",
     )
     expected_plan = ApplicationUpgradePlan(
@@ -197,13 +191,7 @@ def test_generate_plan_ch_migration(model, channel):
         origin="cs",
         series="focal",
         subordinate_to=["nova-compute"],
-        units={
-            "keystone-ldap/0": COUUnit(
-                name="keystone-ldap/0",
-                workload_version="18.1.0",
-                machine=machines["0"],
-            )
-        },
+        units={},
         workload_version="18.1.0",
     )
     expected_plan = ApplicationUpgradePlan(
@@ -251,13 +239,7 @@ def test_generate_plan_from_to(model, from_os, to_os):
         origin="ch",
         series="focal",
         subordinate_to=["nova-compute"],
-        units={
-            "keystone-ldap/0": COUUnit(
-                name="keystone-ldap/0",
-                workload_version="18.1.0",
-                machine=machines["0"],
-            )
-        },
+        units={},
         workload_version="18.1.0",
     )
     expected_plan = ApplicationUpgradePlan(description=f"Upgrade plan for '{app.name}' to {to_os}")
@@ -304,13 +286,7 @@ def test_generate_plan_in_same_version(model, from_to):
         origin="ch",
         series="focal",
         subordinate_to=["nova-compute"],
-        units={
-            "keystone-ldap/0": COUUnit(
-                name="keystone-ldap/0",
-                workload_version="18.1.0",
-                machine=machines["0"],
-            )
-        },
+        units={},
         workload_version="18.1.0",
     )
     expected_plan = ApplicationUpgradePlan(