mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-10 12:16:49 +00:00
feat(instances): a VM's options in one list, each saved when it can be
Settings > Options was four forms under bold sentences, most of them disabled while the VM ran. The owner chose this layout from three rendered options: one list, each row a setting with its description on the left and its control on the right: VM details (title, description), Template, primary video model, QEMU guest agent. vCPU hot plug moved to Resize > CPU, next to the vCPU counts. Codex reviewed the four forms; these are fixed: - Title, description and the video model are saved while the VM runs: they are in the persistent definition, used at the next start. - The template flag has its own form and view, set_template, for superusers and staff, and only for a shut-off VM: a running VM made a template left the quota while it kept running. Turning a template back into a VM checks the quota there, and each change is audited. The title form no longer touches the flag. - A new video model stays the primary one; the old model's memory and head settings, which are its own, go with it. - The agent and vCPU hot plug forms show the persistent definition; they showed the running VM's, so a pending change looked undone. - Enabling the guest agent adds its channel only where it is missing: saving Enabled again failed, and a channel pending the next start kept a running VM from getting one. - The agent line tells a VM that is not running, one without a channel, and a connected or not connected agent apart.
This commit is contained in:
parent
46188d3230
commit
a993c12db0
13 changed files with 272 additions and 172 deletions
|
|
@ -544,12 +544,10 @@ class LiveDataLossTestCase(TestCase):
|
|||
return (f"<disk type='file' device='disk'><driver name='qemu' type='qcow2'/><source file='{path}'/>"
|
||||
"<target dev='sda' bus='scsi'/></disk>")
|
||||
|
||||
agent = "<channel type='unix'><target type='virtio' name='org.qemu.guest_agent.0'/></channel>"
|
||||
cases = (
|
||||
("disk", lambda n: disk(livetest.create_volume(self.conn, P + n + "-pending")),
|
||||
lambda vm, n: vm.attach_disk("sda", livetest.create_volume(self.conn, P + n + "-new"),
|
||||
target_bus="scsi", format_type="qcow2")),
|
||||
("agent", lambda n: agent, lambda vm, n: vm.add_guest_agent()),
|
||||
)
|
||||
both = (0, libvirt.VIR_DOMAIN_XML_INACTIVE)
|
||||
for paused in (False, True):
|
||||
|
|
@ -566,6 +564,19 @@ class LiveDataLossTestCase(TestCase):
|
|||
attach(inst.proxy, name)
|
||||
self.assertEqual([dom.XMLDesc(flags) for flags in both], before)
|
||||
|
||||
def test_an_agent_channel_pending_the_next_start_is_added_to_the_running_vm_only(self):
|
||||
from lxml import etree
|
||||
|
||||
agent = "<channel type='unix'><target type='virtio' name='org.qemu.guest_agent.0'/></channel>"
|
||||
dom, inst = self.vm("agent-pending", [], extra_devices="<controller type='virtio-serial'/>")
|
||||
dom.create()
|
||||
dom.attachDeviceFlags(agent, libvirt.VIR_DOMAIN_AFFECT_CONFIG) # pending: persistent only
|
||||
channels = "devices/channel/target[@name='org.qemu.guest_agent.0']"
|
||||
for _attempt in range(2): # saving Enabled again changes nothing
|
||||
inst.proxy.add_guest_agent()
|
||||
self.assertEqual([len(etree.fromstring(dom.XMLDesc(flags)).xpath(channels))
|
||||
for flags in (0, libvirt.VIR_DOMAIN_XML_INACTIVE)], [1, 1])
|
||||
|
||||
def test_a_nic_without_a_mac_gets_the_same_one_in_both_definitions(self):
|
||||
from lxml import etree
|
||||
|
||||
|
|
|
|||
|
|
@ -132,16 +132,29 @@ class PendingEditsTestCase(TestCase):
|
|||
self.assertEqual(doc.xpath("//select[@name='vcpu']/option[@selected]/@value"), ["4"])
|
||||
self.assertEqual(doc.xpath("//select[@name='cur_vcpu']/option[@selected]/@value"), ["3"])
|
||||
|
||||
def test_the_agent_and_vcpu_hot_plug_forms_show_the_pending_definition(self):
|
||||
# running without an agent channel and hot plug, both pending for the next start
|
||||
hotplug = {"0": {"enabled": "yes", "hotpluggable": "no", "order": "1"},
|
||||
"1": {"enabled": "yes", "hotpluggable": "yes", "order": "2"}}
|
||||
doc = self.page(get_guest_agent=(None, object()), get_vcpus=({}, hotplug))
|
||||
self.assertEqual(doc.xpath("//select[@name='guest_agent']/option[@selected]/@value"), ["True"])
|
||||
self.assertEqual(doc.xpath("//select[@name='vcpu_hotplug']/option[@selected]/@value"), ["True"])
|
||||
self.assertIn("No channel", doc.xpath("string(//form[.//select[@name='guest_agent']])"))
|
||||
|
||||
def test_buttons_shown_as_disabled_cannot_submit(self):
|
||||
# running: the CPU, options and video forms wait for a shutdown
|
||||
# running: the CPU resize, vCPU hot plug, template and disk removal forms wait for a shutdown;
|
||||
# the title, description and video model are of the next start, saved at once
|
||||
doc = self.page(live=[disk("vda", "default", "")], config=[disk("vda", "default", "")])
|
||||
self.assertGreater(len(doc.xpath("//button[@disabled]")), 5)
|
||||
self.assertGreater(len(doc.xpath("//button[@disabled]")), 4)
|
||||
shown = doc.xpath("//button[contains(concat(' ', @class, ' '), ' disabled ')]")
|
||||
self.assertEqual([b for b in shown if b.get("disabled") is None], [])
|
||||
for action in ("resizevm_cpu", "change_options", "set_video_model", "detach_vol", "delete_vol"):
|
||||
for action, waits in (("resizevm_cpu", True), ("set_vcpu_hotplug", True), ("set_template", True),
|
||||
("detach_vol", True), ("delete_vol", True),
|
||||
("change_options", False), ("set_video_model", False)):
|
||||
with self.subTest(action):
|
||||
(form,) = doc.xpath("//form[@action=$a]", a=reverse(f"instances:{action}", args=[self.instance.id]))
|
||||
self.assertTrue(all(b.get("disabled") is not None for b in form.xpath(".//button")))
|
||||
controls = form.xpath(".//button | .//input[@type='checkbox']")
|
||||
self.assertEqual([c.get("disabled") is not None for c in controls], [waits] * len(controls))
|
||||
|
||||
def test_the_xml_editor_shows_the_definition_for_the_next_start(self):
|
||||
# static/js/xml_editor.js opens the review dialog on Change, also while the VM runs
|
||||
|
|
|
|||
|
|
@ -1,8 +1,6 @@
|
|||
"""
|
||||
Only superusers and staff may mark or unmark a VM as a template.
|
||||
The UI only disables the checkbox for other users, and a disabled checkbox is
|
||||
not submitted, so the server must not derive is_template from its absence for
|
||||
them.
|
||||
Only superusers and staff may mark or unmark a VM as a template, on its own
|
||||
form: the title and description form leaves the flag alone.
|
||||
"""
|
||||
from unittest.mock import patch
|
||||
|
||||
|
|
@ -13,6 +11,7 @@ from django.urls import reverse
|
|||
from accounts.models import UserInstance
|
||||
from computes.models import Compute
|
||||
from instances.models import Instance
|
||||
from logs.models import Logs
|
||||
|
||||
|
||||
class TemplateFlagTestCase(TestCase):
|
||||
|
|
@ -36,16 +35,15 @@ class TemplateFlagTestCase(TestCase):
|
|||
for user in (self.owner, self.staff_owner):
|
||||
UserInstance.objects.create(instance=self.instance, user=user, is_change=True)
|
||||
|
||||
def _change_options(self, user, data, status=302):
|
||||
def _post(self, user, action, data, status=302):
|
||||
self.client.force_login(user)
|
||||
with patch("instances.models.wvmInstance") as wvm:
|
||||
wvm.return_value.get_status.return_value = 5
|
||||
# unmarking a template checks the owner's quota with the VM's size
|
||||
wvm.return_value.get_vcpu.return_value = 1
|
||||
wvm.return_value.get_memory.return_value = 1024
|
||||
wvm.return_value.get_disk_devices.return_value = []
|
||||
res = self.client.post(
|
||||
reverse("instances:change_options", args=[self.instance.id]), data
|
||||
)
|
||||
res = self.client.post(reverse(f"instances:{action}", args=[self.instance.id]), data)
|
||||
self.assertEqual(res.status_code, status)
|
||||
self.instance.refresh_from_db()
|
||||
|
||||
|
|
@ -54,23 +52,30 @@ class TemplateFlagTestCase(TestCase):
|
|||
self.instance.save(update_fields=["is_template"])
|
||||
|
||||
def test_owner_cannot_mark_vm_as_template(self):
|
||||
self._change_options(self.owner, {"title": "t", "is_template": "True"})
|
||||
self._post(self.owner, "set_template", {"is_template": "True"}, status=403)
|
||||
self.assertFalse(self.instance.is_template)
|
||||
|
||||
def test_owner_cannot_change_a_template(self):
|
||||
# a template is changed only by superusers and staff owners
|
||||
self._make_template()
|
||||
self._change_options(self.owner, {"title": "new title", "description": ""}, status=403)
|
||||
self._post(self.owner, "change_options", {"title": "new title", "description": ""}, status=403)
|
||||
self.assertTrue(self.instance.is_template)
|
||||
|
||||
def test_staff_owner_can_mark_and_unmark_template(self):
|
||||
self._change_options(self.staff_owner, {"title": "t", "is_template": "True"})
|
||||
self._post(self.staff_owner, "set_template", {"is_template": "True"})
|
||||
self.assertTrue(self.instance.is_template)
|
||||
# An unchecked checkbox is not submitted: for staff that means "not a template".
|
||||
self._change_options(self.staff_owner, {"title": "t"})
|
||||
# an unchecked switch is not submitted: "not a template"
|
||||
self._post(self.staff_owner, "set_template", {})
|
||||
self.assertFalse(self.instance.is_template)
|
||||
self.assertEqual(list(Logs.objects.filter(message__startswith="VM").order_by("id").values_list("message", "success")),
|
||||
[("VM tpl-vm: mark as template", True), ("VM tpl-vm: unmark as template", True)])
|
||||
|
||||
def test_superuser_can_unmark_template(self):
|
||||
self._make_template()
|
||||
self._change_options(self.superuser, {"title": "t"})
|
||||
self._post(self.superuser, "set_template", {})
|
||||
self.assertFalse(self.instance.is_template)
|
||||
|
||||
def test_the_title_form_leaves_the_template_alone(self):
|
||||
self._make_template()
|
||||
self._post(self.superuser, "change_options", {"title": "t", "description": ""})
|
||||
self.assertTrue(self.instance.is_template)
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
"""Templates do not count towards the quota and cannot be started. Turning a
|
||||
template back into a VM adds it to its owner's usage, so for users with a
|
||||
quota it is checked like any other increase."""
|
||||
quota it is checked like any other increase. Only a shut-off VM changes."""
|
||||
|
||||
from unittest.mock import patch
|
||||
|
||||
|
|
@ -22,18 +22,17 @@ class TemplateFlagQuotaTests(TestCase):
|
|||
self.vm = Instance.objects.create(compute=compute, name="tq-vm", uuid="12345678-0000-0000-0000-00000000aaaa")
|
||||
UserInstance.objects.create(user=self.staff, instance=self.vm, is_change=True)
|
||||
|
||||
def post(self, user, is_template, quota=""):
|
||||
def post(self, user, is_template, quota="", status=5):
|
||||
self.client.force_login(user)
|
||||
data = {"title": "", "description": ""}
|
||||
if is_template:
|
||||
data["is_template"] = "on"
|
||||
data = {"is_template": "on"} if is_template else {}
|
||||
with patch("instances.models.wvmInstance") as wvm, patch(
|
||||
"instances.views.utils.check_user_quota", return_value=quota
|
||||
) as check:
|
||||
wvm.return_value.get_status.return_value = status
|
||||
wvm.return_value.get_vcpu.return_value = 2
|
||||
wvm.return_value.get_memory.return_value = 2048
|
||||
wvm.return_value.get_disk_devices.return_value = [{"size": 10 << 30}]
|
||||
response = self.client.post(reverse("instances:change_options", args=[self.vm.id]), data)
|
||||
response = self.client.post(reverse("instances:set_template", args=[self.vm.id]), data)
|
||||
self.vm.refresh_from_db()
|
||||
return check, [str(m) for m in get_messages(response.wsgi_request)]
|
||||
|
||||
|
|
@ -60,6 +59,14 @@ class TemplateFlagQuotaTests(TestCase):
|
|||
self.assertTrue(self.vm.is_template)
|
||||
self.assertTrue(any("cannot be reached" in m for m in messages), messages)
|
||||
|
||||
def test_a_vm_that_is_not_shut_off_stays_as_it_is(self):
|
||||
# a running VM made a template would leave the quota while it runs
|
||||
for status in (1, 3):
|
||||
with self.subTest(status=status):
|
||||
check, messages = self.post(self.staff, is_template=True, status=status)
|
||||
self.assertFalse(self.vm.is_template)
|
||||
self.assertIn("Shut the VM down to change whether it is a template.", messages)
|
||||
|
||||
def test_superusers_have_no_quota(self):
|
||||
Instance.objects.filter(pk=self.vm.pk).update(is_template=True)
|
||||
admin = get_user_model().objects.create_superuser("tq-admin", "tq@example.com", "pw")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue