Merge pull request #13 from networktocode-llc/next

Fix Duplicate DeviceType Models

.github/workflows/ci.yml

@@ -83,7 +83,7 @@ jobs:
       fail-fast: true
       matrix:
         python-version: ["3.8"]
-        nautobot-version: ["1.5.6"]
+        nautobot-version: ["1.6.0"]
     env:
       INVOKE_NAUTOBOT_SSOT_DNA_CENTER_PYTHON_VER: "${{ matrix.python-version }}"
       INVOKE_NAUTOBOT_SSOT_DNA_CENTER_NAUTOBOT_VER: "${{ matrix.nautobot-version }}"
@@ -121,17 +121,17 @@ jobs:
       matrix:
         python-version: ["3.8", "3.9", "3.10"]
         db-backend: ["postgresql"]
-        nautobot-version: ["stable"]
+        nautobot-version: ["1.6.0"]
         include:
           - python-version: "3.10"
             db-backend: "postgresql"
-            nautobot-version: "1.5.0"
+            nautobot-version: "1.6.0"
           - python-version: "3.8"
             db-backend: "mysql"
-            nautobot-version: "1.5.0"
+            nautobot-version: "1.6.0"
           - python-version: "3.10"
             db-backend: "mysql"
-            nautobot-version: "stable"
+            nautobot-version: "1.6.0"
     runs-on: "ubuntu-20.04"
     env:
       INVOKE_NAUTOBOT_SSOT_DNA_CENTER_PYTHON_VER: "${{ matrix.python-version }}"

nautobot_ssot_dna_center/diffsync/adapters/dna_center.py

@@ -340,7 +340,7 @@ def load_devices(self):
                     status="Active" if dev.get("reachabilityStatus") != "Unreachable" else "Offline",
                     role=dev_role,
                     vendor=vendor,
-                    model=dev["platformId"] if dev.get("platformId") else "Unknown",
+                    model=self.conn.get_model_name(models=dev["platformId"]) if dev.get("platformId") else "Unknown",
                     site=loc_data["building"],
                     floor=f"{loc_data['building']} - {loc_data['floor']}" if loc_data.get("floor") else None,
                     serial=dev["serialNumber"] if dev.get("serialNumber") else "",

nautobot_ssot_dna_center/diffsync/models/nautobot.py

@@ -40,7 +40,8 @@ def create(cls, diffsync, ids, attrs):
             Region.objects.get(name=ids["name"])
             diffsync.job.log_warning(message=f"Region {ids['name']} already exists so won't be created.")
         except Region.DoesNotExist:
-            diffsync.job.log_info(message=f"Creating Region {ids['name']}.")
+            if diffsync.job.kwargs.get("debug"):
+                diffsync.job.log_info(message=f"Creating Region {ids['name']}.")
             new_region = Region(
                 name=ids["name"],
             )
@@ -59,7 +60,8 @@ class NautobotBuilding(base.Building):
     @classmethod
     def create(cls, diffsync, ids, attrs):
         """Create Site in Nautobot from Building object."""
