launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #14785
[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)