launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/refactor-celerysettings into lp:maas.

Commit message:
Split the celeryconfig file into 2 files: one for the region controller and one for the cluster controller, both importing from a common config file.  Split the celeryd service (on a dev instance) into a region worker (in charge of DNS-related tasks, etc.) and a cluster controller in charge of the cluster-specific tasks (like powering up nodes, uploading DHCP, etc.)

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/refactor-celerysettings/+merge/126816

Split the celeryconfig file into 2 files: one for the region controller and one for the cluster controller, both importing from a common config file.  Split the celeryd service (on a dev instance) into a region worker (in charge of DNS-related tasks, etc.) and a cluster controller in charge of the cluster-specific tasks (powering up nodes, uploading DHCP, etc.)

= Notes =

- Turns out PXE_TEMPLATES_DIR is not used anywhere so I simply got rid of it.

- I've created two new services: cluster-worker and region-worker, they remplace the old celeryd service.

- All the common settings have been moved into a celeryconfig_common file.  Right now, the code relies on the fact that all the settings are available (because some of the settings are used "declaratively", like "@task(queue=celery_config.WORKER_QUEUE_DNS)") so even if strictly speaking, WORKER_QUEUE_DNS isn't used by the cluster code (this is a task only used by the region controller), it still needs to be there.

= Pre-imp =

This was discussed with Julian.

= Demo =

Run:
'make run-cluster'
'make run-region'
(Or all in one: 'make run')
And then watch logs/cluster-worker/current: you'll see the cluster tasks go through
You can also have a look at logs/webapp/current to see the initial API call used to exchange the RabbitMQ credentials.

= Next =

The packaging needs to be changed a bit in order for that change to be landed:
- the new file celeryconfig_common.py should be copied over by the region and the cluster packaging scripts.
- the cluster controller should:
    - copy celeryconfig_cluster.py instead of celeryconfig.py (which is now the region-specific config file.)
I'm working on it.
-- 
https://code.launchpad.net/~rvb/maas/refactor-celerysettings/+merge/126816
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/refactor-celerysettings into lp:maas.

=== modified file 'Makefile'
--- Makefile	2012-09-27 02:51:21 +0000
+++ Makefile	2012-09-28 13:45:25 +0000
@@ -192,8 +192,8 @@
 # Development services.
 #
 
-service_names_region := database dns reloader txlongpoll web webapp
-service_names_cluster := celeryd pserv reloader
+service_names_region := database dns region-worker reloader txlongpoll web webapp
+service_names_cluster := cluster-worker pserv reloader
 service_names_all := $(service_names_region) $(service_names_cluster)
 
 # The following template is intended to be used with `call`, and it
@@ -277,7 +277,9 @@
 
 services/dns/@deps: bin/py
 
-services/celeryd/@deps: bin/celeryd
+services/cluster-worker/@deps: bin/celeryd
+
+services/region-worker/@deps: bin/celeryd
 
 services/database/@deps: bin/database
 

=== modified file 'etc/celeryconfig.py'
--- etc/celeryconfig.py	2012-09-28 10:16:59 +0000
+++ etc/celeryconfig.py	2012-09-28 13:45:25 +0000
@@ -1,7 +1,7 @@
 # Copyright 2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""Celery settings for the maas project.
