Improve checking for mismatched unit openstack versions (canonical#585)

If the units of an application are running mismatched openstack versions,
this is normally ok
(a user may have manually ran an apt upgrade for example).
But if the versions are so mismatched
that they are detected as being for mismatched openstack releases,
then that's a problem,
potentially indicating a failed upgrade on a unit for example.
Eg. if one unit is running a workload version only for victoria,
while the other two units are running a version for wallaby,
this is a consistency problem,
and COU should block an automated upgrade.

COU was previously checking this case fine,
but the error message was not clear.

This commit improves the error message here,
to clearly show:
- the workload version per unit
- interpreted openstack release per unit
- that the information was retrieved from
  the unit workload information from juju

This should help the user to start debugging the issue.

Fixes: canonical#487

cou/apps/base.py

@@ -912,29 +912,35 @@ def _check_application_target(self, target: OpenStackRelease) -> None:
             )
 
     def _check_mismatched_versions(self) -> None:
-        """Check that there are no mismatched versions on app units.
+        """Check that there are no unexpected mismatched versions on app units.
+
+        For cases where mismatched versions may be expected
+        (eg. nova-compute which is upgraded unit-by-unit),
+        then it will not check.
 
         :raises MismatchedOpenStackVersions: When the units of the app are running
                                              different OpenStack versions.
         """
-        # NOTE (gabrielcocenza) nova-compute is upgraded using paused-single-unit,
-        # so it's possible to have mismatched version in applications units that are
-        # nova-compute or colocated with it.
+        # nova-compute is upgraded one unit at a time,
+        # so it's possible to have mismatched version in
+        # units of applications that are nova-compute or colocated with it.
         if any(
             "nova-compute" in app_charm
             for machine in self.machines.values()
             for app_charm in machine.apps_charms
         ):
             return
 
-        o7k_versions = self.o7k_release_units
-        if len(o7k_versions.keys()) > 1:
-            mismatched_repr = [
-                f"'{openstack_release.codename}': {units}"
-                for openstack_release, units in o7k_versions.items()
-            ]
-
+        if len({self.get_latest_o7k_version(unit) for unit in self.units.values()}) > 1:
+            formatted_results = "\n".join(
+                f"  {unit.name}: {self.get_latest_o7k_version(unit)} "
+                f"(workload: {unit.workload_version})"
+                for unit in sorted(self.units.values(), key=lambda unit: unit.name)
+            )
             raise MismatchedOpenStackVersions(
-                f"Units of application {self.name} are running mismatched OpenStack versions: "
-                f"{', '.join(mismatched_repr)}. This is not currently handled."
+                f"Units of application {self.name} are running mismatched OpenStack releases, "
+                "based on the workload versions as reported by juju status. "
+                "Observed OpenStack releases for each unit:\n"
+                f"{formatted_results}\n"
+                "This requires manually resolving the issue to continue."
             )

tests/unit/apps/test_base.py

@@ -407,15 +407,9 @@ def test_check_application_target_error(o7k_release, apt_source_codename):
         app._check_application_target(target)
 
 
-@patch("cou.apps.base.OpenStackApplication.o7k_release_units", new_callable=PropertyMock)
-def test_check_mismatched_versions_exception(mock_o7k_release_units, model):
+@patch("cou.apps.base.OpenStackApplication.get_latest_o7k_version", new_callable=MagicMock)
+def test_check_mismatched_versions_exception(mock_get_latest_o7k_version, 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 = {f"{i}": generate_cou_machine(f"{i}", f"az-{i}") for i in range(3)}
     units = {
         "my-app/0": Unit(
@@ -435,10 +429,11 @@ def test_check_mismatched_versions_exception(mock_o7k_release_units, model):
         ),
     }
 
-    mock_o7k_release_units.return_value = {
-        OpenStackRelease("ussuri"): ["my-app/0", "my-app/1"],
-        OpenStackRelease("victoria"): ["my-app/2"],
-    }
+    mock_get_latest_o7k_version.side_effect = lambda unit: (
+        OpenStackRelease("ussuri")
+        if unit.name in ["my-app/0", "my-app/1"]
+        else OpenStackRelease("victoria")
+    )
 
     app = OpenStackApplication(
         name="my-app",
@@ -455,12 +450,12 @@ def test_check_mismatched_versions_exception(mock_o7k_release_units, model):
         workload_version="18.1.0",
     )
 
-    with pytest.raises(MismatchedOpenStackVersions, match=exp_error_msg):
+    with pytest.raises(MismatchedOpenStackVersions, match=".*mismatched OpenStack releases.*"):
         app._check_mismatched_versions()
 
 
-@patch("cou.apps.base.OpenStackApplication.o7k_release_units", new_callable=PropertyMock)
-def test_check_mismatched_versions_with_nova_compute(mock_o7k_release_units, model):
+@patch("cou.apps.base.OpenStackApplication.get_latest_o7k_version", new_callable=MagicMock)
+def test_check_mismatched_versions_with_nova_compute(mock_get_latest_o7k_version, model):
     """Not raise exception if workload version is different, but is colocated with nova-compute."""
     machines = {
         f"{i}": generate_cou_machine(
@@ -486,10 +481,11 @@ def test_check_mismatched_versions_with_nova_compute(mock_o7k_release_units, mod
         ),
     }
 
-    mock_o7k_release_units.return_value = {
-        OpenStackRelease("ussuri"): ["my-app/0", "my-app/1"],
-        OpenStackRelease("victoria"): ["my-app/2"],
-    }
+    mock_get_latest_o7k_version.side_effect = lambda unit: (
+        OpenStackRelease("ussuri")
+        if unit.name in ["my-app/0", "my-app/1"]
+        else OpenStackRelease("victoria")
+    )
 
     app = OpenStackApplication(
         name="my-app",
@@ -509,8 +505,8 @@ def test_check_mismatched_versions_with_nova_compute(mock_o7k_release_units, mod
     assert app._check_mismatched_versions() is None
 
 
-@patch("cou.apps.base.OpenStackApplication.o7k_release_units", new_callable=PropertyMock)
-def test_check_mismatched_versions(mock_o7k_release_units, model):
+@patch("cou.apps.base.OpenStackApplication.get_latest_o7k_version", new_callable=MagicMock)
+def test_check_mismatched_versions(mock_get_latest_o7k_version, model):
     """Test that no exceptions is raised if units of the app have the same OpenStack version."""
     machines = {f"{i}": generate_cou_machine(f"{i}", f"az-{i}") for i in range(3)}
     units = {
@@ -531,9 +527,7 @@ def test_check_mismatched_versions(mock_o7k_release_units, model):
         ),
     }
 
-    mock_o7k_release_units.return_value = {
-        OpenStackRelease("ussuri"): ["my-app/0", "my-app/1", "my-app/2"],
-    }
+    mock_get_latest_o7k_version.side_effect = lambda _unit: OpenStackRelease("ussuri")
 
     app = OpenStackApplication(
         name="my-app",