Skip to content

Commit

Permalink
Add log to inform skipped(non-empty) hypervisors (canonical#351)
Browse files Browse the repository at this point in the history
Add log to inform user about skipped hypervisors(those having VMs
running) at plan stage.

fixes:
canonical#326

---------

Co-authored-by: Robert Gildein <[email protected]>
Co-authored-by: TQ X <[email protected]>
  • Loading branch information
3 people authored Apr 11, 2024
1 parent 17b4fdb commit fe2d671
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 6 deletions.
3 changes: 2 additions & 1 deletion cou/steps/plan.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
16 changes: 14 additions & 2 deletions cou/utils/app_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -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)
10 changes: 9 additions & 1 deletion cou/utils/nova_compute.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand All @@ -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:
Expand Down
18 changes: 16 additions & 2 deletions tests/unit/utils/test_nova_compute.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit fe2d671

Please sign in to comment.