fix(instances): the Resize tab changes only what was asked

Disk sizes were pre-filled rounded to whole GiB, so a 3.5 GiB disk showed
4 and was grown, for good, whenever another disk was resized; an empty
field or a disk outside a pool was a 500. A disk's field is now empty with
its size as the placeholder: an empty field leaves the disk alone, a
larger whole number grows it, anything else is refused with a message.

vCPUs change only on a shut-off VM, also on a direct request: a running
VM got them at its next start, past the quota counted against its live
count. Current vCPUs or memory above the maximum are refused instead of
failing (and dropping vCPU hot-plug) or being capped by libvirt. A running
VM's maximum memory select is disabled, since its maximum is kept, and a
custom memory value is sent only while its box is ticked.

instances/templates/instance.html

@@ -320,6 +320,8 @@
             var container  = $(this).closest('.js-custom__container');
             var toggles    = container.find('.js-custom__toggle');
             toggles.toggle();
+            // a custom value is sent only while its box is ticked
+            container.find('input.js-custom__toggle').prop('disabled', !this.checked);
         });
     });
 </script>

instances/templates/instances/resize_tab.html

@@ -97,7 +97,7 @@
                                 <option value="{{ mem }}" {% if mem == instance.cur_memory %}selected{% endif %}>{{ mem }}</option>
                             {% endfor %}
                             </select>
-                            <input type="text" name="cur_memory_custom" class="form-select js-custom__toggle" style="display: none" />
+                            <input type="text" name="cur_memory_custom" class="form-select js-custom__toggle" style="display: none" disabled />
                             <small><input type="checkbox" class="js-custom__checkbox" /> {% trans "Custom value" %}</small>
                         </div>
                     </div>
@@ -106,13 +106,18 @@
                             {% trans "Maximum Allocation" %} ({% trans "MB" %})
                         </label>
                         <div class="col-sm-4 js-custom__container">
