- added UpgradeScope enum class

- removed is_hypervisors_command, is_data_plane_command,
  is_control_plane_command and is_generic_command properties from
  CLIArgs

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

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,23 +185,20 @@ 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
     :type cli_machines: set[str]
     :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

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()

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(