← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~jtv/maas/1.2-bug-1086239 into lp:maas/1.2

 

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/1.2-bug-1086239 into lp:maas/1.2.

Commit message:
Backport trunk r1397: Take CLUSTER_UUID from maas_cluster.conf instead of maas_local_celeryconfig_cluster.py.

Requested reviews:
  MAAS Maintainers (maas-maintainers)
Related bugs:
  Bug #1086239 in MAAS: "CLUSTER_UUID is in cluster celery config, but tftpd needs it"
  https://bugs.launchpad.net/maas/+bug/1086239

For more details, see:
https://code.launchpad.net/~jtv/maas/1.2-bug-1086239/+merge/139498

Straight backport from trunk.  Bug 1086239 is the capstone in the fix for bug 1084507.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/1.2-bug-1086239/+merge/139498
Your team MAAS Maintainers is requested to review the proposed merge of lp:~jtv/maas/1.2-bug-1086239 into lp:maas/1.2.
=== modified file 'etc/democeleryconfig_cluster.py'
--- etc/democeleryconfig_cluster.py	2012-12-03 06:29:58 +0000
+++ etc/democeleryconfig_cluster.py	2012-12-12 15:57:42 +0000
@@ -26,10 +26,6 @@
 
 import_settings(democeleryconfig_common)
 
-# Set a fixed CLUSTER_UUID.  In production, this is taken from
-# maas_local_celeryconfig.
-CLUSTER_UUID = "adfd3977-f251-4f2c-8d61-745dbd690bfc"
-
 CELERYD_LOG_FILE = os.path.join(
     DEV_ROOT_DIRECTORY, 'logs/cluster-worker/current')
 

=== modified file 'services/cluster-worker/run'
--- services/cluster-worker/run	2012-10-03 11:39:37 +0000
+++ services/cluster-worker/run	2012-12-12 15:57:42 +0000
@@ -16,6 +16,11 @@
 
 export PYTHONPATH=etc/:src/
 export CELERY_CONFIG_MODULE=democeleryconfig_cluster
+
+# Set a fixed cluster UUID.  This is identical for all cluster services
+# running in the demo setup: they're all the same cluster.
+export CLUSTER_UUID="adfd3977-f251-4f2c-8d61-745dbd690bfc"
+
 script="$(readlink -f bin/maas-provision)"
 exec fghack "${script}" start-cluster-controller \
     http://0.0.0.0:5240/ -u "$(id -un)" -g "$(id -gn)"

=== modified file 'services/pserv/run'
--- services/pserv/run	2012-09-26 14:16:33 +0000
+++ services/pserv/run	2012-12-12 15:57:42 +0000
@@ -17,5 +17,10 @@
 # Exec the Provisioning Server.
 script="$(readlink -f bin/twistd.pserv)"
 config="$(readlink -f etc/pserv.yaml)"
+
+# Set a fixed cluster UUID.  This is identical for all cluster services
+# running in the demo setup: they're all the same cluster.
+export CLUSTER_UUID="adfd3977-f251-4f2c-8d61-745dbd690bfc"
+
 exec $(command -v authbind && echo --deep) \
     "${script}" --nodaemon --pidfile="" maas-pserv --config-file "${config}"

=== modified file 'src/provisioningserver/auth.py'
--- src/provisioningserver/auth.py	2012-11-26 02:54:49 +0000
+++ src/provisioningserver/auth.py	2012-12-12 15:57:42 +0000
@@ -13,13 +13,10 @@
 __all__ = [
     'get_recorded_api_credentials',
     'get_recorded_nodegroup_uuid',
-    'get_recorded_maas_url',
     'record_api_credentials',
     'record_nodegroup_uuid',
     ]
 
-import os
-
 from apiclient.creds import convert_string_to_tuple
 from provisioningserver import cache
 
@@ -30,11 +27,6 @@
 NODEGROUP_UUID_CACHE_KEY = 'nodegroup_uuid'
 
 
