Skip to content

Commit

Permalink
Remove unused arg from _check_mismatched_versions (#582)
Browse files Browse the repository at this point in the history
`units` was not referenced in `_check_mismatched_versions`,
so we can remove it.
  • Loading branch information
samuelallan72 authored Oct 9, 2024
1 parent a00ecc3 commit d06f793
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 33 deletions.
8 changes: 2 additions & 6 deletions cou/apps/auxiliary.py
Original file line number Diff line number Diff line change
Expand Up @@ -423,23 +423,19 @@ def _check_version_pinning(self) -> None:
)
)

def upgrade_plan_sanity_checks(
self, target: OpenStackRelease, units: Optional[list[Unit]]
) -> None:
def upgrade_plan_sanity_checks(self, target: OpenStackRelease) -> None:
"""Run sanity checks before generating upgrade plan.
:param target: OpenStack release as target to upgrade.
:type target: OpenStackRelease
:param units: Units to generate upgrade plan, defaults to None
:type units: Optional[list[Unit]], optional
:raises ApplicationError: When application is wrongly configured.
:raises HaltUpgradePlanGeneration: When the application halt the upgrade plan generation.
:raises MismatchedOpenStackVersions: When the units of the app are running different
OpenStack versions.
"""
self._check_ovn_support()
self._check_version_pinning()
super().upgrade_plan_sanity_checks(target, units)
super().upgrade_plan_sanity_checks(target)


@AppFactory.register_application(["ovn-central", "ovn-dedicated-chassis"])
Expand Down
17 changes: 4 additions & 13 deletions cou/apps/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -398,23 +398,19 @@ async def _verify_workload_upgrade(self, target: OpenStackRelease, units: list[U
"COU in a few minutes."
)

def upgrade_plan_sanity_checks(
self, target: OpenStackRelease, units: Optional[list[Unit]]
) -> None:
def upgrade_plan_sanity_checks(self, target: OpenStackRelease) -> None:
"""Run sanity checks before generating upgrade plan.
:param target: OpenStack release as target to upgrade.
:type target: OpenStackRelease
:param units: Units to generate upgrade plan, defaults to None
:type units: Optional[list[Unit]], optional
:raises ApplicationError: When application is wrongly configured.
:raises HaltUpgradePlanGeneration: When the application halt the upgrade plan generation.
:raises MismatchedOpenStackVersions: When the units of the app are running
different OpenStack versions.
"""
self._check_channel()
self._check_application_target(target)
self._check_mismatched_versions(units)
self._check_mismatched_versions()
self._check_auto_restarts()
logger.info(
"%s application met all the necessary prerequisites to generate the upgrade plan",
Expand Down Expand Up @@ -497,7 +493,7 @@ def generate_upgrade_plan(
:return: Full upgrade plan if the Application is able to generate it.
:rtype: ApplicationUpgradePlan
"""
self.upgrade_plan_sanity_checks(target, units)
self.upgrade_plan_sanity_checks(target)

upgrade_plan = ApplicationUpgradePlan(f"Upgrade plan for '{self.name}' to '{target}'")
upgrade_plan.add_steps(self.pre_upgrade_steps(target, units))
Expand Down Expand Up @@ -912,14 +908,9 @@ def _check_application_target(self, target: OpenStackRelease) -> None:
f"than {target}. Ignoring."
)

def _check_mismatched_versions(self, units: Optional[list[Unit]]) -> None:
def _check_mismatched_versions(self) -> None:
"""Check that there are no mismatched versions on app units.
If units are passed, the application will be upgraded in unit-by-unit fashion,
and mismatch is not checked.
:param units: Units to generate upgrade plan
:type units: Optional[list[Unit]]
:raises MismatchedOpenStackVersions: When the units of the app are running
different OpenStack versions.
"""
Expand Down
6 changes: 1 addition & 5 deletions cou/apps/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,15 +247,11 @@ class Swift(OpenStackApplication):
valid OpenStack components, but not currently supported by COU for upgrade.
"""

def upgrade_plan_sanity_checks(
self, target: OpenStackRelease, units: Optional[list[Unit]]
) -> None:
def upgrade_plan_sanity_checks(self, target: OpenStackRelease) -> None:
"""Run sanity checks before generating upgrade plan.
:param target: OpenStack release as target to upgrade.
:type target: OpenStackRelease
:param units: Units to generate upgrade plan, defaults to None
:type units: Optional[list[Unit]], optional
:raises ApplicationNotSupported: When application is known but not currently
supported by COU.
"""
Expand Down
5 changes: 2 additions & 3 deletions cou/steps/hypervisor.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,10 @@ def _upgrade_plan_sanity_checks(
)
continue

units = group.app_units[app.name]
logger.info("running sanity checks for %s units of %s app", app.name, units)
logger.info("running sanity checks for %s app", app.name)
# Note(rgildein): We don't catch the error here because we shouldn't generate any
# update plan if sanity checks for any application fails.
app.upgrade_plan_sanity_checks(target, units)
app.upgrade_plan_sanity_checks(target)

def _generate_pre_upgrade_steps(
self, target: OpenStackRelease, group: HypervisorGroup
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/apps/test_auxiliary.py
Original file line number Diff line number Diff line change
Expand Up @@ -1054,7 +1054,7 @@ def test_ovn_version_pinning_principal(model):
)

with pytest.raises(ApplicationError, match=exp_regex_msg):
app.upgrade_plan_sanity_checks(target, list(app.units.values()))
app.upgrade_plan_sanity_checks(target)


@pytest.mark.parametrize("channel", ["55.7", "19.03"])
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/apps/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ def test_check_mismatched_versions_exception(mock_o7k_release_units, model):
)

with pytest.raises(MismatchedOpenStackVersions, match=exp_error_msg):
app._check_mismatched_versions(None)
app._check_mismatched_versions()


@patch("cou.apps.base.OpenStackApplication.o7k_release_units", new_callable=PropertyMock)
Expand Down Expand Up @@ -506,7 +506,7 @@ def test_check_mismatched_versions_with_nova_compute(mock_o7k_release_units, mod
workload_version="18.1.0",
)

assert app._check_mismatched_versions(None) is None
assert app._check_mismatched_versions() is None


@patch("cou.apps.base.OpenStackApplication.o7k_release_units", new_callable=PropertyMock)
Expand Down Expand Up @@ -550,7 +550,7 @@ def test_check_mismatched_versions(mock_o7k_release_units, model):
workload_version="17.0.1",
)

assert app._check_mismatched_versions([units["my-app/0"]]) is None
assert app._check_mismatched_versions() is None


@patch("cou.apps.base.OpenStackApplication.o7k_release", new_callable=PropertyMock)
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/steps/test_hypervisor.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ def test_upgrade_plan_sanity_checks():

planner._upgrade_plan_sanity_checks(target, group)

apps[0].upgrade_plan_sanity_checks.assert_called_once_with(target, app_units["app1"])
apps[1].upgrade_plan_sanity_checks.assert_called_once_with(target, app_units["app2"])
apps[0].upgrade_plan_sanity_checks.assert_called_once_with(target)
apps[1].upgrade_plan_sanity_checks.assert_called_once_with(target)
apps[2].upgrade_plan_sanity_checks.assert_not_called()


Expand Down

0 comments on commit d06f793

Please sign in to comment.