sts-sponsors team mailing list archive

Thread
Date

[Merge] ~ack/maas:fabric-name-required into maas:master

To: mp+435073@xxxxxxxxxxxxxxxxxx
From: Alberto Donato <mp+435073@xxxxxxxxxxxxxxxxxx>
Date: Tue, 03 Jan 2023 12:43:25 -0000
Reply-to: mp+435073@xxxxxxxxxxxxxxxxxx
Sender: noreply@xxxxxxxxxxxxx

Alberto Donato has proposed merging ~ack/maas:fabric-name-required into maas:master.

Commit message:
always set a fabric name



Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~ack/maas/+git/maas/+merge/435073
-- 
Your team MAAS Committers is subscribed to branch maas:master.

diff --git a/src/maasserver/api/fabrics.py b/src/maasserver/api/fabrics.py
index 599bf4f..1073db4 100644
--- a/src/maasserver/api/fabrics.py
+++ b/src/maasserver/api/fabrics.py
@@ -90,11 +90,6 @@ class FabricHandler(OperationsHandler):
         return ("fabric_handler", (fabric_id,))
 
     @classmethod
-    def name(cls, fabric):
-        """Return the name of the fabric."""
-        return fabric.get_name()
-
-    @classmethod
     def vlans(cls, fabric):
         """Return VLANs within the specified fabric."""
         return fabric.vlan_set.all()
diff --git a/src/maasserver/api/tests/test_fabrics.py b/src/maasserver/api/tests/test_fabrics.py
index ab8ccea..865dee3 100644
--- a/src/maasserver/api/tests/test_fabrics.py
+++ b/src/maasserver/api/tests/test_fabrics.py
@@ -134,7 +134,7 @@ class TestFabricAPI(APITestCase.ForUser):
             ContainsDict(
                 {
                     "id": Equals(fabric.id),
-                    "name": Equals(fabric.get_name()),
+                    "name": Equals(fabric.name),
                     "class_type": Equals(class_type),
                 }
             ),
diff --git a/src/maasserver/api/tests/test_interfaces.py b/src/maasserver/api/tests/test_interfaces.py
index 1b4a21a..26f60f0 100644
--- a/src/maasserver/api/tests/test_interfaces.py
+++ b/src/maasserver/api/tests/test_interfaces.py
@@ -64,7 +64,7 @@ def serialize_vlan(vlan):
         "id": vlan.id,
         "dhcp_on": vlan.dhcp_on,
         "external_dhcp": vlan.external_dhcp,
-        "fabric": vlan.fabric.get_name(),
+        "fabric": vlan.fabric.name,
         "fabric_id": vlan.fabric_id,
         "mtu": vlan.mtu,
         "primary_rack": None,
@@ -1004,7 +1004,7 @@ class TestNodeInterfaceAPI(APITransactionTestCase.ForUser):
                 "id": vlan.id,
                 "dhcp_on": vlan.dhcp_on,
                 "external_dhcp": vlan.external_dhcp,
-                "fabric": vlan.fabric.get_name(),
+                "fabric": vlan.fabric.name,
                 "fabric_id": vlan.fabric_id,
                 "mtu": vlan.mtu,
                 "primary_rack": None,
diff --git a/src/maasserver/api/tests/test_subnets.py b/src/maasserver/api/tests/test_subnets.py
index 157991a..7ef7306 100644
--- a/src/maasserver/api/tests/test_subnets.py
+++ b/src/maasserver/api/tests/test_subnets.py
@@ -352,7 +352,7 @@ class TestSubnetAPI(APITestCase.ForUser):
                     "id": subnet.vlan_id,
                     "dhcp_on": subnet.vlan.dhcp_on,
                     "external_dhcp": subnet.vlan.external_dhcp,
-                    "fabric": subnet.vlan.fabric.get_name(),
+                    "fabric": subnet.vlan.fabric.name,
                     "fabric_id": subnet.vlan.fabric_id,
                     "mtu": subnet.vlan.mtu,
                     "primary_rack": None,
@@ -412,7 +412,7 @@ class TestSubnetAPI(APITestCase.ForUser):
                     "id": subnet.vlan_id,
                     "dhcp_on": subnet.vlan.dhcp_on,
                     "external_dhcp": subnet.vlan.external_dhcp,
-                    "fabric": subnet.vlan.fabric.get_name(),
+                    "fabric": subnet.vlan.fabric.name,
                     "fabric_id": subnet.vlan.fabric_id,
                     "mtu": subnet.vlan.mtu,
                     "primary_rack": None,
diff --git a/src/maasserver/api/vlans.py b/src/maasserver/api/vlans.py
index 9154e22..b5f3e38 100644
--- a/src/maasserver/api/vlans.py
+++ b/src/maasserver/api/vlans.py
@@ -146,7 +146,7 @@ class VlanHandler(OperationsHandler):
     @classmethod
     def fabric(cls, vlan):
         """Return fabric name."""
