Skip to content

Commit

Permalink
- apply suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
gabrielcocenza committed Mar 14, 2024
1 parent f2e88c8 commit 46a68ff
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 35 deletions.
6 changes: 3 additions & 3 deletions cou/apps/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def __str__(self) -> str:
"name": unit.name,
"machine": unit.machine.machine_id,
"workload_version": unit.workload_version,
"os_version": str(self.unit_max_os_version(unit)),
"os_version": str(self.get_latest_os_version(unit)),
}
for unit in self.units.values()
},
Expand Down Expand Up @@ -191,7 +191,7 @@ def is_valid_track(self, charm_channel: str) -> bool:
except ValueError:
return self.is_from_charm_store

def unit_max_os_version(self, unit: COUUnit) -> OpenStackRelease:
def get_latest_os_version(self, unit: COUUnit) -> OpenStackRelease:
"""Get the latest compatible OpenStack release based on the unit workload version.
:param unit: Application Unit
Expand Down Expand Up @@ -251,7 +251,7 @@ def os_release_units(self) -> dict[OpenStackRelease, list[str]]:
"""
os_versions = defaultdict(list)
for unit in self.units.values():
os_version = self.unit_max_os_version(unit)
os_version = self.get_latest_os_version(unit)
os_versions[os_version].append(unit.name)
return dict(os_versions)

Expand Down
2 changes: 1 addition & 1 deletion cou/apps/channel_based.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
class ChannelBasedApplication(OpenStackApplication):
"""Application for charms that are channel based."""

def unit_max_os_version(self, unit: COUUnit) -> OpenStackRelease:
def get_latest_os_version(self, unit: COUUnit) -> OpenStackRelease:
"""Get the latest compatible OpenStack release based on the channel.
:param unit: COUUnit
Expand Down
29 changes: 14 additions & 15 deletions cou/steps/hypervisor.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,24 +126,25 @@ def machines(self) -> list[COUMachine]:
"""
return self._machines

def azs(self, target: OpenStackRelease) -> AZs:
def get_azs(self, target: OpenStackRelease) -> AZs:
"""Return a list of AZs defined in individual applications.
Each AZ contains a dictionary of application name and all units in the AZ.
Each AZ contains a dictionary of application name and all units not yet upgraded
in the AZ for a certain target.
eg.
az1:
- cinder:
- cinder/0
- cinder/1
- cinder/2
- cinder/0
- cinder/1
- cinder/2
- nova-compute
- nova-compute/0
- nova-compute/1
- nova-compute/2
- nova-compute/0
- nova-compute/1
- nova-compute/2
...
az2
- cinder
-cinder/3
-cinder/3
...
...
Expand All @@ -156,14 +157,12 @@ def azs(self, target: OpenStackRelease) -> AZs:
for app in self.apps:
for unit in app.units.values():
if unit.machine not in self.machines:
logger.debug("skipping machine %s", unit.machine.machine_id)
logger.info("skipping machine %s", unit.machine.machine_id)
continue

unit_os_release = app.unit_max_os_version(unit)
unit_os_release = app.get_latest_os_version(unit)
if unit_os_release >= target:
logger.debug(
"skipping unit %s is already on %s", unit.name, str(unit_os_release)
)
logger.debug("skipping unit %s is already on %s", unit.name, unit_os_release)
continue

# NOTE(rgildein): If there is no AZ, we will use empty string and all units will
Expand Down Expand Up @@ -279,7 +278,7 @@ def generate_upgrade_plan(self, target: OpenStackRelease, force: bool) -> Upgrad
:rtype: UpgradePlan
"""
plan = UpgradePlan("Upgrading all applications deployed on machines with hypervisor.")
for az, group in self.azs(target).items():
for az, group in self.get_azs(target).items():
hypervisor_plan = HypervisorUpgradePlan(f"Upgrade plan for '{group.name}' to {target}")

# pre upgrade steps
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/apps/test_auxiliary.py
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ def test_auxiliary_app_unknown_version_raise_ApplicationError(model):
workload_version=version,
)
with pytest.raises(ApplicationError, match=exp_msg):
app.unit_max_os_version(app.units[f"{charm}/0"])
app.get_latest_os_version(app.units[f"{charm}/0"])


def test_auxiliary_raise_error_unknown_series(model):
Expand Down Expand Up @@ -589,7 +589,7 @@ def test_ceph_mon_app(model):

assert app.channel == "pacific/stable"
assert app.os_origin == "cloud:focal-xena"
assert app.unit_max_os_version(app.units[f"{charm}/0"]) == OpenStackRelease("xena")
assert app.get_latest_os_version(app.units[f"{charm}/0"]) == OpenStackRelease("xena")
assert app.apt_source_codename == "xena"
assert app.channel_codename == "xena"
assert app.is_subordinate is False
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/apps/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def test_openstack_application_magic_functions(model):

@patch("cou.apps.base.OpenStackApplication._verify_channel", return_value=None)
@patch("cou.utils.openstack.OpenStackCodenameLookup.find_compatible_versions")
def test_applicationunit_max_os_version_failed(mock_find_compatible_versions, model):
def test_applicationget_latest_os_version_failed(mock_find_compatible_versions, model):
charm = "app"
app_name = "my_app"
unit = COUUnit(
Expand Down Expand Up @@ -83,7 +83,7 @@ def test_applicationunit_max_os_version_failed(mock_find_compatible_versions, mo
)

with pytest.raises(ApplicationError, match=exp_error):
app.unit_max_os_version(unit)
app.get_latest_os_version(unit)

mock_find_compatible_versions.assert_called_once_with(charm, unit.workload_version)

Expand Down
2 changes: 1 addition & 1 deletion tests/unit/apps/test_channel_based.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def test_application_versionless(model):

assert app.current_os_release == "ussuri"
assert app.is_versionless is True
assert app.unit_max_os_version(units["glance-simplestreams-sync/0"]) == app.channel_codename
assert app.get_latest_os_version(units["glance-simplestreams-sync/0"]) == app.channel_codename


def test_application_gnocchi_ussuri(model):
Expand Down
22 changes: 11 additions & 11 deletions tests/unit/steps/test_hypervisor.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,12 @@ def test_hypervisor_azs_grouping():
app1 = MagicMock(spec_set=COUApplication)()
app1.name = "app1"
app1.units = {name: unit for name, unit in units.items() if name.startswith("app1")}
app1.unit_max_os_version.return_value = OpenStackRelease("ussuri")
app1.get_latest_os_version.return_value = OpenStackRelease("ussuri")

app2 = MagicMock(spec_set=COUApplication)()
app2.name = "app2"
app2.units = {name: unit for name, unit in units.items() if name.startswith("app2")}
app2.unit_max_os_version.return_value = OpenStackRelease("ussuri")
app2.get_latest_os_version.return_value = OpenStackRelease("ussuri")

# passing all machines to the HypervisorUpgradePlanner
exp_azs_all = AZs()
Expand All @@ -161,22 +161,22 @@ def test_hypervisor_azs_grouping():

hypervisor_planner_all = HypervisorUpgradePlanner([app1, app2], list(machines.values()))

assert dict(hypervisor_planner_all.azs(target)) == exp_azs_all
assert dict(hypervisor_planner_all.get_azs(target)) == exp_azs_all

# passing machine 0 to the HypervisorUpgradePlanner
exp_azs_0 = AZs()
exp_azs_0["az0"].app_units["app1"] = [units["app1/0"]]
exp_azs_0["az0"].app_units["app2"] = [units["app2/0"]]

hypervisor_planner_machine_0 = HypervisorUpgradePlanner([app1, app2], [machines["0"]])
assert dict(hypervisor_planner_machine_0.azs(target)) == exp_azs_0
assert dict(hypervisor_planner_machine_0.get_azs(target)) == exp_azs_0

# passing machine 1 to the HypervisorUpgradePlanner
exp_azs_1 = AZs()
exp_azs_1["az0"].app_units["app1"] = [units["app1/1"]]

hypervisor_planner_machine_1 = HypervisorUpgradePlanner([app1, app2], [machines["1"]])
assert dict(hypervisor_planner_machine_1.azs(target)) == exp_azs_1
assert dict(hypervisor_planner_machine_1.get_azs(target)) == exp_azs_1


def test_hypervisor_azs_grouping_units_different_os_release():
Expand Down Expand Up @@ -241,7 +241,7 @@ def side_effect_app1(value):
}
return os_release[value.name]

app1.unit_max_os_version.side_effect = side_effect_app1
app1.get_latest_os_version.side_effect = side_effect_app1

app2 = MagicMock(spec_set=COUApplication)()
app2.name = "app2"
Expand All @@ -255,7 +255,7 @@ def side_effect_app2(value):
}
return os_release[value.name]

app2.unit_max_os_version.side_effect = side_effect_app2
app2.get_latest_os_version.side_effect = side_effect_app2

# passing all machines to the HypervisorUpgradePlanner
exp_azs_all = AZs()
Expand All @@ -266,19 +266,19 @@ def side_effect_app2(value):

hypervisor_planner_all = HypervisorUpgradePlanner([app1, app2], list(machines.values()))

assert dict(hypervisor_planner_all.azs(target)) == exp_azs_all
assert dict(hypervisor_planner_all.get_azs(target)) == exp_azs_all

# passing machine 0 to the HypervisorUpgradePlanner
exp_azs_0 = AZs()

hypervisor_planner_machine_0 = HypervisorUpgradePlanner([app1, app2], [machines["0"]])
assert dict(hypervisor_planner_machine_0.azs(target)) == exp_azs_0
assert dict(hypervisor_planner_machine_0.get_azs(target)) == exp_azs_0

# passing machine 1 to the HypervisorUpgradePlanner
exp_azs_1 = AZs()

hypervisor_planner_machine_1 = HypervisorUpgradePlanner([app1, app2], [machines["1"]])
assert dict(hypervisor_planner_machine_1.azs(target)) == exp_azs_1
assert dict(hypervisor_planner_machine_1.get_azs(target)) == exp_azs_1


def test_hypervisor_upgrade_plan(model):
Expand Down Expand Up @@ -502,7 +502,7 @@ def test_hypervisor_upgrade_plan_some_units_upgraded(model):
"""Testing generating hypervisors upgrade plan partially upgraded."""
target = OpenStackRelease("victoria")
exp_plan = dedent_plan(
"""
"""\
Upgrading all applications deployed on machines with hypervisor.
Upgrade plan for 'az-1' to victoria
Upgrade software packages of 'cinder' from the current APT repositories
Expand Down

0 comments on commit 46a68ff

Please sign in to comment.