feat(instances): a clone takes the size asked and starts if asked

A VM made from a template came out at the template's size and shut off,
and was then resized and started by hand. The Clone tab now asks for the
clone's vCPUs and memory, filled in with the VM's, and whether to start
it. After the copy, before the clone is recorded, it is resized with the
Resize tab's own functions and started; a step that fails is a warning
of the job, and the clone stays. The quota counts the larger of the size
asked and the VM's, so a resize that fails leaves the clone at a size it
allowed.

When the job ends the page opens the clone's, not the cloned VM's again:
a template's page, shut off, read as a clone that had not started. An
outcome kept for a warning links to it.

instances/jobs.py

@@ -8,6 +8,7 @@ import time
 from django.db import transaction
 from django.core.exceptions import PermissionDenied
 from django.shortcuts import get_object_or_404
+from django.urls import reverse
 from django.template.loader import render_to_string
 from django.utils.translation import gettext_noop
 from django.utils.translation import gettext_noop as _
@@ -112,7 +113,7 @@ def _snapshot_free_space():
 
 
 CLONE_POST_KEY_RE = re.compile(r"^(clone-net-mac-\d+|disk-[a-z0-9]+|meta-[a-z0-9]+)$")
-CLONE_FIELDS = ("name", "clone-title", "clone-description")
+CLONE_FIELDS = ("name", "clone-title", "clone-description", "clone-vcpu", "clone-memory", "clone-start")
 
 
 def clone_data(user, instance, fields):
@@ -126,9 +127,19 @@ def clone_data(user, instance, fields):
     data["clone-title"] = fields.get("clone-title", "").strip()
     data["clone-description"] = fields.get("clone-description", "").strip()
 
-    # the clone copies the persistent definition
-    quota = {"cpus": instance.proxy.get_vcpu(config=True), "memory": instance.proxy.get_memory(config=True),
-             "disk": utils.disk_gib(instance.config_disks)}
+    # the clone copies the persistent definition, with the size the form gives
+    charged = {}
+    for key, source, label in (("vcpu", instance.config_vcpu, _("vCPUs")),
+                               ("memory", instance.config_memory, _("memory (MiB)"))):
+        value = fields.get(f"clone-{key}") or str(source)
+        if not value.isdigit() or int(value) < 1:
+            raise util.OperationError(_("Enter a whole number of %(what)s for the clone") % {"what": label})
+        # None: as the VM, nothing to change after the copy
+        data[key] = int(value) if int(value) != source else None
+        # the larger: a resize that fails after the copy leaves the clone at the VM's size
+        charged[key] = max(int(value), source)
+    data["start"] = fields.get("clone-start") == "true"
+    quota = {"cpus": charged["vcpu"], "memory": charged["memory"], "disk": utils.disk_gib(instance.config_disks)}
     quota_msg = utils.check_user_quota(user, 1, quota["cpus"], quota["memory"], quota["disk"])
 
     data["disk_owner_uid"] = int(app_settings.INSTANCE_VOLUME_DEFAULT_OWNER_UID)
@@ -188,6 +199,37 @@ def clone_data(user, instance, fields):
     return data, quota
 
 
