Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow non-unique device and VM names #3740

Merged
merged 3 commits into from Dec 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 23 additions & 0 deletions 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')},
),
]
16 changes: 14 additions & 2 deletions netbox/dcim/models.py
Expand Up @@ -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,
Expand Down Expand Up @@ -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'],
]
Expand All @@ -1659,6 +1659,18 @@ def __str__(self):
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()
Expand Down
37 changes: 37 additions & 0 deletions 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):
Expand Down Expand Up @@ -281,6 +282,42 @@ def test_device_creation(self):
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):

Expand Down
1 change: 1 addition & 0 deletions netbox/virtualization/api/serializers.py
Expand Up @@ -75,6 +75,7 @@ class Meta:
'primary_ip6', 'vcpus', 'memory', 'disk', 'comments', 'local_context_data', 'tags', 'custom_fields',
'created', 'last_updated',
]
validators = []


class VirtualMachineWithConfigContextSerializer(VirtualMachineSerializer):
Expand Down
23 changes: 23 additions & 0 deletions 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')},
),
]
20 changes: 18 additions & 2 deletions netbox/virtualization/models.py
Expand Up @@ -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,
Expand Down Expand Up @@ -267,13 +266,30 @@ class VirtualMachine(ChangeLoggedModel, ConfigContextModel, CustomFieldModel):

class Meta:
ordering = ['name']
unique_together = [
['cluster', 'tenant', 'name']
]

def __str__(self):
return self.name

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()
Expand Down
44 changes: 44 additions & 0 deletions 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()