j3d1/webvirtcloud
1
0
Fork 0
mirror of https://github.com/retspen/webvirtcloud synced 2026-11-08 11:16:47 +00:00
Code Issues Projects Releases Wiki Activity

fix(instances): a clone takes a free name the form offers

Browse source 
A user who is not a superuser chooses the clone's name from the free
names the clone form lists, or gets the first one with automatic names,
but the server took any posted name. It now refuses a name outside the
list, and with automatic names it takes the first free one whatever is
posted. A superuser still names a clone freely.

The panel checks VM names on every compute, but a job held a name only
on its compute, so two clones from VMs on two hosts could take the same
free name at once; a name claim now holds the name everywhere. A clone
job whose name a VM found on a host took while it was queued stops
before it copies anything.
This commit is contained in:
catborise 2026-10-31 00:32:39 +03:00
parent cd2dd517b7
commit 13d3a16ea5
5 changed files with 72 additions and 11 deletions
Download patch file Download diff file Expand all files Collapse all files

18
instances/jobs.py
View file

@ -138,11 +138,16 @@ def clone_data(user, instance, fields):
if CLONE_POST_KEY_RE.fullmatch(key):
data[key] = value.strip()
if app_settings.CLONE_INSTANCE_AUTO_NAME == "True" and not data["name"]:
auto_vname = utils.get_clone_free_names()[0]
data["name"] = auto_vname
# A user who is not a superuser clones to a free name the form offers, or,
# with automatic names, to the first one; a superuser to any name.
free_names = utils.get_clone_free_names()
auto_name = app_settings.CLONE_INSTANCE_AUTO_NAME == "True"
if not user.is_superuser and not auto_name and data["name"] not in free_names:
raise util.OperationError(_("Choose the clone's name from the list of free names"))
if auto_name and (not data["name"] or not user.is_superuser):
data["name"] = free_names[0]
for disk in instance.config_disks:
data[f"disk-{disk['dev']}"] = utils.get_clone_disk_name(disk, instance.name, auto_vname)
data[f"disk-{disk['dev']}"] = utils.get_clone_disk_name(disk, instance.name, data["name"])
if not user.is_superuser:
for disk in instance.config_disks:
@ -183,7 +188,7 @@ def clone_data(user, instance, fields):
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,
claims.name_key(compute.pk, name): Claim.EXCLUSIVE}
claims.name_key(name): Claim.EXCLUSIVE}
def record_vm(op, owner):
@ -470,6 +475,9 @@ class CloneInstance(Kind):
instance = _clonable(op.user, op.params["instance"])
if instance.compute_id != op.intent["compute"]:
raise util.OperationError("The VM moved to another compute after the job was queued")
# a VM found on a host since the job was queued may have taken the name
if Instance.objects.filter(name=op.intent["name"]).exists():
raise util.OperationError(_("Instance '%(clone_name)s' already exists!") % {"clone_name": op.intent["name"]})
_check_clone_state(instance.proxy, op.intent["clone"])
def run(self, op, ctx):

10
instances/tests/test_clone_job.py
View file

@ -57,7 +57,7 @@ class CloneJobTestCase(TransactionTestCase):
c = self.compute.pk
self.assertEqual(set(Claim.objects.values_list("resource", "mode")), {
(f"compute:{c}", "shared"), (f"reconcile:{c}", "exclusive"),
(f"instance:{self.source.uuid}", "exclusive"), (f"name:{c}:copy", "exclusive"),
(f"instance:{self.source.uuid}", "exclusive"), ("name:copy", "exclusive"),
(f"volume:{c}:default/copy.qcow2", "exclusive")})
self.proxy.clone_instance.assert_not_called()
@ -200,9 +200,17 @@ class LiveCloneJobTestCase(TransactionTestCase):
self.proxy.live_clone_targets.assert_not_called()
self.assertFalse(Operation.objects.get(pk=op.pk).intent["live"])
def test_a_name_taken_since_the_job_was_queued_stops_it(self):
op = self.op()
other = Compute.objects.create(name="live-d", hostname="192.0.2.33", login="root", password="", type=1)
Instance.objects.create(compute=other, name="copy", uuid="cdcdcdcd-9999-1111-2222-333333333333")
with self.assertRaisesMessage(OperationError, "already exists"):
CloneInstance().precheck(op, None)
def test_a_running_or_paused_vm_takes_the_live_path(self):
for status in (1, 3):
with self.subTest(status=status):
Instance.objects.filter(name="copy").delete() # the clone of the run before
self.proxy.reset_mock()
self.proxy.get_status.return_value = status
self.proxy.backup_progress.side_effect = [None]

48
instances/tests/test_clone_permission.py
View file

