Add post-upgrade steps for whole upgrade plan (#279)

Adding ceph-mon post-upgrade step to ensure require-osd-release option
matches with ceph-osd version. Add also add_steps function to BaseStep
so we can easily add more steps at once.

---------

Co-authored-by: TQ X <[email protected]>

cou/apps/auxiliary.py

@@ -195,7 +195,7 @@ def _get_change_require_osd_release_step(self) -> PreUpgradeStep:
         """
         ceph_mon_unit, *_ = self.units.values()
         return PreUpgradeStep(
-            description="Ensure require-osd-release option matches with ceph-osd version",
+            description="Ensure the 'require-osd-release' option matches the 'ceph-osd' version",
             coro=set_require_osd_release_option(ceph_mon_unit.name, self.model),
         )
 

cou/steps/__init__.py

@@ -251,6 +251,15 @@ def add_step(self, step: BaseStep) -> None:
 
         self._sub_steps.append(step)
 
+    def add_steps(self, steps: Iterable[BaseStep]) -> None:
+        """Add multiple steps.
+
+        :param steps: Sequence of BaseStep to be added as sub steps.
+        :type steps: Iterable[BaseStep]
+        """
+        for step in steps:
+            self.add_step(step)
+
     def cancel(self, safe: bool = True) -> None:
         """Cancel step and all its sub steps.
 

cou/steps/plan.py

@@ -43,10 +43,11 @@
     NoTargetError,
     OutOfSupportRange,
 )
-from cou.steps import PreUpgradeStep, UpgradePlan
+from cou.steps import PostUpgradeStep, PreUpgradeStep, UpgradePlan
 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
 from cou.utils.juju_utils import DEFAULT_TIMEOUT, COUMachine
 from cou.utils.nova_compute import get_empty_hypervisors
 from cou.utils.openstack import LTS_TO_OS_RELEASE, OpenStackRelease
@@ -304,7 +305,10 @@ async def generate_plan(analysis_result: Analysis, args: CLIargs) -> UpgradePlan
     pre_plan_sanity_checks(args, analysis_result)
     target = determine_upgrade_target(analysis_result)
 
-    plan = generate_common_plan(target, analysis_result, args)
+    plan = UpgradePlan(
+        f"Upgrade cloud from '{analysis_result.current_cloud_os_release}' to '{target}'"
+    )
+    plan.add_steps(_get_pre_upgrade_steps(analysis_result, args))
 
     # NOTE (gabrielcocenza) upgrade group as None means that the user wants to upgrade
     #  the whole cloud.
@@ -315,27 +319,22 @@ async def generate_plan(analysis_result: Analysis, args: CLIargs) -> UpgradePlan
     if args.upgrade_group in {DATA_PLANE, HYPERVISORS, None}:
         plan.sub_steps.extend(await _generate_data_plane_plan(target, analysis_result, args))
 
+    plan.add_steps(_get_post_upgrade_steps(analysis_result, args))
+
     return plan
 
 
-def generate_common_plan(
-    target: OpenStackRelease, analysis_result: Analysis, args: CLIargs
-) -> UpgradePlan:
-    """Generate the common upgrade plan.
+def _get_pre_upgrade_steps(analysis_result: Analysis, args: CLIargs) -> list[PreUpgradeStep]:
+    """Get the pre-upgrade steps.
 
-    :param target: Target OpenStack release
-    :type target: OpenStackRelease
     :param analysis_result: Analysis result
     :type analysis_result: Analysis
     :param args: CLI arguments
     :type args: CLIargs
-    :return: Common upgrade plan
-    :rtype: UpgradePlan
+    :return: List of pre-upgrade steps.
+    :rtype: list[PreUpgradeStep]
     """
-    plan = UpgradePlan(
-        f"Upgrade cloud from '{analysis_result.current_cloud_os_release}' to '{target}'"
-    )
-    plan.add_step(
+    steps = [
         PreUpgradeStep(
             description="Verify that all OpenStack applications are in idle state",
             parallel=False,
@@ -349,15 +348,57 @@ def generate_common_plan(
                 raise_on_blocked=True,
             ),
         )
-    )
+    ]
     if args.backup:
-        plan.add_step(
+        steps.append(
             PreUpgradeStep(
                 description="Backup mysql databases",
                 coro=backup(analysis_result.model),
             )
         )
-    return plan
+
+    return steps
+
+
+def _get_post_upgrade_steps(analysis_result: Analysis, args: CLIargs) -> list[PostUpgradeStep]:
+    """Get the post-upgrade steps.
+
+    :param analysis_result: Analysis result
+    :type analysis_result: Analysis
+    :param args: CLI arguments
+    :type args: CLIargs
+    :return: List of post-upgrade steps.
+    :rtype: list[PreUpgradeStep]
+    """
+    steps = []
+    if args.upgrade_group in {DATA_PLANE, None}:
+        steps.extend(_get_ceph_mon_post_upgrade_steps(analysis_result.apps_data_plane))
+
+    return steps
+
+
+def _get_ceph_mon_post_upgrade_steps(apps: list[OpenStackApplication]) -> list[PostUpgradeStep]:
+    """Get the post-upgrade step for ceph-mon, where we check the require-osd-release option.
+
+    :param apps: List of OpenStackApplication.
+    :type apps: list[OpenStackApplication]
+    :return: List of post-upgrade steps.
+    :rtype: list[PreUpgradeStep]
+    """
+    ceph_mons_apps = [app for app in apps if isinstance(app, CephMon)]
+
+    steps = []
+    for app in ceph_mons_apps:
+        unit = list(app.units.values())[0]  # getting the first unit, since we don't care which one
+        steps.append(
+            PostUpgradeStep(
+                f"Ensure the 'require-osd-release' option in '{app.name}' matches the 'ceph-osd' "
+                "version",
+                coro=set_require_osd_release_option(unit.name, app.model),
+            )
+        )
+
+    return steps
 
 
 def _generate_control_plane_plan(

cou/utils/app_utils.py

@@ -26,6 +26,9 @@
 logger = logging.getLogger(__name__)
 
 
+RUN_TIMEOUT: int = 600
+
+
 async def upgrade_packages(unit: str, model: COUModel, packages_to_hold: Optional[list]) -> None:
     """Run package updates and upgrades on each unit of an Application.
 
