refactor(vrtManager): a VM's addresses are read once a page, not once an object

wvmInstance kept the guest agent's and the ARP table's addresses from
the first NIC it was asked about, and never read them again on that
object; it carried a lease lookup that was never filled.

The addresses are now one {MAC: (IPv4, IPv6)} read, kept like the
domain XML only inside a cached_reads block: the VM page, which reads
its NICs twice, asks the agent once (it may take its whole timeout),
and a read outside a page's block is a new one. The agent's addresses
still come before the ARP table's.

instances/tests/test_live_guest.py

@@ -16,6 +16,7 @@ import unittest
 
 import libvirt
 import libvirt_qemu
+from vrtManager.instance import agent_interfaces
 from computes.models import Compute
 from django.contrib.auth import get_user_model
 from django.test import TestCase
@@ -267,9 +268,7 @@ class LiveGuestTestCase(TestCase):
         # The NIC is matched by MAC in the agent's report. Whether the guest has
         # an address depends on its network configuration, not on WebVirtCloud.
         mac = self.live(dom).xpath("devices/interface/mac/@address")[0]
-        proxy = inst.proxy
+        self.assertIn(mac, [nic["hwaddr"] for nic in agent_interfaces(inst.proxy.instance).values()])
-        proxy.refresh_interface_addresses()
-        self.assertIn(mac, [nic["hwaddr"] for nic in proxy._ip_cache["qemuga"].values()])
 
     def test_poweroff_shuts_the_guest_down(self):
         dom, inst = self.boot("goff")

vrtManager/instance.py

@@ -613,7 +613,6 @@ class wvmInstance(wvmConnect):
 
     def __init__(self, host, login, passwd, conn, vname, uuid=None):
         wvmConnect.__init__(self, host, login, passwd, conn)
-        self._ip_cache = None
         # A known VM is found by UUID: a name can be renamed or swapped
         # outside WebVirtCloud. No fallback to the name.
         self.instance = self.wvm.lookupByUUIDString(uuid) if uuid else self.get_instance(vname)
@@ -787,61 +786,38 @@ class wvmInstance(wvmConnect):
         return range(1, int(pcpus + 1))
 
     def get_interface_addresses(self, iface_mac):
-        if self._ip_cache is None:
+        """The running VM's addresses of the NIC with iface_mac: (IPv4, IPv6
-            self.refresh_interface_addresses()
+        with prefix), from the guest agent when it has some, else from the
+        host's ARP table; (None, None) when neither has one. Read once in a
+        cached_reads block: the agent may take its whole timeout."""
+        return self._cached("addresses", self._read_addresses).get(iface_mac, (None, None))
 
-        qemuga = self._ip_cache["qemuga"]
+    def _read_addresses(self):
-        arp = self._ip_cache["arp"]
+        """{MAC: (IPv4 list, IPv6 list)}: each MAC's addresses from the first source that has some."""
-        leases = []
+        if self.get_status() != 1:
-
+            return {}
-        def extract_dom(info):
+        sources = []
-            ipv4 = []
+        if self.is_agent_ready():
-            ipv6 = []
+            with contextlib.suppress(libvirtError):
-            for addrs in info.values():
+                sources.append(agent_interfaces(self.instance))
-                if addrs["hwaddr"] != iface_mac:
+        with contextlib.suppress(libvirtError):
-                    continue
+            sources.append(self.instance.interfaceAddresses(VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP))
-                if not addrs["addrs"]:
+        found = {}
-                    continue
+        for interfaces in sources:
-                for addr in addrs["addrs"]:
+            here = {}
+            for interface in interfaces.values():
+                if not interface["hwaddr"]:
+                    continue  # the loopback, a tunnel
+                ipv4, ipv6 = here.setdefault(interface["hwaddr"], ([], []))
+                for addr in interface["addrs"] or []:
                     if addr["type"] == 0:
                         ipv4.append(addr["addr"])
                     elif addr["type"] == 1 and not str(addr["addr"]).startswith("fe80"):
