feat(networks): the network pages on Bootstrap's cards, tables and dialogs

As the storage pages; reviewed with Codex.

- The list is a card per network: its name with Active or Stopped and
  Autostart badges, its type (network_mode: Isolated, NAT, Routed,
  Open, Bridge, Macvtap, SR-IOV), its device, and each address with
  its DHCP range; a bridge or macvtap network's addresses come from
  the outside network.
- A network's page: its heading starts or stops it, the menu sets
  autostart and deletes a stopped one. A Network card tells its state,
  type, bridge or interface, MAC and what the type does; a card per
  address (every <ip>, each with its DHCP range dialog, which libvirt
  takes on a stopped network); its fixed addresses of every subnet in
  one table, searched, each with Edit and Delete, one dialog adding
  and editing them; the VMs whose definition names it, connected or
  configured, linked (wvmNetwork.get_vms, one domain listing, a VM
  undefined meanwhile left out); its
  bandwidth limits where libvirt sets them (not on macvtap or SR-IOV
  networks), in KiB/s, from the saved definition with a note that a
  running network takes them when restarted; its XML in an accordion.
  The page reads the running and the saved definition once each, and
  only to render itself: its actions, which all redirect, read nothing
  of it first.
- The New network dialog is one form, as the New pool one: the type
  as radio cards (the radio_card.html include, now shared by both),
  the chosen type's fields only, IPv6 with its box, a bridge or
  interface suggested from the host's (get_host_interfaces, not the
  bridges libvirt made for its own networks); a refused network or
  one libvirt refuses opens the dialog again with what was entered,
  the reasons beside the fields and focus on the first.
- What the New pool and New network dialogs share is one script,
  static/js/choice_dialog.js: the radio cards show their fieldsets
  (data-kinds) and a refused form opens again focused on its first
  refused field; each dialog keeps only its own changes.
- The fixed-address dialogs per family go to the page's own dialog.

networks/tests/test_audit_rows.py

@@ -96,5 +96,5 @@ class NetworkRowTests(TestCase):
                                "dhcp4": "on"})
         self.client.post(url, {"create": "", "name": "bad name!", "forward": "nat"})  # invalid form
         response = self.client.post(url, {"create": "", "name": "lab", "forward": "nat", "subnet": "203.0.113.0/24"})
-        self.assertIn("* This host has a network of that name", [str(m) for m in get_messages(response.wsgi_request)])
+        self.assertContains(response, "This host has a network of that name")  # in the dialog, open again
         self.assertEqual(rows(), [("Network audit-net: add", True), ("Network lab: add: name already in use", False)])

networks/tests/test_fixed_address_form.py

@@ -1,31 +1,46 @@
-"""The fixed-address dialogs take real MAC addresses and host names: the
-browser refuses what their pattern attributes do not match."""
+"""The network page's fixed-address dialog takes real MAC addresses and host
+names: the browser refuses what its pattern attributes do not match. Runs
+on a private libvirt test driver."""
 
 import re
-from types import SimpleNamespace
+import tempfile
+from unittest.mock import patch
 
-from django.template.loader import render_to_string
-from django.test import SimpleTestCase
+import libvirt
+from computes.models import Compute
+from django.contrib.auth import get_user_model
+from django.test import TestCase
+from django.urls import reverse
 from lxml import html
+from networks.tests.driver import without_inactive_flag
+
+NODE = """<node><network><name>lab</name><bridge name='virbr9'/>
+  <ip address='192.0.2.1' netmask='255.255.255.0'><dhcp><range start='192.0.2.10' end='192.0.2.20'/></dhcp></ip>
+</network></node>"""
 
 
-def pattern(template, name):
-    superuser = SimpleNamespace(user=SimpleNamespace(is_superuser=True))
-    page = html.fromstring(render_to_string(template, {"request": superuser}))
-    return re.compile(page.xpath(f"//input[@name='{name}']/@pattern")[0])
-
-
-class FixedAddressFormTests(SimpleTestCase):
+class FixedAddressFormTests(TestCase):
     def test_macs_and_host_names_are_accepted(self):