-        return vlan.fabric.get_name()
+        return vlan.fabric.name
 
     @classmethod
     def name(cls, vlan):
diff --git a/src/maasserver/migrations/maasserver/0291_fabric_name_required.py b/src/maasserver/migrations/maasserver/0291_fabric_name_required.py
new file mode 100644
index 0000000..eb09713
--- /dev/null
+++ b/src/maasserver/migrations/maasserver/0291_fabric_name_required.py
@@ -0,0 +1,31 @@
+# Generated by Django 3.2.12 on 2023-01-03 12:34
+
+from django.db import migrations, models
+
+import maasserver.models.fabric
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("maasserver", "0290_migrate_node_power_parameters"),
+    ]
+
+    operations = [
+        migrations.RunSQL(
+            """
+            UPDATE maasserver_fabric
+            SET name = 'fabric-' || id
+            WHERE name IS NULL OR name = ''
+            """
+        ),
+        migrations.AlterField(
+            model_name="fabric",
+            name="name",
+            field=models.CharField(
+                max_length=256,
+                unique=True,
+                validators=[maasserver.models.fabric.validate_fabric_name],
+            ),
+        ),
+    ]
diff --git a/src/maasserver/models/fabric.py b/src/maasserver/models/fabric.py
index 209a9ad..1e7c7e2 100644
--- a/src/maasserver/models/fabric.py
+++ b/src/maasserver/models/fabric.py
@@ -18,14 +18,12 @@ from maasserver.models.subnet import Subnet
 from maasserver.models.timestampedmodel import TimestampedModel
 from maasserver.utils.orm import MAASQueriesMixin
 
+FABRIC_NAME_RE = re.compile(r"^[\w-]+$")
+
 
 def validate_fabric_name(value):
-    """Django validator: `value` must be either `None`, or valid."""
-    if value is None:
-        return
-    namespec = re.compile(r"^[\w-]+$")
-    if not namespec.search(value):
-        raise ValidationError("Invalid fabric name: %s." % value)
+    if not FABRIC_NAME_RE.search(value):
+        raise ValidationError(f"Invalid fabric name: {value}.")
 
 
 class FabricQueriesMixin(MAASQueriesMixin):
@@ -137,9 +135,6 @@ class Fabric(CleanSave, TimestampedModel):
     # clean() and save().
     name = CharField(
         max_length=256,
-        editable=True,
-        null=True,
-        blank=True,
         unique=True,
         validators=[validate_fabric_name],
     )
@@ -166,10 +161,6 @@ class Fabric(CleanSave, TimestampedModel):
         # websockets handler.
         return sorted(self.vlan_set.all(), key=attrgetter("id"))[0]
 
-    def get_name(self):
-        """Return the name of the fabric."""
-        return self.name or f"fabric-{self.id}"
-
     def delete(self):
         if self.is_default():
             raise ValidationError(
diff --git a/src/maasserver/models/interface.py b/src/maasserver/models/interface.py
index 225bf7c..1cc0db6 100644
--- a/src/maasserver/models/interface.py
+++ b/src/maasserver/models/interface.py
@@ -859,7 +859,7 @@ class Interface(CleanSave, TimestampedModel):
                     vlan = subnet.vlan
                     maaslog.info(
                         f"{self.get_log_string()}: Observed connected to "
-                        f"{vlan.fabric.get_name()} via {cidr}."
+                        f"{vlan.fabric.name} via {cidr}."
                     )
                     self.vlan = vlan
                     self.save()
@@ -1543,7 +1543,7 @@ class Interface(CleanSave, TimestampedModel):
             if created:
                 maaslog.info(
                     "%s: Automatically created VLAN %d (observed on %s)"
-                    % (self.get_log_string(), vid, vlan.fabric.get_name())
+                    % (self.get_log_string(), vid, vlan.fabric.name)
                 )
 
     def update_neighbour(self, ip, mac, time, vid=None):
diff --git a/src/maasserver/models/tests/test_fabric.py b/src/maasserver/models/tests/test_fabric.py
index 67aa010..3083982 100644
--- a/src/maasserver/models/tests/test_fabric.py
+++ b/src/maasserver/models/tests/test_fabric.py
@@ -124,9 +124,9 @@ class TestFabricManager(MAASServerTestCase):
 
 
 class TestFabric(MAASServerTestCase):
-    def test_get_name_for_empty_name(self):
+    def test_name_for_empty_name(self):
         fabric = factory.make_Fabric()
-        self.assertEqual("fabric-%s" % fabric.id, fabric.get_name())
+        self.assertEqual(f"fabric-{fabric.id}", fabric.name)
 
     def test_invalid_name_raises_exception(self):
         self.assertRaises(
@@ -138,11 +138,6 @@ class TestFabric(MAASServerTestCase):
             ValidationError, factory.make_Fabric, name="fabric-33"
         )
 
-    def test_get_name_for_set_name(self):
-        name = factory.make_name("name")
-        fabric = factory.make_Fabric(name=name)
-        self.assertEqual(name, fabric.get_name())
-
     def test_creates_fabric_with_default_vlan(self):
         name = factory.make_name("name")
         fabric = factory.make_Fabric(name=name)
@@ -158,7 +153,6 @@ class TestFabric(MAASServerTestCase):
     def test_get_default_fabric_creates_default_fabric(self):
         default_fabric = Fabric.objects.get_default_fabric()
         self.assertEqual(0, default_fabric.id)
-        self.assertEqual("fabric-0", default_fabric.get_name())
         self.assertEqual("fabric-0", default_fabric.name)
 
     def test_create_sets_name(self):
diff --git a/src/maasserver/models/tests/test_interface.py b/src/maasserver/models/tests/test_interface.py
index b9c2e9b..cbb9d5c 100644
--- a/src/maasserver/models/tests/test_interface.py
+++ b/src/maasserver/models/tests/test_interface.py
@@ -2607,7 +2607,7 @@ class TestUpdateIpAddresses(MAASServerTestCase):
                     "%s: Observed connected to %s via %s."
                     % (
                         interface1.get_log_string(),
-                        interface1.vlan.fabric.get_name(),
+                        interface1.vlan.fabric.name,
                         subnet1.cidr,
                     )
                 ),
