diff --git a/cou/commands.py b/cou/commands.py index 527ed0d9..75df43df 100644 --- a/cou/commands.py +++ b/cou/commands.py @@ -13,9 +13,12 @@ # limitations under the License. """Command line arguments parsing for 'charmed-openstack-upgrader'.""" +from __future__ import annotations + import argparse import logging from dataclasses import dataclass +from enum import Enum from typing import Any, Iterable, Optional import pkg_resources @@ -408,46 +411,31 @@ def prompt(self) -> bool: return not self.auto_approve @property - def is_hypervisors_command(self) -> bool: - """Whether if the command passed is specific to hypervisors. + def scope(self) -> UpgradeScope: + """Define which is the scope to upgrade. - :return: True if is hypervisors, false otherwise. - :rtype: bool - """ - return self.upgrade_group == HYPERVISORS + Scopes can be control-plane, data-plane or the whole cloud. - @property - def is_data_plane_command(self) -> bool: - """Whether if the command passed is data-plane related. - - :return: True if is data-plane, false otherwise. - :rtype: bool + :return: Scope to upgrade + :rtype: UpgradeScope """ - return self.is_hypervisors_command or self.upgrade_group == DATA_PLANE + return UpgradeScope(self.upgrade_group) - @property - def is_control_plane_command(self) -> bool: - """Whether if the command passed is control-plane related. - :return: True if is control-plane, false otherwise. - :rtype: bool - """ - return self.upgrade_group == CONTROL_PLANE - - @property - def is_generic_command(self) -> bool: - """Whether the command has no specific group for upgrading. +class UpgradeScope(Enum): + """Possible upgrade scopes.""" - When this happens both control-plane and data-plane should generate - plans. - E.g of generic commands: - - cou upgrade - - cou plan + CONTROL_PLANE = CONTROL_PLANE + DATA_PLANE = DATA_PLANE, HYPERVISORS + WHOLE_CLOUD = None - :return: True if upgrade_group is None, false otherwise. - :rtype: bool - """ - return self.upgrade_group is None + def __new__(cls, *values: Any) -> UpgradeScope: + obj = object.__new__(cls) + # first value is canonical value + obj._value_ = values[0] + for other_value in values[1:]: + cls._value2member_map_[other_value] = obj + return obj def parse_args(args: Any) -> CLIargs: # pylint: disable=inconsistent-return-statements diff --git a/cou/steps/plan.py b/cou/steps/plan.py index 4accdf73..aa4e07e8 100644 --- a/cou/steps/plan.py +++ b/cou/steps/plan.py @@ -15,7 +15,7 @@ """Upgrade planning utilities.""" import logging -from typing import Callable, Optional +from typing import Callable # NOTE we need to import the modules to register the charms with the register_application # decorator @@ -37,7 +37,7 @@ OpenStackSubordinateApplication, SubordinateBaseClass, ) -from cou.commands import CLIargs +from cou.commands import CLIargs, UpgradeScope from cou.exceptions import ( DataPlaneCannotUpgrade, DataPlaneMachineFilterError, @@ -116,7 +116,7 @@ def verify_data_plane_ready_to_upgrade(args: CLIargs, analysis_result: Analysis) :type analysis_result: Analysis :raises DataPlaneCannotUpgrade: When data-plane is not ready to upgrade. """ - if args.is_data_plane_command: + if args.scope is UpgradeScope.DATA_PLANE: if not analysis_result.min_os_version_data_plane: raise DataPlaneCannotUpgrade( "Cannot find data-plane apps. Is this a valid OpenStack cloud?" @@ -185,14 +185,13 @@ def verify_hypervisors_cli_input(args: CLIargs, analysis_result: Analysis) -> No :param analysis_result: Analysis result :type analysis_result: Analysis """ - if args.is_hypervisors_command: + if args.machines: verify_hypervisors_cli_machines(args.machines, analysis_result) + elif args.availability_zones: verify_hypervisors_cli_azs(args.availability_zones, analysis_result) -def verify_hypervisors_cli_machines( - cli_machines: Optional[set[str]], analysis_result: Analysis -) -> None: +def verify_hypervisors_cli_machines(cli_machines: set[str], analysis_result: Analysis) -> None: """Verify if the machines passed from the CLI are valid. :param cli_machines: Machines passed to the CLI as arguments @@ -200,8 +199,6 @@ def verify_hypervisors_cli_machines( :param analysis_result: Analysis result :type analysis_result: Analysis """ - if not cli_machines: - return verify_data_plane_membership( all_options=set(analysis_result.machines.keys()), data_plane_options=set(analysis_result.data_plane_machines.keys()), @@ -210,7 +207,7 @@ def verify_hypervisors_cli_machines( ) -def verify_hypervisors_cli_azs(cli_azs: Optional[set[str]], analysis_result: Analysis) -> None: +def verify_hypervisors_cli_azs(cli_azs: set[str], analysis_result: Analysis) -> None: """Verify if the availability zones passed from the CLI are valid. :param cli_azs: AZs passed to the CLI as arguments @@ -219,8 +216,6 @@ def verify_hypervisors_cli_azs(cli_azs: Optional[set[str]], analysis_result: Ana :type analysis_result: Analysis :raises DataPlaneMachineFilterError: When the cloud does not have availability zones. """ - if not cli_azs: - return all_azs: set[str] = { machine.az for machine in analysis_result.machines.values() if machine.az is not None } @@ -314,11 +309,11 @@ async def generate_plan(analysis_result: Analysis, args: CLIargs) -> UpgradePlan plan = generate_common_plan(target, analysis_result, args) - if args.is_generic_command or args.is_control_plane_command: + if args.scope in {UpgradeScope.CONTROL_PLANE, UpgradeScope.WHOLE_CLOUD}: plan.sub_steps.extend( _generate_control_plane_plan(target, analysis_result.apps_control_plane, args.force) ) - if args.is_generic_command or args.is_data_plane_command: + if args.scope in {UpgradeScope.DATA_PLANE, UpgradeScope.WHOLE_CLOUD}: plan.sub_steps.extend(await _generate_data_plane_plan(target, analysis_result, args)) return plan diff --git a/tests/unit/steps/test_steps_plan.py b/tests/unit/steps/test_steps_plan.py index cae9a6cd..32cc9b0d 100644 --- a/tests/unit/steps/test_steps_plan.py +++ b/tests/unit/steps/test_steps_plan.py @@ -18,6 +18,7 @@ from cou.apps.base import OpenStackApplication from cou.apps.core import Keystone from cou.apps.subordinate import OpenStackSubordinateApplication +from cou.commands import UpgradeScope from cou.exceptions import ( DataPlaneCannotUpgrade, DataPlaneMachineFilterError, @@ -149,9 +150,7 @@ async def test_generate_plan(mock_generate_data_plane, model, cli_args): """Test generation of upgrade plan.""" # TODO add data-plane apps once it's ready to properly generate the upgrade plan mock_generate_data_plane.return_value = [] - cli_args.is_generic_command = True - cli_args.is_control_plane_command = False - cli_args.is_data_plane_command = False + cli_args.scope = UpgradeScope.WHOLE_CLOUD cli_args.force = False target = OpenStackRelease("victoria") machines = {"0": MagicMock(spec_set=COUMachine)} @@ -339,7 +338,7 @@ def test_verify_highest_release_achieved(): def test_verify_data_plane_ready_to_upgrade_error( min_os_version_control_plane, min_os_version_data_plane, exp_error_msg, cli_args ): - cli_args.is_data_plane_command = True + cli_args.scope = UpgradeScope.DATA_PLANE mock_analysis_result = MagicMock(spec=Analysis)() mock_analysis_result.current_cloud_series = "focal" mock_analysis_result.min_os_version_control_plane = min_os_version_control_plane @@ -348,16 +347,18 @@ def test_verify_data_plane_ready_to_upgrade_error( cou_plan.verify_data_plane_ready_to_upgrade(cli_args, mock_analysis_result) -@pytest.mark.parametrize("is_data_plane_command", [True, False]) +@pytest.mark.parametrize( + "scope", [UpgradeScope.DATA_PLANE, UpgradeScope.CONTROL_PLANE, UpgradeScope.WHOLE_CLOUD] +) @patch("cou.steps.plan.is_control_plane_upgraded") def test_verify_data_plane_ready_to_upgrade_data_plane_cmd( - mock_control_plane_upgraded, cli_args, is_data_plane_command + mock_control_plane_upgraded, cli_args, scope ): mock_analysis_result = MagicMock(spec=Analysis)() mock_analysis_result.min_os_version_data_plane = OpenStackRelease("ussuri") - cli_args.is_data_plane_command = is_data_plane_command + cli_args.scope = scope cou_plan.verify_data_plane_ready_to_upgrade(cli_args, mock_analysis_result) - if is_data_plane_command: + if scope is UpgradeScope.DATA_PLANE: mock_control_plane_upgraded.assert_called_once_with(mock_analysis_result) else: mock_control_plane_upgraded.assert_not_called() @@ -501,37 +502,28 @@ def test_create_upgrade_plan_failed(force): cou_plan.create_upgrade_group([app], "victoria", "test", force, lambda *_: True) -@pytest.mark.parametrize("is_hypervisors_command", [True, False]) +@pytest.mark.parametrize("machines, azs", [(None, None), ({"1"}, None), (None, {"zone-1"})]) @patch("cou.steps.plan.verify_hypervisors_cli_azs") @patch("cou.steps.plan.verify_hypervisors_cli_machines") -def test_verify_hypervisors_cli_input( - mock_cli_machines, - mock_cli_azs, - cli_args, - is_hypervisors_command, -): - cli_args.is_hypervisors_command = is_hypervisors_command - cli_args.machines = None - cli_args.availability_zones = None +def test_verify_hypervisors_cli_input(mock_cli_machines, mock_cli_azs, cli_args, machines, azs): + cli_args.machines = machines + cli_args.availability_zones = azs analysis_result = MagicMock(spec_set=Analysis)() assert cou_plan.verify_hypervisors_cli_input(cli_args, analysis_result) is None - if is_hypervisors_command: + if machines: mock_cli_machines.assert_called_once_with(cli_args.machines, analysis_result) + mock_cli_azs.assert_not_called() + elif azs: + mock_cli_machines.assert_not_called() mock_cli_azs.assert_called_once_with(cli_args.availability_zones, analysis_result) else: mock_cli_machines.assert_not_called() mock_cli_azs.assert_not_called() -def test_verify_hypervisors_cli_machines_no_input(): - cli_machines = None - analysis_result = MagicMock(spec_set=Analysis)() - assert cou_plan.verify_hypervisors_cli_machines(cli_machines, analysis_result) is None - - @pytest.mark.parametrize( "cli_machines, exp_error_msg", [ @@ -551,12 +543,6 @@ def test_verify_hypervisors_cli_machines_raise(cli_machines, exp_error_msg): cou_plan.verify_hypervisors_cli_machines(cli_machines, analysis_result) -def test_verify_hypervisors_cli_azs_no_input(): - cli_azs = None - analysis_result = MagicMock(spec_set=Analysis)() - assert cou_plan.verify_hypervisors_cli_azs(cli_azs, analysis_result) is None - - @pytest.mark.parametrize( "cli_azs, exp_error_msg", [ @@ -753,8 +739,8 @@ def test_generate_control_plane_plan(mock_create_upgrade_group): @pytest.mark.asyncio @pytest.mark.parametrize( - "is_control_plane_command, is_data_plane_command, is_generic_command", - [(True, False, False), (False, False, True), (False, True, False)], + "scope", + [(UpgradeScope.CONTROL_PLANE, UpgradeScope.DATA_PLANE, UpgradeScope.WHOLE_CLOUD)], ) @patch("cou.steps.plan.generate_common_plan") @patch("cou.steps.plan.determine_upgrade_target") @@ -768,13 +754,9 @@ async def test_generate_plan_control_and_data_plane( mock_determine_upgrade_target, mock_generate_common_plan, cli_args, - is_control_plane_command, - is_data_plane_command, - is_generic_command, + scope, ): - cli_args.is_control_plane_command = is_control_plane_command - cli_args.is_data_plane_command = is_data_plane_command - cli_args.is_generic_command = is_generic_command + cli_args.scope = scope mock_analysis_result = MagicMock(spec=Analysis)() @@ -784,14 +766,14 @@ async def test_generate_plan_control_and_data_plane( mock_determine_upgrade_target.assert_called_once() mock_generate_common_plan.assert_called_once() - if is_generic_command: + if scope is UpgradeScope.WHOLE_CLOUD: mock_control_plane.assert_called_once() mock_data_plane.assert_called_once() - if is_control_plane_command: + if scope is UpgradeScope.CONTROL_PLANE: mock_control_plane.assert_called_once() mock_data_plane.assert_not_called() - if is_data_plane_command: + if scope is UpgradeScope.DATA_PLANE: mock_control_plane.assert_not_called() mock_data_plane.assert_called_once() diff --git a/tests/unit/test_commands.py b/tests/unit/test_commands.py index 5ac4db1d..cdf864de 100644 --- a/tests/unit/test_commands.py +++ b/tests/unit/test_commands.py @@ -18,49 +18,27 @@ import pytest from cou import commands -from cou.commands import CONTROL_PLANE, DATA_PLANE, HYPERVISORS, CLIargs - - -@pytest.mark.parametrize("auto_approve, expected_result", [(True, False), (False, True)]) -def test_CLIargs_prompt(auto_approve, expected_result): - args = CLIargs(command="foo", auto_approve=auto_approve) - assert args.prompt is expected_result +from cou.commands import CONTROL_PLANE, DATA_PLANE, HYPERVISORS, CLIargs, UpgradeScope @pytest.mark.parametrize( "upgrade_group, expected_result", - [(CONTROL_PLANE, True), (DATA_PLANE, False), (None, False), (HYPERVISORS, False)], -) -def test_CLIargs_is_control_plane_command(upgrade_group, expected_result): - args = CLIargs(command="foo", upgrade_group=upgrade_group) - assert args.is_control_plane_command is expected_result - - -@pytest.mark.parametrize( - "upgrade_group, expected_result", - [(CONTROL_PLANE, False), (DATA_PLANE, False), (HYPERVISORS, True), ("foo", False)], -) -def test_CLIargs_is_hypervisors_command(upgrade_group, expected_result): - args = CLIargs(command="foo", upgrade_group=upgrade_group) - assert args.is_hypervisors_command is expected_result - - -@pytest.mark.parametrize( - "upgrade_group, expected_result", - [(CONTROL_PLANE, False), (DATA_PLANE, False), (HYPERVISORS, False), (None, True)], + [ + (CONTROL_PLANE, UpgradeScope.CONTROL_PLANE), + (DATA_PLANE, UpgradeScope.DATA_PLANE), + (HYPERVISORS, UpgradeScope.DATA_PLANE), + (None, UpgradeScope.WHOLE_CLOUD), + ], ) -def test_CLIargs_is_generic_command(upgrade_group, expected_result): - args = CLIargs(command="foo", upgrade_group=upgrade_group) - assert args.is_generic_command is expected_result +def test_upgrade_scope(upgrade_group, expected_result): + args = CLIargs(command="plan/upgrade", upgrade_group=upgrade_group) + assert args.scope is expected_result -@pytest.mark.parametrize( - "upgrade_group, expected_result", - [(CONTROL_PLANE, False), (DATA_PLANE, True), (HYPERVISORS, True), ("foo", False)], -) -def test_CLIargs_is_data_plane_command(upgrade_group, expected_result): - args = CLIargs(command="foo", upgrade_group=upgrade_group) - assert args.is_data_plane_command is expected_result +@pytest.mark.parametrize("auto_approve, expected_result", [(True, False), (False, True)]) +def test_CLIargs_prompt(auto_approve, expected_result): + args = CLIargs(command="foo", auto_approve=auto_approve) + assert args.prompt is expected_result @pytest.mark.parametrize(