mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-08 11:16:47 +00:00
perf(instances): a VM page does not wait for the guest agent's addresses
A running VM's page read its addresses while it rendered: the guest agent, when its channel is connected but it does not answer, holds the page (and a gunicorn worker) for its whole 10 s timeout, on every load. The page now shows them once it has loaded: vm_addresses.js asks the new instances/<pk>/addresses/ for each NIC's addresses and fills the header's IPv4 and each network card's IPv4 and IPv6.
This commit is contained in:
catborise
parent
bf736942eb
commit
aab8dbf1df
18 changed files with 600 additions and 449 deletions
|
|
@ -2,6 +2,7 @@ from computes.models import Compute
|
|||
from django.conf import settings
|
||||
from instances.models import Flavor, Instance
|
||||
from instances.utils import nic_macs
|
||||
from libvirt import libvirtError
|
||||
from rest_framework import serializers
|
||||
|
||||
|
||||
|
|
@ -12,6 +13,17 @@ class InstanceSerializer(serializers.ModelSerializer):
|
|||
|
||||
|
||||
class InstanceDetailsSerializer(serializers.ModelSerializer):
|
||||
networks = serializers.SerializerMethodField()
|
||||
|
||||
def get_networks(self, instance) -> list[dict]:
|
||||
"""Each NIC with its addresses (None: none known), which the VM page
|
||||
reads after it loads (views.nic_addresses)."""
|
||||
try:
|
||||
found = instance.proxy.get_addresses()
|
||||
except libvirtError:
|
||||
found = {}
|
||||
return [{**nic, **dict(zip(("ipv4", "ipv6"), found.get(nic["mac"], (None, None))))} for nic in instance.networks]
|
||||
|
||||
def get_fields(self):
|
||||
fields = super().get_fields()
|
||||
# the host's ISO images are a superuser's, as on the VM page
|
||||
|
|
|
|||
|
|
@ -67,7 +67,7 @@
|
|||
{% if not instance.is_template %}
|
||||
<div class="col-6 col-lg-3">
|
||||
<dt class="fw-normal text-body-secondary">{% trans "IPv4" %}</dt>
|
||||
<dd class="mb-0 font-monospace">{{ ipv4_addresses|join:", "|default:"—" }}</dd>
|
||||
<dd class="mb-0 font-monospace" data-address-mac="{% for nic in instance.networks %}{{ nic.mac }} {% endfor %}" data-address-family="ipv4" data-empty="—">{% trans "Reading…" %}</dd>
|
||||
</div>
|
||||
{% endif %}
|
||||
</dl>
|
||||
|
|
@ -133,6 +133,7 @@
|
|||
<script src="{% static 'js/poll.js' %}"></script>
|
||||
{% if not instance.is_template %}<script src="{% static 'js/instance_stats.js' %}"></script>{% endif %}
|
||||
<script src="{% static 'js/instance_status.js' %}"></script>
|
||||
<script src="{% static 'js/vm_addresses.js' %}" data-url="{% url 'instances:nic_addresses' instance.id %}"></script>
|
||||
{% job_panel_script job_resource %}
|
||||
<script src="{% static 'js/instance_tabs.js' %}"></script>
|
||||
<script src="{% static 'js/instance_page.js' %}"></script>
|
||||
|
|
|
|||
|
|
@ -318,7 +318,7 @@
|
|||
</div>
|
||||
</div>
|
||||
<div class="d-flex flex-wrap align-items-center column-gap-4 row-gap-1 mt-2">
|
||||
<span class="text-break">{% if network.ipv4 %}{{ network.ipv4|join:", " }}{% else %}<span class="text-body-secondary">{% trans "No IPv4 address known" %}</span>{% endif %}</span>
|
||||
<span class="text-break text-body-secondary" data-address-mac="{{ network.mac }}" data-address-family="ipv4" data-empty="{% trans "No IPv4 address known" %}">{% trans "Reading…" %}</span>
|
||||
<form action="{% url 'instances:set_link_state' instance.id %}" method="post" aria-label="{% trans "Link state form" %}">{% csrf_token %}
|
||||
<input type="hidden" name="mac" value="{{ network.mac }}">
|
||||
<input type="hidden" name="set_link_state" value="{{ network.state }}">
|
||||
|
|
@ -337,7 +337,7 @@
|
|||
<dl class="row mb-0 mt-2" style="max-width: 32rem">
|
||||
<dt class="col-4 fw-normal text-body-secondary">{% trans "Type" %}</dt><dd class="col-8 mb-1">{{ network.type }}</dd>
|
||||
<dt class="col-4 fw-normal text-body-secondary">{% trans "Host device" %}</dt><dd class="col-8 mb-1 text-break">{{ network.target|default:_("no target") }}</dd>
|
||||
<dt class="col-4 fw-normal text-body-secondary">{% trans "IPv6" %}</dt><dd class="col-8 mb-1 text-break">{{ network.ipv6|join:", "|default:_("unknown") }}</dd>
|
||||
<dt class="col-4 fw-normal text-body-secondary">{% trans "IPv6" %}</dt><dd class="col-8 mb-1 text-break" data-address-mac="{{ network.mac }}" data-address-family="ipv6" data-empty="{% trans "unknown" %}">{% trans "Reading…" %}</dd>
|
||||
</dl>
|
||||
</details>
|
||||
{% if network.inbound or network.outbound %}
|
||||
|
|
|
|||
35
instances/tests/js/vm_addresses.test.js
Normal file
35
instances/tests/js/vm_addresses.test.js
Normal file
|
|
@ -0,0 +1,35 @@
|
|||
// Behaviour of static/js/vm_addresses.js with a fake document and fetch.
|
||||
// Run by instances/tests/test_vm_addresses.py:
|
||||
// node vm_addresses.test.js static/js/vm_addresses.js
|
||||
const fs = require("fs");
|
||||
const assert = require("assert");
|
||||
|
||||
function place(family, macs, empty) {
|
||||
const classes = new Set(["text-body-secondary"]);
|
||||
return { dataset: { addressFamily: family, addressMac: macs, empty }, textContent: "Reading…",
|
||||
classList: { toggle(name, on) { if (on) classes.add(name); else classes.delete(name); } }, classes };
|
||||
}
|
||||
// the header (the VM's NICs), a NIC's IPv4 and IPv6, a NIC the guest has no address on
|
||||
const header = place("ipv4", "52:54:00:00:00:01 52:54:00:00:00:02 ", "—");
|
||||
const ipv4 = place("ipv4", "52:54:00:00:00:01", "No IPv4 address known");
|
||||
const ipv6 = place("ipv6", "52:54:00:00:00:01", "unknown");
|
||||
const other = place("ipv4", "52:54:00:00:00:02", "No IPv4 address known");
|
||||
global.document = { querySelectorAll: () => [header, ipv4, ipv6, other], currentScript: { dataset: { url: "/instances/7/addresses/" } } };
|
||||
let asked = null;
|
||||
global.fetch = (url) => {
|
||||
asked = url;
|
||||
// a guest's own bridge (docker0) is not one of the VM's NICs
|
||||
return Promise.resolve({ ok: true, json: () => ({
|
||||
"52:54:00:00:00:01": { ipv4: ["192.0.2.10"], ipv6: ["2001:db8::10/64"] },
|
||||
"02:42:ac:11:00:01": { ipv4: ["172.17.0.1"], ipv6: [] } }) });
|
||||
};
|
||||
eval(fs.readFileSync(process.argv[2], "utf8"));
|
||||
setTimeout(() => {
|
||||
assert.strictEqual(asked, "/instances/7/addresses/");
|
||||
assert.strictEqual(header.textContent, "192.0.2.10", "only the VM's NICs");
|
||||
assert.strictEqual(ipv4.textContent, "192.0.2.10");
|
||||
assert.strictEqual(ipv6.textContent, "2001:db8::10/64");
|
||||
assert.ok(!ipv4.classes.has("text-body-secondary"), "an address is not muted");
|
||||
assert.strictEqual(other.textContent, "No IPv4 address known");
|
||||
assert.ok(other.classes.has("text-body-secondary"));
|
||||
}, 0);
|
||||
|
|
@ -140,7 +140,8 @@ class PageControlsTestCase(PageControls, TestCase):
|
|||
self.assertEqual([a.text_content().strip() for a in owner_page.xpath(links)], ["Instances"])
|
||||
self.assertIn("Guest agent not connected", self.last_page)
|
||||
self.assertIn("vda 2.0\xa0GB · vdb 1.0\xa0GB", self.last_page)
|
||||
self.assertEqual(owner_page.xpath("normalize-space(//dt[.='IPv4']/following-sibling::dd)"), "—")
|
||||
# filled by its script once the page has loaded ("—" when it has none)
|
||||
self.assertEqual(owner_page.xpath("//dt[.='IPv4']/following-sibling::dd/@data-empty"), ["—"])
|
||||
|
||||
def test_superuser_sees_every_control(self):
|
||||
superuser = get_user_model().objects.create_superuser("ctl_super", "ctl_super@example.com", "x")
|
||||
|
|
|
|||
|
|
@ -82,6 +82,16 @@ class PendingEditsTestCase(TestCase):
|
|||
self.assertEqual(doc.xpath("//form[.//input[@name='qos_average']]//input[@name='qos_direction']/@value"),
|
||||
["inbound", "outbound"])
|
||||
|
||||
def test_the_page_leaves_the_addresses_to_its_script(self):
|
||||
nic = {"type": "network", "mac": "52:54:10:00:00:01", "nic": "default", "state": "up", "model": "virtio",
|
||||
"filterref": "", "target": "vnet0", "inbound": [], "outbound": []}
|
||||
doc = self.page(nics=([nic], [nic]))
|
||||
self.assertEqual(doc.xpath("//*[@data-address-family]/@data-address-family"), ["ipv4", "ipv4", "ipv6"])
|
||||
# the header's are those of the VM's NICs, not the guest's own bridges
|
||||
self.assertEqual(doc.xpath("//dt[.='IPv4']/following-sibling::dd/@data-address-mac"), ["52:54:10:00:00:01 "])
|
||||
self.assertEqual(doc.xpath("//script[contains(@src, 'vm_addresses')]/@data-url"),
|
||||
[reverse("instances:nic_addresses", args=[self.instance.id])])
|
||||
|
||||
def test_the_nics_are_rows_of_one_list_with_their_dialogs(self):
|
||||
nic = {"type": "network", "mac": "52:54:10:00:00:01", "nic": "default", "state": "up", "model": "virtio",
|
||||
"ipv4": None, "ipv6": None, "filterref": "", "target": "", "inbound": [], "outbound": []}
|
||||
|
|
|
|||
59
instances/tests/test_vm_addresses.py
Normal file
59
instances/tests/test_vm_addresses.py
Normal file
|
|
@ -0,0 +1,59 @@
|
|||
"""A VM page does not wait for the guest agent, which may take its whole
|
||||
timeout: its script asks for the addresses once the page has loaded."""
|
||||
|
||||
import shutil
|
||||
import subprocess
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
from unittest.mock import PropertyMock, patch
|
||||
|
||||
from django.conf import settings
|
||||
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.test import TestCase
|
||||
from django.urls import reverse
|
||||
from libvirt import libvirtError
|
||||
|
||||
from computes.models import Compute
|
||||
from instances.api.serializers import InstanceDetailsSerializer
|
||||
from instances.models import Instance
|
||||
|
||||
|
||||
class AddressesTestCase(TestCase):
|
||||
def setUp(self):
|
||||
self.client.force_login(get_user_model().objects.create_superuser("ad-admin", "ad@example.com", "x"))
|
||||
compute = Compute.objects.create(name="ad", hostname="127.0.0.1:1", login="root", password="", type=1)
|
||||
self.instance = Instance.objects.create(compute=compute, name="ad-vm", uuid="aaaaaaaa-1111-2222-3333-555555555555")
|
||||
self.url = reverse("instances:nic_addresses", args=[self.instance.id])
|
||||
|
||||
def addresses(self, **proxy):
|
||||
with patch("instances.models.wvmInstance") as wvm:
|
||||
wvm.return_value.configure_mock(**proxy)
|
||||
return self.client.get(self.url).json()
|
||||
|
||||
def test_each_nic_s_addresses_by_mac(self):
|
||||
found = {"52:54:10:00:00:01": (["192.0.2.10"], ["2001:db8::10/64"]), "52:54:10:00:00:02": (["192.0.2.20"], None)}
|
||||
self.assertEqual(self.addresses(**{"get_addresses.return_value": found}), {
|
||||
"52:54:10:00:00:01": {"ipv4": ["192.0.2.10"], "ipv6": ["2001:db8::10/64"]},
|
||||
"52:54:10:00:00:02": {"ipv4": ["192.0.2.20"], "ipv6": []}})
|
||||
|
||||
def test_a_host_that_cannot_be_read_gives_none(self):
|
||||
self.assertEqual(self.addresses(**{"get_addresses.side_effect": libvirtError("unable to connect")}), {})
|
||||
|
||||
def test_the_api_s_vm_details_keep_each_nic_s_addresses(self):
|
||||
nics = [{"mac": "52:54:10:00:00:01", "type": "network"}, {"mac": "52:54:10:00:00:02", "type": "network"}]
|
||||
with patch("instances.models.wvmInstance") as wvm, \
|
||||
patch.object(Instance, "networks", new_callable=PropertyMock, return_value=nics):
|
||||
wvm.return_value.get_addresses.return_value = {"52:54:10:00:00:01": (["192.0.2.10"], None)}
|
||||
networks = InstanceDetailsSerializer(self.instance).get_networks(self.instance)
|
||||
self.assertEqual(networks, [{"mac": "52:54:10:00:00:01", "type": "network", "ipv4": ["192.0.2.10"], "ipv6": None},
|
||||
{"mac": "52:54:10:00:00:02", "type": "network", "ipv4": None, "ipv6": None}])
|
||||
|
||||
|
||||
class AddressesScriptTestCase(unittest.TestCase):
|
||||
@unittest.skipUnless(shutil.which("node"), "node is not installed")
|
||||
def test_vm_addresses_js(self):
|
||||
base = Path(settings.BASE_DIR)
|
||||
result = subprocess.run(["node", str(base / "instances/tests/js/vm_addresses.test.js"),
|
||||
str(base / "static/js/vm_addresses.js")], capture_output=True, text=True, timeout=60)
|
||||
self.assertEqual(result.returncode, 0, result.stderr)
|
||||
|
|
@ -21,6 +21,7 @@ urlpatterns = [
|
|||
path("<int:pk>/destroy/", views.destroy, name="destroy"),
|
||||
path("<int:pk>/migrate/", views.migrate, name="migrate"),
|
||||
path("<int:pk>/status/", views.status, name="status"),
|
||||
path("<int:pk>/addresses/", views.nic_addresses, name="nic_addresses"),
|
||||
path("<int:pk>/stats/", views.stats, name="stats"),
|
||||
path("<int:pk>/osinfo/", views.osinfo, name="osinfo"),
|
||||
path("<int:pk>/rootpasswd/", views.set_root_pass, name="rootpasswd"),
|
||||
|
|
|
|||
|
|
@ -296,7 +296,6 @@ def _instance_page(request, pk, instance, compute):
|
|||
if can_manage_console:
|
||||
console_form = ConsoleForm(instance=instance, is_superuser=request.user.is_superuser)
|
||||
console_listener_addresses = settings.QEMU_CONSOLE_LISTENER_ADDRESSES
|
||||
ipv4_addresses = [address for net in instance.networks for address in net["ipv4"] or []]
|
||||
try:
|
||||
userinstance = UserInstance.objects.get(instance=instance, user=request.user)
|
||||
except UserInstance.DoesNotExist:
|
||||
|
|
@ -390,6 +389,18 @@ def status(request, pk):
|
|||
return JsonResponse({"status": instance.proxy.get_status()})
|
||||
|
||||
|
||||
def nic_addresses(request, pk):
|
||||
"""The VM's addresses by MAC, which its page shows once it has loaded
|
||||
(vm_addresses.js): the guest agent may take its whole timeout. A host
|
||||
that cannot be read gives none."""
|
||||
instance = get_instance(request.user, pk)
|
||||
try:
|
||||
found = instance.proxy.get_addresses()
|
||||
except libvirtError:
|
||||
found = {}
|
||||
return JsonResponse({mac: {"ipv4": ipv4 or [], "ipv6": ipv6 or []} for mac, (ipv4, ipv6) in found.items()})
|
||||
|
||||
|
||||
def _valid_ssh_target(compute):
|
||||
"""The login and hostname reach ssh as a command-line argument; a compute
|
||||
whose values would inject an ssh option (an existing or bypassed row that
|
||||
|
|
|
|||
Binary file not shown.
File diff suppressed because it is too large
Load diff
Binary file not shown.
File diff suppressed because it is too large
Load diff
20
static/js/vm_addresses.js
Normal file
20
static/js/vm_addresses.js
Normal file
|
|
@ -0,0 +1,20 @@
|
|||
// The VM's addresses come once the page has loaded: the guest agent may take
|
||||
// its whole timeout (instances.views.nic_addresses). An element with
|
||||
// data-address-family shows those of the NICs in data-address-mac (MACs
|
||||
// separated by spaces: not the guest's own bridges); data-empty when there
|
||||
// are none.
|
||||
(function () {
|
||||
const places = document.querySelectorAll("[data-address-family]");
|
||||
const show = (found) => {
|
||||
for (const place of places) {
|
||||
const nics = place.dataset.addressMac.split(" ").filter(Boolean).map((mac) => found[mac] || {});
|
||||
const addresses = nics.flatMap((nic) => nic[place.dataset.addressFamily] || []);
|
||||
place.textContent = addresses.join(", ") || place.dataset.empty;
|
||||
place.classList.toggle("text-body-secondary", !addresses.length);
|
||||
}
|
||||
};
|
||||
fetch(document.currentScript.dataset.url, {headers: {Accept: "application/json"}})
|
||||
.then((response) => (response.ok ? response.json() : {}))
|
||||
.catch(() => ({}))
|
||||
.then(show);
|
||||
})();
|
||||
|
|
@ -795,15 +795,11 @@ class wvmInstance(wvmConnect):
|
|||
pcpus = hostinfo[4] * hostinfo[5] * hostinfo[6] * hostinfo[7]
|
||||
return range(1, max(int(pcpus), at_least) + 1)
|
||||
|
||||
def get_interface_addresses(self, iface_mac):
|
||||
"""The running VM's addresses of the NIC with iface_mac: (IPv4, IPv6
|
||||
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))
|
||||
|
||||
def _read_addresses(self):
|
||||
"""{MAC: (IPv4 list, IPv6 list)}: each MAC's addresses from the first source that has some."""
|
||||
def get_addresses(self):
|
||||
"""The running VM's addresses, {MAC: (IPv4 list, IPv6 list with
|
||||
prefix)}, each MAC's from the first source that has some: the guest
|
||||
agent, else the host's ARP table. The agent may take its whole
|
||||
timeout: the VM page reads them after it loads (views.nic_addresses)."""
|
||||
if self.get_status() != 1:
|
||||
return {}
|
||||
sources = []
|
||||
|
|
@ -856,10 +852,6 @@ class wvmInstance(wvmConnect):
|
|||
out_burst = out_attr.get("burst")
|
||||
outbound = {"average": out_av, "peak": out_peak, "burst": out_burst}
|
||||
|
||||
try:
|
||||
ipv4, ipv6 = self.get_interface_addresses(mac_inst)
|
||||
except libvirtError:
|
||||
ipv4, ipv6 = None, None
|
||||
result.append(
|
||||
{
|
||||
"type": interface_type,
|
||||
|
|
@ -868,8 +860,6 @@ class wvmInstance(wvmConnect):
|
|||
"target": target_inst,
|
||||
"state": link_state,
|
||||
"model": model_type,
|
||||
"ipv4": ipv4,
|
||||
"ipv6": ipv6,
|
||||
"filterref": filterref_inst,
|
||||
"inbound": inbound,
|
||||
"outbound": outbound,
|
||||
|
|
|
|||
|
|
@ -26,7 +26,6 @@ XML = """<domain>
|
|||
|
||||
def proxy():
|
||||
p = wvmInstance.__new__(wvmInstance) # no connection: only these calls
|
||||
p._read_addresses = dict # no addresses
|
||||
p._XMLDesc = lambda flags: XML
|
||||
volumes = {"/iso/x.iso": ("x.iso", "iso")}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
"""A running VM's addresses come from the guest agent when it answers and
|
||||
from the host's ARP table, so a VM without an agent shows its IPv4 address.
|
||||
The agent is asked with a timeout: interfaceAddresses() would wait for a
|
||||
hung agent indefinitely, and the VM page with it."""
|
||||
hung agent indefinitely; the VM page asks for them after it loads."""
|
||||
|
||||
import json
|
||||
import unittest
|
||||
|
|
@ -38,32 +38,31 @@ class InterfaceAddressesTestCase(unittest.TestCase):
|
|||
|
||||
def test_a_vm_without_an_agent_shows_its_arp_address(self):
|
||||
inst = vm(agent=False)
|
||||
self.assertEqual(inst.get_interface_addresses(MAC), (["192.0.2.10"], []))
|
||||
self.assertEqual(inst.get_addresses(), {MAC: (["192.0.2.10"], [])})
|
||||
self.agent.assert_not_called()
|
||||
|
||||
def test_the_agents_address_comes_first_and_is_asked_with_a_timeout(self):
|
||||
inst = vm(agent=True)
|
||||
self.assertEqual(inst.get_interface_addresses(MAC), (["192.0.2.20"], ["2001:db8::20/64"]))
|
||||
self.assertEqual(inst.get_addresses(), {MAC: (["192.0.2.20"], ["2001:db8::20/64"])})
|
||||
self.agent.assert_called_once_with(inst.instance, '{"execute": "guest-network-get-interfaces"}', 10, 0)
|
||||
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):
|
||||
def test_the_vm_s_definition_does_not_read_them(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)
|
||||
inst._XMLDesc = lambda flags: f"<domain><devices><interface type='network'><mac address='{MAC}'/></interface></devices></domain>"
|
||||
self.assertEqual([nic["mac"] for nic in inst.get_net_devices()], [MAC])
|
||||
self.agent.assert_not_called()
|
||||
inst.instance.interfaceAddresses.assert_not_called()
|
||||
|
||||
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_a_vm_that_does_not_run_has_none(self):
|
||||
inst = vm(agent=True)
|
||||
inst.get_status.return_value = 5
|
||||
self.assertEqual(inst.get_addresses(), {})
|
||||
|
||||
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"], []))
|
||||
self.assertEqual(vm(agent=True).get_addresses(), {MAC: (["192.0.2.10"], [])})
|
||||
|
||||
|
||||
class OsInfoTestCase(unittest.TestCase):
|
||||
|
|
|
|||
|
|
@ -83,6 +83,7 @@ DENIED = (403, 404)
|
|||
VIEW_VM = {
|
||||
"instances:instance",
|
||||
"instances:status",
|
||||
"instances:nic_addresses",
|
||||
"instances:stats",
|
||||
"instances:osinfo",
|
||||
"instances:sshkeys",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue