changes

docs/user/design_lifecycle.md

@@ -91,24 +91,22 @@ The decommissioning job outputs a log with all the detailed operation reverting
 
 ### Design Deployment Import
 
-Design Builder helps in greenfield use cases (i.e., creating a new data from a design) and in brownfield ones too (i.e., importing existing data that are are related to a new deployment). In the "deployment" mode, a Design Deployment tracks all the objects and attributes that are "owned" by it. With the import functionality, orphan objects and attributes will be incorporated to a new Design Deployment as if they have been set by it.
+Design Builder addresses
 
-The import logic works like this:
+- greenfield use cases by creating new data from a design
+- brownfield use cases by importing existing data related to a new design deployment
 
-1. If the object that we reference doesn't exist, normal design creation logic applies.
+In the "deployment" mode, a design deployment tracks all the objects and attributes that are "owned" by it. With the import functionality, orphan objects and attributes will be incorporated into a new design deployment as if they have been set by it.
 
-2. If an object that we want to "create" already exists
-   2.1. If it's not owned by another design deployment, we get "full_control" of it, and of all the attributes that we define (including the identifiers).
-   2.2. If it's owned, the process fails with an exception because the intention of "create" is to have ownership.
+The import logic works like this:
 
+1. If the object that we reference doesn't exist, normal design creation logic applies
+2. If an object that we want to "create" already exists, normal design creation logic _also_ applies
 3. If an object that we want to "create_or_update" already exists
-   3.1. If it's not owned by another design deployment, we get "full_control" of it, and of all the attributes that we define (including the identifiers).
-   3.2. If it has already an owner, we don't claim ownership of the object, but we still may claim the attributes, except the identifiers.
-
+    - If it's not owned by another design deployment, we get "full_control" of it and of all the attributes that we define (including the identifiers)
+    - If it already has an owner, we don't claim ownership of the object, but we still may claim the attributes, except the identifiers
 4. If an object that we want to "update" already exists
-   3.1. There is no claim for full_control ownership.
-   3.2. There is claim for the attributes, except the identifiers.
-
+    - There is no claim for "full_control" ownership
+    - There is a claim for the attributes, except the identifiers
 5. In all cases, the attributes that a design is trying to update are claimed. These attributes can't be claimed by any other design. If so, the import fails pointing to the conflict dependency.
-
-6. The imported changes (attributes) show the same old and new value because we can't infer which was the previous value (in most cases, it would be `null` but we can't be sure).
+6. The imported changes (attributes) show the same old and new value because we can't infer which was the previous value (in most cases, it would be `null` but we can't be sure)

nautobot_design_builder/design.py

@@ -62,7 +62,6 @@ def log(self, model: "ModelInstance"):
         if self.change_set:
             self.change_set.log(model, self.import_mode)
 
-        # TODO: CA - I don't understand this
         if instance.pk not in self.index:
             self.index.add(instance.pk)
 
@@ -130,6 +129,8 @@ class ModelMetadata:  # pylint: disable=too-many-instance-attributes
     CREATE_OR_UPDATE = "create_or_update"
 
     ACTION_CHOICES = [GET, CREATE, UPDATE, CREATE_OR_UPDATE]
