From 2e37b19e9f68f91e6a5e5c1b7bcaee99b95a5f74 Mon Sep 17 00:00:00 2001
From: Jeremy Stretch <stretch@packetlife.net>
Date: Mon, 9 Dec 2019 11:59:30 -0500
Subject: [PATCH] #2269: Allow non-unique VirtualMachine names

---
 .../migrations/0012_vm_name_nonunique.py      | 23 ++++++++++
 netbox/virtualization/models.py               | 20 ++++++++-
 netbox/virtualization/tests/test_models.py    | 44 +++++++++++++++++++
 3 files changed, 85 insertions(+), 2 deletions(-)
 create mode 100644 netbox/virtualization/migrations/0012_vm_name_nonunique.py
 create mode 100644 netbox/virtualization/tests/test_models.py

diff --git a/netbox/virtualization/migrations/0012_vm_name_nonunique.py b/netbox/virtualization/migrations/0012_vm_name_nonunique.py
new file mode 100644
index 000000000..c10b3e5e5
--- /dev/null
+++ b/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')},
+        ),
+    ]
diff --git a/netbox/virtualization/models.py b/netbox/virtualization/models.py
index aa84c403c..86d31afcb 100644
--- a/netbox/virtualization/models.py
+++ b/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()
diff --git a/netbox/virtualization/tests/test_models.py b/netbox/virtualization/tests/test_models.py
new file mode 100644
index 000000000..ce126fc99
--- /dev/null
+++ b/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()