+"""Celery settings for the region controller.
 
 Do not edit this file.  Instead, put custom settings in a module named
 maas_local_celeryconfig.py somewhere on the PYTHONPATH.
@@ -17,51 +17,13 @@
 
 from datetime import timedelta
 
+import celeryconfig_common
 from maas import import_settings
 
-# Location of power action templates.  Use an absolute path, or leave as
-# None to use the templates installed with the running version of MAAS.
-POWER_TEMPLATES_DIR = None
-
-# Location of PXE config templates.  Use an absolute path, or leave as
-# None to use the templates installed with the running version of MAAS.
-PXE_TEMPLATES_DIR = None
-
-# Location of MAAS' bind configuration files.
-DNS_CONFIG_DIR = '/etc/bind/maas'
-
-# RNDC port to be configured by MAAS to communicate with the BIND
-# server.
-DNS_RNDC_PORT = 954
-
-# DHCP leases file, as maintained by ISC dhcpd.
-DHCP_LEASES_FILE = '/var/lib/maas/dhcp/dhcpd.leases'
-
-# ISC dhcpd configuration file.
-DHCP_CONFIG_FILE = '/etc/maas/dhcpd.conf'
-
-# List of interfaces that the dhcpd should service (if managed by MAAS).
-DHCP_INTERFACES_FILE = '/var/lib/maas/dhcpd-interfaces'
-
-# Broker connection information.  This is read by the region controller
-# and sent to connecting cluster controllers.
-# The cluster controllers currently read this same configuration file,
-# but the broker URL they receive from the region controller overrides
-# this setting.
-BROKER_URL = 'amqp://guest:guest@localhost:5672//'
-
-# Cluster UUID.  Will be overridden by the customized setting in the
-# local MAAS Celery config.
-CLUSTER_UUID = None
-
-# Location for log file.
-MAAS_CELERY_LOG = '/var/log/maas/celery.log'
-
-WORKER_QUEUE_DNS = 'celery'
-WORKER_QUEUE_BOOT_IMAGES = 'celery'
-# XXX rvb 2012-09-25, bug=1056250: the WORKER_QUEUE_CLUSTER should be
-# the uuid of the cluster controller.
-WORKER_QUEUE_CLUSTER = 'celery'
+# Silent lint, this will be defined by celeryconfig_common.
+WORKER_QUEUE_BOOT_IMAGES = None
+
+import_settings(celeryconfig_common)
 
 try:
     import maas_local_celeryconfig
@@ -71,35 +33,7 @@
     import_settings(maas_local_celeryconfig)
 
 
-# Each cluster should have its own queue created automatically by Celery.
-CELERY_CREATE_MISSING_QUEUES = True
-
-
-CELERY_IMPORTS = (
-    # Tasks.
-    "provisioningserver.tasks",
-
-    # This import is needed for its side effect: it initializes the
-    # cache that allows workers to share data.
-    "provisioningserver.initialize_cache",
-    )
-
-CELERY_ACKS_LATE = True
-
-# Do not store the tasks' return values (aka tombstones);
-# This improves performance.
-CELERY_IGNORE_RESULT = True
-
-
 CELERYBEAT_SCHEDULE = {
-    'unconditional-dhcp-lease-upload': {
-        'task': 'provisioningserver.tasks.upload_dhcp_leases',
-        'schedule': timedelta(minutes=1),
-        'options': {'queue': WORKER_QUEUE_CLUSTER},
-    },
-
-    # XXX rvb 2012-09-25, bug=1056250: this task should
-    # only on the master worker.
     'report-boot-images': {
         'task': 'provisioningserver.tasks.report_boot_images',
         'schedule': timedelta(minutes=5),

=== added file 'etc/celeryconfig_cluster.py'
--- etc/celeryconfig_cluster.py	1970-01-01 00:00:00 +0000
+++ etc/celeryconfig_cluster.py	2012-09-28 13:45:25 +0000
@@ -0,0 +1,42 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Celery settings for the cluster controller.
+
+Do not edit this file.  Instead, put custom settings in a module named
+maas_local_celeryconfig.py somewhere on the PYTHONPATH.
+"""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+)
+
+__metaclass__ = type
+
+from datetime import timedelta
+
+import celeryconfig_common
+from maas import import_settings
+
+# Silent lint, this will be defined by maas_local_celeryconfig.
+CLUSTER_UUID = None
+
+import_settings(celeryconfig_common)
+
+try:
+    import maas_local_celeryconfig
+except ImportError:
+    pass
+else:
+    import_settings(maas_local_celeryconfig)
+
+
+CELERYBEAT_SCHEDULE = {
+    'unconditional-dhcp-lease-upload': {
+        'task': 'provisioningserver.tasks.upload_dhcp_leases',
+        'schedule': timedelta(minutes=1),
+        'options': {'queue': CLUSTER_UUID},
+    },
+}

