Merge pull request #3740 from netbox-community/2669-device-vm-names

Allow non-unique device and VM names

netbox/dcim/migrations/0086_device_name_nonunique.py

@@ -0,0 +1,23 @@
+# Generated by Django 2.2.6 on 2019-12-09 15:49
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('tenancy', '0006_custom_tag_models'),
+        ('dcim', '0085_3569_poweroutlet_fields'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='device',
+            name='name',
+            field=models.CharField(blank=True, max_length=64, null=True),
+        ),
+        migrations.AlterUniqueTogether(
+            name='device',
+            unique_together={('rack', 'position', 'face'), ('virtual_chassis', 'vc_position'), ('site', 'tenant', 'name')},
+        ),
+    ]

netbox/dcim/models.py

@@ -1523,8 +1523,7 @@ class Device(ChangeLoggedModel, ConfigContextModel, CustomFieldModel):
     name = models.CharField(
         max_length=64,
         blank=True,
-        null=True,
-        unique=True
+        null=True
     )
     serial = models.CharField(
         max_length=50,
@@ -1645,6 +1644,7 @@ class Device(ChangeLoggedModel, ConfigContextModel, CustomFieldModel):
     class Meta:
         ordering = ['name']
         unique_together = [
+            ['site', 'tenant', 'name'],  # See validate_unique below
             ['rack', 'position', 'face'],
             ['virtual_chassis', 'vc_position'],
         ]
@@ -1659,6 +1659,18 @@ class Device(ChangeLoggedModel, ConfigContextModel, CustomFieldModel):
     def get_absolute_url(self):
         return reverse('dcim:device', args=[self.pk])
 
+    def validate_unique(self, exclude=None):
+
+        # Check for a duplicate name on a device assigned to the same Site and no Tenant. This is necessary
+        # because Django does not consider two NULL fields to be equal, and thus will not trigger a violation
+        # of the uniqueness constraint without manual intervention.
+        if self.tenant is None and Device.objects.exclude(pk=self.pk).filter(name=self.name, tenant__isnull=True):
+            raise ValidationError({
+                'name': 'A device with this name already exists.'
+            })
+
+        super().validate_unique(exclude)
+
     def clean(self):
 
         super().clean()

netbox/dcim/tests/test_models.py

@@ -1,6 +1,7 @@
 from django.test import TestCase
 
 from dcim.models import *
+from tenancy.models import Tenant
 
 
 class RackTestCase(TestCase):
@@ -281,6 +282,42 @@ class DeviceTestCase(TestCase):
             name='Device Bay 1'
         )
 
+    def test_device_duplicate_name_per_site(self):
+
+        device1 = Device(
+            site=self.site,
+            device_type=self.device_type,
+            device_role=self.device_role,
+            name='Test Device 1'
+        )
+        device1.save()
+
+        device2 = Device(
+            site=device1.site,
+            device_type=device1.device_type,
+            device_role=device1.device_role,
+            name=device1.name
+        )
+
+        # Two devices assigned to the same Site and no Tenant should fail validation
+        with self.assertRaises(ValidationError):
+            device2.full_clean()
+
+        tenant = Tenant.objects.create(name='Test Tenant 1', slug='test-tenant-1')
+        device1.tenant = tenant
+        device1.save()
+        device2.tenant = tenant
+
+        # Two devices assigned to the same Site and the same Tenant should fail validation
+        with self.assertRaises(ValidationError):
+            device2.full_clean()
+
+        device2.tenant = None
+
+        # Two devices assigned to the same Site and different Tenants should pass validation
+        device2.full_clean()
+        device2.save()
+
 
 class CableTestCase(TestCase):
 

netbox/virtualization/api/serializers.py

@@ -75,6 +75,7 @@ class VirtualMachineSerializer(TaggitSerializer, CustomFieldModelSerializer):
             'primary_ip6', 'vcpus', 'memory', 'disk', 'comments', 'local_context_data', 'tags', 'custom_fields',
             'created', 'last_updated',
         ]
+        validators = []
 
 
 class VirtualMachineWithConfigContextSerializer(VirtualMachineSerializer):

netbox/virtualization/migrations/0012_vm_name_nonunique.py

@@ -0,0 +1,23 @@
+# Generated by Django 2.2.6 on 2019-12-09 16:57
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('tenancy', '0006_custom_tag_models'),
+        ('virtualization', '0011_3569_virtualmachine_fields'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='virtualmachine',
+            name='name',
+            field=models.CharField(max_length=64),
+        ),
+        migrations.AlterUniqueTogether(
+            name='virtualmachine',
+            unique_together={('cluster', 'tenant', 'name')},
+        ),
+    ]

netbox/virtualization/models.py

@@ -193,8 +193,7 @@ class VirtualMachine(ChangeLoggedModel, ConfigContextModel, CustomFieldModel):
         null=True
     )
     name = models.CharField(
-        max_length=64,
-        unique=True
+        max_length=64
     )
     status = models.CharField(
         max_length=50,
@@ -267,6 +266,9 @@ class VirtualMachine(ChangeLoggedModel, ConfigContextModel, CustomFieldModel):
 
     class Meta:
         ordering = ['name']
+        unique_together = [
+            ['cluster', 'tenant', 'name']
+        ]
 
     def __str__(self):
         return self.name
@@ -274,6 +276,20 @@ class VirtualMachine(ChangeLoggedModel, ConfigContextModel, CustomFieldModel):
     def get_absolute_url(self):
         return reverse('virtualization:virtualmachine', args=[self.pk])
 
+    def validate_unique(self, exclude=None):
+
+        # Check for a duplicate name on a VM assigned to the same Cluster and no Tenant. This is necessary
+        # because Django does not consider two NULL fields to be equal, and thus will not trigger a violation
+        # of the uniqueness constraint without manual intervention.
+        if self.tenant is None and VirtualMachine.objects.exclude(pk=self.pk).filter(
+                name=self.name, tenant__isnull=True
+        ):
+            raise ValidationError({
+                'name': 'A virtual machine with this name already exists.'
+            })
+
+        super().validate_unique(exclude)
+
     def clean(self):
 
         super().clean()

netbox/virtualization/tests/test_models.py

@@ -0,0 +1,44 @@
+from django.test import TestCase
+
+from virtualization.models import *
+from tenancy.models import Tenant
+
+
+class VirtualMachineTestCase(TestCase):
+
+    def setUp(self):
+
+        cluster_type = ClusterType.objects.create(name='Test Cluster Type 1', slug='Test Cluster Type 1')
+        self.cluster = Cluster.objects.create(name='Test Cluster 1', type=cluster_type)
+
+    def test_vm_duplicate_name_per_cluster(self):
+
+        vm1 = VirtualMachine(
+            cluster=self.cluster,
+            name='Test VM 1'
+        )
+        vm1.save()
+
+        vm2 = VirtualMachine(
+            cluster=vm1.cluster,
+            name=vm1.name
+        )
+
+        # Two VMs assigned to the same Cluster and no Tenant should fail validation
+        with self.assertRaises(ValidationError):
+            vm2.full_clean()
+
+        tenant = Tenant.objects.create(name='Test Tenant 1', slug='test-tenant-1')
+        vm1.tenant = tenant
+        vm1.save()
+        vm2.tenant = tenant
+
+        # Two VMs assigned to the same Cluster and the same Tenant should fail validation
+        with self.assertRaises(ValidationError):
+            vm2.full_clean()
+
+        vm2.tenant = None
+
+        # Two VMs assigned to the same Cluster and different Tenants should pass validation
+        vm2.full_clean()
+        vm2.save()