diff --git a/oci/remote/oci_gpu_devices.py b/oci/remote/oci_gpu_devices.py index 4ed57c60..3c2a1b4b 100644 --- a/oci/remote/oci_gpu_devices.py +++ b/oci/remote/oci_gpu_devices.py @@ -110,18 +110,18 @@ def container_gid(device, host_value): def _permission_fields(fields): + """Everything but the GID, which the user may change in Proxmox.""" return (fields.get('path'), int(fields.get('mode', '0660'), 8), - int(fields.get('uid', 0)), int(fields.get('gid', 0)), - fields.get('deny-write', '0')) + int(fields.get('uid', 0)), fields.get('deny-write', '0')) def preserve_recorded_container_gids(record, candidate, config): - """Carry forward only device GIDs already captured in the saved config. + """Keep the GID each device has inside the LXC, including one changed in + Proxmox since the last operation, like the other Proxmox changes. - This is not a host-to-container GID translation. The saved config is the - baseline for the GID Proxmox previously assigned inside the LXC. Any - unrecorded change is rejected, and the host device is still verified - independently by ``verify_baseline``. + This is not a host-to-container GID translation: the configured GID is the + group Proxmox gives the node inside the container. Other permission changes + are still refused, and the host device is verified by ``verify_baseline``. """ saved_config = record.get('observed', {}).get('config') if not isinstance(saved_config, str) or not saved_config: @@ -137,13 +137,10 @@ def preserve_recorded_container_gids(record, candidate, config): old_snapshots = planned(record.get('deployment', {})) for device in original_devices: path = device.get('host_path') - if path not in saved or path not in old_snapshots: + if path not in current or path not in old_snapshots: continue - observed_gid = int(saved[path].get('gid', 0)) - expected_gid = container_gid(device, old_snapshots[path]) - if observed_gid != expected_gid and 'container_gid' in device: - raise ValueError(translate('The recorded container device GID does not match its configuration')) - if observed_gid != expected_gid: + observed_gid = int(current[path].get('gid', 0)) + if observed_gid != container_gid(device, old_snapshots[path]): device['container_gid'] = observed_gid for desired in candidate.get('deployment', {}).get('devices', []): diff --git a/oci/tests/test_gpu_device_gid.py b/oci/tests/test_gpu_device_gid.py index d6f4d243..e5dbb4eb 100644 --- a/oci/tests/test_gpu_device_gid.py +++ b/oci/tests/test_gpu_device_gid.py @@ -39,11 +39,25 @@ class ContainerDeviceGidTests(unittest.TestCase): self.assertEqual(devices.check(CURRENT_CONFIG, candidate['deployment'])['/dev/dri/renderD128'], HOST) @patch.object(devices, 'snapshot', return_value=HOST) - def test_unrecorded_guest_gid_change_is_rejected(self, _snapshot): + def test_a_guest_gid_changed_in_proxmox_is_kept(self, _snapshot): + record = {'deployment': {'devices': [dict(DEPLOYMENT['devices'][0], container_gid=44)]}, + 'observed': {'config': SAVED_CONFIG}} + candidate = {'deployment': {'devices': [dict(DEPLOYMENT['devices'][0])]}} + # Written from the Proxmox interface, without `path=`. + changed = b'dev0: /dev/dri/renderD128,mode=0660,deny-write=0,gid=45\n' + + devices.preserve_recorded_container_gids(record, candidate, changed) + + self.assertEqual(record['deployment']['devices'][0]['container_gid'], 45) + self.assertEqual(candidate['deployment']['devices'][0]['container_gid'], 45) + self.assertEqual(devices.check(changed, candidate['deployment'])['/dev/dri/renderD128'], HOST) + + @patch.object(devices, 'snapshot', return_value=HOST) + def test_other_permission_changes_are_still_refused(self, _snapshot): record = {'deployment': {'devices': [dict(DEPLOYMENT['devices'][0])]}, 'observed': {'config': SAVED_CONFIG}} candidate = {'deployment': {'devices': [dict(DEPLOYMENT['devices'][0])]}} - changed = CURRENT_CONFIG.replace(b'gid=44', b'gid=45') + changed = CURRENT_CONFIG.replace(b'mode=0660', b'mode=0666') with self.assertRaisesRegex(ValueError, 'device settings changed'): devices.preserve_recorded_container_gids(record, candidate, changed)