From 4ba41953c1c8f85553b47f2e1d281e8b9e3de704 Mon Sep 17 00:00:00 2001 From: Louis Date: Wed, 5 Aug 2026 15:01:28 +0200 Subject: [PATCH] fix: reconcile rack placement conflicts during import --- README.md | 6 + netbox_export/__init__.py | 2 +- netbox_export/services/exporter.py | 2 +- netbox_export/services/importer.py | 180 +++++++++++++++++- pyproject.toml | 2 +- tests/test_device_placement.py | 291 +++++++++++++++++++++++++++++ 6 files changed, 479 insertions(+), 4 deletions(-) create mode 100644 tests/test_device_placement.py diff --git a/README.md b/README.md index bb96ea2..729b40c 100644 --- a/README.md +++ b/README.md @@ -129,6 +129,12 @@ Bei neuen NetBox-Modulen wird die automatische Komponentenreplikation deaktiviert. Ports, Interfaces und Bays werden stattdessen ausschließlich aus den Archivdatensätzen angelegt beziehungsweise vorhandenen Komponenten zugeordnet. +Geräte werden in einer separaten Abschlussphase im Rack platziert, damit auch +Positionswechsel ohne temporäre Doppelbelegung funktionieren. Bei fremden +Belegungen löst **Aktualisieren** das Zielgerät mit Warnung von seiner Position, +**Überspringen** lässt das importierte Gerät positionslos und **Import abbrechen** +meldet den Rackplatzkonflikt vor dem Datenbankfehler. Mehr-U- und +Full-Depth-Belegungen werden dabei berücksichtigt. Auf Quelle und Ziel müssen jeweils dieselben Plugin-Versionen und Migrationen installiert sein. Verschlüsselte Zugangsdaten von NetBox-VM-Import sind nur bei diff --git a/netbox_export/__init__.py b/netbox_export/__init__.py index 1a34e37..93713b7 100644 --- a/netbox_export/__init__.py +++ b/netbox_export/__init__.py @@ -7,7 +7,7 @@ class NetBoxExportConfig(PluginConfig): name = "netbox_export" verbose_name = "NetBox-Export" description = "Portable ZIP export and import for tenants and locations" - version = "0.3.8" + version = "0.3.9" author = "NetBox Export contributors" base_url = "netbox-export" min_version = "4.6.0" diff --git a/netbox_export/services/exporter.py b/netbox_export/services/exporter.py index 40c21ae..4e7b7e1 100644 --- a/netbox_export/services/exporter.py +++ b/netbox_export/services/exporter.py @@ -39,7 +39,7 @@ def export_scope( "created_at": datetime.now(UTC).isoformat(), "source_instance": str(InstanceIdentity.local_id()), "source_netbox_version": getattr(getattr(settings, "RELEASE", None), "version", "4.6"), - "plugin_version": "0.3.8", + "plugin_version": "0.3.9", "scope": { "type": scope_type, "source_pk": str(scope_id), diff --git a/netbox_export/services/importer.py b/netbox_export/services/importer.py index 2c6a957..428357f 100644 --- a/netbox_export/services/importer.py +++ b/netbox_export/services/importer.py @@ -4,6 +4,7 @@ import logging import uuid from dataclasses import dataclass from dataclasses import field as dataclass_field +from decimal import Decimal from django.apps import apps from django.conf import settings @@ -75,6 +76,13 @@ class ImportReport: counters[action] += 1 +@dataclass(frozen=True) +class DeferredDevicePlacement: + rack_spec: dict | None + position: object + face: object + + def _model_for(label: str): try: model = apps.get_model(label) @@ -213,6 +221,155 @@ def _find_existing(source_instance, model, record, resolver): return natural +def _defer_device_placement(model, record): + if model._meta.label_lower != "dcim.device": + return record, None + + fields = record.get("fields", {}) + relations = record.get("relations", {}) + if "rack" not in relations or "position" not in fields or "face" not in fields: + return record, None + + prepared = dict(record) + prepared["fields"] = { + name: value for name, value in fields.items() if name not in ("position", "face") + } + prepared["relations"] = {name: value for name, value in relations.items() if name != "rack"} + placement = DeferredDevicePlacement( + rack_spec=relations["rack"], + position=decode_scalar(fields["position"]), + face=decode_scalar(fields["face"]), + ) + return prepared, placement + + +def _placement_can_be_applied(placement, resolver): + rack, available = resolver.resolve(placement.rack_spec) + return not (available and rack is MISSING_REFERENCE) + + +def _stage_device_placement(device): + device.position = None + device.face = None + + +def _device_footprint(device): + device_type = getattr(device, "device_type", None) + height = Decimal(str(getattr(device_type, "u_height", 1) or 0)) + if height <= 0: + height = Decimal("0.5") + return height, bool(getattr(device_type, "is_full_depth", False)) + + +def _device_placement_conflicts(device, rack, position, face): + if rack is None or position is None: + return [] + + position = Decimal(str(position)) + height, full_depth = _device_footprint(device) + end = position + height + candidates = ( + type(device) + ._default_manager.select_for_update() + .select_related("device_type") + .filter(rack=rack, position__isnull=False) + .exclude(pk=device.pk) + ) + conflicts = [] + for candidate in candidates: + candidate_position = Decimal(str(candidate.position)) + candidate_height, candidate_full_depth = _device_footprint(candidate) + faces_overlap = full_depth or candidate_full_depth or candidate.face == face + positions_overlap = position < candidate_position + candidate_height and candidate_position < end + if faces_overlap and positions_overlap: + conflicts.append(candidate) + return conflicts + + +def _placement_target(rack, position, face): + rack_name = getattr(rack, "name", None) or str(rack.pk) + return f"Rack {rack_name}, Position {position}, Seite {face or '-'}" + + +def _conflicting_devices(conflicts): + return ", ".join( + f"{device.pk} ({getattr(device, 'name', None) or 'ohne Namen'})" for device in conflicts + ) + + +def _save_device_placement(device, rack, position, face, compatibility): + device.rack = rack + device.position = position + device.face = face + update_fields = ["rack", "position", "face"] + rack_location = getattr(rack, "location", None) + if rack_location is not None: + device.location = rack_location + update_fields.append("location") + compatibility.save(device, update_fields=update_fields) + + +def _apply_device_placements( + placements, + resolved, + resolver, + compatibility, + *, + conflict_strategy, +): + for record_id, placement in placements: + rack, available = resolver.resolve(placement.rack_spec) + if not available: + raise ArchiveValidationError(f"Rack-Referenz für {record_id} konnte nicht aufgelöst werden.") + if rack is MISSING_REFERENCE: + continue + + device = resolved[record_id] + position = placement.position + face = placement.face + if rack is None and (position is not None or face is not None): + resolver.warn( + ("device-placement-without-rack", record_id), + f"Rackplatz für {record_id} wurde ausgelassen, da kein Rack zugeordnet ist.", + ) + position = None + face = None + + if rack is not None and position is not None: + rack = type(rack)._default_manager.select_for_update().get(pk=rack.pk) + + conflicts = _device_placement_conflicts(device, rack, position, face) + if not conflicts: + _save_device_placement(device, rack, position, face, compatibility) + continue + + target = _placement_target(rack, position, face) + occupants = _conflicting_devices(conflicts) + if conflict_strategy == "fail": + raise ImportConflictError( + f"Rackplatzkonflikt für {record_id}: {target} ist durch Zielgerät(e) {occupants} belegt." + ) + if conflict_strategy == "skip": + _save_device_placement(device, rack, None, None, compatibility) + resolver.warn( + ("device-placement-skipped", record_id), + f"Rackplatz für {record_id} wurde übersprungen: {target} bleibt durch " + f"Zielgerät(e) {occupants} belegt. Das importierte Gerät wurde ohne Position im Rack gespeichert.", + ) + continue + + for conflict in conflicts: + conflict.position = None + conflict.face = None + compatibility.save(conflict, update_fields=["position", "face"]) + _save_device_placement(device, rack, position, face, compatibility) + resolver.warn( + ("device-placement-released", record_id), + f"Rackplatzkonflikt für {record_id} gelöst: Zielgerät(e) {occupants} wurden aus {target} " + "gelöst und ohne Position im Rack belassen.", + ) + + def _write_mapping(source_instance, record, obj): content_type = ContentType.objects.get_for_model(obj, for_concrete_model=False) ImportedObjectMapping.objects.update_or_create( @@ -370,6 +527,7 @@ def import_archive(parsed: ParsedArchive, *, conflict_strategy: str, dry_run: bo deferred_relations = [] deferred_generic = [] deferred_values = [] + deferred_device_placements = [] writable = set() saved_files = [] @@ -380,8 +538,16 @@ def import_archive(parsed: ParsedArchive, *, conflict_strategy: str, dry_run: bo progressed = False for record_id, record in list(pending.items()): model = _model_for(record["model"]) + prepared_record, device_placement = _defer_device_placement(model, record) + if device_placement is not None and not _placement_can_be_applied( + device_placement, resolver + ): + device_placement = None kwargs, unresolved, unresolved_value_fields, missing_required = _field_kwargs( - model, record, resolver, tenant_required=compatibility.tenant_required + model, + prepared_record, + resolver, + tenant_required=compatibility.tenant_required, ) if kwargs is None: continue @@ -411,6 +577,8 @@ def import_archive(parsed: ParsedArchive, *, conflict_strategy: str, dry_run: bo progressed = True continue _set_files(obj, record, parsed.assets, saved_files, dry_run=dry_run) + if device_placement is not None: + _stage_device_placement(obj) compatibility.prepare_initial_save(obj, is_new=existing is None) compatibility.save(obj) action = "updated" if existing is not None else "created" @@ -420,6 +588,8 @@ def import_archive(parsed: ParsedArchive, *, conflict_strategy: str, dry_run: bo deferred_values.extend( (record_id, name, encoded) for name, encoded in unresolved_value_fields ) + if device_placement is not None: + deferred_device_placements.append((record_id, device_placement)) resolved[record_id] = obj _write_mapping(source_instance, record, obj) @@ -469,6 +639,14 @@ def import_archive(parsed: ParsedArchive, *, conflict_strategy: str, dry_run: bo setattr(obj, name, value) compatibility.save(obj, update_fields=[name]) + _apply_device_placements( + deferred_device_placements, + resolved, + resolver, + compatibility, + conflict_strategy=conflict_strategy, + ) + for record_id, record in records.items(): if record_id in writable: _apply_m2m(resolved[record_id], record, resolver) diff --git a/pyproject.toml b/pyproject.toml index dffc9d3..ab3fd49 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "netbox-export" -version = "0.3.8" +version = "0.3.9" description = "Portable ZIP export and import for scoped NetBox data" readme = "README.md" requires-python = ">=3.12" diff --git a/tests/test_device_placement.py b/tests/test_device_placement.py new file mode 100644 index 0000000..e7934f5 --- /dev/null +++ b/tests/test_device_placement.py @@ -0,0 +1,291 @@ +from decimal import Decimal +from types import SimpleNamespace +from typing import ClassVar + +import pytest +from django.db import connection, models + +from netbox_export.services.exceptions import ImportConflictError +from netbox_export.services.importer import ( + DeferredDevicePlacement, + _apply_device_placements, + _defer_device_placement, + _save_device_placement, + _stage_device_placement, +) +from netbox_export.services.plugin_compat import PluginCompatibility +from netbox_export.services.references import ReferenceResolver + +pytestmark = pytest.mark.django_db(transaction=True) + + +class PlacementRack(models.Model): + name = models.CharField(max_length=64) + + class Meta: + app_label = "placement_tests" + + +class PlacementDeviceType(models.Model): + u_height = models.DecimalField(max_digits=4, decimal_places=1, default=1) + is_full_depth = models.BooleanField(default=False) + + class Meta: + app_label = "placement_tests" + + +class PlacementDevice(models.Model): + name = models.CharField(max_length=64) + device_type = models.ForeignKey(PlacementDeviceType, on_delete=models.PROTECT) + rack = models.ForeignKey(PlacementRack, on_delete=models.PROTECT, null=True) + position = models.DecimalField(max_digits=4, decimal_places=1, null=True) + face = models.CharField(max_length=16, null=True) + + class Meta: + app_label = "placement_tests" + constraints: ClassVar[list] = [ + models.UniqueConstraint( + fields=("rack", "position", "face"), + name="placement_tests_unique_rack_position_face", + ) + ] + + +@pytest.fixture +def placement_schema(): + with connection.schema_editor() as schema_editor: + schema_editor.create_model(PlacementRack) + schema_editor.create_model(PlacementDeviceType) + schema_editor.create_model(PlacementDevice) + try: + yield + finally: + with connection.schema_editor() as schema_editor: + schema_editor.delete_model(PlacementDevice) + schema_editor.delete_model(PlacementDeviceType) + schema_editor.delete_model(PlacementRack) + + +def placement_context(device, rack, *, position="31.0", face="front"): + record_id = "dcim.device:77" + resolved = {record_id: device, "dcim.rack:2": rack} + warnings = [] + resolver = ReferenceResolver(resolved, warnings, lambda label: None) + compatibility = PluginCompatibility(warnings, dry_run=False, tenant_required=False) + placement = DeferredDevicePlacement( + rack_spec={"ref": "dcim.rack:2"}, + position=Decimal(position) if position is not None else None, + face=face, + ) + return record_id, resolved, warnings, resolver, compatibility, placement + + +def test_device_placement_is_removed_from_initial_record(): + model = SimpleNamespace(_meta=SimpleNamespace(label_lower="dcim.device")) + record = { + "fields": {"name": "Router 1", "position": {"$type": "decimal", "value": "31.0"}, "face": "front"}, + "relations": {"site": {"ref": "dcim.site:1"}, "rack": {"ref": "dcim.rack:2"}}, + } + + prepared, placement = _defer_device_placement(model, record) + + assert prepared["fields"] == {"name": "Router 1"} + assert prepared["relations"] == {"site": {"ref": "dcim.site:1"}} + assert placement == DeferredDevicePlacement( + rack_spec={"ref": "dcim.rack:2"}, + position=Decimal("31.0"), + face="front", + ) + assert "position" in record["fields"] + assert "rack" in record["relations"] + + +def test_saving_placement_persists_location_inherited_from_rack(): + location = object() + rack = SimpleNamespace(location=location) + device = SimpleNamespace(rack=None, position=None, face=None, location=None) + saved = [] + compatibility = SimpleNamespace(save=lambda obj, **kwargs: saved.append((obj, kwargs))) + + _save_device_placement(device, rack, Decimal("31.0"), "front", compatibility) + + assert device.location is location + assert saved == [ + ( + device, + {"update_fields": ["rack", "position", "face", "location"]}, + ) + ] + + +def test_update_releases_exact_rack_occupant_and_places_imported_device(placement_schema): + rack = PlacementRack.objects.create(name="R01") + device_type = PlacementDeviceType.objects.create() + occupant = PlacementDevice.objects.create( + name="Existing", device_type=device_type, rack=rack, position=Decimal("31.0"), face="front" + ) + imported = PlacementDevice.objects.create(name="Imported", device_type=device_type) + record_id, resolved, warnings, resolver, compatibility, placement = placement_context(imported, rack) + + _apply_device_placements( + [(record_id, placement)], + resolved, + resolver, + compatibility, + conflict_strategy="update", + ) + + occupant.refresh_from_db() + imported.refresh_from_db() + assert (occupant.rack, occupant.position, occupant.face) == (rack, None, None) + assert (imported.rack, imported.position, imported.face) == (rack, Decimal("31.0"), "front") + assert len(warnings) == 1 + assert f"Zielgerät(e) {occupant.pk} (Existing)" in warnings[0] + + +def test_skip_keeps_occupant_and_leaves_imported_device_unpositioned(placement_schema): + rack = PlacementRack.objects.create(name="R01") + device_type = PlacementDeviceType.objects.create() + occupant = PlacementDevice.objects.create( + name="Existing", device_type=device_type, rack=rack, position=Decimal("31.0"), face="front" + ) + imported = PlacementDevice.objects.create(name="Imported", device_type=device_type) + record_id, resolved, warnings, resolver, compatibility, placement = placement_context(imported, rack) + + _apply_device_placements( + [(record_id, placement)], + resolved, + resolver, + compatibility, + conflict_strategy="skip", + ) + + occupant.refresh_from_db() + imported.refresh_from_db() + assert (occupant.rack, occupant.position, occupant.face) == (rack, Decimal("31.0"), "front") + assert (imported.rack, imported.position, imported.face) == (rack, None, None) + assert len(warnings) == 1 + assert "wurde ohne Position im Rack gespeichert" in warnings[0] + + +def test_fail_reports_rack_conflict_before_database_constraint(placement_schema): + rack = PlacementRack.objects.create(name="R01") + device_type = PlacementDeviceType.objects.create() + PlacementDevice.objects.create( + name="Existing", device_type=device_type, rack=rack, position=Decimal("31.0"), face="front" + ) + imported = PlacementDevice.objects.create(name="Imported", device_type=device_type) + record_id, resolved, _, resolver, compatibility, placement = placement_context(imported, rack) + + with pytest.raises(ImportConflictError, match="Rackplatzkonflikt.*Rack R01"): + _apply_device_placements( + [(record_id, placement)], + resolved, + resolver, + compatibility, + conflict_strategy="fail", + ) + + +def test_half_depth_devices_can_share_position_on_opposite_faces(placement_schema): + rack = PlacementRack.objects.create(name="R01") + device_type = PlacementDeviceType.objects.create(is_full_depth=False) + occupant = PlacementDevice.objects.create( + name="Existing", device_type=device_type, rack=rack, position=Decimal("31.0"), face="front" + ) + imported = PlacementDevice.objects.create(name="Imported", device_type=device_type) + record_id, resolved, warnings, resolver, compatibility, placement = placement_context( + imported, rack, face="rear" + ) + + _apply_device_placements( + [(record_id, placement)], + resolved, + resolver, + compatibility, + conflict_strategy="update", + ) + + occupant.refresh_from_db() + imported.refresh_from_db() + assert occupant.position == Decimal("31.0") + assert (imported.position, imported.face) == (Decimal("31.0"), "rear") + assert warnings == [] + + +def test_full_depth_multi_u_overlap_is_released(placement_schema): + rack = PlacementRack.objects.create(name="R01") + full_depth = PlacementDeviceType.objects.create(u_height=Decimal("2.0"), is_full_depth=True) + half_depth = PlacementDeviceType.objects.create(u_height=Decimal("1.0"), is_full_depth=False) + occupant = PlacementDevice.objects.create( + name="Existing", + device_type=full_depth, + rack=rack, + position=Decimal("30.5"), + face="front", + ) + imported = PlacementDevice.objects.create(name="Imported", device_type=half_depth) + record_id, resolved, warnings, resolver, compatibility, placement = placement_context( + imported, rack, position="31.0", face="rear" + ) + + _apply_device_placements( + [(record_id, placement)], + resolved, + resolver, + compatibility, + conflict_strategy="update", + ) + + occupant.refresh_from_db() + imported.refresh_from_db() + assert occupant.position is None + assert (imported.position, imported.face) == (Decimal("31.0"), "rear") + assert len(warnings) == 1 + + +def test_imported_devices_can_swap_rack_positions(placement_schema): + rack = PlacementRack.objects.create(name="R01") + device_type = PlacementDeviceType.objects.create() + first = PlacementDevice.objects.create( + name="First", device_type=device_type, rack=rack, position=Decimal("10.0"), face="front" + ) + second = PlacementDevice.objects.create( + name="Second", device_type=device_type, rack=rack, position=Decimal("20.0"), face="front" + ) + for device in (first, second): + _stage_device_placement(device) + device.save(update_fields=["position", "face"]) + + resolved = { + "dcim.device:1": first, + "dcim.device:2": second, + "dcim.rack:2": rack, + } + warnings = [] + resolver = ReferenceResolver(resolved, warnings, lambda label: None) + compatibility = PluginCompatibility(warnings, dry_run=False, tenant_required=False) + placements = [ + ( + "dcim.device:1", + DeferredDevicePlacement({"ref": "dcim.rack:2"}, Decimal("20.0"), "front"), + ), + ( + "dcim.device:2", + DeferredDevicePlacement({"ref": "dcim.rack:2"}, Decimal("10.0"), "front"), + ), + ] + + _apply_device_placements( + placements, + resolved, + resolver, + compatibility, + conflict_strategy="update", + ) + + first.refresh_from_db() + second.refresh_from_db() + assert first.position == Decimal("20.0") + assert second.position == Decimal("10.0") + assert warnings == []