=== added file 'etc/celeryconfig_common.py'
--- etc/celeryconfig_common.py	1970-01-01 00:00:00 +0000
+++ etc/celeryconfig_common.py	2012-09-28 13:45:25 +0000
@@ -0,0 +1,72 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Celery settings common to the region and the cluster controllers."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+)
+
+__metaclass__ = type
+
+
+# Location of power action templates.  Use an absolute path, or leave as
+# None to use the templates installed with the running version of MAAS.
+POWER_TEMPLATES_DIR = None
+
+# Location of MAAS' bind configuration files.
+DNS_CONFIG_DIR = '/etc/bind/maas'
+
+# RNDC port to be configured by MAAS to communicate with the BIND
+# server.
+DNS_RNDC_PORT = 954
+
+# DHCP leases file, as maintained by ISC dhcpd.
+DHCP_LEASES_FILE = '/var/lib/maas/dhcp/dhcpd.leases'
+
+# ISC dhcpd configuration file.
+DHCP_CONFIG_FILE = '/etc/maas/dhcpd.conf'
+
+# List of interfaces that the dhcpd should service (if managed by MAAS).
+DHCP_INTERFACES_FILE = '/var/lib/maas/dhcpd-interfaces'
+
+# Broker connection information.  This is read by the region controller
+# and sent to connecting cluster controllers.
+# The cluster controllers currently read this same configuration file,
+# but the broker URL they receive from the region controller overrides
+# this setting.
+BROKER_URL = 'amqp://guest:guest@localhost:5672//'
+
+# Cluster UUID.  Will be overridden by the customized setting in the
+# local MAAS Celery config.
+CLUSTER_UUID = None
+
+# Location for log file.
+MAAS_CELERY_LOG = '/var/log/maas/celery.log'
+
+
+WORKER_QUEUE_DNS = 'celery'
+WORKER_QUEUE_BOOT_IMAGES = 'celery'
+# The CLUSTER_UUID will be defined in the local configuration file of
+# the cluster controller (maas_local_celeryconfig.py).
+CLUSTER_UUID = None
+
+# Each cluster should have its own queue created automatically by Celery.
+CELERY_CREATE_MISSING_QUEUES = True
+
+CELERY_IMPORTS = (
+    # Tasks.
+    "provisioningserver.tasks",
+
+    # This import is needed for its side effect: it initializes the
+    # cache that allows workers to share data.
+    "provisioningserver.initialize_cache",
+    )
+
+CELERY_ACKS_LATE = True
+
+# Do not store the tasks' return values (aka tombstones);
+# This improves performance.
+CELERY_IGNORE_RESULT = True

=== modified file 'etc/democeleryconfig.py'
--- etc/democeleryconfig.py	2012-09-28 10:16:59 +0000
+++ etc/democeleryconfig.py	2012-09-28 13:45:25 +0000
@@ -1,7 +1,7 @@
 # Copyright 2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""Celery demo settings for the maas project."""
+"""Celery demo settings for the maas project: region settings."""
 
 from __future__ import (
     absolute_import,
@@ -11,36 +11,21 @@
 
 __metaclass__ = type
 
+
 import os
 
 import celeryconfig
+import democeleryconfig_common
 from maas import import_settings
 
+# Silent lint, this will be defined by democeleryconfig_common.
+DEV_ROOT_DIRECTORY = None
+
 # Extend base settings.
 import_settings(celeryconfig)
 
-
-DEV_ROOT_DIRECTORY = os.path.join(
-    os.path.dirname(__file__), os.pardir)
-
-
-DNS_CONFIG_DIR = os.path.join(
-    DEV_ROOT_DIRECTORY, 'run/named/')
-
-
-DNS_RNDC_PORT = 9154
-
-
-DHCP_CONFIG_FILE = os.path.join(
-    DEV_ROOT_DIRECTORY, 'run/dhcpd.conf')
-
-
-DHCP_LEASES_FILE = os.path.join(
-    DEV_ROOT_DIRECTORY, 'run/dhcpd.leases')
-
-
-CLUSTER_UUID = "adfd3977-f251-4f2c-8d61-745dbd690bfc"
-
+import_settings(democeleryconfig_common)
 
 MAAS_CELERY_LOG = os.path.join(
-    DEV_ROOT_DIRECTORY, 'logs/celeryd/current')
+    DEV_ROOT_DIRECTORY, 'logs/region-worker/current')
+

=== added file 'etc/democeleryconfig_cluster.py'
--- etc/democeleryconfig_cluster.py	1970-01-01 00:00:00 +0000
+++ etc/democeleryconfig_cluster.py	2012-09-28 13:45:25 +0000
@@ -0,0 +1,31 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Celery demo settings for the maas project: cluster settings."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+)
+
+__metaclass__ = type
+
+
+import os
+
+import celeryconfig_cluster
+import democeleryconfig_common
+from maas import import_settings
+
+# Silent lint, this will be defined by democeleryconfig_common.
+DEV_ROOT_DIRECTORY = None
+
+# Extend base settings.
+import_settings(celeryconfig_cluster)
+
+import_settings(democeleryconfig_common)
+
+MAAS_CELERY_LOG = os.path.join(
+    DEV_ROOT_DIRECTORY, 'logs/cluster-worker/current')
+