+    # Actions that work with import mode
+    IMPORTABLE_ACTION_CHOICES = [UPDATE, CREATE_OR_UPDATE]
 
     def __init__(self, model_instance: "ModelInstance", **kwargs):
         """Initialize the metadata object for a given model instance.
@@ -537,9 +538,7 @@ def _load_instance(self):  # pylint: disable=too-many-branches
             self.instance = self.model_class.objects.get(**query_filter)
             return
 
-        if self.metadata.action in [ModelMetadata.UPDATE, ModelMetadata.CREATE_OR_UPDATE] or (
-            self.metadata.action is ModelMetadata.CREATE and self.environment.import_mode
-        ):
+        if self.metadata.action in [ModelMetadata.UPDATE, ModelMetadata.CREATE_OR_UPDATE]:
             # perform nested lookups. First collect all the
             # query params for top-level relationships, then
             # perform the actual lookup
@@ -566,9 +565,6 @@ def _load_instance(self):  # pylint: disable=too-many-branches
                     field_values[query_param] = model
             try:
                 self.instance = self.relationship_manager.get(**query_filter)
-                if self.environment.import_mode and self.metadata.action != ModelMetadata.UPDATE:
-                    self.metadata.attributes.update(field_values)
-
                 return
             except ObjectDoesNotExist:
                 if self.metadata.action == ModelMetadata.UPDATE:
@@ -782,7 +778,7 @@ def implement_design(self, design: Dict, commit: bool = False):
         try:
             for key, value in design.items():
                 if key in self.model_map and value:
-                    self._create_or_import_objects(self.model_map[key], value)
+                    self._create_objects(self.model_map[key], value)
                 elif key not in self.model_map:
                     raise errors.DesignImplementationError(f"Unknown model key {key} in design")
 
@@ -852,7 +848,8 @@ def resolve_values(self, value: Union[list, dict, str]) -> Any:
                 value[k] = self.resolve_value(item)
         return value
 
-    def _create_or_import_objects(self, model_class: Type[ModelInstance], objects: Union[List[Any], Dict[str, Any]]):
+    # TODO(2.x): Rename to `_create_or_import-objects`
+    def _create_objects(self, model_class: Type[ModelInstance], objects: Union[List[Any], Dict[str, Any]]):
         if isinstance(objects, dict):
             model = model_class(self, objects)
             model.save()

nautobot_design_builder/design_job.py

@@ -81,18 +81,6 @@ def determine_deployment_name(cls, data):
             return data["deployment_name"]
         return data[deployment_name_field]
 
-    @classmethod
-    def determine_import_mode(cls, data):
-        """Determine the import mode, if specified."""
-        if not cls.is_deployment_job():
-            return None
-
-        if "import_mode" not in data:
-            return False
-            # TODO: not sure why not got the default to False from _get_vars
-            # raise DesignImplementationError("No import mode was provided for the deployment.")
-        return data["import_mode"]
-
     @classmethod
     def _get_vars(cls):
         """Retrieve the script variables for the job.
@@ -294,7 +282,7 @@ def _run_in_transaction(self, **kwargs):  # pylint: disable=too-many-branches, t
 
         self.job_result.job_kwargs = {"data": self.serialize_data(data)}
 
-        data["import_mode"] = self.determine_import_mode(data)
+        data["import_mode"] = self.is_deployment_job() and data.get("import_mode", False)
         data["deployment_name"] = self.determine_deployment_name(data)
         change_set, previous_change_set = self._setup_changeset(data["deployment_name"])
         self.log_info(message=f"Building {getattr(self.Meta, 'name')}")
@@ -306,7 +294,7 @@ def _run_in_transaction(self, **kwargs):  # pylint: disable=too-many-branches, t
             import_mode=data["import_mode"],
         )
 
-        if data["import_mode"] and data["deployment_name"]:
+        if data["import_mode"]:
             self.log_info(message=f'Running in import mode for {data["deployment_name"]}.')
 
         design_files = None

nautobot_design_builder/jobs.py

@@ -17,9 +17,9 @@ class DeploymentDecommissioning(Job):
         query_params={"status": "active"},
         description="Design Deployments to decommission.",
     )
