Apply OCI runtime settings to existing containers with recoverable edits

oci/README.md

@@ -142,7 +142,7 @@ An application made of several images is one entry of the catalog and one instal
 | Operation | What changes | What is kept |
 |---|---|---|
 | **Update** | The rootfs is replaced by the image the saved tag publishes today | Record, container disks, host directories, network and devices |
-| **Modify** | An editor opens with the current resources, network, paths and GPU; the container is rebuilt with the changes | The data of container disks and host directories |
+| **Modify** | Choose a setting, review it, then apply native LXC changes to the existing container; runtime settings may require a restart | Installed image, rootfs, persistent data and unrelated Proxmox settings |
 | **Recreate** (multi-container) | Every container is rebuilt from the image it was installed with | Data, network and devices |
 | **Watchdog** | Whether the application is started again when it stops on its own | Everything; only the choice is saved |
 | **Remove** | The LXC, or every container of the application, its disks and its record | Host directories, with their content |
@@ -159,6 +159,21 @@ Settings changed in Proxmox VE after the installation, such as memory, cores or
 
 A rolling tag is not an unattended update: the digest changes only through an explicit update.
 
+For standalone applications, CPU and RAM changes apply live. Network settings,
+environment overrides, directory mounts, additional managed volumes and supported
+native devices use a controlled stop/start when needed. Modify does not query the
+registry or download an image for these edits, even if the cached archive has
+already been cleaned. Removing a managed mount detaches its disk without deleting
+its data. An optional retained backup does not force recreation.
+
+The native path checks the saved configuration and installation identity, keeps a
+private before/after journal, and restores the original settings if applying them
+or checking the restarted service fails. It refuses to overwrite concurrent edits.
+Application-specific installer adaptations (such as changing an NVIDIA runtime or
+post-start configuration) are not silently converted into a full recreation:
+unsupported same-image changes stop with an explanation before modifying the CT.
+Image changes and coordinated stack operations retain their separate update flow.
+
 ## From a Compose file or a docker run command to an oci-compose
 
 The converter reads a Compose file with one service. A `docker run` command and an image reference alone are first written as a Compose file and follow the same path, so the three produce the same kind of oci-compose.

oci/remote/oci_instance_transaction.py

@@ -453,7 +453,7 @@ def external_changes(record, config, adopt=True):
     return values
 
 
-def preflight(record, candidate, config, coordinated=None):
+def preflight(record, candidate, config, coordinated=None, in_place=False, baseline=True):
     deployment = record['deployment']
     desired = candidate['deployment']
     if coordinated and (record.get('native_stack_intent') or deployment.get('rootfs_adaptation_replay_required')):
@@ -462,7 +462,7 @@ def preflight(record, candidate, config, coordinated=None):
         raise ValueError(translate('Stack members are updated together with their stack'))
     if instances.identity(config) != record['installation_id']:
         raise ValueError(translate('The container identity changed; the container is not replaced'))
-    for key, value in external_changes(record, config, adopt=not coordinated).items():
+    for key, value in (external_changes(record, config, adopt=not coordinated) if baseline else {}).items():
         section, name = ADOPTABLE[key]
         recorded = deployment.get(section, {}) if section else deployment
         target = desired.setdefault(section, {}) if section else desired
@@ -491,7 +491,8 @@ def preflight(record, candidate, config, coordinated=None):
         runtime_keys.add('lxc.mount.entry')
     runtime_settings.check(config, deployment, record['vmid'])
     coordinated_keys = {'hookscript', *KEPT_AS_IS} if coordinated else set(KEPT_AS_IS)
-    if '[' in config.decode() or keys - BASIC - runtime_keys - coordinated_keys - {k for k in keys if re.fullmatch(r'(mp|dev)[0-9]+', k)}:
+    preserved_keys = {'nameserver', 'searchdomain', 'timezone', 'protection'} if in_place else set()
+    if '[' in config.decode() or keys - BASIC - runtime_keys - coordinated_keys - preserved_keys - {k for k in keys if re.fullmatch(r'(mp|dev)[0-9]+', k) or (in_place and re.fullmatch(r'unused[0-9]+', k))}:
         raise ValueError(translate('The container has advanced Proxmox settings outside the supported profile'))
     unprivileged = deployment.get('security', {}).get('unprivileged', True) is not False
     if (desired.get('security', {}).get('unprivileged', True) is not False) != unprivileged \
@@ -532,7 +533,8 @@ def preflight(record, candidate, config, coordinated=None):
         raise ValueError(translate('Changing the rootfs or its storage requires a separate migration'))
     actual = mounts(config)
     gpu_devices.check(config, deployment)
-    gpu_devices.verify_baseline(record['observed'].get('gpu_devices', {}), deployment)
+    if baseline:
+        gpu_devices.verify_baseline(record['observed'].get('gpu_devices', {}), deployment)
     recorded_files = {m['source']: bool(m.get('read_only')) for m in deployment.get('mounts', []) if is_file_bind(m, files)}
     if files != recorded_files:
         raise ValueError(translate('A container mount has a source, backup or permission different from the saved record'))
@@ -1273,6 +1275,9 @@ def complete_recovery(root, journal, state):
 
 def recover(root, journal):
     state = json.loads(journal.read_text())
+    if state.get('kind') == 'native-modify':
+        import oci_native_modify
+        return oci_native_modify.recover(root, journal)
     vmid = state['vmid']
     show = not state.get('coordinated')
     if show:

oci/remote/oci_native_modify.py

