Enhance units display

- If an app has multiple units, the line to verify that the workload of an app has been upgraded can be big. This PR split the units into multiple lines - All new lines in a description will have the same indent as the first line
gabrielcocenza · Mar 15, 2024 · 4fdaaff · 4fdaaff
1 parent 4bdfee3
commit 4fdaaff
Show file tree

Hide file tree

Showing 7 changed files with 102 additions and 47 deletions.
diff --git a/cou/apps/base.py b/cou/apps/base.py
@@ -29,6 +29,7 @@
     MismatchedOpenStackVersions,
 )
 from cou.steps import (
+    TAB,
     ApplicationUpgradePlan,
     PostUpgradeStep,
     PreUpgradeStep,
@@ -696,10 +697,13 @@ def _get_reached_expected_target_step(
         """
         if not units:
             units = list(self.units.values())
+
+        units_repr = "\n".join([f"{TAB}- '{unit.name}'" for unit in units])
+
         return PostUpgradeStep(
             description=(
-                f"Verify that the workload of '{self.name}' has been upgraded on units: "
-                f"{', '.join([unit.name for unit in units])}"
+                f"Verify that the workload of '{self.name}' has been upgraded on units:\n"
+                f"{units_repr}"
             ),
             coro=self._verify_workload_upgrade(target, units),
         )

diff --git a/cou/steps/__init__.py b/cou/steps/__init__.py
@@ -27,6 +27,7 @@
 
 logger = logging.getLogger(__name__)
 DEPENDENCY_DESCRIPTION_PREFIX = "├── "
+TAB = "\t"
 
 
 def compare_step_coroutines(coro1: Optional[Coroutine], coro2: Optional[Coroutine]) -> bool:
@@ -133,11 +134,12 @@ def __str__(self) -> str:
         :rtype: str
         """
         result = ""
-        tab = "\t"
         steps_to_visit = [(self, 0)]
         while steps_to_visit:
             step, indent = steps_to_visit.pop()
-            result += f"{tab * indent}{step.description}{os.linesep}" if step else ""
+            # add tab in the beginning and also on new lines that the description might have
+            description = f"{TAB * indent}{step.description}".replace("\n", f"\n{TAB * indent}")
+            result += f"{description}{os.linesep}" if step else ""
             steps_to_visit.extend([(s, indent + 1) for s in reversed(step.sub_steps)])
 
         return result

diff --git a/tests/unit/apps/test_auxiliary.py b/tests/unit/apps/test_auxiliary.py
@@ -27,6 +27,7 @@
 from cou.apps.core import NovaCompute
 from cou.exceptions import ApplicationError, HaltUpgradePlanGeneration
 from cou.steps import (
+    TAB,
     ApplicationUpgradePlan,
     PostUpgradeStep,
     PreUpgradeStep,
@@ -149,6 +150,8 @@ def test_auxiliary_upgrade_plan_ussuri_to_victoria_change_channel(model):
             )
         )
 
+    units_repr = "\n".join([f"{TAB}- '{unit}'" for unit in app.units])
+
     upgrade_steps = [
         expected_upgrade_package_step,
         PreUpgradeStep(
@@ -179,8 +182,8 @@ def test_auxiliary_upgrade_plan_ussuri_to_victoria_change_channel(model):
         ),
         PostUpgradeStep(
             description=(
-                f"Verify that the workload of '{app.name}' has been upgraded on units: "
-                f"{', '.join([unit for unit in app.units.keys()])}"
+                f"Verify that the workload of '{app.name}' has been upgraded on units:\n"
+                f"{units_repr}"
             ),
             parallel=False,
             coro=app._verify_workload_upgrade(target, list(app.units.values())),
@@ -231,6 +234,7 @@ def test_auxiliary_upgrade_plan_ussuri_to_victoria(model):
                 coro=app_utils.upgrade_packages(unit.name, model, None),
             )
         )
+    units_repr = "\n".join([f"{TAB}- '{unit}'" for unit in app.units])
 
     upgrade_steps = [
         upgrade_packages,
@@ -257,8 +261,8 @@ def test_auxiliary_upgrade_plan_ussuri_to_victoria(model):
         ),
         PostUpgradeStep(
             description=(
-                f"Verify that the workload of '{app.name}' has been upgraded on units: "
-                f"{', '.join([unit for unit in app.units.keys()])}"
+                f"Verify that the workload of '{app.name}' has been upgraded on units:\n"
+                f"{units_repr}"
             ),
             parallel=False,
             coro=app._verify_workload_upgrade(target, list(app.units.values())),
@@ -311,6 +315,8 @@ def test_auxiliary_upgrade_plan_ussuri_to_victoria_ch_migration(model):
             )
         )
 
+    units_repr = "\n".join([f"{TAB}- '{unit}'" for unit in app.units])
+
     upgrade_steps = [
         upgrade_packages,
         PreUpgradeStep(
@@ -341,8 +347,8 @@ def test_auxiliary_upgrade_plan_ussuri_to_victoria_ch_migration(model):
         ),
         PostUpgradeStep(
             description=(
-                f"Verify that the workload of '{app.name}' has been upgraded on units: "
-                f"{', '.join([unit for unit in app.units.keys()])}"
+                f"Verify that the workload of '{app.name}' has been upgraded on units:\n"
+                f"{units_repr}"
             ),
             parallel=False,
             coro=app._verify_workload_upgrade(target, list(app.units.values())),
@@ -636,6 +642,8 @@ def test_ceph_mon_upgrade_plan_xena_to_yoga(model):
             )
         )
 
