diff --git a/README.md b/README.md index 695b805f..00e1d15c 100644 --- a/README.md +++ b/README.md @@ -583,6 +583,7 @@ A version from before v3.0.0 has no worker: `systemctl disable --now webvirt-wor - A disk can be grown while its VM runs or is paused, not only when it is shut off. A disk on an LVM pool cannot be grown (libvirt cannot resize its volumes): the resize says so instead of failing, and the VM's other disks still grow. - Enabling vCPU hot plug keeps the VM's current vCPU count (it enabled all of them up to the maximum). - A host's CPU graph shows the average since its previous point (normally 5 s) instead of a 1 s sample taken while a worker waits. +- A new VM is made in three steps (host, machine, details) on one form: the disk is a new one, existing volumes or a copy of a template, and flavors are presets that fill vCPU, memory and disk size. A request with more than one disk source is refused. - The Logs page records who added, changed or deleted a compute, user, group (with its members and permissions), VM owner, flavor, setting, removed-VM record or SSH key, who mailed an OTP QR code, and every login, failed login, lockout and logout, with the address it came from and whether it worked (Result: Done, Failed, Unknown for older rows); a compute's VMs leaving the panel get a row each. On each compute it records changes to networks, storage pools, volumes (including clones and ISO uploads), host interfaces, secrets (never their values) and network filters. - The CPU usage in the new-VM dialog is the current load (over 1 s), read when the dialog opens; it was the average since the host booted. - A VM's stats are polled every 5 s like the host graph, the first point as soon as the tab opens; each point is the average since the previous one, disk rates in MB/s (they were MiB/s labelled Mb/s); a counter the host does not report shows a gap instead of 0. diff --git a/instances/jobs.py b/instances/jobs.py index 98d3684c..ce7a29ab 100644 --- a/instances/jobs.py +++ b/instances/jobs.py @@ -640,8 +640,11 @@ class CreateInstance(Kind): raise util.OperationError(_("A virtual machine with this name already exists")) if Instance.objects.filter(name__exact=data["name"]): raise util.OperationError(_("There is an instance with same name. Remove it and try again!")) - if not (data["hdd_size"] or data["template"] or data["images"]): + sources = [source for source in ("hdd_size", "template", "images") if data[source]] + if not sources: raise util.OperationError(_("First you need to create or select an image")) + if len(sources) > 1: + raise util.OperationError(_("Choose one disk source: a new disk, existing volumes or a template")) if not data["hdd_size"] and data["template"] and conn.get_volume_path(data["name"] + ".img", data["storage"]): raise util.OperationError(_("Image has already exist. Please check volumes or change instance name")) if data["cache_mode"] not in conn.get_cache_modes(): diff --git a/instances/templates/allinstances.html b/instances/templates/allinstances.html index e57b1328..52aff586 100644 --- a/instances/templates/allinstances.html +++ b/instances/templates/allinstances.html @@ -43,10 +43,6 @@ {% if request.user.is_superuser %} @@ -123,21 +108,7 @@ editor.getSession().on("change",function () { input.val(editor.getSession().getValue()); }) + // the editor measures itself when its section is shown + document.querySelector("#from-xml").addEventListener("shown.bs.collapse", () => editor.resize()); -{% if request.user.is_superuser %} - -{% endif %} {% endblock %} diff --git a/instances/templates/create_instance_w2.html b/instances/templates/create_instance_w2.html index 8b38749f..694618dc 100644 --- a/instances/templates/create_instance_w2.html +++ b/instances/templates/create_instance_w2.html @@ -4,8 +4,8 @@ {% load operations %} {% load django_bootstrap5 %} {% load form_fields %} -{% block body_attrs %} data-error-focus="manual" data-submitted-form="{{ submitted_form }}"{% endblock %} {% load bootstrap_icons %} +{% block body_attrs %} data-error-focus="manual"{% if refused %} data-refused="1"{% endif %}{% endblock %} {% block title %}{% trans "Create new instance" %}{% endblock %} {% block style %} @@ -13,6 +13,13 @@ {% endblock %} @@ -22,907 +29,459 @@ {% block content %} {% include 'pleasewaitdialog.html' %} +
+ {% include "create_steps.html" with step=3 %} + {% if vm_form %} +
+ {% csrf_token %}{% submission_key %} + {% if submitted_lists %}{{ submitted_lists|json_script:"submitted-lists" }}{% endif %} + {% include "instances/form_errors.html" with form=vm_form %} -
-
-
- - - -
-
- {% include 'create_flav_block.html' %} - - {% if not flavors %} -
-
- {% bs_icon 'exclamation-triangle'%} {% trans "Warning" %}: {% trans "Hypervisor doesn't have any Flavors" %} -
-
+
+
+

{% trans "Name and size" %}

+
+ + +
{% trans "Letters, digits, dot, dash and underscore." %}
+ {% field_error vm_form.name "vm_name" %} +
+ +
+
+ {% trans "Presets" %} + +
+ {% if flavors %} +
+ {% for flavor in flavors %} + + {% endfor %} +
+
{% trans "A preset fills vCPUs, memory and a new disk of its size." %}
{% else %} -
- - - - - - - - - - - - - {% for flavor, form_id, vm_form in flavor_forms %} - - - - - - - - - +

{% trans "No flavors yet." %}

+ {% endif %} +
+ + +
+
+ + + {% field_error vm_form.vcpu "vm_vcpu" %} +
+
+ + + {% field_error vm_form.memory "vm_memory" %} +
+ {% if firmwares %} +
+ +
-
#{% trans "Name" %}{% trans "VCPU" %}{% trans "RAM" %}{% trans "HDD" %}{% trans "Action" %}
{{ forloop.counter }}{{ flavor.label }}{{ flavor.vcpu }}{{ flavor.memory }} {% trans "MB" %}{{ flavor.disk }} {% trans "GB" %} - - - {% bs_icon 'plus'%} - - -
- {% csrf_token %} - -
-
+ + {% field_error vm_form.firmware "vm_firmware" %}
{% endif %} -
-
- -
-
-
{% csrf_token %}{% submission_key %} - - {% if custom_lists %}{{ custom_lists|json_script:"custom-lists" }}{% endif %} - {% include "instances/form_errors.html" with form=custom_form %} -
- -
- - {% field_error custom_form.name "custom_name" %} -
-
- {% if firmwares %} -
- -
- - {% field_error custom_form.firmware "select_firmware" %} -
-
- {% endif %} -
- -
- - {% field_error custom_form.vcpu "custom_vcpu" %} -
-
- {% if dom_caps.cpu_modes %} -
- -
- + {% for mode in dom_caps.cpu_modes %} {% if mode == 'custom' %} - + {% for model in dom_caps.cpu_custom_models %} - + {% endfor %} {% else %} - + {% endif %} {% endfor %} - {% field_error custom_form.vcpu_mode "custom_vcpu_mode" %} + {% field_error vm_form.vcpu_mode "vm_vcpu_mode" %}
+ {% endif %}
- {% endif %} -
- -
- - {% field_error custom_form.memory "custom_memory" %} -
- -
-
- -
-
    - -
- -
-
-
- - -
- -
-
- -
-
-
- -
-
    - -
- - - - -
-
-
- -
- -
-
-
-
- -
- +
+
+ +
+
+

{% trans "Disk" %}

+
+ {% trans "Disk source" %} +
+ {% for value, title, help in disk_sources %} +
+
- + {% endfor %}
-
- -
- + {% for storage in storages %} + {% endfor %} - {% field_error custom_form.cache_mode "cache_mode" %} + {% field_error vm_form.storage "new_storage" %} +
+
+ + + {% field_error vm_form.hdd_size "vm_hdd_size" %} +
+
+
+ + +
-
- -
- +
+
+ + +
+
+ + + {% field_error vm_form.images "image-control" %} +
+
+
    + +
    + + +
    +
    + +
    +
    +

    {% trans "Networks" %}

    + + + {% field_error vm_form.networks "network-control" %} + + +
      + + {% field_error vm_form.mac "vm_mac" %} +
      +
      + +
      +
      +

      + +

      +
      +
      +
      + + + {% field_error vm_form.cache_mode "vm_cache_mode" %} +
      +
      + + + {% field_error vm_form.net_model "vm_net_model" %} +
      +
      + + - {% field_error custom_form.nwfilter "custom_nwfilter" %} + {% field_error vm_form.nwfilter "vm_nwfilter" %}
      -
      -
      - -
      - - {% field_error custom_form.net_model "custom_net_model" %} -
      -
      -
      - -
      - + {% if not videos %}{% endif %} {% for video in videos %} - + {% endfor %} - {% field_error custom_form.video "custom_video" %} + {% field_error vm_form.video "vm_video" %}
      -
      -
      - -
      - {% for addr, label in listener_addr %} - + {% endfor %} - {% field_error custom_form.listener_addr "custom_listener_addr" %} + {% field_error vm_form.listener_addr "vm_listener_addr" %}
      -
      -
      - -
      - + {% for bus in cdrom_buses %} - + {% endfor %} - {% field_error custom_form.add_cdrom "add_cdrom" %} + {% field_error vm_form.add_cdrom "vm_add_cdrom" %}
      -
      -
      - -
      - + {% for bus in input_device_buses %} - + {% endfor %} - {% field_error custom_form.add_input "add_input" %} + {% field_error vm_form.add_input "vm_add_input" %}
      -
      -
      - -
      - +
      +
      + + +
      + {% if virtio_support %} +
      + + +
      + {% endif %}
      - {% if virtio_support %} -
      - -
      - -
      -
      - {% endif %} -
      -
      -
      - {% if storages %} - - {% else %} - - {% endif %} -
      -
      -
      -
      +
      -
      -
      -
      {% csrf_token %}{% submission_key %} - - {% include "instances/form_errors.html" with form=template_form %} -
      - -
      - - {% field_error template_form.name "template_name" %} -
      -
      - {% if firmwares %} -
      - -
      - - {% field_error template_form.firmware "template_firmware" %} -
      -
      - {% endif %} -
      - -
      - - {% field_error template_form.vcpu "template_vcpu" %} -
      -
      -
      - -
      - - {% field_error template_form.vcpu_mode "template_vcpu_mode" %} -
      -
      -
      - -
      - - {% field_error template_form.memory "template_memory" %} -
      - -
      -
      - -
      - -
      -
      - - {% field_error template_form.template "template" %} -
      -
      -
      - -
      - - {% field_error template_form.storage "storage" %} -
      -
      -
      - -
      - - {% field_error template_form.networks "template_networks" %} -
      -
      - - {% field_error template_form.mac "template_mac" %} -
      -
      -
      - -
      - -
      -
      -
      -
      - -
      - -
      - -
      -
      - -
      - - {% field_error template_form.cache_mode "template_cache_mode" %} -
      -
      -
      - -
      - - {% field_error template_form.nwfilter "template_nwfilter" %} -
      -
      -
      - -
      - - {% field_error template_form.net_model "template_net_model" %} -
      -
      -
      - -
      - - {% field_error template_form.video "template_video" %} -
      -
      -
      - -
      - - {% field_error template_form.listener_addr "template_listener_addr" %} -
      -
      -
      - -
      - - {% field_error template_form.add_cdrom "template_add_cdrom" %} -
      -
      -
      - -
      - - {% field_error template_form.add_input "template_add_input" %} -
      -
      -
      - -
      - -
      -
      - {% if virtio_support %} -
      - -
      - -
      -
      - {% endif %} -
      -
      -
      - {% if storages %} - - {% else %} - - {% endif %} -
      -
      +
      + {% trans "Cancel" %} + +
      +
      + {% endif %} +
      + +
      - {% endblock %} + {% block script %} -{% if request.user.is_superuser %} - -{% endif %} {% endblock %} diff --git a/instances/templates/create_steps.html b/instances/templates/create_steps.html new file mode 100644 index 00000000..203c70c0 --- /dev/null +++ b/instances/templates/create_steps.html @@ -0,0 +1,19 @@ +{% load i18n %} +{# The steps of creating a VM; step is the current one (1-3), a done step links back to it. #} + diff --git a/instances/templatetags/form_fields.py b/instances/templatetags/form_fields.py index 7db8f630..77c08218 100644 --- a/instances/templatetags/form_fields.py +++ b/instances/templatetags/form_fields.py @@ -20,11 +20,12 @@ def invalid(field): @register.simple_tag -def error_attrs(field, input_id): - """aria-invalid and aria-describedby for a refused field.""" - if not _errors(field): - return "" - return format_html(' aria-invalid="true" aria-describedby="{}_error"', input_id) +def error_attrs(field, input_id, help_id=""): + """aria-invalid and aria-describedby for a refused field; with help_id, + the field is described by its help text too, refused or not.""" + described = " ".join(filter(None, [help_id, f"{input_id}_error" if _errors(field) else ""])) + invalid = format_html(' aria-invalid="true"') if _errors(field) else "" + return format_html('{} aria-describedby="{}"', invalid, described) if described else "" @register.simple_tag diff --git a/instances/tests/test_create_dialog_cpu.py b/instances/tests/test_create_dialog_cpu.py index eefff944..2e627fae 100644 --- a/instances/tests/test_create_dialog_cpu.py +++ b/instances/tests/test_create_dialog_cpu.py @@ -22,9 +22,9 @@ function element() { setAttribute: (k, v) => { attrs[k] = String(v); }, removeAttribute: (k) => { delete attrs[k]; }}; } const cells = URLS.map((url) => { - const bar = element(), label = element(); - return {dataset: {url}, bar, label, - querySelector: (sel) => (sel === ".progress-bar" ? bar : label)}; + const meter = element(), bar = element(), label = element(); + return {dataset: {url}, meter, bar, label, + querySelector: (sel) => ({".progress": meter, ".progress-bar": bar})[sel] || label}; }); const handlers = {}, requests = []; const dialog = {addEventListener: (name, fn) => { handlers[name] = fn; }, querySelectorAll: () => cells}; @@ -39,7 +39,7 @@ const page = { window: {location: {}}, }; vm.runInNewContext(SCRIPT, vm.createContext(page)); -const state = () => cells.map((c) => [c.bar.style.width, c.bar.attrs["aria-valuenow"] || null, c.label.textContent]); +const state = () => cells.map((c) => [c.bar.style.width, c.meter.attrs["aria-valuenow"] || null, c.label.textContent]); const answer = (i, data) => { const r = requests[i]; data === null ? r.err() : r.ok(data); r.end(); }; const out = {}; handlers["show.bs.modal"](); @@ -88,7 +88,7 @@ class CreateDialogCpuTestCase(TestCase): def test_the_page_carries_the_statistics_urls_and_reads_no_cpu_usage(self): page = self.page() - urls = html.fromstring(page).xpath("//td[@class='cpu-usage']/@data-url") + urls = html.fromstring(page).xpath("//*[contains(@class, 'cpu-usage ')]/@data-url") self.assertEqual(urls, [reverse("compute_graph", args=[c.id]) for c in self.computes]) self.details.return_value.get_cpu_usage.assert_not_called() diff --git a/instances/tests/test_create_form_input.py b/instances/tests/test_create_form_input.py index 1fc854db..c965bd53 100644 --- a/instances/tests/test_create_form_input.py +++ b/instances/tests/test_create_form_input.py @@ -1,5 +1,5 @@ -"""A refused VM create form comes back bound and open, with what was -entered; the other create forms keep their own defaults.""" +"""A refused VM create form comes back bound, with what was entered and the +disk source that was chosen; flavors are presets, not forms.""" import json import uuid @@ -23,179 +23,182 @@ def valid(**fields): class CreateFormInputTests(TestCase): - def setUp(self): - self.flavor = Flavor.objects.create(label="wvc-test", memory=512, vcpu=1, disk=10) - - def test_each_create_form_has_its_own_unbound_form(self): + def test_the_page_has_one_unbound_form(self): _, response = render_create_page(self) - forms = response.context["vm_forms"] - self.assertEqual(set(forms), {"custom", "template"} | {f"flavor{f.id}" for f in Flavor.objects.all()}) - self.assertFalse(any(form.is_bound for form in forms.values())) - self.assertEqual(response.context["submitted_form"], "") + self.assertFalse(response.context["vm_form"].is_bound) + self.assertFalse(response.context["refused"]) + page = html.fromstring(response.content) + self.assertEqual(len(page.xpath("//form[.//button[@name='create']]")), 1) def test_the_refused_form_comes_back_bound(self): - _, response = render_create_page(self, valid(form_id="custom", name="bad name!", vcpu="2")) - self.assertEqual(response.context["submitted_form"], "custom") - self.assertTrue(response.context["custom_form"].is_bound) - self.assertFalse(response.context["template_form"].is_bound) - self.assertIn("name", response.context["custom_form"].errors) + _, response = render_create_page(self, valid(name="bad name!", vcpu="2")) + self.assertTrue(response.context["refused"]) + self.assertTrue(response.context["vm_form"].is_bound) + self.assertIn("name", response.context["vm_form"].errors) def test_a_refusal_after_validation_keeps_the_form(self): for refusal in (claims.Busy("compute 1"), jobs.SubmissionConflict("already submitted"), util.OperationError("A virtual machine with this name already exists")): with self.subTest(refusal=type(refusal).__name__), \ patch("instances.views.jobs.submit", side_effect=refusal): - _, response = render_create_page(self, valid(form_id="template")) + _, response = render_create_page(self, valid(disk_source="template")) self.assertEqual(response.status_code, 200) - self.assertEqual(response.context["submitted_form"], "template") - self.assertTrue(response.context["template_form"].is_bound) + self.assertEqual(response.context["disk_source"], "template") + self.assertTrue(response.context["vm_form"].is_bound) self.assertTrue(list(response.context["messages"])) - # the view's own refusal: a new disk needs a MAC - _, response = render_create_page(self, valid(form_id="template", hdd_size="10", mac="")) - self.assertEqual(response.context["submitted_form"], "template") - self.assertTrue(response.context["template_form"].is_bound) - def test_a_stale_form_id_is_no_form(self): - _, response = render_create_page(self, valid(form_id="flavor999", name="bad name!")) - self.assertEqual(response.context["submitted_form"], "") - # no form shows the error: it goes to the messages - self.assertTrue(any("special characters" in str(m) for m in response.context["messages"])) + def test_a_new_disk_without_a_mac_is_queued(self): + # as for the other sources, libvirt gives the interface its MAC + _, response = render_create_page(self, valid(hdd_size="10", storage="default-pool", mac="")) + self.assertEqual(response.status_code, 302) + self.assertEqual(Operation.objects.get().kind, "instance.create") - def test_a_valid_post_with_a_stale_form_id_still_queues_the_job(self): + def test_no_form_without_a_storage_pool(self): + with patch("vrtManager.connection.wvmConnect.get_storages", return_value=[]): + _, response = render_create_page(self) + self.assertIsNone(response.context["vm_form"]) + self.assertEqual(html.fromstring(response.content).xpath("//button[@name='create']"), []) + self.assertTrue(any("storage pools" in str(m) for m in response.context["messages"])) + + def test_a_host_that_cannot_be_read_is_a_message(self): + import libvirt + with patch("instances.views.wvmCreate", side_effect=libvirt.libvirtError("unreachable")): + _, response = render_create_page(self) + self.assertEqual(response.status_code, 200) + self.assertTrue(any("unreachable" in str(m) for m in response.context["messages"])) + + def test_a_valid_post_queues_the_job(self): compute, response = render_create_page(self, valid( - form_id="flavor999", hdd_size="10", storage="default-pool", mac="52:54:00:00:00:01")) + disk_source="new", hdd_size="10", storage="default-pool", mac="52:54:00:00:00:01")) self.assertRedirects(response, reverse("instances", args=[compute.id]), fetch_redirect_response=False) self.assertEqual(Operation.objects.get().kind, "instance.create") + def test_two_disk_sources_are_refused(self): + # the page sends only the chosen source; a crafted request with two queues nothing + _, response = render_create_page(self, valid( + hdd_size="10", template="base.img", storage="default-pool", mac="52:54:00:00:00:01")) + self.assertEqual(response.status_code, 200) + self.assertTrue(any("Choose one disk source" in str(m) for m in response.context["messages"])) + self.assertFalse(Operation.objects.exists()) + + def test_only_the_chosen_disk_source_is_enabled(self): + for posted, chosen in (("template", "template"), ("existing", "existing"), ("nonsense", "new")): + with self.subTest(posted=posted): + _, response = render_create_page(self, valid(name="bad name!", disk_source=posted)) + page = html.fromstring(response.content) + self.assertEqual(page.xpath("//input[@name='disk_source'][@checked]/@value"), [chosen]) + for source in ("new", "existing", "template"): + fieldset = page.get_element_by_id(f"source-{source}") + off = source != chosen + self.assertEqual((fieldset.get("disabled") is not None, fieldset.get("hidden") is not None), + (off, off), source) + class CreateFormValuesTests(TestCase): - """The custom and template forms render their own form's values and errors.""" - - def setUp(self): - Flavor.objects.create(label="wvc-test", memory=512, vcpu=1, disk=10) + """The form renders the submitted values and errors.""" def page(self, **fields): _, response = render_create_page(self, valid(**fields)) self.assertEqual(response.status_code, 200) return html.fromstring(response.content) - def summary(self, page, form_label): - return " ".join(page.xpath(f"//form[@aria-label='{form_label}']//*[contains(@class, 'form-errors')]//text()")) + def summary(self, page): + return " ".join(page.xpath("//form[@id='create-vm-form']//*[contains(@class, 'form-errors')]//text()")) - def test_the_custom_form_keeps_what_was_submitted(self): - page = self.page(form_id="custom", name="bad name!", vcpu="3", memory="2048", cache_mode="writeback") - self.assertEqual(page.get_element_by_id("custom_name").get("value"), "bad name!") - self.assertEqual(page.get_element_by_id("custom_vcpu").get("value"), "3") - self.assertEqual(page.get_element_by_id("custom_memory").get("value"), "2048") - self.assertEqual(page.xpath("//select[@id='cache_mode']/option[@selected]/@value"), ["writeback"]) - self.assertIsNone(page.get_element_by_id("custom_qemu_ga").get("checked")) # not submitted: unchecked - # the template form keeps its defaults - self.assertEqual(page.get_element_by_id("template_vcpu").get("value"), "1") - self.assertIsNotNone(page.get_element_by_id("template_qemu_ga").get("checked")) + def test_the_form_keeps_what_was_submitted(self): + page = self.page(name="bad name!", vcpu="3", memory="2048", cache_mode="writeback") + self.assertEqual(page.get_element_by_id("vm_name").get("value"), "bad name!") + self.assertEqual(page.get_element_by_id("vm_vcpu").get("value"), "3") + self.assertEqual(page.get_element_by_id("vm_memory").get("value"), "2048") + self.assertEqual(page.xpath("//select[@id='vm_cache_mode']/option[@selected]/@value"), ["writeback"]) + self.assertIsNone(page.get_element_by_id("vm_qemu_ga").get("checked")) # not submitted: unchecked + + def test_a_new_form_has_its_defaults(self): + _, response = render_create_page(self) + page = html.fromstring(response.content) + self.assertEqual(page.get_element_by_id("vm_vcpu").get("value"), "1") + self.assertIsNotNone(page.get_element_by_id("vm_qemu_ga").get("checked")) def test_a_blank_number_stays_blank(self): - page = self.page(form_id="custom", vcpu="") - self.assertEqual(page.get_element_by_id("custom_vcpu").get("value"), "") - self.assertTrue(page.get_element_by_id("custom_vcpu_error").text_content().strip()) + page = self.page(vcpu="") + self.assertEqual(page.get_element_by_id("vm_vcpu").get("value"), "") + self.assertTrue(page.get_element_by_id("vm_vcpu_error").text_content().strip()) def test_the_name_is_escaped(self): - page = self.page(form_id="custom", name='x"') - self.assertEqual(page.get_element_by_id("custom_name").get("value"), 'x"') - self.assertEqual(page.xpath("//form[@aria-label='Create custom instance form']//b"), []) + page = self.page(name='x"') + self.assertEqual(page.get_element_by_id("vm_name").get("value"), 'x"') + self.assertEqual(page.xpath("//form[@id='create-vm-form']//b"), []) def test_the_error_sits_beside_its_field(self): - page = self.page(form_id="custom", name="bad name!") - field = page.get_element_by_id("custom_name") + page = self.page(name="bad name!") + field = page.get_element_by_id("vm_name") self.assertIn("is-invalid", field.get("class").split()) - self.assertEqual((field.get("aria-invalid"), field.get("aria-describedby")), ("true", "custom_name_error")) - self.assertIn("special characters", page.get_element_by_id("custom_name_error").text_content()) - # and in the summary at the top of its form - self.assertIn("Name: The name of the virtual machine", self.summary(page, "Create custom instance form")) + self.assertEqual(field.get("aria-invalid"), "true") + # described by its help and its error, in one attribute + self.assertEqual(field.get("aria-describedby").split(), ["vm_name_help", "vm_name_error"]) + self.assertIn("special characters", page.get_element_by_id("vm_name_error").text_content()) + # and in the summary at the top of the form + self.assertIn("Name: The name of the virtual machine", self.summary(page)) - def test_the_template_form_keeps_its_pool_template_and_storage(self): - page = self.page(form_id="template", name="bad name!", template_pool="default-pool", + def test_the_template_source_keeps_its_pool_template_and_storage(self): + page = self.page(name="bad name!", disk_source="template", template_pool="default-pool", template="base.img", storage="default-pool") - form = page.xpath("//form[@aria-label='Create instance from template form']")[0] - self.assertEqual((form.get("data-template-pool"), form.get("data-template"), form.get("data-storage")), - ("default-pool", "base.img", "default-pool")) - self.assertEqual(page.get_element_by_id("template_disk").get("name"), "template_pool") + form = page.get_element_by_id("create-vm-form") + self.assertEqual((form.get("data-template-pool"), form.get("data-template")), ("default-pool", "base.img")) + self.assertEqual(page.xpath("//select[@id='template_storage']/option[@selected]/@value"), ["default-pool"]) def test_an_error_of_a_field_the_form_hides_is_in_its_summary(self): - # the custom form's networks are a hidden input behind a picker - page = self.page(form_id="custom", networks="") - self.assertIn("Networks: This field is required.", self.summary(page, "Create custom instance form")) - - def test_each_form_sends_its_own_form_id(self): - page = self.page(form_id="custom", name="bad name!") - for label, form_id in (("Create custom instance form", "custom"), ("Create instance from template form", "template")): - with self.subTest(form=form_id): - form = page.xpath(f"//form[@aria-label='{label}']")[0] - self.assertEqual(form.xpath(".//input[@name='form_id']/@value"), [form_id]) + # the networks are a hidden input behind a picker + page = self.page(networks="") + self.assertIn("Networks: This field is required.", self.summary(page)) def test_a_firmware_error_without_a_firmware_choice_is_in_the_summary(self): # an architecture with no firmware label renders no firmware select with patch("vrtManager.connection.wvmConnect.label_for_firmware_path", return_value=None): - fields = valid(form_id="custom") + fields = valid() del fields["firmware"] _, response = render_create_page(self, fields) page = html.fromstring(response.content) self.assertEqual(page.xpath("//select[@name='firmware']"), []) - self.assertIn("Firmware: This field is required.", self.summary(page, "Create custom instance form")) + self.assertIn("Firmware: This field is required.", self.summary(page)) -class FlavorFormTests(TestCase): - """Each flavor's modal form renders its own form; the refused one comes back.""" +class FlavorPresetTests(TestCase): + """Flavors are presets of the one form; they are managed outside it.""" def setUp(self): - self.first = Flavor.objects.create(label="wvc-test-1", memory=512, vcpu=1, disk=10) - self.second = Flavor.objects.create(label="wvc-test-2", memory=1024, vcpu=2, disk=20) + self.flavor = Flavor.objects.create(label="wvc-test", memory=512, vcpu=1, disk=10) - def page(self, **fields): - _, response = render_create_page(self, valid(**fields)) - self.assertEqual(response.status_code, 200) - return html.fromstring(response.content) + def test_a_flavor_is_a_preset_button(self): + _, response = render_create_page(self) + page = html.fromstring(response.content) + [preset] = page.xpath(f"//button[contains(@class, 'preset')][@data-label='{self.flavor.label}']") + self.assertEqual((preset.get("data-vcpu"), preset.get("data-memory"), preset.get("data-disk")), ("1", "512", "10")) + self.assertEqual((preset.get("type"), preset.get("aria-pressed")), ("button", "false")) - def test_only_the_refused_flavor_comes_back(self): - page = self.page(form_id=f"flavor{self.second.id}", name="bad name!", vcpu="2", memory="1024", hdd_size="20") - self.assertEqual(page.get_element_by_id(f"flavor{self.second.id}_name").get("value"), "bad name!") - self.assertEqual(page.get_element_by_id(f"flavor{self.first.id}_name").get("value"), "") - self.assertTrue(page.get_element_by_id(f"flavor{self.first.id}_mac").get("value")) - - def test_a_flavor_field_points_at_its_error(self): - page = self.page(form_id=f"flavor{self.second.id}", name="bad name!", vcpu="2", memory="1024", hdd_size="20") - field = page.get_element_by_id(f"flavor{self.second.id}_name") - error = page.get_element_by_id(field.get("aria-describedby")) - self.assertIn("special characters", error.text_content()) - - def test_a_reopened_flavor_holds_the_message(self): - with patch("instances.views.jobs.submit", - side_effect=util.OperationError("A virtual machine with this name already exists")): - page = self.page(form_id=f"flavor{self.second.id}", vcpu="2", memory="1024", hdd_size="20", - storage="default-pool", mac="52:54:00:00:00:01") - messages = page.xpath(f"//*[@id='addVMflavor{self.second.id}']//*[contains(@class, 'modal-messages')]") - self.assertIn("A virtual machine with this name already exists", messages[0].text_content()) - - def test_the_flavor_form_sends_one_cache_mode(self): - page = self.page(form_id="custom", name="bad name!") - form = page.get_element_by_id(f"addVMflavor{self.first.id}").xpath(".//form")[0] - self.assertEqual(len(form.xpath(".//*[@name='cache_mode']")), 1) - self.assertEqual(form.xpath(".//input[@name='form_id']/@value"), [f"flavor{self.first.id}"]) + def test_the_flavor_dialog_sits_outside_the_form(self): + _, response = render_create_page(self) + page = html.fromstring(response.content) + self.assertEqual(page.xpath("//form[@id='create-vm-form']//*[@id='flavors-dialog']"), []) + actions = page.get_element_by_id("flavors-dialog").xpath(".//form/@action") + self.assertIn(reverse("instances:flavor_delete", args=[self.flavor.id]), actions) + self.assertEqual(actions[-1], reverse("instances:flavor_create")) -class CustomListsTests(TestCase): - """The custom form's disks and networks come back with a refusal.""" +class SubmittedListsTests(TestCase): + """The volumes and networks of a refused form come back with it.""" - def custom_lists(self, response): - """The custom form's disks and networks the page restores, or None.""" - element = html.fromstring(response.content).get_element_by_id("custom-lists", None) + def lists(self, response): + """The volumes and networks the page restores, or None.""" + element = html.fromstring(response.content).get_element_by_id("submitted-lists", None) return None if element is None else json.loads(element.text) - def test_a_refused_custom_form_sends_back_its_lists(self): + def test_a_refused_form_sends_back_its_lists(self): _, response = render_create_page(self, valid( - form_id="custom", name="bad name!", **{"storage-control": "default"}, images="a.qcow2,b.qcow2", + name="bad name!", disk_source="existing", **{"storage-control": "default"}, images="a.qcow2,b.qcow2", device0="cdrom", bus0="sata", device1="disk", bus1="virtio", networks="default,isolated", mac="52:54:00:00:00:01")) - self.assertEqual(self.custom_lists(response), { + self.assertEqual(self.lists(response), { "pool": "default", "disks": [{"volume": "a.qcow2", "device": "cdrom", "bus": "sata"}, {"volume": "b.qcow2", "device": "disk", "bus": "virtio"}], @@ -204,12 +207,10 @@ class CustomListsTests(TestCase): def test_a_hostile_volume_name_round_trips_through_the_lists(self): name = "x'\">" - _, response = render_create_page(self, valid(form_id="custom", name="bad name!", images=name)) - self.assertEqual(self.custom_lists(response)["disks"][0]["volume"], name) + _, response = render_create_page(self, valid(name="bad name!", disk_source="existing", images=name)) + self.assertEqual(self.lists(response)["disks"][0]["volume"], name) self.assertNotIn(b"", response.content) - def test_only_a_refused_custom_form_sends_lists(self): + def test_only_a_refused_form_sends_lists(self): _, response = render_create_page(self) - self.assertIsNone(self.custom_lists(response)) - _, response = render_create_page(self, valid(form_id="template", name="bad name!", images="a.qcow2")) - self.assertIsNone(self.custom_lists(response)) + self.assertIsNone(self.lists(response)) diff --git a/instances/tests/test_create_page_js.py b/instances/tests/test_create_page_js.py index 35f0d678..fd688781 100644 --- a/instances/tests/test_create_page_js.py +++ b/instances/tests/test_create_page_js.py @@ -15,20 +15,22 @@ from instances.tests.create_page import render_create_page from lxml import html -# Just enough DOM for the custom form's rows: elements reach their parts by +# Just enough DOM for the page's scripts: elements reach their parts by # selector, a row template clones a fresh row, and writing HTML throws (rows # are built from text and nodes only). A random-MAC request waits in replies. FAKE_DOM = """ class El { constructor(parts = {}) { Object.assign(this, {parts, dataset: {}, value: "", name: "", textContent: "", children: [], listeners: {}, - classes: new Set(), options: []}); - this.classList = {toggle: (c, on) => on ? this.classes.add(c) : this.classes.delete(c)}; + classes: new Set(), options: [], attrs: {}}); + this.classList = {toggle: (c, on) => on ? this.classes.add(c) : this.classes.delete(c), + replace: (a, b) => this.classes.delete(a) && !!this.classes.add(b)}; } set innerHTML(v) { throw new Error("HTML written: " + v); } part(sel) { return this.parts[sel]; } querySelector(sel) { return this.parts[sel]; } addEventListener(event, f) { this.listeners[event] = f; } + setAttribute(k, v) { this.attrs[k] = String(v); } append(child) { child.parent = this; this.children.push(child); } add(option) { this.options.push(option); } remove() { this.parent.children.splice(this.parent.children.indexOf(this), 1); } @@ -37,10 +39,10 @@ const row_template = names => ({content: {firstElementChild: {cloneNode: () => new El(Object.fromEntries(names.map(n => [n, new El()])))}}}); const templates = {"disk-row": row_template([".row-label", ".disk-device", ".disk-bus", ".row-item", ".row-remove"]), "network-row": row_template(["input", ".row-label", ".row-item", ".row-remove"])}; -const fields = {}; +const fields = {}, all = {}; const field = sel => fields[sel] ??= new El(); const rows = list => field(list).children; -const document = {getElementById: id => templates[id], querySelector: field}; +const document = {getElementById: id => templates[id], querySelector: field, querySelectorAll: sel => all[sel] ?? []}; const replies = []; const $ = Object.assign(() => ({ready: f => f()}), {getJSON: (url, f) => { replies.push(f); return {}; }}); const Option = function (text, value) { return {text, value}; }; @@ -51,9 +53,27 @@ class TomSelect { addOption(o) { this.added.push("option " + o.value); } addItem(v) { this.added.push("item " + v); this.settings.onItemAdd(v); } removeItem(v) { this.settings.onItemRemove(v); } + enable() { this.enabled = true; } + disable() { this.enabled = false; } } """ +ROWS = ("make_pickers", "new_row", "add_disk_row", "add_network_row", "row_for", "remove_row", "sync_lists", + "random_mac", "load_buses") + + +def functions(page, *names): + """The page's script functions of these names, in this order.""" + return "".join(re.search(rf"\n function {name}\(.*?\n }}\n", page, re.S).group(0) for name in names) + + +def node(script): + """Run script in Node; returns what it printed, parsed as JSON.""" + result = subprocess.run(["node", "-e", script], capture_output=True, text=True, timeout=30) + if result.returncode: + raise AssertionError(result.stderr) + return json.loads(result.stdout) + class CreatePageScriptTests(TestCase): def page(self): @@ -64,18 +84,14 @@ class CreatePageScriptTests(TestCase): @unittest.skipUnless(shutil.which("node"), "node is not installed") def test_the_disk_bus_list_is_asked_for_the_page_architecture_and_machine(self): compute, page = self.page() - function = re.search(r"function get_disk_bus_choices\(.*?\n }\n", page, re.S).group(0) - script = "let asked; const $ = Object.assign(() => ({find: () => ({remove() {}}), append() {}, val() {}}), " \ - "{getJSON: url => { asked = url; }, each() {}});\n" + function + \ - f"get_disk_bus_choices({compute.id}, {{}}, 'cdrom'); console.log(asked);" - result = subprocess.run(["node", "-e", script], capture_output=True, text=True, timeout=30) - self.assertEqual(result.returncode, 0, result.stderr) - self.assertEqual(result.stdout.strip(), reverse("buses", args=[compute.id, "i686", "pc", "cdrom"])) + out = node(f"const COMPUTE = {compute.id}; let asked; const $ = {{getJSON: url => {{ asked = url; }}}};" + + functions(page, "load_buses") + "load_buses({}, 'cdrom'); console.log(JSON.stringify(asked));") + self.assertEqual(out, reverse("buses", args=[compute.id, "i686", "pc", "cdrom"])) - def test_the_custom_form_has_its_own_mac_field(self): + def test_the_form_has_one_mac_field(self): _, page = self.page() - custom = html.fromstring(page).xpath("//form[@aria-label='Create custom instance form']")[0] - self.assertEqual(custom.xpath(".//input[@name='mac']/@id"), ["custom_mac"]) + form = html.fromstring(page).get_element_by_id("create-vm-form") + self.assertEqual(form.xpath(".//input[@name='mac']/@id"), ["vm_mac"]) def test_the_row_templates_hold_what_the_rows_use(self): # the test driver reports no disk devices or buses @@ -91,6 +107,12 @@ class CreatePageScriptTests(TestCase): for part in ("row-label", "row-item", "row-remove"): self.assertTrue(row.xpath(f".//*[contains(@class, '{part}')]"), part) + def run_rows(self, test): + """Run the page's row script against the fake DOM, then test.""" + _, page = self.page() + return node(FAKE_DOM + "let images_control, networks_control;" + functions(page, *ROWS) + + "make_pickers();" + test) + @unittest.skipUnless(shutil.which("node"), "node is not installed") def test_adding_a_disk_keeps_the_others(self): out = self.run_rows(""" @@ -120,7 +142,7 @@ const first = rows("#net-list")[0]; first.part("input").value = "52:54:00:00:00:01"; first.part("input").listeners.input(); networks_control.addItem("isolated"); replies[0]({mac: "52:54:00:aa:aa:aa"}); replies[1]({mac: "52:54:00:bb:bb:bb"}); -console.log(JSON.stringify([field("#networks").value, field("#custom_mac").value]));""") +console.log(JSON.stringify([field("#networks").value, field("#vm_mac").value]));""") self.assertEqual(out, ["default,isolated", "52:54:00:00:00:01,52:54:00:bb:bb:bb"]) @unittest.skipUnless(shutil.which("node"), "node is not installed") @@ -129,109 +151,149 @@ console.log(JSON.stringify([field("#networks").value, field("#custom_mac").value out = self.run_rows(f"console.log(JSON.stringify(add_disk_row({json.dumps(name)}).part('.row-item').textContent));") self.assertEqual(out, name) - def run_rows(self, test): - """Run the custom form's row script against a fake DOM, then test; - returns what test printed, parsed as JSON.""" - _, page = self.page() - functions = re.search(r" var images_control, networks_control;.*?\n function random_mac\(.*?\n }\n", - page, re.S).group(0) - result = subprocess.run(["node", "-e", FAKE_DOM + functions + test], capture_output=True, text=True, timeout=30) - self.assertEqual(result.returncode, 0, result.stderr) - return json.loads(result.stdout) - - def test_the_selects_without_a_label_of_their_own_are_named(self): + def test_the_pickers_are_named(self): # TomSelect points the label of a select it enhances at its own - # input; the template select sits beside the template pool's label - _, page = self.page() - page = html.fromstring(page) - for select in ("image-control", "network-control", "template"): + # input, so the select is named by aria-label too + page = html.fromstring(self.page()[1]) + for select in ("image-control", "network-control"): with self.subTest(select=select): + self.assertTrue(page.xpath(f"//label[@for='{select}']")) self.assertTrue(page.get_element_by_id(select).get("aria-label")) def test_a_mac_per_nic_fits_the_form(self): macs = ",".join(f"52:54:00:00:00:{n:02x}" for n in range(8)) self.assertEqual(NewVMForm.base_fields["mac"].clean(macs), macs) - @unittest.skipUnless(shutil.which("node"), "node is not installed") - def test_the_page_reopens_the_submitted_form(self): - _, page = self.page() - functions = re.search(r"function reopen_submitted_form\(.*?\n \$\(reopen_submitted_form\);", page, re.S).group(0) - harness = """ -const shown = []; -// the forms' holders: two tab panes and a flavor's dialog, each with an -// "Advanced" section that holds a refused field; a dialog's "shown" -// listeners run when it is shown, as in Bootstrap -const holder = (id, kind) => ({id, listeners: [], classList: {contains: c => c === kind}, - querySelectorAll: () => [{name: "#" + id + " .collapse", querySelector: () => ({})}], - addEventListener(event, f) { this.listeners.push(f); }}); -const holders = {custom: holder("addCustom", "tab-pane"), template: holder("add-from-temp", "tab-pane"), - flavor7: holder("addVMflavor7", "modal")}; -const document = {body: {dataset: {submittedForm: %s}}, getElementById: () => null, querySelector: sel => { - const id = (sel.match(/data-form-id="([^"]*)"/) || [])[1]; - if (id !== undefined) return holders[id] || null; - return sel.startsWith("form[") ? null : {name: sel}; // no refused template form here -}}; -const bootstrap = {Tab: {getOrCreateInstance: e => ({show: () => shown.push("tab " + e.name)})}, - Modal: {getOrCreateInstance: e => ({show: () => { shown.push("modal #" + e.id); e.listeners.forEach(f => f()); }})}, - Collapse: {getOrCreateInstance: e => ({show: () => shown.push("collapse " + e.name)})}}; -const $ = Object.assign(() => ({val() {}}), {when: () => ({then: f => f()})}); -const focusFirstError = () => shown.push("focus"); -%s -reopen_submitted_form(); console.log(JSON.stringify(shown)); + +class DiskSourceAndPresetScriptTests(TestCase): + """The disk source choice enables only its own fields; a preset fills the + sizes, picks a new disk and is announced.""" + + HARNESS = FAKE_DOM + """ +let images_control = new TomSelect("#image-control", {}); +const fieldsets = ["new", "existing", "template"].map(s => Object.assign(new El(), {id: "source-" + s})); +all[".disk-source"] = fieldsets; +const presets = [["small", "2", "2048", "40"], ["large", "4", "8192", "80"]].map(function ([label, vcpu, memory, disk]) { + const button = new El(); + Object.assign(button.dataset, {label, vcpu, memory, disk}); + button.classes.add("text-body"); + return button; +}); +all[".preset"] = presets; +const sources = () => fieldsets.map(f => [f.id, !!f.hidden, !!f.disabled]); +const pressed = () => presets.map(b => [b.attrs["aria-pressed"], [...b.classes].join(" ")]); """ - for submitted, expected in ( - ("custom", ["collapse #addCustom .collapse", 'tab [data-bs-target="#addCustom"]', "focus"]), - ("template", ["collapse #add-from-temp .collapse", 'tab [data-bs-target="#add-from-temp"]', "focus"]), - ("flavor7", ["collapse #addVMflavor7 .collapse", "modal #addVMflavor7", "focus"]), - ("", ["focus"]), - ): - with self.subTest(submitted=submitted): - result = subprocess.run(["node", "-e", harness % (json.dumps(submitted), functions.replace("$(reopen_submitted_form);", ""))], - capture_output=True, text=True, timeout=30) - self.assertEqual(result.returncode, 0, result.stderr) - self.assertEqual(json.loads(result.stdout), expected) + + def run_script(self, test): + _, response = render_create_page(self) + page = response.content.decode() + return node(self.HARNESS + functions(page, "show_source", "apply_preset", "clear_presets") + test) + + @unittest.skipUnless(shutil.which("node"), "node is not installed") + def test_only_the_chosen_source_is_shown_and_sent(self): + out = self.run_script(""" +show_source("existing"); const existing = [sources(), images_control.enabled]; +show_source("template"); const template = [sources(), images_control.enabled]; +console.log(JSON.stringify({existing, template}));""") + self.assertEqual(out["existing"], [[["source-new", True, True], ["source-existing", False, False], + ["source-template", True, True]], True]) + self.assertEqual(out["template"], [[["source-new", True, True], ["source-existing", True, True], + ["source-template", False, False]], False]) + + @unittest.skipUnless(shutil.which("node"), "node is not installed") + def test_a_preset_fills_the_sizes_and_picks_a_new_disk(self): + out = self.run_script(""" +show_source("template"); +apply_preset(presets[1]); +const applied = {sizes: ["#vm_vcpu", "#vm_memory", "#vm_hdd_size"].map(s => field(s).value), + radio: field('input[name="disk_source"][value="new"]').checked, sources: sources(), pressed: pressed(), + status: field("#preset-status").textContent}; +clear_presets(); +console.log(JSON.stringify({applied, cleared: pressed()}));""") + applied = out["applied"] + self.assertEqual(applied["sizes"], ["4", "8192", "80"]) + self.assertTrue(applied["radio"]) + self.assertEqual(applied["sources"][0], ["source-new", False, False]) + self.assertEqual(applied["pressed"], [["false", "text-body"], ["true", "btn-primary"]]) + self.assertEqual(applied["status"], "large: 4 vCPU, 8192 MiB, new 80 GiB disk") + self.assertEqual(out["cleared"], [["false", "text-body"], ["false", "text-body"]]) + + +class NewFormScriptTests(TestCase): + """A new form starts with one interface; the template's copy goes into the + template's pool unless the user picks another.""" + + def script(self, test): + _, response = render_create_page(self) + page = response.content.decode() + return node(FAKE_DOM + "let images_control, networks_control;" + functions(page, *ROWS) + + functions(page, "add_first_network", "select_value", "template_pool_chosen") + "make_pickers();" + test) + + @unittest.skipUnless(shutil.which("node"), "node is not installed") + def test_a_new_form_starts_on_default_if_the_host_has_it(self): + for networks, first in ((["isolated", "default"], "default"), (["isolated", "lab"], "isolated"), ([], None)): + with self.subTest(networks=networks): + out = self.script(f"networks_control.options = Object.fromEntries({json.dumps(networks)}.map(n => [n, {{}}]));" + "add_first_network(); console.log(JSON.stringify(rows('#net-list').map(r => r.dataset.item)));") + self.assertEqual(out, [first] if first else []) + + @unittest.skipUnless(shutil.which("node"), "node is not installed") + def test_the_copy_goes_into_the_template_pool(self): + out = self.script(""" +const load_templates = pool => {}; +field("#template_storage").options = [{value: "default"}, {value: "templates"}]; +template_pool_chosen("templates"); +console.log(JSON.stringify(field("#template_storage").value));""") + self.assertEqual(out, "templates") + + +class RestoreScriptTests(TestCase): + """A refused form's lists and template come back, loaded in order, then + the focus goes to the first error.""" LISTS = {"pool": "default", "disks": [{"volume": "a.qcow2", "device": "cdrom", "bus": "sata"}, {"volume": "b.qcow2", "device": "disk", "bus": "virtio"}], "networks": [{"name": "default", "mac": "52:54:00:00:00:01"}, {"name": "isolated", "mac": ""}]} - def run_restore(self, test): - """Run restore_custom_lists on LISTS against the fake DOM, with - loaders that answer when the test calls answer(); returns what the - test printed, parsed as JSON.""" - _, page = self.page() - rows = re.search(r" var images_control, networks_control;.*?\n function random_mac\(.*?\n }\n", page, re.S) - restore = re.search(r" // The custom form's lists as submitted.*?\n function select_value\(.*?\n }\n", page, re.S) - harness = FAKE_DOM + rows.group(0) + restore.group(0) + """ -templates["custom-lists"] = {textContent: %s}; + def run_restore(self, test, lists=LISTS): + """Run restore_submitted against the fake DOM, with loaders that + answer when the test calls answer().""" + _, response = render_create_page(self) + page = response.content.decode() + harness = FAKE_DOM + "let images_control, networks_control;" \ + + functions(page, *ROWS[:-1], "select_value", "restore_submitted") + """ +templates["submitted-lists"] = {textContent: %s}; $.when = (...loading) => Promise.all(loading); const log = [], answers = []; const later = name => { log.push(name); return new Promise(done => answers.push(() => { log.push("loaded " + name); done(); })); }; -const get_cust_vols = (compute, pool) => later("volumes " + pool); -const get_disk_bus_choices = (compute, bus, device) => later("buses " + device); +const load_volumes = pool => later("volumes " + pool); +const load_buses = (bus, device) => later("buses " + device); +const load_templates = pool => later("templates " + pool); +const bootstrap = {Collapse: {getOrCreateInstance: () => ({show: () => log.push("open advanced")})}}; +let focused; +const restored = new Promise(done => { focused = done; }); +const focusFirstError = () => { log.push("focus"); focused(); }; const settle = () => new Promise(done => setTimeout(done)); const answer = async () => { await settle(); answers.shift()(); await settle(); }; const disks = () => rows("#img-list").map(r => [r.dataset.item, r.part(".disk-device").value, r.part(".disk-bus").value]); -""" % json.dumps(json.dumps(self.LISTS)) - result = subprocess.run(["node", "-e", harness + "(async () => {" + test + "})();"], - capture_output=True, text=True, timeout=30) - self.assertEqual(result.returncode, 0, result.stderr) - return json.loads(result.stdout) +make_pickers(); +""" % json.dumps(json.dumps(lists)) + return node(harness + "(async () => {" + test + "})();") @unittest.skipUnless(shutil.which("node"), "node is not installed") - def test_restoring_the_custom_form_rebuilds_its_rows(self): + def test_the_lists_are_rebuilt_in_order(self): out = self.run_restore(""" -const restored = restore_custom_lists().then(() => log.push("restored")); +restore_submitted(); await answer(); const before = disks(); await answer(); await answer(); await restored; -const typed_before_reply = field("#custom_mac").value; +const typed_before_reply = field("#vm_mac").value; replies[0]({mac: "52:54:00:00:00:bb"}); console.log(JSON.stringify({log, before, after: disks(), networks: field("#networks").value, - typed_before_reply, macs: field("#custom_mac").value, random_requests: replies.length}));""") + typed_before_reply, macs: field("#vm_mac").value, random_requests: replies.length}));""") self.assertEqual(out["log"], ["volumes default", "loaded volumes default", "buses cdrom", "buses disk", - "loaded buses cdrom", "loaded buses disk", "restored"]) + "loaded buses cdrom", "loaded buses disk", "focus"]) self.assertEqual(out["before"], [["a.qcow2", "cdrom", ""], ["b.qcow2", "disk", ""]]) self.assertEqual(out["after"], [["a.qcow2", "cdrom", "sata"], ["b.qcow2", "disk", "virtio"]]) self.assertEqual(out["networks"], "default,isolated") @@ -242,7 +304,7 @@ console.log(JSON.stringify({log, before, after: disks(), networks: field("#netwo @unittest.skipUnless(shutil.which("node"), "node is not installed") def test_a_value_gone_from_the_lists_is_kept(self): out = self.run_restore(""" -const restored = restore_custom_lists(); +restore_submitted(); await answer(); const bus = rows("#img-list")[0].part(".disk-bus"); bus.options.push({value: "virtio"}); // the loaded list has no sata @@ -252,3 +314,23 @@ console.log(JSON.stringify({volumes: images_control.added, bus: bus.value, self.assertEqual(out["volumes"], ["option a.qcow2", "item a.qcow2", "option b.qcow2", "item b.qcow2"]) self.assertEqual(out["bus"], "sata") self.assertEqual(out["options"], ["virtio", "sata"]) + + @unittest.skipUnless(shutil.which("node"), "node is not installed") + def test_an_advanced_section_with_an_error_opens_before_the_focus(self): + out = self.run_restore(""" +field("#advanced").parts[".is-invalid"] = new El(); +restore_submitted(); await restored; +console.log(JSON.stringify(log));""", lists={"pool": "", "disks": [], "networks": []}) + self.assertEqual(out, ["open advanced", "focus"]) + + @unittest.skipUnless(shutil.which("node"), "node is not installed") + def test_the_template_comes_back_after_its_pool_loads(self): + out = self.run_restore(""" +Object.assign(field("#create-vm-form").dataset, {templatePool: "pool-a", template: "base.img"}); +restore_submitted(); +const pool = field("#template_pool").value; +await answer(); await restored; +console.log(JSON.stringify({log, pool, template: field("#template").value}));""", + lists={"pool": "", "disks": [], "networks": []}) + self.assertEqual(out, {"log": ["templates pool-a", "loaded templates pool-a", "focus"], + "pool": "pool-a", "template": "base.img"}) diff --git a/instances/tests/test_create_wizard_url.py b/instances/tests/test_create_wizard_url.py index e9b5ce45..b1fbb678 100644 --- a/instances/tests/test_create_wizard_url.py +++ b/instances/tests/test_create_wizard_url.py @@ -18,8 +18,8 @@ class CreateWizardUrlTests(TestCase): request = RequestFactory().get("/") request.user = get_user_model().objects.create_superuser("cw-admin", "cw@example.com", "pw") page = render_to_string("create_instance_w1.html", {"compute": compute}, request=request) - # the statement that builds the URL from arch and machine - return re.search(r"\burl = (.*?);", page, re.S).group(1) + # the function that builds the URL from arch and machine + return re.search(r"function create_url\(arch, machine\) \{.*?\n \}", page, re.S).group(0) @unittest.skipUnless(shutil.which("node"), "node is not installed") def test_arch_and_machine_names_land_in_their_segments(self): @@ -30,7 +30,24 @@ class CreateWizardUrlTests(TestCase): ("aarch64", "virt"): "aarch64/virt/", } for (arch, machine), tail in cases.items(): - js = "const arch = %r, machine = %r; console.log(%s);" % (arch, machine, script) + js = "%s console.log(create_url(%r, %r));" % (script, arch, machine) out = subprocess.run(["node", "-e", js], capture_output=True, text=True, timeout=30) with self.subTest(arch=arch): self.assertTrue(out.stdout.strip().endswith(f"/{tail}"), out.stdout + out.stderr) + + @unittest.skipUnless(shutil.which("node"), "node is not installed") + def test_next_waits_for_a_machine_type(self): + compute = Compute.objects.create(name="cw2", hostname="10.0.0.7", login="u", password="p", type=1) + request = RequestFactory().get("/") + request.user = get_user_model().objects.create_superuser("cw2-admin", "cw2@example.com", "pw") + page = render_to_string("create_instance_w1.html", {"compute": compute, "compute_id": compute.id}, request=request) + function = re.search(r"function load_machines\(arch\) \{.*?\n \}", page, re.S).group(0) + for machines, disabled in (([], True), (["q35"], False)): + js = ("const next = {disabled: false}, select = {replaceChildren() {}};" + "const document = {querySelector: sel => sel === '#next' ? next : select};" + "const Option = function () {};" + f"const $ = {{getJSON: (url, f) => f({{machines: {machines!r}}})}};" + f"{function} load_machines('x86_64'); console.log(next.disabled);") + out = subprocess.run(["node", "-e", js], capture_output=True, text=True, timeout=30) + with self.subTest(machines=machines): + self.assertEqual(out.stdout.strip(), str(disabled).lower(), out.stderr) diff --git a/instances/views.py b/instances/views.py index 2a6a2f91..3f919cf0 100755 --- a/instances/views.py +++ b/instances/views.py @@ -1995,24 +1995,17 @@ def create_instance_select_type(request, compute_id): return render(request, "create_instance_w1.html", locals()) -def create_forms(flavors, defaults, mac): - """One unbound NewVMForm per create form of the page, keyed by its - form_id, each with that form's defaults.""" - forms = { - "custom": NewVMForm(initial={**defaults, "vcpu": 1, "memory": 512}), - "template": NewVMForm(initial={**defaults, "vcpu": 1, "memory": 512, "mac": mac}), - } - for flavor in flavors: - forms[f"flavor{flavor.id}"] = NewVMForm(initial={ - **defaults, "vcpu": flavor.vcpu, "memory": flavor.memory, "hdd_size": flavor.disk, - "mac": mac, - }) - return forms +# the create form's disk sources: (value, title, help) +DISK_SOURCES = [ + ("new", _("New disk"), _("An empty disk of the size you set")), + ("existing", _("Existing volumes"), _("Volumes already in a pool")), + ("template", _("Clone from template"), _("A copy of a template volume")), +] -def submitted_custom_lists(post): - """The custom form's image pool, disks and networks as submitted, for the - page to rebuild them.""" +def refused_lists(post): + """The volume pool, volumes and networks of a refused create form as + submitted, for the page to rebuild them.""" images = post["images"].split(",") if post.get("images") else [] networks = post["networks"].split(",") if post.get("networks") else [] macs = post.get("mac", "").split(",") @@ -2044,8 +2037,13 @@ def create_instance(request, compute_id, arch, machine): compute = get_object_or_404(Compute, pk=compute_id) flavors = Flavor.objects.filter().order_by("id") appsettings = AppSettings.objects.all() - vm_forms = {} - submitted_form = "" + vm_form = None + flavor_form = FlavorForm() + refused = False + disk_sources = DISK_SOURCES + disk_source = request.POST.get("disk_source", "new") + if disk_source not in [value for value, _title, _help in DISK_SOURCES]: + disk_source = "new" try: conn = wvmCreate( @@ -2070,7 +2068,6 @@ def create_instance(request, compute_id, arch, machine): default_disk_owner_gid = int(app_settings.INSTANCE_VOLUME_DEFAULT_OWNER_GID) default_scsi_disk_model = app_settings.INSTANCE_VOLUME_DEFAULT_SCSI_CONTROLLER listener_addr = settings.QEMU_CONSOLE_LISTENER_ADDRESSES - mac_auto = util.randomMAC() disk_devices = conn.get_disk_device_types(arch, machine) disk_buses = conn.get_disk_bus_types(arch, machine) cdrom_buses = [bus for bus in disk_buses if bus in CDROM_BUSES] @@ -2099,38 +2096,26 @@ def create_instance(request, compute_id, arch, machine): firmwares.append(label) firmwares = list(set(firmwares)) - flavor_form = FlavorForm() + if not storages: + raise util.OperationError(_("You haven't defined any storage pools")) + if not networks: + raise util.OperationError(_("You haven't defined any network pools")) - vm_forms = create_forms(flavors, mac=mac_auto, defaults={ - "firmware": default_firmware, "vcpu_mode": default_cpu_mode, "cache_mode": default_cache, - "net_model": default_nic_type, "listener_addr": "0.0.0.0", "add_cdrom": default_cdrom, - "add_input": default_input_device_bus, "video": default_video, "qemu_ga": True, - "virtio": bool(virtio_support), + vm_form = NewVMForm(initial={ + "vcpu": 1, "memory": 512, "firmware": default_firmware, "vcpu_mode": default_cpu_mode, + "cache_mode": default_cache, "net_model": default_nic_type, "listener_addr": "0.0.0.0", + "add_cdrom": default_cdrom, "add_input": default_input_device_bus, "video": default_video, + "qemu_ga": True, "virtio": bool(virtio_support), }) if conn: - if not storages: - raise util.OperationError(_("You haven't defined any storage pools")) - if not networks: - raise util.OperationError(_("You haven't defined any network pools")) - if request.method == "POST": if "create" in request.POST: - form = NewVMForm(request.POST) - # the submitted form comes back bound if the page shows it again; - # a stale form_id (a flavor deleted since) has no form for its errors - form_id = request.POST.get("form_id", "") - if form_id in vm_forms: - vm_forms[form_id] = form - submitted_form = form_id - else: - for name, errors in form.errors.items(): - for error in errors: - messages.error(request, f"{form[name].label}: {error}" if name in form.fields else error) + # the submitted form comes back bound if the page shows it again + form = vm_form = NewVMForm(request.POST) + refused = True if form.is_valid(): data = form.cleaned_data - if data["hdd_size"] and not data["mac"]: - raise util.OperationError(_("No Virtual Machine MAC has been entered")) images = data["images"].split(",") if data["images"] else [] raw = {**data, "compute": compute.id, "arch": arch, "machine": machine, "devices": [[request.POST.get(f"device{i}", ""), request.POST.get(f"bus{i}", "")] @@ -2156,10 +2141,10 @@ def create_instance(request, compute_id, arch, machine): except libvirtError as lib_err: messages.error(request, lib_err) - custom_form, template_form = vm_forms.get("custom"), vm_forms.get("template") - custom_lists = submitted_custom_lists(request.POST) if submitted_form == "custom" else None - flavor_forms = [(flavor, f"flavor{flavor.id}", vm_forms[f"flavor{flavor.id}"]) - for flavor in flavors if f"flavor{flavor.id}" in vm_forms] + if refused: + submitted_lists = refused_lists(request.POST) + if disk_source == "template": + template_restore = {"pool": request.POST.get("template_pool", ""), "template": request.POST.get("template", "")} return render(request, "create_instance_w2.html", locals()) diff --git a/static/css/webvirtcloud.css b/static/css/webvirtcloud.css index feffaa45..279dfca9 100644 --- a/static/css/webvirtcloud.css +++ b/static/css/webvirtcloud.css @@ -26,6 +26,25 @@ body { left: 0; } +/* Tom Select's control keeps the text colour of the select it replaces: + its own stylesheet fixes one, unreadable on some Bootswatch themes */ +.ts-wrapper.form-select .ts-control, +.ts-wrapper.form-select .ts-control input { + color: inherit; +} + +/* a disclosure's chevron points up while its section is open */ +.disclosure[aria-expanded="true"] .bi-chevron-down { + transform: rotate(180deg); +} + +/* the steps of creating a VM: the separator is drawn, not a list item */ +.create-steps li + li::before { + content: "›"; + margin-right: 0.5rem; + color: var(--bs-secondary-color); +} + .xmlheight { height: 364px; } diff --git a/static/icon_cache/chevron-right_None_None_None.svg b/static/icon_cache/chevron-right_None_None_None.svg new file mode 100644 index 00000000..850450c7 --- /dev/null +++ b/static/icon_cache/chevron-right_None_None_None.svg @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/static/icon_cache/x-lg_None_None_None.svg b/static/icon_cache/x-lg_None_None_None.svg new file mode 100644 index 00000000..8a7e3e6f --- /dev/null +++ b/static/icon_cache/x-lg_None_None_None.svg @@ -0,0 +1,8 @@ + + + + + + + +