Kaynağa Gözat

review fixes

Arthur 2 gün önce
ebeveyn
işleme
ef9770c897

+ 22 - 13
netbox/dcim/models/devices.py

@@ -1018,20 +1018,29 @@ class Device(
                 ).format(virtual_chassis=self.vc_master_for)
             })
 
-        # Prevent leaving a virtual chassis while interfaces form a LAG with another member
+        # Validate cross-chassis LAGs only when virtual chassis membership changes
         if self.pk:
-            lag_members = Interface.objects.filter(device=self, lag__isnull=False).exclude(lag__device=self)
-            lag_peers = Interface.objects.filter(lag__device=self).exclude(device=self)
-            if self.virtual_chassis:
-                lag_members = lag_members.exclude(lag__device__virtual_chassis=self.virtual_chassis)
-                lag_peers = lag_peers.exclude(device__virtual_chassis=self.virtual_chassis)
-            if lag_members.exists() or lag_peers.exists():
-                raise ValidationError({
-                    'virtual_chassis': _(
-                        'Device cannot be removed from its virtual chassis because one or more of its interfaces '
-                        'form a LAG with another member device.'
-                    )
-                })
+            stored_vc_id = Device.objects.filter(pk=self.pk).values_list('virtual_chassis_id', flat=True).first()
+            if self.virtual_chassis_id != stored_vc_id:
+                self.clean_vc_lag_membership()
+
+    def clean_vc_lag_membership(self):
+        """
+        Raise ValidationError if any interface would form a LAG with a device outside this device's virtual chassis.
+        """
+        lag_members = Interface.objects.filter(device=self, lag__isnull=False).exclude(lag__device=self)
+        lag_peers = Interface.objects.filter(lag__device=self).exclude(device=self)
+        if self.virtual_chassis_id:
+            lag_members = lag_members.exclude(lag__device__virtual_chassis=self.virtual_chassis_id)
+            lag_peers = lag_peers.exclude(device__virtual_chassis=self.virtual_chassis_id)
+        interfaces = [*lag_members.select_related('lag__device'), *lag_peers.select_related('device', 'lag')]
+        if interfaces:
+            raise ValidationError({
+                'virtual_chassis': _(
+                    'Virtual chassis membership cannot be changed while interfaces form a LAG with another device '
+                    'outside the virtual chassis: {interfaces}'
+                ).format(interfaces=', '.join(f'{i.device}:{i} ({i.lag.device}:{i.lag})' for i in interfaces))
+            })
 
     def _check_duplicate_component_names(self, components):
         """

+ 21 - 0
netbox/dcim/tests/test_models.py

@@ -3727,6 +3727,27 @@ class VirtualChassisTestCase(TestCase):
             device1.full_clean()
         self.assertIn('virtual_chassis', cm.exception.error_dict)
 
+    def test_unrelated_edit_allowed_with_existing_cross_chassis_lag(self):
+        """
+        Pre-existing cross-chassis LAG data must not block edits that leave VC membership unchanged.
+        """
+        device1 = Device.objects.get(name='TestDevice1')
+        device2 = Device.objects.get(name='TestDevice2')
+
+        vc = VirtualChassis.objects.create(name='Test VC', master=device1)
+        Device.objects.filter(pk=device1.pk).update(virtual_chassis=vc, vc_position=1)
+        lag = Interface.objects.create(device=device1, name='lag0', type=InterfaceTypeChoices.TYPE_LAG)
+        Interface.objects.create(
+            device=device2, name='eth0', type=InterfaceTypeChoices.TYPE_1GE_FIXED, lag=lag
+        )
+
+        device1.refresh_from_db()
+        device2.refresh_from_db()
+        device1.description = 'changed'
+        device1.full_clean()
+        device2.description = 'changed'
+        device2.full_clean()
+
     def test_member_removal_allowed_with_local_lag(self):
         """
         A device may leave a VirtualChassis when its LAG members are all local.

+ 6 - 0
netbox/dcim/tests/test_views.py

@@ -5620,8 +5620,14 @@ class VirtualChassisTestCase(ViewTestCases.PrimaryObjectViewTestCase):
 
     def test_remove_member(self):
         self.add_permissions('dcim.change_device')
+        master = Device.objects.get(name='Device 1')
         member = Device.objects.get(name='Device 2')
 
+        # A primary IP on another member's interface must not block removal
+        interface = Interface.objects.create(device=master, name='eth0', type=InterfaceTypeChoices.TYPE_1GE_FIXED)
+        ip = IPAddress.objects.create(address='192.0.2.1/24', assigned_object=interface)
+        Device.objects.filter(pk=member.pk).update(primary_ip4=ip)
+
         url = reverse('dcim:virtualchassis_remove_member', kwargs={'pk': member.pk})
         response = self.client.post(url, {'confirm': True})
         self.assertEqual(response.status_code, 302)

+ 1 - 1
netbox/dcim/views.py

@@ -5362,7 +5362,7 @@ class VirtualChassisRemoveMemberView(ObjectPermissionRequiredMixin, GetReturnURL
             device.vc_position = None
             device.vc_priority = None
             try:
-                device.full_clean()
+                device.clean_vc_lag_membership()
             except ValidationError as e:
                 for message in e.messages:
                     messages.error(request, message)