Merge lp:~mew/juju-deployer/option-from-file into lp:~gandelman-a/juju-deployer/trunk

Proposed by Matthew Wedgwood
Status: Merged
Merged at revision: 65
Proposed branch: lp:~mew/juju-deployer/option-from-file
Merge into: lp:~gandelman-a/juju-deployer/trunk
Diff against target: 256 lines (+96/-57)
2 files modified
deployer.py (+48/-19)
utils.py (+48/-38)
To merge this branch: bzr merge lp:~mew/juju-deployer/option-from-file
Reviewer Review Type Date Requested Status
Adam Gandelman Pending
Review via email: mp+147236@code.launchpad.net

Description of the change

Allow the value of a charm config option to be read from a file (and optionally base64 encoded). The use case for this is config options with multi-line or encoded values, such as the "vhost_https_template" option from the apache2 charm.
http://jujucharms.com/charms/precise/apache2/config

To post a comment you must log in.
67. By Matthew Wedgwood

remove debugging cruft

Revision history for this message
Adam Gandelman (gandelman-a) wrote :

Matthew, this generally works good. Just wondering if the read()s should also strip() trailing newlines to keep them from showing up as part of the config value in the environment?

Revision history for this message
Matthew Wedgwood (mew) wrote :

Good point. It's probably better to filter the text so that all of the anti-json characters are escaped. I'll look into doing that.

Revision history for this message
Matthew Wedgwood (mew) wrote :

Sorry, I should have thought about that a bit more thoroughly. Escaping isn't the problem, is it?

The intention of include-file and include-base64 is to provide a way to pull in values that may be built externally or have complex internal structure that would be difficult to maintain inside a JSON (or YAML) file.

In the example I mentioned above, "vhost_https_template" option from the apache2 charm is a jinja2 templated httpd.conf-style configuration hunk. It's usually hand-maintained. I could also imagine something like a crontab file being included this way, and I believe cron is particular about having a trailing newline.

Revision history for this message
Adam Gandelman (gandelman-a) wrote :

Sorry for the delay. I had meant to test to use case I had in mind but haven't gotten to it yet. Looking at it again, I think this is generally okay.

I was thinking of a case where i'd like to store a passwd outside of the charm in a file, eg:

service1: {
 options: {
   user: adam
   passwd: include-file:///home/adam/.services/passwd
 }
}

and have the charm do something like:

service-manager user-create username=$(config-get username) password=$(config-get passwd)

OR

user = subprocess.check_output(['config-get', 'user'])
passwd = subprocess.check_output(['config-get', 'passwd'])
utils.manager(username=user, passwd=passwd)

