Remove unused code and small refactoring (#534)

* Revert "Add subordinate units information for Application. (#497)"

This reverts commit 98950e7.

* Partially revert "feat: Restart ceilometer-agent-compute after nova-compute upgrade (#435)"

This partially reverts commit a966c9d.

* Small refactoring

cou/apps/base.py

@@ -115,7 +115,6 @@ def __str__(self) -> str:
                 "origin": self.origin,
                 "series": self.series,
                 "subordinate_to": self.subordinate_to,
-                "subordinate_units": self.subordinate_units,
                 "workload_version": self.workload_version,
                 "units": {
                     unit.name: {

cou/apps/core.py

@@ -106,45 +106,7 @@ def post_upgrade_steps(
         :return: List of post upgrade steps.
         :rtype: list[PostUpgradeStep]
         """
-        return (
-            self._get_enable_scheduler_step(units)
-            + self._get_restart_subordinate_services_steps(units)
-            + super().post_upgrade_steps(target, units)
-        )
-
-    def _get_restart_subordinate_services_steps(
-        self, units: Optional[list[Unit]]
-    ) -> list[PostUpgradeStep]:
-        """Get step to restart all subordinate services if they aren't running.
-
-        :param units: Units to restart subordinate services.
-        :type units: Optional[list[Unit]]
-        :return: Steps to restart to subordinate services
-        :rtype: list[PostUpgradeStep]
-        """
-        if not units:
-            units = list(self.units.values())
-        steps = []
-        for unit in units:
-            for subordinate in unit.subordinates:
-                if "ceilometer-agent" == subordinate.charm:
-                    service = "ceilometer-agent-compute"
-                    steps.append(
-                        PostUpgradeStep(
-                            description=(
-                                "Restart service ceilometer-agent-compute "
-                                f"for subordinate unit: '{subordinate.name}'"
-                            ),
-                            coro=self.model.run_on_unit(
-                                unit_name=subordinate.name,
-                                command=(
-                                    f"systemctl is-active --quiet {service}"
-                                    f" || systemctl restart {service}"
-                                ),
-                            ),
-                        )
-                    )
-        return steps
+        return self._get_enable_scheduler_step(units) + super().post_upgrade_steps(target, units)
 
     def _get_unit_upgrade_steps(self, unit: Unit, force: bool) -> UnitUpgradeStep:
         """Get the upgrade steps for a single unit.

cou/apps/factory.py

@@ -57,7 +57,6 @@ def create(cls, app: Application) -> Optional[OpenStackApplication]:
                 series=app.series,
                 subordinate_to=app.subordinate_to,
                 units=app.units,
-                subordinate_units=app.subordinate_units,
                 workload_version=app.workload_version,
                 actions=app.actions,
             )

cou/steps/ceph.py

@@ -15,14 +15,15 @@
 """Functions for prereq steps related to ceph."""
 import json
 import logging
+from typing import Sequence
 
 from cou.exceptions import (
     ApplicationError,
     ApplicationNotFound,
     RunUpgradeError,
     UnitNotFound,
 )
-from cou.utils.juju_utils import Application, Model
+from cou.utils.juju_utils import Application, Model, get_applications_by_charm_name
 from cou.utils.openstack import CEPH_RELEASES
 
 logger = logging.getLogger(__name__)
@@ -97,25 +98,7 @@ async def _get_current_osd_release(unit: str, model: Model) -> str:
     return current_osd_release
 
 
-async def _get_applications(model: Model) -> list[Application]:
-    """Get the all ceph mon application(s) from the model.
-
-    :param model: The juju model to work with
-    :type model: Model
-    :return: A list of ceph-mon applications
-    :type: Application
-    :raise ApplicationNotFound: if ceph-mon no founnd
-    """
-    apps = await model.get_applications()
-    ceph_mon_apps = [app for app in apps.values() if app.charm == "ceph-mon"]
-    if not ceph_mon_apps:
-        raise ApplicationNotFound(
-            "'ceph-mon' application not found, is this a valid OpenStack cloud?"
-        )
-    return ceph_mon_apps
-
-
-async def _get_unit_name(ceph_mon_app: Application) -> str:
+def _get_unit_name(ceph_mon_app: Application) -> str:
     """Get the one of the unit's name from ceph mon application.
 
     :param ceph_mon_app: The ceph mon application
@@ -130,27 +113,29 @@ async def _get_unit_name(ceph_mon_app: Application) -> str:
     return ceph_mon_units[0].name
 
 
-async def osd_noout(model: Model, enable: bool) -> None:
+async def osd_noout(model: Model, apps: Sequence[Application], enable: bool) -> None:
     """Set or unset 'noout' for ceph cluster(s).
 
     Note this will set or unset 'noout' flag for all ceph clusters present in
     the model.
 
     :param model: The juju model to work with
     :type model: Model
+    :param apps: List of Application
+    :type apps: Sequence[Application]
     :param enable: True to set noout, False to unset noout
     :type enable: bool
     """
     try:
-        ceph_mon_apps = await _get_applications(model)
+        ceph_mon_apps = get_applications_by_charm_name(apps, "ceph-mon")
     except ApplicationNotFound as e:
         logger.warning("%s", str(e))
         logger.warning("Skip changing 'noout', because there's no ceph-mon applications.")
         return
 
     for ceph_mon_app in ceph_mon_apps:
         try:
-            ceph_mon_unit_name = await _get_unit_name(ceph_mon_app)
+            ceph_mon_unit_name = _get_unit_name(ceph_mon_app)
         except UnitNotFound as e:
             logger.warning("%s", str(e))
             logger.warning(
@@ -177,20 +162,22 @@ async def get_osd_noout_state(model: Model, unit_name: str) -> bool:
     return "noout" in json.loads(results["stdout"].strip()).get("flags_set", [])
 
 
-async def assert_osd_noout_state(model: Model, state: bool) -> None:
+async def assert_osd_noout_state(model: Model, apps: Sequence[Application], state: bool) -> None:
     """Assert ceph cluster is set (state=True) or unset (state=False).
 
     Note this will assert 'noout' flag is in the desired state for all ceph
     clusters present in the model.
 
     :param model: The juju model to work with
     :type model: Model
+    :param apps:List of Application
+    :type apps: Sequence[Application]
     :param state: True noout is set, otherwise False
     :type state: bool
     :raise ApplicationError: if noout is not in desired state
     """
     try:
-        ceph_mon_apps = await _get_applications(model)
+        ceph_mon_apps = get_applications_by_charm_name(apps, "ceph-mon")
     except ApplicationNotFound as e:
         logger.warning("%s", str(e))
         logger.warning("Skip verifying 'noout', because there's no ceph-mon applications.")
@@ -199,7 +186,7 @@ async def assert_osd_noout_state(model: Model, state: bool) -> None:
     error = False
     for ceph_mon_app in ceph_mon_apps:
         try:
-            ceph_mon_unit_name = await _get_unit_name(ceph_mon_app)
+            ceph_mon_unit_name = _get_unit_name(ceph_mon_app)
         except UnitNotFound as e:
             logger.warning("%s", str(e))
             logger.warning(
@@ -242,7 +229,7 @@ async def set_require_osd_release_option_on_unit(model: Model, unit_name: str) -
         await model.run_on_unit(unit_name=unit_name, command=set_command, timeout=600)
 
 
-async def set_require_osd_release_option(model: Model) -> None:
+async def set_require_osd_release_option(model: Model, apps: Sequence[Application]) -> None:
     """Check and set the correct value for require-osd-release on all ceph-mon unit.
 
     This function compares the value of require-osd-release option with the
@@ -254,10 +241,12 @@ async def set_require_osd_release_option(model: Model) -> None:
 
     :param model: Model object
     :type model: Model
+    :param apps:List of Application
+    :type apps: Sequence[Application]
     :raises CommandRunFailed: When a command fails to run.
     """
     try:
-        ceph_mon_apps = await _get_applications(model)
+        ceph_mon_apps = get_applications_by_charm_name(apps, "ceph-mon")
     except ApplicationNotFound as e:
         logger.warning("%s", str(e))
         logger.warning(
@@ -267,7 +256,7 @@ async def set_require_osd_release_option(model: Model) -> None:
 
     for ceph_mon_app in ceph_mon_apps:
         try:
-            ceph_mon_unit_name = await _get_unit_name(ceph_mon_app)
+            ceph_mon_unit_name = _get_unit_name(ceph_mon_app)
         except UnitNotFound as e:
             logger.warning("%s", str(e))
             logger.warning(

cou/steps/nova_cloud_controller.py

@@ -14,15 +14,15 @@
 
 """Functions for prereq steps relating to nova."""
 import logging
-from typing import Optional
+from typing import Optional, Sequence
 
-from cou.exceptions import ApplicationNotFound, COUException, UnitNotFound
-from cou.utils.juju_utils import Model
+from cou.exceptions import COUException, UnitNotFound
+from cou.utils.juju_utils import Application, Model, get_applications_by_charm_name
 
 logger = logging.getLogger(__name__)
 
 
-async def archive(model: Model, *, batch_size: int) -> None:
+async def archive(model: Model, apps: Sequence[Application], *, batch_size: int) -> None:
     """Archive data on a nova-cloud-controller unit.
 
     The archive-data action only runs a single batch,
@@ -32,12 +32,14 @@ async def archive(model: Model, *, batch_size: int) -> None:
     https://docs.openstack.org/project-deploy-guide/charm-deployment-guide/wallaby/upgrade-openstack.html#archive-old-database-data
     :param model: juju model to work with
     :type model: Model
+    :param apps: Applications in the model
+    :type apps: Sequence of Applications
     :param batch_size: batch-size to pass to the archive-data action
         (default is 1000; decrease if performance issues seen)
     :type batch_size: int
     :raises COUException: if action returned unexpected output
     """  # noqa: E501 line too long
-    unit_name: str = await _get_nova_cloud_controller_unit_name(model)
+    unit_name: str = _get_nova_cloud_controller_unit_name(apps)
     # The archive-data action only archives a single batch,
     # so we must run it in a loop until everything is archived.
     while True:
@@ -64,12 +66,14 @@ async def archive(model: Model, *, batch_size: int) -> None:
         logger.debug("Potentially more data to archive...")
 
 
-async def purge(model: Model, before: Optional[str]) -> None:
+async def purge(model: Model, apps: Sequence[Application], before: Optional[str]) -> None:
     """Purge data on a nova-cloud-controller unit.
 
     The purge-data action delete rows from shadow tables.
     :param model: juju model to work with
     :type model: Model
+    :param apps: Applications in the model
+    :type apps: Sequence of Applications
     :param before: specifying before will delete data from all shadow tables
         that is older than the data provided.
         Date string format should be YYYY-MM-DD[HH:mm][:ss]
@@ -79,7 +83,7 @@ async def purge(model: Model, before: Optional[str]) -> None:
     if before is not None:
         action_params = {"before": before}
 
-    unit_name: str = await _get_nova_cloud_controller_unit_name(model)
+    unit_name: str = _get_nova_cloud_controller_unit_name(apps)
     action = await model.run_action(
         unit_name=unit_name,
         action_name="purge-data",
@@ -102,27 +106,21 @@ async def purge(model: Model, before: Optional[str]) -> None:
         logger.info("purge-data action succeeded on %s", unit_name)
 
 
-async def _get_nova_cloud_controller_unit_name(model: Model) -> str:
+def _get_nova_cloud_controller_unit_name(apps: Sequence[Application]) -> str:
     """Get nova-cloud-controller application's first unit's name.
 
     Assumes only a single nova-cloud-controller application is deployed.
 
-    :param model: juju model to work with
-    :type model: Model
+    :param apps: Applications in the model
+    :type apps: Sequence of Applications
     :return: unit name
     :rtype: str
     :raises UnitNotFound: When cannot find a valid unit for 'nova-cloud-controller'
     :raises ApplicationNotFound: When cannot find a 'nova-cloud-controller' application
     """
-    status = await model.get_status()
-    for app_name, app_config in status.applications.items():
-        charm_name = await model.get_charm_name(app_name)
-        if charm_name == "nova-cloud-controller":
-            units = list(app_config.units.keys())
-            if units:
-                return units[0]
-            raise UnitNotFound(
-                f"Cannot find unit for 'nova-cloud-controller' in model '{model.name}'."
-            )
-
-    raise ApplicationNotFound(f"Cannot find 'nova-cloud-controller' in model '{model.name}'.")
+    apps = get_applications_by_charm_name(apps, "nova-cloud-controller")
+    for app in apps:
+        units = list(app.units.keys())
+        if units:
+            return units[0]
+    raise UnitNotFound("Cannot find unit for 'nova-cloud-controller'.")

cou/steps/plan.py

@@ -137,7 +137,9 @@ async def post_upgrade_sanity_checks(analysis_result: Analysis) -> None:
     """
     messages = []
     try:
-        await ceph.assert_osd_noout_state(analysis_result.model, state=False)
+        await ceph.assert_osd_noout_state(
+            analysis_result.model, analysis_result.apps_control_plane, state=False
+        )
     except ApplicationError:
         messages.append("Detected ceph 'noout' set, please unset noout for ceph cluster.")
 
@@ -435,7 +437,9 @@ def _get_pre_upgrade_steps(analysis_result: Analysis, args: CLIargs) -> list[Pre
         PreUpgradeStep(
             description="Verify ceph cluster 'noout' is unset",
             parallel=False,
-            coro=ceph.assert_osd_noout_state(analysis_result.model, state=False),
+            coro=ceph.assert_osd_noout_state(
+                analysis_result.model, analysis_result.apps_control_plane, state=False
+            ),
         ),
         PreUpgradeStep(
             description="Verify that all OpenStack applications are in idle state",
@@ -503,7 +507,11 @@ def _get_purge_data_steps(analysis_result: Analysis, args: CLIargs) -> list[PreU
         return [
             PreUpgradeStep(
                 description=msg,
-                coro=purge(analysis_result.model, before=args.purge_before),
+                coro=purge(
+                    analysis_result.model,
+                    analysis_result.apps_data_plane,  # we only need to pass nova-cloud-controller
+                    before=args.purge_before,
+                ),
             )
         ]
     return []
@@ -523,7 +531,11 @@ def _get_archive_data_steps(analysis_result: Analysis, args: CLIargs) -> list[Pr
         return [
             PreUpgradeStep(
                 description="Archive old database data on nova-cloud-controller",
-                coro=archive(analysis_result.model, batch_size=args.archive_batch_size),
+                coro=archive(
+                    analysis_result.model,
+                    analysis_result.apps_data_plane,
+                    batch_size=args.archive_batch_size,
+                ),
             )
         ]
     return []
@@ -543,7 +555,9 @@ def _get_set_noout_steps(analysis_result: Analysis, args: CLIargs) -> list[PreUp
         return [
             PreUpgradeStep(
                 description="Set ceph cluster 'noout' flag before data plane upgrade",
-                coro=ceph.osd_noout(analysis_result.model, enable=True),
+                coro=ceph.osd_noout(
+                    analysis_result.model, analysis_result.apps_control_plane, enable=True
+                ),
             )
         ]
     PlanStatus.add_message(
@@ -570,14 +584,18 @@ def _get_post_upgrade_steps(analysis_result: Analysis, args: CLIargs) -> list[Po
         steps.append(
             PostUpgradeStep(
                 "Ensure ceph-mon's 'require-osd-release' option matches the 'ceph-osd' version",
-                coro=ceph.set_require_osd_release_option(analysis_result.model),
+                coro=ceph.set_require_osd_release_option(
+                    analysis_result.model, analysis_result.apps_control_plane
+                ),
             )
         )
         if args.set_noout:
             steps.append(
                 PostUpgradeStep(
                     description="Unset ceph cluster 'noout' flag after data plane upgrade",
-                    coro=ceph.osd_noout(analysis_result.model, enable=False),
+                    coro=ceph.osd_noout(
+                        analysis_result.model, analysis_result.apps_control_plane, enable=False
+                    ),
                 )
             )
     return steps