From 44f5a5a8a18a7fdf479ed2d58761cee9ed9d4923 Mon Sep 17 00:00:00 2001
From: Samuel Allan <samuel.allan@canonical.com>
Date: Wed, 23 Oct 2024 21:00:53 +1030
Subject: [PATCH] Improve warning message when non-empty hypervisors (#599)

Make it clear that the non-empty hypervisors will be skipped,
why they will be skipped,
and how to upgrade them anyway if the user really wants to.

Fixes: #480
---
 cou/utils/nova_compute.py             | 5 ++++-
 tests/unit/test_commands.py           | 2 +-
 tests/unit/utils/test_nova_compute.py | 9 +--------
 3 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/cou/utils/nova_compute.py b/cou/utils/nova_compute.py
index b0d8e386..3c51a0a2 100644
--- a/cou/utils/nova_compute.py
+++ b/cou/utils/nova_compute.py
@@ -41,7 +41,10 @@ async def get_empty_hypervisors(units: list[Unit], model: Model) -> list[Machine
 
     if skipped_units:
         logger.warning(
-            "Found non-empty hypervisors: %s", sorted(skipped_units, key=lambda unit: unit.name)
+            "Found non-empty hypervisors (hypervisors with VMs still scheduled on them): %s. "
+            "Upgrade will be skipped for these units to avoid downtime. "
+            "Use the --force flag on cou to upgrade these anyway (not recommended on production).",
+            sorted(skipped_units, key=lambda unit: unit.name),
         )
     logger.info("Selected hypervisors: %s", sorted(empty_units, key=lambda unit: unit.name))
 
diff --git a/tests/unit/test_commands.py b/tests/unit/test_commands.py
index 0c3b92fc..2f8541ed 100644
--- a/tests/unit/test_commands.py
+++ b/tests/unit/test_commands.py
@@ -901,7 +901,7 @@ def test_parse_args_raise_exception(mock_error, args):
     with pytest.raises(SystemExit):
         commands.parse_args(args)
     mock_error.assert_called_once()
-    assert "unrecognized arguments" in mock_error.call_args[0][1]
+    assert "unrecognized arguments" in str(mock_error.call_args)
 
 
 def test_capitalize_usage_prefix():
diff --git a/tests/unit/utils/test_nova_compute.py b/tests/unit/utils/test_nova_compute.py
index 0249689a..fa6c06e6 100644
--- a/tests/unit/utils/test_nova_compute.py
+++ b/tests/unit/utils/test_nova_compute.py
@@ -63,10 +63,9 @@ 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, mock_logger, hypervisors_count, expected_result, model
+    mock_instance_count, hypervisors_count, expected_result, model
 ):
     mock_instance_count.side_effect = [count for _, count in hypervisors_count]
     selected_hypervisors = [
@@ -80,12 +79,6 @@ async def test_get_empty_hypervisors(
 
     assert {machine.machine_id for machine in result} == expected_result
 
-    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