@@ -2615,7 +2615,7 @@ class TestUpdateIpAddresses(MAASServerTestCase):
                     "%s: Observed connected to %s via %s."
                     % (
                         interface2.get_log_string(),
-                        interface2.vlan.fabric.get_name(),
+                        interface2.vlan.fabric.name,
                         subnet2.cidr,
                     )
                 ),
@@ -2623,7 +2623,7 @@ class TestUpdateIpAddresses(MAASServerTestCase):
                     "%s: Observed connected to %s via %s."
                     % (
                         interface3.get_log_string(),
-                        interface3.vlan.fabric.get_name(),
+                        interface3.vlan.fabric.name,
                         subnet3.cidr,
                     )
                 ),
diff --git a/src/maasserver/models/vlan.py b/src/maasserver/models/vlan.py
index 62fdeef..bbb920a 100644
--- a/src/maasserver/models/vlan.py
+++ b/src/maasserver/models/vlan.py
@@ -204,7 +204,7 @@ class VLAN(CleanSave, TimestampedModel):
     )
 
     def __str__(self):
-        return f"{self.fabric.get_name()}.{self.get_name()}"
+        return f"{self.fabric.name}.{self.get_name()}"
 
     def clean_vid(self):
         if self.vid is None or self.vid < 0 or self.vid > 4094:
diff --git a/src/maasserver/websockets/handlers/node.py b/src/maasserver/websockets/handlers/node.py
index 0e8bbb7..c669037 100644
--- a/src/maasserver/websockets/handlers/node.py
+++ b/src/maasserver/websockets/handlers/node.py
@@ -876,7 +876,7 @@ class NodeHandler(TimestampedModelHandler):
                 "id": interface.vlan_id,
                 "name": "%s" % interface.vlan.name,
                 "fabric_id": interface.vlan.fabric.id,
-                "fabric_name": "%s" % interface.vlan.fabric.name,
+                "fabric_name": interface.vlan.fabric.name,
             }
 
     def dehydrate_all_ip_addresses(self, obj):
diff --git a/src/maasserver/websockets/handlers/tests/test_fabric.py b/src/maasserver/websockets/handlers/tests/test_fabric.py
index 71e15ab..ef6dab7 100644
--- a/src/maasserver/websockets/handlers/tests/test_fabric.py
+++ b/src/maasserver/websockets/handlers/tests/test_fabric.py
@@ -16,7 +16,7 @@ class TestFabricHandler(MAASServerTestCase):
     def dehydrate_fabric(self, fabric):
         data = {
             "id": fabric.id,
-            "name": fabric.get_name(),
+            "name": fabric.name,
             "description": fabric.description,
             "class_type": fabric.class_type,
             "updated": dehydrate_datetime(fabric.updated),

Follow ups

Re: [UNITTESTS] -b fabric-name-required lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas - TESTS FAILED
From: MAAS Lander, 2023-01-03
Re: [UNITTESTS] -b fabric-name-required lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas - TESTS FAILED
From: MAAS Lander, 2023-01-03
Re: [UNITTESTS] -b fabric-name-required lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas - TESTS FAILED
From: MAAS Lander, 2023-01-03