Merge lp:~mattyw/juju-core/add-service-owner-to-status into lp:~go-bot/juju-core/trunk
- add-service-owner-to-status
- Merge into trunk
Status: | Work in progress |
---|---|
Proposed branch: | lp:~mattyw/juju-core/add-service-owner-to-status |
Merge into: | lp:~go-bot/juju-core/trunk |
Diff against target: |
471 lines (+106/-65) 5 files modified
cmd/juju/status.go (+2/-0) cmd/juju/status_test.go (+99/-65) state/api/client.go (+1/-0) state/apiserver/client/api_test.go (+3/-0) state/statecmd/status.go (+1/-0) |
To merge this branch: | bzr merge lp:~mattyw/juju-core/add-service-owner-to-status |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju Engineering | Pending | ||
Review via email: mp+207859@code.launchpad.net |
Commit message
Description of the change
Added service owner to the output of juju status
Only really useful to land when the juju id branches land.
The output of the api call to Status() (including the output of the juju
status command) now includes the owner of the service
The output looks like this:
services:
ubuntu:
charm: cs:precise/ubuntu-4
exposed: false
units:
ubuntu/0:
agent-state: pending
machine: "1"
owner-tag: user-admin
Dimiter Naydenov (dimitern) wrote : | # |
Looks good, but make sure you're familiar with Martin's recent 1.16
compatibility for status changes:
https:/
You'll need to use the FullStatus API call.
- 2350. By Matthew Williams
-
merged with trunk
- 2351. By Matthew Williams
-
merged with trunk
Matthew Williams (mattyw) wrote : | # |
Please take a look.
Dimiter Naydenov (dimitern) wrote : | # |
On 2014/02/25 10:15:52, dimitern wrote:
> Looks good, but make sure you're familiar with Martin's recent 1.16
> compatibility for status changes:
https:/
> You'll need to use the FullStatus API call.
Sorry, so your changes are in statecmd, which gets called internally in
apiserver for both FullStatus and Status.
This means you don't need to care about FullStatus in your CL.
Your changes LGTM assuming you've tested them live as well, just in
case.
William Reade (fwereade) wrote : | # |
On 2014/02/27 10:40:25, dimitern wrote:
> On 2014/02/25 10:15:52, dimitern wrote:
> > Looks good, but make sure you're familiar with Martin's recent 1.16
> > compatibility for status changes:
https:/
> >
> > You'll need to use the FullStatus API call.
> Sorry, so your changes are in statecmd, which gets called internally
in
> apiserver for both FullStatus and Status.
> This means you don't need to care about FullStatus in your CL.
> Your changes LGTM assuming you've tested them live as well, just in
case.
I'm fairly strongly -1 on this, because status is bloated enough
already. Can we please start adding flags to status for additional
information?
William Reade (fwereade) wrote : | # |
On 2014/02/27 13:45:51, fwereade wrote:
> On 2014/02/27 10:40:25, dimitern wrote:
> > On 2014/02/25 10:15:52, dimitern wrote:
> > > Looks good, but make sure you're familiar with Martin's recent
1.16
> > > compatibility for status changes:
https:/
> > >
> > > You'll need to use the FullStatus API call.
> >
> > Sorry, so your changes are in statecmd, which gets called internally
in
> > apiserver for both FullStatus and Status.
> > This means you don't need to care about FullStatus in your CL.
> > Your changes LGTM assuming you've tested them live as well, just in
case.
> I'm fairly strongly -1 on this, because status is bloated enough
already. Can we
> please start adding flags to status for additional information?
And especially in status, adding the owner to every unit feels wrong. If
units ever get explicit owners we can add it then; for now, the service
feels like a much better place. So not lgtm without discussion, at
least.
Matthew Williams (mattyw) wrote : | # |
On 2014/02/27 13:47:33, fwereade wrote:
> On 2014/02/27 13:45:51, fwereade wrote:
> > On 2014/02/27 10:40:25, dimitern wrote:
> > > On 2014/02/25 10:15:52, dimitern wrote:
> > > > Looks good, but make sure you're familiar with Martin's recent
1.16
> > > > compatibility for status changes:
https:/
> > > >
> > > > You'll need to use the FullStatus API call.
> > >
> > > Sorry, so your changes are in statecmd, which gets called
internally in
> > > apiserver for both FullStatus and Status.
> > > This means you don't need to care about FullStatus in your CL.
> > > Your changes LGTM assuming you've tested them live as well, just
in case.
> >
> > I'm fairly strongly -1 on this, because status is bloated enough
already. Can
> we
> > please start adding flags to status for additional information?
> And especially in status, adding the owner to every unit feels wrong.
If units
> ever get explicit owners we can add it then; for now, the service
feels like a
> much better place. So not lgtm without discussion, at least.
Fair enough - I was hoping to have discussion with you and roger about
this - the implementation here includes the owner as part of the service
though - not part of the unit
Unmerged revisions
- 2351. By Matthew Williams
-
merged with trunk
- 2350. By Matthew Williams
-
merged with trunk
- 2349. By Matthew Williams
-
added owner tag to status, fixed tests, fixed a wrong import for loggo
Preview Diff
1 | === modified file 'cmd/juju/status.go' |
2 | --- cmd/juju/status.go 2014-01-16 05:28:03 +0000 |
3 | +++ cmd/juju/status.go 2014-02-27 04:55:00 +0000 |
4 | @@ -160,6 +160,7 @@ |
5 | Relations map[string][]string `json:"relations,omitempty" yaml:"relations,omitempty"` |
6 | SubordinateTo []string `json:"subordinate-to,omitempty" yaml:"subordinate-to,omitempty"` |
7 | Units map[string]unitStatus `json:"units,omitempty" yaml:"units,omitempty"` |
8 | + OwnerTag string `json:"owner-tag,omitempty" yaml:"owner-tag,omitempty"` |
9 | } |
10 | |
11 | type serviceStatusNoMarshal serviceStatus |
12 | @@ -259,6 +260,7 @@ |
13 | CanUpgradeTo: service.CanUpgradeTo, |
14 | SubordinateTo: service.SubordinateTo, |
15 | Units: make(map[string]unitStatus), |
16 | + OwnerTag: service.OwnerTag, |
17 | } |
18 | for k, m := range service.Units { |
19 | out.Units[k] = formatUnit(m) |
20 | |
21 | === modified file 'cmd/juju/status_test.go' |
22 | --- cmd/juju/status_test.go 2014-01-24 14:52:58 +0000 |
23 | +++ cmd/juju/status_test.go 2014-02-27 04:55:00 +0000 |
24 | @@ -192,12 +192,14 @@ |
25 | "hardware": "arch=amd64 cpu-cores=1 mem=1024M root-disk=8192M", |
26 | } |
27 | unexposedService = M{ |
28 | - "charm": "cs:quantal/dummy-1", |
29 | - "exposed": false, |
30 | + "charm": "cs:quantal/dummy-1", |
31 | + "exposed": false, |
32 | + "owner-tag": "user-admin", |
33 | } |
34 | exposedService = M{ |
35 | - "charm": "cs:quantal/dummy-1", |
36 | - "exposed": true, |
37 | + "charm": "cs:quantal/dummy-1", |
38 | + "exposed": true, |
39 | + "owner-tag": "user-admin", |
40 | } |
41 | ) |
42 | |
43 | @@ -461,8 +463,9 @@ |
44 | }, |
45 | "services": M{ |
46 | "exposed-service": M{ |
47 | - "charm": "cs:quantal/dummy-1", |
48 | - "exposed": true, |
49 | + "charm": "cs:quantal/dummy-1", |
50 | + "exposed": true, |
51 | + "owner-tag": "user-admin", |
52 | "units": M{ |
53 | "exposed-service/0": M{ |
54 | "machine": "2", |
55 | @@ -476,8 +479,9 @@ |
56 | }, |
57 | }, |
58 | "dummy-service": M{ |
59 | - "charm": "cs:quantal/dummy-1", |
60 | - "exposed": false, |
61 | + "charm": "cs:quantal/dummy-1", |
62 | + "exposed": false, |
63 | + "owner-tag": "user-admin", |
64 | "units": M{ |
65 | "dummy-service/0": M{ |
66 | "machine": "1", |
67 | @@ -535,8 +539,9 @@ |
68 | }, |
69 | "services": M{ |
70 | "exposed-service": M{ |
71 | - "charm": "cs:quantal/dummy-1", |
72 | - "exposed": true, |
73 | + "charm": "cs:quantal/dummy-1", |
74 | + "exposed": true, |
75 | + "owner-tag": "user-admin", |
76 | "units": M{ |
77 | "exposed-service/0": M{ |
78 | "machine": "2", |
79 | @@ -550,8 +555,9 @@ |
80 | }, |
81 | }, |
82 | "dummy-service": M{ |
83 | - "charm": "cs:quantal/dummy-1", |
84 | - "exposed": false, |
85 | + "charm": "cs:quantal/dummy-1", |
86 | + "exposed": false, |
87 | + "owner-tag": "user-admin", |
88 | "units": M{ |
89 | "dummy-service/0": M{ |
90 | "machine": "1", |
91 | @@ -576,8 +582,9 @@ |
92 | }, |
93 | "services": M{ |
94 | "dummy-service": M{ |
95 | - "charm": "cs:quantal/dummy-1", |
96 | - "exposed": false, |
97 | + "charm": "cs:quantal/dummy-1", |
98 | + "exposed": false, |
99 | + "owner-tag": "user-admin", |
100 | "units": M{ |
101 | "dummy-service/0": M{ |
102 | "machine": "1", |
103 | @@ -601,8 +608,9 @@ |
104 | }, |
105 | "services": M{ |
106 | "exposed-service": M{ |
107 | - "charm": "cs:quantal/dummy-1", |
108 | - "exposed": true, |
109 | + "charm": "cs:quantal/dummy-1", |
110 | + "exposed": true, |
111 | + "owner-tag": "user-admin", |
112 | "units": M{ |
113 | "exposed-service/0": M{ |
114 | "machine": "2", |
115 | @@ -628,8 +636,9 @@ |
116 | }, |
117 | "services": M{ |
118 | "dummy-service": M{ |
119 | - "charm": "cs:quantal/dummy-1", |
120 | - "exposed": false, |
121 | + "charm": "cs:quantal/dummy-1", |
122 | + "exposed": false, |
123 | + "owner-tag": "user-admin", |
124 | "units": M{ |
125 | "dummy-service/0": M{ |
126 | "machine": "1", |
127 | @@ -653,8 +662,9 @@ |
128 | }, |
129 | "services": M{ |
130 | "exposed-service": M{ |
131 | - "charm": "cs:quantal/dummy-1", |
132 | - "exposed": true, |
133 | + "charm": "cs:quantal/dummy-1", |
134 | + "exposed": true, |
135 | + "owner-tag": "user-admin", |
136 | "units": M{ |
137 | "exposed-service/0": M{ |
138 | "machine": "2", |
139 | @@ -681,8 +691,9 @@ |
140 | }, |
141 | "services": M{ |
142 | "dummy-service": M{ |
143 | - "charm": "cs:quantal/dummy-1", |
144 | - "exposed": false, |
145 | + "charm": "cs:quantal/dummy-1", |
146 | + "exposed": false, |
147 | + "owner-tag": "user-admin", |
148 | "units": M{ |
149 | "dummy-service/0": M{ |
150 | "machine": "1", |
151 | @@ -694,8 +705,9 @@ |
152 | }, |
153 | }, |
154 | "exposed-service": M{ |
155 | - "charm": "cs:quantal/dummy-1", |
156 | - "exposed": true, |
157 | + "charm": "cs:quantal/dummy-1", |
158 | + "exposed": true, |
159 | + "owner-tag": "user-admin", |
160 | "units": M{ |
161 | "exposed-service/0": M{ |
162 | "machine": "2", |
163 | @@ -731,9 +743,10 @@ |
164 | }, |
165 | "services": M{ |
166 | "dummy-service": M{ |
167 | - "charm": "cs:quantal/dummy-1", |
168 | - "exposed": false, |
169 | - "life": "dying", |
170 | + "charm": "cs:quantal/dummy-1", |
171 | + "exposed": false, |
172 | + "owner-tag": "user-admin", |
173 | + "life": "dying", |
174 | "units": M{ |
175 | "dummy-service/0": M{ |
176 | "machine": "0", |
177 | @@ -808,8 +821,9 @@ |
178 | }, |
179 | "services": M{ |
180 | "project": M{ |
181 | - "charm": "cs:quantal/wordpress-3", |
182 | - "exposed": true, |
183 | + "charm": "cs:quantal/wordpress-3", |
184 | + "exposed": true, |
185 | + "owner-tag": "user-admin", |
186 | "units": M{ |
187 | "project/0": M{ |
188 | "machine": "1", |
189 | @@ -823,8 +837,9 @@ |
190 | }, |
191 | }, |
192 | "mysql": M{ |
193 | - "charm": "cs:quantal/mysql-1", |
194 | - "exposed": true, |
195 | + "charm": "cs:quantal/mysql-1", |
196 | + "exposed": true, |
197 | + "owner-tag": "user-admin", |
198 | "units": M{ |
199 | "mysql/0": M{ |
200 | "machine": "2", |
201 | @@ -837,8 +852,9 @@ |
202 | }, |
203 | }, |
204 | "varnish": M{ |
205 | - "charm": "cs:quantal/varnish-1", |
206 | - "exposed": true, |
207 | + "charm": "cs:quantal/varnish-1", |
208 | + "exposed": true, |
209 | + "owner-tag": "user-admin", |
210 | "units": M{ |
211 | "varnish/0": M{ |
212 | "machine": "3", |
213 | @@ -851,8 +867,9 @@ |
214 | }, |
215 | }, |
216 | "private": M{ |
217 | - "charm": "cs:quantal/wordpress-3", |
218 | - "exposed": true, |
219 | + "charm": "cs:quantal/wordpress-3", |
220 | + "exposed": true, |
221 | + "owner-tag": "user-admin", |
222 | "units": M{ |
223 | "private/0": M{ |
224 | "machine": "4", |
225 | @@ -909,8 +926,9 @@ |
226 | }, |
227 | "services": M{ |
228 | "riak": M{ |
229 | - "charm": "cs:quantal/riak-7", |
230 | - "exposed": true, |
231 | + "charm": "cs:quantal/riak-7", |
232 | + "exposed": true, |
233 | + "owner-tag": "user-admin", |
234 | "units": M{ |
235 | "riak/0": M{ |
236 | "machine": "1", |
237 | @@ -991,8 +1009,9 @@ |
238 | }, |
239 | "services": M{ |
240 | "wordpress": M{ |
241 | - "charm": "cs:quantal/wordpress-3", |
242 | - "exposed": true, |
243 | + "charm": "cs:quantal/wordpress-3", |
244 | + "exposed": true, |
245 | + "owner-tag": "user-admin", |
246 | "units": M{ |
247 | "wordpress/0": M{ |
248 | "machine": "1", |
249 | @@ -1011,8 +1030,9 @@ |
250 | }, |
251 | }, |
252 | "mysql": M{ |
253 | - "charm": "cs:quantal/mysql-1", |
254 | - "exposed": true, |
255 | + "charm": "cs:quantal/mysql-1", |
256 | + "exposed": true, |
257 | + "owner-tag": "user-admin", |
258 | "units": M{ |
259 | "mysql/0": M{ |
260 | "machine": "2", |
261 | @@ -1032,8 +1052,9 @@ |
262 | }, |
263 | }, |
264 | "logging": M{ |
265 | - "charm": "cs:quantal/logging-1", |
266 | - "exposed": true, |
267 | + "charm": "cs:quantal/logging-1", |
268 | + "exposed": true, |
269 | + "owner-tag": "user-admin", |
270 | "relations": M{ |
271 | "logging-directory": L{"wordpress"}, |
272 | "info": L{"mysql"}, |
273 | @@ -1056,8 +1077,9 @@ |
274 | }, |
275 | "services": M{ |
276 | "wordpress": M{ |
277 | - "charm": "cs:quantal/wordpress-3", |
278 | - "exposed": true, |
279 | + "charm": "cs:quantal/wordpress-3", |
280 | + "exposed": true, |
281 | + "owner-tag": "user-admin", |
282 | "units": M{ |
283 | "wordpress/0": M{ |
284 | "machine": "1", |
285 | @@ -1076,8 +1098,9 @@ |
286 | }, |
287 | }, |
288 | "mysql": M{ |
289 | - "charm": "cs:quantal/mysql-1", |
290 | - "exposed": true, |
291 | + "charm": "cs:quantal/mysql-1", |
292 | + "exposed": true, |
293 | + "owner-tag": "user-admin", |
294 | "units": M{ |
295 | "mysql/0": M{ |
296 | "machine": "2", |
297 | @@ -1097,8 +1120,9 @@ |
298 | }, |
299 | }, |
300 | "logging": M{ |
301 | - "charm": "cs:quantal/logging-1", |
302 | - "exposed": true, |
303 | + "charm": "cs:quantal/logging-1", |
304 | + "exposed": true, |
305 | + "owner-tag": "user-admin", |
306 | "relations": M{ |
307 | "logging-directory": L{"wordpress"}, |
308 | "info": L{"mysql"}, |
309 | @@ -1120,8 +1144,9 @@ |
310 | }, |
311 | "services": M{ |
312 | "wordpress": M{ |
313 | - "charm": "cs:quantal/wordpress-3", |
314 | - "exposed": true, |
315 | + "charm": "cs:quantal/wordpress-3", |
316 | + "exposed": true, |
317 | + "owner-tag": "user-admin", |
318 | "units": M{ |
319 | "wordpress/0": M{ |
320 | "machine": "1", |
321 | @@ -1140,8 +1165,9 @@ |
322 | }, |
323 | }, |
324 | "logging": M{ |
325 | - "charm": "cs:quantal/logging-1", |
326 | - "exposed": true, |
327 | + "charm": "cs:quantal/logging-1", |
328 | + "exposed": true, |
329 | + "owner-tag": "user-admin", |
330 | "relations": M{ |
331 | "logging-directory": L{"wordpress"}, |
332 | "info": L{"mysql"}, |
333 | @@ -1197,8 +1223,9 @@ |
334 | }, |
335 | "services": M{ |
336 | "wordpress": M{ |
337 | - "charm": "cs:quantal/wordpress-3", |
338 | - "exposed": true, |
339 | + "charm": "cs:quantal/wordpress-3", |
340 | + "exposed": true, |
341 | + "owner-tag": "user-admin", |
342 | "units": M{ |
343 | "wordpress/0": M{ |
344 | "machine": "1", |
345 | @@ -1217,8 +1244,9 @@ |
346 | }, |
347 | }, |
348 | "monitoring": M{ |
349 | - "charm": "cs:quantal/monitoring-0", |
350 | - "exposed": true, |
351 | + "charm": "cs:quantal/monitoring-0", |
352 | + "exposed": true, |
353 | + "owner-tag": "user-admin", |
354 | "relations": M{ |
355 | "monitoring-port": L{"wordpress"}, |
356 | }, |
357 | @@ -1269,8 +1297,9 @@ |
358 | }, |
359 | "services": M{ |
360 | "mysql": M{ |
361 | - "charm": "cs:quantal/mysql-1", |
362 | - "exposed": true, |
363 | + "charm": "cs:quantal/mysql-1", |
364 | + "exposed": true, |
365 | + "owner-tag": "user-admin", |
366 | "units": M{ |
367 | "mysql/0": M{ |
368 | "machine": "1", |
369 | @@ -1299,8 +1328,9 @@ |
370 | }, |
371 | "services": M{ |
372 | "mysql": M{ |
373 | - "charm": "cs:quantal/mysql-1", |
374 | - "exposed": true, |
375 | + "charm": "cs:quantal/mysql-1", |
376 | + "exposed": true, |
377 | + "owner-tag": "user-admin", |
378 | "units": M{ |
379 | "mysql/1": M{ |
380 | "machine": "1/lxc/0", |
381 | @@ -1341,6 +1371,7 @@ |
382 | "charm": "cs:quantal/mysql-1", |
383 | "can-upgrade-to": "cs:quantal/mysql-23", |
384 | "exposed": true, |
385 | + "owner-tag": "user-admin", |
386 | "units": M{ |
387 | "mysql/0": M{ |
388 | "machine": "1", |
389 | @@ -1380,8 +1411,9 @@ |
390 | }, |
391 | "services": M{ |
392 | "mysql": M{ |
393 | - "charm": "local:quantal/mysql-1", |
394 | - "exposed": true, |
395 | + "charm": "local:quantal/mysql-1", |
396 | + "exposed": true, |
397 | + "owner-tag": "user-admin", |
398 | "units": M{ |
399 | "mysql/0": M{ |
400 | "machine": "1", |
401 | @@ -1426,6 +1458,7 @@ |
402 | "charm": "cs:quantal/mysql-2", |
403 | "can-upgrade-to": "cs:quantal/mysql-23", |
404 | "exposed": true, |
405 | + "owner-tag": "user-admin", |
406 | "units": M{ |
407 | "mysql/0": M{ |
408 | "machine": "1", |
409 | @@ -1467,8 +1500,9 @@ |
410 | }, |
411 | "services": M{ |
412 | "mysql": M{ |
413 | - "charm": "local:quantal/mysql-1", |
414 | - "exposed": true, |
415 | + "charm": "local:quantal/mysql-1", |
416 | + "exposed": true, |
417 | + "owner-tag": "user-admin", |
418 | "units": M{ |
419 | "mysql/0": M{ |
420 | "machine": "1", |
421 | |
422 | === modified file 'state/api/client.go' |
423 | --- state/api/client.go 2014-02-24 17:41:18 +0000 |
424 | +++ state/api/client.go 2014-02-27 04:55:00 +0000 |
425 | @@ -50,6 +50,7 @@ |
426 | CanUpgradeTo string |
427 | SubordinateTo []string |
428 | Units map[string]UnitStatus |
429 | + OwnerTag string |
430 | } |
431 | |
432 | // UnitStatus holds status info about a unit. |
433 | |
434 | === modified file 'state/apiserver/client/api_test.go' |
435 | --- state/apiserver/client/api_test.go 2014-02-19 06:31:52 +0000 |
436 | +++ state/apiserver/client/api_test.go 2014-02-27 04:55:00 +0000 |
437 | @@ -179,12 +179,14 @@ |
438 | "logging-directory": []string{"wordpress"}, |
439 | }, |
440 | SubordinateTo: []string{"wordpress"}, |
441 | + OwnerTag: "user-admin", |
442 | }, |
443 | "mysql": api.ServiceStatus{ |
444 | Charm: "local:quantal/mysql-1", |
445 | Relations: map[string][]string{}, |
446 | SubordinateTo: []string{}, |
447 | Units: map[string]api.UnitStatus{}, |
448 | + OwnerTag: "user-admin", |
449 | }, |
450 | "wordpress": api.ServiceStatus{ |
451 | Charm: "local:quantal/wordpress-3", |
452 | @@ -212,6 +214,7 @@ |
453 | }, |
454 | }, |
455 | }, |
456 | + OwnerTag: "user-admin", |
457 | }, |
458 | }, |
459 | } |
460 | |
461 | === modified file 'state/statecmd/status.go' |
462 | --- state/statecmd/status.go 2014-01-23 20:30:35 +0000 |
463 | +++ state/statecmd/status.go 2014-02-27 04:55:00 +0000 |
464 | @@ -335,6 +335,7 @@ |
465 | status.Charm = serviceCharmURL.String() |
466 | status.Exposed = service.IsExposed() |
467 | status.Life = processLife(service) |
468 | + status.OwnerTag = service.GetOwnerTag() |
469 | |
470 | latestCharm, ok := context.latestCharms[*serviceCharmURL.WithRevision(-1)] |
471 | if ok && latestCharm != serviceCharmURL.String() { |
Reviewers: mp+207859_ code.launchpad. net,
Message:
Please take a look.
Description:
Added service owner to the output of juju status
Only really useful to land when the juju id branches land.
The output of the api call to Status() (including the output of the juju
status command) now includes the owner of the service
The output looks like this:
services:
ubuntu:
charm: cs:precise/ubuntu-4
exposed: false
units:
ubuntu/0:
agent-state: pending
machine: "1"
owner-tag: user-admin
https:/ /code.launchpad .net/~mattyw/ juju-core/ add-service- owner-to- status/ +merge/ 207859
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/67870043/
Affected files (+109, -66 lines): status_ test.go /client/ api_test. go status. go peergrouper/ desired. go
A [revision details]
M cmd/juju/status.go
M cmd/juju/
M state/api/client.go
M state/apiserver
M state/statecmd/
M worker/