-        mac = pattern("modify_ipv4_fixed_address.html", "mac")
-        for value in ("52:54:00:ab:cd:ef", "52:54:00:AB:CD:EF"):
+        node = tempfile.NamedTemporaryFile("w", suffix=".xml")
+        self.addCleanup(node.close)
+        node.write(NODE)
+        node.flush()
+        conn = libvirt.open(f"test://{node.name}")
+        self.addCleanup(conn.close)
+        without_inactive_flag(self)
+        compute = Compute.objects.create(name="fixed", hostname="localhost", login="", password="", type=4)
+        self.client.force_login(get_user_model().objects.create_superuser("fixed", "fixed@example.com", "pw"))
+        with patch("vrtManager.connection.connection_manager.get_connection", return_value=conn):
+            page = html.fromstring(self.client.get(reverse("network", args=[compute.id, "lab"])).content)
+        dialog = page.get_element_by_id("FixedAddress")
+        mac = re.compile(dialog.xpath(".//input[@name='mac']/@pattern")[0])
+        name = re.compile(dialog.xpath(".//input[@name='name']/@pattern")[0])
+        for value in ("52:54:00:ab:cd:ef", "52:54:00:AB:CD:EF", "0:1:0:1:2a:3b"):
             with self.subTest(mac=value):
                 self.assertTrue(mac.fullmatch(value))
-        for template in ("modify_ipv4_fixed_address.html", "modify_ipv6_fixed_address.html"):
-            name = pattern(template, "name")
-            for value in ("web-01", "db.lan", "vm_1"):
-                with self.subTest(template=template, name=value):
-                    self.assertTrue(name.fullmatch(value))
-            for value in ("a b", 'x"<'):
-                with self.subTest(template=template, refused=value):
-                    self.assertFalse(name.fullmatch(value))
+        for value in ("web-01", "db.lan", "vm_1"):
+            with self.subTest(name=value):
+                self.assertTrue(name.fullmatch(value))
+        for value in ("a b", 'x"<'):
+            with self.subTest(refused=value):
+                self.assertFalse(name.fullmatch(value))

networks/tests/test_network_changes.py

@@ -1,10 +1,10 @@
-"""A network's addresses are read from its XML however libvirt writes it,
-each <ip> by its own index; a change starts from the saved definition, so
-two in a row keep each other; a fixed address is found by the address it
-had and checked against its subnet; the XML editor defines only this
-network; a network that does not start is not left defined. Runs on a
-private libvirt test driver: "test:///default" is shared by the whole
-process."""
+"""A network's addresses are read from its XML however libvirt writes it, each
+<ip> by its own index; a page reads each definition once; a change starts
+from the saved definition, so two in a row keep each other; a fixed address
+is found by the address it had and checked against its subnet; the XML
+editor defines only this network; a network that does not start is not left
+defined. Runs on a private libvirt test driver: "test:///default" is shared
+by the whole process."""
 
 import tempfile
 from unittest.mock import MagicMock, patch
@@ -61,12 +61,13 @@ class NetworkChangeTests(SimpleTestCase):
     def saved(self):
         return etree.fromstring(self.conn.networkLookupByName("lab").XMLDesc(libvirt.VIR_NETWORK_XML_INACTIVE))
 
-    def test_the_page_reads_the_xml_once(self):
+    def test_the_page_reads_each_definition_once(self):
+        # the running one for its addresses, the saved one for its limits and the XML editor
         net = self.network()
         with patch.object(net.net, "XMLDesc", wraps=net.net.XMLDesc) as xml:
             net.get_ip_networks(), net.get_network_forward(), net.get_network_mac(), net.get_qos()
-            net.get_bridge_device()
-        self.assertEqual(xml.call_count, 1)
+            net.get_bridge_device(), net.get_mode(), net.saved_xml
+        self.assertEqual(xml.call_count, 2)
 
     def test_the_ipv4_address_after_an_ipv6_one_keeps_its_index(self):
         ipv6, ipv4 = self.network().get_ip_networks()
