From a770899a50c24a6c702d48b7bdaaedee746c95f6 Mon Sep 17 00:00:00 2001 From: Christian Adell Date: Thu, 10 Oct 2024 16:32:11 +0200 Subject: [PATCH 1/4] feat: :sparkles: Import mode for design lifecycle --- .yamllint.yml | 1 + docs/user/design_lifecycle.md | 26 ++++++ nautobot_design_builder/design.py | 20 +++- nautobot_design_builder/design_job.py | 11 ++- nautobot_design_builder/fields.py | 3 +- nautobot_design_builder/jobs.py | 26 +++++- nautobot_design_builder/models.py | 92 ++++++++++++++++--- .../designs/templates/simple_design_1.yaml.j2 | 4 + .../designs/templates/simple_design_4.yaml.j2 | 28 ++++++ .../designs/templates/simple_design_5.yaml.j2 | 4 + .../tests/designs/test_designs.py | 30 ++++++ .../tests/test_decommissioning_job.py | 37 ++++++-- .../tests/test_design_job.py | 71 +++++++++++++- 13 files changed, 319 insertions(+), 34 deletions(-) create mode 100644 nautobot_design_builder/tests/designs/templates/simple_design_1.yaml.j2 create mode 100644 nautobot_design_builder/tests/designs/templates/simple_design_4.yaml.j2 create mode 100644 nautobot_design_builder/tests/designs/templates/simple_design_5.yaml.j2 diff --git a/.yamllint.yml b/.yamllint.yml index 8cc3e9a9..31488204 100644 --- a/.yamllint.yml +++ b/.yamllint.yml @@ -11,3 +11,4 @@ rules: ignore: | .venv/ compose.yaml + .vscode/ diff --git a/docs/user/design_lifecycle.md b/docs/user/design_lifecycle.md index 2584c0fe..6dbd2dbb 100644 --- a/docs/user/design_lifecycle.md +++ b/docs/user/design_lifecycle.md @@ -65,3 +65,29 @@ This feature allows to rollback all the changes implemented by a design instance The decommissioning feature takes into account potential dependencies between design implementations. For example, if a new l3vpn design depends on devices that were created by another design, this previous design won't be decommissioned until the l3vpn dependencies are also decommissioned to warrant consistency. Once a design instance is decommissioned, it's still visible in the API/UI to check the history of changes but without any active relationship with Nautobot objects. After decommissioning, the design instance can be deleted completely from Nautobot. + +There is a decommissioning mode to only remove the link between the design objects and the design deployment without actually reverting the state of the objects. Decommissioning, with the `delete` checkbox _not_ set, is only removing the references but keeping the data. + + + +### Design Deployment Import + +Design Builder addresses + +- greenfield use cases by creating new data from a design +- brownfield use cases by importing existing data related to a new design 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 into a new design deployment as if they have been set by it. + +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 + - 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 + - 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) diff --git a/nautobot_design_builder/design.py b/nautobot_design_builder/design.py index 13f5f163..476e30ce 100644 --- a/nautobot_design_builder/design.py +++ b/nautobot_design_builder/design.py @@ -43,12 +43,13 @@ class Journal: will only be in each of those indices at most once. """ - def __init__(self, change_set: models.ChangeSet = None): + def __init__(self, change_set: models.ChangeSet = None, import_mode: bool = False): """Constructor for Journal object.""" self.index = set() self.created = defaultdict(set) self.updated = defaultdict(set) self.change_set = change_set + self.import_mode = import_mode def log(self, model: "ModelInstance"): """Log that a model has been created or updated. @@ -59,7 +60,7 @@ def log(self, model: "ModelInstance"): instance = model.design_instance model_type = instance.__class__ if self.change_set: - self.change_set.log(model) + self.change_set.log(model, self.import_mode) if instance.pk not in self.index: self.index.add(instance.pk) @@ -128,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", environment: "Environment", **kwargs): """Initialize the metadata object for a given model instance. @@ -702,7 +705,11 @@ def __new__(cls, *args, **kwargs): return object.__new__(cls) def __init__( - self, logger: logging.Logger = None, extensions: List[ext.Extension] = None, change_set: models.ChangeSet = None + self, + logger: logging.Logger = None, + extensions: List[ext.Extension] = None, + change_set: models.ChangeSet = None, + import_mode=False, ): """Create a new build environment for implementing designs. @@ -717,6 +724,8 @@ def __init__( log any changes to the database. This behavior is used when a design is in Ad-Hoc mode (classic mode) and does not represent a design lifecycle. + import_mode (bool): Whether or not the environment is in import mode. Defaults to False. + Raises: errors.DesignImplementationError: If a provided extension is not a subclass of `ext.Extension`. @@ -725,6 +734,8 @@ def __init__( if self.logger is None: self.logger = logging.getLogger(__name__) + self.import_mode = import_mode + self.extensions = { "extensions": [], "attribute": {}, @@ -748,7 +759,7 @@ def __init__( self.extensions["extensions"].append(extn) - self.journal = Journal(change_set=change_set) + self.journal = Journal(change_set=change_set, import_mode=import_mode) if change_set: self.deployment = change_set.deployment @@ -871,6 +882,7 @@ def resolve_values(self, value: Union[list, dict, str]) -> Any: value[k] = self.resolve_value(item) return value + # IDEA: rename to `_create_or_import_objects` to better reflect the import mode def _create_objects(self, model_class: Type[ModelInstance], objects: Union[List[Any], Dict[str, Any]]): if isinstance(objects, dict): model = model_class(self, objects) diff --git a/nautobot_design_builder/design_job.py b/nautobot_design_builder/design_job.py index 5cdfe427..282b120d 100644 --- a/nautobot_design_builder/design_job.py +++ b/nautobot_design_builder/design_job.py @@ -15,7 +15,7 @@ from jinja2 import TemplateError from nautobot.extras.models import Status -from nautobot.apps.jobs import Job, DryRunVar, StringVar +from nautobot.apps.jobs import Job, DryRunVar, StringVar, BooleanVar from nautobot.extras.models import FileProxy from nautobot.extras.jobs import JobForm @@ -79,7 +79,7 @@ def determine_deployment_name(cls, data): deployment_name_field = cls.deployment_name_field() if deployment_name_field is None: if "deployment_name" not in data: - raise DesignImplementationError("No instance name was provided for the deployment.") + raise DesignImplementationError("No Deployment name was provided for the deployment.") return data["deployment_name"] return data[deployment_name_field] @@ -97,6 +97,8 @@ def _get_vars(cls): label="Deployment Name", max_length=models.DESIGN_NAME_MAX_LENGTH, ) + cls_vars["import_mode"] = BooleanVar(label="Import Mode", default=False) + cls_vars.update(super()._get_vars()) return cls_vars @@ -275,6 +277,7 @@ def _run_in_transaction(self, dryrun: bool, **data): # pylint: disable=too-many design_files = None + 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"]) @@ -286,8 +289,12 @@ def _run_in_transaction(self, dryrun: bool, **data): # pylint: disable=too-many logger=self.logger, extensions=extensions, change_set=change_set, + import_mode=data["import_mode"], ) + if data["import_mode"]: + self.logger.info("Running in import mode for %s", data["deployment_name"]) + design_files = None if hasattr(self.Meta, "context_class"): diff --git a/nautobot_design_builder/fields.py b/nautobot_design_builder/fields.py index 9e37f766..c0677e6b 100644 --- a/nautobot_design_builder/fields.py +++ b/nautobot_design_builder/fields.py @@ -79,7 +79,8 @@ def change_log(model_instance: "ModelInstance", attr_name: str): old_value = _get_change_value(getattr(model_instance.design_instance, attr_name)) yield new_value = _get_change_value(getattr(model_instance.design_instance, attr_name)) - if old_value != new_value: + + if old_value != new_value or model_instance.design_metadata.import_mode: if isinstance(old_value, set): model_instance.design_metadata.changes[attr_name] = { "old_items": old_value, diff --git a/nautobot_design_builder/jobs.py b/nautobot_design_builder/jobs.py index 1796ecb8..152a63e2 100644 --- a/nautobot_design_builder/jobs.py +++ b/nautobot_design_builder/jobs.py @@ -1,6 +1,6 @@ """Generic Design Builder Jobs.""" -from nautobot.apps.jobs import Job, MultiObjectVar, register_jobs +from nautobot.apps.jobs import Job, MultiObjectVar, register_jobs, BooleanVar from .models import Deployment @@ -17,13 +17,18 @@ class DeploymentDecommissioning(Job): description="Design Deployments to decommission.", ) + delete = BooleanVar( + description="Actually delete the objects, not just their link to the design deployment.", + default=True, + ) + class Meta: # pylint: disable=too-few-public-methods """Meta class.""" name = "Decommission Design Deployments" description = """Job to decommission one or many Design Deployments from Nautobot.""" - def run(self, deployments): # pylint:disable=arguments-differ + def run(self, deployments, delete): # pylint:disable=arguments-differ """Execute Decommissioning job.""" self.logger.info( "Starting decommissioning of design deployments: %s", @@ -31,9 +36,20 @@ def run(self, deployments): # pylint:disable=arguments-differ ) for deployment in deployments: - self.logger.info("Working on resetting objects for this Design Instance...", extra={"object": deployment}) - deployment.decommission(local_logger=self.logger) - self.logger.info("%s has been successfully decommissioned from Nautobot.", deployment) + if delete: + message = "Working on deleting objects for this Design Deployment." + else: + message = "Working on unlinking objects from this Design Deployment." + self.logger.info(message, extra={"object": deployment}) + + deployment.decommission(local_logger=self.logger, delete=delete) + + if delete: + message = f"{deployment} has been successfully decommissioned from Nautobot." + else: + message = f"Objects have been successfully unlinked from {deployment}." + + self.logger.info(message) register_jobs(DeploymentDecommissioning) diff --git a/nautobot_design_builder/models.py b/nautobot_design_builder/models.py index 483cbe66..9d4704bb 100644 --- a/nautobot_design_builder/models.py +++ b/nautobot_design_builder/models.py @@ -1,7 +1,8 @@ """Collection of models that DesignBuilder uses to track design implementations.""" import logging -from typing import List +from typing import List, Optional +from uuid import UUID from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes import fields as ct_fields from django.core.exceptions import ValidationError, ObjectDoesNotExist @@ -239,7 +240,7 @@ def __str__(self): """Stringify instance.""" return f"{self.design.name} - {self.name}" - def decommission(self, *object_ids, local_logger=logger): + 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 @@ -251,7 +252,10 @@ def decommission(self, *object_ids, local_logger=logger): # 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"): - change_set.revert(*object_ids, local_logger=local_logger) + 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) @@ -350,7 +354,7 @@ def _next_index(self): setattr(self, "_index", index) return index - def log(self, model_instance): + def log(self, model_instance, import_mode: bool): """Log changes to a model instance. This will log the differences between a model instance's @@ -361,6 +365,7 @@ def log(self, model_instance): Args: model_instance: Model instance to log changes. + import_mode: Boolean used to import design objects already present in the database. """ # Note: We always need to create a change record, even when there # are no individual attribute changes. Change records that don't @@ -380,13 +385,53 @@ def log(self, model_instance): entry.changes.update(model_instance.design_metadata.changes) entry.save() except ChangeRecord.DoesNotExist: - entry = self.records.create( - _design_object_type=content_type, - _design_object_id=instance.id, - changes=model_instance.design_metadata.changes, - full_control=model_instance.design_metadata.created, - index=self._next_index(), - ) + entry_parameters = { + "_design_object_type": content_type, + "_design_object_id": instance.id, + "changes": model_instance.design_metadata.changes, + "full_control": model_instance.design_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.design_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.design_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.design_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}" + ) + + entry = self.records.create(**entry_parameters) + + # TODO: not sure about this return... + return entry def revert(self, *object_ids, local_logger: logging.Logger = logger): """Revert the changes represented in this ChangeSet. @@ -443,6 +488,15 @@ def __sub__(self, other: "ChangeSet"): .values_list("_design_object_id", flat=True) ) + # TODO: check if this method is it's necessary + def deactivate(self): + """Mark the change_set and its records as not active.""" + self.active = False + for change_set_record in self.records.all(): + change_set_record.active = False + change_set_record.save() + self.save() + class ChangeRecordQuerySet(RestrictedQuerySet): """Queryset for `ChangeRecord` objects.""" @@ -511,6 +565,22 @@ def design_objects(self, deployment: "Deployment"): } return self.filter(id__in=design_object_ids.values()) + def filter_by_design_object_id(self, _design_object_id: UUID, full_control: Optional[bool] = None): + """Lookup all the active records for a design object ID and an full_control. + + Args: + _design_object_id (UUID): The design object UUID. + full_control (type, optional): Include the full_control filter. Defaults to None. + + Returns: + Query set matching the options. + """ + if full_control is not None: + queryset = self.filter(_design_object_id=_design_object_id, active=True, full_control=full_control) + else: + queryset = self.filter(_design_object_id=_design_object_id, active=True) + return queryset.exclude_decommissioned() + class ChangeRecord(BaseModel): """A single entry in the change set for exactly 1 object. diff --git a/nautobot_design_builder/tests/designs/templates/simple_design_1.yaml.j2 b/nautobot_design_builder/tests/designs/templates/simple_design_1.yaml.j2 new file mode 100644 index 00000000..28871b67 --- /dev/null +++ b/nautobot_design_builder/tests/designs/templates/simple_design_1.yaml.j2 @@ -0,0 +1,4 @@ +--- +manufacturers: + - "!create_or_update:name": "Test Manufacturer" + "description": "Test description" diff --git a/nautobot_design_builder/tests/designs/templates/simple_design_4.yaml.j2 b/nautobot_design_builder/tests/designs/templates/simple_design_4.yaml.j2 new file mode 100644 index 00000000..67ec2b6e --- /dev/null +++ b/nautobot_design_builder/tests/designs/templates/simple_design_4.yaml.j2 @@ -0,0 +1,28 @@ +--- +manufacturers: + - "!create_or_update:name": "Test Manufacturer" + "description": "Test description" +roles: + - "!create_or_update:name": "Switch" + color: "3f51b5" + content_types: + - "!get:app_label": "dcim" + "!get:model": "device" +device_types: + - "!create_or_update:model": "Test Device Type" + "manufacturer__name": "Test Manufacturer" +location_types: + - "!create_or_update:name": "Test Location Type" + content_types: + - "!get:app_label": "dcim" + "!get:model": "device" +locations: + - "!create_or_update:name": "Test Location" + "location_type__name": "Test Location Type" + "status__name": "Active" +devices: + - "!create_or_update:name": "Test Device" + "role__name": "Switch" + "device_type__model": "Test Device Type" + "location__name": "Test Location" + "status__name": "Active" diff --git a/nautobot_design_builder/tests/designs/templates/simple_design_5.yaml.j2 b/nautobot_design_builder/tests/designs/templates/simple_design_5.yaml.j2 new file mode 100644 index 00000000..6de9ce32 --- /dev/null +++ b/nautobot_design_builder/tests/designs/templates/simple_design_5.yaml.j2 @@ -0,0 +1,4 @@ +--- +manufacturers: + - "!update:name": "Test Manufacturer" + "description": "Test description" diff --git a/nautobot_design_builder/tests/designs/test_designs.py b/nautobot_design_builder/tests/designs/test_designs.py index b9454814..981f46ec 100644 --- a/nautobot_design_builder/tests/designs/test_designs.py +++ b/nautobot_design_builder/tests/designs/test_designs.py @@ -120,6 +120,33 @@ class Meta: # pylint: disable=too-few-public-methods design_file = "templates/design_with_validation_error.yaml.j2" +class SimpleDesignDeploymentMode(DesignJob): + """Simple design job in deployment mode.""" + + class Meta: # pylint: disable=too-few-public-methods + name = "Simple Design in deployment mode with create_or_update" + design_file = "templates/simple_design_1.yaml.j2" + design_mode = DesignModeChoices.DEPLOYMENT + + +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 multiple objects." "" + design_file = "templates/simple_design_4.yaml.j2" + design_mode = DesignModeChoices.DEPLOYMENT + + +class SimpleDesignDeploymentModeUpdate(DesignJob): + """Simple design job in deployment mode for 'update'.""" + + class Meta: # pylint: disable=too-few-public-methods + name = "Simple Design in deployment mode with update" + design_file = "templates/simple_design_5.yaml.j2" + design_mode = DesignModeChoices.DEPLOYMENT + + class NextInterfaceExtension(AttributeExtension): """Attribute extension to calculate the next available interface name.""" @@ -202,4 +229,7 @@ class Meta: # pylint: disable=too-few-public-methods DesignWithRefError, DesignWithValidationError, IntegrationDesign, + SimpleDesignDeploymentMode, + SimpleDesignDeploymentModeMultipleObjects, + SimpleDesignDeploymentModeUpdate, ) diff --git a/nautobot_design_builder/tests/test_decommissioning_job.py b/nautobot_design_builder/tests/test_decommissioning_job.py index 07f41339..c1d46488 100644 --- a/nautobot_design_builder/tests/test_decommissioning_job.py +++ b/nautobot_design_builder/tests/test_decommissioning_job.py @@ -102,7 +102,7 @@ def test_basic_decommission_run_with_full_control(self): ) change_record.validated_save() - self.job.run(deployments=[self.deployment]) + self.job.run(deployments=[self.deployment], delete=True) self.assertEqual(0, Secret.objects.count()) @@ -131,6 +131,7 @@ def test_decommission_run_with_dependencies(self): ValueError, self.job.run, deployments=[self.deployment], + delete=True, ) self.assertEqual(1, Secret.objects.count()) @@ -158,7 +159,7 @@ def test_decommission_run_with_dependencies_but_decommissioned(self): self.deployment_2.decommission() - self.job.run(deployments=[self.deployment]) + self.job.run(deployments=[self.deployment], delete=True) self.assertEqual(0, Secret.objects.count()) @@ -174,7 +175,7 @@ def test_basic_decommission_run_without_full_control(self): ) record_1.validated_save() - self.job.run(deployments=[self.deployment]) + self.job.run(deployments=[self.deployment], delete=True) self.assertEqual(1, Secret.objects.count()) @@ -193,7 +194,7 @@ def test_decommission_run_without_full_control_string_value(self): ) record.validated_save() - self.job.run(deployments=[self.deployment]) + self.job.run(deployments=[self.deployment], delete=True) self.assertEqual(1, Secret.objects.count()) self.assertEqual("previous description", Secret.objects.first().description) @@ -210,7 +211,7 @@ def test_decommission_run_without_full_control_dict_value_with_overlap(self): ) record.validated_save() - self.job.run(deployments=[self.deployment]) + self.job.run(deployments=[self.deployment], delete=True) self.assertEqual(self.initial_params, Secret.objects.first().parameters) @@ -229,7 +230,7 @@ def test_decommission_run_without_full_control_dict_value_without_overlap(self): ) record.validated_save() - self.job.run(deployments=[self.deployment]) + self.job.run(deployments=[self.deployment], delete=True) self.assertEqual(self.initial_params, Secret.objects.first().parameters) @@ -256,7 +257,7 @@ def test_decommission_run_without_full_control_dict_value_with_new_values_and_ol self.secret.parameters = {**self.changed_params, **new_params} self.secret.validated_save() - self.job.run(deployments=[self.deployment]) + self.job.run(deployments=[self.deployment], delete=True) self.assertEqual({**self.initial_params, **new_params}, Secret.objects.first().parameters) @@ -272,7 +273,7 @@ def test_decommission_run_with_pre_hook_pass(self): ) change_record_1.validated_save() - self.job.run(deployments=[self.deployment]) + self.job.run(deployments=[self.deployment], delete=True) self.assertEqual(0, Secret.objects.count()) models.Deployment.pre_decommission.disconnect(fake_ok) @@ -292,6 +293,7 @@ def test_decommission_run_with_pre_hook_fail(self): DesignValidationError, self.job.run, deployments=[self.deployment], + delete=True, ) self.assertEqual(1, Secret.objects.count()) @@ -323,6 +325,23 @@ def test_decommission_run_multiple_deployment(self): self.assertEqual(2, Secret.objects.count()) - self.job.run(deployments=[self.deployment, self.deployment_2]) + self.job.run(deployments=[self.deployment, self.deployment_2], delete=True) self.assertEqual(0, Secret.objects.count()) + + def test_decommission_run_without_delete(self): + self.assertEqual(1, Secret.objects.count()) + + record = models.ChangeRecord.objects.create( + change_set=self.change_set1, + design_object=self.secret, + full_control=True, + index=self.change_set1._next_index(), # pylint:disable=protected-access + ) + record.validated_save() + + self.job.run(deployments=[self.deployment], delete=False) + + self.assertEqual(1, Secret.objects.count()) + record.refresh_from_db() + self.assertEqual(False, record.active) diff --git a/nautobot_design_builder/tests/test_design_job.py b/nautobot_design_builder/tests/test_design_job.py index 33b1464e..abf95d7b 100644 --- a/nautobot_design_builder/tests/test_design_job.py +++ b/nautobot_design_builder/tests/test_design_job.py @@ -1,7 +1,7 @@ """Test running design jobs.""" import copy -from unittest.mock import patch, Mock, ANY +from unittest.mock import patch, Mock, ANY, MagicMock from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ValidationError @@ -10,7 +10,7 @@ from nautobot.ipam.models import VRF, Prefix, IPAddress from nautobot.extras.models import Status, Role -from nautobot_design_builder.models import Deployment +from nautobot_design_builder.models import Deployment, ChangeRecord from nautobot_design_builder.errors import DesignImplementationError, DesignValidationError from nautobot_design_builder.tests import DesignTestCase from nautobot_design_builder.tests.designs import test_designs @@ -98,8 +98,75 @@ def test_custom_extensions(self, environment: Mock): logger=job.logger, extensions=test_designs.DesignJobWithExtensions.Meta.extensions, change_set=ANY, + import_mode=False, ) + def test_import_design_create_or_update(self): + """Confirm that existing data can be imported with 'create_or_update'.""" + job = self.get_mocked_job(test_designs.SimpleDesignDeploymentMode) + + # The object to be imported by the design deployment already exists + manufacturer = Manufacturer.objects.create(name="Test Manufacturer") + self.data["import_mode"] = True + self.data["deployment_name"] = "deployment name example" + job.run(dryrun=False, **self.data) + self.assertEqual(Deployment.objects.first().name, "deployment name example") + self.assertEqual(ChangeRecord.objects.first().design_object, manufacturer) + self.assertEqual(ChangeRecord.objects.first().design_object.description, "Test description") + + # Running the import twice for a 'create_or_update' operation should raise an exception + job = self.get_mocked_job(test_designs.SimpleDesignDeploymentMode) + self.data["deployment_name"] = "another deployment name example" + with self.assertRaises(ValueError) as error: + job.run(dryrun=False, **self.data) + self.assertEqual( + str(error.exception), + "The description attribute for Test Manufacturer is already owned by Design Deployment Simple Design in deployment mode with create_or_update - deployment name example", + ) + + def test_import_design_update(self): + """Confirm that existing data can be imported with 'update'.""" + job = self.get_mocked_job(test_designs.SimpleDesignDeploymentModeUpdate) + + # The object to be imported by the design deployment already exists + manufacturer = Manufacturer.objects.create(name="Test Manufacturer", description="old description") + self.data["import_mode"] = True + self.data["deployment_name"] = "deployment name example" + job.run(dryrun=False, **self.data) + self.assertEqual(Deployment.objects.first().name, "deployment name example") + self.assertEqual(ChangeRecord.objects.first().design_object, manufacturer) + self.assertEqual(ChangeRecord.objects.first().design_object.description, "Test description") + + # Running the import twice for a 'update' operation should raise an exception when attribute conflict + job = self.get_mocked_job(test_designs.SimpleDesignDeploymentModeUpdate) + self.data["deployment_name"] = "another deployment name example" + with self.assertRaises(ValueError) as error: + job.run(dryrun=False, **self.data) + self.assertEqual( + str(error.exception), + "The description attribute for Test Manufacturer is already owned by Design Deployment Simple Design in deployment mode with update - deployment name example", + ) + + def test_import_design_multiple_objects(self): + """Confirming that multiple, interrelated objects can be imported.""" + job = self.get_mocked_job(test_designs.SimpleDesignDeploymentModeMultipleObjects) + + # Create data initially + self.data["deployment_name"] = "I will be deleted" + job.run(dryrun=False, **self.data) + + # Unlink the objects from the deployment so that they can be re-imported + deployment = Deployment.objects.get(name=self.data["deployment_name"]) + deployment.decommission(local_logger=MagicMock(), delete=False) + deployment.delete() + + self.data["import_mode"] = True + self.data["deployment_name"] = "I will persist" + job.run(dryrun=False, **self.data) + + self.assertEqual(ChangeRecord.objects.count(), 8) + self.assertTrue(ChangeRecord.objects.filter_by_design_object_id(Device.objects.first().pk).exists()) + class TestDesignJobLogging(DesignTestCase): """Test that the design job logs errors correctly.""" From 3cbce7b44505b0b6f0d18a97fb3f1a3e8e180e0a Mon Sep 17 00:00:00 2001 From: Christian Adell Date: Fri, 11 Oct 2024 11:19:32 +0200 Subject: [PATCH 2/4] add change description --- changes/196.added | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/196.added diff --git a/changes/196.added b/changes/196.added new file mode 100644 index 00000000..1f7ce166 --- /dev/null +++ b/changes/196.added @@ -0,0 +1 @@ +Import feature for deployment mode that provides incorporating existing data into a design deployment when the identifiers match. From 28ee56ae321f2493ad9549ff07bc733a30d26698 Mon Sep 17 00:00:00 2001 From: Christian Adell Date: Fri, 11 Oct 2024 13:52:46 +0200 Subject: [PATCH 3/4] Apply suggestions from code review Co-authored-by: Andrew Bates --- nautobot_design_builder/design_job.py | 2 +- nautobot_design_builder/jobs.py | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/nautobot_design_builder/design_job.py b/nautobot_design_builder/design_job.py index 282b120d..a94e6086 100644 --- a/nautobot_design_builder/design_job.py +++ b/nautobot_design_builder/design_job.py @@ -79,7 +79,7 @@ def determine_deployment_name(cls, data): deployment_name_field = cls.deployment_name_field() if deployment_name_field is None: if "deployment_name" not in data: - raise DesignImplementationError("No Deployment name was provided for the deployment.") + raise DesignImplementationError("No name was provided for the deployment.") return data["deployment_name"] return data[deployment_name_field] diff --git a/nautobot_design_builder/jobs.py b/nautobot_design_builder/jobs.py index 152a63e2..a039571d 100644 --- a/nautobot_design_builder/jobs.py +++ b/nautobot_design_builder/jobs.py @@ -37,19 +37,17 @@ def run(self, deployments, delete): # pylint:disable=arguments-differ for deployment in deployments: if delete: - message = "Working on deleting objects for this Design Deployment." + message = "Deleting objects for this Design Deployment." else: - message = "Working on unlinking objects from this Design Deployment." + message = "Unlinking objects from this Design Deployment." self.logger.info(message, extra={"object": deployment}) deployment.decommission(local_logger=self.logger, delete=delete) if delete: - message = f"{deployment} has been successfully decommissioned from Nautobot." + self.logger.info("%s has been successfully decommissioned from Nautobot.", deployment) else: - message = f"Objects have been successfully unlinked from {deployment}." - - self.logger.info(message) + self.logger.info("Objects have been successfully unlinked from %s", deployment) register_jobs(DeploymentDecommissioning) From e46fa4cb665ddad8c261125a51314251c4c3ac67 Mon Sep 17 00:00:00 2001 From: Christian Adell Date: Mon, 14 Oct 2024 09:51:34 +0200 Subject: [PATCH 4/4] Adjust tests --- nautobot_design_builder/models.py | 10 +++------- nautobot_design_builder/tests/test_design_job.py | 4 ++++ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/nautobot_design_builder/models.py b/nautobot_design_builder/models.py index f3065f74..dc2c16a2 100644 --- a/nautobot_design_builder/models.py +++ b/nautobot_design_builder/models.py @@ -399,8 +399,8 @@ def log(self, model_instance, import_mode: bool): # 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.design_metadata.action not in ModelMetadata.IMPORTABLE_ACTION_CHOICES: - entry = self.records.create(**entry_parameters) - return entry + self.records.create(**entry_parameters) + return # 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. @@ -430,10 +430,7 @@ def log(self, model_instance, import_mode: bool): f"The {attribute} attribute for {instance} is already owned by Design Deployment {record.change_set.deployment}" ) - entry = self.records.create(**entry_parameters) - - # TODO: not sure about this return... - return entry + self.records.create(**entry_parameters) def revert(self, *object_ids, local_logger: logging.Logger = logger): """Revert the changes represented in this ChangeSet. @@ -490,7 +487,6 @@ def __sub__(self, other: "ChangeSet"): .values_list("_design_object_id", flat=True) ) - # TODO: check if this method is it's necessary def deactivate(self): """Mark the change_set and its records as not active.""" self.active = False diff --git a/nautobot_design_builder/tests/test_design_job.py b/nautobot_design_builder/tests/test_design_job.py index abf95d7b..05ca7f0e 100644 --- a/nautobot_design_builder/tests/test_design_job.py +++ b/nautobot_design_builder/tests/test_design_job.py @@ -117,6 +117,8 @@ def test_import_design_create_or_update(self): # Running the import twice for a 'create_or_update' operation should raise an exception job = self.get_mocked_job(test_designs.SimpleDesignDeploymentMode) self.data["deployment_name"] = "another deployment name example" + manufacturer.description = "new description to show changes" + manufacturer.save() with self.assertRaises(ValueError) as error: job.run(dryrun=False, **self.data) self.assertEqual( @@ -140,6 +142,8 @@ def test_import_design_update(self): # Running the import twice for a 'update' operation should raise an exception when attribute conflict job = self.get_mocked_job(test_designs.SimpleDesignDeploymentModeUpdate) self.data["deployment_name"] = "another deployment name example" + manufacturer.description = "new description to show changes" + manufacturer.save() with self.assertRaises(ValueError) as error: job.run(dryrun=False, **self.data) self.assertEqual(