Merge lp:~jtv/maas/pkg-bug-1086239 into lp:~maas-maintainers/maas/packaging

Proposed by Jeroen T. Vermeulen on 2012-12-11
Status: Merged
Approved by: Jeroen T. Vermeulen on 2012-12-11
Approved revision: 158
Merged at revision: 156
Proposed branch: lp:~jtv/maas/pkg-bug-1086239
Merge into: lp:~maas-maintainers/maas/packaging
Diff against target: 112 lines (+63/-10)
3 files modified
debian/changelog (+1/-1)
debian/maas-cluster-controller.maas-pserv.upstart (+16/-2)
debian/maas-cluster-controller.postinst (+46/-7)
To merge this branch: bzr merge lp:~jtv/maas/pkg-bug-1086239
Reviewer Review Type Date Requested Status
Julian Edwards (community) 2012-12-11 Approve on 2012-12-11
Review via email: mp+139146@code.launchpad.net

Commit message

Duplicate CLUSTER_UUID setting from celery config into maas_cluster.conf.

Description of the change

As per discussion with Julian. The tftpd needs to know the UUID for the cluster it serves, in order to pass it to the pxeconfig API call. Currently that UUID is only configured in the celery config, and we don't want to import that into the pserv process (which is what serves our tftp). We intend to clean up the duplication later, leaving only the new location for the UUID setting, where both our celery processes and our pserv processes can read it.

Once this branch is landed, we can have pserv read the UUID from maas_cluster.conf instead of from maas_local_celeryconfig_cluster.py. To prepare for this, I made the upstart script for pserv read the cluster config just like the one for the main cluster process does.

The main cost factor for this branch was our inability to unit-test the packaging scripts. I verified manually that the new code does the right thing both during upgrade and in a fresh install.

Jeroen

To post a comment you must log in.
lp:~jtv/maas/pkg-bug-1086239 updated on 2012-12-11
157. By Jeroen T. Vermeulen on 2012-12-11

Read cluster config before starting pserv.

158. By Jeroen T. Vermeulen on 2012-12-11

Set CONFIG_FILE, as in the celery upstart config.

Julian Edwards (julian-edwards) wrote :

20 +pre-start script
21 + if [ ! -f $CONFIG_FILE ]; then
22 + echo "$CONFIG_FILE does not exist. Aborting."

You've not set CONFIG_FILE anywhere.

47 + grep "^CLUSTER_UUID *= *[\"'][^\"']*[\"']" $config_file |
48 + sed -e "s/^CLUSTER_UUID *= *\"\([^\"']*\)\".*/\1/"

MY EYES. THEY BURN.

78 + # Write it to maas_cluster.conf as well. There is no initial
79 + # placeholder in this file, so just append the setting.
80 + echo "CLUSTER_UUID=\"$uuid\"" >>/etc/maas/maas_cluster.conf

This will duplicate lines every time the package is upgraded.

review: Needs Fixing
Julian Edwards (julian-edwards) wrote :

> This will duplicate lines every time the package is upgraded.

ah bollocks, I missed that you check much earlier in the func

Still needs fixing because of the earlier problem though!

Jeroen T. Vermeulen (jtv) wrote :

I fixed the other problem in the meantime. I'm working on the follow-up branch that will allow me to test it; it was a last-minute change that's currently still unused.

Julian Edwards (julian-edwards) wrote :

Ok approved on that basis since I'm EODing now. Cheers.

review: Approve
Gavin Panella (allenap) wrote :

Shell, and the tools typically available therein (i.e. sed, awk,
etc.), is a difficult environment in which to edit config files. Shell
- and I'm mainly thinking of /bin/sh and its descendants - is
wonderful for joining processes together, but beyond that it gets into
quick and dirty territory. Which can be exactly what we need for
interactive use. However, the scripting capabilities are well into the
toxic sludge end of dirty.

I've been thinking about this problem a bit.

Bazaar uses configobj to manage its configuration files, which allows
Bazaar to edit config files (in Python, not shell) without losing
comments and formatting. This would be quite neat, but I'm not sure
it's a low enough common denominator; we still have shell scripts in
MAAS that need to ingest something as stupid as they are, and the .ini
style files that configobj works with are just too structured.

I can't remember all of my thought processes, but I think the best way
to do this such that shell scripts can work with config (and are less
likely to fuck it up) is to store one shared configuration value per
file. This means that the cluster UUID should go in, say:

  /etc/maas/cluster/uuid

The contents of this file should be the UUID and nothing else.

