From fe2d6710be876af7533d8b9107a12ffb4acef685 Mon Sep 17 00:00:00 2001 From: Xuhui Zhu Date: Thu, 11 Apr 2024 14:31:35 -0400 Subject: [PATCH] Add log to inform skipped(non-empty) hypervisors (#351) Add log to inform user about skipped hypervisors(those having VMs running) at plan stage. fixes: https://github.com/canonical/charmed-openstack-upgrader/issues/326 --------- Co-authored-by: Robert Gildein Co-authored-by: TQ X --- cou/steps/plan.py | 3 ++- cou/utils/app_utils.py | 16 ++++++++++++++-- cou/utils/nova_compute.py | 10 +++++++++- tests/unit/utils/test_nova_compute.py | 18 ++++++++++++++++-- 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/cou/steps/plan.py b/cou/steps/plan.py index 3162383d..f03945e4 100644 --- a/cou/steps/plan.py +++ b/cou/steps/plan.py @@ -50,7 +50,7 @@ from cou.steps.analyze import Analysis from cou.steps.backup import backup from cou.steps.hypervisor import HypervisorUpgradePlanner -from cou.utils.app_utils import set_require_osd_release_option +from cou.utils.app_utils import set_require_osd_release_option, stringify_units from cou.utils.juju_utils import DEFAULT_TIMEOUT, Machine, Unit from cou.utils.nova_compute import get_empty_hypervisors from cou.utils.openstack import LTS_TO_OS_RELEASE, OpenStackRelease @@ -594,6 +594,7 @@ async def _get_upgradable_hypervisors_machines( ) if cli_force: + logger.info("Selected all hypervisors: %s", stringify_units(nova_compute_units)) return nova_compute_machines return await get_empty_hypervisors(nova_compute_units, analysis_result.model) diff --git a/cou/utils/app_utils.py b/cou/utils/app_utils.py index 5acc3867..67656423 100644 --- a/cou/utils/app_utils.py +++ b/cou/utils/app_utils.py @@ -15,10 +15,10 @@ """Application utilities.""" import json import logging -from typing import Optional +from typing import Iterable, Optional from cou.exceptions import RunUpgradeError -from cou.utils.juju_utils import Model +from cou.utils.juju_utils import Model, Unit from cou.utils.openstack import CEPH_RELEASES logger = logging.getLogger(__name__) @@ -133,3 +133,15 @@ async def _get_current_osd_release(unit: str, model: Model) -> str: logger.debug("Currently OSDs are on the '%s' release", current_osd_release) return current_osd_release + + +def stringify_units(units: Iterable[Unit]) -> str: + """Convert Units into a comma-separated string of unit names, sorted alphabetically. + + :param units: An iterable of Unit objects to be converted. + :type units: Iterable[Unit] + :return: A comma-separated string of sorted unit names. + :rtype: str + """ + sorted_unit_names = sorted([unit.name for unit in units]) + return ", ".join(sorted_unit_names) diff --git a/cou/utils/nova_compute.py b/cou/utils/nova_compute.py index faf433a4..dfc6e38c 100644 --- a/cou/utils/nova_compute.py +++ b/cou/utils/nova_compute.py @@ -18,6 +18,7 @@ import logging from cou.exceptions import HaltUpgradeExecution +from cou.utils.app_utils import stringify_units from cou.utils.juju_utils import Machine, Model, Unit logger = logging.getLogger(__name__) @@ -36,7 +37,14 @@ async def get_empty_hypervisors(units: list[Unit], model: Model) -> 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] + empty_units = {unit for unit, instances in units_instances if instances == 0} + skipped_units = set(units) - empty_units + + if skipped_units: + logger.warning("Found non-empty hypervisors: %s", stringify_units(skipped_units)) + logger.info("Selected hypervisors: %s", stringify_units(empty_units)) + + return [unit.machine for unit in empty_units] async def get_instance_count(unit: str, model: Model) -> int: diff --git a/tests/unit/utils/test_nova_compute.py b/tests/unit/utils/test_nova_compute.py index c82daa4b..77178f15 100644 --- a/tests/unit/utils/test_nova_compute.py +++ b/tests/unit/utils/test_nova_compute.py @@ -11,7 +11,6 @@ # 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 @@ -64,16 +63,31 @@ async def test_get_instance_count_invalid_result(model, result_key, value): ], ) @pytest.mark.asyncio +@patch("cou.utils.nova_compute.logger") @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, mock_logger, 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 + selected_hypervisors = ", ".join( + [f"nova-compute/{i}" for i, count in hypervisors_count if count == 0] + ) + non_empty_hypervisors = ", ".join( + [f"nova-compute/{i}" for i, count in hypervisors_count if count != 0] + ) + + mock_logger.info.assert_called_once_with("Selected hypervisors: %s", selected_hypervisors) + if non_empty_hypervisors: + mock_logger.warning.assert_called_once_with( + "Found non-empty hypervisors: %s", non_empty_hypervisors + ) + @pytest.mark.parametrize("instance_count", [1, 10, 50]) @pytest.mark.asyncio