Merge lp:~mew/juju-deployer/option-from-file into lp:~gandelman-a/juju-deployer/trunk
- option-from-file
- Merge into trunk
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 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Adam Gandelman | Pending | ||
Review via email: mp+147236@code.launchpad.net |
Commit message
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_
http://
- 67. By Matthew Wedgwood
-
remove debugging cruft
Adam Gandelman (gandelman-a) wrote : | # |
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.
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_
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-
}
}
and have the charm do something like:
service-manager user-create username=
OR
user = subprocess.
passwd = subprocess.
utils.manager(
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
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) |
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?