@ -67,9 +67,10 @@ class ClonePermissionTestCase(TestCase):
@contextmanager
def mocked_libvirt(self):
# the name these tests post is the first free one
with patch("instances.models.wvmInstance") as mock_wvm, patch(
"instances.views.utils.check_user_quota", return_value=""
):
), patch("instances.utils.get_clone_free_names", return_value=["clone-vm", "clone-vm2"]):
proxy = mock_wvm.return_value
proxy.get_disk_devices.return_value = [SRC_DISK]
proxy.get_net_devices.return_value = [{"mac": "52:54:00:aa:bb:cc"}]
@ -174,7 +175,7 @@ class ClonePermissionTestCase(TestCase):
def test_a_vm_without_a_nic_is_cloned(self):
for user, auto_name in ((self.superuser, "False"), (self.owner_change, "False"), (self.owner_change, "True")):
with self.subTest(user.username, auto_name=auto_name), self.mocked_libvirt() as proxy, patch(
"instances.views.utils.get_clone_free_names", return_value=["auto-clone"]
"instances.views.utils.get_clone_free_names", return_value=["auto-clone", "clone-vm"]
):
AppSettings.objects.filter(key="CLONE_INSTANCE_AUTO_NAME").update(value=auto_name)
proxy.get_net_devices.return_value = []
@ -239,3 +240,46 @@ class DhcpdConfTestCase(TestCase):
response = self.client.get(reverse("instances:guess_clone_name"))
self.assertEqual(response.json(), {"name": "clone7"})
self.assertEqual(get_dhcp_mac_address("clone7"), "52:54:00:00:07:07")
class CloneNameTestCase(TestCase):
"""A user who is not a superuser clones to a name the form offers: one of
the free names, or with automatic names the first; the server checks it."""
def setUp(self):
AppSettings.objects.filter(key="CLONE_INSTANCE_DEFAULT_PREFIX").update(value="copy")
AppSettings.objects.filter(key="CLONE_INSTANCE_AUTO_NAME").update(value="False")
self.user = get_user_model().objects.create_user("clone_names", password="pw")
self.user.user_permissions.add(Permission.objects.get(codename="clone_instances"))
compute = Compute.objects.create(name="names-compute", hostname="192.0.2.40", login="root", password="", type=1)
other = Compute.objects.create(name="names-other", hostname="192.0.2.41", login="root", password="", type=1)
self.instance = Instance.objects.create(compute=compute, name="src-vm", uuid="45454545-3434-5656-7878-909090909090")
UserInstance.objects.create(instance=self.instance, user=self.user, is_change=True)
# copy1 is taken on another host: names are the panel's on every host
Instance.objects.create(compute=other, name="copy1", uuid="45454545-0000-5656-7878-909090909090")
self.client.force_login(self.user)
def clone(self, name):
with patch("instances.models.wvmInstance") as mock_wvm, patch(
"instances.views.utils.check_user_quota", return_value=""
):
proxy = mock_wvm.return_value
proxy.get_disk_devices.return_value = [SRC_DISK]
proxy.get_net_devices.return_value = [{"mac": "52:54:00:aa:bb:cc"}]
proxy.get_status.return_value = 5
self.client.post(reverse("instances:clone", args=[self.instance.id]),
{"name": name, "submission_key": f"key-names-{name}"})
op = Operation.objects.first()
return op and op.intent["name"]
def test_a_free_name_from_the_list(self):
self.assertEqual(self.clone("copy2"), "copy2")
def test_any_other_name_is_refused(self):
for name in ("copy1", "my-own-name", "", "copy12"): # taken, not offered, empty, past the ten offered
with self.subTest(name=name):
self.assertIsNone(self.clone(name))
def test_automatic_names_ignore_the_posted_name(self):
AppSettings.objects.filter(key="CLONE_INSTANCE_AUTO_NAME").update(value="True")
self.assertEqual(self.clone("my-own-name"), "copy2")

2
instances/tests/test_migrate_job.py
View file

@ -49,7 +49,7 @@ class MigrateJobTestCase(TransactionTestCase):
def test_the_worker_migrates_and_moves_the_record(self):
op = self.queue(live_migrate="true")
self.assertEqual(set(Claim.objects.values_list("resource", "mode")), {
(f"instance:{UUID}", "exclusive"), (f"name:{self.dest.pk}:mig-vm", "exclusive"),
(f"instance:{UUID}", "exclusive"), ("name:mig-vm", "exclusive"),
(f"compute:{self.source.pk}", "shared"), (f"reconcile:{self.source.pk}", "exclusive"),
(f"compute:{self.dest.pk}", "shared"), (f"reconcile:{self.dest.pk}", "exclusive")})
self.dest_conn.moveto.assert_not_called() # nothing ran in the request

5
operations/claims.py
View file

@ -48,8 +48,9 @@ def volume_key(compute_id, pool, name):
return f"volume:{compute_id}:{pool}/{name}"
def name_key(compute_id, name):
return f"name:{compute_id}:{name}"
def name_key(name):
"""A VM name is the panel's on every compute (the name checks are global)."""
return f"name:{name}"
def quota_key(user_id):