Skip to content

Commit

Permalink
Check if applications status are active idle. (#201)
Browse files Browse the repository at this point in the history
* Check if applications status are active on wait_for_idle.

- Without status check if the unit is on maintenance it will
  proceed to upgrade or pass the post upgrade step.

Closes: #199
  • Loading branch information
gabrielcocenza authored Dec 15, 2023
1 parent d4bf059 commit 20b8a54
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 35 deletions.
2 changes: 1 addition & 1 deletion cou/apps/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -697,5 +697,5 @@ def _get_wait_step(self) -> PostUpgradeStep:
return PostUpgradeStep(
description=description,
parallel=False,
coro=self.model.wait_for_idle(self.wait_timeout, apps=apps),
coro=self.model.wait_for_active_idle(self.wait_timeout, apps=apps),
)
2 changes: 1 addition & 1 deletion cou/steps/plan.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ async def generate_plan(analysis_result: Analysis, backup_database: bool) -> Upg
PreUpgradeStep(
description="Verify that all OpenStack applications are in idle state",
parallel=False,
coro=analysis_result.model.wait_for_idle(
coro=analysis_result.model.wait_for_active_idle(
# NOTE (rgildein): We need to DEFAULT_TIMEOUT so it's possible to change if
# a network is too slow, this could cause an issue.
# We are using max function to ensure timeout is always at least 11 (1 second
Expand Down
11 changes: 6 additions & 5 deletions cou/utils/juju_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -445,14 +445,14 @@ async def upgrade_charm(
switch=switch,
)

async def wait_for_idle(
async def wait_for_active_idle(
self,
timeout: int,
idle_period: int = DEFAULT_MODEL_IDLE_PERIOD,
apps: Optional[list[str]] = None,
raise_on_blocked: bool = False,
) -> None:
"""Wait for applications to reach an idle state.
"""Wait for application(s) to reach active idle state.
If no applications are provided, this function will wait for all COU-related applications.
Expand All @@ -471,8 +471,8 @@ async def wait_for_idle(
"""

@retry(timeout=timeout, no_retry_exceptions=(WaitForApplicationsTimeout,))
@wraps(self.wait_for_idle)
async def _wait_for_idle() -> None:
@wraps(self.wait_for_active_idle)
async def _wait_for_active_idle() -> None:
# NOTE(rgildein): Defining wrapper so we can use retry with proper timeout
model = await self._get_model()
try:
Expand All @@ -481,6 +481,7 @@ async def _wait_for_idle() -> None:
timeout=timeout,
idle_period=idle_period,
raise_on_blocked=raise_on_blocked,
status="active",
)
except (asyncio.exceptions.TimeoutError, JujuAppError, JujuUnitError) as error:
# NOTE(rgildein): Catching TimeoutError raised as exception when wait_for_idle
Expand All @@ -497,4 +498,4 @@ async def _wait_for_idle() -> None:
if apps is None:
apps = await self._get_supported_apps()

await _wait_for_idle()
await _wait_for_active_idle()
4 changes: 2 additions & 2 deletions tests/functional/tests/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def test_changing_app_configuration(self):

# changing configuration and validating it was changed
zaza.sync_wrapper(self.model.set_application_config)(TESTED_APP, new_config)
zaza.sync_wrapper(self.model.wait_for_idle)(120, apps=[TESTED_APP])
zaza.sync_wrapper(self.model.wait_for_active_idle)(120, apps=[TESTED_APP])
config = zaza.sync_wrapper(self.model.get_application_config)(TESTED_APP)

self.assertTrue(config["debug"]["value"])
Expand All @@ -89,4 +89,4 @@ def test_upgrade_charm(self):
# get the current channel, so we will not change it
channel = status.applications[TESTED_APP].charm_channel
zaza.sync_wrapper(self.model.upgrade_charm)(TESTED_APP, channel=channel)
zaza.sync_wrapper(self.model.wait_for_idle)(120, apps=[TESTED_APP])
zaza.sync_wrapper(self.model.wait_for_active_idle)(120, apps=[TESTED_APP])
14 changes: 7 additions & 7 deletions tests/unit/apps/test_auxiliary.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ def test_auxiliary_upgrade_plan_ussuri_to_victoria_change_channel(status, config
PostUpgradeStep(
description=f"Wait 1800s for model {model.name} to reach the idle state.",
parallel=False,
coro=model.wait_for_idle(1800, apps=None),
coro=model.wait_for_active_idle(1800, apps=None),
),
PostUpgradeStep(
description=f"Check if the workload of '{app.name}' has been upgraded",
Expand Down Expand Up @@ -194,7 +194,7 @@ def test_auxiliary_upgrade_plan_ussuri_to_victoria(status, config, model):
PostUpgradeStep(
description=f"Wait 1800s for model {model.name} to reach the idle state.",
parallel=False,
coro=model.wait_for_idle(1800, apps=None),
coro=model.wait_for_active_idle(1800, apps=None),
),
PostUpgradeStep(
description=f"Check if the workload of '{app.name}' has been upgraded",
Expand Down Expand Up @@ -255,7 +255,7 @@ def test_auxiliary_upgrade_plan_ussuri_to_victoria_ch_migration(status, config,
PostUpgradeStep(
description=f"Wait 1800s for model {model.name} to reach the idle state.",
parallel=False,
coro=model.wait_for_idle(1800, apps=None),
coro=model.wait_for_active_idle(1800, apps=None),
),
PostUpgradeStep(
description=f"Check if the workload of '{app.name}' has been upgraded",
Expand Down Expand Up @@ -435,7 +435,7 @@ def test_ceph_mon_upgrade_plan_xena_to_yoga(
PostUpgradeStep(
description=f"Wait 1800s for model {model.name} to reach the idle state.",
parallel=False,
coro=model.wait_for_idle(1800, apps=None),
coro=model.wait_for_active_idle(1800, apps=None),
),
PostUpgradeStep(
description=f"Check if the workload of '{app.name}' has been upgraded",
Expand Down Expand Up @@ -498,7 +498,7 @@ def test_ceph_mon_upgrade_plan_ussuri_to_victoria(
PostUpgradeStep(
description=f"Wait 1800s for model {model.name} to reach the idle state.",
parallel=False,
coro=model.wait_for_idle(1800, apps=None),
coro=model.wait_for_active_idle(1800, apps=None),
),
PostUpgradeStep(
description=f"Check if the workload of '{app.name}' has been upgraded",
Expand Down Expand Up @@ -605,7 +605,7 @@ def test_ovn_principal_upgrade_plan(status, config, model):
PostUpgradeStep(
description=f"Wait 300s for app {app.name} to reach the idle state.",
parallel=False,
coro=model.wait_for_idle(300, apps=[app.name]),
coro=model.wait_for_active_idle(300, apps=[app.name]),
),
PostUpgradeStep(
description=f"Check if the workload of '{app.name}' has been upgraded",
Expand Down Expand Up @@ -660,7 +660,7 @@ def test_mysql_innodb_cluster_upgrade(status, config, model):
PostUpgradeStep(
description=f"Wait 1800s for app {app.name} to reach the idle state.",
parallel=False,
coro=model.wait_for_idle(1800, apps=[app.name]),
coro=model.wait_for_active_idle(1800, apps=[app.name]),
),
PostUpgradeStep(
description=f"Check if the workload of '{app.name}' has been upgraded",
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/apps/test_channel_based.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ def test_application_gnocchi_upgrade_plan_ussuri_to_victoria(status, config, mod
PostUpgradeStep(
description=f"Wait 300s for app {app.name} to reach the idle state.",
parallel=False,
coro=model.wait_for_idle(300, apps=[app.name]),
coro=model.wait_for_active_idle(300, apps=[app.name]),
),
PostUpgradeStep(
description=f"Check if the workload of '{app.name}' has been upgraded",
Expand Down Expand Up @@ -246,7 +246,7 @@ def test_application_designate_bind_upgrade_plan_ussuri_to_victoria(status, conf
PostUpgradeStep(
description=f"Wait 300s for app {app.name} to reach the idle state.",
parallel=False,
coro=model.wait_for_idle(300, apps=[app.name]),
coro=model.wait_for_active_idle(300, apps=[app.name]),
),
PostUpgradeStep(
description=f"Check if the workload of '{app.name}' has been upgraded",
Expand Down
10 changes: 5 additions & 5 deletions tests/unit/apps/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ def test_upgrade_plan_ussuri_to_victoria(status, config, model):
PostUpgradeStep(
description=f"Wait 1800s for model {model.name} to reach the idle state.",
parallel=False,
coro=model.wait_for_idle(1800, apps=None),
coro=model.wait_for_active_idle(1800, apps=None),
),
PostUpgradeStep(
description=f"Check if the workload of '{app.name}' has been upgraded",
Expand Down Expand Up @@ -425,7 +425,7 @@ def test_upgrade_plan_ussuri_to_victoria_ch_migration(status, config, model):
PostUpgradeStep(
description=f"Wait 1800s for model {model.name} to reach the idle state.",
parallel=False,
coro=model.wait_for_idle(1800, apps=None),
coro=model.wait_for_active_idle(1800, apps=None),
),
PostUpgradeStep(
description=f"Check if the workload of '{app.name}' has been upgraded",
Expand Down Expand Up @@ -477,7 +477,7 @@ def test_upgrade_plan_channel_on_next_os_release(status, config, model):
PostUpgradeStep(
description=f"Wait 1800s for model {model.name} to reach the idle state.",
parallel=False,
coro=model.wait_for_idle(1800, apps=None),
coro=model.wait_for_active_idle(1800, apps=None),
),
PostUpgradeStep(
description=f"Check if the workload of '{app.name}' has been upgraded",
Expand Down Expand Up @@ -527,7 +527,7 @@ def test_upgrade_plan_origin_already_on_next_openstack_release(status, config, m
PostUpgradeStep(
description=f"Wait 1800s for model {model.name} to reach the idle state.",
parallel=False,
coro=model.wait_for_idle(1800, apps=None),
coro=model.wait_for_active_idle(1800, apps=None),
),
PostUpgradeStep(
description=f"Check if the workload of '{app.name}' has been upgraded",
Expand Down Expand Up @@ -601,7 +601,7 @@ def test_upgrade_plan_application_already_disable_action_managed(status, config,
PostUpgradeStep(
description=f"Wait 1800s for model {model.name} to reach the idle state.",
parallel=False,
coro=model.wait_for_idle(1800, apps=None),
coro=model.wait_for_active_idle(1800, apps=None),
),
PostUpgradeStep(
description=f"Check if the workload of '{app.name}' has been upgraded",
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/steps/test_steps_plan.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,13 @@ def generate_expected_upgrade_plan_principal(app, target, model):
wait_step = PostUpgradeStep(
description=f"Wait 1800s for model {model.name} to reach the idle state.",
parallel=False,
coro=model.wait_for_idle(1800, apps=None),
coro=model.wait_for_active_idle(1800, apps=None),
)
else:
wait_step = PostUpgradeStep(
description=f"Wait 300s for app {app.name} to reach the idle state.",
parallel=False,
coro=model.wait_for_idle(300, apps=[app.name]),
coro=model.wait_for_active_idle(300, apps=[app.name]),
)

upgrade_steps = [
Expand Down Expand Up @@ -149,7 +149,7 @@ async def test_generate_plan(apps, model):
PreUpgradeStep(
description="Verify that all OpenStack applications are in idle state",
parallel=False,
coro=analysis_result.model.wait_for_idle(
coro=analysis_result.model.wait_for_active_idle(
timeout=11, idle_period=10, raise_on_blocked=True
),
)
Expand Down
21 changes: 12 additions & 9 deletions tests/unit/utils/test_juju_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -473,57 +473,60 @@ async def test_coumodel_upgrade_charm(mocked_model):

@pytest.mark.asyncio
@patch("cou.utils.juju_utils.COUModel._get_supported_apps")
async def test_coumodel_wait_for_idle(mock_get_supported_apps, mocked_model):
"""Test COUModel wait for related apps to be idle."""
async def test_coumodel_wait_for_active_idle(mock_get_supported_apps, mocked_model):
"""Test COUModel wait for related apps to be active idle."""
timeout = 60
model = juju_utils.COUModel("test-model")
mock_get_supported_apps.return_value = ["app1", "app2"]

await model.wait_for_idle(timeout)
await model.wait_for_active_idle(timeout)

mocked_model.wait_for_idle.assert_awaited_once_with(
apps=["app1", "app2"],
timeout=timeout,
idle_period=juju_utils.DEFAULT_MODEL_IDLE_PERIOD,
raise_on_blocked=False,
status="active",
)
mock_get_supported_apps.assert_awaited_once_with()


@pytest.mark.asyncio
@patch("cou.utils.juju_utils.COUModel._get_supported_apps")
async def test_coumodel_wait_for_idle_apps(mock_get_supported_apps, mocked_model):
"""Test COUModel wait for specific apps to be idle."""
async def test_coumodel_wait_for_active_idle_apps(mock_get_supported_apps, mocked_model):
"""Test COUModel wait for specific apps to be active idle."""
timeout = 60
model = juju_utils.COUModel("test-model")

await model.wait_for_idle(timeout, apps=["app1"])
await model.wait_for_active_idle(timeout, apps=["app1"])

mocked_model.wait_for_idle.assert_awaited_once_with(
apps=["app1"],
timeout=timeout,
idle_period=juju_utils.DEFAULT_MODEL_IDLE_PERIOD,
raise_on_blocked=False,
status="active",
)
mock_get_supported_apps.assert_not_awaited()


@pytest.mark.asyncio
@patch("cou.utils.juju_utils.COUModel._get_supported_apps")
async def test_coumodel_wait_for_idle_timeout(mock_get_supported_apps, mocked_model):
"""Test COUModel wait for model to be idle reach timeout."""
async def test_coumodel_wait_for_active_idle_timeout(mock_get_supported_apps, mocked_model):
"""Test COUModel wait for model to be active idle reach timeout."""
timeout = 60
exp_apps = ["app1", "app2"]
mocked_model.wait_for_idle.side_effect = asyncio.exceptions.TimeoutError
model = juju_utils.COUModel(None)

with pytest.raises(WaitForApplicationsTimeout):
await model.wait_for_idle(timeout, apps=exp_apps)
await model.wait_for_active_idle(timeout, apps=exp_apps)

mocked_model.wait_for_idle.assert_awaited_once_with(
apps=exp_apps,
timeout=timeout,
idle_period=juju_utils.DEFAULT_MODEL_IDLE_PERIOD,
raise_on_blocked=False,
status="active",
)
mock_get_supported_apps.assert_not_awaited()

0 comments on commit 20b8a54

Please sign in to comment.