-def get_recorded_maas_url():
-    """Return the base URL for the MAAS server."""
-    return os.environ.get("MAAS_URL")
-
-
 def record_api_credentials(api_credentials):
     """Update the recorded API credentials.
 

=== modified file 'src/provisioningserver/boot_images.py'
--- src/provisioningserver/boot_images.py	2012-12-04 00:52:42 +0000
+++ src/provisioningserver/boot_images.py	2012-12-12 15:57:42 +0000
@@ -26,13 +26,13 @@
     MAASDispatcher,
     MAASOAuth,
     )
-from provisioningserver.auth import (
-    get_recorded_api_credentials,
-    get_recorded_maas_url,
+from provisioningserver.auth import get_recorded_api_credentials
+from provisioningserver.cluster_config import (
+    get_cluster_uuid,
+    get_maas_url,
     )
 from provisioningserver.config import Config
 from provisioningserver.pxe import tftppath
-from provisioningserver.start_cluster_controller import get_cluster_uuid
 
 
 logger = getLogger(__name__)
@@ -44,7 +44,7 @@
     :return: Tuple of cached items: (maas_url, api_credentials).  Either may
         be None if the information has not been received from the server yet.
     """
-    maas_url = get_recorded_maas_url()
+    maas_url = get_maas_url()
     if maas_url is None:
         logger.debug("Not reporting boot images: don't have API URL yet.")
     api_credentials = get_recorded_api_credentials()

=== added file 'src/provisioningserver/cluster_config.py'
--- src/provisioningserver/cluster_config.py	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/cluster_config.py	2012-12-12 15:57:42 +0000
@@ -0,0 +1,44 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Accessors for cluster configuration as set in `maas_cluster.conf`."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = [
+    'get_cluster_uuid',
+    'get_maas_url',
+    ]
+
+from os import environ
+
+
+def get_cluster_variable(var):
+    """Obtain the given environment variable from maas_cluster.conf.
+
+    If the variable is not set, it probably means that whatever script
+    started the current process neglected to run maas_cluster.conf.
+    In that case, fail helpfully but utterly.
+    """
+    value = environ.get(var)
+    if value is None:
+        raise AssertionError(
+            "%s is not set.  This probably means that the script which "
+            "started this program failed to source maas_cluster.conf."
+            % var)
+    return value
+
+
+def get_cluster_uuid():
+    """Return the `CLUSTER_UUID` setting."""
+    return get_cluster_variable('CLUSTER_UUID')
+
+
+def get_maas_url():
+    """Return the `MAAS_URL` setting."""
+    return get_cluster_variable('MAAS_URL')

=== modified file 'src/provisioningserver/dhcp/leases.py'
--- src/provisioningserver/dhcp/leases.py	2012-12-04 00:52:42 +0000
+++ src/provisioningserver/dhcp/leases.py	2012-12-12 15:57:42 +0000
@@ -49,9 +49,9 @@
 from provisioningserver import cache
 from provisioningserver.auth import (
     get_recorded_api_credentials,
-    get_recorded_maas_url,
     get_recorded_nodegroup_uuid,
     )
+from provisioningserver.cluster_config import get_maas_url
 from provisioningserver.dhcp.leases_parser import parse_leases
 
 
@@ -150,7 +150,7 @@
     """Send lease updates to the server API."""
     # Items that the server must have sent us before we can do this.
     knowledge = {
-        'maas_url': get_recorded_maas_url(),
+        'maas_url': get_maas_url(),
         'api_credentials': get_recorded_api_credentials(),
         'nodegroup_uuid': get_recorded_nodegroup_uuid(),
     }

=== modified file 'src/provisioningserver/dhcp/tests/test_leases.py'
--- src/provisioningserver/dhcp/tests/test_leases.py	2012-11-26 02:54:49 +0000
+++ src/provisioningserver/dhcp/tests/test_leases.py	2012-12-12 15:57:42 +0000
@@ -328,15 +328,6 @@
             (get_write_time(leases_file), {params['ip']: params['mac']}),
             parse_leases_file())
 
