From 1a8edafd14af071e42c605075d10f13ef4a77a1e Mon Sep 17 00:00:00 2001 From: catborise Date: Thu, 5 Nov 2026 18:38:26 +0300 Subject: [PATCH] fix(instances): a snapshot job checks its name again in the worker The name was checked only when the job was queued; recovery counts any snapshot of that name as the job's, so one taken outside the panel meanwhile made a failed job succeed. precheck now refuses the name before any effect. --- instances/jobs.py | 5 ++++- instances/tests/test_snapshot_jobs.py | 9 +++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/instances/jobs.py b/instances/jobs.py index 8737d7d9..3ffddf36 100644 --- a/instances/jobs.py +++ b/instances/jobs.py @@ -384,7 +384,10 @@ class CreateSnapshot(Kind): return _vm_plan(params) def precheck(self, op, ctx): - _checked_proxy(op) + # a snapshot taken outside the panel since the job was queued may have + # the name, and recovery would count it as this job's + if op.params["name"] in _checked_proxy(op).instance.snapshotListNames(0): + raise util.OperationError(_("A snapshot named %(name)s already exists"), name=op.params["name"]) def run(self, op, ctx): p = op.params diff --git a/instances/tests/test_snapshot_jobs.py b/instances/tests/test_snapshot_jobs.py index 6af3a451..bd5299e4 100644 --- a/instances/tests/test_snapshot_jobs.py +++ b/instances/tests/test_snapshot_jobs.py @@ -70,6 +70,15 @@ class SnapshotJobsTestCase(TransactionTestCase): self.assertContains(response, "Snapshot names are 1-64 letters") self.assertFalse(Operation.objects.exists()) + def test_a_name_taken_outside_the_panel_meanwhile_is_refused_before_effects(self): + self.client.post(reverse("instances:snapshot", args=[self.instance.pk]), + {"name": "b", "submission_key": "key-snap0001"}) + op = Operation.objects.get() + self.proxy.instance.snapshotListNames.return_value = ["b"] # virsh snapshot-create-as + with self.assertRaisesMessage(OperationError, "A snapshot named b already exists"): + CreateSnapshot().precheck(op, None) + self.proxy.create_snapshot.assert_not_called() + def test_create_recovery(self): op = Operation(params={"instance": self.instance.pk, "name": "a", "external": True}, intent={"compute": self.instance.compute_id})