Merge e864528795 into 2f79d987a8
This commit is contained in:
Arthur Hanson
GitHub
commit
4f2ac2d03d
|
|
@ -74,13 +74,29 @@ class CableBundle(PrimaryModel):
|
|||
# Cables
|
||||
#
|
||||
|
||||
class CableQuerySet(RestrictedQuerySet):
|
||||
|
||||
def delete(self):
|
||||
# Track these Cables as being deleted for the duration, as Cable.delete() does for a single
|
||||
# instance: a queryset delete never calls it. Between them the two cover every deletion, as a
|
||||
# Cable is never itself cascade-deleted (nothing points at it with on_delete=CASCADE).
|
||||
pks = list(self.values_list('pk', flat=True))
|
||||
for pk in pks:
|
||||
Cable._track_deletion(pk)
|
||||
try:
|
||||
return super().delete()
|
||||
finally:
|
||||
for pk in pks:
|
||||
Cable._untrack_deletion(pk)
|
||||
|
||||
|
||||
class Cable(PrimaryModel):
|
||||
"""
|
||||
A physical connection between two endpoints.
|
||||
"""
|
||||
# Per-thread tracking of Cable PKs currently in delete(); referenced by
|
||||
# dcim.signals.nullify_connected_endpoints to skip per-CableTermination
|
||||
# cable path retracing during cascade (retrace_cable_paths handles it once).
|
||||
# Per-thread tracking of Cable PKs currently being deleted; referenced by
|
||||
# dcim.signals.nullify_connected_endpoints to record the disconnect on each terminating object and
|
||||
# to skip per-CableTermination path retracing during the cascade (retrace_cable_paths does it once).
|
||||
_deletion_tracking = threading.local()
|
||||
|
||||
type = models.CharField(
|
||||
|
|
@ -150,6 +166,8 @@ class Cable(PrimaryModel):
|
|||
|
||||
clone_fields = ('tenant', 'type', 'profile', 'bundle')
|
||||
|
||||
objects = CableQuerySet.as_manager()
|
||||
|
||||
class Meta:
|
||||
ordering = ('pk',)
|
||||
verbose_name = _('cable')
|
||||
|
|
@ -385,20 +403,32 @@ class Cable(PrimaryModel):
|
|||
self._terminations_modified = False
|
||||
|
||||
def delete(self, *args, **kwargs):
|
||||
# Track this Cable as being deleted so the post_delete signal handler
|
||||
# for cascaded CableTerminations can skip redundant path retracing;
|
||||
# retrace_cable_paths() will retrace each affected path once after the
|
||||
# Cable itself is deleted. Cache the PK locally because super().delete()
|
||||
# clears self.pk before the finally block runs. The tracking set lives
|
||||
# on a threading.local() to isolate concurrent deletions across threads.
|
||||
if not hasattr(Cable._deletion_tracking, 'pks'):
|
||||
Cable._deletion_tracking.pks = set()
|
||||
# Cache the PK locally because super().delete() clears self.pk before the finally block runs. The
|
||||
# tracking itself is done by the pre_delete/post_delete receivers in dcim.signals, which also cover a
|
||||
# queryset delete; this pairing just guarantees the PK is discarded if the delete raises.
|
||||
pk = self.pk
|
||||
Cable._deletion_tracking.pks.add(pk)
|
||||
Cable._track_deletion(pk)
|
||||
try:
|
||||
return super().delete(*args, **kwargs)
|
||||
finally:
|
||||
Cable._deletion_tracking.pks.discard(pk)
|
||||
Cable._untrack_deletion(pk)
|
||||
|
||||
@classmethod
|
||||
def _track_deletion(cls, pk):
|
||||
"""
|
||||
Track a Cable as being deleted, so that the post_delete handler for its cascaded CableTerminations can
|
||||
record the disconnect on each terminating object and skip redundant path retracing (retrace_cable_paths()
|
||||
retraces each affected path once, after the Cable itself is deleted). The tracking set lives on a
|
||||
threading.local() to isolate concurrent deletions across threads.
|
||||
"""
|
||||
if not hasattr(cls._deletion_tracking, 'pks'):
|
||||
cls._deletion_tracking.pks = set()
|
||||
cls._deletion_tracking.pks.add(pk)
|
||||
|
||||
@classmethod
|
||||
def _untrack_deletion(cls, pk):
|
||||
if hasattr(cls._deletion_tracking, 'pks'):
|
||||
cls._deletion_tracking.pks.discard(pk)
|
||||
|
||||
@classmethod
|
||||
def _is_being_deleted(cls, pk):
|
||||
|
|
|
|||
|
|
@ -323,21 +323,60 @@ def nullify_connected_endpoints(instance, **kwargs):
|
|||
Disassociate the Cable from the termination object, and retrace any affected CablePaths.
|
||||
"""
|
||||
model = instance.termination_type.model_class()
|
||||
model.objects.filter(pk=instance.termination_id).update(
|
||||
cable=None,
|
||||
cable_end=None,
|
||||
cable_connector=None,
|
||||
cable_positions=None,
|
||||
)
|
||||
|
||||
# If the removed termination was a channelized interface, also clear the cable attributes mirrored onto its channel
|
||||
# subinterfaces. This must happen before the retrace below so that each channel's (now dead) path is torn down
|
||||
# rather than rebuilt from a stale cable reference.
|
||||
if model is Interface:
|
||||
Interface.objects.filter(parent_id=instance.termination_id, channel_id__isnull=False).update(
|
||||
cable=None, cable_end='', cable_connector=None, cable_positions=None
|
||||
# Deleting a Cable deletes its terminations in bulk, bypassing CableTermination.delete() and the
|
||||
# change-logged clear it performs on the terminating object; do the same here so the disconnect is
|
||||
# recorded. `cable` is a SET_NULL FK which the deletion collector has already nulled by now, so the
|
||||
# pre-delete values are restored before snapshotting, or the record would show no change. They are
|
||||
# restored field by field rather than via set_cable_termination(), whose Interface override would
|
||||
# propagate the cable back onto the channel subinterfaces we are about to clear.
|
||||
termination = None
|
||||
if Cable._is_being_deleted(instance.cable_id):
|
||||
# The change record serializes the object twice (before and after), and ComponentModel.save()
|
||||
# re-caches its denormalized references off the parent device: fetch both up front so neither
|
||||
# costs a round trip per terminating object.
|
||||
queryset = model.objects.filter(pk=instance.termination_id).prefetch_related('tags')
|
||||
if hasattr(model, 'device'):
|
||||
queryset = queryset.select_related('device__site', 'device__location', 'device__rack')
|
||||
termination = queryset.first()
|
||||
|
||||
if termination is not None:
|
||||
termination.cable_id = instance.cable_id
|
||||
termination.cable_end = instance.cable_end
|
||||
termination.cable_connector = instance.connector
|
||||
termination.cable_positions = instance.positions
|
||||
termination.snapshot()
|
||||
# clear_cable_termination() also clears the mirrored attributes on any channel subinterfaces
|
||||
termination.clear_cable_termination(instance)
|
||||
update_fields = ['cable', 'cable_end', 'cable_connector', 'cable_positions', 'last_updated']
|
||||
|
||||
# retrace_cable_paths() tears down the originating path once the Cable itself is deleted, clearing
|
||||
# _path outside the changelog. Clear it here so the recorded state doesn't outlive the path.
|
||||
if isinstance(termination, PathEndpoint):
|
||||
termination._path = None
|
||||
update_fields.append('_path')
|
||||
|
||||
# A narrow write: this row was read mid-cascade, and its full save() would pull in unrelated work
|
||||
termination.save(update_fields=update_fields)
|
||||
else:
|
||||
# Already recorded by CableTermination.delete(), or the terminating object is going away too.
|
||||
model.objects.filter(pk=instance.termination_id).update(
|
||||
cable=None,
|
||||
cable_end=None,
|
||||
cable_connector=None,
|
||||
cable_positions=None,
|
||||
)
|
||||
|
||||
# If the removed termination was a channelized interface, also clear the cable attributes mirrored onto
|
||||
# its channel subinterfaces. This must happen before the retrace below so that each channel's (now dead)
|
||||
# path is torn down rather than rebuilt from a stale cable reference. These writes are deliberately not
|
||||
# change-logged: propagate_channel_cables() doesn't log the mirrored attributes when it sets them
|
||||
# either, so a channel subinterface has no recorded cable state for this to contradict.
|
||||
if model is Interface:
|
||||
Interface.objects.filter(parent_id=instance.termination_id, channel_id__isnull=False).update(
|
||||
cable=None, cable_end='', cable_connector=None, cable_positions=None
|
||||
)
|
||||
|
||||
# If the parent Cable is being deleted in this same operation, skip the
|
||||
# per-termination retrace; retrace_cable_paths() will retrace each affected
|
||||
# path once after the Cable is deleted.
|
||||
|
|
|
|||
|
|
@ -1,21 +1,25 @@
|
|||
import uuid
|
||||
from decimal import Decimal
|
||||
|
||||
from django.core.exceptions import ValidationError
|
||||
from django.db import connection
|
||||
from django.db.models import ProtectedError
|
||||
from django.db.models.signals import post_save
|
||||
from django.test import TestCase, tag
|
||||
from django.test import RequestFactory, TestCase, tag
|
||||
from django.test.utils import CaptureQueriesContext
|
||||
|
||||
from circuits.models import *
|
||||
from core.models import ObjectType
|
||||
from core.choices import ObjectChangeActionChoices
|
||||
from core.models import ObjectChange, ObjectType
|
||||
from dcim.choices import *
|
||||
from dcim.models import *
|
||||
from extras.events import serialize_for_event
|
||||
from extras.models import CustomField
|
||||
from ipam.models import Prefix
|
||||
from netbox.choices import DiameterUnitChoices, FlowRateUnitChoices, WeightUnitChoices
|
||||
from netbox.context_managers import event_tracking
|
||||
from tenancy.models import Tenant
|
||||
from users.models import User
|
||||
from utilities.data import drange
|
||||
from virtualization.models import Cluster, ClusterType
|
||||
|
||||
|
|
@ -2922,6 +2926,160 @@ class CableTerminationTestCase(TestCase):
|
|||
cable_termination.cache_related_objects()
|
||||
|
||||
|
||||
class CableDisconnectChangeLoggingTestCase(TestCase):
|
||||
"""
|
||||
Deleting a Cable must change-log the disconnect on each terminating object. Its CableTerminations are
|
||||
deleted by the cascade, so CableTermination.delete() -- which records the disconnect when a termination
|
||||
is removed from a Cable -- never runs.
|
||||
"""
|
||||
|
||||
@classmethod
|
||||
def setUpTestData(cls):
|
||||
site = Site.objects.create(name='Test Site 1', slug='test-site-1')
|
||||
manufacturer = Manufacturer.objects.create(name='Test Manufacturer 1', slug='test-manufacturer-1')
|
||||
device_type = DeviceType.objects.create(
|
||||
manufacturer=manufacturer, model='Test Device Type 1', slug='test-device-type-1'
|
||||
)
|
||||
role = DeviceRole.objects.create(name='Test Device Role 1', slug='test-device-role-1')
|
||||
cls.device1 = Device.objects.create(
|
||||
name='Device 1', site=site, device_type=device_type, role=role
|
||||
)
|
||||
cls.device2 = Device.objects.create(
|
||||
name='Device 2', site=site, device_type=device_type, role=role
|
||||
)
|
||||
cls.interface1 = Interface.objects.create(
|
||||
device=cls.device1, name='eth0', type=InterfaceTypeChoices.TYPE_1GE_FIXED
|
||||
)
|
||||
cls.interface2 = Interface.objects.create(
|
||||
device=cls.device2, name='eth0', type=InterfaceTypeChoices.TYPE_1GE_FIXED
|
||||
)
|
||||
cls.interface3 = Interface.objects.create(
|
||||
device=cls.device2, name='eth1', type=InterfaceTypeChoices.TYPE_1GE_FIXED
|
||||
)
|
||||
cls.rear_port = RearPort.objects.create(
|
||||
device=cls.device2, name='Rear Port 1', type=PortTypeChoices.TYPE_8P8C, positions=4
|
||||
)
|
||||
cls.front_port = FrontPort.objects.create(
|
||||
device=cls.device2, name='Front Port 1', type=PortTypeChoices.TYPE_8P8C, positions=4
|
||||
)
|
||||
cls.user = User.objects.create_user(username='testuser')
|
||||
|
||||
def _request(self):
|
||||
request = RequestFactory().get('/')
|
||||
request.id = uuid.uuid4()
|
||||
request.user = self.user
|
||||
return request
|
||||
|
||||
def _connect(self, termination_a, termination_b, **kwargs):
|
||||
with event_tracking(self._request()):
|
||||
cable = Cable(a_terminations=[termination_a], b_terminations=[termination_b], **kwargs)
|
||||
cable.save()
|
||||
return cable
|
||||
|
||||
def _updates(self, obj):
|
||||
return ObjectChange.objects.filter(
|
||||
changed_object_type=ObjectType.objects.get_for_model(obj),
|
||||
changed_object_id=obj.pk,
|
||||
action=ObjectChangeActionChoices.ACTION_UPDATE,
|
||||
).order_by('time')
|
||||
|
||||
@tag('regression') # Ref: netbox-branching#631
|
||||
def test_cable_deletion_records_disconnect(self):
|
||||
cable = self._connect(self.interface1, self.interface2)
|
||||
cable_pk = cable.pk
|
||||
|
||||
with event_tracking(self._request()):
|
||||
Cable.objects.get(pk=cable_pk).delete()
|
||||
|
||||
for interface in (self.interface1, self.interface2):
|
||||
# One update for the connect, one for the disconnect
|
||||
self.assertEqual(self._updates(interface).count(), 2, f'No disconnect recorded for {interface}')
|
||||
change = self._updates(interface).last()
|
||||
|
||||
# The cable attributes are cleared in the database before this is recorded, so the pre-change
|
||||
# data is only correct if they were restored before the snapshot was taken
|
||||
self.assertEqual(change.prechange_data['cable'], cable_pk)
|
||||
self.assertIn(change.prechange_data['cable_end'], (CableEndChoices.SIDE_A, CableEndChoices.SIDE_B))
|
||||
|
||||
self.assertIsNone(change.postchange_data['cable'])
|
||||
self.assertIsNone(change.postchange_data['cable_end'])
|
||||
# The originating path is deleted along with the Cable, outside the changelog
|
||||
self.assertIsNone(change.postchange_data['_path'])
|
||||
|
||||
self.assertIsNone(Interface.objects.get(pk=interface.pk).cable_id)
|
||||
self.assertFalse(CablePath.objects.exists())
|
||||
|
||||
def test_profiled_cable_deletion_records_connector_and_positions(self):
|
||||
cable = self._connect(self.interface1, self.rear_port, profile=CableProfileChoices.SINGLE_1C4P)
|
||||
cable_pk = cable.pk
|
||||
connector = Interface.objects.get(pk=self.interface1.pk).cable_connector
|
||||
positions = Interface.objects.get(pk=self.interface1.pk).cable_positions
|
||||
self.assertIsNotNone(connector)
|
||||
self.assertTrue(positions)
|
||||
|
||||
with event_tracking(self._request()):
|
||||
Cable.objects.get(pk=cable_pk).delete()
|
||||
|
||||
change = self._updates(self.interface1).last()
|
||||
self.assertEqual(change.prechange_data['cable_connector'], connector)
|
||||
self.assertEqual(change.prechange_data['cable_positions'], positions)
|
||||
self.assertIsNone(change.postchange_data['cable_connector'])
|
||||
self.assertIsNone(change.postchange_data['cable_positions'])
|
||||
|
||||
def test_queryset_deletion_records_disconnect(self):
|
||||
# A queryset delete never calls Cable.delete(), so the disconnect is tracked by a pre_delete receiver
|
||||
cable = self._connect(self.interface1, self.interface2)
|
||||
cable_pk = cable.pk
|
||||
|
||||
with event_tracking(self._request()):
|
||||
Cable.objects.filter(pk=cable_pk).delete()
|
||||
|
||||
change = self._updates(self.interface1).last()
|
||||
self.assertEqual(change.prechange_data['cable'], cable_pk)
|
||||
self.assertIsNone(change.postchange_data['cable'])
|
||||
|
||||
def test_non_path_endpoint_termination(self):
|
||||
# A front port carries no _path of its own; the disconnect is recorded the same way
|
||||
cable = self._connect(self.interface1, self.front_port)
|
||||
cable_pk = cable.pk
|
||||
|
||||
with event_tracking(self._request()):
|
||||
Cable.objects.get(pk=cable_pk).delete()
|
||||
|
||||
change = self._updates(self.front_port).last()
|
||||
self.assertEqual(change.prechange_data['cable'], cable_pk)
|
||||
self.assertIsNone(change.postchange_data['cable'])
|
||||
self.assertIsNone(FrontPort.objects.get(pk=self.front_port.pk).cable_id)
|
||||
|
||||
def test_termination_removal_records_disconnect(self):
|
||||
# Removing a termination from a Cable (rather than deleting the Cable) is recorded by
|
||||
# CableTermination.delete(); the disconnect must not be logged twice or logged as a no-op.
|
||||
cable = self._connect(self.interface1, self.interface2)
|
||||
cable_pk = cable.pk
|
||||
|
||||
request = self._request()
|
||||
with event_tracking(request):
|
||||
cable = Cable.objects.get(pk=cable_pk)
|
||||
cable.b_terminations = [self.interface3]
|
||||
cable.save()
|
||||
|
||||
changes = self._updates(self.interface2).filter(request_id=request.id)
|
||||
self.assertEqual(changes.count(), 1)
|
||||
self.assertEqual(changes[0].prechange_data['cable'], cable_pk)
|
||||
self.assertIsNone(changes[0].postchange_data['cable'])
|
||||
|
||||
def test_terminating_object_deletion_records_no_update(self):
|
||||
# The cascade from deleting the terminating object itself reaches the same handler, but the object
|
||||
# is on its way out: it must be left alone rather than resurrected as an update.
|
||||
self._connect(self.interface1, self.interface2)
|
||||
|
||||
request = self._request()
|
||||
with event_tracking(request):
|
||||
Interface.objects.get(pk=self.interface1.pk).delete()
|
||||
|
||||
self.assertFalse(self._updates(self.interface1).filter(request_id=request.id).exists())
|
||||
|
||||
|
||||
class VirtualDeviceContextTestCase(TestCase):
|
||||
|
||||
@classmethod
|
||||
|
|
|
|||
Loading…
Reference in New Issue