Merge lp:~rvb/maas/refactor-celerysettings into lp:~maas-committers/maas/trunk

[Raphaël Badin]

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.)

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.

[Jeroen T. Vermeulen (jtv)]

I like the nomenclature of "region worker" vs. "cluster worker."

PXE_TEMPLATES_DIR was used once upon a time, but somebody realized that we only had one template and so dropped the directory meant for PXE templates... juuuussst before we had to create multiple PXE templates. And then we unified them again.

[Gavin Panella (allenap)]

> PXE_TEMPLATES_DIR was used once upon a time, but somebody realized that we
> only had one template and so dropped the directory meant for PXE templates...
> juuuussst before we had to create multiple PXE templates. And then we unified
> them again.

Erm, I suspect that was me. Still, I'm not sure that the celery stuff actually needs to know where those templates are anyway, only the TFTP server.

[Jeroen T. Vermeulen (jtv)]

Wonderful. This makes all kind of sense.

Apart maybe from having CLUSTER_UUID in the common settings. :)

review: Approve

[Raphaël Badin (rvb)]

> Wonderful. This makes all kind of sense.
>
> Apart maybe from having CLUSTER_UUID in the common settings. :)

Right, I've moved it into etc/celeryconfig_cluster.py.

Thanks for the review!

=== modified file 'Makefile'
--- Makefile	2012-09-27 02:51:21 +0000
+++ Makefile	2012-09-28 23:10:28 +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 23:10:28 +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'
+# Silence 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 23:10:28 +0000
@@ -0,0 +1,43 @@
+# 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
+
+# Cluster UUID.  Will be overridden by the customized setting in the
+# local MAAS Celery config.
+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 23:10:28 +0000
@@ -0,0 +1,65 @@
+# 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//'
+
+# Location for log file.
+MAAS_CELERY_LOG = '/var/log/maas/celery.log'
+
+
+WORKER_QUEUE_DNS = 'celery'
+WORKER_QUEUE_BOOT_IMAGES = 'celery'
+
+# 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 23:10:28 +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
 
+# Silence 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 23:10:28 +0000
@@ -0,0 +1,35 @@
+# 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
+
+# Silence lint, this will be defined by democeleryconfig_common.
+DEV_ROOT_DIRECTORY = None
+
+# Extend base settings.
+import_settings(celeryconfig_cluster)
+
+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"
+
+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 23:10:28 +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 23:10:28 +0000
@@ -0,0 +1,20 @@
+#!/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"
+
+export PYTHONPATH=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 23:10:28 +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

=== modified file 'setup.py'
--- setup.py	2012-09-28 20:32:53 +0000
+++ setup.py	2012-09-28 23:10:28 +0000
@@ -67,7 +67,9 @@
             ['etc/cron.d/maas-gc']),
         ('/usr/share/maas',
             ['contrib/wsgi.py',
-             'etc/celeryconfig.py']),
+             'etc/celeryconfig.py',
+             'etc/celeryconfig_cluster.py',
+             'etc/celeryconfig_common.py']),
         ('/usr/share/maas/preseeds',
             ['contrib/preseeds_v2/commissioning',
              'contrib/preseeds_v2/enlist',