Fix bug that creates duplicate interfaces when syncing interfaces installed in a module (#476)

Author: gsnider2195

changes/476.fixed

@@ -0,0 +1 @@
+Fixed interfaces attached to modules being recreated when running the Sync Network Data job.

nautobot_device_onboarding/constants.py

@@ -54,3 +54,6 @@
 
 # The git repository data source folder name for custom command mappers.
 ONBOARDING_COMMAND_MAPPERS_REPOSITORY_FOLDER = "onboarding_command_mappers"
+
+# Support 4 modules deep (device -> modulebay -> module -> modulebay -> module -> modulebay -> module -> modulebay -> module -> interface)
+ONBOARDING_DEVICE_MODULE_RECURSION_LIMIT = 4

nautobot_device_onboarding/diffsync/adapters/sync_devices_adapters.py

@@ -107,18 +107,14 @@ def load_devices(self):
             self.job.logger.debug("Loading Device data from Nautobot...")
 
         for device in Device.objects.filter(primary_ip4__host__in=list(self.job.ip_address_inventory)):
-            interface_list = []
+            interfaces = []
             # Only interfaces with the device's primary ip should be considered for diff calculations
             # Ultimately, only the first matching interface is used but this list could support multiple
             # interface syncs in the future.
-            for interface in device.interfaces.all():
+            for interface in device.all_interfaces.order_by("name"):
                 if device.primary_ip4 in interface.ip_addresses.all():
-                    interface_list.append(interface.name)
-            if interface_list:
-                interface_list.sort()
-                interfaces = [interface_list[0]]
-            else:
-                interfaces = []
+                    interfaces = [interface.name]
+                    break
             onboarding_device = self.device(
                 adapter=self,
                 pk=device.pk,

nautobot_device_onboarding/diffsync/adapters/sync_network_data_adapters.py

@@ -167,60 +167,63 @@ def load_tagged_vlans_to_interface(self):
 
         Only Vlan assignments that were returned by the CommandGetter job should be loaded.
         """
-        for interface in Interface.objects.filter(device__in=self.job.devices_to_load):
-            tagged_vlans = []
-            for vlan in interface.tagged_vlans.all():
-                vlan_dict = {}
-                vlan_dict["name"] = vlan.name
-                vlan_dict["id"] = str(vlan.vid)
-                tagged_vlans.append(vlan_dict)
-            sorted_tagged_vlans = sorted(tagged_vlans, key=lambda x: x["id"])
-
-            network_tagged_vlans_to_interface = self.tagged_vlans_to_interface(
-                adapter=self,
-                device__name=interface.device.name,
-                name=interface.name,
-                tagged_vlans=sorted_tagged_vlans,
-            )
-            network_tagged_vlans_to_interface.model_flags = DiffSyncModelFlags.SKIP_UNMATCHED_DST
-            self.add(network_tagged_vlans_to_interface)
+        for device in self.job.devices_to_load:
+            for interface in device.all_interfaces:
+                tagged_vlans = []
+                for vlan in interface.tagged_vlans.all():
+                    vlan_dict = {}
+                    vlan_dict["name"] = vlan.name
+                    vlan_dict["id"] = str(vlan.vid)
+                    tagged_vlans.append(vlan_dict)
+                sorted_tagged_vlans = sorted(tagged_vlans, key=lambda x: x["id"])
+
+                network_tagged_vlans_to_interface = self.tagged_vlans_to_interface(
+                    adapter=self,
+                    device__name=device.name,
+                    name=interface.name,
+                    tagged_vlans=sorted_tagged_vlans,
+                )
+                network_tagged_vlans_to_interface.model_flags = DiffSyncModelFlags.SKIP_UNMATCHED_DST
+                self.add(network_tagged_vlans_to_interface)
 
     def load_untagged_vlan_to_interface(self):
         """
         Load UnTagged VLAN interface assignments into the Diffsync store.
 
         Only UnTagged Vlan assignments that were returned by the CommandGetter job should be synced.
         """
-        for interface in Interface.objects.filter(device__in=self.job.devices_to_load):
-            untagged_vlan = {}
-            if interface.untagged_vlan:
-                untagged_vlan["name"] = interface.untagged_vlan.name
-                untagged_vlan["id"] = str(interface.untagged_vlan.vid)
+        for device in self.job.devices_to_load:
+            for interface in device.all_interfaces:
+                untagged_vlan = {}
+                if interface.untagged_vlan:
+                    untagged_vlan["name"] = interface.untagged_vlan.name
+                    untagged_vlan["id"] = str(interface.untagged_vlan.vid)
 
-            network_untagged_vlan_to_interface = self.untagged_vlan_to_interface(
-                adapter=self,
-                device__name=interface.device.name,
-                name=interface.name,
-                untagged_vlan=untagged_vlan,
-            )
-            network_untagged_vlan_to_interface.model_flags = DiffSyncModelFlags.SKIP_UNMATCHED_DST
-            self.add(network_untagged_vlan_to_interface)
+                network_untagged_vlan_to_interface = self.untagged_vlan_to_interface(
+                    adapter=self,
+                    device__name=device.name,
+                    name=interface.name,
+                    untagged_vlan=untagged_vlan,
+                )
+                network_untagged_vlan_to_interface.model_flags = DiffSyncModelFlags.SKIP_UNMATCHED_DST
+                self.add(network_untagged_vlan_to_interface)
 
     def load_lag_to_interface(self):
         """
         Load Lag interface assignments into the Diffsync store.
 
         Only Lag assignments that were returned by the CommandGetter job should be synced.
         """
-        for interface in Interface.objects.filter(device__in=self.job.devices_to_load):
-            network_lag_to_interface = self.lag_to_interface(
-                adapter=self,
-                device__name=interface.device.name,
-                name=interface.name,
-                lag__interface__name=interface.lag.name if interface.lag else "",
-            )
-            network_lag_to_interface.model_flags = DiffSyncModelFlags.SKIP_UNMATCHED_DST
-            self.add(network_lag_to_interface)
+        for device in self.job.devices_to_load:
+            for interface in device.all_interfaces:
+                network_lag_to_interface = self.lag_to_interface(
+                    adapter=self,
+                    device__name=device.name,
+                    name=interface.name,
+                    lag__interface__name=interface.lag.name if interface.lag else "",
+                )
+                network_lag_to_interface.model_flags = DiffSyncModelFlags.SKIP_UNMATCHED_DST
+                self.add(network_lag_to_interface)
 
     def load_vrfs(self):
         """
@@ -246,19 +249,20 @@ def load_vrf_to_interface(self):
 
         Only Vrf assignments that were returned by the CommandGetter job should be synced.
         """
-        for interface in Interface.objects.filter(device__in=self.job.devices_to_load):
-            vrf = {}
-            if interface.vrf:
-                vrf["name"] = interface.vrf.name
+        for device in self.job.devices_to_load:
+            for interface in device.all_interfaces:
+                vrf = {}
+                if interface.vrf:
+                    vrf["name"] = interface.vrf.name
 
-            network_vrf_to_interface = self.vrf_to_interface(
-                adapter=self,
-                device__name=interface.device.name,
-                name=interface.name,
-                vrf=vrf,
-            )
-            network_vrf_to_interface.model_flags = DiffSyncModelFlags.SKIP_UNMATCHED_DST
-            self.add(network_vrf_to_interface)
+                network_vrf_to_interface = self.vrf_to_interface(
+                    adapter=self,
+                    device__name=device.name,
+                    name=interface.name,
+                    vrf=vrf,
+                )
+                network_vrf_to_interface.model_flags = DiffSyncModelFlags.SKIP_UNMATCHED_DST
+                self.add(network_vrf_to_interface)
 
     def load_cables(self):
         """
@@ -345,6 +349,21 @@ def load_software_version_to_device(self):
             network_software_version_to_device.model_flags = DiffSyncModelFlags.SKIP_UNMATCHED_DST
             self.add(network_software_version_to_device)
 
+    def _handle_single_parameter(self, parameters, parameter_name, database_object, diffsync_model):
+        """Overload parameter handling to add special handling for modular interfaces."""
+        if parameter_name == "device__name":
+            if database_object.parent:
+                parameters["device__name"] = database_object.parent.name
+            else:
+                parameters["device__name"] = ""
+        elif parameter_name == "interface__device__name":
+            if database_object.interface.parent:
+                parameters["interface__device__name"] = database_object.interface.parent.name
+            else:
+                parameters["interface__device__name"] = ""
+        else:
+            super()._handle_single_parameter(parameters, parameter_name, database_object, diffsync_model)
+
     def load(self):
         """Generic implementation of the load function."""
         if not hasattr(self, "top_level") or not self.top_level:
@@ -414,7 +433,7 @@ def sync_complete(self, source, diff, *args, **kwargs):
                     )
                 if ip_address:
                     try:
-                        interface = Interface.objects.get(device=device, ip_addresses__in=[ip_address])
+                        interface = device.all_interfaces.get(ip_addresses__in=[ip_address])
                         interface.mgmt_only = True
                         interface.validated_save()
                         self.job.logger.info(

nautobot_device_onboarding/diffsync/models/sync_devices_models.py

@@ -100,10 +100,7 @@ def _get_or_create_interface(cls, adapter, device, ip_address, interface_name):
         """Attempt to get a Device Interface, create a new one if necessary."""
         device_interface = None
         try:
-            device_interface = Interface.objects.get(
-                name=interface_name,
-                device=device,
-            )
+            device_interface = device.all_interfaces.get(name=interface_name)
         except ObjectDoesNotExist:
             try:
                 job_form_attrs = adapter.job.ip_address_inventory[ip_address]
@@ -156,10 +153,7 @@ def _update_device_with_attrs(cls, device, platform, ids, attrs, adapter):
     def _remove_old_interface_assignment(self, device, ip_address):
         """Remove a device's primary IP address from an interface."""
         try:
-            old_interface = Interface.objects.get(
-                device=device,
-                ip_addresses__in=[ip_address],
-            )
+            old_interface = device.all_interfaces.get(ip_addresses__in=[ip_address])
             old_interface_assignment = IPAddressToInterface.objects.get(
                 interface=old_interface,
                 ip_address=ip_address,