From d286b650d78ca1300222b95ef2d835a358350f2c Mon Sep 17 00:00:00 2001 From: Jonatan <190036+jcastro@users.noreply.github.com> Date: Sat, 17 Oct 2026 16:49:39 +0200 Subject: [PATCH] Keep native resource edits independent of OCI recreation checks --- oci/remote/oci_resource_modify.py | 6 +++++ oci/remote/oci_update_current.py | 20 +++++++++------- oci/tests/test_modify_image_reuse.py | 35 ++++++++++++++++++++++++---- 3 files changed, 48 insertions(+), 13 deletions(-) diff --git a/oci/remote/oci_resource_modify.py b/oci/remote/oci_resource_modify.py index 6ae818bc..9d4083cf 100644 --- a/oci/remote/oci_resource_modify.py +++ b/oci/remote/oci_resource_modify.py @@ -14,6 +14,10 @@ CONFIG_DIR = Path('/etc/pve/lxc') def changes(record, candidate, config): """Return a narrow native edit, or None when the normal installer is needed.""" + if (any(key in record for key in ('stack', 'stack_member', 'native_stack_intent')) + or record['deployment'].get('stack_managed') + or instances.identity(config) != record['installation_id']): + return None previous = transaction.with_default_healthcheck(copy.deepcopy(record)) candidate = transaction.with_default_healthcheck(copy.deepcopy(candidate)) old, new = previous['deployment'], candidate['deployment'] @@ -66,6 +70,8 @@ def apply(root, record, candidate, config, parameters): """The caller holds the instance lock and has checked image identity/preflight.""" if changes(record, candidate, config) != parameters: raise ValueError(translate('The instance changed while it was being edited; open Modify again')) + if transaction.external_changes(record, config): + raise ValueError(translate('The instance changed while it was being edited; open Modify again')) vmid = record['vmid'] directory = instances.location(root, vmid).parent / 'modifications' / uuid.uuid4().hex instances.write(directory / 'before.json', {'record': record, 'config': config.decode()}) diff --git a/oci/remote/oci_update_current.py b/oci/remote/oci_update_current.py index 4ade01c1..b5101fc9 100644 --- a/oci/remote/oci_update_current.py +++ b/oci/remote/oci_update_current.py @@ -191,13 +191,21 @@ def update(vmid, acknowledge_external_data=False, proposal=None, keep_backup=Non config = instances.command('pct', 'config', str(vmid)) desired = transaction.candidate_contract(record, operation, proposal) changes = transaction.external_changes(record, config) - transaction.preflight(record, desired, config) - msg_ok(translate('Container checked')) resource_changes = None - if operation == 'recreate' and not keep_backup: + if operation == 'recreate' and not keep_backup and not changes: import oci_resource_modify resource_changes = oci_resource_modify.changes(record, desired, config) - current = record['observed']['image'] if operation == 'update' or resource_changes else None + # CPU/RAM edits retain the installed image and all unrelated native + # settings. They do not implicitly update the application or depend + # on the registry and the narrower recreation profile. + if resource_changes: + msg_ok(translate('Container checked')) + import oci_operation_notice + with oci_operation_notice.operation([vmid], 'modify', application_name(record, vmid)): + return oci_resource_modify.apply(instances.ROOT, record, desired, config, resource_changes) + transaction.preflight(record, desired, config) + msg_ok(translate('Container checked')) + current = record['observed']['image'] if operation == 'update' else None check = lambda: work_backup.locate( instances.location(instances.ROOT, vmid).parent / 'transactions' / 'check', [vmid], record, vmid) if operation == 'recreate': @@ -206,10 +214,6 @@ def update(vmid, acknowledge_external_data=False, proposal=None, keep_backup=Non else: archive, digest = resolve_archive(desired, config, current, check) if archive is None: - if resource_changes: - import oci_operation_notice - with oci_operation_notice.operation([vmid], 'modify', application_name(record, vmid)): - return oci_resource_modify.apply(instances.ROOT, record, desired, config, resource_changes) msg_ok(translate('The image is already up to date; nothing was changed.')) return file_storage = None diff --git a/oci/tests/test_modify_image_reuse.py b/oci/tests/test_modify_image_reuse.py index 03020ced..95c7339d 100644 --- a/oci/tests/test_modify_image_reuse.py +++ b/oci/tests/test_modify_image_reuse.py @@ -163,6 +163,19 @@ class ResourceModifyTests(unittest.TestCase): self.assertTrue(resources.expected_config(CONFIG, changed, self.params)) self.assertFalse(resources.expected_config(CONFIG, changed.replace(b'A=one', b'A=foreign'), self.params)) + def test_cpu_change_preserves_homepage_dns_timezone_and_cpu_priority(self): + extra = b'nameserver: 10.0.1.1\ntimezone: Europe/Madrid\ncpuunits: 100\n' + config = CONFIG + extra + self.file.write_bytes(config) + self.saved['observed'].update(config=config.decode(), config_sha256=resources.sha(config)) + self.saved['deployment']['resources']['cpu_units'] = 100 + self.candidate = copy.deepcopy(self.saved) + self.candidate['deployment']['resources']['cores'] = 3 + instances.write(instances.location(self.root,115),self.saved) + resources.apply(self.root,self.saved,self.candidate,config,{'cores':'3'}) + self.assertEqual(self.file.read_bytes(), config.replace(b'cores: 2',b'cores: 3')) + self.assertEqual(instances.read(self.root,115)['deployment']['resources']['cpu_units'],100) + def test_a_concurrent_unrelated_change_is_not_overwritten_during_rollback(self): def observe(*args): self.file.write_bytes(self.file.read_bytes().replace(b'A=one', b'A=foreign')) @@ -268,7 +281,7 @@ class CachedArchiveTests(unittest.TestCase): class UpdateRoutingTests(unittest.TestCase): - def run_update(self, new_image=False, resource_edit=True): + def run_update(self, new_image=False, resource_edit=True, reject_recreation=False): saved = record() candidate = copy.deepcopy(saved) candidate['deployment']['resources']['memory_mb'] = 768 @@ -281,7 +294,7 @@ class UpdateRoutingTests(unittest.TestCase): patch.object(instances, 'command', return_value=CONFIG), \ patch.object(update.transaction, 'candidate_contract', return_value=candidate), \ patch.object(update.transaction, 'external_changes', return_value={}), \ - patch.object(update.transaction, 'preflight'), \ + patch.object(update.transaction, 'preflight', side_effect=ValueError('advanced Proxmox settings') if reject_recreation else None), \ patch.object(update, 'resolve_archive', return_value=(Path('/new.tar') if new_image or not resource_edit else None, NEW if new_image else DIGEST)) as resolve, \ patch.object(resources, 'apply') as apply_resources, \ patch.object(update.transaction, 'apply') as apply_image, \ @@ -291,15 +304,27 @@ class UpdateRoutingTests(unittest.TestCase): def test_unchanged_image_routes_cpu_ram_edits_to_the_existing_guest(self): resolve, native, image = self.run_update() - self.assertEqual(resolve.call_args.args[2]['manifest_digest'], DIGEST) + resolve.assert_not_called() native.assert_called_once() image.assert_not_called() - def test_a_new_image_still_uses_the_normal_verified_recreation(self): - _resolve, native, image = self.run_update(new_image=True) + def test_a_new_image_for_other_configuration_changes_uses_verified_recreation(self): + _resolve, native, image = self.run_update(new_image=True,resource_edit=False) native.assert_not_called() image.assert_called_once() + def test_cpu_edits_do_not_implicitly_upgrade_the_image(self): + resolve,native,image=self.run_update(new_image=True) + resolve.assert_not_called() + native.assert_called_once() + image.assert_not_called() + + def test_homepage_resource_edit_does_not_run_the_recreation_preflight(self): + resolve,native,image=self.run_update(reject_recreation=True) + resolve.assert_not_called() + native.assert_called_once() + image.assert_not_called() + def test_other_modifications_offer_the_recorded_archive_to_the_resolver(self): resolve, native, image = self.run_update(resource_edit=False) self.assertIsNone(resolve.call_args.args[2])