Merge branch 'dev/data-plane' into mismatched_versions

cou/apps/__init__.py

@@ -12,3 +12,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 """Applications for charmed-openstack-upgrader."""
+import os
+
+STANDARD_IDLE_TIMEOUT: int = int(
+    os.environ.get("COU_STANDARD_IDLE_TIMEOUT", 5 * 60)
+)  # default of 5 min
+LONG_IDLE_TIMEOUT: int = int(os.environ.get("COU_LONG_IDLE_TIMEOUT", 30 * 60))  # default of 30 min

cou/apps/auxiliary.py

@@ -15,6 +15,7 @@
 import logging
 from typing import Optional
 
+from cou.apps import LONG_IDLE_TIMEOUT
 from cou.apps.base import OpenStackApplication
 from cou.apps.factory import AppFactory
 from cou.exceptions import ApplicationError
@@ -129,15 +130,15 @@ class RabbitMQServer(AuxiliaryApplication):
     RabbitMQ must wait for the entire model to be idle before declaring the upgrade complete.
     """
 
-    wait_timeout = 30 * 60  # 30 min
+    wait_timeout = LONG_IDLE_TIMEOUT
     wait_for_model = True
 
 
 @AppFactory.register_application(["ceph-mon"])
 class CephMon(AuxiliaryApplication):
     """Application for Ceph Monitor charm."""
 
-    wait_timeout = 30 * 60  # 30 min
+    wait_timeout = LONG_IDLE_TIMEOUT
     wait_for_model = True
 
     def pre_upgrade_steps(
@@ -200,7 +201,7 @@ class MysqlInnodbCluster(AuxiliaryApplication):
     # NOTE(agileshaw): holding 'mysql-server-core-8.0' package prevents undesired
     # mysqld processes from restarting, which lead to outages
     packages_to_hold: Optional[list] = ["mysql-server-core-8.0"]
-    wait_timeout = 30 * 60  # 30 min
+    wait_timeout = LONG_IDLE_TIMEOUT
 
 
 # NOTE (gabrielcocenza): Although CephOSD class is empty now, it will be

cou/apps/base.py

@@ -22,6 +22,7 @@
 
 import yaml
 
+from cou.apps import STANDARD_IDLE_TIMEOUT
 from cou.exceptions import ApplicationError, HaltUpgradePlanGeneration
 from cou.steps import (
     ApplicationUpgradePlan,
@@ -40,7 +41,6 @@
 
 logger = logging.getLogger(__name__)
 
-DEFAULT_WAITING_TIMEOUT = 5 * 60  # 5 min
 ORIGIN_SETTINGS = ("openstack-origin", "source")
 REQUIRED_SETTINGS = ("enable-auto-restarts", "action-managed-upgrade", *ORIGIN_SETTINGS)
 
@@ -59,7 +59,7 @@ class OpenStackApplication(COUApplication):
     """
 
     packages_to_hold: Optional[list] = field(default=None, init=False)
-    wait_timeout: int = field(default=DEFAULT_WAITING_TIMEOUT, init=False)
+    wait_timeout: int = field(default=STANDARD_IDLE_TIMEOUT, init=False)
     wait_for_model: bool = field(default=False, init=False)  # waiting only for application itself
 
     def __post_init__(self) -> None:
