From 1b137f991bafe8f6cc59947a453692964c41263e Mon Sep 17 00:00:00 2001 From: Gabriel Cocenza Date: Tue, 6 Feb 2024 13:51:39 -0300 Subject: [PATCH] Filter hypervisors. (#229) - do the nova-compute instance-count to find empty hypervisors - integrate the force flag to include non-empty hypervisors --- cou/apps/base.py | 11 +- cou/steps/plan.py | 53 ++++++ cou/utils/app_utils.py | 25 --- cou/utils/nova_compute.py | 62 +++++++ tests/unit/apps/test_core.py | 9 + tests/unit/conftest.py | 19 ++ tests/unit/steps/test_steps_plan.py | 242 ++++++++++++++++---------- tests/unit/utils/test_app_utils.py | 33 ---- tests/unit/utils/test_nova_compute.py | 86 +++++++++ 9 files changed, 392 insertions(+), 148 deletions(-) create mode 100644 cou/utils/nova_compute.py create mode 100644 tests/unit/utils/test_nova_compute.py diff --git a/cou/apps/base.py b/cou/apps/base.py index a9c10f02..08cc4fca 100644 --- a/cou/apps/base.py +++ b/cou/apps/base.py @@ -49,7 +49,7 @@ DEFAULT_WAITING_TIMEOUT = 5 * 60 # 5 min -@dataclass +@dataclass(frozen=True) class ApplicationUnit: """Representation of a single unit of application.""" @@ -58,6 +58,14 @@ class ApplicationUnit: machine: Machine workload_version: str = "" + def __repr__(self) -> str: + """Representation of the application unit. + + :return: Representation of the application unit + :rtype: str + """ + return f"Unit[{self.name}]-Machine[{self.machine.machine_id}]" + @dataclass class OpenStackApplication: @@ -337,6 +345,7 @@ def current_os_release(self) -> OpenStackRelease: f"'{openstack_release.codename}': {units}" for openstack_release, units in os_versions.items() ] + raise MismatchedOpenStackVersions( f"Units of application {self.name} are running mismatched OpenStack versions: " f"{', '.join(mismatched_repr)}. This is not currently handled." diff --git a/cou/steps/plan.py b/cou/steps/plan.py index a6771d4a..650c50d6 100644 --- a/cou/steps/plan.py +++ b/cou/steps/plan.py @@ -33,6 +33,7 @@ from cou.apps.base import OpenStackApplication from cou.apps.channel_based import OpenStackChannelBasedApplication # noqa: F401 from cou.apps.core import Keystone, Octavia # noqa: F401 +from cou.apps.machine import Machine from cou.apps.subordinate import ( # noqa: F401 OpenStackSubordinateApplication, SubordinateBaseClass, @@ -50,6 +51,7 @@ from cou.steps.analyze import Analysis from cou.steps.backup import backup from cou.utils.juju_utils import DEFAULT_TIMEOUT +from cou.utils.nova_compute import get_empty_hypervisors from cou.utils.openstack import LTS_TO_OS_RELEASE, OpenStackRelease logger = logging.getLogger(__name__) @@ -327,6 +329,8 @@ async def generate_plan(analysis_result: Analysis, args: CLIargs) -> UpgradePlan :rtype: UpgradePlan """ pre_plan_sanity_checks(args, analysis_result) + hypervisors = await filter_hypervisors_machines(args, analysis_result) + logger.info("Hypervisors selected: %s", hypervisors) target = determine_upgrade_target(analysis_result) plan = UpgradePlan( @@ -375,6 +379,55 @@ async def generate_plan(analysis_result: Analysis, args: CLIargs) -> UpgradePlan return plan +async def filter_hypervisors_machines(args: CLIargs, analysis_result: Analysis) -> list[Machine]: + """Filter the hypervisors to generate plan and upgrade. + + :param args: CLI arguments + :type args: CLIargs + :param analysis_result: Analysis result + :type analysis_result: Analysis + :return: hypervisors filtered to generate plan and upgrade. + :rtype: list[Machine] + """ + hypervisors_machines = await _get_upgradable_hypervisors_machines(args.force, analysis_result) + + if cli_machines := args.machines: + return [machine for machine in hypervisors_machines if machine.machine_id in cli_machines] + + if cli_hostnames := args.hostnames: + return [machine for machine in hypervisors_machines if machine.hostname in cli_hostnames] + + if cli_azs := args.availability_zones: + return [machine for machine in hypervisors_machines if machine.az in cli_azs] + + return hypervisors_machines + + +async def _get_upgradable_hypervisors_machines( + cli_force: bool, analysis_result: Analysis +) -> list[Machine]: + """Get the hypervisors that are possible to upgrade. + + :param cli_force: If force is used, it gets all hypervisors, otherwise just the empty ones + :type cli_force: bool + :param analysis_result: Analysis result + :type analysis_result: Analysis + :return: List of nova-compute units to upgrade + :rtype: list[Machine] + """ + nova_compute_units = [ + unit + for app in analysis_result.apps_data_plane + for unit in app.units + if app.charm == "nova-compute" + ] + + if cli_force: + return [unit.machine for unit in nova_compute_units] + + return await get_empty_hypervisors(nova_compute_units, analysis_result.model) + + async def create_upgrade_group( apps: list[OpenStackApplication], target: OpenStackRelease, diff --git a/cou/utils/app_utils.py b/cou/utils/app_utils.py index edd6ac70..8d2505c5 100644 --- a/cou/utils/app_utils.py +++ b/cou/utils/app_utils.py @@ -50,31 +50,6 @@ async def upgrade_packages( await model.run_on_unit(unit_name=unit, command=command, timeout=600) -async def get_instance_count(unit: str, model: COUModel) -> int: - """Get instance count on a nova-compute unit. - - :param unit: Name of the nova-compute unit where the action runs on. - :type unit: str - :param model: COUModel object - :type model: COUModel - :return: Instance count of the nova-compute unit - :rtype: int - :raises ValueError: When the action result is not valid. - """ - action_name = "instance-count" - action = await model.run_action(unit_name=unit, action_name=action_name) - - if ( - instance_count := action.results.get("instance-count", "").strip() - ) and instance_count.isdigit(): - return int(instance_count) - - raise ValueError( - f"No valid instance count value found in the result of {action_name} action " - f"running on '{unit}': {action.results}" - ) - - async def set_require_osd_release_option(unit: str, model: COUModel) -> None: """Check and set the correct value for require-osd-release on a ceph-mon unit. diff --git a/cou/utils/nova_compute.py b/cou/utils/nova_compute.py new file mode 100644 index 00000000..62afb29a --- /dev/null +++ b/cou/utils/nova_compute.py @@ -0,0 +1,62 @@ +# Copyright 2023 Canonical Limited +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Nova Compute utilities.""" + +import asyncio + +from cou.apps.base import ApplicationUnit +from cou.apps.machine import Machine +from cou.utils.juju_utils import COUModel + + +async def get_empty_hypervisors(units: list[ApplicationUnit], model: COUModel) -> list[Machine]: + """Get the empty hypervisors in the model. + + :param units: all nova-compute units. + :type units: list[ApplicationUnit] + :param model: COUModel object + :type model: COUModel + :return: List with just the empty hypervisors machines. + :rtype: list[Machine] + """ + tasks = [get_instance_count(unit.name, model) for unit in units] + instances = await asyncio.gather(*tasks) + units_instances = zip(units, instances) + return [unit.machine for unit, instances in units_instances if instances == 0] + + +async def get_instance_count(unit: str, model: COUModel) -> int: + """Get instance count on a nova-compute unit. + + :param unit: Name of the nova-compute unit where the action runs on. + :type unit: str + :param model: COUModel object + :type model: COUModel + :return: Instance count of the nova-compute unit + :rtype: int + :raises ValueError: When the action result is not valid. + """ + action_name = "instance-count" + action = await model.run_action(unit_name=unit, action_name=action_name) + + if ( + instance_count := action.results.get("instance-count", "").strip() + ) and instance_count.isdigit(): + return int(instance_count) + + raise ValueError( + f"No valid instance count value found in the result of {action_name} action " + f"running on '{unit}': {action.results}" + ) diff --git a/tests/unit/apps/test_core.py b/tests/unit/apps/test_core.py index ba473821..e32c90cd 100644 --- a/tests/unit/apps/test_core.py +++ b/tests/unit/apps/test_core.py @@ -15,7 +15,9 @@ import pytest +from cou.apps.base import ApplicationUnit from cou.apps.core import Keystone +from cou.apps.machine import Machine from cou.exceptions import ( ApplicationError, HaltUpgradePlanGeneration, @@ -32,6 +34,13 @@ from tests.unit.apps.utils import add_steps +def test_repr_ApplicationUnit(): + app_unit = ApplicationUnit( + "keystone/0", OpenStackRelease("ussuri"), Machine("0", "juju-cef38-0", "zone-1"), "17.0.1" + ) + assert repr(app_unit) == "Unit[keystone/0]-Machine[0]" + + def test_application_eq(status, config, model, apps_machines): """Name of the app is used as comparison between Applications objects.""" status_keystone_1 = status["keystone_focal_ussuri"] diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 28139e5d..f89d3634 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -28,6 +28,7 @@ from cou.apps.machine import Machine from cou.apps.subordinate import OpenStackSubordinateApplication from cou.commands import CLIargs +from cou.steps.analyze import Analysis from cou.utils.openstack import OpenStackRelease STANDARD_AZS = ["zone-1", "zone-2", "zone-3"] @@ -592,6 +593,16 @@ def model(config, apps_machines): return model +@pytest.fixture +def analysis_result(model, apps): + """Generate a simple analysis result to be used on unit-tests.""" + return Analysis( + model=model, + apps_control_plane=[apps["keystone_focal_ussuri"]], + apps_data_plane=[apps["nova_focal_ussuri"]], + ) + + @pytest.fixture def apps(status, config, model, apps_machines): keystone_focal_ussuri_status = status["keystone_focal_ussuri"] @@ -682,3 +693,11 @@ def cli_args() -> MagicMock: """ # spec_set needs an instantiated class to be strict with the fields. return MagicMock(spec_set=CLIargs(command="plan"))() + + +def generate_mock_machine(machine_id, hostname, az): + mock_machine = MagicMock(spec_set=Machine(machine_id, hostname, az)) + mock_machine.machine_id = machine_id + mock_machine.hostname = hostname + mock_machine.az = az + return mock_machine diff --git a/tests/unit/steps/test_steps_plan.py b/tests/unit/steps/test_steps_plan.py index 1f00fc7b..f9dc502d 100644 --- a/tests/unit/steps/test_steps_plan.py +++ b/tests/unit/steps/test_steps_plan.py @@ -31,27 +31,13 @@ UpgradePlan, UpgradeStep, ) +from cou.steps import plan as cou_plan from cou.steps.analyze import Analysis from cou.steps.backup import backup -from cou.steps.plan import ( - create_upgrade_group, - determine_upgrade_target, - generate_plan, - is_control_plane_upgraded, - manually_upgrade_data_plane, - pre_plan_sanity_checks, - verify_data_plane_cli_azs, - verify_data_plane_cli_hostnames, - verify_data_plane_cli_input, - verify_data_plane_cli_machines, - verify_data_plane_ready_to_upgrade, - verify_highest_release_achieved, - verify_supported_series, -) from cou.utils import app_utils from cou.utils.openstack import OpenStackRelease from tests.unit.apps.utils import add_steps -from tests.unit.conftest import KEYSTONE_MACHINES, NOVA_MACHINES +from tests.unit.conftest import KEYSTONE_MACHINES, NOVA_MACHINES, generate_mock_machine def generate_expected_upgrade_plan_principal(app, target, model): @@ -155,7 +141,7 @@ async def test_generate_plan(apps, model, cli_args): apps_data_plane=[], ) - upgrade_plan = await generate_plan(analysis_result, cli_args) + upgrade_plan = await cou_plan.generate_plan(analysis_result, cli_args) expected_plan = UpgradePlan("Upgrade cloud from 'ussuri' to 'victoria'") expected_plan.add_step( @@ -216,7 +202,7 @@ def test_pre_plan_sanity_checks( mock_analysis_result.current_cloud_os_release = OpenStackRelease("ussuri") mock_analysis_result.current_cloud_series = "focal" cli_args.is_data_plane_command = is_data_plane_command - pre_plan_sanity_checks(cli_args, mock_analysis_result) + cou_plan.pre_plan_sanity_checks(cli_args, mock_analysis_result) mock_verify_highest_release_achieved.assert_called_once() mock_verify_supported_series.assert_called_once() if expected_call: @@ -249,7 +235,7 @@ def test_verify_supported_series(current_os_release, current_series, exp_error_m with pytest.raises(OutOfSupportRange, match=exp_error_msg): mock_analysis_result.current_cloud_os_release = current_os_release mock_analysis_result.current_cloud_series = current_series - verify_supported_series(mock_analysis_result) + cou_plan.verify_supported_series(mock_analysis_result) def test_verify_highest_release_achieved(): @@ -261,7 +247,7 @@ def test_verify_highest_release_achieved(): "Newer OpenStack releases may be available after upgrading to a later Ubuntu series." ) with pytest.raises(HighestReleaseAchieved, match=exp_error_msg): - verify_highest_release_achieved(mock_analysis_result) + cou_plan.verify_highest_release_achieved(mock_analysis_result) @pytest.mark.parametrize( @@ -288,7 +274,7 @@ def test_verify_data_plane_ready_to_upgrade_error( mock_analysis_result.min_os_version_control_plane = min_os_version_control_plane mock_analysis_result.min_os_version_data_plane = min_os_version_data_plane with pytest.raises(DataPlaneCannotUpgrade, match=exp_error_msg): - verify_data_plane_ready_to_upgrade(mock_analysis_result) + cou_plan.verify_data_plane_ready_to_upgrade(mock_analysis_result) @pytest.mark.parametrize( @@ -306,7 +292,7 @@ def test_is_control_plane_upgraded( mock_analysis_result = MagicMock(spec=Analysis)() mock_analysis_result.min_os_version_control_plane = min_os_version_control_plane mock_analysis_result.min_os_version_data_plane = min_os_version_data_plane - assert is_control_plane_upgraded(mock_analysis_result) is expected_result + assert cou_plan.is_control_plane_upgraded(mock_analysis_result) is expected_result @pytest.mark.parametrize( @@ -321,7 +307,7 @@ def test_determine_upgrade_target(current_os_release, current_series, next_relea mock_analysis_result.current_cloud_os_release = current_os_release mock_analysis_result.current_cloud_series = current_series - target = determine_upgrade_target(mock_analysis_result) + target = cou_plan.determine_upgrade_target(mock_analysis_result) assert target == next_release @@ -350,7 +336,7 @@ def test_determine_upgrade_target_current_os_and_series( mock_analysis_result = MagicMock(spec=Analysis)() mock_analysis_result.current_cloud_series = current_series mock_analysis_result.current_cloud_os_release = current_os_release - determine_upgrade_target(mock_analysis_result) + cou_plan.determine_upgrade_target(mock_analysis_result) def test_determine_upgrade_target_no_next_release(): @@ -368,7 +354,7 @@ def test_determine_upgrade_target_no_next_release(): "ussuri" ) # instantiate OpenStackRelease with any valid codename mock_analysis_result.current_cloud_os_release = current_os_release - determine_upgrade_target(mock_analysis_result) + cou_plan.determine_upgrade_target(mock_analysis_result) def test_determine_upgrade_target_out_support_range(): @@ -382,7 +368,7 @@ def test_determine_upgrade_target_out_support_range(): "Ubuntu series 'focal': ussuri, victoria, wallaby, xena, yoga." ) with pytest.raises(OutOfSupportRange, match=exp_error_msg): - determine_upgrade_target(mock_analysis_result) + cou_plan.determine_upgrade_target(mock_analysis_result) @pytest.mark.asyncio @@ -393,7 +379,7 @@ async def test_create_upgrade_plan(): target = OpenStackRelease("victoria") description = "test" - plan = await create_upgrade_group([app], target, description, lambda *_: True) + plan = await cou_plan.create_upgrade_group([app], target, description, lambda *_: True) assert plan.description == description assert plan.parallel is False @@ -412,7 +398,7 @@ async def test_create_upgrade_plan_HaltUpgradePlanGeneration(): target = OpenStackRelease("victoria") description = "test" - plan = await create_upgrade_group([app], target, description, lambda *_: True) + plan = await cou_plan.create_upgrade_group([app], target, description, lambda *_: True) assert len(plan.sub_steps) == 0 app.generate_upgrade_plan.assert_called_once_with(target) @@ -426,7 +412,7 @@ async def test_create_upgrade_plan_failed(): app.generate_upgrade_plan.side_effect = Exception("test") with pytest.raises(Exception, match="test"): - await create_upgrade_group([app], "victoria", "test", lambda *_: True) + await cou_plan.create_upgrade_group([app], "victoria", "test", lambda *_: True) @patch("builtins.print") @@ -436,20 +422,15 @@ def test_plan_print_warn_manually_upgrade(mock_print, model, apps): apps_control_plane=[apps["keystone_focal_wallaby"]], apps_data_plane=[apps["nova_focal_ussuri"]], ) - manually_upgrade_data_plane(result) + cou_plan.manually_upgrade_data_plane(result) mock_print.assert_called_with( "WARNING: Please upgrade manually the data plane apps: nova-compute" ) @patch("builtins.print") -def test_analysis_not_print_warn_manually_upgrade(mock_print, model, apps): - result = Analysis( - model=model, - apps_control_plane=[apps["keystone_focal_ussuri"]], - apps_data_plane=[apps["nova_focal_ussuri"]], - ) - manually_upgrade_data_plane(result) +def test_analysis_not_print_warn_manually_upgrade(mock_print, analysis_result): + cou_plan.manually_upgrade_data_plane(analysis_result) mock_print.assert_not_called() @@ -460,20 +441,14 @@ def test_verify_data_plane_cli_no_input( mock_verify_machines, mock_verify_hostnames, mock_verify_azs, - model, - apps, + analysis_result, cli_args, ): - result = Analysis( - model=model, - apps_control_plane=[apps["keystone_focal_ussuri"]], - apps_data_plane=[apps["nova_focal_ussuri"]], - ) cli_args.machines = None cli_args.hostnames = None cli_args.availability_zones = None - assert verify_data_plane_cli_input(cli_args, result) is None + assert cou_plan.verify_data_plane_cli_input(cli_args, analysis_result) is None mock_verify_machines.assert_not_called() mock_verify_hostnames.assert_not_called() @@ -495,20 +470,14 @@ def test_verify_data_plane_cli_input_machines( mock_verify_hostnames, mock_verify_azs, cli_machines, - model, - apps, + analysis_result, cli_args, ): - result = Analysis( - model=model, - apps_control_plane=[apps["keystone_focal_ussuri"]], - apps_data_plane=[apps["nova_focal_ussuri"]], - ) cli_args.machines = cli_machines cli_args.hostnames = None cli_args.availability_zones = None - assert verify_data_plane_cli_input(cli_args, result) is None + assert cou_plan.verify_data_plane_cli_input(cli_args, analysis_result) is None mock_verify_hostnames.assert_not_called() mock_verify_azs.assert_not_called() @@ -524,21 +493,16 @@ def test_verify_data_plane_cli_input_hostnames( mock_verify_machines, mock_verify_azs, nova_unit, - model, + analysis_result, apps, cli_args, ): nova_machine = list(apps["nova_focal_ussuri"].machines.values())[nova_unit] - result = Analysis( - model=model, - apps_control_plane=[apps["keystone_focal_ussuri"]], - apps_data_plane=[apps["nova_focal_ussuri"]], - ) cli_args.machines = None cli_args.hostnames = {nova_machine.hostname} cli_args.availability_zones = None - assert verify_data_plane_cli_input(cli_args, result) is None + assert cou_plan.verify_data_plane_cli_input(cli_args, analysis_result) is None mock_verify_machines.assert_not_called() mock_verify_azs.assert_not_called() @@ -554,21 +518,16 @@ def test_verify_data_plane_cli_input_azs( mock_verify_machines, mock_verify_hostnames, nova_unit, - model, + analysis_result, apps, cli_args, ): nova_machine = list(apps["nova_focal_ussuri"].machines.values())[nova_unit] - result = Analysis( - model=model, - apps_control_plane=[apps["keystone_focal_ussuri"]], - apps_data_plane=[apps["nova_focal_ussuri"]], - ) cli_args.machines = None cli_args.hostnames = None cli_args.availability_zones = {nova_machine.az} - assert verify_data_plane_cli_input(cli_args, result) is None + assert cou_plan.verify_data_plane_cli_input(cli_args, analysis_result) is None mock_verify_machines.assert_not_called() mock_verify_hostnames.assert_not_called() @@ -581,14 +540,9 @@ def test_verify_data_plane_cli_input_azs( ({"5/lxd/18"}, r"Machine.*don't exist."), ], ) -def test_verify_data_plane_cli_machines_raise(apps, model, machine, exp_error_msg): - result = Analysis( - model=model, - apps_control_plane=[apps["keystone_focal_ussuri"]], - apps_data_plane=[apps["nova_focal_ussuri"]], - ) +def test_verify_data_plane_cli_machines_raise(analysis_result, machine, exp_error_msg): with pytest.raises(DataPlaneMachineFilterError, match=exp_error_msg): - verify_data_plane_cli_machines(machine, result) + cou_plan.verify_data_plane_cli_machines(machine, analysis_result) @pytest.mark.parametrize( @@ -598,15 +552,10 @@ def test_verify_data_plane_cli_machines_raise(apps, model, machine, exp_error_ms ("cinder", r"Hostname.*don't exist."), # cinder is not on the Analysis ], ) -def test_verify_data_plane_cli_hostname_raise(apps, model, app, exp_error_msg): - result = Analysis( - model=model, - apps_control_plane=[apps["keystone_focal_ussuri"]], - apps_data_plane=[apps["nova_focal_ussuri"]], - ) +def test_verify_data_plane_cli_hostname_raise(apps, analysis_result, app, exp_error_msg): with pytest.raises(DataPlaneMachineFilterError, match=exp_error_msg): machine = list(apps[f"{app}_focal_ussuri"].machines.values())[0] - verify_data_plane_cli_hostnames({machine.hostname}, result) + cou_plan.verify_data_plane_cli_hostnames({machine.hostname}, analysis_result) @pytest.mark.parametrize( @@ -616,14 +565,9 @@ def test_verify_data_plane_cli_hostname_raise(apps, model, app, exp_error_msg): ({"zone-1", "zone-foo"}, r"Availability Zone.*don't exist."), ], ) -def test_verify_data_plane_cli_azs_raise_dont_exist(apps, model, azs, exp_error_msg): - result = Analysis( - model=model, - apps_control_plane=[apps["keystone_focal_ussuri"]], - apps_data_plane=[apps["nova_focal_ussuri"]], - ) +def test_verify_data_plane_cli_azs_raise_dont_exist(analysis_result, azs, exp_error_msg): with pytest.raises(DataPlaneMachineFilterError, match=exp_error_msg): - verify_data_plane_cli_azs(azs, result) + cou_plan.verify_data_plane_cli_azs(azs, analysis_result) def test_verify_data_plane_cli_azs_raise_cannot_find(): @@ -638,4 +582,124 @@ def test_verify_data_plane_cli_azs_raise_cannot_find(): mock_analyze.machines = {"1": mock_machine} mock_analyze.data_plane_machines = {"1": mock_machine} with pytest.raises(DataPlaneMachineFilterError, match=exp_error_msg): - verify_data_plane_cli_azs({"zone-1"}, mock_analyze) + cou_plan.verify_data_plane_cli_azs({"zone-1"}, mock_analyze) + + +@pytest.mark.parametrize( + "force, cli_machines, cli_hostnames, cli_azs, expected_machines", + [ + # machines input + (False, {"0", "1", "2"}, None, None, {"0", "1"}), + (False, {"2"}, None, None, set()), + (False, {"0", "1"}, None, None, {"0", "1"}), + (False, {"0"}, None, None, {"0"}), + (True, {"0", "1", "2"}, None, None, {"0", "1", "2"}), + (True, {"2"}, None, None, {"2"}), + (True, {"0"}, None, None, {"0"}), + # hostnames input + (False, None, {"juju-c307f8-0", "juju-c307f8-1", "juju-c307f8-2"}, None, {"0", "1"}), + (False, None, {"juju-c307f8-2"}, None, set()), + (False, None, {"juju-c307f8-0", "juju-c307f8-1"}, None, {"0", "1"}), + (False, None, {"juju-c307f8-0"}, None, {"0"}), + ( + True, + None, + {"juju-c307f8-0", "juju-c307f8-1", "juju-c307f8-2"}, + None, + {"0", "1", "2"}, + ), + (True, None, {"juju-c307f8-2"}, None, {"2"}), + (True, None, {"juju-c307f8-0"}, None, {"0"}), + # az input + (False, None, None, {"zone-1", "zone-2", "zone-3"}, {"0", "1"}), + (False, None, None, {"zone-3"}, set()), + (False, None, None, {"zone-1", "zone-2"}, {"0", "1"}), + (False, None, None, {"zone-1"}, {"0"}), + (True, None, None, {"zone-1", "zone-2", "zone-3"}, {"0", "1", "2"}), + (True, None, None, {"zone-3"}, {"2"}), + (True, None, None, {"zone-1"}, {"0"}), + # no input + (False, None, None, None, {"0", "1"}), + (True, None, None, None, {"0", "1", "2"}), + ], +) +@pytest.mark.asyncio +@patch("cou.steps.plan._get_upgradable_hypervisors_machines") +async def test_filter_hypervisors_machines( + mock_hypervisors_machines, + force, + cli_machines, + cli_hostnames, + cli_azs, + expected_machines, + cli_args, +): + + empty_hypervisors_machines = { + generate_mock_machine( + str(machine_id), f"juju-c307f8-{machine_id}", f"zone-{machine_id + 1}" + ) + for machine_id in range(2) + } + # assuming that machine-2 has some VMs running + non_empty_hypervisor_machine = generate_mock_machine("2", "juju-c307f8-2", "zone-3") + + upgradable_hypervisors = empty_hypervisors_machines + if force: + upgradable_hypervisors.add(non_empty_hypervisor_machine) + + mock_hypervisors_machines.return_value = upgradable_hypervisors + + cli_args.machines = cli_machines + cli_args.hostnames = cli_hostnames + cli_args.availability_zones = cli_azs + cli_args.force = force + + machines = await cou_plan.filter_hypervisors_machines(cli_args, MagicMock()) + + assert {machine.machine_id for machine in machines} == expected_machines + + +@pytest.mark.parametrize( + "cli_force, empty_hypervisors, expected_result", + [ + (True, {0, 1, 2}, {"0", "1", "2"}), + (True, {0, 1}, {"0", "1", "2"}), + (True, {0, 2}, {"0", "1", "2"}), + (True, {1, 2}, {"0", "1", "2"}), + (True, {0}, {"0", "1", "2"}), + (True, {1}, {"0", "1", "2"}), + (True, {2}, {"0", "1", "2"}), + (True, set(), {"0", "1", "2"}), + (False, {0, 1, 2}, {"0", "1", "2"}), + (False, {0, 1}, {"0", "1"}), + (False, {0, 2}, {"0", "2"}), + (False, {1, 2}, {"1", "2"}), + (False, {0}, {"0"}), + (False, {1}, {"1"}), + (False, {2}, {"2"}), + (False, set(), set()), + ], +) +@pytest.mark.asyncio +@patch("cou.steps.plan.get_empty_hypervisors") +async def test_get_upgradable_hypervisors_machines( + mock_empty_hypervisors, + cli_force, + empty_hypervisors, + expected_result, + analysis_result, +): + mock_empty_hypervisors.return_value = { + generate_mock_machine( + str(machine_id), f"juju-c307f8-{machine_id}", f"zone-{machine_id + 1}" + ) + for machine_id in empty_hypervisors + } + hypervisors_possible_to_upgrade = await cou_plan._get_upgradable_hypervisors_machines( + cli_force, analysis_result + ) + + assert { + hypervisor.machine_id for hypervisor in hypervisors_possible_to_upgrade + } == expected_result diff --git a/tests/unit/utils/test_app_utils.py b/tests/unit/utils/test_app_utils.py index 6a7e365d..f40171cd 100644 --- a/tests/unit/utils/test_app_utils.py +++ b/tests/unit/utils/test_app_utils.py @@ -15,7 +15,6 @@ from unittest.mock import AsyncMock, call, patch import pytest -from juju.action import Action from cou.exceptions import RunUpgradeError from cou.utils import app_utils @@ -79,38 +78,6 @@ async def test_application_upgrade_packages_with_hold(model): model.run_on_unit.assert_has_awaits(expected_calls) -@pytest.mark.asyncio -async def test_get_instance_count(model): - expected_count = 1 - model.run_action.return_value = mocked_action = AsyncMock(spec_set=Action).return_value - mocked_action.results = {"Code": "0", "instance-count": str(expected_count)} - - actual_count = await app_utils.get_instance_count(unit="nova-compute/0", model=model) - - model.run_action.assert_called_once_with( - unit_name="nova-compute/0", - action_name="instance-count", - ) - assert actual_count == expected_count - - -@pytest.mark.asyncio -@pytest.mark.parametrize( - "result_key, value", - [ - ("not_valid", "1"), # invalid key - ("instance-count", "not_valid"), # invalid value - ("not_valid", "not_valid"), # invalid key and value - ], -) -async def test_get_instance_count_invalid_result(model, result_key, value): - model.run_action.return_value = mocked_action = AsyncMock(spec_set=Action).return_value - mocked_action.results = {"Code": "0", result_key: value} - - with pytest.raises(ValueError): - await app_utils.get_instance_count(unit="nova-compute/0", model=model) - - @pytest.mark.asyncio @pytest.mark.parametrize( "current_required_osd_release, current_osd_release", diff --git a/tests/unit/utils/test_nova_compute.py b/tests/unit/utils/test_nova_compute.py new file mode 100644 index 00000000..923e15ab --- /dev/null +++ b/tests/unit/utils/test_nova_compute.py @@ -0,0 +1,86 @@ +# Copyright 2023 Canonical Limited +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from juju.action import Action + +from cou.apps.base import ApplicationUnit +from cou.utils import nova_compute +from tests.unit.conftest import generate_mock_machine + + +@pytest.mark.asyncio +async def test_get_instance_count(model): + expected_count = 1 + model.run_action.return_value = mocked_action = AsyncMock(spec_set=Action).return_value + mocked_action.results = {"Code": "0", "instance-count": str(expected_count)} + + actual_count = await nova_compute.get_instance_count(unit="nova-compute/0", model=model) + + model.run_action.assert_called_once_with( + unit_name="nova-compute/0", + action_name="instance-count", + ) + assert actual_count == expected_count + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "result_key, value", + [ + ("not_valid", "1"), # invalid key + ("instance-count", "not_valid"), # invalid value + ("not_valid", "not_valid"), # invalid key and value + ], +) +async def test_get_instance_count_invalid_result(model, result_key, value): + model.run_action.return_value = mocked_action = AsyncMock(spec_set=Action).return_value + mocked_action.results = {"Code": "0", result_key: value} + + with pytest.raises(ValueError): + await nova_compute.get_instance_count(unit="nova-compute/0", model=model) + + +@pytest.mark.parametrize( + "hypervisors_count, expected_result", + [ + ([(0, 0), (1, 0), (2, 0)], {"0", "1", "2"}), + ([(0, 1), (1, 0), (2, 0)], {"1", "2"}), + ([(0, 1), (1, 3), (2, 0)], {"2"}), + ([(0, 1), (1, 3), (2, 5)], set()), + ], +) +@pytest.mark.asyncio +@patch("cou.utils.nova_compute.get_instance_count") +async def test_get_empty_hypervisors( + mock_instance_count, hypervisors_count, expected_result, model +): + mock_instance_count.side_effect = [count for _, count in hypervisors_count] + result = await nova_compute.get_empty_hypervisors( + [_mock_nova_unit(nova_unit) for nova_unit, _ in hypervisors_count], model + ) + assert {machine.machine_id for machine in result} == expected_result + + +def _mock_nova_unit(nova_unit): + mock_nova_unit = MagicMock(spec_set=ApplicationUnit(MagicMock(), MagicMock(), MagicMock())) + mock_nova_unit.name = f"nova-compute/{nova_unit}" + nova_machine = generate_mock_machine( + str(nova_unit), f"juju-c307f8-{nova_unit}", f"zone-{nova_unit + 1}" + ) + mock_nova_unit.machine = nova_machine + + return mock_nova_unit