diff --git a/computes/views.py b/computes/views.py index 2e175035..69d759c3 100644 --- a/computes/views.py +++ b/computes/views.py @@ -9,6 +9,7 @@ from django.urls import reverse from django.utils import timezone from django.utils.translation import gettext_lazy as _ from instances.models import Instance +from instances.utils import read_list_rows from libvirt import libvirtError from logs.audit import audit, changed_fields, snapshot from operations import claims @@ -134,7 +135,8 @@ def instances(request, compute_id): compute = get_object_or_404(Compute, pk=compute_id) utils.refresh_instance_database(compute) - instances = Instance.objects.filter(compute=compute).order_by("name").prefetch_related("userinstance_set") + instances = list(Instance.objects.filter(compute=compute).order_by("name").prefetch_related("userinstance_set")) + read_list_rows(instances) return render( request, "computes/instances.html", diff --git a/instances/templates/allinstances.html b/instances/templates/allinstances.html index ae62ccb2..2e83df3d 100644 --- a/instances/templates/allinstances.html +++ b/instances/templates/allinstances.html @@ -39,7 +39,7 @@ {% endif %}
- {% if instances.exists %} + {% if instances %} {# the state filter: table_filter.js counts each state's VMs the search finds #} {% endif %} - {% if not instances.exists %} + {% if not instances %}

{% trans "There are no VMs here yet." %}

{% elif app_settings.VIEW_INSTANCES_LIST_STYLE == 'grouped' and request.user.is_superuser %} {% include 'allinstances_index_grouped.html' %} diff --git a/instances/tests/test_list_rpc.py b/instances/tests/test_list_rpc.py index ac8c5b0e..88aa2df4 100644 --- a/instances/tests/test_list_rpc.py +++ b/instances/tests/test_list_rpc.py @@ -1,8 +1,11 @@ -"""Instance lists ask libvirt for a VM's info once per row.""" +"""Instance lists read their VMs once a host (get_domain_rows): the states in +one call and each VM's live XML once, no lookup or info call a VM; on +libvirt's test driver.""" -from types import SimpleNamespace +from collections import Counter from unittest.mock import PropertyMock, patch +import libvirt from appsettings.models import AppSettings from computes.models import Compute from django.contrib.auth import get_user_model @@ -10,78 +13,71 @@ from django.test import TestCase from django.urls import reverse from instances.models import Instance -# virDomainGetInfo: state, max memory KiB, memory KiB, vCPUs, CPU time -INFO = [1, 4194304, 2097152, 3, 0] +DOMAIN = """{name}{uuid}{name} title +409620484 +hvm""" +VMS = {"vm-a": "11111111-1111-1111-1111-111111111111", "vm-b": "22222222-2222-2222-2222-222222222222"} -class FakeDomain: - # a plain class: Django templates do not call MagicMock attributes - def __init__(self): - self.info_calls = 0 - self.xml_calls = 0 - - def info(self): - self.info_calls += 1 - return INFO - - def XMLDesc(self, flags=0): - self.xml_calls += 1 - return "2097152" - - -class ListInfoCallTests(TestCase): +class ListReadTests(TestCase): def setUp(self): self.client.force_login( get_user_model().objects.create_superuser(username="rpc-admin", password="pwd", email="rpc@example.com") ) + self.conn = libvirt.open("test:///default") + self.addCleanup(self.conn.close) self.compute = Compute.objects.create(name="rpc-compute", hostname="localhost", type=4, login="", password="") - Instance.objects.create(compute=self.compute, name="vm-a", uuid="11111111-1111-1111-1111-111111111111") - Instance.objects.create(compute=self.compute, name="vm-b", uuid="22222222-2222-2222-2222-222222222222") - self.domains = [] - - def proxy(self, *args, **kwargs): - domain = FakeDomain() - self.domains.append(domain) - return SimpleNamespace( - instance=domain, - get_uuid=lambda: "uuid", - get_title=lambda: domain.XMLDesc() and "", - get_cur_memory=lambda: int(domain.XMLDesc().split(">")[4].split("<")[0]) // 1024, - ) + for name, uuid in VMS.items(): + domain = self.conn.defineXML(DOMAIN.format(name=name, uuid=uuid)) + domain.create() + self.addCleanup(domain.undefine) + self.addCleanup(domain.destroy) + Instance.objects.create(compute=self.compute, name=name, uuid=uuid) def get(self, url): - compute_patches = {"status": True, "cpu_count": 1, "ram_size": 1024, "ram_usage": 0} - with patch("instances.views.utils.refr"), patch("computes.views.utils.refresh_instance_database"), patch( - "instances.models.wvmInstance", side_effect=self.proxy - ): - patches = [patch.object(Compute, k, new_callable=PropertyMock, return_value=v) for k, v in compute_patches.items()] - for p in patches: - p.start() - try: - response = self.client.get(url) - finally: - for p in patches: - p.stop() - self.assertEqual(response.status_code, 200) - return response + calls = Counter() - def assert_one_info_per_vm(self, response): - self.assertEqual(len(self.domains), 2, "one proxy per VM row") - for domain in self.domains: - self.assertEqual(domain.info_calls, 1) - self.assertEqual(domain.xml_calls, 1, "only the title reads the XML") - self.assertContains(response, "2048 MiB", count=2) + def counted(cls, name): + real = getattr(cls, name) + + def call(*args, **kwargs): + calls[name] += 1 + return real(*args, **kwargs) + + return patch.object(cls, name, call) + + patches = [patch("instances.views.utils.refr"), patch("computes.views.utils.refresh_instance_database"), + patch("vrtManager.connection.connection_manager.get_connection", return_value=self.conn), + counted(libvirt.virDomain, "info"), counted(libvirt.virDomain, "XMLDesc"), + counted(libvirt.virConnect, "lookupByUUIDString")] + # the grouped list's host line; the test driver has no memory stats + patches += [patch.object(Compute, k, new_callable=PropertyMock, return_value=v) + for k, v in {"cpu_count": 1, "ram_size": 1024, "ram_usage": 0}.items()] + for p in patches: + p.start() + try: + response = self.client.get(url) + finally: + for p in reversed(patches): + p.stop() + self.assertEqual(response.status_code, 200) + return response, calls + + def assert_read_once_a_host(self, url): + response, calls = self.get(url) + # every domain of the host once (the test driver's own, and other tests', too) + self.assertEqual(calls, {"XMLDesc": len(self.conn.listAllDomains())}) + self.assertContains(response, "2048 MiB", count=len(VMS)) self.assertContains(response, "Active") + self.assertContains(response, "vm-a title") def test_nongrouped_index(self): AppSettings.objects.filter(key="VIEW_INSTANCES_LIST_STYLE").update(value="nongrouped") - self.assert_one_info_per_vm(self.get(reverse("instances:index"))) + self.assert_read_once_a_host(reverse("instances:index")) def test_grouped_index(self): AppSettings.objects.filter(key="VIEW_INSTANCES_LIST_STYLE").update(value="grouped") - self.assert_one_info_per_vm(self.get(reverse("instances:index"))) + self.assert_read_once_a_host(reverse("instances:index")) def test_compute_instances(self): - # this page shows the maximum memory, as before - # the VM's current memory, as on the Instances page, not its maximum - self.assert_one_info_per_vm(self.get(reverse("instances", args=[self.compute.id]))) + self.assert_read_once_a_host(reverse("instances", args=[self.compute.id])) diff --git a/instances/tests/test_list_unreachable.py b/instances/tests/test_list_unreachable.py index f0366818..6d831ac2 100644 --- a/instances/tests/test_list_unreachable.py +++ b/instances/tests/test_list_unreachable.py @@ -17,19 +17,6 @@ ERROR = "Connection Failed: No route to host" UP_INFO = [1, 4194304, 2097152, 3, 0] -class Domain: - def info(self): - return UP_INFO - - -def live(*args, **kwargs): - return SimpleNamespace(instance=Domain(), get_title=lambda: "", get_uuid=lambda: "uuid") - - -def unreachable(*args, **kwargs): - raise OperationError(ERROR) - - class UnreachableListTests(TestCase): def setUp(self): self.client.force_login( @@ -39,7 +26,15 @@ class UnreachableListTests(TestCase): self.vm_a = Instance.objects.create(compute=self.compute, name="vm-a", uuid="11111111-1111-1111-1111-111111111111") self.vm_b = Instance.objects.create(compute=self.compute, name="vm-b", uuid="22222222-2222-2222-2222-222222222222") - def get(self, url, wvm_instance, up=False): + def get(self, url, rows=None): + """The page with the host down, or up with these VM rows (get_domain_rows), + or up with the error its listing raises.""" + up = rows is not None + + def domain_rows(): + if isinstance(rows, Exception): + raise rows + return rows def no_live_host_data(_self): raise AssertionError("host data read for a host that is down") @@ -53,7 +48,8 @@ class UnreachableListTests(TestCase): patches += [ patch("instances.views.utils.refr"), patch("computes.views.utils.refresh_instance_database"), - patch("instances.models.wvmInstance", side_effect=wvm_instance), + patch.object(Compute, "proxy", new_callable=PropertyMock, + return_value=SimpleNamespace(get_domain_rows=domain_rows)), ] for p in patches: p.start() @@ -75,35 +71,36 @@ class UnreachableListTests(TestCase): def test_nongrouped_index_lists_the_vms_of_a_down_host(self): AppSettings.objects.filter(key="VIEW_INSTANCES_LIST_STYLE").update(value="nongrouped") - response = self.get(reverse("instances:index"), unreachable) + response = self.get(reverse("instances:index")) self.assert_unknown(response, [self.vm_a, self.vm_b]) self.assertContains(response, ERROR) def test_grouped_index_lists_the_vms_of_a_down_host(self): AppSettings.objects.filter(key="VIEW_INSTANCES_LIST_STYLE").update(value="grouped") - response = self.get(reverse("instances:index"), unreachable) + response = self.get(reverse("instances:index")) self.assert_unknown(response, [self.vm_a, self.vm_b]) self.assertContains(response, "Not connected") def test_one_unreadable_vm_does_not_hide_the_others(self): AppSettings.objects.filter(key="VIEW_INSTANCES_LIST_STYLE").update(value="nongrouped") - def vm_b_gone(*args, uuid=None, **kwargs): - if uuid == self.vm_b.uuid: - raise OperationError("Domain not found") - return live() - - response = self.get(reverse("instances:index"), vm_b_gone, up=True) + # vm-b is gone from its host + response = self.get(reverse("instances:index"), {self.vm_a.uuid: (UP_INFO, "")}) self.assert_unknown(response, [self.vm_b]) self.assertContains(response, reverse("instances:poweroff", args=[self.vm_a.id])) + def test_a_host_whose_listing_fails_shows_its_vms_unknown(self): + AppSettings.objects.filter(key="VIEW_INSTANCES_LIST_STYLE").update(value="nongrouped") + response = self.get(reverse("instances:index"), OperationError(ERROR)) + self.assert_unknown(response, [self.vm_a, self.vm_b]) + def test_compute_instances_page_of_a_down_host(self): - response = self.get(reverse("instances", args=[self.compute.id]), unreachable) + response = self.get(reverse("instances", args=[self.compute.id])) self.assert_unknown(response, [self.vm_a, self.vm_b]) self.assertContains(response, ERROR) def test_compute_instances_page_does_not_claim_a_down_host_is_empty(self): Instance.objects.all().delete() - response = self.get(reverse("instances", args=[self.compute.id]), unreachable) + response = self.get(reverse("instances", args=[self.compute.id])) self.assertNotContains(response, "have any Instances") self.assertContains(response, ERROR) diff --git a/instances/tests/test_states.py b/instances/tests/test_states.py index 2164fba8..41781530 100644 --- a/instances/tests/test_states.py +++ b/instances/tests/test_states.py @@ -54,16 +54,13 @@ class StateTests(TestCase): AppSettings.objects.filter(key="VIEW_INSTANCES_LIST_STYLE").update(value="nongrouped") def get(self, user, url): - by_uuid = {vm.uuid: state for state, vm in self.vms.items()} - - def wvm(*args, uuid=None, **kwargs): - return SimpleNamespace(instance=Domain(by_uuid[uuid]), get_title=lambda: "", get_uuid=lambda: uuid) + rows = {vm.uuid: (Domain(state).info(), "") for state, vm in self.vms.items()} self.client.force_login(user) - props = {"status": True, "connection_error": None, "cpu_count": 1, "ram_size": 1, "ram_usage": 0} + props = {"status": True, "connection_error": None, "cpu_count": 1, "ram_size": 1, "ram_usage": 0, + "proxy": SimpleNamespace(get_domain_rows=lambda: rows)} patches = [patch.object(Compute, k, new_callable=PropertyMock, return_value=v) for k, v in props.items()] - patches += [patch("instances.views.utils.refr"), patch("computes.views.utils.refresh_instance_database"), - patch("instances.models.wvmInstance", side_effect=wvm)] + patches += [patch("instances.views.utils.refr"), patch("computes.views.utils.refresh_instance_database")] for p in patches: p.start() try: diff --git a/instances/tests/test_vm_states.py b/instances/tests/test_vm_states.py index 6f54e35f..41771b24 100644 --- a/instances/tests/test_vm_states.py +++ b/instances/tests/test_vm_states.py @@ -79,9 +79,9 @@ class VmStatesTestCase(TestCase): def test_the_lists_poll_it(self): self.client.force_login(self.admin) with patch.object(Compute, "status", property(lambda c: c.pk == self.up.pk)), \ - patch.object(Compute, "proxy", new_callable=PropertyMock, return_value=MagicMock()), \ - patch("instances.views.utils.refr"), patch("computes.views.utils.refresh_instance_database"), \ - patch.object(Instance, "proxy", new_callable=PropertyMock, return_value=MagicMock()): + patch.object(Compute, "proxy", new_callable=PropertyMock, + return_value=MagicMock(**{"get_domain_rows.return_value": {}})), \ + patch("instances.views.utils.refr"), patch("computes.views.utils.refresh_instance_database"): for url, states_url in ((reverse("instances:index"), reverse("instances:states")), (reverse("instances", args=[self.up.pk]), reverse("instances:states") + f"?compute={self.up.pk}")): diff --git a/instances/utils.py b/instances/utils.py index ee1ce19a..2ce3f5af 100644 --- a/instances/utils.py +++ b/instances/utils.py @@ -352,6 +352,20 @@ def record_migration(instance, new_compute): instance.save() +def read_list_rows(instances): + """Each VM's info and title for a VM list, read once a host + (get_domain_rows), not three calls a VM; a VM whose host or whose + domain cannot be read has no info: the list shows it unknown.""" + rows = {} + for compute in {inst.compute_id: inst.compute for inst in instances}.values(): + try: + rows[compute.pk] = compute.proxy.get_domain_rows() if compute.status is True else {} + except libvirtError: + rows[compute.pk] = {} + for inst in instances: + inst.info, inst.title = rows[inst.compute_id].get(inst.uuid, (None, "")) + + def vm_states(user, computes): """{compute pk: {"ok": True, "vms": {uuid: [name, state]}}, or {"ok": False} for a host that cannot be read}: what the VM lists poll, read from libvirt; diff --git a/instances/views.py b/instances/views.py index ddd9efed..a039af04 100755 --- a/instances/views.py +++ b/instances/views.py @@ -81,22 +81,25 @@ def index(request): computes = list(Compute.objects.all().order_by("name")) for compute in computes: utils.refr(compute) - # each host's VMs by name: the database keeps no order of its own - prefetch_related_objects(computes, Prefetch("instance_set", queryset=Instance.objects.order_by("name")), - "instance_set__userinstance_set__user") show_hosts = request.user.is_superuser or request.user.has_perm("instances.view_instances") own_hosts_down = False - if show_hosts: - instances = Instance.objects.order_by("name").prefetch_related("userinstance_set__user") + if request.user.is_superuser and app_settings.VIEW_INSTANCES_LIST_STYLE == "grouped": + # each host's VMs by name: the database keeps no order of its own + prefetch_related_objects(computes, Prefetch("instance_set", queryset=Instance.objects.order_by("name")), + "instance_set__userinstance_set__user") + instances = [inst for compute in computes for inst in compute.instance_set.all()] + elif show_hosts: + instances = list(Instance.objects.order_by("name").prefetch_related("userinstance_set__user")) else: - instances = Instance.objects.filter( + instances = list(Instance.objects.filter( userinstance__user=request.user - ).order_by("name").prefetch_related("userinstance_set__user") + ).order_by("name").prefetch_related("userinstance_set__user")) # other users get no host names or connection errors, only whether # a host of their own VMs is down - own = set(instances.values_list("compute_id", flat=True)) + own = {inst.compute_id for inst in instances} own_hosts_down = any(c.status is not True for c in computes if c.pk in own) + utils.read_list_rows(instances) return render( request, diff --git a/vrtManager/connection.py b/vrtManager/connection.py index 56ece172..746cb56e 100644 --- a/vrtManager/connection.py +++ b/vrtManager/connection.py @@ -1098,6 +1098,23 @@ class wvmConnect(object): return {dom.UUIDString(): [dom.name(), stats["state.state"]] for dom, stats in self.wvm.getAllDomainStats(libvirt.VIR_DOMAIN_STATS_STATE)} + def get_domain_rows(self): + """{uuid: (info, title)} of every domain, as the VM lists show them: + info in virDomainGetInfo's shape (state, max memory KiB, memory KiB, + vCPUs), the states in one call and the rest from each domain's live + XML, which the title needs anyway. A domain gone meanwhile is left out.""" + rows = {} + for dom, stats in self.wvm.getAllDomainStats(libvirt.VIR_DOMAIN_STATS_STATE): + try: + doc = etree.fromstring(dom.XMLDesc(0)) + except libvirtError: + continue + vcpu = doc.find("vcpu") + info = [stats["state.state"], int(doc.findtext("memory")), int(doc.findtext("currentMemory")), + int(vcpu.get("current", vcpu.text))] + rows[dom.UUIDString()] = (info, doc.findtext("title") or "") + return rows + def domain_trees(self, running_only=False): """Each domain's name with its definitions, parsed: the live one, and for a persistent domain the one saved for its next boot too, read