Merge branch 'main' into dev/data-plane

cou/apps/base.py

@@ -410,18 +410,17 @@ def generate_upgrade_plan(
         :return: Full upgrade plan if the Application is able to generate it.
         :rtype: ApplicationUpgradePlan
         """
-        upgrade_steps = ApplicationUpgradePlan(
+        upgrade_plan = ApplicationUpgradePlan(
             description=f"Upgrade plan for '{self.name}' to {target}",
         )
-        all_steps = (
-            self.pre_upgrade_steps(target, units)
-            + self.upgrade_steps(target, units, force)
-            + self.post_upgrade_steps(target, units)
-        )
-        for step in all_steps:
-            if step:
-                upgrade_steps.add_step(step)
-        return upgrade_steps
+
+        upgrade_plan.sub_steps = [
+            *self.pre_upgrade_steps(target, units),
+            *self.upgrade_steps(target, units, force),
+            *self.post_upgrade_steps(target, units),
+        ]
+
+        return upgrade_plan
 
     def _get_upgrade_current_release_packages_step(
         self, units: Optional[list[COUUnit]]

cou/steps/__init__.py

@@ -188,6 +188,7 @@ def description(self, description: str) -> None:
         """
         if not description and self._coro:
             raise ValueError("Every coroutine should have a description")
+
         self._description = description
 
     @property
@@ -244,6 +245,10 @@ def add_step(self, step: BaseStep) -> None:
         if not isinstance(step, BaseStep):
             raise TypeError("Cannot add an upgrade step that is not derived from BaseStep")
 
+        if not step:
+            logger.debug("skipping adding empty step")
+            return
+
         self._sub_steps.append(step)
 
     def cancel(self, safe: bool = True) -> None:

tests/unit/steps/test_steps.py

@@ -115,12 +115,14 @@ def test_step_bool():
     sub_step = BaseStep(description="a.a")
     assert bool(sub_step) is False
 
-    plan.sub_steps = [sub_step]
+    plan.add_step(sub_step)
     assert bool(plan) is False
 
     # coroutine in the plan sub_steps tree
     sub_sub_step = BaseStep(description="a.a.a", coro=mock_coro("a.a.a"))
-    sub_step.sub_steps = [sub_sub_step]
+    sub_step.add_step(sub_sub_step)
+    plan.add_step(sub_step)
+
     assert bool(sub_sub_step) is True
     assert bool(sub_step) is True
     assert bool(plan) is True
@@ -225,6 +227,16 @@ def test_step_add_step():
     exp_sub_steps = 3
     plan = BaseStep(description="plan")
     for i in range(exp_sub_steps):
+        plan.add_step(BaseStep(description=f"sub-step-{i}", coro=mock_coro()))
+
+    assert len(plan.sub_steps) == exp_sub_steps
+
+
+def test_step_add_step_skipping_empty():
+    """Test BaseStep skipping to add empty sub steps."""
+    exp_sub_steps = 0
+    plan = BaseStep(description="plan")
+    for i in range(3):
         plan.add_step(BaseStep(description=f"sub-step-{i}"))
 
     assert len(plan.sub_steps) == exp_sub_steps
@@ -242,9 +254,13 @@ def test_step_add_step_failed():
 def test_step_cancel_safe():
     """Test step safe cancel."""
     plan = BaseStep(description="plan")
-    plan.sub_steps = sub_steps = [BaseStep(description=f"sub-{i}") for i in range(10)]
+    plan.sub_steps = sub_steps = [
+        BaseStep(description=f"sub-{i}", coro=mock_coro()) for i in range(10)
+    ]
     # add sub-sub-steps to one sub-step
-    sub_steps[0].sub_steps = [BaseStep(description=f"sub-0.{i}") for i in range(3)]
+    sub_steps[0].sub_steps = [
+        BaseStep(description=f"sub-0.{i}", coro=mock_coro()) for i in range(3)
+    ]
 
     plan.cancel()