Avoid unnecessary probes while opening OCI menus

This commit is contained in:
Jonatan
2026-10-17 18:48:33 +02:00
parent 8a774d05c7
commit 1e97f732ac
5 changed files with 137 additions and 17 deletions
@@ -56,7 +56,8 @@ class RemovalWordings(unittest.TestCase):
state = ModuleType('oci_installation_state')
state.parse_config = lambda raw: {}
scope = {'sys': SimpleNamespace(path=[]), 'source_text': lambda value: value or '',
'translate': translated or (lambda value: value), 're': re}
'translate': translated or (lambda value: value), 're': re,
'_selected_configuration': remover.guest_config, 'subprocess': subprocess}
with patch.dict(sys.modules, {'oci_instances': instances, 'oci_remove': remover,
'oci_installation_state': state}):
return extract(MENU, '_removal_summary', scope)(Path('/inert'), 101)
@@ -87,7 +88,8 @@ class RemovalWordings(unittest.TestCase):
with patch.dict(sys.modules, {'oci_instances': instances, 'oci_remove': remover,
'oci_installation_state': state}):
scope = {'sys': SimpleNamespace(path=[]), 'source_text': lambda value: value or '',
'translate': lambda value: value, 're': re}
'translate': lambda value: value, 're': re,
'_selected_configuration': remover.guest_config, 'subprocess': subprocess}
preview = extract(MENU, '_removal_summary', scope)(Path('/inert'), 102)
self.assertIn(WHOLE, preview)
+5 -1
View File
@@ -35,4 +35,8 @@ jobs:
- name: Run OCI Modify latency regressions
env:
PYTHONDONTWRITEBYTECODE: '1'
run: python3 -m unittest discover -s oci/tests -p test_modify_latency.py -v
run: |
python3 -m unittest discover -s oci/tests -p test_modify_latency.py -v
python3 -m unittest discover -s oci/tests -p test_menu_performance.py -v
python3 -m unittest discover -s oci/tests -p test_work_backup.py -v
python3 -m unittest discover -s oci/tests -p test_recreation.py -v
+10 -6
View File
@@ -192,21 +192,25 @@ def application(root, vmid):
return primary_id, members or [vmid]
def status(root, vmid):
def status(root, vmid, only_needed=False):
"""What the menu needs to know before an operation: whether the backup
fits on the system disk, the storage chosen earlier and the ones that
have room."""
have room. The menu can skip unused alternatives once a destination fits;
the default retains the complete status used by the maintenance CLI."""
primary_id, vmids = application(root, vmid)
record = instances.read(root, primary_id)
default = instances.location(root, primary_id).parent
needed, free = required(vmids), available(default)
saved = (record.get('deployment') or {}).get(KEY)
path = storage_path(saved) if saved else None
fits = free >= needed
path = storage_path(saved) if saved and not (only_needed and fits) else None
saved_free = available(path) if path is not None and path.is_dir() else None
return {'needed': needed, 'free': free, 'fits': free >= needed, 'saved': saved,
'saved_fits': saved_free is not None and saved_free >= needed,
saved_fits = saved_free is not None and saved_free >= needed
candidates = [] if only_needed and (fits or saved_fits) else file_storages()
return {'needed': needed, 'free': free, 'fits': fits, 'saved': saved,
'saved_fits': saved_fits,
'candidates': [{'storage': item['storage'], 'free': item['free']}
for item in file_storages() if item['free'] >= needed]}
for item in candidates if item['free'] >= needed]}
def choose(root, vmid, storage):
+9 -8
View File
@@ -332,7 +332,7 @@ def _work_backup_ready(project, ui, vmid):
import oci_instances as instances
import oci_work_backup
try:
state = oci_work_backup.status(instances.ROOT, vmid)
state = oci_work_backup.status(instances.ROOT, vmid, only_needed=True)
except (OSError, ValueError, KeyError, RuntimeError, subprocess.SubprocessError):
# What cannot be measured here is told by the engine when it stops.
return True
@@ -416,8 +416,6 @@ def manage_instance(project, ui, row, action=None, lifecycle_args=()):
return _toggle_watchdog(project, ui, row['vmid'], watched)
if action == 'remove':
return _remove(project, ui, row['vmid'])
if action != 'modify' and not _work_backup_ready(project, ui, row['vmid']):
return False
record = instances.read(instances.ROOT, row['vmid'])
import oci_instance_reconcile as reconcile
try:
@@ -462,13 +460,13 @@ def manage_instance(project, ui, row, action=None, lifecycle_args=()):
wizard.close()
if not approved:
return False
# Opening or cancelling the editor needs no disk/storage probes.
# Keep the existing backup checks after the user confirms edits.
if not _work_backup_ready(project, ui, row['vmid']):
return False
elif not ui.review(translate('If the image channel has a new version, the CT is stopped, backed up and verified, and replaced with the new image to update the container. If the update fails after the container was changed, the backup is restored. An update that stops halfway is recovered from the OCI management menu.'),
translate('Update OCI'), question=translate('Update now?'), default=True):
return False
# Opening/cancelling a confirmation needs no disk or storage probes.
# Both accepted operations retain this check before the backend starts.
if not _work_backup_ready(project, ui, row['vmid']):
return False
command = [sys.executable, str(project / 'remote/oci_update_current.py'), str(row['vmid']),
*lifecycle_args]
# Host directories are configured by the user, who knows they are
@@ -518,7 +516,10 @@ def _removal_summary(project, vmid):
record = instances.read(instances.ROOT, member)
except (OSError, ValueError, KeyError):
record = {}
config = oci_remove.guest_config(member)
try:
config = _selected_configuration(member)
except (OSError, RuntimeError, subprocess.SubprocessError):
config = None
cfg = parse_config(config) if config else {}
name = (cfg.get('hostname') or (record.get('stack_member') or {}).get('name')
or record.get('deployment', {}).get('hostname') or '')
+109
View File
@@ -0,0 +1,109 @@
"""Menu speedups avoid unused probes, retaining confirmation and capacity checks."""
from pathlib import Path
import sys
import tempfile
import unittest
from unittest.mock import Mock,patch
ROOT=Path(__file__).resolve().parents[1]
sys.path[:0]=[str(ROOT/'src'),str(ROOT/'remote')]
from proxmenux_oci import management as menu
import oci_instances as instances
import oci_instance_reconcile as reconcile
import oci_remove as remover
import oci_work_backup as backup
class UpdateEntryTests(unittest.TestCase):
def run_update(self,approved,ready):
events=[]
record={'status':'installed','vmid':110,'deployment':{},'template':{}}
row={'vmid':110,'reason':'matched','stack':False,'pending':False,'status':'installed'}
ui=Mock();ui.unattended=True
ui.review.side_effect=lambda *a,**kw:events.append('review') or approved
with patch.object(menu,'carry_records',return_value={}) as carry, \
patch.object(menu,'check_selected',return_value=row), \
patch.object(menu,'_selected_configuration',return_value=b'current config'), \
patch.object(instances,'read',return_value=record),patch.object(reconcile,'propose',return_value=None), \
patch.object(menu,'_work_backup_ready',side_effect=lambda *a:events.append('backup') or ready), \
patch.object(menu,'_run_lifecycle',side_effect=lambda *a:events.append('lifecycle') or True):
result=menu.manage_instance(ROOT,ui,row,action='update')
carry.assert_called_once_with(ROOT,vmids=[110],verify=True)
return result,events
def test_declining_update_does_not_probe_disks_or_storages(self):
result,events=self.run_update(False,False)
self.assertFalse(result);self.assertEqual(events,['review'])
def test_accepted_update_checks_backup_space_before_starting(self):
result,events=self.run_update(True,True)
self.assertTrue(result);self.assertEqual(events,['review','backup','lifecycle'])
def test_insufficient_space_still_prevents_the_update(self):
result,events=self.run_update(True,False)
self.assertFalse(result);self.assertEqual(events,['review','backup'])
class BackupDestinationTests(unittest.TestCase):
def status(self,local_free,saved_free=None,only_needed=True):
with tempfile.TemporaryDirectory() as directory:
root=Path(directory);saved= root/'nas';saved.mkdir()
record={'deployment':{backup.KEY:'nas'}}
with patch.object(backup,'application',return_value=(110,[110,111])), \
patch.object(instances,'read',return_value=record), \
patch.object(backup,'required',return_value=100) as required, \
patch.object(backup,'available',side_effect=lambda p:saved_free if Path(p)==saved else local_free), \
patch.object(backup,'storage_path',return_value=saved if saved_free is not None else None) as path, \
patch.object(backup,'file_storages',return_value=[{'storage':'enough','free':200},{'storage':'small','free':50}]) as alternatives:
result=backup.status(root,110,only_needed=only_needed)
required.assert_called_once_with([110,111])
return result,path.call_count,alternatives.call_count
def test_local_room_avoids_saved_storage_and_alternative_queries(self):
result,saved,alternatives=self.status(200,saved_free=1000)
self.assertTrue(result['fits']);self.assertEqual((saved,alternatives),(0,0))
def test_saved_destination_that_fits_avoids_alternative_queries(self):
result,saved,alternatives=self.status(50,saved_free=200)
self.assertTrue(result['saved_fits']);self.assertEqual((saved,alternatives),(1,0))
def test_when_neither_fits_all_available_destinations_are_checked(self):
result,saved,alternatives=self.status(50,saved_free=40)
self.assertFalse(result['fits']);self.assertFalse(result['saved_fits'])
self.assertEqual(result['candidates'],[{'storage':'enough','free':200}])
self.assertEqual(alternatives,1)
def test_missing_saved_storage_still_offers_alternatives(self):
result,saved,alternatives=self.status(50)
self.assertEqual(result['candidates'],[{'storage':'enough','free':200}])
self.assertEqual(alternatives,1)
def test_complete_status_keeps_legacy_fields_and_all_candidates(self):
result,saved,alternatives=self.status(200,saved_free=300,only_needed=False)
self.assertTrue(result['fits']);self.assertTrue(result['saved_fits'])
self.assertEqual((saved,alternatives),(1,1))
self.assertEqual(result['candidates'],[{'storage':'enough','free':200}])
class RemovalPreviewTests(unittest.TestCase):
def preview(self,reader):
record={'vmid':110,'deployment':{},'template':{'catalog_ui':{'title':'Example'}}}
with patch.object(remover,'members_of',return_value=(110,record,[110])), \
patch.object(instances,'read',return_value=record), \
patch.object(menu,'_selected_configuration',side_effect=reader), \
patch.object(remover,'guest_config',side_effect=AssertionError('Duplicate pct process')), \
patch.object(remover,'host_directories',return_value=[]), \
patch.object(remover,'private_bridge',return_value=None):
return menu._removal_summary(ROOT,110)
def test_preview_reuses_verified_config_and_keeps_volume_and_host_path_details(self):
config=b'hostname: example\nrootfs: local:subvol-110-disk-0,size=8G\nmp0: /mnt/media,mp=/media,backup=0\n'
text=self.preview(lambda v:config)
self.assertIn('8G',text);self.assertIn('/mnt/media',text)
self.assertIn('not targeted for removal',text)
def test_missing_guest_can_still_be_reviewed_without_a_second_probe(self):
text=self.preview(lambda v:(_ for _ in ()).throw(RuntimeError('guest missing')))
self.assertIn('CT 110',text);self.assertIn('rootfs -',text)
if __name__=='__main__':unittest.main()