@@ -151,7 +152,7 @@ class NetworkChangeTests(SimpleTestCase):
     def test_the_xml_editor_takes_an_encoding_declaration(self):
         self.conn.networkLookupByName("lab").destroy()
         net = self.network()
-        net.edit_network('<?xml version="1.0" encoding="UTF-8"?>\n' + net.config_xml().replace("virbr9", "virbr6"))
+        net.edit_network('<?xml version="1.0" encoding="UTF-8"?>\n' + net.saved_xml.replace("virbr9", "virbr6"))
         self.assertEqual(self.saved().xpath("bridge/@name"), ["virbr6"])
 
     def test_a_fixed_address_is_of_the_subnet_and_its_own(self):
@@ -165,7 +166,7 @@ class NetworkChangeTests(SimpleTestCase):
     def test_the_xml_editor_defines_this_network_only(self):
         self.conn.networkLookupByName("lab").destroy()  # its definition read back at once
         net = self.network()
-        xml = net.config_xml()
+        xml = net.saved_xml
         for other, why in ((xml.replace("<name>lab</name>", "<name>other</name>"), "must define network lab"),
                            (xml.replace(net.net.UUIDString(), "11111111-2222-3333-4444-555555555555"), "keep the network's UUID"),
                            ("<network>", "not valid")):

networks/tests/test_network_pages.py

