diff --git a/oci/remote/oci_instance_transaction.py b/oci/remote/oci_instance_transaction.py index 0e07d7c5..fa01b2dc 100644 --- a/oci/remote/oci_instance_transaction.py +++ b/oci/remote/oci_instance_transaction.py @@ -624,7 +624,11 @@ def without_public_routes(config, vmid): def refuse_high_availability(vmid): """Recreating a container that HA manages would race the HA manager, which restarts or relocates it in the middle of the replacement.""" - resources = json.loads(run('pvesh', 'get', '/cluster/ha/resources', '--output-format', 'json')) + try: + answer = run('pvesh', 'get', '/cluster/ha/resources', '--output-format', 'json') + except FileNotFoundError: + return # Not a Proxmox VE node: there is no HA manager to race. + resources = json.loads(answer) if any(r.get('sid') == f'ct:{vmid}' for r in resources): raise ValueError(translate( 'This container is managed by high availability. Remove ct:{vmid} from HA (Datacenter → HA, ' @@ -641,6 +645,9 @@ def preflight(record, candidate, config, coordinated=None, in_place=False, basel 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')) + if not in_place and not coordinated: + # A stack checks its members itself; an in-place edit keeps the container. + refuse_high_availability(record['vmid']) 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 @@ -1213,7 +1220,8 @@ def apply(root, vmid, archive, operation, proposal=None, registry_digest=None, i runtime_settings.file_binds(before)) original_gpu = gpu_devices.planned(record['deployment']) desired_gpu = gpu_devices.planned(candidate['deployment']) - refuse_high_availability(vmid) + if coordinated: + refuse_high_availability(vmid) archive = archive.resolve(strict=True) check_archive(archive) image = image_from_archive(str(archive)) diff --git a/oci/remote/oci_update_current.py b/oci/remote/oci_update_current.py index b0278615..e8163fb9 100644 --- a/oci/remote/oci_update_current.py +++ b/oci/remote/oci_update_current.py @@ -232,9 +232,8 @@ def update(vmid, acknowledge_external_data=False, proposal=None, keep_backup=Non # An unsupported native edit still uses the existing installer. Decide # the route before changing anything; actual validation/API failures # must not silently switch to a destructive fallback. + # Also refuses a container that HA manages, before the image is downloaded. transaction.preflight(record, desired, config) - # Refused before the image is downloaded, not after. - transaction.refuse_high_availability(vmid) msg_ok(translate('Container checked')) current = record['observed']['image'] if operation == 'update' else None check = lambda: work_backup.locate( diff --git a/oci/tests/test_update_high_availability.py b/oci/tests/test_update_high_availability.py index 356dfa6b..e2cfdbfd 100644 --- a/oci/tests/test_update_high_availability.py +++ b/oci/tests/test_update_high_availability.py @@ -26,6 +26,28 @@ class HighAvailabilityTests(unittest.TestCase): with ha_resources(200): transaction.refuse_high_availability(101) + def test_a_host_without_pvesh_has_no_ha_to_refuse(self): + with patch.object(transaction, 'run', side_effect=FileNotFoundError('pvesh')): + transaction.refuse_high_availability(101) + + def test_the_preflight_of_a_replacement_refuses_it(self): + record = {'vmid': 101, 'installation_id': 'install-1', 'deployment': {}} + with patch.object(transaction.instances, 'identity', return_value='install-1'), ha_resources(101), \ + self.assertRaises(ValueError) as refused: + transaction.preflight(record, {'deployment': {}}, b'') + self.assertIn('ct:101', str(refused.exception)) + + def test_an_in_place_edit_and_a_stack_member_are_not_refused_here(self): + record = {'vmid': 101, 'installation_id': 'install-1', 'deployment': {}} + refuse = Mock() + with patch.object(transaction.instances, 'identity', return_value='install-1'), \ + patch.object(transaction, 'refuse_high_availability', refuse), \ + patch.object(transaction, 'external_changes', side_effect=RuntimeError('past the check')): + for options in ({'in_place': True}, {'coordinated': {'id': 'stack'}}): + with self.subTest(options=options), self.assertRaises(RuntimeError): + transaction.preflight(record, {'deployment': {}}, b'', **options) + refuse.assert_not_called() + def test_an_update_stops_before_downloading_the_image(self): record = {'status': 'installed', 'observed': {'image': {}}} resolve = Mock() @@ -35,11 +57,11 @@ class HighAvailabilityTests(unittest.TestCase): stack.enter_context(patch.object(update.instances, 'command', return_value=b'')) stack.enter_context(patch.object(update.transaction, 'candidate_contract', return_value={})) stack.enter_context(patch.object(update.transaction, 'external_changes', return_value={})) - stack.enter_context(patch.object(update.transaction, 'preflight')) + stack.enter_context(patch.object(update.transaction, 'preflight', + side_effect=ValueError('managed by high availability'))) stack.enter_context(patch.object(update, 'resolve_archive', resolve)) stack.enter_context(patch.object(update, 'msg_info')) stack.enter_context(patch.object(update, 'msg_ok')) - stack.enter_context(ha_resources(101)) with self.assertRaises(ValueError): update.update(101) resolve.assert_not_called()