Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix update-status hook can be missing in some charms #475

Merged
2 changes: 1 addition & 1 deletion cou/apps/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ async def _verify_workload_upgrade(self, target: OpenStackRelease, units: list[U
"""
# NOTE (gabrielcocenza) force the update-status hook on units
# to update the workload version
tasks = [self.model.run_on_unit(unit.name, "hooks/update-status") for unit in units]
tasks = [self.model.update_status(unit.name) for unit in units]
await asyncio.gather(*tasks)

status = await self.model.get_status()
Expand Down
57 changes: 57 additions & 0 deletions cou/utils/juju_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,63 @@ async def get_status(self) -> FullStatus:
model = await self._get_model()
return await model.get_status()

async def _use_dispatch(self, unit_name: str) -> bool:
"""Check if the charm support dispatch the hook or not.

Legacy and reactive charm allows the operators to directly run hooks
inside the charm code directly; while the operator framework uses
./dispatch script to dispatch the hooks. This method check if we could
use dispatch to run the hook.

:param unit_name: Name of the unit to run update-status hook
:type unit_name: str
:return: true if we can use dispatch
:rtype: bool
"""
try:
await self.run_on_unit(unit_name, "ls ./dispatch")
except CommandRunFailed:
return False
return True

async def _use_hooks(self, unit_name: str) -> bool:
"""Check if the charm support running the hook or not.

Legacy and reactive charm allows the operators to directly run hooks
inside the charm code directly; while the operator framework uses
./dispatch script to dispatch the hooks. This method check if we could
run the hook directly.

:param unit_name: Name of the unit to run update-status hook
:type unit_name: str
:return: true if we can use hooks
:rtype: bool
"""
try:
await self.run_on_unit(unit_name, "ls hooks/update-status")
except CommandRunFailed:
return False
return True

async def update_status(self, unit_name: str) -> None:
"""Run the update_status hook on the given unit.

:param unit_name: Name of the unit to run update-status hook
:type unit_name: str
:raises CommandRunFailed: When update-status hook failed
"""
# For charm written in operator framework
if await self._use_dispatch(unit_name):
await self.run_on_unit(unit_name, "JUJU_DISPATCH_PATH=hooks/update-status ./dispatch")
return

# For charm written in legacy / reactive framework
if await self._use_hooks(unit_name):
await self.run_on_unit(unit_name, "hooks/update-status")
return
chanchiwai-ray marked this conversation as resolved.
Show resolved Hide resolved

logger.debug("Skipped running hooks/update-status: file does not exist")

# NOTE (rgildein): There is no need to add retry here, because we don't want to repeat
# `unit.run_action(...)` and the rest of the function is covered by retry.
async def run_action(
Expand Down
78 changes: 78 additions & 0 deletions tests/unit/utils/test_juju_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,84 @@ async def test_coumodel_run_on_unit_failed_command(mocked_model):
mocked_unit.run.assert_awaited_once_with(command, timeout=None, block=True)


@pytest.mark.asyncio
@patch("cou.utils.juju_utils.Model._use_hooks")
@patch("cou.utils.juju_utils.Model._use_dispatch")
async def test_coumodel_update_status_use_dispatch(use_dispatch, use_hooks, mocked_model):
"""Test Model update_status using dispatch."""
use_hooks.return_value = False
use_dispatch.return_value = True
expected_results = {"return-code": 0, "stderr": ""}
mocked_model.units.get.return_value = mocked_unit = AsyncMock(Unit)
mocked_unit.run.return_value = mocked_action = AsyncMock(Action)
mocked_action.results = expected_results
model = juju_utils.Model("test-model")
await model.update_status("test-unit/0")

mocked_unit.run.assert_awaited_once_with(
"JUJU_DISPATCH_PATH=hooks/update-status ./dispatch", timeout=None, block=True
)


@pytest.mark.asyncio
@patch("cou.utils.juju_utils.Model._use_hooks")
@patch("cou.utils.juju_utils.Model._use_dispatch")
async def test_coumodel_update_status_use_hooks(use_dispatch, use_hooks, mocked_model):
"""Test Model update_status using hooks."""
use_hooks.return_value = True
use_dispatch.return_value = False
expected_results = {"return-code": 0, "stderr": ""}
mocked_model.units.get.return_value = mocked_unit = AsyncMock(Unit)
mocked_unit.run.return_value = mocked_action = AsyncMock(Action)
mocked_action.results = expected_results
model = juju_utils.Model("test-model")
await model.update_status("test-unit/0")

mocked_unit.run.assert_awaited_once_with("hooks/update-status", timeout=None, block=True)


@pytest.mark.asyncio
@patch("cou.utils.juju_utils.logger")
@patch("cou.utils.juju_utils.Model._use_hooks")
@patch("cou.utils.juju_utils.Model._use_dispatch")
async def test_coumodel_update_status_skipped(use_dispatch, use_hooks, mock_logger, mocked_model):
"""Test skip Model update_status."""
use_hooks.return_value = False
use_dispatch.return_value = False
expected_results = {"return-code": 0, "stderr": ""}
mocked_model.units.get.return_value = mocked_unit = AsyncMock(Unit)
mocked_unit.run.return_value = mocked_action = AsyncMock(Action)
mocked_action.results = expected_results
model = juju_utils.Model("test-model")
await model.update_status("test-unit/0")

mock_logger.debug.assert_called_with(
"Skipped running hooks/update-status: file does not exist"
)


@pytest.mark.parametrize("return_code", [0, 1])
@pytest.mark.asyncio
async def test_coumodel_use_dispatch(return_code, mocked_model):
expected_results = {"return-code": return_code, "stderr": ""}
mocked_model.units.get.return_value = mocked_unit = AsyncMock(Unit)
mocked_unit.run.return_value = mocked_action = AsyncMock(Action)
mocked_action.results = expected_results
model = juju_utils.Model("test-model")
assert await model._use_dispatch("test-unit/0") == (return_code == 0)


@pytest.mark.parametrize("return_code", [0, 1])
@pytest.mark.asyncio
async def test_coumodel_use_hooks(return_code, mocked_model):
expected_results = {"return-code": return_code, "stderr": ""}
mocked_model.units.get.return_value = mocked_unit = AsyncMock(Unit)
mocked_unit.run.return_value = mocked_action = AsyncMock(Action)
mocked_action.results = expected_results
model = juju_utils.Model("test-model")
assert await model._use_hooks("test-unit/0") == (return_code == 0)


@pytest.mark.asyncio
async def test_coumodel_set_application_configs(mocked_model):
"""Test Model set application configuration."""
Expand Down