-    def test_send_leases_does_nothing_without_maas_url(self):
-        self.patch(MAASClient, 'post', FakeMethod())
-        self.set_lease_state()
-        self.set_items_needed_for_lease_update()
-        self.clear_maas_url()
-        leases = factory.make_random_leases()
-        send_leases(leases)
-        self.assertEqual([], MAASClient.post.calls)
-
     def test_send_leases_does_nothing_without_credentials(self):
         self.patch(MAASClient, 'post', FakeMethod())
         self.set_items_needed_for_lease_update()

=== modified file 'src/provisioningserver/start_cluster_controller.py'
--- src/provisioningserver/start_cluster_controller.py	2012-12-04 00:52:42 +0000
+++ src/provisioningserver/start_cluster_controller.py	2012-12-12 15:57:42 +0000
@@ -32,6 +32,7 @@
     MAASDispatcher,
     NoAuth,
     )
+from provisioningserver.cluster_config import get_cluster_uuid
 from provisioningserver.network import discover_networks
 
 
@@ -65,14 +66,6 @@
     return MAASClient(NoAuth(), MAASDispatcher(), server_url)
 
 
-def get_cluster_uuid():
-    """Read this cluster's UUID from the config."""
-    # Import this lazily.  It reads config as a side effect, which can
-    # produce warnings.
-    from celery.app import app_or_default
-    return app_or_default().conf.CLUSTER_UUID
-
-
 def register(server_url):
     """Request Rabbit connection details from the domain controller.
 

=== modified file 'src/provisioningserver/tags.py'
--- src/provisioningserver/tags.py	2012-12-04 00:52:42 +0000
+++ src/provisioningserver/tags.py	2012-12-12 15:57:42 +0000
@@ -30,9 +30,9 @@
 from lxml import etree
 from provisioningserver.auth import (
     get_recorded_api_credentials,
-    get_recorded_maas_url,
     get_recorded_nodegroup_uuid,
     )
+from provisioningserver.cluster_config import get_maas_url
 import simplejson as json
 
 
@@ -51,10 +51,6 @@
 
     :return: (client, nodegroup_uuid)
     """
-    maas_url = get_recorded_maas_url()
-    if maas_url is None:
-        logger.error("Not updating tags: don't have API URL yet.")
-        return None, None
     api_credentials = get_recorded_api_credentials()
     if api_credentials is None:
         logger.error("Not updating tags: don't have API key yet.")
@@ -64,7 +60,7 @@
         logger.error("Not updating tags: don't have UUID yet.")
         return None, None
     client = MAASClient(MAASOAuth(*api_credentials), MAASDispatcher(),
-        maas_url)
+        get_maas_url())
     return client, nodegroup_uuid
 
 

=== modified file 'src/provisioningserver/tests/test_auth.py'
--- src/provisioningserver/tests/test_auth.py	2012-12-04 03:33:53 +0000
+++ src/provisioningserver/tests/test_auth.py	2012-12-12 15:57:42 +0000
@@ -14,8 +14,6 @@
 
 from apiclient.creds import convert_tuple_to_string
 from apiclient.testing.credentials import make_api_credentials
