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
55 changes: 55 additions & 0 deletions cou/utils/juju_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,61 @@ async def get_status(self) -> FullStatus:
model = await self._get_model()
return await model.get_status()

async def _dispatch_update_status_hook(self, unit_name: str) -> None:
"""Use dispatch to run the update-status hook.

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 use dispatch to
run the hook.

:param unit_name: Name of the unit to run update-status hook
:type unit_name: str
:raises CommandRunFailed: When update-status hook failed
"""
await self.run_on_unit(unit_name, "JUJU_DISPATCH_PATH=hooks/update-status ./dispatch")

async def _run_update_status_hook(self, unit_name: str) -> None:
"""Run the update-status hook directly.

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 run the hook
directly.

:param unit_name: Name of the unit to run update-status hook
:type unit_name: str
:raises CommandRunFailed: When update-status hook failed
"""
await self.run_on_unit(unit_name, "hooks/update-status")

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
try:
await self._dispatch_update_status_hook(unit_name)
except CommandRunFailed as e:
if "No such file or directory" not in str(e):
raise e
else:
return

# For charm written in legacy / reactive framework
try:
await self._run_update_status_hook(unit_name)
except CommandRunFailed as e:
if "No such file or directory" not in str(e):
raise e
else:
return

logger.debug("Skipped updating 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
141 changes: 141 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,123 @@ 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.logger")
@patch("cou.utils.juju_utils.Model._run_update_status_hook")
@patch("cou.utils.juju_utils.Model._dispatch_update_status_hook")
async def test_coumodel_update_status_use_dispatch(
use_dispatch, use_hooks, mocked_logger, mocked_model
):
"""Test Model update_status using dispatch."""
model = juju_utils.Model("test-model")
await model.update_status("test-unit/0")

use_dispatch.assert_awaited_once()
use_hooks.assert_not_awaited()
mocked_logger.assert_not_called()


@pytest.mark.asyncio
@patch("cou.utils.juju_utils.logger")
@patch("cou.utils.juju_utils.Model._run_update_status_hook")
@patch("cou.utils.juju_utils.Model._dispatch_update_status_hook")
async def test_coumodel_update_status_use_dispatch_failed(
use_dispatch, use_hooks, mocked_logger, mocked_model
):
"""Test Model update_status using dispatch failed."""
use_dispatch.side_effect = CommandRunFailed("some cmd", result={})
model = juju_utils.Model("test-model")

with pytest.raises(CommandRunFailed):
await model.update_status("test-unit/0")
use_dispatch.assert_awaited_once()
use_hooks.assert_not_awaited()
mocked_logger.assert_not_called()


@pytest.mark.asyncio
@patch("cou.utils.juju_utils.logger")
@patch("cou.utils.juju_utils.Model._run_update_status_hook")
@patch("cou.utils.juju_utils.Model._dispatch_update_status_hook")
async def test_coumodel_update_status_use_hooks(use_dispatch, use_hooks, mocked_model):
"""Test Model update_status using hooks."""
use_dispatch.side_effect = CommandRunFailed(
"some cmd",
result={
"stderr": (
"/tmp/juju-exec4159838212/script.sh: "
"line 1: ./dispatch: No such file or directory"
),
},
)
model = juju_utils.Model("test-model")
await model.update_status("test-unit/0")

use_dispatch.assert_awaited_once()
use_hooks.assert_awaited_once()


@pytest.mark.asyncio
@patch("cou.utils.juju_utils.logger")
@patch("cou.utils.juju_utils.Model._run_update_status_hook")
@patch("cou.utils.juju_utils.Model._dispatch_update_status_hook")
async def test_coumodel_update_status_use_hooks_failed(
use_dispatch, use_hooks, mocked_logger, mocked_model
):
"""Test Model update_status using hooks failed."""
use_dispatch.side_effect = CommandRunFailed(
"some cmd",
result={
"stderr": (
"/tmp/juju-exec4159838212/script.sh: "
"line 1: ./dispatch: No such file or directory"
)
},
)
use_hooks.side_effect = CommandRunFailed("some cmd", result={})
model = juju_utils.Model("test-model")

with pytest.raises(CommandRunFailed):
await model.update_status("test-unit/0")
use_dispatch.assert_awaited_once()
use_hooks.assert_not_awaited()
mocked_logger.assert_not_called()


@pytest.mark.asyncio
@patch("cou.utils.juju_utils.logger")
@patch("cou.utils.juju_utils.Model._run_update_status_hook")
@patch("cou.utils.juju_utils.Model._dispatch_update_status_hook")
async def test_coumodel_update_status_skipped(
use_dispatch, use_hooks, mocked_logger, mocked_model
):
"""Test skip Model update_status."""
use_dispatch.side_effect = CommandRunFailed(
"some cmd",
result={
"stderr": (
"/tmp/juju-exec4159838212/script.sh: "
"line 1: ./dispatch: No such file or directory"
)
},
)
use_hooks.side_effect = CommandRunFailed(
"some cmd",
result={
"stderr": (
"/tmp/juju-exec1660320022/script.sh: "
"line 1: hooks/update-status: No such file or directory"
)
},
)
model = juju_utils.Model("test-model")
await model.update_status("test-unit/0")

use_dispatch.assert_awaited_once()
use_hooks.assert_awaited_once()
mocked_logger.debug.assert_called_once()


@pytest.mark.asyncio
async def test_coumodel_set_application_configs(mocked_model):
"""Test Model set application configuration."""
Expand Down Expand Up @@ -769,3 +886,27 @@ def test_unit_repr():
def test_suborinate_unit_repr():
unit = juju_utils.SubordinateUnit(name="foo/0", charm="foo")
assert repr(unit) == "foo/0"


@pytest.mark.asyncio
async def test_run_update_status_hook(mocked_model):
"""Test Model _run_update_status hook."""
mocked_model.units.get.return_value = mocked_unit = AsyncMock(Unit)
mocked_unit.run.return_value = mocked_action = AsyncMock(Action)
mocked_action.results = {"return-code": 0, "stderr": ""}
model = juju_utils.Model("test-model")
await model._run_update_status_hook(mocked_unit)
mocked_unit.run.assert_awaited_once_with("hooks/update-status", timeout=None, block=True)


@pytest.mark.asyncio
async def test_dispatch_update_status_hook(mocked_model):
"""Test Model _dispatch_update_status hook."""
mocked_model.units.get.return_value = mocked_unit = AsyncMock(Unit)
mocked_unit.run.return_value = mocked_action = AsyncMock(Action)
mocked_action.results = {"return-code": 0, "stderr": ""}
model = juju_utils.Model("test-model")
await model._dispatch_update_status_hook(mocked_unit)
mocked_unit.run.assert_awaited_once_with(
"JUJU_DISPATCH_PATH=hooks/update-status ./dispatch", timeout=None, block=True
)