launchpad-reviewers team mailing list archive

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

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/drop-master-nodegroup-cache/+merge/117242

This is one bit of spinoff from my work to make Node.nodegroup NOT NULL: tests (and debugging of tests!) were being complicated by the caching of the master nodegroup.  There's no semantic need for that cache (hence no change in tests) so let's just drop it as a premature optimization.

(Not properly pre-imped, but sanity-checked by mentioning it to Raphaël).


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

=== modified file 'src/maasserver/models/nodegroup.py'
--- src/maasserver/models/nodegroup.py	2012-07-27 10:23:53 +0000
+++ src/maasserver/models/nodegroup.py	2012-07-30 11:24:18 +0000
@@ -39,9 +39,6 @@
     the model class it manages.
     """
 
-    # Cached master nodegroup.
-    cached_master = None
-
     def new(self, name, worker_ip, subnet_mask=None, broadcast_ip=None,
             router_ip=None, ip_range_low=None, ip_range_high=None):
         """Create a :class:`NodeGroup` with the given parameters.
@@ -77,19 +74,20 @@
         # Avoid circular imports.
         from maasserver.models import Node
 
-        if self.cached_master is None:
-            try:
-                self.cached_master = self.get(name='master')
-            except NodeGroup.DoesNotExist:
-                self.cached_master = self.new('master', '127.0.0.1')
-                Node.objects.filter(nodegroup=None).update(
-                    nodegroup=self.cached_master)
-
-        return self.cached_master
+        try:
+            master = self.get(name='master')
+        except NodeGroup.DoesNotExist:
+            # The master did not exist yet; create it on demand.
+            master = self.new('master', '127.0.0.1')
+
+            # If any legacy nodes were still not associated with a node
+            # group, enroll them in the master node group.
+            Node.objects.filter(nodegroup=None).update(nodegroup=master)
+
+        return master
 
     def _delete_master(self):
         """For use by tests: delete the master nodegroup."""
-        self.cached_master = None
         self.filter(name='master').delete()