-from fixtures import EnvironmentVariableFixture
-from maastesting.factory import factory
 from provisioningserver import (
     auth,
     cache,
@@ -38,13 +36,3 @@
 
     def test_get_recorded_api_credentials_returns_None_without_creds(self):
         self.assertIsNone(auth.get_recorded_api_credentials())
-
-    def test_get_recorded_nodegroup_uuid_vs_record_nodegroup_uuid(self):
-        nodegroup_uuid = factory.make_name('nodegroupuuid')
-        auth.record_nodegroup_uuid(nodegroup_uuid)
-        self.assertEqual(nodegroup_uuid, auth.get_recorded_nodegroup_uuid())
-
-    def test_get_recorded_maas_url_uses_environment_override(self):
-        required_url = factory.make_name("MAAS_URL")
-        self.useFixture(EnvironmentVariableFixture("MAAS_URL", required_url))
-        self.assertEqual(required_url, auth.get_recorded_maas_url())

=== modified file 'src/provisioningserver/tests/test_boot_images.py'
--- src/provisioningserver/tests/test_boot_images.py	2012-11-26 02:54:49 +0000
+++ src/provisioningserver/tests/test_boot_images.py	2012-12-12 15:57:42 +0000
@@ -12,7 +12,6 @@
 __metaclass__ = type
 __all__ = []
 
-import os
 import json
 
 from apiclient.maas_client import MAASClient
@@ -46,15 +45,6 @@
         self.assertIs(sentinel.uuid, kwargs["nodegroup"])
         self.assertItemsEqual([image], json.loads(kwargs['images']))
 
-    def test_does_nothing_without_maas_url(self):
-        os.environ.pop("MAAS_URL")  # Ensure nothing is set.
-        self.set_api_credentials()
-        self.patch(
-            tftppath, 'list_boot_images',
-            Mock(return_value=make_boot_image_params()))
-        boot_images.report_to_server()
-        self.assertItemsEqual([], tftppath.list_boot_images.call_args_list)
-
     def test_does_nothing_without_credentials(self):
         self.set_maas_url()
         self.patch(

=== added file 'src/provisioningserver/tests/test_cluster_config.py'
--- src/provisioningserver/tests/test_cluster_config.py	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/tests/test_cluster_config.py	2012-12-12 15:57:42 +0000
@@ -0,0 +1,46 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for `provisioningserver.cluster_config`."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = []
+
+from fixtures import EnvironmentVariableFixture
+from maastesting.factory import factory
+from maastesting.testcase import TestCase
+from provisioningserver.cluster_config import (
+    get_cluster_uuid,
+    get_cluster_variable,
+    get_maas_url,
+    )
+
+
+class TestClusterConfig(TestCase):
+
+    def test_get_cluster_variable_reads_env(self):
+        var = factory.make_name('variable')
+        value = factory.make_name('value')
+        self.useFixture(EnvironmentVariableFixture(var, value))
+        self.assertEqual(value, get_cluster_variable(var))
+
+    def test_get_cluster_variable_fails_if_not_set(self):
+        self.assertRaises(
+            AssertionError,
+            get_cluster_variable, factory.make_name('nonexistent-variable'))
+
+    def test_get_cluster_uuid_reads_CLUSTER_UUID(self):
+        uuid = factory.make_name('uuid')
+        self.useFixture(EnvironmentVariableFixture('CLUSTER_UUID', uuid))
+        self.assertEqual(uuid, get_cluster_uuid())
+
+    def test_get_maas_url_reads_MAAS_URL(self):
+        maas_url = factory.make_name('maas_url')
+        self.useFixture(EnvironmentVariableFixture('MAAS_URL', maas_url))
+        self.assertEqual(maas_url, get_maas_url())

=== modified file 'src/provisioningserver/tftp.py'
--- src/provisioningserver/tftp.py	2012-12-11 12:00:25 +0000
+++ src/provisioningserver/tftp.py	2012-12-12 15:57:42 +0000
@@ -25,10 +25,10 @@
     urlparse,
     )
 
+from provisioningserver.cluster_config import get_cluster_uuid
 from provisioningserver.enum import ARP_HTYPE
 from provisioningserver.kernel_opts import KernelParameters
 from provisioningserver.pxe.config import render_pxe_config
-from provisioningserver.start_cluster_controller import get_cluster_uuid
 from provisioningserver.utils import deferred
 from tftp.backend import (
     FilesystemSynchronousBackend,
@@ -218,9 +218,6 @@
             params["local"] = local_host
             remote_host, remote_port = get("remote", (None, None))
             params["remote"] = remote_host
-            # XXX 2012-12-04 JeroenVermeulen bug=1086239: move
-            # get_cluster_uuid to a properly reusable location, and
-            # implement it without the celery import (and side effects).
             params["cluster_uuid"] = get_cluster_uuid()
             d = self.get_config_reader(params)
             d.addErrback(self.get_page_errback, file_name)