I think this would be okay in both cases. The bash case would strip out newlines, anyway and the python case should probably cleaning up stdout anyway. I'll give this branch just another test one more time otherwise LGTM.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'deployer.py'
2--- deployer.py 2013-02-01 01:39:36 +0000
3+++ deployer.py 2013-02-07 22:06:20 +0000
4@@ -23,10 +23,13 @@
5
6 parser = optparse.OptionParser()
7 parser.add_option('-c', '--config',
8- help='File containing deployment(s) json config',
9- dest='config', action='store', default="deployments.cfg")
10+ help='File containing deployment(s) json config. This option can be repeated, with later files overriding values in earlier ones.',
11+ dest='configs', action='append')
12 parser.add_option('-d', '--debug', help='Enable debugging to stdout', dest="debug",
13 action="store_true", default=False)
14+parser.add_option('-L', '--local-mods',
15+ help='Allow deployment of locally-modified charms',
16+ dest="no_local_mods", default=True, action='store_false')
17 parser.add_option('-u', '--update-charms', help='Update existing charm branches',
18 dest="update_charms", default=False, action="store_true")
19 parser.add_option('-l', '--ls', help='List available deployments',
20@@ -71,7 +74,8 @@
21 'subordinates started. (default: 60)')
22 (opts, args) = parser.parse_args()
23
24-config=opts.config
25+if not opts.configs:
26+ opts.configs = ['deployments.cfg']
27 update_charms=opts.update_charms
28
29 # temporarily abuse __builtin__ till this is setup properly
30@@ -94,12 +98,20 @@
31 exit(rc)
32
33 # load the json configuration for possible deployments.
34-if not os.path.exists(config):
35- log.error("Cannot load deployment configuration: %s", config)
36+missing_configs = [c for c in opts.configs if not os.path.exists(c)]
37+if missing_configs:
38+ log.error("Configuration not found: {}".format(", ".join(missing_configs)))
39 exit(1)
40
41-debug_msg("Loading deployments from %s" % config)
42-cfg = json.load(open(config, "r"))
43+debug_msg("Loading deployments from {}".format(", ".join(opts.configs)))
44+cfg = {}
45+for config in opts.configs:
46+ with open(config,'r') as f:
47+ try:
48+ cfg = dict_merge(cfg, json.load(f))
49+ except ValueError:
50+ log.error("Error parsing config: {}".format(config))
51+ exit(1)
52
53 if opts.list_deploys:
54 display_deploys(cfg)
55@@ -121,23 +133,39 @@
56 else:
57 debug_msg("Series charm store already exists: %s" % series_store)
58
59-os.chdir(SERIES)
60-
61 # either clone all charms if we dont have them or update branches
62 for k in CHARMS.keys():
63 charm_path = "%s/%s" % (series_store, k)
64 debug_msg("Charm '%s' - using charm path '%s'" % (k, charm_path))
65+ (branch, sep, revno) = CHARMS[k]["branch"].partition('@')
66+ debug_msg("Branch: {}, revision: {}".format(branch, revno))
67 if os.path.exists(charm_path):
68+ if opts.no_local_mods:
69+ with cd(charm_path):
70+ # is there a better way to check for changes?
71+ bzrstatus = subprocess.check_output(['bzr','st']).strip()
72+ if bzrstatus not in ("","working tree is out of date, run 'bzr update'"):
73+ log.error("Charm is locally modified: {}".format(
74+ charm_path))
75+ log.error("Aborting")
76+ exit(1)
77 debug_msg("Charm path exists @ %s." % charm_path)
78 if update_charms:
79 debug_msg("Updating charm branch '%s'" % k)
80- # This uses the remembered pull location, if it's changed in the
81- # config since the branch contents may not be as expected.
82- code = subprocess.call(["bzr", "pull", "-d", charm_path])
83- # TODO: react to non-zero return code from bzr
84+ code = subprocess.call(
85+ ["bzr", "pull", "-d", charm_path, '--remember', branch])
86+ if code != 0:
87+ log.error("Could not update branch at {} from {}".format(
88+ charm_path, branch))
89+ exit(code)
90 else:
91- print "- Cloning %s from %s" % (k, CHARMS[k]["branch"])
92- subprocess.call(["bzr", "branch", CHARMS[k]["branch"], k])
93+ print "- Cloning %s from %s" % (k, branch)
94+ subprocess.call(["bzr", "branch", branch, charm_path])
95+ if revno:
96+ code = subprocess.call(["bzr", "update", charm_path, '-r', revno])
97+ if code != 0:
98+ log.error("Unable to check out branch revision {}".format(revno))
99+ exit(code)
100 # load charms metadata
101 mdf = open("%s/metadata.yaml" % charm_path, "r")
102 debug_msg("Loading metadata from %s/metadata.yaml" % charm_path)
103@@ -280,10 +308,11 @@
104 # Ensure they're started, if they exist.
105 wait_for_subordinates_started(opts.debug, opts.juju_env)
106
107-print "- Sleeping for %s before ensuring relation state." % opts.rel_wait
108-# give all relations a minute to settle down, and make sure no errors are
109-# reported.
110-time.sleep(float(opts.rel_wait))
111+if RELATIONS:
112+ print "- Sleeping for %s before ensuring relation state." % opts.rel_wait
113+ # give all relations a minute to settle down, and make sure no errors are
114+ # reported.
115+ time.sleep(float(opts.rel_wait))
116
117 if not ensure_relations_up(juju_status(opts.juju_env)):
118 exit(1)
119
120=== modified file 'utils.py'
121--- utils.py 2013-02-01 01:38:29 +0000
122+++ utils.py 2013-02-07 22:06:20 +0000
123@@ -5,11 +5,36 @@
124 import pdb
125 import sys
126 import time
127+import os
128+from contextlib import contextmanager
129+from copy import deepcopy
130+from base64 import b64encode
131
132
133 log = logging.getLogger("juju-deployer")
134 debug_msg = log.debug
135
136+
137+@contextmanager
138+def cd(path):
139+ cwd = os.getcwd()
140+ os.chdir(path)
141+ try:
142+ yield
143+ finally:
144+ os.chdir(cwd)
145+
146+
147+def dict_merge(onto, source):
148+ target = deepcopy(onto)
149+ for (key, value) in source.items():
150+ if key in target and isinstance(target[key], dict) and isinstance(value, dict):
151+ target[key] = dict_merge(target[key], value)
152+ else:
153+ target[key] = value
154+ return target
155+
156+
157 def init_logging(filename, debug):
158 """Set up logging handlers to write messages to stdout and a file"""
159 log.setLevel(logging.DEBUG)
160@@ -140,11 +165,18 @@
161 debug_msg("WARNING: Options passed to %s, but it has" \
162 "no config.yaml" % (c))
163 else:
164- for opt in charms[c]["options"].keys():
165+ for opt, val in charms[c]["options"].items():
166 if opt in charms[c]["config"].keys():
167 debug_msg("Adding option to '%s' to deploy config: %s" \
168 % (opt, c))
169- cc[opt] = charms[c]["options"][opt]
170+ if isinstance(val, basestring) and val.startswith("include-file://"):
171+ with open(val[15:], 'r') as f:
172+ cc[opt] = f.read()
173+ elif isinstance(val, basestring) and val.startswith("include-base64://"):
174+ with open(val[17:], 'r') as f:
175+ cc[opt] = b64encode(f.read())
176+ else:
177+ cc[opt] = val
178 else:
179 debug_msg("WARNING: Skipping unknown config option to" \
180 "%s: %s" % (c, opt))
181@@ -376,7 +408,10 @@
182
183 env_config = EnvironmentsConfig()
184 env_config.load_or_write_sample()
185- environment = env_config.get_default()
186+ if os.environ.has_key("JUJU_ENV"):
187+ environment = env_config.get(os.environ.get("JUJU_ENV"))
188+ else:
189+ environment = env_config.get_default()
190
191 @inlineCallbacks
192 def _clean_juju_state():
193@@ -412,19 +447,13 @@
194 reactor.run()
195
196 def load_deployment(config, deployment):
197- relations = {}
198- series = None
199- charms = {}
200- overrides = {}
201 if deployment not in config.keys():
202 log.error("deployment %s not found.", deployment)
203 display_deploys(config)
204 exit(1)
205
206- if 'series' in config[deployment]:
207- series = config[deployment]['series']
208-
209- if 'inherits' in config[deployment]:
210+ deploy_config = config[deployment]
211+ if 'inherits' in deploy_config:
212 cur = config[deployment]
213 configs = []
214
215@@ -442,33 +471,14 @@
216 configs.insert(0, base)
217 configs.append(config[deployment])
218
219- for config in configs:
220- if 'series' in config:
221- series = config['series']
222- if 'overrides' in config:
223- for k, v in config['overrides'].iteritems():
224- overrides[k] = v
225- if 'services' in config:
226- for k, v in config['services'].iteritems():
227- if k not in charms:
228- charms[k] = v
229- else:
230- if 'branch' in v:
231- charms[k]['branch'] = v['branch']
232- if 'options' in v:
233- for opt, value in v['options'].iteritems():
234- charms[k]['options'][opt] = value
235- if 'relations' in config:
236- rels = relations_json_to_tuples(config['relations'])
237- for w, rs in rels.iteritems():
238- if w in relations:
239- [relations[w].append(r) for r in rs]
240- else:
241- relations[w] = rs
242+ deploy_config = reduce(dict_merge, configs)
243+
244+ series = deploy_config.get('series')
245+ charms = deploy_config.get('services',{})
246+ overrides = deploy_config.get('overrides',{})
247+ if 'relations' in deploy_config:
248+ relations = relations_json_to_tuples(deploy_config["relations"])
249 else:
250- series = config[deployment]['series']
251- charms = config[deployment]['services']
252- if 'relations' in config[deployment]:
253- relations = relations_json_to_tuples(config[deployment]["relations"])
254+ relations = {}
255
256 return (series, charms, relations, overrides)

Subscribers

People subscribed via source and target branches