@@ -0,0 +1,396 @@
+"""Edit an installed application's LXC configuration, retaining its rootfs/data.
+
+Runtime changes use a controlled stop/start, not pct destroy/create. Values,
+including secrets and NUL-separated environment, reach PVE over stdin only.
+"""
+import copy
+import hashlib
+import ipaddress
+import json
+from pathlib import Path
+import re
+import subprocess
+import uuid
+
+import oci_instances as instances
+import oci_instance_transaction as transaction
+import oci_resource_modify as resources
+import oci_gpu_devices as devices
+import oci_host_mounts as host_mounts
+from oci_installation_state import parse_config, sha
+from oci_ui import translate, msg_info, msg_ok, msg_warn
+
+PERL_UPDATE = r'''
+use strict; use warnings; use JSON::PP;
+use PVE::RPCEnvironment; use PVE::INotify; use PVE::API2::LXC::Config;
+PVE::RPCEnvironment->setup_default_cli_env();
+my $p = decode_json(do { local $/; <STDIN> });
+$p->{node} = PVE::INotify::nodename();
+PVE::API2::LXC::Config->update_vm($p);
+'''
+
+
+def environment(config):
+    result = {}
+    entries = []
+    for line in config.decode().splitlines():
+        if line.startswith('lxc.environment.runtime: '):
+            entries.append(line.split(': ', 1)[1])
+        elif line.startswith('env: '):
+            # pct config exposes PVE's public env property, whereas older
+            # observations and native files contain repeated runtime lines.
+            entries.extend(line.split(': ', 1)[1].split('\0'))
+    for entry in entries:
+        name, sep, value = entry.partition('=')
+        if not sep or name in result:
+            raise ValueError(translate('Duplicate variable in the contract; review it before editing'))
+        result[name] = value
+    return result
+
+
+def environment_values(items):
+    result = {}
+    for item in items:
+        name, value = item['name'], str(item['value'])
+        if (not re.fullmatch(r'[A-Za-z_][A-Za-z0-9_]*', name) or name in result
+                or any(ord(c) < 32 or ord(c) == 127 for c in value)):
+            raise ValueError(translate('The value contains an unsupported character'))
+        result[name] = value
+    return result
+
+
+def changes(record, candidate, config):
+    """Only unchanged recipes and native settings; adapters keep their installer."""
+    old = transaction.with_default_healthcheck(copy.deepcopy(record))
+    candidate = transaction.with_default_healthcheck(copy.deepcopy(candidate))
+    if (old['template'] != candidate['template'] or old['deployment'].get('host_monitor')
+            or any(k in record for k in ('stack', 'stack_member', 'native_stack_intent'))
+            or old['deployment'].get('stack_managed')):
+        return None
+    before, after = old['deployment'], candidate['deployment']
+    changed = {k for k in before.keys() | after.keys() if before.get(k) != after.get(k)}
+    allowed = {'resources', 'network', 'environment', 'devices', 'mounts', 'hardware_profile'}
+    if not changed or changed - allowed:
+        return None
+    resource_keys = {k for k in before.get('resources', {}).keys() | after.get('resources', {}).keys()
+                     if before.get('resources', {}).get(k) != after.get('resources', {}).get(k)}
+    if resource_keys - {'cores', 'memory_mb', 'swap_mb', 'cpu_units'}:
+        return None
+    if 'devices' in changed:
+        if any(d.get('kind', 'character-device') not in ('character-device', 'block-device')
+               or d.get('append_host_device_gid_to_environment')
+               for plan in (before, after) for d in plan.get('devices', [])):
+            return None
+        if before.get('device_permissions') or after.get('device_permissions'):
+            return None
+    if 'mounts' in changed:
+        if any(transaction.is_file_bind(m) for plan in (before, after) for m in plan.get('mounts', [])):
+            return None
+    return changed
+
+
+def plan(record, candidate, config):
+    changed = changes(record, candidate, config)
+    if not changed:
+        return None
+    transaction.preflight(record, candidate, config, in_place=True)
+    old, new = record['deployment'], candidate['deployment']
+    cfg = parse_config(config)
+    values, delete = {}, []
+    if 'resources' in changed:
+        for key, native, minimum in (('cores', 'cpulimit' if 'cpulimit' in cfg and 'cores' not in cfg else 'cores', 1),
+                                     ('memory_mb', 'memory', 128), ('swap_mb', 'swap', 0), ('cpu_units', 'cpuunits', 1)):
+            value = new['resources'].get(key)
+            if old['resources'].get(key) == value:
+                continue
+            if value is None and key == 'cpu_units':
+                if native in cfg: delete.append(native)
+            elif isinstance(value, bool) or not isinstance(value, int) or value < minimum:
+                raise ValueError(translate('Invalid resource value'))
+            else:
+                values[native] = str(value)
+    if 'network' in changed:
+        network = new['network']
+        if set(network) - {'bridge', 'ipv4', 'gateway', 'firewall', 'host_managed', 'mac_address'}:
+            raise ValueError(translate('Unsupported network settings'))
+        if not re.fullmatch(r'[A-Za-z0-9_.:-]+', network['bridge']):
+            raise ValueError(translate('Invalid bridge'))
+        if not Path('/sys/class/net', network['bridge'], 'bridge').is_dir():
+            raise ValueError(translate('The selected bridge does not exist'))
+        address = network['ipv4']
+        if address not in ('dhcp', 'manual'):
+            if ipaddress.ip_interface(address).version != 4:
+                raise ValueError(translate('Invalid IPv4 address'))
+        gateway = network.get('gateway')
+        if gateway and ipaddress.ip_address(gateway).version != 4:
+            raise ValueError(translate('Invalid IPv4 gateway'))
+        fields = dict(p.split('=', 1) for p in cfg['net0'].split(',') if '=' in p)
+        fields.update(bridge=network['bridge'], ip=address)
+        if gateway: fields['gw'] = gateway
+        else: fields.pop('gw', None)
+        for key, native in (('firewall', 'firewall'), ('host_managed', 'host-managed')):
+            if old['network'].get(key) != network.get(key):
+                fields[native] = '1' if network.get(key) else '0'
+        if old['network'].get('mac_address') != network.get('mac_address'):
+            raise ValueError(translate('The MAC address of the container cannot be kept'))
+        values['net0'] = ','.join(k+'='+v for k, v in fields.items())
+    if 'environment' in changed:
+        env = environment(config)
+        previous = environment_values(old.get('environment', []))
+        wanted = environment_values(new.get('environment', []))
+        defaults = dict(item.split('=', 1) for item in record['observed']['image'].get('defaults', {}).get('Env', []) if '=' in item)
+        for name in previous.keys() - wanted.keys():
+            if name in defaults: env[name] = defaults[name]
+            else: env.pop(name, None)
+        env.update(wanted)
+        if env: values['env'] = '\0'.join(k+'='+v for k, v in env.items())
+        else: delete.append('env')
+    if 'devices' in changed:
+        inventory = devices.planned(new)
+        existing = {v['path']: k for k, v in ((k, dict(p.split('=', 1) for p in v.split(',')))
+                   for k, v in cfg.items() if re.fullmatch(r'dev[0-9]+', k))}
+        used = set(existing.values())
+        for path, key in existing.items():
+            if path not in inventory: delete.append(key)
+        for device in new.get('devices', []):
+            path = device['host_path']
+            key = existing.get(path)
+            if not key:
+                key = next('dev'+str(n) for n in range(256) if 'dev'+str(n) not in used)
+                used.add(key)
+            info = inventory[path]
+            mode = info['mode'] if device.get('mode') == 'preserve-host' else int(device.get('mode', '0660'), 8)
+            values[key] = (f"path={path},mode={mode:04o},uid={int(device.get('uid', 0))},"
+                           f"gid={info['gid'] if device['gid_strategy'] == 'host-device-gid' else 0},"
+                           f"deny-write={1 if device.get('deny_write') else 0}")
+    if 'mounts' in changed:
+        actual = transaction.mounts(config)
+        used = {m['key'] for m in actual.values()}
+        wanted = {m['container_path']: m for m in new.get('mounts', [])}
+        for target, mount in actual.items():
+            if target not in wanted: delete.append(mount['key'])
+        for target, mount in wanted.items():
+            if target in actual:
+                original = next(m for m in old['mounts'] if m['container_path'] == target)
+                if mount == original: continue
+                fields = dict(p.split('=', 1) for p in actual[target]['value'].split(',')[1:] if '=' in p)
+                fields['ro'] = '1' if mount.get('read_only') else '0'
+                values[actual[target]['key']] = actual[target]['volume']+','+','.join(k+'='+v for k, v in fields.items())
+                continue
+            key = next('mp'+str(n) for n in range(256) if 'mp'+str(n) not in used)
+            used.add(key)
+            source = mount['source']
+            if mount['type'] == 'managed-volume':
+                if not re.fullmatch(r'[A-Za-z0-9_-]+', source):
+                    raise ValueError(translate('Invalid storage'))
+                source += ':'+str(mount['size_gb'])
+            else:
+                source = host_mounts.valid_path(source)
+                if not Path(source).is_dir():
+                    if not mount.get('create_if_missing'):
+                        raise ValueError(translate('The host directory does not exist'))
+            values[key] = (f"{source},mp={target},backup={1 if mount['backup'] else 0},"
+                           f"ro={1 if mount.get('read_only') else 0}")
+    return {'values': values, 'delete': delete, 'restart': bool(changed - {'resources'}),
+            'changed': sorted(changed)}
+
+
+def verify_values(config, operation):
+    """PVE may canonicalize property strings; compare their actual fields."""
+    cfg = parse_config(config)
+    for key, value in operation['values'].items():
+        if key == 'env':
+            if environment(config) != dict(item.split('=', 1) for item in value.split('\0')):
+                raise ValueError(translate('Proxmox could not apply the container settings'))
+        elif re.fullmatch(r'(mp|dev|net)[0-9]+', key):
+            desired = dict(p.split('=', 1) for p in value.split(',') if '=' in p)
+            actual = dict(p.split('=', 1) for p in cfg.get(key, '').split(',') if '=' in p)
+            defaults = {'uid': '0', 'gid': '0', 'deny-write': '0', 'ro': '0', 'backup': '0', 'mode': '0660', 'firewall': '0'}
+            def matches(k, v):
+                observed = actual.get(k, defaults.get(k))
+                if k == 'mode': return observed is not None and int(observed, 8) == int(v, 8)
+                return observed == v
+            if any(not matches(k, v) for k, v in desired.items()):
+                raise ValueError(translate('Proxmox could not apply the container settings'))
+        elif cfg.get(key) != value:
+            raise ValueError(translate('Proxmox could not apply the container settings'))
+    if any(key in cfg or (key == 'env' and environment(config)) for key in operation['delete']):
+        raise ValueError(translate('Proxmox could not apply the container settings'))
+
+
+def set_config(vmid, installation_id, expected, values, delete=()):
+    raw = (resources.CONFIG_DIR / f'{vmid}.conf').read_bytes()
+    if instances.command('pct', 'config', str(vmid)) != expected or instances.identity(expected) != installation_id:
+        raise ValueError(translate('The instance changed while it was being edited; open Modify again'))
+    payload = dict(values, vmid=vmid, digest=hashlib.sha1(raw).hexdigest())
+    if delete: payload['delete'] = ','.join(delete)
+    transaction.log('Applying native settings: '+', '.join(sorted(set(values) | set(delete))))
+    process = subprocess.run(['perl', '-e', PERL_UPDATE], input=json.dumps(payload).encode(),
+                             capture_output=True, timeout=120)
+    # API errors may include secret values. Only keep the exit status.
+    if process.returncode:
+        raise RuntimeError(translate('Proxmox could not apply the container settings'))
+
+
+def untouched(before, after, keys):
+    keys = set(keys)
+    if 'env' in keys: keys.add('lxc.environment.runtime')
+    def keep(config):
+        return b''.join(line for line in config.splitlines(keepends=True)
+                        if line.split(b':', 1)[0].decode() not in keys
+                        and not re.match(rb'unused[0-9]+:', line))
+    return keep(before) == keep(after)
+
+
+def restore_settings(root, record, before, now, operation):
+    vmid = record['vmid']
+    keys = set(operation['values']) | set(operation['delete'])
+    if operation['restart']: transaction.stop(vmid)
+    original = parse_config(before)
+    restore = {k: original[k] for k in keys if k in original and k != 'env'}
+    remove = [k for k in keys if k not in original and k != 'env' and k in parse_config(now)]
+    if 'env' in keys:
+        env = environment(before)
+        if env: restore['env'] = '\0'.join(k+'='+v for k, v in env.items())
+        elif environment(now): remove.append('env')
+    if restore or remove:
+        set_config(vmid, record['installation_id'], now, restore, remove)
+    restored_config = instances.command('pct', 'config', str(vmid))
+    verify_values(restored_config, {'values': restore, 'delete': remove})
+    if not untouched(before, restored_config, keys):
+        raise ValueError(translate('The instance changed while it was being edited; open Modify again'))
+    restored = copy.deepcopy(record)
+    obs = record['observed']
+    restored['observed'] = instances.observe(vmid, record['installation_id'],
+        obs['archive_path'], obs['resolved_registry_digest'], obs['image'])
+    if restored['observed']['config_sha256'] != sha(restored_config):
+        raise ValueError(translate('The instance changed while it was being edited; open Modify again'))
+    return restored
+
+
+def recover(root, journal):
+    """Recover through the normal Manage interrupted-operation menu."""
+    state = json.loads(journal.read_text())
+    record, operation = state['record'], state['operation_plan']
+    vmid = record['vmid']
+    current = instances.read(root, vmid)
+    if (state['kind'] != 'native-modify' or state['phase'] in ('committed', 'rolled-back')
+            or current.get('pending_transaction') != str(journal)
+            or current.get('transaction_id') != state['id']
+            or current['installation_id'] != record['installation_id']):
+        raise ValueError(translate('The record does not belong to this operation'))
+    before = state['config'].encode()
+    now = instances.command('pct', 'config', str(vmid))
+    keys = set(operation['values']) | set(operation['delete'])
+    if instances.identity(now) != record['installation_id'] or not untouched(before, now, keys):
+        raise ValueError(translate('The instance changed while it was being edited; open Modify again'))
+    if state.get('after_config'):
+        if now not in (before, state['after_config'].encode()):
+            raise ValueError(translate('The instance changed while it was being edited; open Modify again'))
+    elif now != before:
+        # A crash during the PVE API call is recoverable only when the full
+        # planned edit is present. Refuse unknown partial/concurrent changes.
+        verify_values(now, operation)
+    host_mounts.verify_sources(state.get('original_host_sources', {}))
+    tx_record = restore_settings(root, record, before, now, operation) if now != before else copy.deepcopy(record)
+    if state['was_running']:
+        if transaction.run('pct', 'status', str(vmid)).strip() != b'status: running':
+            transaction.run('pct', 'start', str(vmid))
+        transaction.healthcheck(vmid, record['template'])
+    instances.write(instances.location(root, vmid), tx_record)
+    state['phase'] = 'rolled-back'
+    instances.write(journal, state)
+    msg_ok(translate('Previous installation restored'))
+
+
+def apply(root, record, candidate, config, acknowledge_external_data=False):
+    operation = plan(record, candidate, config)
+    if not operation:
+        raise ValueError(translate('The changes need the application installer'))
+    original_sources, desired_sources = transaction.freeze_host_sources(
+        record, candidate, acknowledge_external_data)
+    vmid = record['vmid']
+    folder = instances.location(root, vmid).parent / 'modifications' / uuid.uuid4().hex
+    running = transaction.run('pct', 'status', str(vmid)).strip() == b'status: running'
+    journal = {'kind': 'native-modify', 'id': folder.name, 'vmid': vmid, 'operation': 'recreate',
+               'record': record, 'config': config.decode(), 'operation_plan': operation,
+               'phase': 'prepared', 'was_running': running, 'original_host_sources': original_sources}
+    instances.write(folder / 'before.json', journal)
+    transaction.open_log(folder)
+    transaction._run['journal'] = folder / 'before.json'
+    pending = copy.deepcopy(record)
+    pending.update(status='updating', transaction_id=folder.name, pending_transaction=str(folder / 'before.json'))
+    instances.write(instances.location(root, vmid), pending)
+    updated_config = None
+    changed = False
+    try:
+        if operation['restart'] and running:
+            transaction.stop(vmid)
+        # Do not accept a concurrent edit merely because the service stopped.
+        if instances.command('pct', 'config', str(vmid)) != config:
+            raise ValueError(translate('The instance changed while it was being edited; open Modify again'))
+        host_mounts.verify_sources(desired_sources)
+        for mount in candidate['deployment'].get('mounts', []):
+            if mount['type'] == 'host-bind' and mount.get('create_if_missing'):
+                Path(mount['source']).mkdir(parents=True, exist_ok=True)
+        host_mounts.verify_sources(original_sources)
+        # Capture newly-created directories before installing their mounts.
+        pinned = {p: host_mounts.validate_source(p) for p in desired_sources}
+        for path, previous in desired_sources.items():
+            if previous['exists'] and not host_mounts.same_source(previous, pinned[path]):
+                raise ValueError(translate('A shared source does not match its recorded identity'))
+        host_mounts.verify_sources(pinned)
+        msg_info(translate('Applying container settings without rebuilding the container...'))
+        if operation['values'] or operation['delete']:
+            changed = True  # API updates may partially succeed before an error.
+            set_config(vmid, record['installation_id'], config, operation['values'], operation['delete'])
+        updated_config = instances.command('pct', 'config', str(vmid))
+        verify_values(updated_config, operation)
+        keys = set(operation['values']) | set(operation['delete'])
+        if not untouched(config, updated_config, keys):
+            raise ValueError(translate('The instance changed while it was being edited; open Modify again'))
+        transaction.preflight(candidate, candidate, updated_config, in_place=True, baseline=False)
+        journal.update(phase='configured', after_config=updated_config.decode())
+        instances.write(folder / 'before.json', journal)
+        if operation['restart'] and running:
+            transaction.run('pct', 'start', str(vmid))
+            transaction.healthcheck(vmid, candidate['template'])
+        observed = record['observed']
+        fresh = instances.observe(vmid, record['installation_id'], observed['archive_path'],
+                                  observed['resolved_registry_digest'], observed['image'])
+        host_mounts.verify_observation(pinned, fresh)
+        if fresh['config_sha256'] != sha(updated_config):
+            raise ValueError(translate('The instance changed while it was being edited; open Modify again'))
+        result = copy.deepcopy(record)
+        result.update(deployment=copy.deepcopy(candidate['deployment']), observed=fresh)
+        instances.write(instances.location(root, vmid), result)
+        journal['phase'] = 'committed'
+        instances.write(folder / 'before.json', journal)
+    except BaseException:
+        try:
+            now = instances.command('pct', 'config', str(vmid))
+            keys = set(operation['values']) | set(operation['delete'])
+            if changed:
+                if not untouched(config, now, keys) or (updated_config is not None and now != updated_config):
+                    raise ValueError('Concurrent edit; rollback refused')
+                restored = restore_settings(root, record, config, now, operation)
+            else:
+                restored = copy.deepcopy(record)
+            if operation['restart'] and running:
+                if transaction.run('pct', 'status', str(vmid)).strip() != b'status: running':
+                    transaction.run('pct', 'start', str(vmid))
+                transaction.healthcheck(vmid, record['template'])
+            instances.write(instances.location(root, vmid), restored)
+            journal['phase'] = 'rolled-back'
+            instances.write(folder / 'before.json', journal)
+        except Exception as error:
+            transaction.log('native rollback failed: '+type(error).__name__)
+            msg_warn(translate('The settings could not be fully rolled back. Review the container and its saved modification before continuing.'))
+        raise
+    try:
+        import oci_carried_record
+        oci_carried_record.carry(root, vmid, mount_stopped=False)
+    except (OSError, ValueError, RuntimeError):
+        transaction.log('carried record will be refreshed on the next management entry')
+    msg_ok(translate('Changes applied. Data kept.'))
+    return folder