-    only_traceability = BooleanVar(
-        description="Only remove the objects traceability, not decommissioning the actual data.",
-        default=False,
+    delete = BooleanVar(
+        description="Actually delete the objects, not just their link to the design delpoyment.",
+        default=True,
     )
 
     class Meta:  # pylint: disable=too-few-public-methods
@@ -31,27 +31,25 @@ class Meta:  # pylint: disable=too-few-public-methods
     def run(self, data, commit):
         """Execute Decommissioning job."""
         deployments = data["deployments"]
-        only_traceability = data["only_traceability"]
+        delete = data["delete"]
 
         self.log_info(
             message=f"Starting decommissioning of design deployments: {', '.join([instance.name for instance in deployments])}",
         )
 
         for deployment in deployments:
-            if only_traceability:
-                message = "Working on resetting traceability for this Design Deployment..."
+            if delete:
+                message = "Working on deleting objects for this Design Deployment."
             else:
-                message = "Working on resetting objects for this Design Deployment..."
+                message = "Working on unlinking objects from this Design Deployment."
             self.log_info(obj=deployment, message=message)
 
-            deployment.decommission(
-                local_logger=get_logger(__name__, self.job_result), only_traceability=only_traceability
-            )
+            deployment.decommission(local_logger=get_logger(__name__, self.job_result), delete=delete)
 
-            if only_traceability:
-                message = f"Traceability for {deployment} has been successfully removed from Nautobot."
-            else:
+            if delete:
                 message = f"{deployment} has been successfully decommissioned from Nautobot."
+            else:
+                message = f"Objects have been successfully unlinked from {deployment}."
 
             self.log_success(message)
 

nautobot_design_builder/models.py

@@ -229,7 +229,7 @@ def __str__(self):
         """Stringify instance."""
         return f"{self.design.name} - {self.name}"
 
-    def decommission(self, *object_ids, local_logger=logger, only_traceability=False):
+    def decommission(self, *object_ids, local_logger=logger, delete=True):
         """Decommission a design instance.
 
         This will reverse the change records for the design instance and
@@ -241,10 +241,10 @@ def decommission(self, *object_ids, local_logger=logger, only_traceability=False
         # Iterate the change sets in reverse order (most recent first) and
         # revert each change set.
         for change_set in self.change_sets.filter(active=True).order_by("-last_updated"):
-            if only_traceability:
-                change_set.deactivate()
-            else:
+            if delete:
                 change_set.revert(*object_ids, local_logger=local_logger)
+            else:
+                change_set.deactivate()
 
         if not object_ids:
             content_type = ContentType.objects.get_for_model(Deployment)
@@ -373,58 +373,50 @@ def log(self, model_instance, import_mode: bool):
             entry.changes.update(model_instance.metadata.changes)
             entry.save()
         except ChangeRecord.DoesNotExist:
-            # Default full_control for created objects
-            full_control = model_instance.metadata.created
-
-            # This boolean signals the intention to claim existing data because
-            # the action is "create_or_update" and is running in import_mode
-            # It assumes that we will "try" to own all the objects, if are not owned
-            intention_to_full_control_by_importing = (
-                model_instance.metadata.action in ["create_or_update", "create"] and import_mode
-            )
-            intention_to_import = (
-                model_instance.metadata.action in ["create_or_update", "create", "update"] and import_mode
-            )
+            entry_parameters = {
+                "_design_object_type": content_type,
+                "_design_object_id": instance.id,
+                "changes": model_instance.metadata.changes,
+                "full_control": model_instance.metadata.created,
+                "index": self._next_index(),
+            }
+            # Deferred import as otherwise Nautobot doesn't start
+            from .design import ModelMetadata  # pylint: disable=import-outside-toplevel,cyclic-import
+
+            # Path when not importing, either because it's not enabled or the action is not supported for importing.
+            if not import_mode or model_instance.metadata.action not in ModelMetadata.IMPORTABLE_ACTION_CHOICES:
+                entry = self.records.create(**entry_parameters)
+                return entry
+
+            # When we have intention to claim ownership (i.e. the action is CREATE_OR_UPDATE) we first try to obtain
+            # `full_control` over the object, thus pretending that we have created it.
+            # If the object is already owned with full_control by another Design Deployment,
+            # we acknowledge it and set `full_control` to `False`.
+            # TODO: Shouldn't this change record object also need to be active?
+            change_records_for_instance = ChangeRecord.objects.filter_by_design_object_id(_design_object_id=instance.id)
+            if model_instance.metadata.action == ModelMetadata.CREATE_OR_UPDATE:
+                entry_parameters["full_control"] = not change_records_for_instance.filter(full_control=True).exists()
+
+            # When we don't want to assume full control, make sure we don't try to own any of the query filter values.
+            # We do this by removing any query filter values from the `changes` dictionary, which is the structure that
+            # defines which attributes are owned by the deployment.
+            if not entry_parameters["full_control"]:
+                for attribute in model_instance.metadata.query_filter_values:
+                    entry_parameters["changes"].pop(attribute, None)
+
+            # Check if any owned attributes exist that conflict with the changes for this instance.
+            # We do this by iterating over all change records that exist for this instance, ...
+            for record in change_records_for_instance:
+                # ...iterating over all attributes in those instances changes...
+                for attribute in record.changes:
+                    # ...and, finally, by raising an error if any of those overlap with those attributes that we are
+                    # trying to own by importing the object.
+                    if attribute in entry_parameters["changes"]:
+                        raise ValueError(  # pylint: disable=raise-missing-from
+                            f"The {attribute} attribute for {instance} is already owned by Design Deployment {record.change_set.deployment}"
+                        )
 
-            # When we have intention to claim ownership, the first try is to get a full_control
-            # of the object, in fact, assume that we would have created it.
-            # If the object was already owned with full_control by another Design Deployment,
-            # we acknowledge it and set it to full_control=False, if not, True.
-
-            change_record = ChangeRecord.objects.filter_by_design_object_id(
-                _design_object_id=instance.id, full_control=True
-            ).first()
-            if intention_to_full_control_by_importing and change_record:
-                if model_instance.metadata.action == "create":
-                    raise ValueError(  # pylint: disable=raise-missing-from
-                        f"The design requires importing {instance} but is already owned by Design Deployment {change_record.change_set.deployment}"
-                    )
-                full_control = False
-            elif intention_to_full_control_by_importing:
-                full_control = True
-
-            # Independently of having full_control or not, we check that all the attributes
-            # we claim as ours are not tracked by another design
-            if intention_to_import:
-                if not full_control:
-                    for attribute in model_instance.metadata.query_filter_values:
-                        if attribute in model_instance.metadata.changes:
-                            del model_instance.metadata.changes[attribute]
-
-                for record in ChangeRecord.objects.filter_by_design_object_id(_design_object_id=instance.id):
-                    for attribute in record.changes:
-                        if attribute in model_instance.metadata.changes:
-                            raise ValueError(  # pylint: disable=raise-missing-from
-                                f"The {attribute} attribute for {instance} is already owned by Design Deployment {record.change_set.deployment}"
-                            )
-
-            entry = self.records.create(
-                _design_object_type=content_type,
-                _design_object_id=instance.id,
-                changes=model_instance.metadata.changes,
-                full_control=full_control,
-                index=self._next_index(),
-            )
+            entry = self.records.create(**entry_parameters)
         return entry
 
     def revert(self, *object_ids, local_logger: logging.Logger = logger):

nautobot_design_builder/tests/designs/templates/simple_design_4.yaml.j2

@@ -1,3 +1,19 @@
 ---
 manufacturers:
-    "name": "Test Manufacturer"
+  - "!create_or_update:name": "Test Manufacturer"
+    "description": "Test description"
+device_roles:
+  - "!create_or_update:name": "Switch"
+    "description": "Test description"
+device_types:
+  - "!create_or_update:model": "Test Device Type"
+    "manufacturer__name": "Test Manufacturer"
+sites:
+  - "!create_or_update:name": "Test Site"
+    "status__name": "Active"
+devices:
+  - "!create_or_update:name": "Test Device"
+    "device_role__name": "Switch"
+    "device_type__model": "Test Device Type"
+    "site__name": "Test Site"
+    "status__name": "Active"

nautobot_design_builder/tests/designs/test_designs.py

@@ -112,11 +112,11 @@ class Meta:  # pylint: disable=too-few-public-methods
         design_mode = DesignModeChoices.DEPLOYMENT
 
 
-class SimpleDesignDeploymentModeCreate(DesignJob):
-    """Simple design job in deployment mode for 'create'."""
+class SimpleDesignDeploymentModeMultipleObjects(DesignJob):
+    """Simple design job in deployment mode with multiple objects."""
 
     class Meta:  # pylint: disable=too-few-public-methods
-        name = "Simple Design in deployment mode with create"
+        name = "Simple Design in deployment mode with multiple objects." ""
         design_file = "templates/simple_design_4.yaml.j2"
         design_mode = DesignModeChoices.DEPLOYMENT