Merge lp:~rvb/maas/bug-1104215 into lp:~maas-committers/maas/trunk

[Raphaël Badin]

Detect if the connecting cluster is the local cluster by comparing its UUID with the local cluster UUID.

- We now support an external cluster to connect first (for instance if no cluster was installed alongside the region cluster). It will need to be accepted by an admin.
- No pre-imp for this but consensus was reached in the discussion on the bug's comments.
- This has been tested in the lab.

[Gavin Panella (allenap)]

Looks good.

[1]

+                    if is_local_cluster:
+                        return get_celery_credentials()
+                    else:
+                        return HttpResponse(
+                            "Cluster registered.  Awaiting admin approval.",
+                            status=httplib.ACCEPTED)

The condition - if is_local_cluster - is a secondhand indicator for
what you should be testing here: that the nodegroup status is
ACCEPTED.

[2]

+    try:
+        cluster_config = file(settings.LOCAL_CLUSTER_CONFIG).read()

`file` is gone in Python 3.x. Use `open`.

[3]

+    except IOError:
+        # Cluster config file is not present.
+        return False

Be more specific here:

    except IOError as error:
        if error.errno == errno.ENOENT:
            # Cluster config file is not present.
            return False
        else:
            # Anything else is an error.
            raise

[4]

+        match = re.search('CLUSTER_UUID="([^"]*)"', cluster_config)
+        if match is not None:
+            return uuid == match.groups()[0]

I know you're probably well aware of this, but: euargh.

[5]

+        match = re.search('CLUSTER_UUID="([^"]*)"', cluster_config)

Just in case someone comes along and removes the quotes, or changes
them to single quotes, or does either of those things in another part
of MAAS's code base, consider the following expression:

    re.search(
        "CLUSTER_UUID=(?P<quote>[\"']?)([^\"']+)(?P=quote)",
        cluster_config)

review: Approve

[Raphaël Badin (rvb)]

A great review, thanks. I've done all the changes you suggested.

=== modified file 'src/maas/settings.py'
--- src/maas/settings.py	2012-12-18 12:31:47 +0000
+++ src/maas/settings.py	2013-01-30 15:35:12 +0000
@@ -85,6 +85,11 @@
 # maasserver.middleware.APIErrorsMiddleware)
 PISTON_DISPLAY_ERRORS = False
 
+# Location of the local cluster config file (installed by
+# the package maas-cluster-controller).  Use to distinguish the local cluster
+# from the others.
+LOCAL_CLUSTER_CONFIG = "/etc/maas/maas_cluster.conf"
+
 TEMPLATE_DEBUG = DEBUG
 
 # Set this to where RaphaelJS files can be found.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2013-01-16 11:05:53 +0000
+++ src/maasserver/api.py	2013-01-30 15:35:12 +0000
@@ -178,6 +178,7 @@
     absolute_reverse,
     build_absolute_uri,
     find_nodegroup,
+    is_local_cluster_UUID,
     map_enum,
     strip_domain,
     )
