launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/nodegroup-name into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/nodegroup-name/+merge/114171

The factory's make_node_group method didn't let us set a zone name for the new NodeGroup it creates.  Nor does it set a zone name itself.  And so the nodegroup gets the default zone name, which is the blank string.  Which is fine if your test creates one nodegroup, but given Django's weird 19th-century ideas about empty strings and nulls, gives you a weird unique violation if you create a second one!

I think this needs a bit of a rethink.  But we can't do that right now because the intention of that field seems to be entirely undocumented.  I only know it's meant to indicate a DNS zone name because Raphaël seems to recall it.  This is not good.  All I can do about it right now is document what we think we know.

Anyway.  For now I just extended the factory method with a “name” parameter, which defaults to a randomized name with a recognizable “nodegroup-” prefix.  If you want a blank name for your test you can just pass an empty string.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/nodegroup-name/+merge/114171
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/nodegroup-name into lp:maas.

=== modified file 'src/maasserver/models/nodegroup.py'
--- src/maasserver/models/nodegroup.py	2012-06-05 07:10:07 +0000
+++ src/maasserver/models/nodegroup.py	2012-07-10 11:51:19 +0000
@@ -44,8 +44,8 @@
 
     objects = NodeGroupManager()
 
-    name = CharField(
-        max_length=80, unique=True, editable=True, default="")
+    # Name for the node group's DNS zone.
+    name = CharField(max_length=80, unique=True, editable=True, default="")
 
     api_token = ForeignKey(Token, null=False, editable=False, unique=True)
     api_key = CharField(

=== modified file 'src/maasserver/testing/factory.py'
--- src/maasserver/testing/factory.py	2012-06-28 15:44:17 +0000
+++ src/maasserver/testing/factory.py	2012-07-10 11:51:19 +0000
@@ -107,9 +107,11 @@
             Node.objects.filter(id=node.id).update(created=created)
         return reload_object(node)
 
-    def make_node_group(self, api_token=None, worker_ip=None,
+    def make_node_group(self, name=None, api_token=None, worker_ip=None,
                         subnet_mask=None, broadcast_ip=None, router_ip=None,
                         ip_range_low=None, ip_range_high=None, **kwargs):
+        if name is None:
+            name = self.make_name('nodegroup')
         if api_token is None:
             user = self.make_user()
             api_token = create_auth_token(user)
@@ -126,10 +128,10 @@
         if ip_range_high is None:
             ip_range_high = factory.getRandomIPAddress()
         ng = NodeGroup(
-            api_token=api_token, api_key=api_token.key, worker_ip=worker_ip,
-            subnet_mask=subnet_mask, broadcast_ip=broadcast_ip,
-            router_ip=router_ip, ip_range_low=ip_range_low,
-            ip_range_high=ip_range_high, **kwargs)
+            name=name, api_token=api_token, api_key=api_token.key,
+            worker_ip=worker_ip, subnet_mask=subnet_mask,
+            broadcast_ip=broadcast_ip, router_ip=router_ip,
+            ip_range_low=ip_range_low, ip_range_high=ip_range_high, **kwargs)
         ng.save()
         return ng