fix(oci): keep a device GID changed in Proxmox since the last operation

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)),
+            int(fields.get('uid', 0)), fields.get('deny-write', '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
+    This is not a host-to-container GID translation: the configured GID is the
-    baseline for the GID Proxmox previously assigned inside the LXC. Any
+    group Proxmox gives the node inside the container. Other permission changes
-    unrecorded change is rejected, and the host device is still verified
+    are still refused, and the host device is verified by ``verify_baseline``.
-    independently 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))
+        observed_gid = int(current[path].get('gid', 0))
-        expected_gid = container_gid(device, old_snapshots[path])
+        if observed_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:
             device['container_gid'] = observed_gid
 
         for desired in candidate.get('deployment', {}).get('devices', []):

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)