Wait for units to settle after charm refresh (canonical#528)

Avoids hitting race conditions in charms,
such as https://bugs.launchpad.net/charm-barbican/+bug/2039604

cou/apps/auxiliary.py

@@ -628,7 +628,7 @@ def post_upgrade_steps(
         :return: List of post upgrade steps.
         :rtype: list[PostUpgradeStep]
         """
-        upgrade_step = self._get_upgrade_charm_step(target=target)
+        upgrade_step = self._get_upgrade_charm_steps(target=target)
         steps = []
 
         # Add unseal steps only if chaneel is changed.

cou/apps/base.py

@@ -436,7 +436,7 @@ def pre_upgrade_steps(
         """
         return [
             self._get_upgrade_current_release_packages_step(units),
-            self._get_refresh_charm_step(target),
+            *self._get_refresh_charm_steps(target),
         ]
 
     def upgrade_steps(
@@ -455,7 +455,7 @@ def upgrade_steps(
         """
         return [
             self._set_action_managed_upgrade(enable=bool(units)),
-            self._get_upgrade_charm_step(target),
+            *self._get_upgrade_charm_steps(target),
             self._get_change_install_repository_step(target),
             self._get_units_upgrade_steps(units, force),
         ]
@@ -569,28 +569,37 @@ def _get_upgrade_current_release_packages_step(
 
         return step
 
-    def _get_refresh_charm_step(self, target: OpenStackRelease) -> PreUpgradeStep:
-        """Get step for refreshing the charm.
+    def _get_refresh_charm_steps(self, target: OpenStackRelease) -> list[PreUpgradeStep]:
+        """Get steps for refreshing the charm.
 
         :param target: OpenStack release as target to upgrade
         :type target: OpenStackRelease
         :raises ApplicationError: When application has unexpected channel.
-        :return: Step for refreshing the charm
-        :rtype: PreUpgradeStep
+        :return: Steps for refreshing the charm
+        :rtype: list[PreUpgradeStep]
         """
+        wait_step = PreUpgradeStep(
+            description=f"Wait for up to {STANDARD_IDLE_TIMEOUT}s for "
+            f"app '{self.name}' to reach the idle state",
+            parallel=False,
+            coro=self.model.wait_for_idle(STANDARD_IDLE_TIMEOUT, apps=[self.name]),
+        )
         if self.is_from_charm_store:
-            return self._get_charmhub_migration_step(target)
+            return [self._get_charmhub_migration_step(target), wait_step]
         if self.channel in LATEST_STABLE:
-            return self._get_change_channel_possible_downgrade_step(
-                target, self.expected_current_channel(target)
-            )
+            return [
+                self._get_change_channel_possible_downgrade_step(
+                    target, self.expected_current_channel(target)
+                ),
+                wait_step,
+            ]
 
         if self._need_current_channel_refresh(target):
-            return self._get_refresh_current_channel_step()
+            return [self._get_refresh_current_channel_step(), wait_step]
         logger.info(
             "'%s' does not need to refresh the current channel: %s", self.name, self.channel
         )
-        return PreUpgradeStep()
+        return []
 
     def _get_charmhub_migration_step(self, target: OpenStackRelease) -> PreUpgradeStep:
         """Get the step for charm hub migration from charm store.
@@ -656,30 +665,38 @@ def _need_current_channel_refresh(self, target: OpenStackRelease) -> bool:
         """
         return bool(self.can_upgrade_to) and self.channel_o7k_release <= target
 
-    def _get_upgrade_charm_step(self, target: OpenStackRelease) -> UpgradeStep:
-        """Get step for upgrading the charm.
+    def _get_upgrade_charm_steps(self, target: OpenStackRelease) -> list[UpgradeStep]:
+        """Get steps for upgrading the charm.
 
         :param target: OpenStack release as target to upgrade.
         :type target: OpenStackRelease
         :raises ApplicationError: When the current channel is ahead from expected and the target.
-        :return: Step for upgrading the charm.
-        :rtype: UpgradeStep
+        :return: List of steps for upgrading the charm.
+        :rtype: list[UpgradeStep]
         """
         channel = self.expected_current_channel(target) if self.need_crossgrade else self.channel
 
         if channel == self.target_channel(target):
             logger.debug("%s channel already set to %s", self.name, self.channel)
-            return UpgradeStep()
+            return []
 
         # Normally, prior the upgrade the channel is equal to the application release.
         # However, when colocated with other app, the channel can be in a release lesser than the
         # workload version of the application.
         if self.channel_o7k_release <= self.o7k_release or self.multiple_channels:
-            return UpgradeStep(
-                description=f"Upgrade '{self.name}' from '{channel}' to the new channel: "
-                f"'{self.target_channel(target)}'",
-                coro=self.model.upgrade_charm(self.name, self.target_channel(target)),
-            )
+            return [
+                UpgradeStep(
+                    description=f"Upgrade '{self.name}' from '{channel}' to the new channel: "
+                    f"'{self.target_channel(target)}'",
+                    coro=self.model.upgrade_charm(self.name, self.target_channel(target)),
+                ),
+                UpgradeStep(
+                    description=f"Wait for up to {STANDARD_IDLE_TIMEOUT}s for "
+                    f"app '{self.name}' to reach the idle state",
+                    parallel=False,
+                    coro=self.model.wait_for_idle(STANDARD_IDLE_TIMEOUT, apps=[self.name]),
+                ),
+            ]
 
         raise ApplicationError(
             f"The '{self.name}' application is using channel '{self.channel}'. Channels supported "

cou/apps/subordinate.py

@@ -69,7 +69,7 @@ def upgrade_steps(
         :return: List of upgrade steps.
         :rtype: list[UpgradeStep]
         """
-        return [self._get_upgrade_charm_step(target)]
+        return self._get_upgrade_charm_steps(target)
 
     def post_upgrade_steps(
         self, target: OpenStackRelease, units: Optional[list[Unit]]

tests/functional/tests/smoke.py

@@ -146,6 +146,7 @@ def generate_expected_plan(self, backup: bool = True) -> str:
             "\t\t\tUpgrade 'designate-bind' from 'ussuri/stable' to the new channel: "
             "'victoria/stable'\n"
             "\t\t\tWait for up to 300s for app 'designate-bind' to reach the idle state\n"
+            "\t\t\tWait for up to 300s for app 'designate-bind' to reach the idle state\n"
             "\t\t\tVerify that the workload of 'designate-bind' has been upgraded on units:"
             " designate-bind/0\n"
             "\t\tUpgrade plan for 'mysql-innodb-cluster' to 'victoria'\n"