@@ -912,17 +913,30 @@
             "eth0"}]'
         """
         uuid = get_mandatory_param(request.data, 'uuid')
+        is_local_cluster = is_local_cluster_UUID(uuid)
         existing_nodegroup = get_one(NodeGroup.objects.filter(uuid=uuid))
         if existing_nodegroup is None:
             master = NodeGroup.objects.ensure_master()
             # Does master.uuid look like it's a proper uuid?
             if master.uuid in ('master', ''):
                 # Master nodegroup not yet configured, configure it.
+                if is_local_cluster:
+                    # Connecting from localhost, accept the cluster
+                    # controller.
+                    status = NODEGROUP_STATUS.ACCEPTED
+                else:
+                    # Connecting remotely, mark the cluster as pending.
+                    status = NODEGROUP_STATUS.PENDING
                 form = NodeGroupWithInterfacesForm(
-                    data=request.data, instance=master)
+                    data=request.data, status=status, instance=master)
                 if form.is_valid():
                     form.save()
-                    return get_celery_credentials()
+                    if status == NODEGROUP_STATUS.ACCEPTED:
+                        return get_celery_credentials()
+                    else:
+                        return HttpResponse(
+                            "Cluster registered.  Awaiting admin approval.",
+                            status=httplib.ACCEPTED)
                 else:
                     raise ValidationError(form.errors)
             else:

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2013-01-16 11:05:53 +0000
+++ src/maasserver/tests/test_api.py	2013-01-30 15:35:12 +0000
@@ -31,6 +31,7 @@
 import random
 import shutil
 import sys
+from textwrap import dedent
 from urlparse import urlparse
 
 from apiclient.maas_client import MAASClient
@@ -3673,9 +3674,20 @@
         # validated by an admin.
         self.assertEqual(httplib.ACCEPTED, response.status_code)
 
-    def test_register_nodegroup_returns_credentials_if_master(self):
+    def setup_local_cluster_config(self, uuid):
+        # Helper to get settings.LOCAL_CLUSTER_CONFIG to point to a valid
+        # cluster config file with CLUSTER_UUID set to the given uuid.
+        contents = dedent("""
+            MAAS_URL=http://localhost/MAAS
+            CLUSTER_UUID="%s"
+            """ % uuid)
+        file_name = self.make_file(contents=contents)
+        self.patch(settings, 'LOCAL_CLUSTER_CONFIG', file_name)
+
+    def test_register_nodegroup_returns_credentials_if_local_cluster(self):
         name = factory.make_name('name')
         uuid = factory.getRandomUUID()
+        self.setup_local_cluster_config(uuid)
         fake_broker_url = factory.make_name('fake-broker_url')
         celery_conf = app_or_default().conf
         self.patch(celery_conf, 'BROKER_URL', fake_broker_url)
@@ -3692,9 +3704,27 @@
             ({'BROKER_URL': fake_broker_url}, uuid),
             (parsed_result, NodeGroup.objects.ensure_master().uuid))
 
+    def test_register_nodegroup_gets_accepted_if_not_local_cluster(self):
+        name = factory.make_name('name')
+        uuid = factory.getRandomUUID()
+        fake_broker_url = factory.make_name('fake-broker_url')
+        celery_conf = app_or_default().conf
+        self.patch(celery_conf, 'BROKER_URL', fake_broker_url)
+        response = self.client.post(
+            reverse('nodegroups_handler'),
+            {
+                'op': 'register',
+                'name': name,
+                'uuid': uuid,
+            })
+        self.assertEqual(httplib.ACCEPTED, response.status_code, response)
+        self.assertEqual(
+            response.content, "Cluster registered.  Awaiting admin approval.")
+
     def test_register_nodegroup_configures_master_if_unconfigured(self):
         name = factory.make_name('nodegroup')
         uuid = factory.getRandomUUID()
+        self.setup_local_cluster_config(uuid)
         interface = make_interface_settings()
         self.client.post(
             reverse('nodegroups_handler'),
@@ -3713,6 +3743,7 @@
 
     def test_register_nodegroup_uses_default_zone_name(self):
         uuid = factory.getRandomUUID()
+        self.setup_local_cluster_config(uuid)
         self.client.post(
             reverse('nodegroups_handler'),
             {

=== modified file 'src/maasserver/utils/__init__.py'
--- src/maasserver/utils/__init__.py	2012-12-03 10:06:56 +0000
+++ src/maasserver/utils/__init__.py	2013-01-30 15:35:12 +0000
@@ -16,10 +16,13 @@
     'find_nodegroup',
     'get_db_state',
     'ignore_unused',
+    'is_local_cluster_UUID',
     'map_enum',
     'strip_domain',
     ]
 
+import errno
+import re
 from urllib import urlencode
 from urlparse import urljoin
 
@@ -110,6 +113,26 @@
     return hostname.split('.', 1)[0]
 
 
+def is_local_cluster_UUID(uuid):
+    """Return whether the given UUID is the UUID of the local cluster."""
+    try:
+        cluster_config = open(settings.LOCAL_CLUSTER_CONFIG).read()
+        match = re.search(
+            "CLUSTER_UUID=(?P<quote>[\"']?)([^\"']+)(?P=quote)",
+            cluster_config)
+        if match is not None:
+            return uuid == match.groups()[1]
+        else:
+            return False
+    except IOError as error:
+        if error.errno == errno.ENOENT:
+            # Cluster config file is not present.
+            return False
+        else:
+            # Anything else is an error.
+            raise
+
+
 def find_nodegroup(request):
     """Find the nodegroup whose subnet contains the IP Address of the
     originating host of the request..

=== modified file 'src/maasserver/utils/tests/test_utils.py'
--- src/maasserver/utils/tests/test_utils.py	2012-11-29 20:15:12 +0000
+++ src/maasserver/utils/tests/test_utils.py	2013-01-30 15:35:12 +0000
@@ -34,6 +34,7 @@
     build_absolute_uri,
     find_nodegroup,
     get_db_state,
+    is_local_cluster_UUID,
     map_enum,
     strip_domain,
     )
@@ -194,6 +195,32 @@
         self.assertEqual(results, map(strip_domain, inputs))
 
 
+class TestIsLocalClusterUUID(TestCase):
+
+    def test_is_local_cluster_UUID_returns_false_if_no_config_file(self):
+        bogus_file_name = '/tmp/bogus/%s' % factory.make_name('name')
+        self.patch(settings, 'LOCAL_CLUSTER_CONFIG', bogus_file_name)
+        self.assertFalse(is_local_cluster_UUID(factory.getRandomUUID()))
+
+    def test_is_local_cluster_UUID_returns_false_if_parsing_fails(self):
+        file_name = self.make_file(contents="wrong content")
+        self.patch(settings, 'LOCAL_CLUSTER_CONFIG', file_name)
+        self.assertFalse(is_local_cluster_UUID(factory.getRandomUUID()))
+
+    def test_is_local_cluster_UUID_returns_false_if_wrong_UUID(self):
+        uuid = factory.getRandomUUID()
+        other_uuid = factory.getRandomUUID()
+        file_name = self.make_file(contents='CLUSTER_UUID="%s"' % other_uuid)
+        self.patch(settings, 'LOCAL_CLUSTER_CONFIG', file_name)
+        self.assertFalse(is_local_cluster_UUID(uuid))
+
+    def test_is_local_cluster_UUID_returns_true_if_local_UUID(self):
+        uuid = factory.getRandomUUID()
+        file_name = self.make_file(contents='CLUSTER_UUID="%s"' % uuid)
+        self.patch(settings, 'LOCAL_CLUSTER_CONFIG', file_name)
+        self.assertTrue(is_local_cluster_UUID(uuid))
+
+
 def get_request(origin_ip):
     return RequestFactory().post('/', REMOTE_ADDR=origin_ip)