-        diffsync.job.log_info(message=f"Creating Site {ids['name']}.")
+        if diffsync.job.kwargs.get("debug"):
+            diffsync.job.log_info(message=f"Creating Site {ids['name']}.")
         new_site = Site(
             name=ids["name"],
             physical_address=attrs["address"] if attrs.get("address") else "",
@@ -73,19 +75,20 @@ def create(cls, diffsync, ids, attrs):
             if attrs.get("area"):
                 new_site.region = Region.objects.get(name=attrs["area"])
         except Region.DoesNotExist:
-            diffsync.job.log_info(message=f"Unable to find parent {attrs['area']}")
+            diffsync.job.log_warning(message=f"Unable to find parent {attrs['area']}")
         new_site.validated_save()
         return super().create(diffsync=diffsync, ids=ids, attrs=attrs)
 
     def update(self, attrs):
         """Update Site in Nautobot from Building object."""
         if not settings.PLUGINS_CONFIG["nautobot_ssot_dna_center"].get("update_locations"):
-            self.diffsync.job.log_warning(
+            self.diffsync.job.log_info(
                 message=f"`update_locations` setting is disabled so will skip updating {self.name}."
             )
             return None
         site = Site.objects.get(id=self.uuid)
-        self.diffsync.job.log_info(message=f"Updating Site {site.name}.")
+        if self.diffsync.job.kwargs.get("debug"):
+            self.diffsync.job.log_info(message=f"Updating Site {site.name}.")
         if "address" in attrs:
             site.physical_address = attrs["address"]
         if "area" in attrs:
@@ -110,7 +113,8 @@ def delete(self):
             )
             return None
         site = Site.objects.get(id=self.uuid)
-        self.diffsync.job.log_info(message=f"Deleting Site {site.name}.")
+        if self.diffsync.job.kwargs.get("debug"):
+            self.diffsync.job.log_info(message=f"Deleting Site {site.name}.")
         self.diffsync.objects_to_delete["sites"].append(site)
         return self
 
@@ -121,7 +125,8 @@ class NautobotFloor(base.Floor):
     @classmethod
     def create(cls, diffsync, ids, attrs):
         """Create LocationType: Floor in Nautobot from Floor object."""
-        diffsync.job.log_info(message=f"Creating Floor {ids['name']}.")
+        if diffsync.job.kwargs.get("debug"):
+            diffsync.job.log_info(message=f"Creating Floor {ids['name']}.")
         loc_type, created = LocationType.objects.get_or_create(
             name="Floor",
             nestable=False,
@@ -145,7 +150,8 @@ def create(cls, diffsync, ids, attrs):
     def update(self, attrs):
         """Update LocationType: Floor in Nautobot from Floor object."""
         floor = Location.objects.get(name=self.name, location_type=LocationType.objects.get(name="Floor"))
-        self.diffsync.job.log_info(message=f"Updating Floor {floor.name} with {attrs}")
+        if self.diffsync.job.kwargs.get("debug"):
+            self.diffsync.job.log_info(message=f"Updating Floor {floor.name} with {attrs}")
         if "tenant" in attrs:
             if attrs.get("tenant"):
                 floor.tenant = Tenant.objects.get(name=attrs["tenant"])
@@ -157,7 +163,8 @@ def update(self, attrs):
     def delete(self):
         """Delete LocationType: Floor in Nautobot from Floor object."""
         floor = Location.objects.get(id=self.uuid)
-        self.diffsync.job.log_info(message=f"Deleting Floor {floor.name} in {floor.site.name}.")
+        if self.diffsync.job.kwargs.get("debug"):
+            self.diffsync.job.log_info(message=f"Deleting Floor {floor.name} in {floor.site.name}.")
         self.diffsync.objects_to_delete["floors"].append(floor)
         return self
 
@@ -168,7 +175,8 @@ class NautobotDevice(base.Device):
     @classmethod
     def create(cls, diffsync, ids, attrs):
         """Create Device in Nautobot from NautobotDevice object."""
-        diffsync.job.log_info(message=f"Creating Device {ids['name']}.")
+        if diffsync.job.kwargs.get("debug"):
+            diffsync.job.log_info(message=f"Creating Device {ids['name']}.")
         site = Site.objects.get(name=attrs["site"])
         manufacturer, _ = Manufacturer.objects.get_or_create(name=attrs["vendor"])
         device_role, _ = DeviceRole.objects.get_or_create(name=attrs["role"])
@@ -213,7 +221,8 @@ def create(cls, diffsync, ids, attrs):
     def update(self, attrs):
         """Update Device in Nautobot from NautobotDevice object."""
         device = Device.objects.get(id=self.uuid)
-        self.diffsync.job.log_info(message=f"Updating Device {device.name} with {attrs}")
+        if self.diffsync.job.kwargs.get("debug"):
+            self.diffsync.job.log_info(message=f"Updating Device {device.name} with {attrs}")
         if "status" in attrs:
             device.status = Status.objects.get(name=attrs["status"])
         if "role" in attrs:
@@ -237,7 +246,11 @@ def update(self, attrs):
                 location = None
             device.location = location
         if "model" in attrs:
-            device.device_type = DeviceType.objects.get_or_create(model=attrs["model"])[0]
+            if attrs.get("vendor"):
+                vendor = Manufacturer.objects.get_or_create(name=attrs["vendor"])[0]
+            else:
+                vendor = Manufacturer.objects.get_or_create(name=self.vendor)[0]
+            device.device_type = DeviceType.objects.get_or_create(model=attrs["model"], manufacturer=vendor)[0]
         if "serial" in attrs:
             device.serial = attrs["serial"]
         if "platform" in attrs:
@@ -271,7 +284,8 @@ def update(self, attrs):
     def delete(self):
         """Delete Device in Nautobot from NautobotDevice object."""
         dev = Device.objects.get(id=self.uuid)
-        self.diffsync.job.log_info(message=f"Deleting Device: {dev.name}.")
+        if self.diffsync.job.kwargs.get("debug"):
+            self.diffsync.job.log_info(message=f"Deleting Device: {dev.name}.")
         super().delete()
         self.diffsync.objects_to_delete["devices"].append(dev)
         return self
@@ -283,7 +297,8 @@ class NautobotPort(base.Port):
     @classmethod
     def create(cls, diffsync, ids, attrs):
         """Create Interface in Nautobot from Port object."""
-        diffsync.job.log_info(message=f"Creating Port {ids['name']} for Device {ids['device']}.")
+        if diffsync.job.kwargs.get("debug"):
+            diffsync.job.log_info(message=f"Creating Port {ids['name']} for Device {ids['device']}.")
         new_port = Interface(
             name=ids["name"],
             device=Device.objects.get(name=ids["device"]),
@@ -304,7 +319,8 @@ def create(cls, diffsync, ids, attrs):
     def update(self, attrs):
         """Update Interface in Nautobot from Port object."""
         port = Interface.objects.get(id=self.uuid)
-        self.diffsync.job.log_info(message=f"Updating Port {port.name} for Device {port.device.name}.")
+        if self.diffsync.job.kwargs.get("debug"):
+            self.diffsync.job.log_info(message=f"Updating Port {port.name} for Device {port.device.name}.")
         if "description" in attrs:
             port.description = attrs["description"]
         if "mac_addr" in attrs:

nautobot_ssot_dna_center/tests/test_adapters_dna_center.py

@@ -48,6 +48,7 @@ def setUp(self):
         ]
         self.dna_center_client.find_latitude_and_longitude.return_value = ("", "")
         self.dna_center_client.get_device_detail.return_value = DEVICE_DETAIL_FIXTURE
+        self.dna_center_client.get_model_name.return_value = "WS-C3850-24P-L"
         self.dna_center_client.parse_site_hierarchy.return_value = {
             "areas": ["Global", "NY"],
             "building": "Building1",

nautobot_ssot_dna_center/tests/test_models_nautobot.py

@@ -75,14 +75,15 @@ def setUp(self):
         self.test_bldg.diffsync = MagicMock()
         self.test_bldg.diffsync.job = MagicMock()
         self.test_bldg.diffsync.job.log_info = MagicMock()
+        self.test_bldg.diffsync.job.log_warning = MagicMock()
 
     def test_create_wo_parent(self):
         """Validate the NautobotBuilding create() method creates a Site without a matching parent Region."""
         ids = {"name": "HQ"}
         attrs = {"address": "123 Main St", "area": "NY", "latitude": "12.345", "longitude": "-67.890", "tenant": "G&A"}
         result = NautobotBuilding.create(self.diffsync, ids, attrs)
         self.assertIsInstance(result, NautobotBuilding)
-        self.diffsync.job.log_info.assert_called_with(message="Unable to find parent NY")
+        self.diffsync.job.log_warning.assert_called_with(message="Unable to find parent NY")
         site_obj = Site.objects.get(name=ids["name"])
         self.assertFalse(getattr(site_obj, "region"))
         self.assertEqual(site_obj.physical_address, attrs["address"])

nautobot_ssot_dna_center/tests/test_utils_dna_center.py

@@ -55,9 +55,8 @@ def test_connect_success(self, mock_api):
     def test_connect_error(self, mock_api):
         self.dnac.conn = None
         mock_api.side_effect = dnacentersdkException(self.mock_response)
-        with self.assertLogs(level="ERROR") as log:
+        with self.assertRaises(dnacentersdkException):
             self.dnac.connect()
-            self.assertIn("Unable to connect to DNA Center", log.output[0])
         mock_api.assert_called_once_with(  # nosec B106
             base_url="https://dnac.testexample.com:443", password="testpassword", username="testuser", verify=False
         )
@@ -213,3 +212,15 @@ def test_parse_hostname_for_role_failure(self):
         hostname = "core-router.example.com"
         result = self.dnac.parse_hostname_for_role(hostname_map=hostname_mapping, device_hostname=hostname)
         self.assertEqual(result, "Unknown")
+
+    def test_get_model_name_single_model(self):
+        """Validate the functionality of get_model_name method with single model in string."""
+        test_model = "CSR1000v"
+        result = self.dnac.get_model_name(models=test_model)
+        self.assertEqual(result, "CSR1000v")
+
+    def test_get_model_name_multiple_models(self):
+        """Validate the functionality of get_model_name method with multiple models in string."""
+        test_models = "CSR1000v, CSR1000v, CSR1000v"
+        result = self.dnac.get_model_name(models=test_models)
+        self.assertEqual(result, "CSR1000v")

nautobot_ssot_dna_center/utils/dna_center.py

@@ -35,7 +35,7 @@ def connect(self):  # pylint: disable=inconsistent-return-statements
                 base_url=self.base_url, username=self.username, password=self.password, verify=self.verify
             )
         except dnacentersdkException as err:
-            LOGGER.error("Unable to connect to DNA Center: %s", err)
+            raise dnacentersdkException(f"Unable to connect to DNA Center: {err}") from err
 
     def get_locations(self):
         """Retrieve all location data from DNA Center.
@@ -224,3 +224,20 @@ def parse_hostname_for_role(hostname_map: List[Tuple[str, str]], device_hostname
                 if match:
                     device_role = entry[1]
         return device_role
+
+    @staticmethod
+    def get_model_name(models: str) -> str:
+        """Obtain DeviceType model from a list of models.
+
+        Args:
+            models (str): String specifying DeviceType model. Potentially a list of models.
+
+        Returns:
+            str: Parsed model name. If list of models, just a single model.
+        """
+        if "," in models:
+            model_list = models.split(", ")
+            model_list = list(set(model_list))
+            if len(model_list) == 1:
+                return model_list[0]
+        return models