Merge branch 'develop'

Author: glennmatthews

CHANGELOG.md

@@ -1,5 +1,21 @@
 # Changelog
 
+## v1.2.0 (2021-04-14)
+
+### Added
+
+- #33 - Now supports the Django parameters `--no-color` and `--force-color`
+
+### Changed
+
+- #29 - Improved formatting of log output, added dynamic progress bars using `tqdm` library
+
+### Fixed
+
+- #31 - Records containing outdated custom field data should now be updated successfully
+- #32 - Status objects should not show as changed when resyncing data
+
+
 ## v1.1.0 (2021-04-07)
 
 ### Added

development/Dockerfile

@@ -1,7 +1,7 @@
 ARG python_ver=3.7
 FROM python:${python_ver}
 
-ARG nautobot_ver=1.0.0a1
+ARG nautobot_ver=1.0.0b3
 ENV PYTHONUNBUFFERED=1 \
     PATH="/root/.poetry/bin:$PATH" \
     NAUTOBOT_CONFIG="/source/development/nautobot_config.py"

development/nautobot_config.py

@@ -216,7 +216,7 @@
     },
     "loggers": {
         "django": {"handlers": ["normal_console"], "level": "INFO"},
-        "nautobot": {"handlers": ["verbose_console" if DEBUG else "normal_console"], "level": LOG_LEVEL},
+        "nautobot": {"handlers": ["verbose_console" if DEBUG else "normal_console"], "level": "INFO"},
         "rq.worker": {"handlers": ["verbose_console" if DEBUG else "normal_console"], "level": LOG_LEVEL},
     },
 }

media/screenshot1.png

@@ -1,7 +1,7 @@
 """Abstract base DiffSync adapter class for code shared by NetBox and Nautobot adapters."""
 
 from collections import defaultdict
-from typing import MutableMapping, Union
+from typing import Callable, MutableMapping, Union
 from uuid import UUID
 
 from diffsync import Diff, DiffSync, DiffSyncFlags, DiffSyncModel
@@ -133,6 +133,8 @@ class N2NDiffSync(DiffSync):
         "user",  # Includes NetBox "userconfig" model as well
         "objectpermission",
         "token",
+        "customfield",
+        "customfieldchoice",
         # "status", Not synced, as these are hard-coded in NetBox and autogenerated in Nautobot
         # Need Tenant and TenantGroup before we can populate Sites
         "tenantgroup",
@@ -208,15 +210,12 @@ class N2NDiffSync(DiffSync):
         "webhook",
         "taggeditem",
         "jobresult",
-        # Imported last so that any "required=True" CustomFields do not cause Nautobot to reject
-        # NetBox records that predate the creation of those CustomFields
-        "customfield",
-        "customfieldchoice",
     )
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, *args, verbosity: int = 0, **kwargs):
         """Initialize this container, including its PK-indexed alternate data store."""
         super().__init__(*args, **kwargs)
+        self.verbosity = verbosity
         self._data_by_pk = defaultdict(dict)
         self._sync_summary = None
 