oci/remote/oci_update_current.py

@@ -180,6 +180,16 @@ def application_name(record, vmid):
     return f'CT {vmid}'
 
 
+def backup_native_settings(vmid, storage):
+    """An optional retained backup does not require rebuilding the guest."""
+    if storage:
+        import oci_keep_backup
+        oci_keep_backup.validate(storage)
+        transaction.run('vzdump', str(vmid), '--storage', storage, '--mode', 'snapshot',
+                        '--compress', 'zstd', '--notes-template',
+                        'ProxMenux OCI: before modifying container settings')
+
+
 def update(vmid, acknowledge_external_data=False, proposal=None, keep_backup=None):
     operation = 'recreate' if proposal is not None else 'update'
     msg_info(translate('Checking the container before the update...') if operation == 'update'
@@ -192,7 +202,7 @@ def update(vmid, acknowledge_external_data=False, proposal=None, keep_backup=Non
         desired = transaction.candidate_contract(record, operation, proposal)
         changes = transaction.external_changes(record, config)
         resource_changes = None
-        if operation == 'recreate' and not keep_backup and not changes:
+        if operation == 'recreate' and not changes:
             import oci_resource_modify
             resource_changes = oci_resource_modify.changes(record, desired, config)
         # CPU/RAM edits retain the installed image and all unrelated native
@@ -202,7 +212,22 @@ def update(vmid, acknowledge_external_data=False, proposal=None, keep_backup=Non
             msg_ok(translate('Container checked'))
             import oci_operation_notice
             with oci_operation_notice.operation([vmid], 'modify', application_name(record, vmid)):
+                backup_native_settings(vmid, keep_backup)
                 return oci_resource_modify.apply(instances.ROOT, record, desired, config, resource_changes)
+        if operation == 'recreate' and not changes:
+            import oci_native_modify
+            if oci_native_modify.changes(record, desired, config):
+                import oci_operation_notice
+                with oci_operation_notice.operation([vmid], 'modify', application_name(record, vmid)):
+                    # Validate the native plan before an optional full backup.
+                    oci_native_modify.plan(record, desired, config)
+                    backup_native_settings(vmid, keep_backup)
+                    return oci_native_modify.apply(instances.ROOT, record, desired, config,
+                                                   acknowledge_external_data=acknowledge_external_data)
+        if (operation == 'recreate'
+                and record['template']['container_contract']['image']['reference']
+                == desired['template']['container_contract']['image']['reference']):
+            raise ValueError(translate('These settings require an application-specific adaptation. The image was not downloaded and the container was not recreated.'))
         transaction.preflight(record, desired, config)
         msg_ok(translate('Container checked'))
         current = record['observed']['image'] if operation == 'update' else None

oci/src/proxmenux_oci/installer.py

@@ -1588,7 +1588,7 @@ def _run_remote_install(
             shutil.copy2(project_root / 'remote' / 'haos_healthcheck.py', temporary / 'haos_healthcheck.py')
             shutil.copy2(project_root / 'remote' / 'oci_installation_state.py', temporary / 'oci_installation_state.py')
             shutil.copy2(project_root / 'remote' / 'oci_instances.py', temporary / 'oci_instances.py')
-            for helper in ('oci_ui.sh', 'oci_ui.py', 'oci_description.py', 'oci_console.py', 'oci_native_stack.py', 'oci_native_stack.sh', 'oci_instance_transaction.py', 'oci_host_mounts.py', 'oci_nested_mounts.py', 'oci_runtime_settings.py', 'oci_gpu_devices.py', 'oci_accelerators.py', 'oci_nvidia_runtime.py', 'oci_nvidia_refresh.py', 'oci_nvidia_dynamic.py', 'oci_update_current.py', 'oci_stack_replay.py', 'oci_stack_plan.py', 'oci_stack_transaction.py', 'oci_stack_native.py', 'oci_image_cache.py', 'nvidia_lxc_mount_lab.sh', 'oci_nvidia_setup.sh', 'oci_immich_ml.sh', 'oci_rocm_check.py'):
+            for helper in ('oci_ui.sh', 'oci_ui.py', 'oci_description.py', 'oci_console.py', 'oci_native_stack.py', 'oci_native_stack.sh', 'oci_instance_transaction.py', 'oci_host_mounts.py', 'oci_nested_mounts.py', 'oci_runtime_settings.py', 'oci_gpu_devices.py', 'oci_accelerators.py', 'oci_nvidia_runtime.py', 'oci_nvidia_refresh.py', 'oci_nvidia_dynamic.py', 'oci_update_current.py', 'oci_resource_modify.py', 'oci_native_modify.py', 'oci_stack_replay.py', 'oci_stack_plan.py', 'oci_stack_transaction.py', 'oci_stack_native.py', 'oci_image_cache.py', 'nvidia_lxc_mount_lab.sh', 'oci_nvidia_setup.sh', 'oci_immich_ml.sh', 'oci_rocm_check.py'):
                 shutil.copy2(project_root / 'remote' / helper, temporary / helper)
             if deployment_kind == 'generic-multi-lxc-stack':
                 shutil.copy2(project_root / 'remote' / 'install_generic_stack.py', temporary / 'install_generic_stack.py')
@@ -1625,7 +1625,7 @@ def _run_remote_install(
             archive.add(project_root / 'remote' / 'haos_healthcheck.py', arcname='haos_healthcheck.py')
             archive.add(project_root / 'remote' / 'oci_installation_state.py', arcname='oci_installation_state.py')
             archive.add(project_root / 'remote' / 'oci_instances.py', arcname='oci_instances.py')
-            for helper in ('oci_ui.sh', 'oci_ui.py', 'oci_description.py', 'oci_console.py', 'oci_native_stack.py', 'oci_native_stack.sh', 'oci_instance_transaction.py', 'oci_host_mounts.py', 'oci_nested_mounts.py', 'oci_runtime_settings.py', 'oci_gpu_devices.py', 'oci_accelerators.py', 'oci_nvidia_runtime.py', 'oci_nvidia_refresh.py', 'oci_nvidia_dynamic.py', 'oci_update_current.py', 'oci_stack_replay.py', 'oci_stack_plan.py', 'oci_stack_transaction.py', 'oci_stack_native.py', 'oci_image_cache.py', 'nvidia_lxc_mount_lab.sh', 'oci_nvidia_setup.sh', 'oci_immich_ml.sh', 'oci_rocm_check.py'):
+            for helper in ('oci_ui.sh', 'oci_ui.py', 'oci_description.py', 'oci_console.py', 'oci_native_stack.py', 'oci_native_stack.sh', 'oci_instance_transaction.py', 'oci_host_mounts.py', 'oci_nested_mounts.py', 'oci_runtime_settings.py', 'oci_gpu_devices.py', 'oci_accelerators.py', 'oci_nvidia_runtime.py', 'oci_nvidia_refresh.py', 'oci_nvidia_dynamic.py', 'oci_update_current.py', 'oci_resource_modify.py', 'oci_native_modify.py', 'oci_stack_replay.py', 'oci_stack_plan.py', 'oci_stack_transaction.py', 'oci_stack_native.py', 'oci_image_cache.py', 'nvidia_lxc_mount_lab.sh', 'oci_nvidia_setup.sh', 'oci_immich_ml.sh', 'oci_rocm_check.py'):
                 archive.add(project_root / 'remote' / helper, arcname=helper)
             if deployment_kind == 'generic-multi-lxc-stack':
                 archive.add(project_root / 'remote' / 'install_generic_stack.py', arcname='install_generic_stack.py')

oci/tests/test_modify_image_reuse.py

@@ -287,6 +287,7 @@ class UpdateRoutingTests(unittest.TestCase):
         candidate['deployment']['resources']['memory_mb'] = 768
         if not resource_edit:
             candidate['deployment']['environment'][0]['value'] = 'changed'
+            candidate['template']['container_contract']['image']['reference'] = 'example/app:other'
         proposal = {'operation': 'recreate', 'candidate': candidate}
         import contextlib, oci_operation_notice
         with patch.object(instances, 'locked', return_value=contextlib.nullcontext()), \

oci/tests/test_native_modify.py

@@ -0,0 +1,258 @@
+"""Native runtime edits keep rootfs, defaults and secrets; failures roll back."""
+import contextlib
+import copy
+import json
+from pathlib import Path
+import re
+import subprocess
+import sys
+import tempfile
+import unittest
+from unittest.mock import patch, Mock
+
+sys.path.insert(0, str(Path(__file__).resolve().parents[1]/'remote'))
+import oci_native_modify as native
+import oci_instances as instances
+import oci_update_current as update
+import oci_operation_notice
+from test_modify_image_reuse import record, CONFIG
+
+
+class NativePlanTests(unittest.TestCase):
+    def setUp(self):
+        self.record = record()
+        self.record['observed']['image']['defaults'] = {'Env': ['A=image-default']}
+        self.record['deployment']['environment'] = [{'name': 'A', 'value': 'one'}]
+        self.config = CONFIG+b'net0: name=eth0,bridge=vmbr0,ip=dhcp,hwaddr=AA:BB:CC:DD:EE:FF,tag=20,ip6=auto\n'
+        self.candidate = copy.deepcopy(self.record)
+        p = patch.object(native.transaction, 'preflight')
+        self.preflight = p.start(); self.addCleanup(p.stop)
+
+    def test_environment_preserves_image_defaults_and_other_runtime_values(self):
+        self.candidate['deployment']['environment'] = [{'name': 'SECRET', 'value': "a' $x \" ; = value", 'sensitive': True}]
+        result = native.plan(self.record, self.candidate, self.config)
+        env = dict(x.split('=', 1) for x in result['values']['env'].split('\0'))
+        self.assertEqual(env, {'A': 'image-default', 'B': 'two', 'SECRET': "a' $x \" ; = value"})
+        self.assertTrue(result['restart'])
+
+    def test_remove_environment_without_default_restores_image_environment(self):
+        self.record['observed']['image']['defaults'] = {}
+        self.candidate['deployment']['environment'] = []
+        self.assertEqual(native.plan(self.record, self.candidate, self.config)['values']['env'], 'B=two')
+
+    def test_public_pct_environment_and_native_lines_have_the_same_values(self):
+        raw=b'lxc.environment.runtime: A=one\nlxc.environment.runtime: B=two\n'
+        public=b'env: A=one\0B=two\n'
+        self.assertEqual(native.environment(raw),native.environment(public))
+        self.assertEqual(native.environment(public),{'A':'one','B':'two'})
+
+    def test_duplicate_invalid_and_control_character_values_are_refused(self):
+        for items in ([{'name':'X','value':'a'},{'name':'X','value':'b'}],
+                      [{'name':'X','value':'a\nb'}],[{'name':'BAD NAME','value':'a'}]):
+            self.candidate['deployment']['environment'] = items
+            with self.assertRaises(ValueError): native.plan(self.record, self.candidate, self.config)
+
+    def test_network_preserves_mac_vlan_ipv6_and_interface_identity(self):
+        self.candidate['deployment']['network'].update(ipv4='192.0.2.2/24', gateway='192.0.2.1')
+        with patch.object(Path, 'is_dir', return_value=True):
+            result = native.plan(self.record, self.candidate, self.config)
+        fields = dict(x.split('=',1) for x in result['values']['net0'].split(','))
+        self.assertEqual(fields['hwaddr'], 'AA:BB:CC:DD:EE:FF')
+        self.assertEqual(fields['tag'], '20'); self.assertEqual(fields['ip6'], 'auto')
+        self.assertEqual(fields['ip'], '192.0.2.2/24'); self.assertEqual(fields['gw'], '192.0.2.1')
+
+    def test_invalid_network_is_rejected_before_stopping(self):
+        for value in ('nonsense', '2001:db8::1/64'):
+            self.candidate['deployment']['network']['ipv4'] = value
+            with patch.object(Path, 'is_dir', return_value=True), self.assertRaises(ValueError):
+                native.plan(self.record,self.candidate,self.config)
+
+    def test_new_managed_disk_uses_native_allocation_and_existing_disk_is_untouched(self):
+        self.candidate['deployment']['mounts'] = [{'type':'managed-volume','container_path':'/extra',
+            'source':'local-zfs','size_gb':1,'backup':True,'read_only':False}]
+        result = native.plan(self.record,self.candidate,self.config)
+        self.assertEqual(result['values'], {'mp0':'local-zfs:1,mp=/extra,backup=1,ro=0'})
+        self.assertNotIn('rootfs', result['values'])
+
+    def test_removing_mount_detaches_it_without_freeing_its_volume(self):
+        mount={'type':'managed-volume','container_path':'/extra','source':'local-zfs','size_gb':1,'backup':True}
+        self.record['deployment']['mounts']=[mount]
+        result=native.plan(self.record,self.candidate,self.config+b'mp4: local-zfs:subvol-115-disk-1,mp=/extra,backup=1,size=1G\n')
+        self.assertEqual(result['delete'],['mp4'])
+
+    def test_image_or_installer_adapter_changes_do_not_take_native_path(self):
+        for key in ('image', 'adapter'):
+            candidate=copy.deepcopy(self.record)
+            if key=='image':candidate['template']['container_contract']['image']['reference']='new/image'
+            else:candidate['deployment']['post_start_configurations']=[{'id':'rootfs-adapter'}]
+            self.assertIsNone(native.changes(self.record,candidate,self.config))
+
+    def test_secrets_use_stdin_never_arguments_or_logs(self):
+        secret="super-secret with 'quotes' and $shell"
+        with tempfile.TemporaryDirectory() as directory:
+            root=Path(directory);(root/'115.conf').write_bytes(self.config)
+            with patch.object(native.resources,'CONFIG_DIR',root), \
+                    patch.object(instances,'command',return_value=self.config), \
+                    patch.object(native.transaction,'log') as log, \
+                    patch.object(native.subprocess,'run',return_value=Mock(returncode=0)) as run:
+                native.set_config(115,self.record['installation_id'],self.config,{'env':'SECRET='+secret+'\0B=two'})
+            self.assertNotIn(secret,str(run.call_args.args))
+            self.assertNotIn(secret,str(log.call_args_list))
+            self.assertIn(secret,json.loads(run.call_args.kwargs['input'])['env'])
+
+    def test_native_api_failure_does_not_echo_secret_stderr(self):
+        with tempfile.TemporaryDirectory() as directory:
+            root=Path(directory);(root/'115.conf').write_bytes(self.config)
+            with patch.object(native.resources,'CONFIG_DIR',root),patch.object(instances,'command',return_value=self.config), \
+                    patch.object(native.subprocess,'run',return_value=Mock(returncode=1,stderr=b'secret')):
+                with self.assertRaisesRegex(RuntimeError,'Proxmox could not apply') as error:
+                    native.set_config(115,self.record['installation_id'],self.config,{'env':'SECRET=secret'})
+            self.assertNotIn('secret',str(error.exception))
+
+    def test_verifies_all_environment_entries_and_native_values(self):
+        native.verify_values(CONFIG,{'values':{'env':'A=one\0B=two','cores':'2'},'delete':[]})
+        for values in ({'env':'A=wrong\0B=two'},{'cores':'3'}):
+            with self.assertRaises(ValueError):native.verify_values(CONFIG,{'values':values,'delete':[]})
+        with self.assertRaises(ValueError):native.verify_values(CONFIG,{'values':{},'delete':['env']})
+
+    def test_native_property_canonicalization_keeps_modes_and_default_flags(self):
+        cfg=CONFIG+b'dev0: path=/dev/fuse,mode=660\n'
+        native.verify_values(cfg,{'values':{'dev0':'path=/dev/fuse,mode=0660,uid=0,gid=0,deny-write=0'},'delete':[]})
+
+
+class NativeApplyTests(unittest.TestCase):
+    def setUp(self):
+        self.temp=tempfile.TemporaryDirectory();self.addCleanup(self.temp.cleanup)
+        self.root=Path(self.temp.name)
+        self.record=record();self.candidate=copy.deepcopy(self.record)
+        self.candidate['deployment']['environment']=[{'name':'A','value':'changed'}]
+        self.config=CONFIG
+        self.state={'config':CONFIG,'running':True}
+        self.commands=[]
+        def run(*args):
+            self.commands.append(args)
+            if args[:2]==('pct','status'):return b'status: running' if self.state['running'] else b'status: stopped'
+            if args[:2]==('pct','start'):self.state['running']=True
+            return b''
+        def stop(vmid):self.state['running']=False
+        def setter(vmid,identity,expected,values,delete=()):
+            self.assertEqual(expected,self.state['config'])
+            text=self.state['config'].decode()
+            if 'env' in values or 'env' in delete:
+                public='env: ' in text
+                text=''.join(x for x in text.splitlines(keepends=True) if not x.startswith(('lxc.environment.runtime: ','env: ')))
+                if public and values.get('env'):text+='env: '+values['env']+'\n'
+                else:
+                    for item in values.get('env','').split('\0'):
+                        if item:text+='lxc.environment.runtime: '+item+'\n'
+            self.state['config']=text.encode()
+        def observe(*args):
+            observed=copy.deepcopy(self.record['observed'])
+            observed.update(config=self.state['config'].decode(),config_sha256=native.sha(self.state['config']))
+            return observed
+        patches=[patch.object(native,'plan',return_value={'values':{'env':'A=changed\0B=two'},'delete':[],'restart':True,'changed':['environment']}),
+            patch.object(native,'set_config',side_effect=setter),patch.object(instances,'command',side_effect=lambda *a:self.state['config']),
+            patch.object(native.transaction,'run',side_effect=run),patch.object(native.transaction,'stop',side_effect=stop),
+            patch.object(native.transaction,'healthcheck'),patch.object(native.transaction,'preflight'),
+            patch.object(native.transaction,'freeze_host_sources',return_value=({},{})),
+            patch.object(native.transaction,'open_log'),patch.object(native.transaction,'log'),
+            patch.object(instances,'observe',side_effect=observe),patch('oci_carried_record.carry')]
+        for p in patches:p.start();self.addCleanup(p.stop)
+        instances.write(instances.location(self.root,115),self.record)
+
+    def apply(self):return native.apply(self.root,self.record,self.candidate,self.config)
+
+    def test_runtime_edit_restarts_existing_guest_and_keeps_rootfs_image_and_data(self):
+        folder=self.apply()
+        saved=instances.read(self.root,115)
+        self.assertEqual(saved['observed']['image'],self.record['observed']['image'])
+        self.assertIn(b'rootfs: local-zfs:subvol-115-disk-0',self.state['config'])
+        self.assertEqual(saved['deployment'],self.candidate['deployment'])
+        self.assertTrue(self.state['running'])
+        self.assertEqual(json.loads((folder/'before.json').read_text())['phase'],'committed')
+        self.assertFalse(any(c[:2] in (('pct','create'),('pct','destroy')) for c in self.commands))
+
+    def test_failed_healthcheck_rolls_back_and_starts_original_service(self):
+        with patch.object(native.transaction,'healthcheck',side_effect=[RuntimeError('not healthy'),None]):
+            with self.assertRaisesRegex(RuntimeError,'not healthy'):self.apply()
+        self.assertEqual(self.state['config'],CONFIG)
+        self.assertTrue(self.state['running'])
+        self.assertEqual(instances.read(self.root,115)['deployment'],self.record['deployment'])
+
+    def test_health_failure_restores_public_nul_separated_environment(self):
+        self.config=CONFIG.replace(b'lxc.environment.runtime: A=one\nlxc.environment.runtime: B=two\n',b'env: A=one\0B=two\n')
+        self.state['config']=self.config
+        self.record['observed'].update(config=self.config.decode(),config_sha256=native.sha(self.config))
+        instances.write(instances.location(self.root,115),self.record)
+        with patch.object(native.transaction,'healthcheck',side_effect=[RuntimeError('not healthy'),None]):
+            with self.assertRaises(RuntimeError):self.apply()
+        self.assertEqual(native.environment(self.state['config']),{'A':'one','B':'two'})
+        self.assertTrue(self.state['running'])
+
+    def test_stopped_guest_stays_stopped(self):
+        self.state['running']=False
+        self.apply()
+        self.assertFalse(self.state['running'])
+        self.assertNotIn(('pct','start','115'),self.commands)
+
+    def test_interrupted_edit_is_recovered_through_transaction_dispatch(self):
+        folder=self.apply()
+        path=folder/'before.json';state=json.loads(path.read_text());state['phase']='configured'
+        instances.write(path,state)
+        pending=copy.deepcopy(self.record)
+        pending.update(status='updating',transaction_id=folder.name,pending_transaction=str(path))
+        instances.write(instances.location(self.root,115),pending)
+        native.transaction.recover(self.root,path)
+        self.assertEqual(self.state['config'],CONFIG)
+        saved=instances.read(self.root,115)
+        self.assertEqual(saved['status'],'installed');self.assertNotIn('pending_transaction',saved)
+        self.assertEqual(json.loads(path.read_text())['phase'],'rolled-back')
+
+    def test_recovery_refuses_a_concurrent_environment_edit(self):
+        folder=self.apply();path=folder/'before.json';state=json.loads(path.read_text());state['phase']='configured'
+        instances.write(path,state)
+        pending=copy.deepcopy(self.record)
+        pending.update(status='updating',transaction_id=folder.name,pending_transaction=str(path))
+        instances.write(instances.location(self.root,115),pending)
+        self.state['config']=self.state['config'].replace(b'A=changed',b'A=concurrent')
+        with self.assertRaisesRegex(ValueError,'changed'):native.transaction.recover(self.root,path)
+        self.assertIn(b'A=concurrent',self.state['config'])
+
+    def test_concurrent_change_before_write_is_not_overwritten(self):
+        def stop(vmid):
+            self.state['running']=False
+            self.state['config']+=b'timezone: host\n'
+        with patch.object(native.transaction,'stop',side_effect=stop):
+            with self.assertRaisesRegex(ValueError,'changed'):self.apply()
+        self.assertIn(b'timezone: host',self.state['config'])
+        self.assertTrue(self.state['running'])
+
+    def test_router_does_not_resolve_download_or_recreate_for_environment_edit(self):
+        with patch.object(instances,'ROOT',self.root),patch.object(instances,'locked',return_value=contextlib.nullcontext()), \
+                patch.object(native.transaction,'external_changes',return_value={}), \
+                patch.object(update,'resolve_archive',side_effect=AssertionError('registry path')), \
+                patch.object(native.transaction,'apply',side_effect=AssertionError('recreation')), \
+                patch.object(oci_operation_notice,'operation',return_value=contextlib.nullcontext()):
+            update.update(115,proposal={'operation':'recreate','candidate':self.candidate})
+        self.assertEqual(instances.read(self.root,115)['deployment']['environment'],self.candidate['deployment']['environment'])
+
+    def test_unsupported_same_image_adaptation_never_silently_recreates(self):
+        self.candidate['deployment']['post_start_configurations']=[{'id':'unsupported-adapter'}]
+        with patch.object(instances,'ROOT',self.root),patch.object(instances,'locked',return_value=contextlib.nullcontext()), \
+                patch.object(native.transaction,'external_changes',return_value={}), \
+                patch.object(update,'resolve_archive',side_effect=AssertionError('registry path')), \
+                patch.object(native.transaction,'apply',side_effect=AssertionError('recreation')):
+            with self.assertRaisesRegex(ValueError,'application-specific adaptation'):
+                update.update(115,proposal={'operation':'recreate','candidate':self.candidate})
+
+    def test_optional_backup_does_not_force_recreation(self):
+        with patch.object(instances,'ROOT',self.root),patch.object(instances,'locked',return_value=contextlib.nullcontext()), \
+                patch.object(native.transaction,'external_changes',return_value={}), \
+                patch.object(update,'resolve_archive',side_effect=AssertionError('registry path')), \
+                patch.object(update,'backup_native_settings') as backup, \
+                patch.object(oci_operation_notice,'operation',return_value=contextlib.nullcontext()):
+            update.update(115,proposal={'operation':'recreate','candidate':self.candidate},keep_backup='test-storage')
+        backup.assert_called_once_with(115,'test-storage')
+
+
+if __name__=='__main__':unittest.main()