mirror of
https://github.com/MacRimi/ProxMenux.git
synced 2026-11-03 03:12:07 +00:00
Carry over OCI variables changed in Proxmox and keep the LXC update badge on old checks
This commit is contained in:
MacRimi
@@ -9,6 +9,7 @@ import oci_instances as instances
|
||||
import oci_gpu_devices as gpu_devices
|
||||
import oci_host_mounts as host_mounts
|
||||
import oci_instance_transaction as transaction
|
||||
import oci_native_modify as native_modify
|
||||
from oci_installation_state import parse_config, sha
|
||||
from oci_ui import translate
|
||||
|
||||
@@ -88,6 +89,43 @@ def _device(key, value):
|
||||
return device
|
||||
|
||||
|
||||
ENVIRONMENT_KEYS = ('env', 'lxc.environment.runtime')
|
||||
|
||||
|
||||
def _environment(record, config):
|
||||
"""Variables changed in Proxmox since the last operation. Values are
|
||||
applied but never listed: they may be passwords."""
|
||||
before = native_modify.environment(record['observed']['config'].encode())
|
||||
now = native_modify.environment(config)
|
||||
defaults = {item.split('=', 1)[0] for item in record['observed']['image'].get('defaults', {}).get('Env', [])
|
||||
if '=' in item}
|
||||
removed = sorted(before.keys() - now.keys())
|
||||
# An image default comes back with a new container; its absence cannot be kept.
|
||||
if any(name in defaults for name in removed):
|
||||
raise ValueError(f"{translate('These manual changes cannot be adopted safely:')} env")
|
||||
changed = sorted(name for name in now if before.get(name) != now[name])
|
||||
return {name: now[name] for name in changed}, removed
|
||||
|
||||
|
||||
def _adopt_environment(deployment, values, removed):
|
||||
items = deployment.setdefault('environment', [])
|
||||
details = []
|
||||
for name, value in values.items():
|
||||
matches = [item for item in items if item['name'] == name]
|
||||
if len(matches) > 1:
|
||||
raise ValueError(translate('Duplicate variable in the contract; review it before editing'))
|
||||
if matches:
|
||||
matches[0]['value'] = value
|
||||
details.append(f"env {name}: {translate('value changed in Proxmox')}")
|
||||
else:
|
||||
items.append({'name': name, 'value': value, 'sensitive': True})
|
||||
details.append(f"env {name}: {translate('added in Proxmox')}")
|
||||
for name in removed:
|
||||
items[:] = [item for item in items if item['name'] != name]
|
||||
details.append(f"env {name}: {translate('removed in Proxmox')}")
|
||||
return details
|
||||
|
||||
|
||||
def propose(record, config):
|
||||
if (record.get('status') != 'installed' or record.get('stack') or record.get('stack_member')
|
||||
or record.get('native_stack_intent') or record.get('deployment', {}).get('stack_managed')):
|
||||
@@ -101,16 +139,20 @@ def propose(record, config):
|
||||
# was checked above, so a presentation-only change needs no adoption.
|
||||
if 'description' in changed:
|
||||
changed.remove('description')
|
||||
environment = None
|
||||
if any(key in ENVIRONMENT_KEYS for key in changed):
|
||||
changed = [key for key in changed if key not in ENVIRONMENT_KEYS]
|
||||
environment = _environment(record, config)
|
||||
new_keys = [key for key in changed if key not in before and re.fullmatch(r'(mp|dev)[0-9]+', key)]
|
||||
unsupported = [key for key in changed if key not in new_keys and key not in transaction.ADOPTABLE
|
||||
and key not in transaction.KEPT_AS_IS]
|
||||
if unsupported:
|
||||
raise ValueError(f"{translate('These manual changes cannot be adopted safely:')} {', '.join(unsupported)}")
|
||||
if not new_keys:
|
||||
if not new_keys and environment is None:
|
||||
return None
|
||||
candidate = copy.deepcopy(record)
|
||||
deployment = candidate['deployment']
|
||||
details = []
|
||||
details = _adopt_environment(deployment, *environment) if environment else []
|
||||
for key in new_keys:
|
||||
if key.startswith('mp'):
|
||||
mount = _mount(key, current[key], record['vmid'])
|
||||
@@ -122,8 +164,13 @@ def propose(record, config):
|
||||
deployment.setdefault('devices', []).append(device)
|
||||
kind = 'GPU' if gpu_devices.gpu_path(device['host_path']) else 'USB/Coral/NPU'
|
||||
details.append(f"{key}: {device['host_path']} ({kind})")
|
||||
skipped = [*new_keys, *(ENVIRONMENT_KEYS if environment else ())]
|
||||
filtered = b'\n'.join(line for line in config.splitlines()
|
||||
if not any(line.startswith(key.encode() + b': ') for key in new_keys)) + b'\n'
|
||||
if not any(line.startswith(key.encode() + b': ') for key in skipped))
|
||||
if environment:
|
||||
filtered += b''.join(b'\n' + line for line in record['observed']['config'].encode().splitlines()
|
||||
if any(line.startswith(key.encode() + b': ') for key in ENVIRONMENT_KEYS))
|
||||
filtered += b'\n'
|
||||
for key, value in transaction.external_changes(record, filtered).items():
|
||||
section, name = transaction.ADOPTABLE[key]
|
||||
target = deployment.setdefault(section, {}) if section else deployment
|
||||
|
||||
@@ -260,6 +260,9 @@ def update(vmid, acknowledge_external_data=False, proposal=None, keep_backup=Non
|
||||
transaction.apply(instances.ROOT, vmid, archive, operation, proposal=proposal,
|
||||
registry_digest=digest, acknowledge_external_data=acknowledge_external_data,
|
||||
keep_backup=file_storage)
|
||||
marker = os.environ.get('OCI_RECREATED_MARKER')
|
||||
if marker:
|
||||
Path(marker).touch()
|
||||
|
||||
|
||||
def main():
|
||||
|
||||
@@ -441,11 +441,12 @@ def manage_instance(project, ui, row, action=None, lifecycle_args=()):
|
||||
return False
|
||||
if adoption:
|
||||
summary = '\n'.join(adoption['details'])
|
||||
if not ui.review(
|
||||
f"{translate('These Proxmox resources were added outside ProxMenux:')}\n\n{summary}\n\n"
|
||||
+ translate('They will be added to oci-compose before continuing. Unsupported or changed resources are not imported.'),
|
||||
# Variables reordered in Proxmox change nothing that needs a decision.
|
||||
if summary and not ui.review(
|
||||
f"{translate('These changes were made in Proxmox outside ProxMenux:')}\n\n{summary}\n\n"
|
||||
+ translate('They will be saved in oci-compose before continuing.'),
|
||||
translate('Review external OCI changes'),
|
||||
question=translate('Include these resources in oci-compose?'), default=False):
|
||||
question=translate('Include these changes in oci-compose?'), default=False):
|
||||
return False
|
||||
try:
|
||||
record = reconcile.commit(instances.ROOT, row['vmid'], adoption)
|
||||
@@ -472,15 +473,24 @@ def manage_instance(project, ui, row, action=None, lifecycle_args=()):
|
||||
if '--acknowledge-external-data' not in command:
|
||||
command.append('--acknowledge-external-data')
|
||||
title = translate('Modify OCI') if proposal else translate('Update OCI')
|
||||
if proposal is None:
|
||||
completed = _run_lifecycle(command, title)
|
||||
else:
|
||||
# Saved environment/secrets must never be passed on the command line.
|
||||
with tempfile.NamedTemporaryFile(mode='w', suffix='.json') as file:
|
||||
json.dump(proposal, file)
|
||||
file.flush()
|
||||
completed = _run_lifecycle(command + ['--proposal', file.name], title)
|
||||
if completed and not getattr(ui, 'unattended', False):
|
||||
# The downloaded image is only worth mentioning when a new container
|
||||
# was created from it, not after a change applied to the running one.
|
||||
with tempfile.TemporaryDirectory() as folder:
|
||||
marker = Path(folder) / 'recreated'
|
||||
os.environ['OCI_RECREATED_MARKER'] = str(marker)
|
||||
try:
|
||||
if proposal is None:
|
||||
completed = _run_lifecycle(command, title)
|
||||
else:
|
||||
# Saved environment/secrets must never be passed on the command line.
|
||||
with tempfile.NamedTemporaryFile(mode='w', suffix='.json') as file:
|
||||
json.dump(proposal, file)
|
||||
file.flush()
|
||||
completed = _run_lifecycle(command + ['--proposal', file.name], title)
|
||||
finally:
|
||||
os.environ.pop('OCI_RECREATED_MARKER', None)
|
||||
recreated = marker.exists()
|
||||
if completed and recreated and not getattr(ui, 'unattended', False):
|
||||
images.offer_removal(ui, [row['vmid']])
|
||||
return completed
|
||||
action = ui.choose(translate('Interrupted operation'), [('status', translate('View status')),
|
||||
|
||||
@@ -155,5 +155,66 @@ class ProposalTests(unittest.TestCase):
|
||||
self.assertEqual(json.loads(history[0].read_text())['deployment']['mounts'], [])
|
||||
|
||||
|
||||
class EnvironmentAdoptionTests(unittest.TestCase):
|
||||
marker = '11111111-1111-1111-1111-111111111111'
|
||||
|
||||
def record(self, env):
|
||||
config = f'description: proxmenux-instance={self.marker}\nenv: {env}\n'.encode()
|
||||
return {
|
||||
'vmid': 200, 'status': 'installed', 'installation_id': self.marker,
|
||||
'deployment': {'mounts': [], 'devices': [], 'environment': [
|
||||
{'name': 'APP_PASSWORD', 'value': 'first-secret', 'sensitive': True}]},
|
||||
'observed': {'config': config.decode(), 'config_sha256': sha(config),
|
||||
'archive_path': '/unused.tar', 'resolved_registry_digest': 'sha256:test',
|
||||
'image': {'manifest_digest': 'sha256:test', 'defaults': {'Env': ['PATH=/bin']}}},
|
||||
}
|
||||
|
||||
def propose(self, record, env):
|
||||
config = f'description: proxmenux-instance={self.marker}\nenv: {env}\n'.encode()
|
||||
with (patch.object(reconcile.instances, 'ROOT', Path('/inert')),
|
||||
patch.object(reconcile.instances, 'identity', return_value=self.marker),
|
||||
patch.object(reconcile.instances, 'location', return_value=Path(__file__)),
|
||||
patch.object(reconcile.gpu_devices, 'capture', return_value={}),
|
||||
patch.object(reconcile.host_mounts, 'capture_sources', return_value={}),
|
||||
patch.object(reconcile.transaction, 'preflight')):
|
||||
return reconcile.propose(record, config)
|
||||
|
||||
def test_a_password_changed_in_proxmox_is_adopted_without_showing_it(self):
|
||||
record = self.record('PATH=/bin\0APP_PASSWORD=first-secret')
|
||||
proposal = self.propose(record, 'PATH=/bin\0APP_PASSWORD=my-own-secret\0TZ=Europe/Madrid')
|
||||
environment = proposal['candidate']['deployment']['environment']
|
||||
self.assertEqual(environment, [
|
||||
{'name': 'APP_PASSWORD', 'value': 'my-own-secret', 'sensitive': True},
|
||||
{'name': 'TZ', 'value': 'Europe/Madrid', 'sensitive': True}])
|
||||
self.assertEqual(proposal['details'], ['env APP_PASSWORD: value changed in Proxmox',
|
||||
'env TZ: added in Proxmox'])
|
||||
self.assertNotIn('my-own-secret', str(proposal['details']))
|
||||
self.assertEqual(record['deployment']['environment'][0]['value'], 'first-secret')
|
||||
|
||||
def test_reordered_variables_need_no_decision(self):
|
||||
record = self.record('PATH=/bin\0APP_PASSWORD=first-secret')
|
||||
proposal = self.propose(record, 'APP_PASSWORD=first-secret\0PATH=/bin')
|
||||
self.assertEqual(proposal['details'], [])
|
||||
|
||||
def test_removing_an_image_default_is_refused(self):
|
||||
record = self.record('PATH=/bin\0APP_PASSWORD=first-secret')
|
||||
with self.assertRaisesRegex(ValueError, 'env'):
|
||||
self.propose(record, 'APP_PASSWORD=first-secret')
|
||||
|
||||
def test_removing_a_saved_variable_removes_it_from_the_contract(self):
|
||||
record = self.record('PATH=/bin\0APP_PASSWORD=first-secret')
|
||||
proposal = self.propose(record, 'PATH=/bin')
|
||||
self.assertEqual(proposal['candidate']['deployment']['environment'], [])
|
||||
self.assertEqual(proposal['details'], ['env APP_PASSWORD: removed in Proxmox'])
|
||||
|
||||
def test_other_changes_next_to_the_variables_are_still_refused(self):
|
||||
record = self.record('PATH=/bin')
|
||||
config = (f'description: proxmenux-instance={self.marker}\nenv: PATH=/bin\0A=1\n'
|
||||
'net0: name=eth0,bridge=vmbr1\n').encode()
|
||||
with (patch.object(reconcile.instances, 'identity', return_value=self.marker),
|
||||
self.assertRaisesRegex(ValueError, 'net0')):
|
||||
reconcile.propose(record, config)
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
|
||||
@@ -169,12 +169,13 @@ class ModifyMenuTests(unittest.TestCase):
|
||||
|
||||
|
||||
class ModifyDispatchTests(unittest.TestCase):
|
||||
def call_manager(self, proposal, lifecycle):
|
||||
def call_manager(self, proposal, lifecycle, ui=None, offer=None):
|
||||
import oci_instances
|
||||
import oci_instance_reconcile
|
||||
saved = record()
|
||||
row = {'vmid': 115, 'reason': 'matched', 'stack': False, 'pending': False, 'status': 'installed'}
|
||||
with patch.object(management, 'carry_records', return_value={}), \
|
||||
with patch.object(management.images, 'offer_removal', offer or Mock()), \
|
||||
patch.object(management, 'carry_records', return_value={}), \
|
||||
patch.object(management, 'check_selected', return_value=row), \
|
||||
patch.object(management, '_work_backup_ready', return_value=True), \
|
||||
patch.object(oci_instances, 'read', return_value=saved), \
|
||||
@@ -182,7 +183,7 @@ class ModifyDispatchTests(unittest.TestCase):
|
||||
patch.object(oci_instance_reconcile, 'propose', return_value=None), \
|
||||
patch.object(recreation, 'edit_recreation', return_value=proposal), \
|
||||
patch.object(management, '_run_lifecycle', side_effect=lifecycle):
|
||||
return management.manage_instance(ROOT, Mock(), row, action='modify')
|
||||
return management.manage_instance(ROOT, ui or Mock(), row, action='modify')
|
||||
|
||||
def test_cancel_does_not_start_a_lifecycle_transaction(self):
|
||||
lifecycle = Mock()
|
||||
@@ -203,6 +204,20 @@ class ModifyDispatchTests(unittest.TestCase):
|
||||
self.assertTrue(self.call_manager(proposal, lifecycle))
|
||||
self.assertEqual(seen, [proposal])
|
||||
|
||||
def test_the_image_question_only_follows_a_recreated_container(self):
|
||||
proposal = {'operation': 'recreate', 'candidate': record(), 'base_config_sha256': 'fingerprint'}
|
||||
ui = Mock(unattended=False)
|
||||
for recreated in (False, True):
|
||||
with self.subTest(recreated=recreated):
|
||||
def lifecycle(command, title):
|
||||
if recreated:
|
||||
Path(management.os.environ['OCI_RECREATED_MARKER']).touch()
|
||||
return True
|
||||
offer = Mock()
|
||||
self.assertTrue(self.call_manager(proposal, lifecycle, ui, offer))
|
||||
self.assertEqual(offer.called, recreated)
|
||||
self.assertNotIn('OCI_RECREATED_MARKER', management.os.environ)
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user