+def _finish_clone(op, clone, warning):
+    """The clone, defined but not recorded yet (the quota counts the job, at the
+    size it charged, until then): the size the form gave, and started if
+    asked. It is made either way: a step that fails is a warning of the job."""
+    c = Compute.objects.get(pk=op.intent["compute"])
+    proxy = wvmInstance(c.hostname, c.login, c.password, c.type, op.intent["name"], uuid=op.intent["uuid"])
+    notes = [warning] if warning else []
+    try:
+        if clone.get("vcpu"):
+            proxy.resize_cpu(str(clone["vcpu"]), str(clone["vcpu"]))
+        if clone.get("memory"):
+            proxy.resize_mem(clone["memory"], clone["memory"])
+        if clone.get("start"):
+            proxy.start()
+    except libvirtError as err:
+        notes.append(_("The clone is made, but not as asked: %(error)s") % {"error": err})
+    return {"warning": " ".join(notes)} if notes else {}
+
+
+def _clone_made(op, result):
+    """Records the clone; the job's result, with the clone's page, which the
+    job panel opens: the page the job ran from is the VM cloned."""
+    record_vm(op, owner=True)
+    return {**result, "open": _clone_page(op)}
+
+
+def _clone_page(op):
+    clone = Instance.objects.get(compute_id=op.intent["compute"], uuid=op.intent["uuid"])
+    return reverse("instances:instance", args=[clone.pk])
+
+
 def vm_claims(compute, name):
     """A job that adds a VM: other VMs stay usable; the VM list reconcile waits."""
     return {claims.compute_key(compute.pk): Claim.SHARED, claims.reconcile_key(compute.pk): Claim.EXCLUSIVE,
@@ -494,8 +536,7 @@ class CloneInstance(Kind):
             ctx.record_intent(live=False)
             ctx.set_phase(gettext_noop("Copying disks"))
             proxy.clone_instance(clone, uuid)
-            record_vm(op, owner=True)
-            return {}
+            return _clone_made(op, _finish_clone(op, clone, ""))
         if status not in (1, 3):
             raise util.OperationError(CLONE_STATE_REFUSED)
         targets = proxy.live_clone_targets(clone, **_snapshot_free_space())
@@ -555,8 +596,7 @@ class CloneInstance(Kind):
         success, errmsg = result
         if success:
             proxy.define_clone(clone, uuid, {c["dev"]: c["path"] for c in copies})
-            record_vm(op, owner=True)
-            return {"warning": warning} if warning else {}
+            return _clone_made(op, _finish_clone(op, clone, warning))
         _delete_created(proxy, op)
         op.refresh_from_db(fields=["cancel_requested"])
         if op.cancel_requested:
@@ -578,7 +618,10 @@ class CloneInstance(Kind):
     def reconcile(self, op):
         if not op.intent.get("live"):
             disks = [v for k, v in op.intent["clone"].items() if k.startswith("disk-") and v]
-            return vm_outcome(op, owner=True, volumes=disks)
+            outcome = vm_outcome(op, owner=True, volumes=disks)
+            if outcome.state == Operation.SUCCEEDED:
+                outcome.result = {**outcome.result, "open": _clone_page(op)}
+            return outcome
         proxy = _recovery_proxy(op)
         if proxy is None:
             created = ", ".join(c["path"] for c in op.intent.get("created", []))
@@ -597,8 +640,7 @@ class CloneInstance(Kind):
         """The spec's recovery table: a defined clone first, then a volume
         still being created, since it comes before the copy."""
         if _domain(op.intent["compute"], op.intent["uuid"]) is not None:
-            record_vm(op, owner=True)
-            return Outcome(Operation.SUCCEEDED)
+            return Outcome(Operation.SUCCEEDED, result=_clone_made(op, {}))
         created = op.intent.get("created", [])
         paths = [c["path"] for c in created]
         ended = Operation.CANCELLED if op.cancel_requested else Operation.FAILED
@@ -637,8 +679,7 @@ class CloneInstance(Kind):
             _delete_created(proxy, op)
             return Outcome(ended, error=errmsg or "The copy failed")
         proxy.define_clone(op.intent["clone"], op.intent["uuid"], {c["dev"]: c["path"] for c in created})
-        record_vm(op, owner=True)
-        return Outcome(Operation.SUCCEEDED)
+        return Outcome(Operation.SUCCEEDED, result=_clone_made(op, _finish_clone(op, op.intent["clone"], "")))
 
     def log_entry(self, op, state):
         source = op.resource_label.rpartition("/")[2]

instances/models.py

@@ -187,6 +187,11 @@ class Instance(models.Model):
     def config_cur_vcpu(self) -> int:
         return self.proxy.get_cur_vcpu(config=True)
 
+    @cached_property
+    def config_memory(self) -> int:
+        """Memory (MiB) of the persistent definition, which a clone starts from."""
+        return self.proxy.get_memory(config=True)
+
     @cached_property
     def config_title(self) -> str:
         """Title and description of the persistent definition, which the

instances/templates/instances/settings_tab.html

@@ -834,6 +834,24 @@
                     </div>
                 </div>
                 {% endif %}
+                <fieldset class="mb-4" aria-describedby="clone-size-help">
+                    <legend class="h6">{% trans "Size" %}</legend>
+                    <div class="row g-3">
+                        <div class="col-6 col-md-3">
+                            <label class="form-label" for="clone-vcpu">{% trans "vCPUs" %}</label>
+                            <input class="form-control" id="clone-vcpu" name="clone-vcpu" type="number" min="1" max="{{ vcpu_host }}" inputmode="numeric" value="{{ instance.config_vcpu }}">
+                        </div>
+                        <div class="col-6 col-md-3">
+                            <label class="form-label" for="clone-memory">{% trans "Memory (MiB)" %}</label>
+                            <input class="form-control" id="clone-memory" name="clone-memory" type="number" min="1" inputmode="numeric" value="{{ instance.config_memory }}">
+                        </div>
+                    </div>
+                    <p class="small text-body-secondary mt-1 mb-2" id="clone-size-help">{% trans "The VM's size to start from; the quota counts the larger of the two." %}</p>
+                    <div class="form-check form-switch">
+                        <input class="form-check-input" type="checkbox" role="switch" id="clone-start" name="clone-start" value="true">
+                        <label class="form-check-label" for="clone-start">{% trans "Start the clone when it is made" %}</label>
+                    </div>
+                </fieldset>
                 <fieldset class="mb-4">
                     <legend class="h6">{% trans "Optional details" %}</legend>
                     <label class="form-label" for="clone-title">{% trans "Title" %}</label>

instances/tests/test_clone_job.py

@@ -43,9 +43,11 @@ class CloneJobTestCase(TransactionTestCase):
         self.client.force_login(self.admin)
         self.compute = Compute.objects.create(name="clone-c", hostname="192.0.2.30", login="root", password="", type=1)
         self.source = Instance.objects.create(compute=self.compute, name="src", uuid="12121212-3434-5656-7878-909090909090")
-        patcher = patch("instances.models.wvmInstance", return_value=proxy())
-        self.proxy = patcher.start().return_value
-        self.addCleanup(patcher.stop)
+        self.proxy = proxy()
+        for target_name in ("instances.models.wvmInstance", "instances.jobs.wvmInstance"):
+            patcher = patch(target_name, return_value=self.proxy)
+            patcher.start()
+            self.addCleanup(patcher.stop)
 
     def test_the_page_queues_the_clone_and_the_worker_makes_and_records_it(self):
         response = self.client.post(reverse("instances:clone", args=[self.source.pk]),
@@ -69,6 +71,56 @@ class CloneJobTestCase(TransactionTestCase):
         clone = Instance.objects.get(uuid=op.intent["uuid"])
         self.assertTrue(UserInstance.objects.filter(instance=clone, user=self.admin).exists())
 
+    def clone_with(self, **fields):
+        self.client.post(reverse("instances:clone", args=[self.source.pk]),
+                         {"name": "copy", "disk-vda": "copy.qcow2", "clone-net-mac-0": "52:54:00:00:00:01",
+                          "submission_key": f"key-{uuid4().hex[:12]}", **fields})
+        op = Operation.objects.filter(kind="instance.clone").first()
+        if op is None:
+            return None, None
+        acquire("wvc-test:1:a", "wvc-test")
+        Operation.objects.filter(pk=op.pk).update(state=Operation.RUNNING, worker="wvc-test:1:a")
+        op.refresh_from_db()
+        return op, CloneInstance().run(op, Context(op, SimpleNamespace(identity="wvc-test:1:a")))
+
+    def test_the_clone_takes_the_size_given_and_starts(self):
+        # before the clone is recorded: until then the quota counts the job, at the size charged
+        self.proxy.resize_cpu.side_effect = lambda *a: self.assertFalse(Instance.objects.filter(name="copy").exists())
+        op, result = self.clone_with(**{"clone-vcpu": "4", "clone-memory": "2048", "clone-start": "true"})
+        self.assertEqual((op.intent["quota"]["cpus"], op.intent["quota"]["memory"]), (4, 2048))  # what the quota counts
+        self.proxy.resize_cpu.assert_called_once_with("4", "4")
+        self.proxy.resize_mem.assert_called_once_with(2048, 2048)
+        self.proxy.start.assert_called_once_with()
+        # the job panel opens the clone's page
+        clone = Instance.objects.get(uuid=op.intent["uuid"])
+        self.assertEqual(result, {"open": reverse("instances:instance", args=[clone.pk])})
+
+    def test_a_smaller_clone_is_charged_the_vm_s_size(self):
+        # a shrink that fails after the copy leaves the clone at the VM's size, which the quota allowed
+        op, _result = self.clone_with(**{"clone-vcpu": "1", "clone-memory": "256"})
+        self.assertEqual((op.intent["quota"]["cpus"], op.intent["quota"]["memory"]), (1, 512))
+        self.proxy.resize_mem.assert_called_once_with(256, 256)
+
+    def test_the_vm_s_size_changes_nothing_after_the_copy(self):
+        _op, result = self.clone_with(**{"clone-vcpu": "1", "clone-memory": "512"})
+        self.proxy.resize_cpu.assert_not_called()
+        self.proxy.resize_mem.assert_not_called()
+        self.proxy.start.assert_not_called()
+        self.assertEqual(list(result), ["open"])
+
+    def test_a_size_that_is_not_a_whole_number_is_refused(self):
+        for value in ("0", "abc", "-2"):
+            with self.subTest(value=value):
+                op, _result = self.clone_with(**{"clone-vcpu": value})
+                self.assertIsNone(op)
+                self.proxy.clone_instance.assert_not_called()
+
+    def test_a_clone_that_cannot_start_is_made_with_a_warning(self):
+        self.proxy.start.side_effect = libvirt.libvirtError("no memory")
+        op, result = self.clone_with(**{"clone-start": "true"})
+        self.assertTrue(Instance.objects.filter(uuid=op.intent["uuid"]).exists())
+        self.assertIn("The clone is made, but not as asked", result["warning"])
+
     def queued(self):
         self.client.post(reverse("instances:clone", args=[self.source.pk]),
                          {"name": "copy", "disk-vda": "copy.qcow2", "clone-net-mac-0": "52:54:00:00:00:01",
@@ -93,8 +145,10 @@ class CloneJobTestCase(TransactionTestCase):
     def test_recovery(self, conn):
         op = Operation(user=self.admin, intent={"compute": self.compute.pk, "uuid": "abababab-0000-1111-2222-333333333333",
                                                 "name": "copy", "clone": {"name": "copy", "disk-vda": "copy.qcow2"}})
-        self.assertEqual(CloneInstance().reconcile(op).state, Operation.SUCCEEDED)  # the VM is defined
-        self.assertTrue(Instance.objects.filter(uuid=op.intent["uuid"]).exists())
+        recovered = CloneInstance().reconcile(op)
+        self.assertEqual(recovered.state, Operation.SUCCEEDED)  # the VM is defined
+        clone = Instance.objects.get(uuid=op.intent["uuid"])
+        self.assertEqual(recovered.result["open"], reverse("instances:instance", args=[clone.pk]))
         missing = libvirt.libvirtError("no domain")
         missing.err = (libvirt.VIR_ERR_NO_DOMAIN, libvirt.VIR_FROM_QEMU, "no domain")
         conn.return_value.wvm.lookupByUUIDString.side_effect = missing
@@ -248,7 +302,7 @@ class LiveCloneJobTestCase(TransactionTestCase):
 
     def test_the_live_run_records_each_step_before_it(self):
         op = self.op()
-        self.assertEqual(self.run_job(op), {})
+        result = self.run_job(op)
         self.assertEqual(self.events, [
             ("intent", "live", "targets"),
             ("intent", "planned"), ("create", "vda"), ("intent", "created"),
@@ -269,6 +323,7 @@ class LiveCloneJobTestCase(TransactionTestCase):
         clone = Instance.objects.get(uuid=op.intent["uuid"])
         self.assertTrue(UserInstance.objects.filter(instance=clone, user=self.admin).exists())
         self.assertEqual(self.deleted(), [])
+        self.assertEqual(result, {"open": reverse("instances:instance", args=[clone.pk])})
 
     def test_a_paused_vm_or_one_without_a_ready_agent_is_not_frozen(self):
         for status, agent in ((3, True), (1, False)):
@@ -505,13 +560,23 @@ class LiveCloneJobTestCase(TransactionTestCase):
                 Instance.objects.filter(uuid=op.intent["uuid"]).delete()
                 op.delete()
 
+    def test_recovery_gives_the_clone_the_size_asked_and_starts_it(self):
+        clone = {"name": "copy", "disk-vda": "copy-vda.qcow2", "disk_owner_uid": 107, "disk_owner_gid": 107,
+                 "vcpu": 2, "memory": None, "start": True}
+        outcome, _op = self.recovered(copy_result=[True, ""], clone=clone)
+        self.assertEqual(outcome.state, Operation.SUCCEEDED)
+        self.proxy.resize_cpu.assert_called_once_with("2", "2")
+        self.proxy.resize_mem.assert_not_called()
+        self.proxy.start.assert_called_once_with()
+
     def test_recovery_of_a_defined_clone_succeeds(self):
         self.proxy.backup_result.return_value = None  # libvirtd restarted after the clone was defined
         self.proxy.backup_targets.return_value = {"/other/x.qcow2"}  # checked after it
         outcome, op = self.recovered(defined=True)
         self.assertEqual(outcome.state, Operation.SUCCEEDED)
         self.assertNotIn("define_clone", self.events)
-        self.assertTrue(Instance.objects.filter(uuid=op.intent["uuid"]).exists())
+        clone = Instance.objects.get(uuid=op.intent["uuid"])
+        self.assertEqual(outcome.result["open"], reverse("instances:instance", args=[clone.pk]))
 
     def test_recovery_of_a_failed_copy_deletes_what_was_created(self):
         outcome, _op = self.recovered(copy_result=[False, "No space left on device"])

instances/tests/test_pending_edits.py

@@ -183,7 +183,7 @@ class PendingEditsTestCase(TestCase):
                 # the DHCP entry of the clone's name holds one address: only the first NIC guesses it
                 self.assertEqual(len(form.xpath(".//button[contains(@onclick, 'guess_mac_address')]")), 1)
                 # a running or paused VM is copied by a backup, which does not preallocate
-                self.assertEqual(form.xpath(".//input[@type='checkbox']/@name"), metadata)
+                self.assertEqual(form.xpath(".//input[@type='checkbox']/@name"), [*metadata, "clone-start"])
 
     def test_a_disk_with_a_pending_source_is_not_deleted(self):
         # running: vda is A, its definition for the next start uses B

operations/static/operations/job_panel.js

@@ -74,7 +74,8 @@ function jobPanel(element, urls, csrfToken, initialIds) {
         var note = job.result && job.result.warning ? '<div class="small mt-2">' + esc(job.result.warning) + "</div>" : "";
         var warn = note + (left.length ? '<div class="small mt-2">Files not removed:</div><ul class="list-unstyled small mb-0">'
             + left.map(function (path) { return '<li class="text-break"><code>' + esc(path) + "</code></li>"; }).join("") + "</ul>" : "");
-        var buttons = '<button type="button" class="btn btn-sm btn-primary" data-refresh="1">Refresh</button>'
+        var buttons = (opens(job) ? '<a class="btn btn-sm btn-primary" href="' + esc(job.result.open) + '">Open</a>' : "")
+            + '<button type="button" class="btn btn-sm btn-primary" data-refresh="1">Refresh</button>'
             + '<button type="button" class="btn btn-sm btn-outline-secondary" data-dismiss="' + esc(job.id)
             + '">Dismiss</button>';
         return alertHtml(job.state, header(job.label, job.state, "", buttons) + errorHtml(job) + warn);
@@ -114,6 +115,11 @@ function jobPanel(element, urls, csrfToken, initialIds) {
         holding = busy;
     }
 
+    // the page of what the job made, a path of this site
+    function opens(job) {
+        return !!(job.result && /^\/[^\/]/.test(job.result.open || ""));
+    }
+
     // a job that did what it was asked, with nothing more to read: the page shows its effect
     function quiet(job) {
         var result = job.result || {};
@@ -134,7 +140,13 @@ function jobPanel(element, urls, csrfToken, initialIds) {
                 var others = Object.keys(outcomes).length + Object.keys(pending).length + Object.keys(shown).length;
                 // the page in use (poll.js): its outcome waits in the panel, with Refresh
                 if (quiet(job) && !pageInUse() && !others) {
-                    window.location.reload(); // the job is not remembered again: no loop
+                    // a job that made a VM opens its page (result.open); another reloads this one,
+                    // where the job is not remembered again: no loop
+                    if (opens(job)) {
+                        window.location.assign(job.result.open);
+                    } else {
+                        window.location.reload();
+                    }
                     return;
                 }
                 outcomes[id] = job;

operations/tests/js/job_panel.test.js

@@ -19,7 +19,8 @@ function advance(ms) {
 const realNow = Date.now;
 Date.now = () => 1700000000000 + now;
 global.document = { hidden: false, addEventListener() {}, activeElement: null, querySelector: () => null };
-global.window = { backgroundJobRunning: false, location: { reload() { window.reloaded = true; } } };
+global.window = { backgroundJobRunning: false,
+                  location: { reload() { window.reloaded = true; }, assign(url) { window.opened = url; } } };
 
 // requests settle only when the test says so; callbacks run in registration order
 let open = [];
@@ -264,6 +265,26 @@ answer("/j/u", { ...running, id: "u", label: "Upload", state: "succeeded" });
 assert.ok(window.reloaded, "a job that succeeds without a warning reloads the page: it shows the job's effect");
 assert.ok(!el11.innerHTML.includes('<span class="badge bg-success">succeeded</span>'), "not shown as well");
 
+// a job that made a VM opens its page; a warning keeps the outcome, with Open; only a path of this site
+open = []; timers = []; window.reloaded = false; window.opened = null;
+const elv = { innerHTML: "", addEventListener() {} };
+jobPanel(elv, urls, "tok", ["v"]);
+advance(0);
+answer("/r", empty);
+answer("/j/v", { ...running, id: "v", label: "Clone VM", state: "succeeded", result: { open: "/instances/9/" } });
+assert.ok(window.opened === "/instances/9/" && !window.reloaded, "opens the clone's page");
+for (const [result, shown] of [[{ open: "/instances/9/", warning: "not started" }, true],
+                               [{ open: "//evil.example.com/" }, false], [{ open: "javascript:alert(1)" }, false]]) {
+    open = []; timers = []; window.reloaded = false; window.opened = null;
+    const elw = { innerHTML: "", addEventListener() {} };
+    jobPanel(elw, urls, "tok", ["w"]);
+    advance(0);
+    answer("/r", empty);
+    answer("/j/w", { ...running, id: "w", state: "succeeded", result });
+    assert.strictEqual(window.opened, null, JSON.stringify(result));
+    assert.strictEqual(elw.innerHTML.includes('href="/instances/9/">Open</a>'), shown, JSON.stringify(result));
+}
+
 // a dialog open or a field in use: the outcome waits in the panel, with Refresh
 for (const [label, doc] of [["dialog", { querySelector: (s) => s === ".modal.show" ? {} : null }],
                             ["field", { activeElement: { matches: (s) => s === "input, select, textarea" } }]]) {