-                            <select name="memory" class="form-select js-custom__toggle">
+                            {# a running VM keeps its maximum: only the current memory changes #}
+                            <select name="memory" class="form-select js-custom__toggle"{% if instance.status != 5 %} disabled{% endif %}>
                             {% for mem in memory_range %}
                                 <option value="{{ mem }}" {% if mem == instance.memory %}selected{% endif %}>{{ mem }}</option>
                             {% endfor %}
                             </select>
-                            <input type="text" name="memory_custom" class="form-select js-custom__toggle" style="display: none" />
+                            {% if instance.status == 5 %}
+                            <input type="text" name="memory_custom" class="form-select js-custom__toggle" style="display: none" disabled />
                             <small><input type="checkbox" class="js-custom__checkbox" /> {% trans "Custom value" %}</small>
+                            {% else %}
+                            <small class="text-body-secondary">{% trans "Shut off the VM to change the maximum" %}</small>
+                            {% endif %}
                         </div>
                     </div>
                     <button type="submit" class="btn btn-lg btn-success float-end" name="resizevm_mem">{% trans "Resize" %}</button>
@@ -127,7 +132,7 @@
             {% if can_change %}
                 <form action="{% url 'instances:resize_disk' instance.id %}" method="post" role="form" aria-label="Resize instance disk form">
                     {% csrf_token %}
-                    <p class="fw-bold">{% trans "Disk allocation (GB)" %}:</p>
+                    <p class="fw-bold">{% trans "New disk size (GiB); leave empty to keep a disk as it is" %}:</p>
                     {% for disk in instance.disks %}
                     <div class="row">
                         <label class="col-sm-4 col-form-label">{% trans "Current Allocation" %} ({{ disk.dev }})</label>
@@ -139,7 +144,9 @@
                         </div>
                         {% else %}
                         <div class="col-sm-4 js-custom__container">
-                            <input type="number" name="disk_size_{{ disk.dev }}" class="form-control" value="{% widthratio disk.size 1073741824 1 %}" />
+                            {# empty: the disk stays as it is #}
+                            <input type="number" name="disk_size_{{ disk.dev }}" class="form-control" min="1" step="1"
+                                   placeholder="{{ disk.size|filesizeformat }}" aria-label="{% trans "New size (GiB)" %} {{ disk.dev }}" />
                         </div>
                         {% endif %}
                     </div>

instances/tests/test_quota.py

@@ -163,7 +163,8 @@ class QuotaViewTests(TestCase):
         return messages, check, wvm.return_value
 
     def test_unverified_quota_has_its_own_message(self):
-        messages, _, proxy = self.post("instances:resizevm_cpu", {"vcpu": 2, "cur_vcpu": 2}, quota=QUOTA_UNVERIFIED)
+        messages, _, proxy = self.post("instances:resizevm_cpu", {"vcpu": 2, "cur_vcpu": 2}, quota=QUOTA_UNVERIFIED,
+                                         status=5)
         self.assertEqual(len(messages), 1)
         self.assertIn("a host of your virtual machines cannot be reached", messages[0])
         self.assertNotIn("quota reached", messages[0])

instances/tests/test_quota_lock.py

@@ -52,6 +52,7 @@ class QuotaRaceTestCase(TransactionTestCase):
             "instances.models.wvmInstance"
         ) as wvm:
             wvm.return_value.get_vcpu.return_value = 1
+            wvm.return_value.instance.isActive.return_value = False  # vCPUs change on a shut-off VM
             wvm.return_value.resize_cpu.side_effect = self.fake_resize
             threads = [threading.Thread(target=self.resize, args=(vm,)) for vm in self.vms]
             for t in threads:

instances/tests/test_resize_input.py

@@ -0,0 +1,78 @@
+"""The Resize tab changes only what was asked, and refuses what libvirt would
+not apply as asked."""
+
+from unittest.mock import patch
+
+from accounts.models import UserInstance
+from appsettings.settings import get_settings
+from computes.models import Compute
+from django.contrib.auth import get_user_model
+from django.contrib.messages import get_messages
+from django.test import TestCase
+from django.urls import reverse
+from instances.models import Instance
+from libvirt import VIR_DOMAIN_RUNNING
+
+GIB = 1 << 30
+
+
+class ResizeInputTests(TestCase):
+    def setUp(self):
+        get_settings()
+        self.user = get_user_model().objects.create_user("resize-user", password="pw")
+        self.client.force_login(self.user)
+        compute = Compute.objects.create(name="rs", hostname="192.0.2.1", login="u", password="p", type=1)
+        self.vm = Instance.objects.create(compute=compute, name="rs-vm", uuid="13131313-0000-0000-0000-000000000000")
+        UserInstance.objects.create(user=self.user, instance=self.vm, is_change=True)
+
+    def post(self, view, data, status=5, disks=()):
+        with patch("instances.views.utils.check_user_quota", return_value=""), patch(
+            "instances.models.wvmInstance"
+        ) as wvm:
+            proxy = wvm.return_value
+            proxy.get_vcpu.return_value = 2
+            proxy.get_memory.return_value = 2048
+            proxy.get_cur_memory.return_value = 2048
+            proxy.get_status.return_value = status
+            proxy.instance.isActive.return_value = status != 5
+            proxy.get_disk_devices.return_value = [dict(disk) for disk in disks]
+            response = self.client.post(reverse(view, args=[self.vm.id]), data)
+        self.assertEqual(response.status_code, 302)
+        return [str(m) for m in get_messages(response.wsgi_request)], proxy
+
+    def test_only_a_disk_given_a_larger_size_grows(self):
+        # a 3.5 GiB disk left as it was is not grown; a disk outside a pool has no size
+        disks = [{"dev": "vda", "size": 3 * GIB + GIB // 2}, {"dev": "vdb", "size": 20 * GIB}, {"dev": "vdc", "size": None}]
+        messages, proxy = self.post("instances:resize_disk", {"disk_size_vda": "", "disk_size_vdb": "30"}, disks=disks)
+        proxy.resize_disk.assert_called_once()
+        self.assertEqual([(d["dev"], d["size_new"]) for d in proxy.resize_disk.call_args.args[0]], [("vdb", 30 * GIB)])
+        self.assertEqual(messages, ["Disk is resized: vdb"])
+
+    def test_a_disk_size_that_is_not_a_whole_number_is_refused(self):
+        messages, proxy = self.post("instances:resize_disk", {"disk_size_vda": "4.5"}, disks=[{"dev": "vda", "size": GIB}])
+        proxy.resize_disk.assert_not_called()
+        self.assertEqual(messages, ["Enter disk sizes as whole GiB"])
+
+    def test_vcpus_change_only_on_a_shut_off_vm(self):
+        messages, proxy = self.post("instances:resizevm_cpu", {"vcpu": "4", "cur_vcpu": "4"}, status=VIR_DOMAIN_RUNNING)
+        proxy.resize_cpu.assert_not_called()
+        self.assertEqual(messages, ["Shut off the VM to change its vCPUs"])
+
+    def test_current_vcpus_above_the_maximum_are_refused(self):
+        messages, proxy = self.post("instances:resizevm_cpu", {"vcpu": "2", "cur_vcpu": "4"})
+        proxy.resize_cpu.assert_not_called()
+        self.assertEqual(messages, ["The current vCPUs cannot be more than the maximum"])
+
+    def test_current_memory_above_the_maximum_is_refused(self):
+        messages, proxy = self.post("instances:resize_memory", {"memory": "4096", "cur_memory": "8192"})
+        proxy.resize_mem.assert_not_called()
+        self.assertEqual(messages, ["The current memory cannot be more than the maximum"])
+
+    def test_a_running_vm_offers_no_maximum_memory(self):
+        with patch("instances.models.wvmInstance"):
+            self.vm.__dict__["status"] = VIR_DOMAIN_RUNNING
+            from django.template.loader import render_to_string
+            html = render_to_string("instances/resize_tab.html", {"instance": self.vm, "can_change": True,
+                                                                   "memory_range": [1024, 2048]})
+        self.assertRegex(html, r'<select name="memory" class="form-select js-custom__toggle" disabled>')
+        self.assertNotIn('name="memory_custom"', html)

instances/views.py

@@ -966,6 +966,13 @@ def resizevm_cpu(request, pk):
     if not _is_int(new_vcpu) or not _is_int(new_cur_vcpu):
         messages.error(request, _("Enter a whole number of vCPUs"))
         return redirect(reverse("instances:instance", args=[instance.id]) + "#resize")
+    # a running VM would get them only at its next start, past the quota counted now
+    if instance.proxy.instance.isActive():
+        messages.error(request, _("Shut off the VM to change its vCPUs"))
+        return redirect(reverse("instances:instance", args=[instance.id]) + "#resize")
+    if int(new_cur_vcpu) > int(new_vcpu):
+        messages.error(request, _("The current vCPUs cannot be more than the maximum"))
+        return redirect(reverse("instances:instance", args=[instance.id]) + "#resize")
 
     quota_msg = utils.check_user_quota(
         request.user, 0, int(new_vcpu) - vcpu, 0, 0
@@ -1015,6 +1022,9 @@ def resize_memory(request, pk):
     if not _is_int(new_memory) or not _is_int(new_cur_memory):
         messages.error(request, _("Enter a whole number for memory"))
         return redirect(reverse("instances:instance", args=[instance.id]) + "#resize")
+    if int(new_cur_memory) > int(new_memory):
+        messages.error(request, _("The current memory cannot be more than the maximum"))
+        return redirect(reverse("instances:instance", args=[instance.id]) + "#resize")
     quota_msg = utils.check_user_quota(request.user, 0, 0, int(new_memory) - memory, 0)
     if not request.user.is_superuser and quota_msg:
         msg = quota_refused(quota_msg, _(
@@ -1049,11 +1059,15 @@ def resize_disk(request, pk):
 
     disks_new = list()
     for disk in disks:
-        input_disk_size = (
+        # an empty field leaves the disk as it is; a disk outside a pool has no size and no field
-            int(request.POST.get("disk_size_" + disk["dev"], "0")) * 1073741824
+        size = request.POST.get("disk_size_" + disk["dev"], "")
-        )
+        if not size or disk["size"] is None:
-        if input_disk_size > disk["size"] + (64 << 20):
+            continue
-            disk["size_new"] = input_disk_size
+        if not _is_int(size):
+            messages.error(request, _("Enter disk sizes as whole GiB"))
+            return redirect(reverse("instances:instance", args=[instance.id]) + "#resize")
+        if int(size) << 30 > disk["size"]:
+            disk["size_new"] = int(size) << 30
             disks_new.append(disk)
 
     if not disks_new: