Keep native resource edits independent of OCI recreation checks

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()})

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

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])