=== added file 'etc/democeleryconfig_common.py'
--- etc/democeleryconfig_common.py	1970-01-01 00:00:00 +0000
+++ etc/democeleryconfig_common.py	2012-09-28 13:45:25 +0000
@@ -0,0 +1,33 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Celery demo settings for the maas project: common settings."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+)
+
+__metaclass__ = type
+
+import os
+
+
+DEV_ROOT_DIRECTORY = os.path.join(
+    os.path.dirname(__file__), os.pardir)
+
+
+DNS_CONFIG_DIR = os.path.join(
+    DEV_ROOT_DIRECTORY, 'run/named/')
+
+
+DNS_RNDC_PORT = 9154
+
+
+DHCP_CONFIG_FILE = os.path.join(
+    DEV_ROOT_DIRECTORY, 'run/dhcpd.conf')
+
+
+DHCP_LEASES_FILE = os.path.join(
+    DEV_ROOT_DIRECTORY, 'run/dhcpd.leases')

=== removed directory 'services/celeryd'
=== removed file 'services/celeryd/down'
=== removed file 'services/celeryd/run'
--- services/celeryd/run	2012-09-27 02:51:21 +0000
+++ services/celeryd/run	1970-01-01 00:00:00 +0000
@@ -1,22 +0,0 @@
-#!/usr/bin/env bash
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-# Exit immediately if a command exits with a non-zero status.
-set -o errexit
-# Treat unset variables as an error when substituting.
-set -o nounset
-
-# Move to the project root.
-cd "$(dirname "$0")/../.."
-
-# Start logging, if requested. Not using multilog here right now
-# because there are race issues when restarting.
-[ -z "${logdir:-}" ] || exec &>> "${logdir}/current"
-
-# XXX JeroenVermeulen 2012-08-23, bug=1040529: Use fghack to kludge around
-# hanging celery shutdown.
-script="$(readlink -f bin/celeryd)"
-exec fghack "${script}" \
-    --loglevel INFO --beat --queues celery \
-    --config democeleryconfig

=== added directory 'services/cluster-worker'
=== added file 'services/cluster-worker/down'
=== added file 'services/cluster-worker/run'
--- services/cluster-worker/run	1970-01-01 00:00:00 +0000
+++ services/cluster-worker/run	2012-09-28 13:45:25 +0000
@@ -0,0 +1,34 @@
+#!/usr/bin/env bash
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+# Exit immediately if a command exits with a non-zero status.
+set -o errexit
+# Treat unset variables as an error when substituting.
+set -o nounset
+
+# Move to the project root.
+cd "$(dirname "$0")/../.."
+
+# Start logging, if requested. Not using multilog here right now
+# because there are race issues when restarting.
+[ -z "${logdir:-}" ] || exec &>> "${logdir}/current"
+
+# Simulate what the package does:
+# - generate a uuid
+# - create a maas_local_celeryconfig.py file with CLUSTER_UUID=$uuid and
+#   make sure that file is on the path.
+# - run bin/maas-provision start-cluster-controller
+local_config="run/cluster-worker/maas_local_celeryconfig.py"
+if [ ! -f $local_config ];
+then
+uuid=`uuidgen`
+mkdir -p run/cluster-worker/
+cat <<EOF > "$local_config"
+CLUSTER_UUID='$uuid'
+EOF
+fi
+export PYTHONPATH=run/cluster-worker/:etc/:src/
+export CELERY_CONFIG_MODULE=democeleryconfig_cluster
+script="$(readlink -f bin/maas-provision)"
+exec fghack "${script}" start-cluster-controller http://0.0.0.0:5240/

=== added directory 'services/region-worker'
=== added file 'services/region-worker/down'
=== added file 'services/region-worker/run'
--- services/region-worker/run	1970-01-01 00:00:00 +0000
+++ services/region-worker/run	2012-09-28 13:45:25 +0000
@@ -0,0 +1,22 @@
+#!/usr/bin/env bash
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+# Exit immediately if a command exits with a non-zero status.
+set -o errexit
+# Treat unset variables as an error when substituting.
+set -o nounset
+
+# Move to the project root.
+cd "$(dirname "$0")/../.."
+
+# Start logging, if requested. Not using multilog here right now
+# because there are race issues when restarting.
+[ -z "${logdir:-}" ] || exec &>> "${logdir}/current"
+
+# XXX JeroenVermeulen 2012-08-23, bug=1040529: Use fghack to kludge around
+# hanging celery shutdown.
+script="$(readlink -f bin/celeryd)"
+exec fghack "${script}" \
+    --loglevel INFO --beat --queues celery,master \
+    --config democeleryconfig