+    units_repr = "\n".join([f"{TAB}- '{unit}'" for unit in app.units])
+
     upgrade_steps = [
         upgrade_packages,
         PreUpgradeStep(
@@ -672,8 +680,8 @@ def test_ceph_mon_upgrade_plan_xena_to_yoga(model):
         ),
         PostUpgradeStep(
             description=(
-                f"Verify that the workload of '{app.name}' has been upgraded on units: "
-                f"{', '.join([unit for unit in app.units.keys()])}"
+                f"Verify that the workload of '{app.name}' has been upgraded on units:\n"
+                f"{units_repr}"
             ),
             parallel=False,
             coro=app._verify_workload_upgrade(target, list(app.units.values())),
@@ -727,6 +735,8 @@ def test_ceph_mon_upgrade_plan_ussuri_to_victoria(model):
             )
         )
 
+    units_repr = "\n".join([f"{TAB}- '{unit}'" for unit in app.units])
+
     upgrade_steps = [
         upgrade_packages,
         PreUpgradeStep(
@@ -758,8 +768,8 @@ def test_ceph_mon_upgrade_plan_ussuri_to_victoria(model):
         ),
         PostUpgradeStep(
             description=(
-                f"Verify that the workload of '{app.name}' has been upgraded on units: "
-                f"{', '.join([unit for unit in app.units.keys()])}"
+                f"Verify that the workload of '{app.name}' has been upgraded on units:\n"
+                f"{units_repr}"
             ),
             parallel=False,
             coro=app._verify_workload_upgrade(target, list(app.units.values())),
@@ -917,6 +927,8 @@ def test_ovn_principal_upgrade_plan(model):
             )
         )
 
+    units_repr = "\n".join([f"{TAB}- '{unit}'" for unit in app.units])
+
     upgrade_steps = [
         upgrade_packages,
         PreUpgradeStep(
@@ -941,8 +953,8 @@ def test_ovn_principal_upgrade_plan(model):
         ),
         PostUpgradeStep(
             description=(
-                f"Verify that the workload of '{app.name}' has been upgraded on units: "
-                f"{', '.join([unit for unit in app.units.keys()])}"
+                f"Verify that the workload of '{app.name}' has been upgraded on units:\n"
+                f"{units_repr}"
             ),
             parallel=False,
             coro=app._verify_workload_upgrade(target, list(app.units.values())),
@@ -996,6 +1008,8 @@ def test_mysql_innodb_cluster_upgrade(model):
             )
         )
 
+    units_repr = "\n".join([f"{TAB}- '{unit}'" for unit in app.units])
+
     upgrade_steps = [
         upgrade_packages,
         PreUpgradeStep(
@@ -1020,8 +1034,8 @@ def test_mysql_innodb_cluster_upgrade(model):
         ),
         PostUpgradeStep(
             description=(
-                f"Verify that the workload of '{app.name}' has been upgraded on units: "
-                f"{', '.join([unit for unit in app.units.keys()])}"
+                f"Verify that the workload of '{app.name}' has been upgraded on units:\n"
+                f"{units_repr}"
             ),
             parallel=False,
             coro=app._verify_workload_upgrade(target, list(app.units.values())),
@@ -1212,7 +1226,10 @@ def test_ceph_osd_upgrade_plan(model):
             Upgrade software packages on unit 'ceph-osd/2'
         Change charm config of 'ceph-osd' 'source' to 'cloud:focal-victoria'
         Wait for up to 300s for app 'ceph-osd' to reach the idle state
-        Verify that the workload of 'ceph-osd' has been upgraded on units: ceph-osd/0, ceph-osd/1, ceph-osd/2
+        Verify that the workload of 'ceph-osd' has been upgraded on units:
+            - 'ceph-osd/0'
+            - 'ceph-osd/1'
+            - 'ceph-osd/2'
     """  # noqa: E501 line too long
     )
     target = OpenStackRelease("victoria")

diff --git a/tests/unit/apps/test_channel_based.py b/tests/unit/apps/test_channel_based.py
@@ -14,6 +14,7 @@
 
 from cou.apps.channel_based import ChannelBasedApplication
 from cou.steps import (
+    TAB,
     ApplicationUpgradePlan,
     PostUpgradeStep,
     PreUpgradeStep,
@@ -278,6 +279,8 @@ def test_application_gnocchi_upgrade_plan_ussuri_to_victoria(model):
             )
         )
 
+    units_repr = "\n".join([f"{TAB}- '{unit}'" for unit in app.units])
+
     upgrade_steps = [
         upgrade_packages,
         PreUpgradeStep(
@@ -308,8 +311,8 @@ def test_application_gnocchi_upgrade_plan_ussuri_to_victoria(model):
         ),
         PostUpgradeStep(
             description=(
-                f"Verify that the workload of '{app.name}' has been upgraded on units: "
-                f"{', '.join([unit for unit in app.units.keys()])}"
+                f"Verify that the workload of '{app.name}' has been upgraded on units:\n"
+                f"{units_repr}"
             ),
             parallel=False,
             coro=app._verify_workload_upgrade(target, list(app.units.values())),
@@ -367,6 +370,8 @@ def test_application_designate_bind_upgrade_plan_ussuri_to_victoria(model):
             )
         )
 
+    units_repr = "\n".join([f"{TAB}- '{unit}'" for unit in app.units])
+
     upgrade_steps = [
         upgrade_packages,
         PreUpgradeStep(
@@ -397,8 +402,8 @@ def test_application_designate_bind_upgrade_plan_ussuri_to_victoria(model):
         ),
         PostUpgradeStep(
             description=(
-                f"Verify that the workload of '{app.name}' has been upgraded on units: "
-                f"{', '.join([unit for unit in app.units.keys()])}"
+                f"Verify that the workload of '{app.name}' has been upgraded on units:\n"
+                f"{units_repr}"
             ),
             parallel=False,
             coro=app._verify_workload_upgrade(target, list(app.units.values())),