@@ -43,7 +46,7 @@ async def upgrade_packages(unit: str, model: COUModel, packages_to_hold: Optiona
         packages = " ".join(packages_to_hold)
         command = f"apt-mark hold {packages} && {command} ; apt-mark unhold {packages}"
 
-    await model.run_on_unit(unit_name=unit, command=command, timeout=600)
+    await model.run_on_unit(unit_name=unit, command=command, timeout=RUN_TIMEOUT)
 
 
 async def set_require_osd_release_option(unit: str, model: COUModel) -> None:
@@ -65,7 +68,7 @@ async def set_require_osd_release_option(unit: str, model: COUModel) -> None:
 
     if current_require_osd_release != current_running_osd_release:
         set_command = f"ceph osd require-osd-release {current_running_osd_release}"
-        await model.run_on_unit(unit_name=unit, command=set_command, timeout=600)
+        await model.run_on_unit(unit_name=unit, command=set_command, timeout=RUN_TIMEOUT)
 
 
 def validate_ovn_support(version: str) -> None:
@@ -88,7 +91,6 @@ def validate_ovn_support(version: str) -> None:
         )
 
 
-# Private functions
 async def _get_required_osd_release(unit: str, model: COUModel) -> str:
     """Get the value of require-osd-release option on a ceph-mon unit.
 
@@ -103,7 +105,7 @@ async def _get_required_osd_release(unit: str, model: COUModel) -> str:
     check_command = "ceph osd dump -f json"
 
     check_option_result = await model.run_on_unit(
-        unit_name=unit, command=check_command, timeout=600
+        unit_name=unit, command=check_command, timeout=RUN_TIMEOUT
     )
     current_require_osd_release = json.loads(check_option_result["Stdout"]).get(
         "require_osd_release", ""
@@ -129,7 +131,9 @@ async def _get_current_osd_release(unit: str, model: COUModel) -> str:
     """
     check_command = "ceph versions -f json"
 
-    check_osd_result = await model.run_on_unit(unit_name=unit, command=check_command, timeout=600)
+    check_osd_result = await model.run_on_unit(
+        unit_name=unit, command=check_command, timeout=RUN_TIMEOUT
+    )
 
     osd_release_output = json.loads(check_osd_result["Stdout"]).get("osd", None)
     # throw exception if ceph-mon doesn't contain osd release information in `ceph`

tests/unit/apps/test_auxiliary.py

@@ -642,7 +642,7 @@ def test_ceph_mon_upgrade_plan_xena_to_yoga(model):
             coro=model.upgrade_charm(app.name, "pacific/stable", switch=None),
         ),
         PreUpgradeStep(
-            description="Ensure require-osd-release option matches with ceph-osd version",
+            description="Ensure the 'require-osd-release' option matches the 'ceph-osd' version",
             parallel=False,
             coro=app_utils.set_require_osd_release_option("ceph-mon/0", model),
         ),
@@ -731,7 +731,7 @@ def test_ceph_mon_upgrade_plan_ussuri_to_victoria(model):
             coro=model.upgrade_charm(app.name, "octopus/stable", switch=None),
         ),
         PreUpgradeStep(
-            description="Ensure require-osd-release option matches with ceph-osd version",
+            description="Ensure the 'require-osd-release' option matches the 'ceph-osd' version",
             parallel=False,
             coro=app_utils.set_require_osd_release_option("ceph-mon/0", model),
         ),

tests/unit/steps/test_steps.py

@@ -251,6 +251,18 @@ def test_step_add_step_failed():
         plan.add_step(MagicMock())
 
 
+def test_step_add_steps():
+    """Test BaseStep adding sub steps at once."""
+    exp_sub_steps = 3
+    plan = BaseStep(description="plan")
+    plan.add_steps(
+        [BaseStep(description=f"sub-step-{i}", coro=mock_coro()) for i in range(exp_sub_steps)]
+        + [BaseStep(description="empty-step")]  # we also check that empty step will not be added
+    )
+
+    assert len(plan.sub_steps) == exp_sub_steps
+
+
 def test_step_cancel_safe():
     """Test step safe cancel."""
     plan = BaseStep(description="plan")