I think this approach would work better with shell scripts that need
to share configration with other tools, and with packaging. It would
give us the freedom to move away from these awful packaging hacks
which, apart from being less readable than obfuscated Perl, are also
untested and difficult to test.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2012-12-03 13:13:07 +0000
3+++ debian/changelog 2012-12-11 09:05:25 +0000
4@@ -1,4 +1,4 @@
5-maas (0.1+bzr1366+dfsg-0ubuntu2) UNRELEASED; urgency=low
6+maas (0.1+bzr1393+dfsg-0ubuntu2) UNRELEASED; urgency=low
7
8 [ Raphaƫl Badin ]
9 * debian/maas-dns.postinst: Call write_dns_config.
10
11=== modified file 'debian/maas-cluster-controller.maas-pserv.upstart'
12--- debian/maas-cluster-controller.maas-pserv.upstart 2012-09-25 08:02:30 +0000
13+++ debian/maas-cluster-controller.maas-pserv.upstart 2012-12-11 09:05:25 +0000
14@@ -10,5 +10,19 @@
15
16 respawn
17
18-# To add options to your daemon, edit the line below:
19-exec /usr/bin/twistd -n --uid=maas --gid=maas --pidfile=/run/maas-pserv.pid --logfile=/dev/null maas-pserv --config-file=/etc/maas/pserv.yaml
20+env CONFIG_FILE=/etc/maas/maas_cluster.conf
21+
22+pre-start script
23+ if [ ! -f $CONFIG_FILE ]; then
24+ echo "$CONFIG_FILE does not exist. Aborting."
25+ stop
26+ exit 0
27+ fi
28+end script
29+
30+script
31+ # Prepare settings.
32+ . $CONFIG_FILE
33+ # To add options to your daemon, edit the line below:
34+ exec /usr/bin/twistd -n --uid=maas --gid=maas --pidfile=/run/maas-pserv.pid --logfile=/dev/null maas-pserv --config-file=/etc/maas/pserv.yaml
35+end script
36
37=== modified file 'debian/maas-cluster-controller.postinst'
38--- debian/maas-cluster-controller.postinst 2012-11-27 14:08:08 +0000
39+++ debian/maas-cluster-controller.postinst 2012-12-11 09:05:25 +0000
40@@ -26,6 +26,48 @@
41 ln -sf /var/lib/maas/ephemeral/tgt.conf /etc/tgt/conf.d/maas.conf
42 }
43
44+extract_cluster_uuid(){
45+ # Extract ClUSTER_UUID setting from config file $1. This will work
46+ # both the cluster celery config (which is python) and the cluster
47+ # config (which is shell).
48+ local config_file="$1"
49+ grep "^CLUSTER_UUID *= *[\"'][^\"']*[\"']" $config_file |
50+ sed -e "s/^CLUSTER_UUID *= *\"\([^\"']*\)\".*/\1/"
51+}
52+
53+configure_cluster_uuid(){
54+ # The cluster uuid goes into maas_cluster.conf, but we also still
55+ # keep a copy in maas_local_celeryconfig_cluster.py (hopefully just
56+ # temporarily). If an old uuid is configured, we replicate that to
57+ # maas_cluster.conf; otherwise, we want to generate one.
58+ local uuid
59+
60+
61+ if [ -n "$(extract_cluster_uuid /etc/maas/maas_cluster.conf)" ]; then
62+ # UUID is already set up. Wonderful.
63+ return
64+ fi
65+
66+ # Look for a UUID stored in the old location.
67+ uuid="$(extract_cluster_uuid /etc/maas/maas_local_celeryconfig_cluster.py)"
68+
69+ if [ -z "$uuid" ]; then
70+ # No UUID at all yet. Generate one, and insert it into its
71+ # placeholder in the old config location.
72+ uuid="$(uuidgen)"
73+ sed -i "s|^CLUSTER_UUID = None$|CLUSTER_UUID = '$uuid'|" \
74+ /etc/maas/maas_local_celeryconfig_cluster.py
75+ fi
76+
77+ # Either way, at this point we have a uuid, and it is configured in
78+ # the old config location.
79+ #
80+ # Write it to maas_cluster.conf as well. There is no initial
81+ # placeholder in this file, so just append the setting.
82+ echo "CLUSTER_UUID=\"$uuid\"" >>/etc/maas/maas_cluster.conf
83+}
84+
85+
86 if [ "$1" = "configure" ] && [ -z "$2" ]; then
87 # logging
88 create_log_dir
89@@ -35,13 +77,6 @@
90 chown root:maas /etc/maas/maas_local_celeryconfig_cluster.py
91 chmod 0640 /etc/maas/maas_local_celeryconfig_cluster.py
92
93- # Generate cluster UUID.
94- if grep -qs "^CLUSTER_UUID\ \= None$" /etc/maas/maas_local_celeryconfig_cluster.py; then
95- uuid="$(uuidgen)"
96- sed -i "s|^CLUSTER_UUID\ \= None$|CLUSTER_UUID = '"$uuid"'|" \
97- /etc/maas/maas_local_celeryconfig_cluster.py
98- fi
99-
100 configure_maas_tgt
101 fi
102
103@@ -63,5 +98,9 @@
104 fi
105 fi
106
107+if [ "$1" = "configure" ]; then
108+ configure_cluster_uuid
109+fi
110+
111 #DEBHELPER#
112 exit 0

Subscribers

People subscribed via source and target branches