@@ -0,0 +1,104 @@
+"""The network pages: a card per network with its mode, device and
+addresses (none of its own for a bridge); a network's page with an address
+card per <ip>, its fixed addresses in one table, the VMs on it, bandwidth
+limits only where libvirt sets them; the new network dialog opens again
+with what was entered and why. Runs on a private libvirt test driver."""
+
+import tempfile
+from unittest.mock import patch
+
+import libvirt
+from computes.models import Compute
+from django.contrib.auth import get_user_model
+from django.test import TestCase
+from django.urls import reverse
+from instances.models import Instance
+from lxml import html
+from networks.tests.driver import without_inactive_flag
+
+NODE = """<node>
+  <network><name>lab</name><forward mode='nat'/><bridge name='virbr9'/>
+    <ip address='192.0.2.1' netmask='255.255.255.0'><dhcp><range start='192.0.2.10' end='192.0.2.20'/>
+      <host mac='52:54:00:00:00:31' name='web' ip='192.0.2.31'/></dhcp></ip>
+    <ip family='ipv6' address='2001:db8:1::1' prefix='64'><dhcp><host id='0:1:0:1' ip='2001:db8:1::31'/></dhcp></ip>
+  </network>
+  <network><name>lan</name><forward mode='bridge'/><bridge name='br0'/></network>
+  <network><name>direct</name><forward mode='bridge'><interface dev='eth0'/></forward></network>
+  <domain type='test'><name>web</name><memory>1024</memory><os><type>hvm</type></os>
+    <devices><interface type='network'><source network='lab'/></interface></devices></domain>
+</node>"""
+
+
+class NetworkPageTests(TestCase):
+    def setUp(self):
+        node = tempfile.NamedTemporaryFile("w", suffix=".xml")
+        self.addCleanup(node.close)
+        node.write(NODE)
+        node.flush()
+        self.conn = libvirt.open(f"test://{node.name}")
+        self.addCleanup(self.conn.close)
+        patcher = patch("vrtManager.connection.connection_manager.get_connection", return_value=self.conn)
+        patcher.start()
+        self.addCleanup(patcher.stop)
+        without_inactive_flag(self)
+        self.compute = Compute.objects.create(name="np", hostname="localhost", login="", password="", type=4)
+        self.client.force_login(get_user_model().objects.create_superuser("np", "np@example.com", "pw"))
+
+    def page(self, response):
+        self.assertEqual(response.status_code, 200)
+        return html.fromstring(response.content)
+
+    def text(self, element):
+        return " ".join(" ".join(element.itertext()).split())
+
+    def test_the_list_tells_each_network_mode_and_addresses(self):
+        page = self.page(self.client.get(reverse("networks", args=[self.compute.id])))
+        cards = {card.xpath("string(.//div[contains(@class,'card-header')]//a)"): self.text(card)
+                 for card in page.xpath("//div[@class='card h-100']")}
+        self.assertIn("Type NAT Device virbr9 IPv4 192.0.2.0/24 DHCP 192.0.2.10 – 192.0.2.20 "
+                      "IPv6 2001:db8:1::/64 DHCP: fixed addresses only", cards["lab"])
+        self.assertIn("Type Bridge Device br0 Addresses from the outside network", cards["lan"])
+        self.assertIn("Type Macvtap Device eth0 Addresses from the outside network", cards["direct"])
+
+    def test_a_network_page(self):
+        vm = Instance.objects.create(compute=self.compute, name="web", uuid=self.conn.lookupByName("web").UUIDString())
+        page = self.page(self.client.get(reverse("network", args=[self.compute.id, "lab"])))
+        # an address card per <ip>, the fixed addresses of both in one table
+        self.assertEqual(len(page.xpath("//button[@data-bs-target='#DhcpRange']")), 1)
+        rows = [self.text(row) for row in page.xpath("//tbody[@class='searchable']/tr")]
+        self.assertEqual([row.split(" Edit")[0] for row in rows], ["192.0.2.31 52:54:00:00:00:31 web", "2001:db8:1::31 0:1:0:1 –"])
+        self.assertEqual(page.xpath("//tbody/tr/td//input[@name='ip_index']/@value"), ["0", "1"])
+        # the VMs on it, linked
+        self.assertEqual(page.xpath(f"//a[@href='{reverse('instances:instance', args=[vm.id])}']/text()"), ["web"])
+        self.assertTrue(page.xpath("//div[@id='QosLimit']"))
+
+    def test_a_bridge_or_macvtap_network_has_no_addresses_of_libvirt(self):
+        for name, qos in (("lan", True), ("direct", False)):  # libvirt limits a host bridge, not macvtap
+            with self.subTest(name=name):
+                page = self.page(self.client.get(reverse("network", args=[self.compute.id, name])))
+                self.assertIn("take their addresses from the outside network", self.text(page))
+                self.assertFalse(page.xpath("//div[@id='FixedAddress']"))
+                self.assertEqual(bool(page.xpath("//div[@id='QosLimit']")), qos)
+
+    def test_a_vm_undefined_meanwhile_is_left_out(self):
+        error = libvirt.libvirtError("Domain not found")
+        error.get_error_code = lambda: libvirt.VIR_ERR_NO_DOMAIN
+        with patch.object(libvirt.virDomain, "isActive", side_effect=error):
+            page = self.page(self.client.get(reverse("network", args=[self.compute.id, "lab"])))
+        self.assertIn("No VM is defined on this network.", self.text(page))
+
+    def test_an_action_does_not_read_the_page(self):
+        # every action redirects: the VMs on the network are not scanned for it
+        with patch("vrtManager.network.wvmNetwork.get_vms") as get_vms:
+            response = self.client.post(reverse("network", args=[self.compute.id, "lab"]), {"set_autostart": ""})
+        self.assertEqual(response.status_code, 302)
+        get_vms.assert_not_called()
+
+    def test_a_refused_network_opens_the_dialog_again(self):
+        page = self.page(self.client.post(reverse("networks", args=[self.compute.id]), {
+            "create": "", "name": "lab2", "forward": "route", "subnet": "192.0.2.0/25"}))
+        dialog = page.get_element_by_id("AddNetPool")
+        self.assertIsNotNone(dialog.get("data-open"))
+        self.assertEqual(page.xpath("//input[@name='forward'][@checked]/@value"), ["route"])
+        self.assertEqual(page.get_element_by_id("net_subnet").get("aria-invalid"), "true")
+        self.assertIn("Network lab has 192.0.2.0/24", page.get_element_by_id("net_subnet_error").text_content())