-                        ipv6.append(addr["addr"] + "/" + str(addr["prefix"]))
+                        ipv6.append(f"{addr['addr']}/{addr['prefix']}")
-            return ipv4, ipv6
+            for mac, (ipv4, ipv6) in here.items():
-
+                if (ipv4 or ipv6) and mac not in found:
-        def extract_lease(info):
+                    found[mac] = (ipv4, ipv6)
-            ipv4 = []
+        return found
-            ipv6 = []
-            if info["mac"] == iface_mac:
-                if info["type"] == 0:
-                    ipv4.append(info["ipaddr"])
-                elif info["type"] == 1:
-                    ipv6.append(info["ipaddr"])
-            return ipv4, ipv6
-
-        for ips in [qemuga] + leases + [arp]:
-            ipv4, ipv6 = extract_lease(ips) if "expirytime" in ips else extract_dom(ips)
-            if ipv4 or ipv6:
-                return ipv4, ipv6
-        return None, None
-
-    def _get_interface_addresses(self, source):
-        # ("Calling interfaceAddresses source=%s", source)
-        with contextlib.suppress(libvirtError):
-            return self.instance.interfaceAddresses(source)
-        return {}
-
-    def refresh_interface_addresses(self):
-        self._ip_cache = {"qemuga": {}, "arp": {}}
-
-        if self.get_status() != 1:
-            return
-
-        if self.is_agent_ready():
-            with contextlib.suppress(libvirtError):
-                self._ip_cache["qemuga"] = agent_interfaces(self.instance)
-
-        self._ip_cache["arp"] = self._get_interface_addresses(VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP)
 
     def get_net_devices(self, config=False):
         def networks(ctx):

vrtManager/tests/test_instance_getters.py

@@ -31,7 +31,7 @@ XML = """<domain>
 
 def proxy():
     p = wvmInstance.__new__(wvmInstance)  # no connection: only these calls
-    p._ip_cache = {"qemuga": {}, "arp": {}}
+    p._read_addresses = dict  # no addresses
     p._XMLDesc = lambda flags: XML
     volumes = {"/iso/x.iso": ("x.iso", "iso")}
 

vrtManager/tests/test_interface_addresses.py

@@ -27,7 +27,6 @@ AGENT = [{"name": "lo", "ip-addresses": [{"ip-address-type": "ipv4", "ip-address
 
 def vm(agent):
     inst = wvmInstance.__new__(wvmInstance)
-    inst._ip_cache = None
     inst.instance = MagicMock()
     inst.instance.interfaceAddresses.side_effect = lambda source: {VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP: ARP}[source]
     patch.object(inst, "get_status", return_value=1).start()
@@ -54,6 +53,19 @@ class InterfaceAddressesTestCase(unittest.TestCase):
         self.assertEqual([c.args for c in inst.instance.interfaceAddresses.call_args_list],
                          [(VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP,)])
 
+    def test_the_addresses_are_read_once_in_a_page_and_again_after_it(self):
+        inst = vm(agent=True)
+        with inst.cached_reads():
+            inst.get_interface_addresses(MAC)
+            inst.get_interface_addresses("52:54:00:00:00:02")
+        self.assertEqual(self.agent.call_count, 1)
+        # outside a page's block, a new read: a NIC's address can change
+        inst.get_interface_addresses(MAC)
+        self.assertEqual(self.agent.call_count, 2)
+
+    def test_a_mac_without_addresses_has_none(self):
+        self.assertEqual(vm(agent=False).get_interface_addresses("52:54:00:00:00:02"), (None, None))
+
     def test_an_agent_that_does_not_answer_leaves_the_arp_address(self):
         self.agent.side_effect = libvirtError("guest agent didn't respond to command within '10' seconds")
         self.assertEqual(vm(agent=True).get_interface_addresses(MAC), (["192.0.2.10"], []))