@@ -360,14 +360,6 @@ def upgrade_steps(
         :rtype: list[UpgradeStep]
         """
         # pylint: disable=unused-argument
-        if self.current_os_release >= target and self.apt_source_codename >= target:
-            msg = (
-                f"Application '{self.name}' already configured for release equal or greater "
-                f"than {target}. Ignoring."
-            )
-            logger.info(msg)
-            raise HaltUpgradePlanGeneration(msg)
-
         return [
             self._set_action_managed_upgrade(enable=bool(units)),
             self._get_upgrade_charm_step(target),
@@ -394,6 +386,21 @@ def post_upgrade_steps(
             self._get_reached_expected_target_step(target, units),
         ]
 
+    def upgrade_plan_sanity_checks(self, target: OpenStackRelease) -> None:
+        """Run sanity checks before generating upgrade plan.
+
+        :param target: OpenStack release as target to upgrade.
+        :type target: OpenStackRelease
+        :raises ApplicationError: When enable-auto-restarts is not enabled.
+        :raises HaltUpgradePlanGeneration: When the application halt the upgrade plan generation.
+        """
+        self._check_application_target(target)
+        self._check_auto_restarts()
+        logger.info(
+            "%s application met all the necessary prerequisites to generate the upgrade plan",
+            self.name,
+        )
+
     def generate_upgrade_plan(
         self,
         target: OpenStackRelease,
@@ -413,6 +420,8 @@ def generate_upgrade_plan(
         :return: Full upgrade plan if the Application is able to generate it.
         :rtype: ApplicationUpgradePlan
         """
+        self.upgrade_plan_sanity_checks(target)
+
         upgrade_plan = ApplicationUpgradePlan(
             description=f"Upgrade plan for '{self.name}' to {target}",
         )
@@ -733,3 +742,43 @@ def _get_wait_step(self) -> PostUpgradeStep:
             parallel=False,
             coro=self.model.wait_for_active_idle(self.wait_timeout, apps=apps),
         )
+
+    def _check_auto_restarts(self) -> None:
+        """Check if enable-auto-restarts is enabled.
+
+        If the enable-auto-restart option is not enabled, this check will raise an exception.
+
+        :raises ApplicationError: When enable-auto-restarts is not enabled.
+        """
+        if "enable-auto-restarts" not in self.config:
+            logger.debug(
+                "%s application does not have an enable-auto-restarts config option", self.name
+            )
+            return
+
+        if self.config["enable-auto-restarts"].get("value") is False:
+            raise ApplicationError(
+                "COU does not currently support upgrading applications that disable service "
+                "restarts. Please enable charm option enable-auto-restart and rerun COU to "
+                f"upgrade the {self.name} application."
+            )
+
+    def _check_application_target(self, target: OpenStackRelease) -> None:
+        """Check if application release is not lower than or equal to target.
+
+        :param target: OpenStack release as target to upgrade.
+        :type target: OpenStackRelease
+        :raises HaltUpgradePlanGeneration: When the application halt the upgrade plan generation.
+        """
+        logger.debug(
+            "%s application current os_release is %s and apt source is %s",
+            self.name,
+            self.current_os_release,
+            self.apt_source_codename,
+        )
+
+        if self.current_os_release >= target and self.apt_source_codename >= target:
+            raise HaltUpgradePlanGeneration(
+                f"Application '{self.name}' already configured for release equal to or greater "
+                f"than {target}. Ignoring."
+            )

cou/apps/core.py

@@ -16,6 +16,7 @@
 import logging
 from typing import Optional
 
+from cou.apps import LONG_IDLE_TIMEOUT
 from cou.apps.base import OpenStackApplication
 from cou.apps.factory import AppFactory
 from cou.steps import UnitUpgradeStep, UpgradeStep
@@ -33,7 +34,7 @@ class Keystone(OpenStackApplication):
     Keystone must wait for the entire model to be idle before declaring the upgrade complete.
     """
 
-    wait_timeout = 30 * 60  # 30 min
+    wait_timeout = LONG_IDLE_TIMEOUT
     wait_for_model = True
 
 
@@ -44,7 +45,7 @@ class Octavia(OpenStackApplication):
     Octavia required more time to settle before COU can continue.
     """
 
-    wait_timeout = 30 * 60  # 30 min
+    wait_timeout = LONG_IDLE_TIMEOUT
 
 
 @AppFactory.register_application(["nova-compute"])
