diff --git a/computes/models.py b/computes/models.py index ec7c769b..961f3740 100644 --- a/computes/models.py +++ b/computes/models.py @@ -9,7 +9,6 @@ from libvirt import libvirtError, virConnect from computes.validators import validate_hostname, validate_login, validate_migration_address from vrtManager.connection import connection_manager from vrtManager.hostdetails import wvmHostDetails -from vrtManager.storage import wvmStorages class Compute(Model): @@ -90,10 +89,10 @@ class Compute(Model): @cached_property def capacity(self): - """What the host has given its VMs (wvmHostDetails.get_capacity), its - vCPUs per CPU, and its active storage pools, the default one first - (None when the pools cannot be read); or None when the host cannot be - read.""" + """What the host has given its VMs (wvmHostDetails.get_capacity) and + its vCPUs per CPU; or None when the host cannot be read. Not its + pools: their space is measured by a refresh that scans each pool, + too slow for pages that read every host (computes.views.host_pools).""" if self.status is not True: return None try: @@ -101,12 +100,6 @@ class Compute(Model): except libvirtError: return None given["ratio"] = given["vcpus"]["all"] / given["cpus"] if given["cpus"] else 0 - try: - storages = wvmStorages(self.hostname, self.login, self.password, self.type) - pools = [pool for pool in storages.get_storages_info() if pool["status"]] - given["pools"] = sorted(pools, key=lambda pool: (pool["name"] != "default", pool["name"])) - except libvirtError: - given["pools"] = None return given def __str__(self): diff --git a/computes/templates/computes/capacity_line.txt b/computes/templates/computes/capacity_line.txt index 9a1d704b..06a794be 100644 --- a/computes/templates/computes/capacity_line.txt +++ b/computes/templates/computes/capacity_line.txt @@ -1,6 +1,5 @@ {% load i18n %}{% autoescape off %}{% if given %} {% blocktrans count counter=given.vcpus.all with ratio=given.ratio|floatformat:1 assigned=given.assigned.all|filesizeformat %}Given to VMs: {{ counter }} vCPU ({{ ratio }}:1), {{ assigned }} memory.{% plural %}Given to VMs: {{ counter }} vCPUs ({{ ratio }}:1), {{ assigned }} memory.{% endblocktrans %} -{% with pool=given.pools.0 %}{% if pool %}{% blocktrans with name=pool.name free=pool.free|filesizeformat %}Pool {{ name }}: {{ free }} free.{% endblocktrans %}{% endif %}{% endwith %} {% else %} {% trans "The capacity could not be read." %} {% endif %}{% endautoescape %} diff --git a/computes/templates/computes/capacity_storage.html b/computes/templates/computes/capacity_storage.html index 569b5d7e..388f6f4e 100644 --- a/computes/templates/computes/capacity_storage.html +++ b/computes/templates/computes/capacity_storage.html @@ -1,15 +1,16 @@ {% load i18n %} -{# a host's storage pools: given, from views.shown_capacity #} -{% if given.pools is None %} +{# a host's storage pools: storage, from views.host_pools #} +{% if storage is None %}
{% trans "The storage pools could not be read." %}
-{% elif not given.pool %} +{% elif not storage.pool %}
{% trans "No active storage pool." %}
+{% else %} +{% include "computes/pool_bar.html" with pool=storage.pool %} {% endif %} -{% if given.pool %}{% include "computes/pool_bar.html" with pool=given.pool %}{% endif %} -{% if given.more_pools %} +{% if storage.more_pools %}
- {% blocktrans count counter=given.more_pools|length %}{{ counter }} more pool{% plural %}{{ counter }} more pools{% endblocktrans %} - {% for pool in given.more_pools %} + {% blocktrans count counter=storage.more_pools|length %}{{ counter }} more pool{% plural %}{{ counter }} more pools{% endblocktrans %} + {% for pool in storage.more_pools %}
{% include "computes/pool_bar.html" %}
{% endfor %}
diff --git a/computes/templates/computes/list.html b/computes/templates/computes/list.html index 1009cd12..1a57583e 100644 --- a/computes/templates/computes/list.html +++ b/computes/templates/computes/list.html @@ -29,7 +29,6 @@ {% trans "Host" %} {% trans "vCPUs" %} {% trans "Memory" %} - {% trans "Storage" %} @@ -60,11 +59,8 @@ {% include "computes/capacity_memory.html" %} - - {% include "computes/capacity_storage.html" %} - {% else %} - {% if compute.status is True %}{% trans "The capacity could not be read." %}{% else %}{% trans "No capacity while the host cannot be reached." %}{% endif %} + {% if compute.status is True %}{% trans "The capacity could not be read." %}{% else %}{% trans "No capacity while the host cannot be reached." %}{% endif %} {% endif %}
diff --git a/computes/tests/test_capacity.py b/computes/tests/test_capacity.py index 5e88203a..34a99ade 100644 --- a/computes/tests/test_capacity.py +++ b/computes/tests/test_capacity.py @@ -14,33 +14,15 @@ GIB = 1024 ** 3 VMS = {"running": 3, "paused": 1, "shutoff": 2, "other": 0} -def pool(name, size_gib, free_gib, status=1): - return {"name": name, "status": status, "size": size_gib * GIB, "free": free_gib * GIB} - - class CapacityTests(TestCase): def setUp(self): self.compute = Compute(name="host-one", hostname="192.0.2.1", login="u", password="p", type=1) self.compute.status = True - @patch("computes.models.wvmStorages") @patch("computes.models.wvmHostDetails") - def test_a_host_tells_what_it_gave_and_its_active_pools(self, details, storages): + def test_a_host_tells_what_it_gave(self, details): details.return_value.get_capacity.return_value = {"cpus": 16, "vcpus": {"all": 24, "running": 8}} - storages.return_value.get_storages_info.return_value = [ - pool("nfs", 900, 800), pool("off", 10, 0, status=0), pool("default", 500, 120), pool("iso", 100, 90)] - # the default pool first, the others by name - self.assertEqual(self.compute.capacity, { - "cpus": 16, "vcpus": {"all": 24, "running": 8}, "ratio": 1.5, - "pools": [pool("default", 500, 120), pool("iso", 100, 90), pool("nfs", 900, 800)]}) - - @patch("computes.models.wvmStorages") - @patch("computes.models.wvmHostDetails") - def test_pools_that_cannot_be_read_leave_the_cpus_and_memory(self, details, storages): - details.return_value.get_capacity.return_value = {"cpus": 16, "vcpus": {"all": 24, "running": 8}} - storages.return_value.get_storages_info.side_effect = libvirtError("cannot refresh pool") - self.assertEqual(self.compute.capacity, { - "cpus": 16, "vcpus": {"all": 24, "running": 8}, "ratio": 1.5, "pools": None}) + self.assertEqual(self.compute.capacity, {"cpus": 16, "vcpus": {"all": 24, "running": 8}, "ratio": 1.5}) @patch("computes.models.wvmHostDetails") def test_a_host_whose_read_fails_has_no_capacity(self, details): @@ -62,13 +44,13 @@ class ComputesPageTests(TestCase): c1 = Compute.objects.create(name="host-one", hostname="192.0.2.1", login="u", password="p", type=1) Compute.objects.create(name="host-two", hostname="192.0.2.2", login="u", password="p", type=1) found = {c1.id: {"cpus": 16, "memory": 64 * GIB, "vcpus": {"all": 40, "running": 24}, - "assigned": {"all": 48 * GIB, "running": 32 * GIB}, "ratio": 2.5, "vms": VMS, - "pools": [pool("default", 500, 120)]}} + "assigned": {"all": 48 * GIB, "running": 32 * GIB}, "ratio": 2.5, "vms": VMS}} with patch.object(Compute, "capacity", new=property(lambda c: found.get(c.id))), \ patch.object(Compute, "status", new=True): page = html.fromstring(self.client.get(reverse("computes")).content) headers = [" ".join(th.text_content().split()) for th in page.xpath("//thead//th")] - self.assertEqual(headers, ["Host", "vCPUs", "Memory", "Storage", "Actions"]) + # no pools: measuring their space scans each pool of each host (views.host_pools) + self.assertEqual(headers, ["Host", "vCPUs", "Memory", "Actions"]) one, two = page.xpath("//tbody/tr") cells = [" ".join(td.text_content().split()) for td in one.xpath("td")] self.assertIn("host-one", cells[0]) @@ -76,27 +58,9 @@ class ComputesPageTests(TestCase): self.assertIn("40 vCPUs on 16 CPUs 24 running", cells[1]) self.assertIn("75 % 48.0 GB / 64.0 GB", cells[2]) self.assertIn("32.0 GB running", cells[2]) - self.assertIn("120.0 GB free of 500.0 GB", cells[3]) # host-two could not be read: one cell says so self.assertIn("capacity could not be read", " ".join(two.text_content().split())) - def test_the_default_pool_shows_and_the_others_open_on_a_click(self): - admin = get_user_model().objects.create_superuser("cap-pools", "pools@example.com", "pw") - self.client.force_login(admin) - Compute.objects.create(name="host-one", hostname="192.0.2.1", login="u", password="p", type=1) - pools = [pool("default", 500, 120), pool("backup", 900, 800), pool("iso", 100, 90), pool("nfs", 2000, 1000)] - given = {"cpus": 16, "memory": 64 * GIB, "vcpus": {"all": 8, "running": 8}, "ratio": 0.5, - "assigned": {"all": 8 * GIB, "running": 8 * GIB}, "pools": pools, "vms": VMS} - with patch.object(Compute, "capacity", new=given), patch.object(Compute, "status", new=True): - page = html.fromstring(self.client.get(reverse("computes")).content) - (storage,) = page.xpath("//tbody/tr/td[4]") - shown = " ".join(" ".join(storage.xpath("./div//text()")).split()) - self.assertIn("default", shown) - self.assertNotIn("backup", shown) - (more,) = storage.xpath(".//details") - self.assertEqual(" ".join(more.xpath("summary")[0].text_content().split()), "3 more pools") - self.assertEqual(more.xpath(".//div[@class='small']/text()"), ["backup", "iso", "nfs"]) - class SingularTests(TestCase): def test_one_vcpu_on_one_cpu_is_singular(self): @@ -104,7 +68,7 @@ class SingularTests(TestCase): self.client.force_login(admin) c = Compute.objects.create(name="host-one", hostname="192.0.2.1", login="u", password="p", type=1) given = {"cpus": 1, "memory": 4 * GIB, "vcpus": {"all": 1, "running": 1}, - "assigned": {"all": GIB, "running": GIB}, "ratio": 1.0, "pools": [], "vms": VMS} + "assigned": {"all": GIB, "running": GIB}, "ratio": 1.0, "vms": VMS} with patch.object(Compute, "capacity", new=given), patch.object(Compute, "status", new=True): page = html.fromstring(self.client.get(reverse("computes")).content) line = self.client.get(reverse("compute_capacity", args=[c.id])).content.decode() @@ -130,13 +94,11 @@ class NewVmDialogLineTests(TestCase): def test_the_line_tells_what_the_host_has_given(self): given = {"cpus": 16, "memory": 64 * GIB, "vcpus": {"all": 24, "running": 8}, "assigned": {"all": 48 * GIB, "running": 32 * GIB}, "ratio": 1.5, - "pools": [pool("a&b", 500, 120), pool("iso", 100, 90)]} - # plain text: a pool name is not escaped - self.assertEqual(self.line(given), "Given to VMs: 24 vCPUs (1.5:1), 48.0 GB memory. " - "Pool a&b: 120.0 GB free.") + } + self.assertEqual(self.line(given), "Given to VMs: 24 vCPUs (1.5:1), 48.0 GB memory.") def test_a_host_without_pools_or_capacity_says_so(self): given = {"cpus": 2, "memory": 4 * GIB, "vcpus": {"all": 0, "running": 0}, - "assigned": {"all": 0, "running": 0}, "ratio": 0, "pools": []} + "assigned": {"all": 0, "running": 0}, "ratio": 0} self.assertEqual(self.line(given), "Given to VMs: 0 vCPUs (0.0:1), 0 bytes memory.") self.assertEqual(self.line(None), "The capacity could not be read.") diff --git a/computes/tests/test_cpu_graph.py b/computes/tests/test_cpu_graph.py index 30ed4d57..6e7a4657 100644 --- a/computes/tests/test_cpu_graph.py +++ b/computes/tests/test_cpu_graph.py @@ -139,7 +139,7 @@ class GraphScriptTestCase(TestCase): compute = Compute.objects.create(name="gs", hostname="192.0.2.1", login="u", password="p", type=1) with patch("computes.views.wvmHostDetails") as details, patch.object( Compute, "status", new_callable=PropertyMock, return_value=True - ), patch.object(Compute, "capacity", new=None): + ), patch.object(Compute, "capacity", new=None), patch("computes.views.host_pools", return_value=None): details.return_value.get_node_info.return_value = ("h", "x86_64", 1024, 2, "cpu", "qemu+tcp://h/system") details.return_value.get_cpu_topology.return_value = TOPOLOGY page = self.client.get(reverse("overview", args=[compute.id])).content.decode() diff --git a/computes/tests/test_overview.py b/computes/tests/test_overview.py index 2e33db30..7d06ff40 100644 --- a/computes/tests/test_overview.py +++ b/computes/tests/test_overview.py @@ -14,8 +14,11 @@ from computes.models import Compute GIB = 1024 ** 3 GIVEN = {"cpus": 8, "memory": 32 * GIB, "vcpus": {"all": 12, "running": 6}, "assigned": {"all": 24 * GIB, "running": 12 * GIB}, "ratio": 1.5, - "vms": {"running": 3, "paused": 1, "shutoff": 2, "other": 0}, - "pools": [{"name": "default", "status": 1, "size": 500 * GIB, "free": 120 * GIB}]} + "vms": {"running": 3, "paused": 1, "shutoff": 2, "other": 0}} + + +def pool(name, size_gib, free_gib, status=1): + return {"name": name, "status": status, "size": size_gib * GIB, "free": free_gib * GIB} def text(page, xpath): @@ -29,11 +32,13 @@ class OverviewTestCase(TestCase): type=1, details="rack 4") self.url = reverse("overview", args=[self.compute.id]) - def page(self, connected=True, given=GIVEN): - with patch.object(Compute, "status", new_callable=PropertyMock, return_value=connected), \ + def page(self, connected=True, given=GIVEN, pools=(pool("default", 500, 120),)): + with patch("computes.views.wvmStorages") as storages, \ + patch.object(Compute, "status", new_callable=PropertyMock, return_value=connected), \ patch.object(Compute, "connection_error", new_callable=PropertyMock, return_value="Connection Failed: 192.0.2.1 did not answer within 5 s"), \ patch.object(Compute, "capacity", new=given and {**given}): + storages.return_value.get_storages_info.return_value = [dict(p) for p in pools] response = self.client.get(self.url) self.assertEqual(response.status_code, 200) return html.fromstring(response.content) @@ -64,6 +69,28 @@ class OverviewTestCase(TestCase): self.assertIn("x86_64 kvm ยท qemu", software) self.assertTrue(page.xpath("//canvas[@id='cpuChart']")) + @patch("computes.views.wvmHostDetails") + def test_the_default_pool_shows_and_the_others_open_on_a_click(self, details): + details.return_value.get_node_info.return_value = ("h", "x86_64", GIB, 2, None, "qemu+tcp://192.0.2.1/system") + details.return_value.get_lib_version.side_effect = libvirtError("details are not this test's") + pools = [pool("nfs", 2000, 1000), pool("off", 10, 0, status=0), pool("default", 500, 120), + pool("backup", 900, 800), pool("iso", 100, 90)] + page = self.page(pools=pools) + storage = page.xpath("//div[@class='card-header'][normalize-space()='Storage']/..")[0] + self.assertIn("default 120.0 GB free of 500.0 GB", " ".join(" ".join(storage.xpath("./div/div//text()")).split())) + (more,) = storage.xpath(".//details") + self.assertEqual(" ".join(more.xpath("summary")[0].text_content().split()), "3 more pools") + # active pools only, by name after the default one + self.assertEqual(more.xpath(".//div[@class='small']/text()"), ["backup", "iso", "nfs"]) + + @patch("computes.views.wvmHostDetails") + def test_pools_that_cannot_be_read_leave_the_rest(self, details): + details.return_value.get_node_info.return_value = ("h", "x86_64", GIB, 2, None, "qemu+tcp://192.0.2.1/system") + with patch("computes.views.host_pools", return_value=None): + page = self.page() + self.assertIn("The storage pools could not be read.", text(page, "//div[@class='card-header'][normalize-space()='Storage']/..")) + self.assertTrue(page.xpath("//div[@class='card-header'][normalize-space()='VMs']")) + @patch("computes.views.wvmHostDetails") def test_a_host_the_panel_cannot_reach_keeps_its_page_with_the_reason(self, details): page = self.page(connected=False, given=None) diff --git a/computes/views.py b/computes/views.py index 827e7320..45210e48 100644 --- a/computes/views.py +++ b/computes/views.py @@ -13,6 +13,7 @@ from libvirt import libvirtError from logs.audit import audit, changed_fields, snapshot from operations import claims from vrtManager.hostdetails import wvmHostDetails +from vrtManager.storage import wvmStorages from computes.forms import ( SocketComputeForm, @@ -44,16 +45,24 @@ def shown_capacity(compute): "over": assigned["all"] > memory} given["memory_percent"] = round(assigned["all"] * 100 / memory) if memory else 0 given["vm_count"] = sum(given["vms"].values()) - # the default pool (else the first) shows; the others (a host may - # have dozens) open on a click - pools = given["pools"] or [] - for pool in pools: - pool["used_percent"] = (pool["size"] - pool["free"]) * 100 / pool["size"] if pool["size"] else 0 - given["pool"] = pools[0] if pools else None - given["more_pools"] = pools[1:] return given +def host_pools(compute): + """The compute's active storage pools, measured now: the default one (else + the first) to show, the others (a host may have dozens) to open on a + click; or None when they cannot be read.""" + try: + storages = wvmStorages(compute.hostname, compute.login, compute.password, compute.type) + pools = [pool for pool in storages.get_storages_info() if pool["status"]] + except libvirtError: + return None + pools.sort(key=lambda pool: (pool["name"] != "default", pool["name"])) + for pool in pools: + pool["used_percent"] = (pool["size"] - pool["free"]) * 100 / pool["size"] if pool["size"] else 0 + return {"pool": pools[0] if pools else None, "more_pools": pools[1:]} + + @superuser_only def computes(request): """The computes, with what each has given its VMs.""" @@ -96,6 +105,7 @@ def overview(request, compute_id): except libvirtError as error: context["read_error"] = str(error) context["given"] = shown_capacity(compute) + context["storage"] = host_pools(compute) return render(request, "overview.html", context) diff --git a/vrtManager/storage.py b/vrtManager/storage.py index 03285258..243dc229 100644 --- a/vrtManager/storage.py +++ b/vrtManager/storage.py @@ -7,11 +7,18 @@ from vrtManager.connection import wvmConnect class wvmStorages(wvmConnect): def get_storages_info(self): + """Each pool's state, type, volume count, size and free space. An active + pool is refreshed first: libvirt measures its space only when it is + refreshed, so two hosts sharing a pool's storage told different free + space, each as of its own last refresh.""" get_storages = self.get_storages() storages = [] for pool in get_storages: stg = self.get_storage(pool) stg_status = stg.isActive() + if stg_status: + with contextlib.suppress(libvirt.libvirtError): # refused while a volume is cloned in it + stg.refresh(0) stg_type = util.get_xml_path(stg.XMLDesc(0), "/pool/@type") stg_vol = len(stg.listVolumes()) if stg_status else None _state, stg_size, _allocation, stg_free = stg.info() diff --git a/vrtManager/tests/test_host_reads.py b/vrtManager/tests/test_host_reads.py index f544fb28..137d4355 100644 --- a/vrtManager/tests/test_host_reads.py +++ b/vrtManager/tests/test_host_reads.py @@ -196,3 +196,24 @@ class StoragesInfoTestCase(unittest.TestCase): conn.get_storage = lambda name: pool (info,) = conn.get_storages_info() self.assertEqual((info["name"], info["size"], info["free"]), ("default", 500, 120)) + # measured now: libvirt keeps a pool's space as of its last refresh + self.assertEqual([c[0] for c in pool.mock_calls[:2]], ["isActive", "refresh"]) + + def test_a_pool_that_refuses_a_refresh_tells_its_last_space(self): + conn = connection(wvmStorages) + pool = MagicMock(**{"isActive.return_value": 1, "XMLDesc.return_value": "", + "listVolumes.return_value": [], "info.return_value": [2, 500, 380, 120], + "refresh.side_effect": libvirtError("pool is busy: a volume is cloned")}) + conn.get_storages = lambda: ["default"] + conn.get_storage = lambda name: pool + (info,) = conn.get_storages_info() + self.assertEqual(info["free"], 120) + + def test_an_inactive_pool_is_not_refreshed(self): + conn = connection(wvmStorages) + pool = MagicMock(**{"isActive.return_value": 0, "XMLDesc.return_value": "", + "info.return_value": [0, 0, 0, 0]}) + conn.get_storages = lambda: ["off"] + conn.get_storage = lambda name: pool + conn.get_storages_info() + pool.refresh.assert_not_called()