Merge lp:~hazmat/pyjuju/ostack-constraints into lp:pyjuju
- ostack-constraints
- Merge into trunk
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | 560 | ||||
Proposed branch: | lp:~hazmat/pyjuju/ostack-constraints | ||||
Merge into: | lp:pyjuju | ||||
Diff against target: |
942 lines (+282/-129) 12 files modified
juju/providers/common/instance_type.py (+52/-0) juju/providers/common/tests/test_instance_type.py (+53/-0) juju/providers/openstack/__init__.py (+2/-0) juju/providers/openstack/client.py (+23/-20) juju/providers/openstack/files.py (+4/-4) juju/providers/openstack/launch.py (+26/-15) juju/providers/openstack/ports.py (+16/-18) juju/providers/openstack/provider.py (+18/-9) juju/providers/openstack/tests/__init__.py (+21/-2) juju/providers/openstack/tests/test_bootstrap.py (+11/-9) juju/providers/openstack/tests/test_files.py (+8/-11) juju/providers/openstack/tests/test_launch.py (+48/-41) |
||||
To merge this branch: | bzr merge lp:~hazmat/pyjuju/ostack-constraints | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju Engineering | Pending | ||
Review via email: mp+116027@code.launchpad.net |
Commit message
Description of the change
Openstack provider constraints support.
Allows openstack providers to utilize standard cli constraints
when bootstrapping or deploying services. Dynamically introspects
flavors/
Deprecates default-
Tested against hpcloud.
Kapil Thangavelu (hazmat) wrote : | # |
Benjamin Saller (bcsaller) wrote : | # |
LGTM +1
Just a couple of nitpicks. Otherwise this looks good and brings some of
the conventions more in line with the coding std.
I glanced at the Nova docs, it didn't look like they supported 'arch' on
a quick read, I did find that surprising.
https:/
File juju/providers/
https:/
juju/providers/
constraints[
.get("instance-
the None, but maybe not a KeyError.
https:/
File juju/providers/
https:/
juju/providers/
deprecated, use cli --constraints")
With no users of the provider yet (or are there) you could remove the
option from the config and skip even the warning.
https:/
juju/providers/
I see that the underlying impl won't search 'arch' but using a fake
value here that looks real might be an issue down the line. Maybe
'ignored-arch' or 'unused' is better?
https:/
File juju/providers/
https:/
juju/providers/
provider.
Isn't the return value ignored here, unless I'm missing something the
'return' is undesirable on any of these tests.
Kapil Thangavelu (hazmat) wrote : | # |
Thanks for the review.
On Fri, Jul 20, 2012 at 1:05 PM, <email address hidden> wrote:
> LGTM +1
>
> Just a couple of nitpicks. Otherwise this looks good and brings some of
> the conventions more in line with the coding std.
>
> I glanced at the Nova docs, it didn't look like they supported 'arch' on
> a quick read, I did find that surprising.
>
>
It is surprising, trystack now has arm support, i'll have to check and see
how it describes that, my understanding is that they do implicitly by
segmenting an entire region for it. we'll need to have some external zk
support though to make that viable in practice do to arm java gymnastics.
>
> https:/
> providers/
> File juju/providers/
>
> https:/
> providers/
> juju/providers/
> constraints[
> .get("instance-
> the None, but maybe not a KeyError.
>
for the solver to even be used means the provider is explicitly using it
which means it registered instance type constraints, which means
instance-type will always be available as a key when its constraint set
parse any values. ie. keyerror can't happen here, and if does i think its
better for it to blow up as something is very wrong.
>
> https:/
> providers/
> File juju/providers/
>
> https:/
> providers/
> juju/providers/
> deprecated, use cli --constraints")
> With no users of the provider yet (or are there) you could remove the
> option from the config and skip even the warning.
>
doh.. i should remove the option from the config, even then it feels good
to warn about this. the config is kinda of worthless, since its validation
of known values only, unknown values silently pass through.
> https:/
> providers/
> juju/providers/
> I see that the underlying impl won't search 'arch' but using a fake
> value here that looks real might be an issue down the line. Maybe
> 'ignored-arch' or 'unused' is better?
>
hmm. the type solver should be searching arch, oversight. i'll fix that. i
had ripped it out my azure provider.
>
> https:/
> providers/
Martin Packman (gz) wrote : | # |
Generic whine about mixing imports rearrangement into a code change that
needs review, could easily have been a separate branch to make life
easier.
https:/
File juju/providers/
https:/
juju/providers/
constraints):
Not that it really matters given the small number of flavors generally,
but all this faffing around with filters seems complicated given these
constraints have order, and sort/bisect would work just as well.
https:/
juju/providers/
self._instance_
Instead just do `possible_
would give a deterministic result for instances with same cpu but
different mem as well?
https:/
File juju/providers/
https:/
juju/providers/
Rename to list_flavor_detail so it's the same form as the others and the
url.
https:/
File juju/providers/
https:/
juju/providers/
self._provider.
Rename list_flavors_
https:/
juju/providers/
flavor_name, flavors):
Private helper isn't related to launch and doesn't use self. I'd make it
a function rather than a method if we're not going down the route of an
interface on the provider.
https:/
File juju/providers/
https:/
juju/providers/
instance types (just names)
I'd prefer if this created a provider instance shared store of flavor
information, that launch then accessed to resolve its constraints. Would
mean one list_flavor_detail call rather than one or two per launch, and
would also tie flavor business into a single class.
https:/
File juju/providers/
https:/
juju/providers/
self.nova.
Rename list_flavors_
Kapil Thangavelu (hazmat) wrote : | # |
On Tue, Jul 24, 2012 at 10:40 AM, Martin Packman <
<email address hidden>> wrote:
> Generic whine about mixing imports rearrangement into a code change that
> needs review, could easily have been a separate branch to make life
> easier.
>
>
noted, sorry, i didn't think i'd be able to come back to this and make
those changes.
>
>
> https:/
> File juju/providers/
>
>
> https:/
> juju/providers/
> constraints):
> Not that it really matters given the small number of flavors generally,
> but all this faffing around with filters seems complicated given these
> constraints have order, and sort/bisect would work just as well.
>
there is no guarantee of ordering, they come in a dictionary, derived
possibly from external systems. This is in providers/commons and can't
assume implementation detail of a given provider.
>
> https:/
> juju/providers/
> self._instance_
> Instead just do `possible_
> would give a deterministic result for instances with same cpu but
> different mem as well?
>
possible_types is a list of names, a sort on type names isn't provably
correct without knowledge of types. I've added mem to the key to give the
desired result.
>
>
> https:/
> File juju/providers/
>
>
> https:/
> juju/providers/
> Rename to list_flavor_detail so it's the same form as the others and the
> url.
>
i've done this, and i'm okay with it, but the naming to me feels like it
would be listing the details of a single flavor instead of denoting that
its listing the details of all flavors.
>
>
> https:/
> File juju/providers/
>
>
> https:/
> juju/providers/
> self._provider.
> Rename list_flavors_
>
done
>
>
> https:/
> juju/providers/
> flavor_name, flavors):
> Private helper isn't related to launch and doesn't use self. I'd make it
> a function rather than a method if we're not going down the route of an
> interface on the provider.
>
>
done
>
> https:/
> File juju/providers/
>
>
> https:/
> ...
Kapil Thangavelu (hazmat) wrote : | # |
*** Submitted:
Openstack provider constraints support.
Allows openstack providers to utilize standard cli constraints
when bootstrapping or deploying services. Dynamically introspects
flavors/
Deprecates default-
Tested against hpcloud.
R=bcsaller, gz
CC=
https:/
Preview Diff
1 | === added file 'juju/providers/common/instance_type.py' |
2 | --- juju/providers/common/instance_type.py 1970-01-01 00:00:00 +0000 |
3 | +++ juju/providers/common/instance_type.py 2012-07-20 16:18:25 +0000 |
4 | @@ -0,0 +1,52 @@ |
5 | +import operator |
6 | + |
7 | +from juju.errors import ProviderError |
8 | +from collections import namedtuple |
9 | + |
10 | +# Just a generic base instance type, with required attrs. |
11 | +InstanceType = namedtuple("InstanceType", "arch cpu mem") |
12 | + |
13 | + |
14 | +class TypeSolver(object): |
15 | + |
16 | + def __init__(self, instance_types): |
17 | + self._instance_types = instance_types |
18 | + |
19 | + def run(self, constraints): |
20 | + instance_type = constraints["instance-type"] |
21 | + if instance_type is not None: |
22 | + if not instance_type in self._instance_types: |
23 | + raise ProviderError( |
24 | + "Invalid instance type %s" % instance_type) |
25 | + return instance_type |
26 | + |
27 | + return self._solve(constraints) |
28 | + |
29 | + def _solve(self, constraints): |
30 | + possible_types = list(self._instance_types) |
31 | + for f in self._get_filters(constraints): |
32 | + possible_types = filter(f, possible_types) |
33 | + |
34 | + if not possible_types: |
35 | + raise ProviderError( |
36 | + "No instance type satisifes %s" % dict(constraints)) |
37 | + return sorted( |
38 | + possible_types, |
39 | + key=lambda x: self._instance_types[x].cpu)[0] |
40 | + |
41 | + def _get_filters(self, constraints): |
42 | + return ( |
43 | + self._filter_func("cpu", constraints, operator.ge), |
44 | + self._filter_func("mem", constraints, operator.ge) |
45 | + ) |
46 | + |
47 | + def _filter_func(self, name, constraints, op): |
48 | + |
49 | + desired = constraints[name] |
50 | + if not desired: |
51 | + return lambda _: True |
52 | + |
53 | + def f(type_): |
54 | + value = getattr(self._instance_types[type_], name) |
55 | + return op(value, desired) |
56 | + return f |
57 | |
58 | === added file 'juju/providers/common/tests/test_instance_type.py' |
59 | --- juju/providers/common/tests/test_instance_type.py 1970-01-01 00:00:00 +0000 |
60 | +++ juju/providers/common/tests/test_instance_type.py 2012-07-20 16:18:25 +0000 |
61 | @@ -0,0 +1,53 @@ |
62 | +from juju.lib.testing import TestCase |
63 | + |
64 | +from juju.providers.common.instance_type import ( |
65 | + TypeSolver, InstanceType) |
66 | +from juju.machine.constraints import ConstraintSet |
67 | + |
68 | + |
69 | +from twisted.internet.defer import inlineCallbacks |
70 | + |
71 | + |
72 | +class InstanceTypeSolverTest(TestCase): |
73 | + |
74 | + @inlineCallbacks |
75 | + def setUp(self): |
76 | + self.instance_types = { |
77 | + "ExtraSmall": InstanceType("amd64", 0.1, 768), |
78 | + "Small": InstanceType("amd64", 1, 1792), |
79 | + "Medium": InstanceType("amd64", 2, 3584), |
80 | + "Large": InstanceType("amd64", 4, 7168), |
81 | + "ExtraLarge": InstanceType("amd64", 8, 14336), |
82 | + } |
83 | + self.solver = TypeSolver(self.instance_types) |
84 | + self.cs = ConstraintSet("cloud") |
85 | + self.cs.register_generics(self.instance_types.keys()) |
86 | + yield super(InstanceTypeSolverTest, self).setUp() |
87 | + |
88 | + def test_type_solver(self): |
89 | + |
90 | + self.assertEqual( |
91 | + self.solver.run(self.cs.parse(["cpu=4"])), |
92 | + "Large") |
93 | + |
94 | + self.assertEqual( |
95 | + self.solver.run(self.cs.parse(["cpu=3.5"])), |
96 | + "Large") |
97 | + |
98 | + self.assertEqual( |
99 | + self.solver.run(self.cs.parse(["mem=2G"])), |
100 | + "Medium") |
101 | + |
102 | + self.assertEqual( |
103 | + self.solver.run(self.cs.parse(["cpu=2.5", "mem=2G"])), |
104 | + "Large") |
105 | + |
106 | + # Any auto solves for min 1 cpu, 512mb ram. |
107 | + self.assertEqual( |
108 | + self.solver.run(self.cs.parse(["instance-type=any"])), |
109 | + "Small") |
110 | + |
111 | + self.assertEqual( |
112 | + self.solver.run(self.cs.parse( |
113 | + ["instance-type=ExtraLarge"])), |
114 | + "ExtraLarge") |
115 | |
116 | === modified file 'juju/providers/openstack/__init__.py' |
117 | --- juju/providers/openstack/__init__.py 2012-03-02 18:11:23 +0000 |
118 | +++ juju/providers/openstack/__init__.py 2012-07-20 16:18:25 +0000 |
119 | @@ -1,3 +1,5 @@ |
120 | """Support for using OpenStack as a cloud provider for juju""" |
121 | |
122 | +__all__ = ['MachineProvider'] |
123 | + |
124 | from .provider import MachineProvider |
125 | |
126 | === modified file 'juju/providers/openstack/client.py' |
127 | --- juju/providers/openstack/client.py 2012-07-18 15:19:22 +0000 |
128 | +++ juju/providers/openstack/client.py 2012-07-20 16:18:25 +0000 |
129 | @@ -24,12 +24,12 @@ |
130 | import urllib |
131 | |
132 | import twisted |
133 | -from twisted.internet import ( |
134 | - defer, |
135 | - interfaces, |
136 | - protocol, |
137 | - reactor, |
138 | - ) |
139 | +from twisted.internet.defer import ( |
140 | + Deferred, inlineCallbacks, returnValue, succeed) |
141 | +from twisted.internet.protocol import Protocol |
142 | +from twisted.internet.interfaces import IProducer |
143 | +from twisted.internet import reactor |
144 | + |
145 | from twisted.web import ( |
146 | client, |
147 | http_headers, |
148 | @@ -49,7 +49,7 @@ |
149 | class BytestringProducer(object): |
150 | """Wrap basic bytestring as a needlessly fancy twisted producer.""" |
151 | |
152 | - implements(interfaces.IProducer) |
153 | + implements(IProducer) |
154 | |
155 | def __init__(self, bytestring): |
156 | self.content = bytestring |
157 | @@ -61,13 +61,13 @@ |
158 | def startProducing(self, consumer): |
159 | """Write entire contents when production starts""" |
160 | consumer.write(self.content) |
161 | - return defer.succeed(None) |
162 | + return succeed(None) |
163 | |
164 | def stopProducing(self): |
165 | """Nothing to do when production halts""" |
166 | |
167 | |
168 | -class ResponseReader(protocol.Protocol): |
169 | +class ResponseReader(Protocol): |
170 | """Protocol object suitable for use with Response.deliverBody |
171 | |
172 | The 'onConnectionLost' deferred will be called back once the connection |
173 | @@ -75,7 +75,7 @@ |
174 | """ |
175 | |
176 | def __init__(self): |
177 | - self.onConnectionLost = defer.Deferred() |
178 | + self.onConnectionLost = Deferred() |
179 | |
180 | def connectionMade(self): |
181 | self.data = [] |
182 | @@ -92,7 +92,7 @@ |
183 | self.onConnectionLost.callback("".join(self.data)) |
184 | |
185 | |
186 | -@defer.inlineCallbacks |
187 | +@inlineCallbacks |
188 | def request(method, url, extra_headers=(), body=None): |
189 | headers = http_headers.Headers({ |
190 | # GZ 2012-07-03: Previously passed Accept: application/json header |
191 | @@ -111,11 +111,11 @@ |
192 | body = BytestringProducer(body) |
193 | response = yield client.Agent(reactor).request(method, url, headers, body) |
194 | if response.length == 0: |
195 | - defer.returnValue((response, "")) |
196 | + returnValue((response, "")) |
197 | reader = ResponseReader() |
198 | response.deliverBody(reader) |
199 | body = yield reader.onConnectionLost |
200 | - defer.returnValue((response, body)) |
201 | + returnValue((response, body)) |
202 | |
203 | |
204 | class _OpenStackClient(object): |
205 | @@ -152,7 +152,7 @@ |
206 | log.debug('access %s @ %s', service, url) |
207 | return url |
208 | |
209 | - @defer.inlineCallbacks |
210 | + @inlineCallbacks |
211 | def authenticate_v1(self): |
212 | deferred = request( |
213 | "GET", |
214 | @@ -244,7 +244,7 @@ |
215 | def is_authenticated(self): |
216 | return self.token is not None |
217 | |
218 | - @defer.inlineCallbacks |
219 | + @inlineCallbacks |
220 | def authed_request(self, method, url, headers=None, body=None): |
221 | log.debug("openstack: %s %r", method, url) |
222 | request_headers = [("X-Auth-Token", self.token)] |
223 | @@ -252,7 +252,7 @@ |
224 | request_headers += headers |
225 | response, body = yield request(method, url, request_headers, body) |
226 | log.debug("openstack: %d %r", response.code, body) |
227 | - defer.returnValue((response, body)) |
228 | + returnValue((response, body)) |
229 | |
230 | def _empty(self, result, code): |
231 | response, body = result |
232 | @@ -287,13 +287,13 @@ |
233 | def __init__(self, client): |
234 | self._client = client |
235 | |
236 | - @defer.inlineCallbacks |
237 | + @inlineCallbacks |
238 | def request(self, method, parts, headers=None, body=None): |
239 | if not self._client.is_authenticated(): |
240 | yield self._client.authenticate() |
241 | url = self._client._make_url("compute", parts) |
242 | result = yield self._client.authed_request(method, url, headers, body) |
243 | - defer.returnValue(result) |
244 | + returnValue(result) |
245 | |
246 | def delete(self, parts, code=202): |
247 | deferred = self.request("DELETE", parts) |
248 | @@ -318,6 +318,9 @@ |
249 | def list_flavors(self): |
250 | return self.get("flavors", "flavors") |
251 | |
252 | + def list_flavors_details(self): |
253 | + return self.get(["flavors", "detail"], "flavors") |
254 | + |
255 | def get_server(self, server_id): |
256 | return self.get(["servers", server_id], "server") |
257 | |
258 | @@ -434,13 +437,13 @@ |
259 | def __init__(self, client): |
260 | self._client = client |
261 | |
262 | - @defer.inlineCallbacks |
263 | + @inlineCallbacks |
264 | def request(self, method, parts, headers=None, body=None): |
265 | if not self._client.is_authenticated(): |
266 | yield self._client.authenticate() |
267 | url = self._client._make_url("object-store", parts) |
268 | result = yield self._client.authed_request(method, url, headers, body) |
269 | - defer.returnValue(result) |
270 | + returnValue(result) |
271 | |
272 | def public_object_url(self, container, object_name): |
273 | if not self._client.is_authenticated(): |
274 | |
275 | === modified file 'juju/providers/openstack/files.py' |
276 | --- juju/providers/openstack/files.py 2012-07-05 16:09:11 +0000 |
277 | +++ juju/providers/openstack/files.py 2012-07-20 16:18:25 +0000 |
278 | @@ -25,7 +25,7 @@ |
279 | |
280 | from cStringIO import StringIO |
281 | |
282 | -from twisted.internet import defer |
283 | +from twisted.internet.defer import inlineCallbacks, returnValue |
284 | |
285 | from juju import errors |
286 | |
287 | @@ -40,7 +40,7 @@ |
288 | def get_url(self, name): |
289 | return self._swift.public_object_url(self._container, name) |
290 | |
291 | - @defer.inlineCallbacks |
292 | + @inlineCallbacks |
293 | def get(self, name): |
294 | """Get a file object from Swift. |
295 | |
296 | @@ -57,9 +57,9 @@ |
297 | if response.code != 200: |
298 | raise errors.ProviderInteractionError( |
299 | "Couldn't fetch object %r %r" % (response.code, body)) |
300 | - defer.returnValue(StringIO(body)) |
301 | + returnValue(StringIO(body)) |
302 | |
303 | - @defer.inlineCallbacks |
304 | + @inlineCallbacks |
305 | def put(self, remote_path, file_object): |
306 | """Upload a file to Swift. |
307 | |
308 | |
309 | === modified file 'juju/providers/openstack/launch.py' |
310 | --- juju/providers/openstack/launch.py 2012-07-18 15:19:22 +0000 |
311 | +++ juju/providers/openstack/launch.py 2012-07-20 16:18:25 +0000 |
312 | @@ -22,14 +22,13 @@ |
313 | |
314 | from cStringIO import StringIO |
315 | |
316 | -from twisted.internet import ( |
317 | - defer, |
318 | - reactor, |
319 | - ) |
320 | +from twisted.internet.defer import inlineCallbacks, returnValue |
321 | + |
322 | |
323 | from juju.errors import ProviderError, ProviderInteractionError |
324 | from juju.lib import twistutils |
325 | from juju.providers.common.launch import LaunchMachine |
326 | +from juju.providers.common.instance_type import TypeSolver, InstanceType |
327 | |
328 | from .machine import machine_from_instance, get_server_status |
329 | |
330 | @@ -41,7 +40,7 @@ |
331 | |
332 | _DELAY_FOR_ADDRESSES = 5 # seconds |
333 | |
334 | - @defer.inlineCallbacks |
335 | + @inlineCallbacks |
336 | def start_machine(self, machine_id, zookeepers): |
337 | """Actually launch an instance on Nova. |
338 | |
339 | @@ -80,17 +79,16 @@ |
340 | # type and resolve it to a flavor id here. |
341 | flavor_name = self._provider.config.get("default-instance-type") |
342 | if flavor_name is None: |
343 | - raise ProviderError("Need to specify a default-instance-type") |
344 | - flavors = yield self._provider.nova.list_flavors() |
345 | - flavor_map = dict((f['name'], f['id']) for f in flavors) |
346 | - if not flavor_name in flavor_map: |
347 | - ProviderError("Unknown instance type given: %r" % (flavor_name,)) |
348 | + log.warning( |
349 | + "default-instance-type is deprecated, use cli --constraints") |
350 | + flavors = yield self._provider.nova.list_flavors_details() |
351 | + flavor_id = self._solve_flavor(flavor_name, flavors) |
352 | |
353 | server = yield self._provider.nova.run_server( |
354 | name="juju %s instance %s" % |
355 | (self._provider.environment_name, machine_id,), |
356 | image_id=image_id, |
357 | - flavor_id=flavor_map[flavor_name], |
358 | + flavor_id=flavor_id, |
359 | security_group_names=security_groups, |
360 | user_data=user_data) |
361 | |
362 | @@ -117,10 +115,23 @@ |
363 | server = yield self._provider.nova.get_server(server['id']) |
364 | yield _assign_floating_ip(self._provider, server['id']) |
365 | |
366 | - defer.returnValue([machine_from_instance(server)]) |
367 | - |
368 | - |
369 | -@defer.inlineCallbacks |
370 | + returnValue([machine_from_instance(server)]) |
371 | + |
372 | + def _solve_flavor(self, flavor_name, flavors): |
373 | + flavor_map, flavor_types = {}, {} |
374 | + for f in flavors: |
375 | + flavor_map[f['name']] = f['id'] |
376 | + flavor_types[f['name']] = InstanceType( |
377 | + '686', f['vcpus'], f['ram']) |
378 | + |
379 | + solver = TypeSolver(flavor_types) |
380 | + flavor_name = solver.run(self._constraints) |
381 | + if not flavor_name in flavor_map: |
382 | + ProviderError("Unknown instance type given: %r" % (flavor_name,)) |
383 | + return flavor_map[flavor_name] |
384 | + |
385 | + |
386 | +@inlineCallbacks |
387 | def _assign_floating_ip(provider, server_id): |
388 | floating_ips = yield provider.nova.list_floating_ips() |
389 | for floating_ip in floating_ips: |
390 | |
391 | === modified file 'juju/providers/openstack/ports.py' |
392 | --- juju/providers/openstack/ports.py 2012-07-18 02:46:56 +0000 |
393 | +++ juju/providers/openstack/ports.py 2012-07-20 16:18:25 +0000 |
394 | @@ -20,9 +20,7 @@ |
395 | looking at the details of the one with the matching name. |
396 | """ |
397 | |
398 | -from twisted.internet import ( |
399 | - defer, |
400 | - ) |
401 | +from twisted.internet.defer import inlineCallbacks, returnValue |
402 | |
403 | from juju import errors |
404 | |
405 | @@ -47,7 +45,7 @@ |
406 | def _machine_group_name(self, machine_id): |
407 | return "juju-%s-%s" % (self.tag, machine_id) |
408 | |
409 | - @defer.inlineCallbacks |
410 | + @inlineCallbacks |
411 | def _get_machine_group(self, machine, machine_id): |
412 | """Get details of the machine specific security group |
413 | |
414 | @@ -59,12 +57,12 @@ |
415 | groups = yield self.nova.get_server_security_groups(server_id) |
416 | for group in groups: |
417 | if group['name'] == group_name: |
418 | - defer.returnValue(group) |
419 | + returnValue(group) |
420 | raise errors.ProviderInteractionError( |
421 | "Missing security group %r for machine %r" % |
422 | (group_name, server_id)) |
423 | |
424 | - @defer.inlineCallbacks |
425 | + @inlineCallbacks |
426 | def open_port(self, machine, machine_id, port, protocol="tcp"): |
427 | """Allow access to a port for the given machine only""" |
428 | group = yield self._get_machine_group(machine, machine_id) |
429 | @@ -73,7 +71,7 @@ |
430 | log.debug("Opened %s/%s on machine %r", |
431 | port, protocol, machine.instance_id) |
432 | |
433 | - @defer.inlineCallbacks |
434 | + @inlineCallbacks |
435 | def close_port(self, machine, machine_id, port, protocol="tcp"): |
436 | """Revoke access to a port for the given machine only""" |
437 | group = yield self._get_machine_group(machine, machine_id) |
438 | @@ -88,7 +86,7 @@ |
439 | "Couldn't close unopened %s/%s on machine %r", |
440 | port, protocol, machine.instance_id) |
441 | |
442 | - @defer.inlineCallbacks |
443 | + @inlineCallbacks |
444 | def get_opened_ports(self, machine, machine_id): |
445 | """Get a set of opened port/protocol pairs for a machine""" |
446 | group = yield self._get_machine_group(machine, machine_id) |
447 | @@ -100,9 +98,9 @@ |
448 | to_port = rule["to_port"] |
449 | if from_port == to_port: |
450 | opened_ports.add((from_port, protocol)) |
451 | - defer.returnValue(opened_ports) |
452 | + returnValue(opened_ports) |
453 | |
454 | - @defer.inlineCallbacks |
455 | + @inlineCallbacks |
456 | def ensure_groups(self, machine_id): |
457 | """Get names of the security groups for a machine, creating if needed |
458 | |
459 | @@ -133,9 +131,9 @@ |
460 | yield self.nova.create_security_group(machine_group, |
461 | "juju group for %s machine %s" % (self.tag, machine_id)) |
462 | |
463 | - defer.returnValue([juju_group, machine_group]) |
464 | + returnValue([juju_group, machine_group]) |
465 | |
466 | - @defer.inlineCallbacks |
467 | + @inlineCallbacks |
468 | def get_machine_groups(self, machine, with_juju_group=False): |
469 | try: |
470 | ret = yield self.get_machine_groups_pure(machine, with_juju_group) |
471 | @@ -147,11 +145,11 @@ |
472 | except errors.ProviderInteractionError, e: |
473 | pass # just rebinding e |
474 | if True or getattr(e, "kind", None) == "itemNotFound": |
475 | - defer.returnValue(None) |
476 | + returnValue(None) |
477 | raise |
478 | - defer.returnValue(ret) |
479 | + returnValue(ret) |
480 | |
481 | - @defer.inlineCallbacks |
482 | + @inlineCallbacks |
483 | def get_machine_groups_pure(self, machine, with_juju_group=False): |
484 | server_id = machine.instance_id |
485 | groups = yield self.nova.get_server_security_groups(server_id) |
486 | @@ -160,13 +158,13 @@ |
487 | if g['name'].startswith(juju_group)) |
488 | if juju_group not in groups_by_name: |
489 | # Not a juju machine, shouldn't touch |
490 | - defer.returnValue(None) |
491 | + returnValue(None) |
492 | if not with_juju_group: |
493 | groups_by_name.pop(juju_group) |
494 | # else assumption: only one remaining group, is the machine group |
495 | - defer.returnValue(groups_by_name) |
496 | + returnValue(groups_by_name) |
497 | |
498 | - @defer.inlineCallbacks |
499 | + @inlineCallbacks |
500 | def delete_juju_group(self): |
501 | security_groups = yield self.nova.list_security_groups() |
502 | juju_group = self._juju_group_name() |
503 | |
504 | === modified file 'juju/providers/openstack/provider.py' |
505 | --- juju/providers/openstack/provider.py 2012-07-18 15:19:22 +0000 |
506 | +++ juju/providers/openstack/provider.py 2012-07-20 16:18:25 +0000 |
507 | @@ -12,7 +12,7 @@ |
508 | |
509 | import logging |
510 | |
511 | -from twisted.internet import defer |
512 | +from twisted.internet.defer import inlineCallbacks, returnValue, gatherResults |
513 | |
514 | from juju import errors |
515 | from juju.providers.common.base import MachineProviderBase |
516 | @@ -61,6 +61,15 @@ |
517 | """Retrieve a Swift-backed :class:`FileStorage`.""" |
518 | return FileStorage(self.swift, self.config["control-bucket"]) |
519 | |
520 | + @inlineCallbacks |
521 | + def get_constraint_set(self): |
522 | + cs = yield super(MachineProvider, self).get_constraint_set() |
523 | + # Fetch provider defined instance types (just names) |
524 | + flavors = yield self.nova.list_flavors() |
525 | + flavor_names = [f['name'] for f in flavors] |
526 | + cs.register_generics(flavor_names) |
527 | + returnValue(cs) |
528 | + |
529 | def start_machine(self, machine_data, master=False): |
530 | """Start an OpenStack machine. |
531 | |
532 | @@ -74,7 +83,7 @@ |
533 | """ |
534 | return NovaLaunchMachine.launch(self, machine_data, master) |
535 | |
536 | - @defer.inlineCallbacks |
537 | + @inlineCallbacks |
538 | def get_machines(self, instance_ids=()): |
539 | """List machines running in the provider. |
540 | |
541 | @@ -122,9 +131,9 @@ |
542 | if missing: |
543 | raise errors.MachinesNotFound(missing) |
544 | |
545 | - defer.returnValue(machines) |
546 | + returnValue(machines) |
547 | |
548 | - @defer.inlineCallbacks |
549 | + @inlineCallbacks |
550 | def _delete_machine(self, machine, full=False): |
551 | server_id = machine.instance_id |
552 | server = yield self.nova.get_server(server_id) |
553 | @@ -132,7 +141,7 @@ |
554 | "juju %s instance" % self.environment_name): |
555 | raise errors.MachinesNotFound(set([machine.instance_id])) |
556 | yield self.nova.delete_server(server_id) |
557 | - defer.returnValue(machine) |
558 | + returnValue(machine) |
559 | |
560 | def shutdown_machine(self, machine): |
561 | if not isinstance(machine, NovaProviderMachine): |
562 | @@ -142,7 +151,7 @@ |
563 | # and can be shutdown, instead just handle an error? 404-ish? |
564 | return self._delete_machine(machine) |
565 | |
566 | - @defer.inlineCallbacks |
567 | + @inlineCallbacks |
568 | def destroy_environment(self): |
569 | """Terminate all associated machines and security groups. |
570 | |
571 | @@ -153,10 +162,10 @@ |
572 | :rtype: :class:`twisted.internet.defer.Deferred` |
573 | """ |
574 | machines = yield self.get_machines() |
575 | - deleted_machines = yield defer.gatherResults( |
576 | + deleted_machines = yield gatherResults( |
577 | [self._delete_machine(m, True) for m in machines]) |
578 | yield self.save_state({}) |
579 | - defer.returnValue(deleted_machines) |
580 | + returnValue(deleted_machines) |
581 | |
582 | def shutdown_machines(self, machines): |
583 | """Terminate machines associated with this provider. |
584 | @@ -172,7 +181,7 @@ |
585 | """ |
586 | # XXX: need to actually handle errors as non-terminated machines |
587 | # and not include them in the resulting list |
588 | - return defer.gatherResults( |
589 | + return gatherResults( |
590 | [self.shutdown_machine(m) for m in machines], consumeErrors=True) |
591 | |
592 | def open_port(self, machine, machine_id, port, protocol="tcp"): |
593 | |
594 | === modified file 'juju/providers/openstack/tests/__init__.py' |
595 | --- juju/providers/openstack/tests/__init__.py 2012-07-18 21:17:01 +0000 |
596 | +++ juju/providers/openstack/tests/__init__.py 2012-07-20 16:18:25 +0000 |
597 | @@ -23,6 +23,8 @@ |
598 | provider, |
599 | ) |
600 | |
601 | +from juju.machine.constraints import ConstraintSet |
602 | + |
603 | |
604 | class FakeResponse(object): |
605 | |
606 | @@ -34,7 +36,22 @@ |
607 | {"Content-Type": ["application/json"]}) |
608 | |
609 | |
610 | -class OpenStackTestMixin(object): |
611 | +class ConstraintSupportMixin(object): |
612 | + |
613 | + provider_type = "openstack" |
614 | + |
615 | + default_flavors = [ |
616 | + {'id': 1, 'name': "standard.xsmall", 'vcpus': 1, 'ram': 1024}, |
617 | + {'id': 2, 'name': "standard.medium", 'vcpus': 2, 'ram': 4096}, |
618 | + ] |
619 | + |
620 | + def setup_constraints(self): |
621 | + self.constraint_set = ConstraintSet(self.provider_type) |
622 | + self.constraint_set.register_generics( |
623 | + [f['name'] for f in self.default_flavors]) |
624 | + |
625 | + |
626 | +class OpenStackTestMixin(ConstraintSupportMixin): |
627 | """Helpers for test classes that want to exercise the OpenStack APIs |
628 | |
629 | This goes all the way down to the http layer, which is really a little too |
630 | @@ -67,6 +84,7 @@ |
631 | } |
632 | ] |
633 | }})))) |
634 | + self.setup_constraints() |
635 | # Clear the environment so provider won't pick up config from envvars |
636 | self.change_environment() |
637 | |
638 | @@ -154,7 +172,7 @@ |
639 | self.mocker.result(defer.succeed((FakeResponse(code), response))) |
640 | |
641 | |
642 | -class MockedProvider(object): |
643 | +class MockedProvider(ConstraintSupportMixin): |
644 | |
645 | provider_type = "openstack" |
646 | environment_name = "testing" |
647 | @@ -183,6 +201,7 @@ |
648 | ports.NovaPortManager(self.nova, self.environment_name)) |
649 | self.provider_actions = mocker.mock() |
650 | self.mocker = mocker |
651 | + self.setup_constraints() |
652 | |
653 | def get_file_storage(self): |
654 | return files.FileStorage(self.swift, self.config['control-bucket']) |
655 | |
656 | === modified file 'juju/providers/openstack/tests/test_bootstrap.py' |
657 | --- juju/providers/openstack/tests/test_bootstrap.py 2012-07-18 15:03:39 +0000 |
658 | +++ juju/providers/openstack/tests/test_bootstrap.py 2012-07-20 16:18:25 +0000 |
659 | @@ -12,10 +12,8 @@ |
660 | mocker, |
661 | testing, |
662 | ) |
663 | -from juju.machine import constraints |
664 | |
665 | from juju.providers.openstack.machine import NovaProviderMachine |
666 | - |
667 | from juju.providers.openstack.tests import OpenStackTestMixin |
668 | |
669 | |
670 | @@ -74,8 +72,8 @@ |
671 | def _match_server(self, data): |
672 | userdata = data['server'].pop('user_data').decode("base64") |
673 | self.assertEqual("#cloud-config", userdata.split("\n", 1)[0]) |
674 | - cloud_init = yaml.load(userdata) |
675 | # TODO: assertions on cloud-init content |
676 | + # cloud_init = yaml.load(userdata) |
677 | self.assertEqual({'server': { |
678 | 'flavorRef': 1, |
679 | 'imageRef': 42, |
680 | @@ -89,8 +87,9 @@ |
681 | return True |
682 | |
683 | def expect_launch(self): |
684 | - self.expect_nova_get("flavors", |
685 | - response={'flavors': [{'id': 1, 'name': "standard.xsmall"}]}) |
686 | + self.expect_nova_get("flavors/detail", |
687 | + response={'flavors': self.default_flavors}) |
688 | + |
689 | self.expect_nova_post("servers", mocker.MATCH(self._match_server), |
690 | code=202, response={'server': { |
691 | 'id': '1000', |
692 | @@ -114,8 +113,8 @@ |
693 | |
694 | def bootstrap(self): |
695 | provider = self.get_provider() |
696 | - constraint_set = constraints.ConstraintSet(provider.provider_type) |
697 | - return provider.bootstrap(constraint_set.load({})) |
698 | + return provider.bootstrap( |
699 | + self.constraint_set.load({'instance-type': None})) |
700 | |
701 | def test_bootstrap_clean(self): |
702 | """Bootstrap from a clean slate makes groups and zookeeper instance""" |
703 | @@ -130,6 +129,7 @@ |
704 | log = self.capture_logging("juju.common", level=logging.DEBUG) |
705 | deferred = self.bootstrap() |
706 | deferred.addCallback(self._check_machine) |
707 | + |
708 | def check_log(_): |
709 | log_text = log.getvalue() |
710 | self.assertIn("Launching juju bootstrap instance", log_text) |
711 | @@ -175,9 +175,10 @@ |
712 | self.mocker.replay() |
713 | |
714 | log = self.capture_logging("juju.common") |
715 | - self.capture_logging("juju.openstack") # Drop to avoid stderr kipple |
716 | + self.capture_logging("juju.openstack") # Drop to avoid stderr kipple |
717 | deferred = self.bootstrap() |
718 | deferred.addCallback(self._check_machine) |
719 | + |
720 | def check_log(_): |
721 | self.assertEqual("juju environment previously bootstrapped.\n", |
722 | log.getvalue()) |
723 | @@ -203,9 +204,10 @@ |
724 | self.mocker.replay() |
725 | |
726 | log = self.capture_logging("juju.common", level=logging.DEBUG) |
727 | - self.capture_logging("juju.openstack") # Drop to avoid stderr kipple |
728 | + self.capture_logging("juju.openstack") # Drop to avoid stderr kipple |
729 | deferred = self.bootstrap() |
730 | deferred.addCallback(self._check_machine) |
731 | + |
732 | def check_log(_): |
733 | log_text = log.getvalue() |
734 | self.assertIn("Launching juju bootstrap instance", log_text) |
735 | |
736 | === modified file 'juju/providers/openstack/tests/test_files.py' |
737 | --- juju/providers/openstack/tests/test_files.py 2012-06-18 15:03:50 +0000 |
738 | +++ juju/providers/openstack/tests/test_files.py 2012-07-20 16:18:25 +0000 |
739 | @@ -2,14 +2,11 @@ |
740 | |
741 | from cStringIO import StringIO |
742 | |
743 | -from twisted.internet import ( |
744 | - defer, |
745 | - ) |
746 | +from twisted.internet.defer import inlineCallbacks, fail |
747 | |
748 | from juju import errors |
749 | -from juju.lib import ( |
750 | - testing, |
751 | - ) |
752 | +from juju.lib import testing |
753 | + |
754 | from juju.providers.openstack.tests import OpenStackTestMixin |
755 | |
756 | |
757 | @@ -47,12 +44,12 @@ |
758 | """Unexpected errors from client propogate""" |
759 | content = "some text" |
760 | self._mock_swift("PUT", "testing/object", content) |
761 | - self.mocker.result(defer.fail(ValueError("Something unexpected"))) |
762 | + self.mocker.result(fail(ValueError("Something unexpected"))) |
763 | self.mocker.replay() |
764 | deferred = self.get_storage().put("object", StringIO(content)) |
765 | return self.assertFailure(deferred, ValueError) |
766 | |
767 | - @defer.inlineCallbacks |
768 | + @inlineCallbacks |
769 | def test_get_url(self): |
770 | """A url can be generated for any stored file.""" |
771 | self.mocker.replay() |
772 | @@ -61,7 +58,7 @@ |
773 | url = storage.get_url("object") |
774 | self.assertEqual(self.swift_url + "/testing/object", url) |
775 | |
776 | - @defer.inlineCallbacks |
777 | + @inlineCallbacks |
778 | def test_get_url_unicode(self): |
779 | """A url can be generated for *any* stored file.""" |
780 | self.mocker.replay() |
781 | @@ -70,7 +67,7 @@ |
782 | url = storage.get_url(u"\xa7") |
783 | self.assertEqual(self.swift_url + "/testing/%C2%A7", url) |
784 | |
785 | - @defer.inlineCallbacks |
786 | + @inlineCallbacks |
787 | def test_get_file(self): |
788 | """Retrieving a file returns a file-like object with the content""" |
789 | content = "some text" |
790 | @@ -79,7 +76,7 @@ |
791 | result = yield self.get_storage().get("object") |
792 | self.assertEqual(result.read(), content) |
793 | |
794 | - @defer.inlineCallbacks |
795 | + @inlineCallbacks |
796 | def test_get_file_unicode(self): |
797 | """Retrieving a file with a unicode key uses a UTF-8 url""" |
798 | content = "some text" |
799 | |
800 | === modified file 'juju/providers/openstack/tests/test_launch.py' |
801 | --- juju/providers/openstack/tests/test_launch.py 2012-07-18 21:17:01 +0000 |
802 | +++ juju/providers/openstack/tests/test_launch.py 2012-07-20 16:18:25 +0000 |
803 | @@ -1,62 +1,55 @@ |
804 | """Tests for launching a new server customised for juju using Nova""" |
805 | |
806 | -from twisted.internet import ( |
807 | - defer, |
808 | - ) |
809 | +from twisted.internet.defer import succeed |
810 | |
811 | from juju import errors |
812 | -from juju.lib import ( |
813 | - mocker, |
814 | - testing, |
815 | - ) |
816 | -from juju.machine import ( |
817 | - constraints, |
818 | - ) |
819 | - |
820 | -from juju.providers.openstack import ( |
821 | - launch, |
822 | - tests, |
823 | - ) |
824 | - |
825 | - |
826 | -class MockedLaunchProvider(tests.MockedProvider): |
827 | - |
828 | - def launch(self, machine_id, master=False): |
829 | - constraint_set = constraints.ConstraintSet(self.provider_type) |
830 | +from juju.lib.mocker import ANY |
831 | +from juju.lib.testing import TestCase |
832 | + |
833 | + |
834 | +from juju.providers.openstack.tests import MockedProvider |
835 | +from juju.providers.openstack.launch import NovaLaunchMachine |
836 | + |
837 | + |
838 | +class MockedLaunchProvider(MockedProvider): |
839 | + |
840 | + def launch(self, machine_id, master=False, constraints=None): |
841 | + if constraints is None: |
842 | + constraints = self.constraint_set.load({'instance-type': None}) |
843 | + else: |
844 | + constraints = self.constraint_set.parse(constraints) |
845 | details = { |
846 | 'machine-id': machine_id, |
847 | - 'constraints': constraint_set.load({}), |
848 | + 'constraints': constraints |
849 | } |
850 | - return launch.NovaLaunchMachine.launch(self, details, master) |
851 | + return NovaLaunchMachine.launch(self, details, master) |
852 | |
853 | def expect_launch_setup(self, machine_id): |
854 | self.port_manager.ensure_groups(machine_id) |
855 | - self.mocker.result(defer.succeed(["juju-x", "juju-y"])) |
856 | - self.nova.list_flavors() |
857 | - self.mocker.result(defer.succeed([ |
858 | - {'id': 1, 'name': "standard.xsmall"}, |
859 | - ])) |
860 | + self.mocker.result(succeed(["juju-x", "juju-y"])) |
861 | + self.nova.list_flavors_details() |
862 | + self.mocker.result(succeed(self.default_flavors)) |
863 | |
864 | - def expect_run_server(self, machine_id, response): |
865 | + def expect_run_server(self, machine_id, response, flavor_id=1): |
866 | self.nova.run_server( |
867 | name="juju testing instance " + machine_id, |
868 | image_id=42, |
869 | - flavor_id=1, |
870 | + flavor_id=flavor_id, |
871 | security_group_names=["juju-x", "juju-y"], |
872 | - user_data=mocker.ANY, |
873 | + user_data=ANY, |
874 | ) |
875 | - self.mocker.result(defer.succeed(response)) |
876 | + self.mocker.result(succeed(response)) |
877 | |
878 | def expect_available_floating_ip(self, server_id): |
879 | self.nova.list_floating_ips() |
880 | - self.mocker.result(defer.succeed([ |
881 | + self.mocker.result(succeed([ |
882 | {'instance_id': None, 'ip': "198.162.1.0"}, |
883 | ])) |
884 | self.nova.add_floating_ip(server_id, "198.162.1.0") |
885 | - self.mocker.result(defer.succeed(None)) |
886 | - |
887 | - |
888 | -class NovaLaunchMachineTests(testing.TestCase): |
889 | + self.mocker.result(succeed(None)) |
890 | + |
891 | + |
892 | +class NovaLaunchMachineTests(TestCase): |
893 | |
894 | def test_launch_requires_default_image_id(self): |
895 | config = dict(MockedLaunchProvider.default_config) |
896 | @@ -67,6 +60,20 @@ |
897 | deferred = provider.launch("1") |
898 | return self.assertFailure(deferred, errors.ProviderError) |
899 | |
900 | + def test_start_machine_with_constraints(self): |
901 | + provider = MockedLaunchProvider(self.mocker) |
902 | + provider.expect_zookeeper_machines(1000) |
903 | + provider.expect_launch_setup("1") |
904 | + provider.expect_run_server( |
905 | + "1", |
906 | + response={ |
907 | + 'id': 1001, |
908 | + 'addresses': {'public': []}, |
909 | + }, |
910 | + flavor_id=2) |
911 | + self.mocker.replay() |
912 | + return provider.launch("1", constraints=["cpu=2", "mem=3G"]) |
913 | + |
914 | def test_start_machine(self): |
915 | provider = MockedLaunchProvider(self.mocker) |
916 | provider.expect_zookeeper_machines(1000) |
917 | @@ -87,14 +94,14 @@ |
918 | 'id': 1001, |
919 | }) |
920 | provider.nova.get_server(1001) |
921 | - self.mocker.result(defer.succeed({ |
922 | + self.mocker.result(succeed({ |
923 | 'id': 1001, |
924 | 'addresses': {'public': []}, |
925 | })) |
926 | provider.expect_available_floating_ip(1001) |
927 | - self.mocker.result(defer.succeed(None)) |
928 | + self.mocker.result(succeed(None)) |
929 | self.mocker.replay() |
930 | - self.patch(launch.NovaLaunchMachine, "_DELAY_FOR_ADDRESSES", 0) |
931 | + self.patch(NovaLaunchMachine, "_DELAY_FOR_ADDRESSES", 0) |
932 | return provider.launch("1") |
933 | |
934 | def test_start_machine_master(self): |
935 | @@ -107,6 +114,6 @@ |
936 | }) |
937 | provider.expect_swift_put("juju_master_id", "1000") |
938 | provider.provider_actions.save_state({'zookeeper-instances': [1000]}) |
939 | - self.mocker.result(defer.succeed(None)) |
940 | + self.mocker.result(succeed(None)) |
941 | self.mocker.replay() |
942 | return provider.launch("0", master=True) |
Reviewers: mp+116027_ code.launchpad. net,
Message:
Please take a look.
Description:
Openstack provider constraints support.
Allows openstack providers to utilize standard cli constraints instance- types from service provider.
when bootstrapping or deploying services. Dynamically introspects
flavors/
Deprecates default- instance- type (logged and ignored).
Tested against hpcloud.
https:/ /code.launchpad .net/~hazmat/ juju/ostack- constraints/ +merge/ 116027
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/6421053/
Affected files: common/ instance_ type.py common/ tests/test_ instance_ type.py openstack/ __init_ _.py openstack/ client. py openstack/ files.py openstack/ launch. py openstack/ ports.py openstack/ provider. py openstack/ tests/_ _init__ .py openstack/ tests/test_ bootstrap. py openstack/ tests/test_ files.py openstack/ tests/test_ launch. py
A [revision details]
A juju/providers/
A juju/providers/
M juju/providers/
M juju/providers/
M juju/providers/
M juju/providers/
M juju/providers/
M juju/providers/
M juju/providers/
M juju/providers/
M juju/providers/
M juju/providers/