@@ -54,7 +55,7 @@ class NovaCompute(OpenStackApplication):
     Nova Compute must wait for the entire model to be idle before declaring the upgrade complete.
     """
 
-    wait_timeout = 30 * 60  # 30 min
+    wait_timeout = LONG_IDLE_TIMEOUT
     wait_for_model = True
     upgrade_units_running_vms = False
 

docs/reference/environment-variables.rst

@@ -6,3 +6,5 @@ Environment Variables
 * **COU_TIMEOUT** - define timeout for **COU** retry policy. Default value is 10 seconds.
 * **COU_MODEL_RETRIES** - define how many times to retry the connection to Juju model before giving up. Default value is 5 times.
 * **COU_MODEL_RETRY_BACKOFF** - define number of seconds to increase the wait between connection to the Juju model retry attempts. Default value is 2 seconds.
+* **COU_STANDARD_IDLE_TIMEOUT** - how long COU will wait for an application to settle to active/idle and declare the upgrade complete. The default value is 300 seconds.
+* **COU_LONG_IDLE_TIMEOUT** - a longer version of COU_STANDARD_IDLE_TIMEOUT for applications that are known to need more time than usual to upgrade like such as Keystone and Octavia. The default value is 1800 seconds.

tests/unit/apps/test_auxiliary.py

@@ -487,7 +487,7 @@ def test_auxiliary_raise_halt_upgrade(model):
     target = OpenStackRelease("victoria")
     charm = "rabbitmq-server"
     exp_msg = (
-        f"Application '{charm}' already configured for release equal or greater than {target}. "
+        f"Application '{charm}' already configured for release equal to or greater than {target}. "
         "Ignoring."
     )
     machines = {"0": MagicMock(spec_set=COUMachine)}

tests/unit/apps/test_base.py

@@ -12,12 +12,12 @@
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
 
-from unittest.mock import MagicMock, call, patch
+from unittest.mock import MagicMock, PropertyMock, call, patch
 
 import pytest
 
 from cou.apps.base import OpenStackApplication
-from cou.exceptions import ApplicationError
+from cou.exceptions import ApplicationError, HaltUpgradePlanGeneration
 from cou.steps import UnitUpgradeStep, UpgradeStep
 from cou.utils.juju_utils import COUMachine, COUUnit
 from cou.utils.openstack import OpenStackRelease
@@ -296,3 +296,69 @@ def test_get_reached_expected_target_step(mock_workload_upgrade, units, model):
 
     app._get_reached_expected_target_step(target, units)
     mock_workload_upgrade.assert_has_calls(expected_calls)
+
+
+@pytest.mark.parametrize("config", ({}, {"enable-auto-restarts": {"value": True}}))
+@patch("cou.apps.base.OpenStackApplication._verify_channel", return_value=None)
+def test_check_auto_restarts(_, config):
+    """Test function to verify that enable-auto-restarts is disabled."""
+    app_name = "app"
+    app = OpenStackApplication(
+        app_name, "", app_name, "stable", config, {}, MagicMock(), "ch", "focal", [], {}, "1"
+    )
+
+    app._check_auto_restarts()
+
+
+@patch("cou.apps.base.OpenStackApplication._verify_channel", return_value=None)
+def test_check_auto_restarts_error(_):
+    """Test function to verify that enable-auto-restarts is disabled raising error."""
+    app_name = "app"
+    exp_error_msg = (
+        "COU does not currently support upgrading applications that disable service restarts. "
+        f"Please enable charm option enable-auto-restart and rerun COU to upgrade the {app_name} "
+        "application."
+    )
+    config = {"enable-auto-restarts": {"value": False}}
+    app = OpenStackApplication(
+        app_name, "", app_name, "stable", config, {}, MagicMock(), "ch", "focal", [], {}, "1"
+    )
+
+    with pytest.raises(ApplicationError, match=exp_error_msg):
+        app._check_auto_restarts()
+
+
+@patch("cou.apps.base.OpenStackApplication._verify_channel", return_value=None)
+@patch("cou.apps.base.OpenStackApplication.apt_source_codename", new_callable=PropertyMock)
+@patch("cou.apps.base.OpenStackApplication.current_os_release", new_callable=PropertyMock)
+def test_check_application_target(current_os_release, apt_source_codename, _):
+    """Test function to verify target."""
+    target = OpenStackRelease("victoria")
+    release = OpenStackRelease("ussuri")
+    app_name = "app"
+    app = OpenStackApplication(
+        app_name, "", app_name, "stable", {}, {}, MagicMock(), "ch", "focal", [], {}, "1"
+    )
+    current_os_release.return_value = apt_source_codename.return_value = release
+
+    app._check_application_target(target)
+
+
+@patch("cou.apps.base.OpenStackApplication._verify_channel", return_value=None)
+@patch("cou.apps.base.OpenStackApplication.apt_source_codename", new_callable=PropertyMock)
+@patch("cou.apps.base.OpenStackApplication.current_os_release", new_callable=PropertyMock)
+def test_check_application_target_error(current_os_release, apt_source_codename, _):
+    """Test function to verify target raising error."""
+    target = OpenStackRelease("victoria")
+    app_name = "app"
+    exp_error_msg = (
+        f"Application '{app_name}' already configured for release equal to or greater than "
+        f"{target}. Ignoring."
+    )
+    app = OpenStackApplication(
+        app_name, "", app_name, "stable", {}, {}, MagicMock(), "ch", "focal", [], {}, "1"
+    )
+    current_os_release.return_value = apt_source_codename.return_value = target
+
+    with pytest.raises(HaltUpgradePlanGeneration, match=exp_error_msg):
+        app._check_application_target(target)

tests/unit/apps/test_core.py

@@ -631,7 +631,7 @@ def test_upgrade_plan_origin_already_on_next_openstack_release(model):
 def test_upgrade_plan_application_already_upgraded(model):
     """Test generate plan to upgrade Keystone from Victoria to Victoria."""
     exp_error_msg = (
-        "Application 'keystone' already configured for release equal or greater "
+        "Application 'keystone' already configured for release equal to or greater "
         "than victoria. Ignoring."
     )
     target = OpenStackRelease("victoria")

tests/unit/apps/test_subordinate.py

@@ -36,7 +36,7 @@ def test_current_os_release(model):
         can_upgrade_to="ussuri/stable",
         charm="keystone-ldap",
         channel="ussuri/stable",
-        config={"source": {"value": "distro"}},
+        config={},
         machines=machines,
         model=model,
         origin="ch",
@@ -64,7 +64,7 @@ def test_generate_upgrade_plan(model):
         can_upgrade_to="ussuri/stable",
         charm="keystone-ldap",
         channel="ussuri/stable",
-        config={"source": {"value": "distro"}},
+        config={},
         machines=machines,
         model=model,
         origin="ch",
@@ -119,7 +119,7 @@ def test_channel_valid(model, channel):
         can_upgrade_to=channel,
         charm="keystone-ldap",
         channel=channel,
-        config={"source": {"value": "distro"}},
+        config={},
         machines=machines,
         model=model,
         origin="ch",
@@ -157,7 +157,7 @@ def test_channel_setter_invalid(model, channel):
             can_upgrade_to=channel,
             charm="keystone-ldap",
             channel=channel,
-            config={"source": {"value": "distro"}},
+            config={},
             machines=machines,
             model=model,
             origin="ch",
@@ -191,7 +191,7 @@ def test_generate_plan_ch_migration(model, channel):
         can_upgrade_to="wallaby/stable",
         charm="keystone-ldap",
         channel=f"ussuri/{channel}",
-        config={"source": {"value": "distro"}},
+        config={},
         machines=machines,
         model=model,
         origin="cs",
@@ -245,7 +245,7 @@ def test_generate_plan_from_to(model, from_os, to_os):
         can_upgrade_to=f"{to_os}/stable",
         charm="keystone-ldap",
         channel=f"{from_os}/stable",
-        config={"source": {"value": "distro"}},
+        config={},
         machines=machines,
         model=model,
         origin="ch",
@@ -298,7 +298,7 @@ def test_generate_plan_in_same_version(model, from_to):
         can_upgrade_to=f"{from_to}/stable",
         charm="keystone-ldap",
         channel=f"{from_to}/stable",
-        config={"source": {"value": "distro"}},
+        config={},
         machines=machines,
         model=model,
         origin="ch",