@@ -265,8 +264,8 @@ def make_model(self, diffsync_model, data):
             instance = diffsync_model(**data, diffsync=self)
         except ValidationError as exc:
             self.logger.error(
-                "Invalid data according to internal data model. "
-                "This may be an issue with your source data or may reflect a bug in this plugin.",
+                "Invalid data according to internal data model",
+                comment="This may be an issue with your source data or may reflect a bug in this plugin.",
                 action="load",
                 exception=str(exc),
                 model=diffsync_model.get_type(),
@@ -278,19 +277,25 @@ def make_model(self, diffsync_model, data):
         except ObjectAlreadyExists:
             existing_instance = self.get(diffsync_model, instance.get_unique_id())
             self.logger.warning(
-                "Apparent duplicate object encountered? "
-                "This may be an issue with your source data or may reflect a bug in this plugin.",
+                "Apparent duplicate object encountered?",
+                comment="This may be an issue with your source data or may reflect a bug in this plugin.",
                 duplicate_id=instance.get_identifiers(),
                 model=diffsync_model.get_type(),
                 pk_1=existing_instance.pk,
                 pk_2=instance.pk,
             )
         return instance
 
-    def sync_from(self, source: DiffSync, diff_class: Diff = Diff, flags: DiffSyncFlags = DiffSyncFlags.NONE):
+    def sync_from(
+        self,
+        source: DiffSync,
+        diff_class: Diff = Diff,
+        flags: DiffSyncFlags = DiffSyncFlags.NONE,
+        callback: Callable[[str, int, int], None] = None,
+    ):
         """Synchronize data from the given source DiffSync object into the current DiffSync object."""
         self._sync_summary = None
-        return super().sync_from(source, diff_class=diff_class, flags=flags)
+        return super().sync_from(source, diff_class=diff_class, flags=flags, callback=callback)
 
     def sync_complete(
         self,

media/screenshot2.png

@@ -2,12 +2,14 @@
 
 from uuid import UUID
 
+from diffsync import DiffSync
 from django.contrib.contenttypes.fields import GenericForeignKey, GenericRel
 from django.db import models
 import structlog
 
+from nautobot_netbox_importer.diffsync.models.abstract import NautobotBaseModel
+from nautobot_netbox_importer.utils import ProgressBar
 from .abstract import N2NDiffSync
-from ..models.abstract import NautobotBaseModel
 
 
 IGNORED_FIELD_CLASSES = (GenericRel, GenericForeignKey, models.ManyToManyRel, models.ManyToOneRel)
@@ -54,6 +56,10 @@ def load_model(self, diffsync_model, record):  # pylint: disable=too-many-branch
             # What's the name of the model that this is a reference to?
             target_name = diffsync_model.fk_associations()[field.name]
 
+            if target_name == "status":
+                data[field.name] = {"slug": self.status.nautobot_model().objects.get(pk=value).slug}
+                continue
+
             # Special case: for generic foreign keys, the target_name is actually the name of
             # another field on this record that describes the content-type of this foreign key id.
             # We flag this by starting the target_name string with a '*', as if this were C or something.
@@ -101,8 +107,36 @@ def load(self):
         self.logger.info("Loading data from Nautobot into DiffSync...")
         for modelname in ("contenttype", "permission", "status", *self.top_level):
             diffsync_model = getattr(self, modelname)
-            self.logger.info(f"Loading all {modelname} records...")
-            for instance in diffsync_model.nautobot_model().objects.all():
-                self.load_model(diffsync_model, instance)
+            if diffsync_model.nautobot_model().objects.exists():
+                for instance in ProgressBar(
+                    diffsync_model.nautobot_model().objects.all(),
+                    total=diffsync_model.nautobot_model().objects.count(),
+                    desc=f"{modelname:<25}",  # len("consoleserverporttemplate")
+                    verbosity=self.verbosity,
+                ):
+                    self.load_model(diffsync_model, instance)
 
         self.logger.info("Data loading from Nautobot complete.")
+
+    def restore_required_custom_fields(self, source: DiffSync):
+        """Post-synchronization cleanup function to restore any 'required=True' custom field records."""
+        self.logger.debug("Restoring the 'required=True' flag on any such custom fields")
+        for customfield in source.get_all(source.customfield):
+            if customfield.actual_required:
+                # We don't want to change the DiffSync record's `required` flag, only the Nautobot record
+                customfield.update_nautobot_record(
+                    customfield.nautobot_model(),
+                    ids=customfield.get_identifiers(),
+                    attrs={"required": True},
+                    multivalue_attrs={},
+                )
+
+    def sync_complete(self, source: DiffSync, *args, **kwargs):
+        """Callback invoked after completing a sync operation in which changes occurred."""
+        # During the sync, we intentionally marked all custom fields as "required=False"
+        # so that we could sync records that predated the creation of said custom fields.
+        # Now that we've updated all records that might contain custom field data,
+        # only now can we re-mark any "required" custom fields as such.
+        self.restore_required_custom_fields(source)
+
+        return super().sync_complete(source, *args, **kwargs)

nautobot_netbox_importer/diffsync/adapters/abstract.py

@@ -6,9 +6,10 @@
 from diffsync.enum import DiffSyncModelFlags
 import structlog
 
+from nautobot_netbox_importer.diffsync.models.abstract import NautobotBaseModel
+from nautobot_netbox_importer.diffsync.models.validation import netbox_pk_to_nautobot_pk
+from nautobot_netbox_importer.utils import ProgressBar
 from .abstract import N2NDiffSync
-from ..models.abstract import NautobotBaseModel
-from ..models.validation import netbox_pk_to_nautobot_pk
 
 
 class NetBox210DiffSync(N2NDiffSync):
@@ -103,18 +104,29 @@ def load_record(self, diffsync_model, record):  # pylint: disable=too-many-branc
             else:
                 self.logger.warning("No UserConfig found for User", username=data["username"], pk=record["pk"])
                 data["config_data"] = {}
-        elif diffsync_model == self.customfield and data["type"] == "select":
-            # NetBox stores the choices for a "select" CustomField (NetBox has no "multiselect" CustomFields)
-            # locally within the CustomField model, whereas Nautobot has a separate CustomFieldChoices model.
-            # So we need to split the choices out into separate DiffSync instances.
-            # Since "choices" is an ArrayField, we have to parse it from the JSON string
-            # see also models.abstract.ArrayField
-            for choice in json.loads(data["choices"]):
-                self.make_model(
-                    self.customfieldchoice,
-                    {"pk": uuid4(), "field": netbox_pk_to_nautobot_pk("customfield", record["pk"]), "value": choice},
-                )
-            del data["choices"]
+        elif diffsync_model == self.customfield:
+            # Because marking a custom field as "required" doesn't automatically assign a value to pre-existing records,
+            # we never want to enforce 'required=True' at import time as there may be otherwise valid records that predate
+            # the creation of this field. Store it on a private field instead and we'll fix it up at the end.
+            data["actual_required"] = data["required"]
+            data["required"] = False
+
+            if data["type"] == "select":
+                # NetBox stores the choices for a "select" CustomField (NetBox has no "multiselect" CustomFields)
+                # locally within the CustomField model, whereas Nautobot has a separate CustomFieldChoices model.
+                # So we need to split the choices out into separate DiffSync instances.
+                # Since "choices" is an ArrayField, we have to parse it from the JSON string
+                # see also models.abstract.ArrayField
+                for choice in json.loads(data["choices"]):
+                    self.make_model(
+                        self.customfieldchoice,
+                        {
+                            "pk": uuid4(),
+                            "field": netbox_pk_to_nautobot_pk("customfield", record["pk"]),
+                            "value": choice,
+                        },
+                    )
+                del data["choices"]
 
         return self.make_model(diffsync_model, data)
 
@@ -123,14 +135,18 @@ def load(self):
         self.logger.info("Loading imported NetBox source data into DiffSync...")
         for modelname in ("contenttype", "permission", *self.top_level):
             diffsync_model = getattr(self, modelname)
-            self.logger.info(f"Loading all {modelname} records...")
             content_type_label = diffsync_model.nautobot_model()._meta.label_lower
             # Handle a NetBox vs Nautobot discrepancy - the Nautobot target model is 'users.user',
             # but the NetBox data export will have user records under the label 'auth.user'.
             if content_type_label == "users.user":
                 content_type_label = "auth.user"
-            for record in self.source_data:
-                if record["model"] == content_type_label:
+            records = [record for record in self.source_data if record["model"] == content_type_label]
+            if records:
+                for record in ProgressBar(
+                    records,
+                    desc=f"{modelname:<25}",  # len("consoleserverporttemplate")
+                    verbosity=self.verbosity,
+                ):
                     self.load_record(diffsync_model, record)
 
         self.logger.info("Data loading from NetBox source data complete.")

nautobot_netbox_importer/diffsync/adapters/nautobot.py

@@ -75,9 +75,9 @@ def fk_associations(cls):
         """Get the mapping between foreign key (FK) fields and the corresponding DiffSync models they reference."""
         return cls._fk_associations
 
-    @staticmethod
+    @classmethod
     def _get_nautobot_record(
-        diffsync_model: DiffSyncModel, diffsync_value: Any, fail_quiet: bool = False
+        cls, diffsync_model: DiffSyncModel, diffsync_value: Any, fail_quiet: bool = False
     ) -> Optional[models.Model]:
         """Given a diffsync model and identifier (natural key or primary key) look up the Nautobot record."""
         try:
@@ -93,6 +93,7 @@ def _get_nautobot_record(
             log = logger.debug if fail_quiet else logger.error
             log(
                 "Expected but did not find an existing Nautobot record",
+                source=cls.get_type(),
                 target=diffsync_model.get_type(),
                 unique_id=diffsync_value,
             )
@@ -186,6 +187,7 @@ def clean_attrs(cls, diffsync: DiffSync, attrs: dict) -> Tuple[dict, dict]:
     @staticmethod
     def create_nautobot_record(nautobot_model, ids: Mapping, attrs: Mapping, multivalue_attrs: Mapping):
         """Helper method to create() - actually populate Nautobot data."""
+        model_data = dict(**ids, **attrs, **multivalue_attrs)
         try:
             # Custom fields are a special case - because in NetBox the values defined on a particular record are
             # only loosely coupled to the CustomField definition itself, it's quite possible that these two may be
@@ -208,23 +210,23 @@ def create_nautobot_record(nautobot_model, ids: Mapping, attrs: Mapping, multiva
                 action="create",
                 exception=str(exc),
                 model=nautobot_model,
-                model_data=dict(**ids, **attrs, **multivalue_attrs),
+                model_data=model_data,
             )
         except DjangoValidationError as exc:
             logger.error(
                 "Nautobot reported a data validation error - check your source data",
                 action="create",
                 exception=str(exc),
                 model=nautobot_model,
-                model_data=dict(**ids, **attrs, **multivalue_attrs),
+                model_data=model_data,
             )
         except ObjectDoesNotExist as exc:  # Including RelatedObjectDoesNotExist
             logger.error(
                 "Nautobot reported an error about a missing required object",
                 action="create",
                 exception=str(exc),
                 model=nautobot_model,
-                model_data=dict(**ids, **attrs, **multivalue_attrs),
+                model_data=model_data,
             )
 
         return None
@@ -272,9 +274,17 @@ def create(cls, diffsync: DiffSync, ids: Mapping, attrs: Mapping) -> Optional["N
     @staticmethod
     def update_nautobot_record(nautobot_model, ids: Mapping, attrs: Mapping, multivalue_attrs: Mapping):
         """Helper method to update() - actually update Nautobot data."""
+        model_data = dict(**ids, **attrs, **multivalue_attrs)
         try:
             record = nautobot_model.objects.get(**ids)
             custom_field_data = attrs.pop("custom_field_data", None)
+            # Temporarily clear any existing custom field data as part of the model update,
+            # so that in case the model contains "stale" data referring to no-longer-existent fields,
+            # Nautobot won't reject it out of hand.
+            if not custom_field_data and hasattr(record, "custom_field_data"):
+                custom_field_data = record.custom_field_data
+            if custom_field_data:
+                record._custom_field_data = {}  # pylint: disable=protected-access
             for attr, value in attrs.items():
                 setattr(record, attr, value)
             record.clean()
@@ -291,23 +301,23 @@ def update_nautobot_record(nautobot_model, ids: Mapping, attrs: Mapping, multiva
                 action="update",
                 exception=str(exc),
                 model=nautobot_model,
-                model_data=dict(**ids, **attrs, **multivalue_attrs),
+                model_data=model_data,
             )
         except DjangoValidationError as exc:
             logger.error(
                 "Nautobot reported a data validation error - check your source data",
                 action="update",
                 exception=str(exc),
                 model=nautobot_model,
-                model_data=dict(**ids, **attrs, **multivalue_attrs),
+                model_data=model_data,
             )
         except ObjectDoesNotExist as exc:  # Including RelatedObjectDoesNotExist
             logger.error(
                 "Nautobot reported an error about a missing required object",
                 action="update",
                 exception=str(exc),
                 model=nautobot_model,
-                model_data=dict(**ids, **attrs, **multivalue_attrs),
+                model_data=model_data,
             )
 
         return None

nautobot_netbox_importer/diffsync/adapters/netbox.py

@@ -114,6 +114,11 @@ class CustomField(NautobotBaseModel):
     validation_maximum: Optional[int]
     validation_regex: str
 
+    # Because marking a custom field as "required" doesn't automatically assign a value to pre-existing records,
+    # we never want, when adding custom fields from NetBox, to flag fields as required=True.
+    # Instead we store it in "actual_required" and fix it up only afterwards.
+    actual_required: Optional[bool]
+
     @classmethod
     def special_clean(cls, diffsync, ids, attrs):
         """Special-case handling for the "default" attribute."""