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

feat: Restart ceilometer-agent-compute after nova-compute upgrade #435

39 changes: 38 additions & 1 deletion cou/apps/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,44 @@ def post_upgrade_steps(
:return: List of post upgrade steps.
:rtype: list[PostUpgradeStep]
"""
return self._get_enable_scheduler_step(units) + super().post_upgrade_steps(target, units)
return (
self._get_enable_scheduler_step(units)
+ self._get_restart_subordinate_services_steps(units)
+ super().post_upgrade_steps(target, units)
)

def _get_restart_subordinate_services_steps(
jneo8 marked this conversation as resolved.
Show resolved Hide resolved
self, units: Optional[list[Unit]]
) -> list[PostUpgradeStep]:
"""Get step to restart all subordinate services if they aren't running.

:param units: Units to restart subordinate services.
:type units: Optional[list[Unit]]
:return: Steps to restart to subordinate services
:rtype: list[PostUpgradeStep]
"""
if not units:
return []
steps = []
for unit in units:
for subordinate in unit.subordinates:
if "ceilometer-agent" in subordinate.name:
gabrielcocenza marked this conversation as resolved.
Show resolved Hide resolved
service = "ceilometer-agent-compute"
steps.append(
PostUpgradeStep(
description=(
f"Restart subordinate service for unit: {subordinate.name}"
),
coro=self.model.run_on_unit(
unit_name=subordinate.name,
command=(
f"systemctl is-active --quiet {service}"
f" || systemctl restart {service}"
),
),
)
)
return steps

def _get_unit_upgrade_steps(self, unit: Unit, force: bool) -> UnitUpgradeStep:
"""Get the upgrade steps for a single unit.
Expand Down
38 changes: 35 additions & 3 deletions cou/utils/juju_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@
import asyncio
import logging
import os
from dataclasses import dataclass
from dataclasses import dataclass, field
from datetime import datetime
from typing import Any, Callable, Optional
from typing import Any, Callable, List, Optional

from juju.action import Action
from juju.application import Application as JujuApplication
Expand Down Expand Up @@ -142,13 +142,30 @@ class Machine:
az: Optional[str] = None # simple deployments may not have azs


@dataclass(frozen=True)
class SubordinateUnit:
"""Representation of a single unit of subordinate unit."""

name: str
workload_version: str
jneo8 marked this conversation as resolved.
Show resolved Hide resolved

def __repr__(self) -> str:
"""App representation.

:return: Name of the application
:rtype: str
"""
return self.name


@dataclass(frozen=True)
class Unit:
"""Representation of a single unit of application."""

name: str
machine: Machine
workload_version: str
subordinates: List[SubordinateUnit] = field(default_factory=lambda: [], compare=False)

def __repr__(self) -> str:
"""App representation.
Expand Down Expand Up @@ -367,6 +384,13 @@ async def get_applications(self) -> dict[str, Application]:
full_status = await self.get_status()
machines = await self._get_machines()

for app, status in full_status.applications.items():
logger.debug(app)
for name, unit in status.units.items():
logger.debug(name)
logger.debug(unit)
logger.debug(unit.subordinates)

gabrielcocenza marked this conversation as resolved.
Show resolved Hide resolved
return {
app: Application(
name=app,
Expand All @@ -383,7 +407,15 @@ async def get_applications(self) -> dict[str, Application]:
series=status.series,
subordinate_to=status.subordinate_to,
units={
name: Unit(name, machines[unit.machine], unit.workload_version)
name: Unit(
name,
machines[unit.machine],
unit.workload_version,
[
SubordinateUnit(subordinate_name, subordinate.workload_version)
for subordinate_name, subordinate in unit.subordinates.items()
jneo8 marked this conversation as resolved.
Show resolved Hide resolved
],
)
for name, unit in status.units.items()
},
workload_version=status.workload_version,
Expand Down
45 changes: 42 additions & 3 deletions tests/unit/apps/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
)
from cou.utils import app_utils
from cou.utils import nova_compute as nova_compute_utils
from cou.utils.juju_utils import Unit
from cou.utils.juju_utils import SubordinateUnit, Unit
from cou.utils.openstack import OpenStackRelease
from tests.unit.utils import assert_steps, dedent_plan, generate_cou_machine

Expand Down Expand Up @@ -631,7 +631,14 @@ def test_nova_compute_pre_upgrade_steps(
@patch("cou.apps.base.OpenStackApplication._get_wait_step")
@patch("cou.apps.base.OpenStackApplication._get_reached_expected_target_step")
@patch("cou.apps.core.NovaCompute._get_enable_scheduler_step")
def test_nova_compute_post_upgrade_steps(mock_enable, mock_expected_target, mock_wait_step, model):
@patch("cou.apps.core.NovaCompute._get_restart_subordinate_services_steps")
def test_nova_compute_post_upgrade_steps(
mock_restart_subordinate,
mock_enable,
mock_expected_target,
mock_wait_step,
model,
):
app = _generate_nova_compute_app(model)
target = OpenStackRelease("victoria")
units = list(app.units.values())
Expand All @@ -640,6 +647,7 @@ def test_nova_compute_post_upgrade_steps(mock_enable, mock_expected_target, mock
mock_enable.assert_called_once_with(units)
mock_expected_target.assert_called_once_with(target, units)
mock_wait_step.assert_called_once_with()
mock_restart_subordinate.assert_called_once_with(units)


@pytest.mark.parametrize("force", [True, False])
Expand Down Expand Up @@ -708,6 +716,32 @@ def test_nova_compute_get_enable_scheduler_step(model, units):
assert app._get_enable_scheduler_step(units_selected) == expected_step


@pytest.mark.parametrize(
"units",
[
[f"nova-compute/{unit}" for unit in range(1)],
[f"nova-compute/{unit}" for unit in range(2)],
[f"nova-compute/{unit}" for unit in range(3)],
samuelallan72 marked this conversation as resolved.
Show resolved Hide resolved
],
)
def test_nova_compute_get_restart_subordinate_services_steps(model, units):
app = _generate_nova_compute_app(model)
units_selected = [app.units[unit] for unit in units]
assert app._get_restart_subordinate_services_steps(units_selected) == [
PostUpgradeStep(
description=f"Restart subordinate service for unit: {unit.subordinates[0].name}",
coro=model.run_on_unit(
unit_name=unit.subordinates[0].name,
command=(
"systemctl is-active --quiet ceilometer-agent-compute"
" || systemctl restart ceilometer-agent-compute"
),
),
)
for unit in units_selected
]


def test_nova_compute_get_enable_scheduler_step_no_units(model):
"""Enable the scheduler on all units if no units are passed."""
app = _generate_nova_compute_app(model)
Expand Down Expand Up @@ -770,7 +804,12 @@ def _generate_nova_compute_app(model):
channel = "ussuri/stable"

units = {
f"nova-compute/{unit_num}": Unit(f"nova-compute/{unit_num}", MagicMock(), "21.0.1")
f"nova-compute/{unit_num}": Unit(
f"nova-compute/{unit_num}",
MagicMock(),
"21.0.1",
[SubordinateUnit("ceilometer-agent/{unit_num}", "14.1.0")],
)
for unit_num in range(3)
}
app = NovaCompute(
Expand Down
30 changes: 27 additions & 3 deletions tests/unit/utils/test_juju_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -625,10 +625,16 @@ def _generate_juju_machine(machine_id: str) -> MagicMock:
return machine


def _generate_unit_status(app: str, unit_id: int, machine_id: str) -> tuple[str, MagicMock]:
def _generate_unit_status(
app: str,
unit_id: int,
machine_id: str,
subordinates: dict[str, MagicMock] = {},
) -> tuple[str, MagicMock]:
"""Generate unit name and status."""
status = MagicMock(spec_set=UnitStatus)()
status.machine = machine_id
status.subordinates = subordinates
return f"{app}/{unit_id}", status


Expand Down Expand Up @@ -678,7 +684,9 @@ async def test_get_applications(mock_get_machines, mock_get_status, mocked_model
}
exp_units_from_status = {
"app1": dict([_generate_unit_status("app1", i, f"{i}") for i in range(3)]),
"app2": dict([_generate_unit_status("app2", 0, "0")]),
"app2": dict(
[_generate_unit_status("app2", 0, "0", dict([_generate_unit_status("app4", 0, "")]))]
),
"app3": dict([_generate_unit_status("app3", 0, "1")]),
"app4": {}, # subordinate application has no units defined in juju status
}
Expand Down Expand Up @@ -712,7 +720,18 @@ async def test_get_applications(mock_get_machines, mock_get_status, mocked_model
series=status.series,
subordinate_to=status.subordinate_to,
units={
name: juju_utils.Unit(name, exp_machines[unit.machine], unit.workload_version)
name: juju_utils.Unit(
name,
exp_machines[unit.machine],
unit.workload_version,
[
juju_utils.SubordinateUnit(
subordinate_name,
subordinate.workload_version,
)
for subordinate_name, subordinate in unit.subordinates.items()
],
)
for name, unit in exp_units_from_status[app].items()
},
workload_version=status.workload_version,
Expand Down Expand Up @@ -746,3 +765,8 @@ async def test_get_applications(mock_get_machines, mock_get_status, mocked_model
def test_unit_repr():
unit = juju_utils.Unit(name="foo/0", machine=MagicMock(), workload_version="1")
assert repr(unit) == "foo/0"


def test_suborinate_unit_repr():
unit = juju_utils.SubordinateUnit(name="foo/0", workload_version="1")
assert repr(unit) == "foo/0"
Loading