From 1a466e65c026052270b28b52e93bf4ddc4ac5014 Mon Sep 17 00:00:00 2001 From: Gabriel Cocenza Date: Tue, 6 Feb 2024 11:40:52 -0300 Subject: [PATCH] Change suffix of upgrade steps functions from plan to step (#235) - removed unnecessary parallel parameter. - fix lint to pass the recent [black](https://black.readthedocs.io/en/stable/guides/using_black_with_other_tools.html#configuration) changes --- cou/apps/auxiliary.py | 25 ++++---- cou/apps/auxiliary_subordinate.py | 8 +-- cou/apps/base.py | 98 ++++++++++++------------------- cou/apps/channel_based.py | 9 +-- cou/apps/subordinate.py | 23 ++++---- cou/utils/openstack.py | 10 ++-- pyproject.toml | 1 + tests/functional/tests/backup.py | 1 + tests/unit/steps/test_steps.py | 3 +- 9 files changed, 74 insertions(+), 104 deletions(-) diff --git a/cou/apps/auxiliary.py b/cou/apps/auxiliary.py index 82adb1ab..8c0f9290 100644 --- a/cou/apps/auxiliary.py +++ b/cou/apps/auxiliary.py @@ -139,31 +139,28 @@ class CephMonApplication(OpenStackAuxiliaryApplication): wait_timeout = 30 * 60 # 30 min wait_for_model = True - def pre_upgrade_plan(self, target: OpenStackRelease) -> list[PreUpgradeStep]: - """Pre Upgrade planning. + def pre_upgrade_steps(self, target: OpenStackRelease) -> list[PreUpgradeStep]: + """Pre Upgrade steps planning. :param target: OpenStack release as target to upgrade. :type target: OpenStackRelease - :return: Plan that will add pre upgrade as sub steps. + :return: List of pre upgrade steps. :rtype: list[PreUpgradeStep] """ - return super().pre_upgrade_plan(target) + [self._get_change_require_osd_release_plan()] + return super().pre_upgrade_steps(target) + [self._get_change_require_osd_release_step()] - def _get_change_require_osd_release_plan(self, parallel: bool = False) -> PreUpgradeStep: - """Get plan to set correct value for require-osd-release option on ceph-mon. + def _get_change_require_osd_release_step(self) -> PreUpgradeStep: + """Get the step to set correct value for require-osd-release option on ceph-mon. This step is needed as a workaround for LP#1929254. Reference: https://docs.openstack.org/charm-guide/latest/project/issues/upgrade-issues.html#ceph-require-osd-release - :param parallel: Parallel running, defaults to False - :type parallel: bool, optional - :return: Plan to check and set correct value for require-osd-release + :return: Step to check and set correct value for require-osd-release :rtype: PreUpgradeStep """ ceph_mon_unit, *_ = self.units return PreUpgradeStep( description="Ensure require-osd-release option matches with ceph-osd version", - parallel=parallel, coro=set_require_osd_release_option(ceph_mon_unit.name, self.model), ) @@ -172,17 +169,17 @@ def _get_change_require_osd_release_plan(self, parallel: bool = False) -> PreUpg class OvnPrincipalApplication(OpenStackAuxiliaryApplication): """Ovn principal application class.""" - def pre_upgrade_plan(self, target: OpenStackRelease) -> list[PreUpgradeStep]: - """Pre Upgrade planning. + def pre_upgrade_steps(self, target: OpenStackRelease) -> list[PreUpgradeStep]: + """Pre Upgrade steps planning. :param target: OpenStack release as target to upgrade. :type target: OpenStackRelease - :return: Plan that will add pre upgrade as sub steps. + :return: List of pre upgrade steps. :rtype: list[PreUpgradeStep] """ for unit in self.units: validate_ovn_support(unit.workload_version) - return super().pre_upgrade_plan(target) + return super().pre_upgrade_steps(target) @AppFactory.register_application(["mysql-innodb-cluster"]) diff --git a/cou/apps/auxiliary_subordinate.py b/cou/apps/auxiliary_subordinate.py index f094378d..fee3c52b 100644 --- a/cou/apps/auxiliary_subordinate.py +++ b/cou/apps/auxiliary_subordinate.py @@ -42,13 +42,13 @@ def current_os_release(self) -> OpenStackRelease: class OvnSubordinateApplication(OpenStackAuxiliarySubordinateApplication): """Ovn subordinate application class.""" - def pre_upgrade_plan(self, target: OpenStackRelease) -> list[PreUpgradeStep]: - """Pre Upgrade planning. + def pre_upgrade_steps(self, target: OpenStackRelease) -> list[PreUpgradeStep]: + """Pre Upgrade steps planning. :param target: OpenStack release as target to upgrade. :type target: OpenStackRelease - :return: Plan that will add pre upgrade as sub steps. + :return: List of pre upgrade steps. :rtype: list[PreUpgradeStep] """ validate_ovn_support(self.status.workload_version) - return super().pre_upgrade_plan(target) + return super().pre_upgrade_steps(target) diff --git a/cou/apps/base.py b/cou/apps/base.py index a612f97d..6af67ba2 100644 --- a/cou/apps/base.py +++ b/cou/apps/base.py @@ -439,26 +439,26 @@ async def _check_upgrade(self, target: OpenStackRelease) -> None: f"Cannot upgrade units '{units_not_upgraded_string}' to {target}." ) - def pre_upgrade_plan(self, target: OpenStackRelease) -> list[PreUpgradeStep]: - """Pre Upgrade planning. + def pre_upgrade_steps(self, target: OpenStackRelease) -> list[PreUpgradeStep]: + """Pre Upgrade steps planning. :param target: OpenStack release as target to upgrade. :type target: OpenStackRelease - :return: Plan that will add pre upgrade as sub steps. + :return: List of pre upgrade steps. :rtype: list[PreUpgradeStep] """ return [ - self._get_upgrade_current_release_packages_plan(), - self._get_refresh_charm_plan(target), + self._get_upgrade_current_release_packages_step(), + self._get_refresh_charm_step(target), ] - def upgrade_plan(self, target: OpenStackRelease) -> list[UpgradeStep]: - """Upgrade planning. + def upgrade_steps(self, target: OpenStackRelease) -> list[UpgradeStep]: + """Upgrade steps planning. :param target: OpenStack release as target to upgrade. :type target: OpenStackRelease :raises HaltUpgradePlanGeneration: When the application halt the upgrade plan generation. - :return: Plan that will add upgrade as sub steps. + :return: List of upgrade steps. :rtype: list[UpgradeStep] """ if self.current_os_release >= target and self.apt_source_codename >= target: @@ -470,24 +470,24 @@ def upgrade_plan(self, target: OpenStackRelease) -> list[UpgradeStep]: raise HaltUpgradePlanGeneration(msg) return [ - self._get_disable_action_managed_plan(), - self._get_upgrade_charm_plan(target), - self._get_workload_upgrade_plan(target), + self._get_disable_action_managed_step(), + self._get_upgrade_charm_step(target), + self._get_workload_upgrade_step(target), ] - def post_upgrade_plan(self, target: OpenStackRelease) -> list[PostUpgradeStep]: - """Post Upgrade planning. + def post_upgrade_steps(self, target: OpenStackRelease) -> list[PostUpgradeStep]: + """Post Upgrade steps planning. Wait until the application reaches the idle state and then check the target workload. :param target: OpenStack release as target to upgrade. :type target: OpenStackRelease - :return: Plan that will add post upgrade as sub steps. + :return: List of post upgrade steps. :rtype: list[PostUpgradeStep] """ return [ self._get_wait_step(), - self._get_reached_expected_target_plan(target), + self._get_reached_expected_target_step(target), ] def generate_upgrade_plan(self, target: OpenStackRelease) -> ApplicationUpgradePlan: @@ -502,44 +502,37 @@ def generate_upgrade_plan(self, target: OpenStackRelease) -> ApplicationUpgradeP description=f"Upgrade plan for '{self.name}' to {target}", ) all_steps = ( - self.pre_upgrade_plan(target) - + self.upgrade_plan(target) - + self.post_upgrade_plan(target) + self.pre_upgrade_steps(target) + + self.upgrade_steps(target) + + self.post_upgrade_steps(target) ) for step in all_steps: if step: upgrade_steps.add_step(step) return upgrade_steps - def _get_upgrade_current_release_packages_plan(self, parallel: bool = False) -> PreUpgradeStep: - """Get Plan for upgrading software packages to the latest of the current release. + def _get_upgrade_current_release_packages_step(self) -> PreUpgradeStep: + """Get step for upgrading software packages to the latest of the current release. - :param parallel: Parallel running, defaults to False - :type parallel: bool, optional - :return: Plan for upgrading software packages to the latest of the current release. + :return: Step for upgrading software packages to the latest of the current release. :rtype: PreUpgradeStep """ return PreUpgradeStep( description=( f"Upgrade software packages of '{self.name}' from the current APT repositories" ), - parallel=parallel, coro=upgrade_packages(self.status.units.keys(), self.model, self.packages_to_hold), ) - def _get_refresh_charm_plan( - self, target: OpenStackRelease, parallel: bool = False - ) -> PreUpgradeStep: - """Get plan for refreshing the current channel. + def _get_refresh_charm_step(self, target: OpenStackRelease) -> PreUpgradeStep: + """Get step for refreshing the current channel. This function also identifies if charm comes from charmstore and in that case, makes the migration. :param target: OpenStack release as target to upgrade. :type target: OpenStackRelease - :param parallel: Parallel running, defaults to False - :type parallel: bool, optional :raises ApplicationError: When application has unexpected channel. - :return: Plan for refreshing the charm. + :return: Step for refreshing the charm. :rtype: PreUpgradeStep """ if not self.can_upgrade_current_channel: @@ -582,20 +575,15 @@ def _get_refresh_charm_plan( return PreUpgradeStep( description=description, - parallel=parallel, coro=self.model.upgrade_charm(self.name, channel, switch=switch), ) - def _get_upgrade_charm_plan( - self, target: OpenStackRelease, parallel: bool = False - ) -> UpgradeStep: - """Get plan for upgrading the charm. + def _get_upgrade_charm_step(self, target: OpenStackRelease) -> UpgradeStep: + """Get step for upgrading the charm. :param target: OpenStack release as target to upgrade. :type target: OpenStackRelease - :param parallel: Parallel running, defaults to False - :type parallel: bool, optional - :return: Plan for upgrading the charm. + :return: Step for upgrading the charm. :rtype: UpgradeStep """ if self.channel != self.target_channel(target): @@ -603,19 +591,16 @@ def _get_upgrade_charm_plan( description=( f"Upgrade '{self.name}' to the new channel: '{self.target_channel(target)}'" ), - parallel=parallel, coro=self.model.upgrade_charm(self.name, self.target_channel(target)), ) return UpgradeStep() - def _get_disable_action_managed_plan(self, parallel: bool = False) -> UpgradeStep: - """Get plan to disable action-managed-upgrade. + def _get_disable_action_managed_step(self) -> UpgradeStep: + """Get step to disable action-managed-upgrade. This is used to upgrade as "all-in-one" strategy. - :param parallel: Parallel running, defaults to False - :type parallel: bool, optional - :return: Plan to disable action-managed-upgrade + :return: Step to disable action-managed-upgrade :rtype: UpgradeStep """ if self.config.get("action-managed-upgrade", {}).get("value", False): @@ -623,23 +608,18 @@ def _get_disable_action_managed_plan(self, parallel: bool = False) -> UpgradeSte description=( f"Change charm config of '{self.name}' 'action-managed-upgrade' to False." ), - parallel=parallel, coro=self.model.set_application_config( self.name, {"action-managed-upgrade": False} ), ) return UpgradeStep() - def _get_workload_upgrade_plan( - self, target: OpenStackRelease, parallel: bool = False - ) -> UpgradeStep: - """Get workload upgrade plan by changing openstack-origin or source. + def _get_workload_upgrade_step(self, target: OpenStackRelease) -> UpgradeStep: + """Get workload upgrade step by changing openstack-origin or source. :param target: OpenStack release as target to upgrade. :type target: OpenStackRelease - :param parallel: Parallel running, defaults to False - :type parallel: bool, optional - :return: Workload upgrade plan + :return: Workload upgrade step :rtype: UpgradeStep """ if self.os_origin != self.new_origin(target) and self.origin_setting: @@ -648,7 +628,6 @@ def _get_workload_upgrade_plan( f"Change charm config of '{self.name}' " f"'{self.origin_setting}' to '{self.new_origin(target)}'" ), - parallel=parallel, coro=self.model.set_application_config( self.name, {self.origin_setting: self.new_origin(target)} ), @@ -661,21 +640,16 @@ def _get_workload_upgrade_plan( ) return UpgradeStep() - def _get_reached_expected_target_plan( - self, target: OpenStackRelease, parallel: bool = False - ) -> PostUpgradeStep: - """Get plan to check if application workload has been upgraded. + def _get_reached_expected_target_step(self, target: OpenStackRelease) -> PostUpgradeStep: + """Get post upgrade step to check if application workload has been upgraded. :param target: OpenStack release as target to upgrade. :type target: OpenStackRelease - :param parallel: Parallel running, defaults to False - :type parallel: bool, optional - :return: Plan to check if application workload has been upgraded + :return: Post Upgrade step to check if application workload has been upgraded. :rtype: PostUpgradeStep """ return PostUpgradeStep( description=f"Check if the workload of '{self.name}' has been upgraded", - parallel=parallel, coro=self._check_upgrade(target), ) diff --git a/cou/apps/channel_based.py b/cou/apps/channel_based.py index 0ecb7b33..0fa0b3b2 100644 --- a/cou/apps/channel_based.py +++ b/cou/apps/channel_based.py @@ -60,16 +60,17 @@ def is_versionless(self) -> bool: """ return not all(unit.workload_version for unit in self.status.units.values()) - def post_upgrade_plan(self, target: OpenStackRelease) -> list[PostUpgradeStep]: - """Post Upgrade planning. + def post_upgrade_steps(self, target: OpenStackRelease) -> list[PostUpgradeStep]: + """Post Upgrade steps planning. Wait until the application reaches the idle state and then check the target workload. In case the application is versionless, there are no post upgrade steps to run. + :param target: OpenStack release as target to upgrade. :type target: OpenStackRelease - :return: Plan that will add post upgrade as sub steps. + :return: List of post upgrade steps. :rtype: list[PostUpgradeStep] """ if self.is_versionless: return [] - return super().post_upgrade_plan(target) + return super().post_upgrade_steps(target) diff --git a/cou/apps/subordinate.py b/cou/apps/subordinate.py index adcefc0a..b90b96a6 100644 --- a/cou/apps/subordinate.py +++ b/cou/apps/subordinate.py @@ -25,33 +25,32 @@ class SubordinateBaseClass(OpenStackApplication): """Subordinate base class.""" - def pre_upgrade_plan(self, target: OpenStackRelease) -> list[PreUpgradeStep]: - """Pre Upgrade planning. + def pre_upgrade_steps(self, target: OpenStackRelease) -> list[PreUpgradeStep]: + """Pre Upgrade steps planning. :param target: OpenStack release as target to upgrade. :type target: OpenStackRelease - :return: Plan that will add pre upgrade as sub steps. + :return: List of pre upgrade steps. :rtype: list[PreUpgradeStep] """ - return [self._get_refresh_charm_plan(target)] + return [self._get_refresh_charm_step(target)] - def upgrade_plan(self, target: OpenStackRelease) -> list[UpgradeStep]: - """Upgrade planning. + def upgrade_steps(self, target: OpenStackRelease) -> list[UpgradeStep]: + """Upgrade steps planning. :param target: OpenStack release as target to upgrade. :type target: OpenStackRelease - :raises HaltUpgradePlanGeneration: When the application halt the upgrade plan generation. - :return: Plan that will add upgrade as sub steps. + :return: List of upgrade steps. :rtype: list[UpgradeStep] """ - return [self._get_upgrade_charm_plan(target)] + return [self._get_upgrade_charm_step(target)] - def post_upgrade_plan(self, target: OpenStackRelease) -> list[PostUpgradeStep]: - """Post Upgrade planning. + def post_upgrade_steps(self, target: OpenStackRelease) -> list[PostUpgradeStep]: + """Post Upgrade steps planning. :param target: OpenStack release as target to upgrade. :type target: OpenStackRelease - :return: Plan that will add post upgrade as sub steps. + :return: List of post upgrade steps. :rtype: list[PostUpgradeStep] """ return [] diff --git a/cou/utils/openstack.py b/cou/utils/openstack.py index b09546e2..46766b47 100644 --- a/cou/utils/openstack.py +++ b/cou/utils/openstack.py @@ -439,12 +439,10 @@ def is_charm_supported(charm: str) -> bool: ) -def _generate_track_mapping() -> ( - tuple[ - defaultdict[tuple[str, str, str], list[str]], - defaultdict[tuple[str, str, str], list[OpenStackRelease]], - ] -): +def _generate_track_mapping() -> tuple[ + defaultdict[tuple[str, str, str], list[str]], + defaultdict[tuple[str, str, str], list[OpenStackRelease]], +]: """Generate the track mappings for the auxiliary charms. Those mappings should be updated periodically by adding new lines in the file diff --git a/pyproject.toml b/pyproject.toml index 82399639..574506fb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,6 +7,7 @@ ignore = ["C901", "D100", "D101", "D102", "D103", "W503", "W504"] exclude = ['.eggs', '.git', '.tox', '.venv', '.build', 'build', 'report', 'docs'] max-line-length = 99 max-complexity = 10 +extend-ignore = ["E203", "E704"] [tool.black] line-length = 99 diff --git a/tests/functional/tests/backup.py b/tests/functional/tests/backup.py index 6aa2b547..0863c7a5 100644 --- a/tests/functional/tests/backup.py +++ b/tests/functional/tests/backup.py @@ -1,4 +1,5 @@ """Generic setup for functional tests.""" + import logging import os import unittest diff --git a/tests/unit/steps/test_steps.py b/tests/unit/steps/test_steps.py index 35fa86f8..8b067218 100644 --- a/tests/unit/steps/test_steps.py +++ b/tests/unit/steps/test_steps.py @@ -29,8 +29,7 @@ ) -async def mock_coro(*args, **kwargs): - ... +async def mock_coro(*args, **kwargs): ... @pytest.mark.parametrize(