From 232e9a407ca07f6c63d1c122af5fe5490ea09abd Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Tue, 22 Sep 2026 18:54:09 +0000 Subject: [PATCH 1/2] diagnostics: include resource usage We're updating the diagnostic report to include resource usage (disk, memory and cpu). See the updated API samples for examples. --- .../diagnostics/diagnostics-get-resp.json | 54 ++++++++ coriolis/api-refs/source/diagnostics.inc | 3 + coriolis/api-refs/source/parameters.yaml | 23 ++++ .../management/test_diagnostics.py | 54 +++++++- coriolis/tests/test_utils.py | 126 ++++++++++++++++++ coriolis/utils.py | 95 ++++++++++++- 6 files changed, 352 insertions(+), 3 deletions(-) diff --git a/coriolis/api-refs/api_samples/diagnostics/diagnostics-get-resp.json b/coriolis/api-refs/api_samples/diagnostics/diagnostics-get-resp.json index 5aefc4dff..4abbbfdf8 100644 --- a/coriolis/api-refs/api_samples/diagnostics/diagnostics-get-resp.json +++ b/coriolis/api-refs/api_samples/diagnostics/diagnostics-get-resp.json @@ -24,6 +24,33 @@ "" ], "hostname": "coriolis-conductor", + "filesystems": [ + { + "filesystem": "/dev/sda1", + "size": 21474836480, + "used": 10737418240, + "available": 10737418240, + "capacity": 50, + "mounted_on": "/" + } + ], + "memory": { + "total": 8589934592, + "used": 2147483648, + "free": 4294967296, + "shared": 134217728, + "buff_cache": 2147483648, + "available": 6442450944, + "swap": { + "total": 2147483648, + "used": 0, + "free": 2147483648 + } + }, + "cpu_usage": [ + {"core": 0, "percent": 3.2}, + {"core": 1, "percent": 1.0} + ], "ip_addresses": [ { "eth0": { @@ -113,6 +140,33 @@ "" ], "hostname": "coriolis-replica-cron", + "filesystems": [ + { + "filesystem": "/dev/sda1", + "size": 21474836480, + "used": 10737418240, + "available": 10737418240, + "capacity": 50, + "mounted_on": "/" + } + ], + "memory": { + "total": 8589934592, + "used": 2147483648, + "free": 4294967296, + "shared": 134217728, + "buff_cache": 2147483648, + "available": 6442450944, + "swap": { + "total": 2147483648, + "used": 0, + "free": 2147483648 + } + }, + "cpu_usage": [ + {"core": 0, "percent": 3.2}, + {"core": 1, "percent": 1.0} + ], "ip_addresses": [ { "eth0": { diff --git a/coriolis/api-refs/source/diagnostics.inc b/coriolis/api-refs/source/diagnostics.inc index 7513cf22c..81f69b027 100644 --- a/coriolis/api-refs/source/diagnostics.inc +++ b/coriolis/api-refs/source/diagnostics.inc @@ -29,6 +29,9 @@ Response - os_info : diagnostic_os_info - hostname : diagnostic_hostname - ip_addresses : diagnostic_ip_addresses + - filesystems : diagnostic_filesystems + - memory : diagnostic_memory + - cpu_usage : diagnostic_cpu_usage - licensing_status : diagnostic_licensing_status - packages : diagnostic_packages - licences : diagnostic_licences diff --git a/coriolis/api-refs/source/parameters.yaml b/coriolis/api-refs/source/parameters.yaml index 205446c1e..4d4a8456c 100644 --- a/coriolis/api-refs/source/parameters.yaml +++ b/coriolis/api-refs/source/parameters.yaml @@ -407,6 +407,21 @@ diagnostic_application: in: body type: string required: true +diagnostic_cpu_usage: + description: | + CPU usage percentage for each core. Each entry has core (zero-based + index) and percent. + in: body + type: array + required: true +diagnostic_filesystems: + description: | + Usage of mounted block devices, in bytes. Each entry reports the + filesystem, mount point, size, used and available bytes, and capacity + percentage. Nodev filesystems such as tmpfs are omitted. + in: body + type: array + required: true diagnostic_hostname: description: | The hostname of the Coriolis service container. @@ -431,6 +446,14 @@ diagnostic_licensing_status: in: body type: object required: true +diagnostic_memory: + description: | + Memory usage in bytes, from /proc/meminfo. Includes total, used, free, + shared, buff_cache, available, and swap (total, used, free). used is + total minus free minus buff_cache. + in: body + type: object + required: true diagnostic_os_info: description: | The Coriolis appliance's host OS information. diff --git a/coriolis/tests/integration/management/test_diagnostics.py b/coriolis/tests/integration/management/test_diagnostics.py index f9dd38d0a..def09faa6 100644 --- a/coriolis/tests/integration/management/test_diagnostics.py +++ b/coriolis/tests/integration/management/test_diagnostics.py @@ -42,4 +42,56 @@ def test_get_diagnostics(self): self.assertEqual(diag.os_info, utils._get_host_os_info()) self.assertEqual(diag.hostname, socket.gethostname()) - self.assertEqual(diag.to_dict(), utils.get_diagnostics_info()) + actual = diag.to_dict() + expected = utils.get_diagnostics_info() + # Disk, memory, and CPU are sampled when each diagnostics payload is + # built, so two reads are not identical. + for key in ("filesystems", "memory", "cpu_usage"): + self.assertIn(key, actual) + actual.pop(key) + expected.pop(key) + + self.assertEqual(actual, expected) + self._assert_filesystems(diag.filesystems) + self._assert_memory(diag.memory) + self._assert_cpu_usage(diag.cpu_usage) + + def _assert_filesystems(self, filesystems): + self.assertIsInstance(filesystems, list) + self.assertTrue(filesystems, "Expected at least one filesystem") + mounts = [] + for entry in filesystems: + self.assertEqual( + set(entry.keys()), + {"filesystem", "size", "used", "available", "capacity", "mounted_on"}, + ) + self.assertIsInstance(entry["filesystem"], str) + self.assertIsInstance(entry["mounted_on"], str) + for field in ("size", "used", "available", "capacity"): + self.assertIsInstance(entry[field], int) + mounts.append(entry["mounted_on"]) + self.assertIn("/", mounts) + + def _assert_memory(self, memory): + self.assertEqual( + set(memory.keys()), + {"total", "used", "free", "shared", "buff_cache", "available", "swap"}, + ) + for field in ("total", "used", "free", "shared", "buff_cache", "available"): + self.assertIsInstance(memory[field], int) + self.assertGreater(memory["total"], 0) + self.assertEqual(set(memory["swap"].keys()), {"total", "used", "free"}) + for field in ("total", "used", "free"): + self.assertIsInstance(memory["swap"][field], int) + + def _assert_cpu_usage(self, cpu_usage): + self.assertIsInstance(cpu_usage, list) + self.assertTrue(cpu_usage, "Expected at least one CPU core") + cores = [] + for entry in cpu_usage: + self.assertEqual(set(entry.keys()), {"core", "percent"}) + self.assertIsInstance(entry["core"], int) + self.assertIsInstance(entry["percent"], (int, float)) + self.assertGreaterEqual(entry["percent"], 0) + cores.append(entry["core"]) + self.assertEqual(cores, list(range(len(cpu_usage)))) diff --git a/coriolis/tests/test_utils.py b/coriolis/tests/test_utils.py index 6aece2fa1..03d115939 100644 --- a/coriolis/tests/test_utils.py +++ b/coriolis/tests/test_utils.py @@ -74,6 +74,132 @@ def test_get_single_result_single_element(self): result = utils.get_single_result([1]) self.assertEqual(result, 1) + @mock.patch.object(utils.psutil, 'disk_usage') + @mock.patch.object(utils.psutil, 'disk_partitions') + def test_get_filesystems(self, mock_partitions, mock_usage): + mock_partitions.return_value = [ + mock.Mock(device='/dev/sda1', mountpoint='/'), + mock.Mock(device='/dev/sdb1', mountpoint='/mnt/my data'), + ] + mock_usage.side_effect = lambda path: { + '/': mock.Mock(total=1000, used=400, free=600), + # 1 / (1 + 2) is 33.3%, which rounds up to 34. + '/mnt/my data': mock.Mock(total=10, used=1, free=2), + }[path] + + result = utils._get_filesystems() + + mock_partitions.assert_called_once_with(all=False) + self.assertEqual( + result, + [ + { + "filesystem": "/dev/sda1", + "size": 1000, + "used": 400, + "available": 600, + "capacity": 40, + "mounted_on": "/", + }, + { + "filesystem": "/dev/sdb1", + "size": 10, + "used": 1, + "available": 2, + "capacity": 34, + "mounted_on": "/mnt/my data", + }, + ], + ) + + @mock.patch.object(utils.psutil, 'disk_usage') + @mock.patch.object(utils.psutil, 'disk_partitions') + def test_get_filesystems_skips_unreadable_mounts(self, mock_partitions, mock_usage): + mock_partitions.return_value = [ + mock.Mock(device='/dev/sda1', mountpoint='/'), + mock.Mock(device='/dev/sdb1', mountpoint='/mnt/data'), + ] + + def _usage(path): + if path == '/': + raise OSError('denied') + return mock.Mock(total=1000, used=400, free=600) + + mock_usage.side_effect = _usage + + with self.assertLogs('coriolis.utils', level=logging.WARNING): + result = utils._get_filesystems() + + self.assertEqual(len(result), 1) + self.assertEqual(result[0]["mounted_on"], "/mnt/data") + + @mock.patch.object(utils.psutil, 'swap_memory') + @mock.patch.object(utils.psutil, 'virtual_memory') + def test_get_memory(self, mock_virtual_memory, mock_swap_memory): + # psutil's used is total - available (4000). free's used is + # total - free - buffers - cached (5000). + mock_virtual_memory.return_value = mock.Mock( + total=10000, + used=4000, + free=2000, + buffers=500, + cached=2500, + shared=100, + available=6000, + ) + mock_swap_memory.return_value = mock.Mock(total=1000, used=0, free=1000) + + result = utils._get_memory() + + mock_virtual_memory.assert_called_once_with() + mock_swap_memory.assert_called_once_with() + self.assertEqual( + result, + { + "total": 10000, + "used": 5000, + "free": 2000, + "shared": 100, + "buff_cache": 3000, + "available": 6000, + "swap": {"total": 1000, "used": 0, "free": 1000}, + }, + ) + + @mock.patch.object(utils.psutil, 'cpu_percent', return_value=[1.5, 0.0]) + def test_get_cpu_usage(self, mock_cpu_percent): + result = utils._get_cpu_usage() + + mock_cpu_percent.assert_called_once_with( + interval=utils._CPU_SAMPLE_SECONDS, percpu=True + ) + self.assertEqual( + result, + [ + {"core": 0, "percent": 1.5}, + {"core": 1, "percent": 0.0}, + ], + ) + + @mock.patch.object( + utils, '_get_cpu_usage', return_value=[{"core": 0, "percent": 1.0}] + ) + @mock.patch.object(utils, '_get_memory', return_value={"total": 1}) + @mock.patch.object( + utils, '_get_filesystems', return_value=[{"filesystem": "/dev/sda1"}] + ) + def test_get_diagnostics_info_includes_host_resources( + self, mock_filesystems, mock_memory, mock_cpu_usage + ): + info = utils.get_diagnostics_info() + + self.assertEqual(info["filesystems"], mock_filesystems.return_value) + self.assertEqual(info["memory"], mock_memory.return_value) + self.assertEqual(info["cpu_usage"], mock_cpu_usage.return_value) + mock_filesystems.assert_called_once_with() + mock_memory.assert_called_once_with() + mock_cpu_usage.assert_called_once_with() + def test_retry_on_error_no_exception(self): result = utils.retry_on_error( max_attempts=5, sleep_seconds=0, terminal_exceptions=[] diff --git a/coriolis/utils.py b/coriolis/utils.py index 9e53e4c5b..6e9e288c7 100644 --- a/coriolis/utils.py +++ b/coriolis/utils.py @@ -26,6 +26,7 @@ import netifaces import OpenSSL import paramiko +import psutil from oslo_config import cfg from oslo_log import log as logging from oslo_serialization import jsonutils @@ -57,6 +58,9 @@ UNSPACED_MAC_ADDRESS_REGEX = "^([0-9a-f]{12})$" SPACED_MAC_ADDRESS_REGEX = "^(([0-9a-f]{2}:){5}([0-9a-f]{2}))$" +# Short sample so diagnostics stay responsive when several services are queried. +_CPU_SAMPLE_SECONDS = 0.1 + SYSTEMD_TEMPLATE = """ [Unit] Description=Coriolis %(svc_name)s @@ -124,9 +128,93 @@ def _get_release_tag(): return release_tag +def _filesystem_capacity(used, available): + """Return the used-space percentage, rounded up. + + This is the integer Capacity column ``df`` prints: + ``used / (used + available) * 100``. + """ + total = used + available + if total == 0: + return 100 if used else 0 + return (used * 100 + total - 1) // total + + +def _get_filesystems(): + """Return usage for mounted block devices, in bytes. + + The mount list is ``psutil.disk_partitions(all=False)``: devices with a + real block filesystem, including squashfs, and not nodev types such as + tmpfs. ``capacity`` is the integer percentage of used space, rounded up. + """ + filesystems = [] + for partition in psutil.disk_partitions(all=False): + try: + usage = psutil.disk_usage(partition.mountpoint) + except OSError as exc: + LOG.warning( + "Unable to read usage for %s (%s): %s", + partition.mountpoint, + partition.device, + exc, + ) + continue + filesystems.append( + { + "filesystem": partition.device, + "size": usage.total, + "used": usage.used, + "available": usage.free, + "capacity": _filesystem_capacity(usage.used, usage.free), + "mounted_on": partition.mountpoint, + } + ) + return filesystems + + +def _get_memory(): + """Return memory usage in bytes. + + Values come from ``psutil``, which reads ``/proc/meminfo``. ``used`` and + ``buff_cache`` follow the ``free`` command: ``buff_cache`` is buffers plus + cached memory, and ``used`` is total minus free minus that cache. psutil's + own ``used`` field (total minus available) is not used. + """ + mem = psutil.virtual_memory() + swap = psutil.swap_memory() + buff_cache = mem.buffers + mem.cached + return { + "total": mem.total, + "used": mem.total - mem.free - buff_cache, + "free": mem.free, + "shared": mem.shared, + "buff_cache": buff_cache, + "available": mem.available, + "swap": { + "total": swap.total, + "used": swap.used, + "free": swap.free, + }, + } + + +def _get_cpu_usage(): + """Return recent CPU usage percentage for each core. + + The list is ordered by core index, matching ``psutil.cpu_percent``. + """ + percents = psutil.cpu_percent(interval=_CPU_SAMPLE_SECONDS, percpu=True) + return [{"core": core, "percent": percent} for core, percent in enumerate(percents)] + + def get_diagnostics_info(): - # TODO(gsamfira): decide if we want any other kind of - # diagnostics. + """Return diagnostic details for this Coriolis service process. + + Host resource fields: + - ``filesystems``: block-device usage from psutil, in bytes + - ``memory``: usage from ``/proc/meminfo`` via psutil, in bytes, including swap + - ``cpu_usage``: CPU usage percentage for each core + """ packages = list(freeze.freeze()) return { "application": get_binary_name(), @@ -135,6 +223,9 @@ def get_diagnostics_info(): "hostname": get_hostname(), "ip_addresses": _get_local_ips(), "release_tag": _get_release_tag(), + "filesystems": _get_filesystems(), + "memory": _get_memory(), + "cpu_usage": _get_cpu_usage(), } From 5e6052eedc26ef87f0d3d5fa85a38f57afda809b Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Thu, 24 Sep 2026 09:55:06 +0000 Subject: [PATCH 2/2] diagnostics: rename "capacity" to used_percentage and update "used" meaning "capacity" is misleading, we'll report it as "used_percentage" instead. Also, we'll ensure that "used" also includes reserved space so that used + free == total. --- .../diagnostics/diagnostics-get-resp.json | 4 ++-- coriolis/api-refs/source/parameters.yaml | 5 +++-- .../management/test_diagnostics.py | 12 +++++++++-- coriolis/tests/test_utils.py | 8 +++---- coriolis/utils.py | 21 ++++++++++++------- 5 files changed, 32 insertions(+), 18 deletions(-) diff --git a/coriolis/api-refs/api_samples/diagnostics/diagnostics-get-resp.json b/coriolis/api-refs/api_samples/diagnostics/diagnostics-get-resp.json index 4abbbfdf8..c2a25902b 100644 --- a/coriolis/api-refs/api_samples/diagnostics/diagnostics-get-resp.json +++ b/coriolis/api-refs/api_samples/diagnostics/diagnostics-get-resp.json @@ -30,7 +30,7 @@ "size": 21474836480, "used": 10737418240, "available": 10737418240, - "capacity": 50, + "used_percentage": 50, "mounted_on": "/" } ], @@ -146,7 +146,7 @@ "size": 21474836480, "used": 10737418240, "available": 10737418240, - "capacity": 50, + "used_percentage": 50, "mounted_on": "/" } ], diff --git a/coriolis/api-refs/source/parameters.yaml b/coriolis/api-refs/source/parameters.yaml index 4d4a8456c..6b982029d 100644 --- a/coriolis/api-refs/source/parameters.yaml +++ b/coriolis/api-refs/source/parameters.yaml @@ -417,8 +417,9 @@ diagnostic_cpu_usage: diagnostic_filesystems: description: | Usage of mounted block devices, in bytes. Each entry reports the - filesystem, mount point, size, used and available bytes, and capacity - percentage. Nodev filesystems such as tmpfs are omitted. + filesystem, mount point, size, used and available bytes, and + used_percentage. used includes blocks reserved for root, so size + equals used plus available. Nodev filesystems such as tmpfs are omitted. in: body type: array required: true diff --git a/coriolis/tests/integration/management/test_diagnostics.py b/coriolis/tests/integration/management/test_diagnostics.py index def09faa6..f6e7cf4d4 100644 --- a/coriolis/tests/integration/management/test_diagnostics.py +++ b/coriolis/tests/integration/management/test_diagnostics.py @@ -63,12 +63,20 @@ def _assert_filesystems(self, filesystems): for entry in filesystems: self.assertEqual( set(entry.keys()), - {"filesystem", "size", "used", "available", "capacity", "mounted_on"}, + { + "filesystem", + "size", + "used", + "available", + "used_percentage", + "mounted_on", + }, ) self.assertIsInstance(entry["filesystem"], str) self.assertIsInstance(entry["mounted_on"], str) - for field in ("size", "used", "available", "capacity"): + for field in ("size", "used", "available", "used_percentage"): self.assertIsInstance(entry[field], int) + self.assertEqual(entry["size"], entry["used"] + entry["available"]) mounts.append(entry["mounted_on"]) self.assertIn("/", mounts) diff --git a/coriolis/tests/test_utils.py b/coriolis/tests/test_utils.py index 03d115939..a5e23e6a4 100644 --- a/coriolis/tests/test_utils.py +++ b/coriolis/tests/test_utils.py @@ -83,7 +83,7 @@ def test_get_filesystems(self, mock_partitions, mock_usage): ] mock_usage.side_effect = lambda path: { '/': mock.Mock(total=1000, used=400, free=600), - # 1 / (1 + 2) is 33.3%, which rounds up to 34. + # 7 bytes are reserved for root, so reported used is 8, not 1. '/mnt/my data': mock.Mock(total=10, used=1, free=2), }[path] @@ -98,15 +98,15 @@ def test_get_filesystems(self, mock_partitions, mock_usage): "size": 1000, "used": 400, "available": 600, - "capacity": 40, + "used_percentage": 40, "mounted_on": "/", }, { "filesystem": "/dev/sdb1", "size": 10, - "used": 1, + "used": 8, "available": 2, - "capacity": 34, + "used_percentage": 80, "mounted_on": "/mnt/my data", }, ], diff --git a/coriolis/utils.py b/coriolis/utils.py index 6e9e288c7..462ab869a 100644 --- a/coriolis/utils.py +++ b/coriolis/utils.py @@ -128,15 +128,15 @@ def _get_release_tag(): return release_tag -def _filesystem_capacity(used, available): +def _filesystem_used_percentage(used, available): """Return the used-space percentage, rounded up. - This is the integer Capacity column ``df`` prints: + ``available`` is space a normal user can still use. The percentage is ``used / (used + available) * 100``. """ total = used + available if total == 0: - return 100 if used else 0 + return 0 return (used * 100 + total - 1) // total @@ -145,7 +145,9 @@ def _get_filesystems(): The mount list is ``psutil.disk_partitions(all=False)``: devices with a real block filesystem, including squashfs, and not nodev types such as - tmpfs. ``capacity`` is the integer percentage of used space, rounded up. + tmpfs. ``size`` is the whole filesystem. ``used`` includes blocks + reserved for root, so ``size`` equals ``used + available``. + ``used_percentage`` is that used share, rounded up. """ filesystems = [] for partition in psutil.disk_partitions(all=False): @@ -159,13 +161,16 @@ def _get_filesystems(): exc, ) continue + total = usage.total + available = usage.free + used = total - available filesystems.append( { "filesystem": partition.device, - "size": usage.total, - "used": usage.used, - "available": usage.free, - "capacity": _filesystem_capacity(usage.used, usage.free), + "size": total, + "used": used, + "available": available, + "used_percentage": _filesystem_used_percentage(used, available), "mounted_on": partition.mountpoint, } )