Merge lp:~thumper/juju-core/juju-run-cli into lp:~go-bot/juju-core/trunk
- juju-run-cli
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Tim Penhey |
Approved revision: | no longer in the source branch. |
Merged at revision: | 2193 |
Proposed branch: | lp:~thumper/juju-core/juju-run-cli |
Merge into: | lp:~go-bot/juju-core/trunk |
Prerequisite: | lp:~thumper/juju-core/api-juju-run |
Diff against target: |
736 lines (+690/-0) 5 files modified
cmd/juju/main.go (+1/-0) cmd/juju/main_test.go (+1/-0) cmd/juju/run.go (+233/-0) cmd/juju/run_test.go (+443/-0) testing/environ.go (+12/-0) |
To merge this branch: | bzr merge lp:~thumper/juju-core/juju-run-cli |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju Engineering | Pending | ||
Review via email: mp+201124@code.launchpad.net |
Commit message
Add the "run" command to the command line tool.
Hook up the Client Run and RunOnAllMachines API calls
to the command line tool.
Description of the change
Add the "run" command to the command line tool.
Hook up the Client Run and RunOnAllMachines API calls
to the command line tool.
Tim Penhey (thumper) wrote : | # |
Andrew Wilkins (axwalk) wrote : | # |
https:/
File cmd/juju/run.go (right):
https:/
cmd/juju/run.go:33: Run the commands on the specified targets.
This just occurred to me: should we be supporting patterns, like in juju
status?
e.g. juju run --service='nova-*' <commands>
Just a thought.
https:/
cmd/juju/
Why are you initialising this with 0 length, and then appending? Why not
initialise with len(runResults) and assign elements?
https:/
cmd/juju/
You're assuming that stdout/stderr are UTF-8 here, which they may not
be. At the least, can we please use utf8.Valid before converting them to
strings?
https:/
cmd/juju/
Nice
https:/
File cmd/juju/
https:/
cmd/juju/
In light of my comments in run.go, I think a test for binary
stdout/stderr is in order.
Tim Penhey (thumper) wrote : | # |
Please take a look.
https:/
File cmd/juju/run.go (right):
https:/
cmd/juju/run.go:33: Run the commands on the specified targets.
On 2014/01/10 02:13:53, axw wrote:
> This just occurred to me: should we be supporting patterns, like in
juju status?
> e.g. juju run --service='nova-*' <commands>
> Just a thought.
I do think that this is a good idea, but not for the first release :-)
https:/
cmd/juju/
On 2014/01/10 02:13:53, axw wrote:
> Why are you initialising this with 0 length, and then appending? Why
not
> initialise with len(runResults) and assign elements?
Hmm good point. Since we know the size, I'll do that.
https:/
cmd/juju/
On 2014/01/10 02:13:53, axw wrote:
> You're assuming that stdout/stderr are UTF-8 here, which they may not
be. At the
> least, can we please use utf8.Valid before converting them to strings?
Now base64 encoding non-utf8 byte slice results.
https:/
File cmd/juju/
https:/
cmd/juju/
On 2014/01/10 02:13:53, axw wrote:
> In light of my comments in run.go, I think a test for binary
stdout/stderr is in
> order.
Added.
Andrew Wilkins (axwalk) wrote : | # |
On 2014/01/10 03:40:49, thumper wrote:
> Please take a look.
> https:/
> File cmd/juju/run.go (right):
https:/
> cmd/juju/run.go:33: Run the commands on the specified targets.
> On 2014/01/10 02:13:53, axw wrote:
> > This just occurred to me: should we be supporting patterns, like in
juju
> status?
> > e.g. juju run --service='nova-*' <commands>
> >
> > Just a thought.
> I do think that this is a good idea, but not for the first release :-)
https:/
> cmd/juju/
> On 2014/01/10 02:13:53, axw wrote:
> > Why are you initialising this with 0 length, and then appending? Why
not
> > initialise with len(runResults) and assign elements?
> Hmm good point. Since we know the size, I'll do that.
https:/
> cmd/juju/
> On 2014/01/10 02:13:53, axw wrote:
> > You're assuming that stdout/stderr are UTF-8 here, which they may
not be. At
> the
> > least, can we please use utf8.Valid before converting them to
strings?
> Now base64 encoding non-utf8 byte slice results.
> https:/
> File cmd/juju/
https:/
> cmd/juju/
> On 2014/01/10 02:13:53, axw wrote:
> > In light of my comments in run.go, I think a test for binary
stdout/stderr is
> in
> > order.
> Added.
Thanks, LGTM
Preview Diff
1 | === modified file 'cmd/juju/main.go' |
2 | --- cmd/juju/main.go 2013-12-11 07:55:37 +0000 |
3 | +++ cmd/juju/main.go 2014-01-12 20:39:24 +0000 |
4 | @@ -86,6 +86,7 @@ |
5 | jujucmd.Register(wrap(&EndpointCommand{})) |
6 | |
7 | // Error resolution and debugging commands. |
8 | + jujucmd.Register(wrap(&RunCommand{})) |
9 | jujucmd.Register(wrap(&SCPCommand{})) |
10 | jujucmd.Register(wrap(&SSHCommand{})) |
11 | jujucmd.Register(wrap(&ResolvedCommand{})) |
12 | |
13 | === modified file 'cmd/juju/main_test.go' |
14 | --- cmd/juju/main_test.go 2013-12-11 07:55:37 +0000 |
15 | +++ cmd/juju/main_test.go 2014-01-12 20:39:24 +0000 |
16 | @@ -244,6 +244,7 @@ |
17 | "remove-relation", // alias for destroy-relation |
18 | "remove-unit", // alias for destroy-unit |
19 | "resolved", |
20 | + "run", |
21 | "scp", |
22 | "set", |
23 | "set-constraints", |
24 | |
25 | === added file 'cmd/juju/run.go' |
26 | --- cmd/juju/run.go 1970-01-01 00:00:00 +0000 |
27 | +++ cmd/juju/run.go 2014-01-12 20:39:24 +0000 |
28 | @@ -0,0 +1,233 @@ |
29 | +// Copyright 2013 Canonical Ltd. |
30 | +// Licensed under the AGPLv3, see LICENCE file for details. |
31 | + |
32 | +package main |
33 | + |
34 | +import ( |
35 | + "encoding/base64" |
36 | + "errors" |
37 | + "fmt" |
38 | + "strings" |
39 | + "time" |
40 | + "unicode/utf8" |
41 | + |
42 | + "launchpad.net/gnuflag" |
43 | + |
44 | + "launchpad.net/juju-core/cmd" |
45 | + "launchpad.net/juju-core/juju" |
46 | + "launchpad.net/juju-core/names" |
47 | + "launchpad.net/juju-core/state/api/params" |
48 | +) |
49 | + |
50 | +// RunCommand is responsible for running arbitrary commands on remote machines. |
51 | +type RunCommand struct { |
52 | + cmd.EnvCommandBase |
53 | + out cmd.Output |
54 | + all bool |
55 | + timeout time.Duration |
56 | + machines []string |
57 | + services []string |
58 | + units []string |
59 | + commands string |
60 | +} |
61 | + |
62 | +const runDoc = ` |
63 | +Run the commands on the specified targets. |
64 | + |
65 | +Targets are specified using either machine ids, service names or unit |
66 | +names. At least one target specifier is needed. |
67 | + |
68 | +Multiple values can be set for --machine, --service, and --unit by using |
69 | +comma separated values. |
70 | + |
71 | +If the target is a machine, the command is run as the "ubuntu" user on |
72 | +the remote machine. |
73 | + |
74 | +If the target is a service, the command is run on all units for that |
75 | +service. For example, if there was a service "mysql" and that service |
76 | +had two units, "mysql/0" and "mysql/1", then |
77 | + --service mysql |
78 | +is equivalent to |
79 | + --unit mysql/0,mysql/1 |
80 | + |
81 | +Commands run for services or units are executed in a 'hook context' for |
82 | +the unit. |
83 | + |
84 | +--all is provided as a simple way to run the command on all the machines |
85 | +in the environment. If you specify --all you cannot provide additional |
86 | +targets. |
87 | + |
88 | +` |
89 | + |
90 | +func (c *RunCommand) Info() *cmd.Info { |
91 | + return &cmd.Info{ |
92 | + Name: "run", |
93 | + Args: "<commands>", |
94 | + Purpose: "run the commands on the remote targets specified", |
95 | + Doc: runDoc, |
96 | + } |
97 | +} |
98 | + |
99 | +func (c *RunCommand) SetFlags(f *gnuflag.FlagSet) { |
100 | + c.EnvCommandBase.SetFlags(f) |
101 | + c.out.AddFlags(f, "smart", cmd.DefaultFormatters) |
102 | + f.BoolVar(&c.all, "all", false, "run the commands on all the machines") |
103 | + f.DurationVar(&c.timeout, "timeout", 5*time.Minute, "how long to wait before the remote command is considered to have failed") |
104 | + f.Var(cmd.NewStringsValue(nil, &c.machines), "machine", "one or more machine ids") |
105 | + f.Var(cmd.NewStringsValue(nil, &c.services), "service", "one or more service names") |
106 | + f.Var(cmd.NewStringsValue(nil, &c.units), "unit", "one or more unit ids") |
107 | +} |
108 | + |
109 | +func (c *RunCommand) Init(args []string) error { |
110 | + if len(args) == 0 { |
111 | + return errors.New("no commands specified") |
112 | + } |
113 | + c.commands, args = args[0], args[1:] |
114 | + |
115 | + if c.all { |
116 | + if len(c.machines) != 0 { |
117 | + return fmt.Errorf("You cannot specify --all and individual machines") |
118 | + } |
119 | + if len(c.services) != 0 { |
120 | + return fmt.Errorf("You cannot specify --all and individual services") |
121 | + } |
122 | + if len(c.units) != 0 { |
123 | + return fmt.Errorf("You cannot specify --all and individual units") |
124 | + } |
125 | + } else { |
126 | + if len(c.machines) == 0 && len(c.services) == 0 && len(c.units) == 0 { |
127 | + return fmt.Errorf("You must specify a target, either through --all, --machine, --service or --unit") |
128 | + } |
129 | + } |
130 | + |
131 | + var nameErrors []string |
132 | + for _, machineId := range c.machines { |
133 | + if !names.IsMachine(machineId) { |
134 | + nameErrors = append(nameErrors, fmt.Sprintf(" %q is not a valid machine id", machineId)) |
135 | + } |
136 | + } |
137 | + for _, service := range c.services { |
138 | + if !names.IsService(service) { |
139 | + nameErrors = append(nameErrors, fmt.Sprintf(" %q is not a valid service name", service)) |
140 | + } |
141 | + } |
142 | + for _, unit := range c.units { |
143 | + if !names.IsUnit(unit) { |
144 | + nameErrors = append(nameErrors, fmt.Sprintf(" %q is not a valid unit name", unit)) |
145 | + } |
146 | + } |
147 | + if len(nameErrors) > 0 { |
148 | + return fmt.Errorf("The following run targets are not valid:\n%s", |
149 | + strings.Join(nameErrors, "\n")) |
150 | + } |
151 | + |
152 | + return cmd.CheckEmpty(args) |
153 | +} |
154 | + |
155 | +func encodeBytes(input []byte) (value string, encoding string) { |
156 | + if utf8.Valid(input) { |
157 | + value = string(input) |
158 | + encoding = "utf8" |
159 | + } else { |
160 | + value = base64.StdEncoding.EncodeToString(input) |
161 | + encoding = "base64" |
162 | + } |
163 | + return value, encoding |
164 | +} |
165 | + |
166 | +func storeOutput(values map[string]interface{}, key string, input []byte) { |
167 | + value, encoding := encodeBytes(input) |
168 | + values[key] = value |
169 | + if encoding != "utf8" { |
170 | + values[key+".encoding"] = encoding |
171 | + } |
172 | +} |
173 | + |
174 | +// ConvertRunResults takes the results from the api and creates a map |
175 | +// suitable for format converstion to YAML or JSON. |
176 | +func ConvertRunResults(runResults []params.RunResult) interface{} { |
177 | + var results = make([]interface{}, len(runResults)) |
178 | + |
179 | + for i, result := range runResults { |
180 | + // We always want to have a string for stdout, but only show stderr, |
181 | + // code and error if they are there. |
182 | + values := make(map[string]interface{}) |
183 | + values["MachineId"] = result.MachineId |
184 | + if result.UnitId != "" { |
185 | + values["UnitId"] = result.UnitId |
186 | + |
187 | + } |
188 | + storeOutput(values, "Stdout", result.Stdout) |
189 | + if len(result.Stderr) > 0 { |
190 | + storeOutput(values, "Stderr", result.Stderr) |
191 | + } |
192 | + if result.Code != 0 { |
193 | + values["ReturnCode"] = result.Code |
194 | + } |
195 | + if result.Error != "" { |
196 | + values["Error"] = result.Error |
197 | + } |
198 | + results[i] = values |
199 | + } |
200 | + |
201 | + return results |
202 | +} |
203 | + |
204 | +func (c *RunCommand) Run(ctx *cmd.Context) error { |
205 | + client, err := getAPIClient(c.EnvName) |
206 | + if err != nil { |
207 | + return err |
208 | + } |
209 | + defer client.Close() |
210 | + |
211 | + var runResults []params.RunResult |
212 | + if c.all { |
213 | + runResults, err = client.RunOnAllMachines(c.commands, c.timeout) |
214 | + } else { |
215 | + params := params.RunParams{ |
216 | + Commands: c.commands, |
217 | + Timeout: c.timeout, |
218 | + Machines: c.machines, |
219 | + Services: c.services, |
220 | + Units: c.units, |
221 | + } |
222 | + runResults, err = client.Run(params) |
223 | + } |
224 | + |
225 | + if err != nil { |
226 | + return err |
227 | + } |
228 | + |
229 | + // If we are just dealing with one result, AND we are using the smart |
230 | + // format, then pretend we were running it locally. |
231 | + if len(runResults) == 1 && c.out.Name() == "smart" { |
232 | + result := runResults[0] |
233 | + ctx.Stdout.Write(result.Stdout) |
234 | + ctx.Stderr.Write(result.Stderr) |
235 | + if result.Error != "" { |
236 | + // Convert the error string back into an error object. |
237 | + return fmt.Errorf("%s", result.Error) |
238 | + } |
239 | + if result.Code != 0 { |
240 | + return cmd.NewRcPassthroughError(result.Code) |
241 | + } |
242 | + return nil |
243 | + } |
244 | + |
245 | + c.out.Write(ctx, ConvertRunResults(runResults)) |
246 | + return nil |
247 | +} |
248 | + |
249 | +// In order to be able to easily mock out the API side for testing, |
250 | +// the API client is got using a function. |
251 | + |
252 | +type RunClient interface { |
253 | + Close() error |
254 | + RunOnAllMachines(commands string, timeout time.Duration) ([]params.RunResult, error) |
255 | + Run(run params.RunParams) ([]params.RunResult, error) |
256 | +} |
257 | + |
258 | +// Here we need the signature to be correct for the interface. |
259 | +var getAPIClient = func(name string) (RunClient, error) { |
260 | + return juju.NewAPIClientFromName(name) |
261 | +} |
262 | |
263 | === added file 'cmd/juju/run_test.go' |
264 | --- cmd/juju/run_test.go 1970-01-01 00:00:00 +0000 |
265 | +++ cmd/juju/run_test.go 2014-01-12 20:39:24 +0000 |
266 | @@ -0,0 +1,443 @@ |
267 | +// Copyright 2013 Canonical Ltd. |
268 | +// Licensed under the AGPLv3, see LICENCE file for details. |
269 | + |
270 | +package main |
271 | + |
272 | +import ( |
273 | + "fmt" |
274 | + "time" |
275 | + |
276 | + gc "launchpad.net/gocheck" |
277 | + |
278 | + "launchpad.net/juju-core/cmd" |
279 | + "launchpad.net/juju-core/state/api/params" |
280 | + "launchpad.net/juju-core/testing" |
281 | + jc "launchpad.net/juju-core/testing/checkers" |
282 | +) |
283 | + |
284 | +type RunSuite struct { |
285 | + testing.FakeHomeSuite |
286 | +} |
287 | + |
288 | +var _ = gc.Suite(&RunSuite{}) |
289 | + |
290 | +func (*RunSuite) TestTargetArgParsing(c *gc.C) { |
291 | + for i, test := range []struct { |
292 | + message string |
293 | + args []string |
294 | + all bool |
295 | + machines []string |
296 | + units []string |
297 | + services []string |
298 | + commands string |
299 | + errMatch string |
300 | + }{{ |
301 | + message: "no args", |
302 | + errMatch: "no commands specified", |
303 | + }, { |
304 | + message: "no target", |
305 | + args: []string{"sudo reboot"}, |
306 | + errMatch: "You must specify a target, either through --all, --machine, --service or --unit", |
307 | + }, { |
308 | + message: "too many args", |
309 | + args: []string{"--all", "sudo reboot", "oops"}, |
310 | + errMatch: `unrecognized args: \["oops"\]`, |
311 | + }, { |
312 | + message: "command to all machines", |
313 | + args: []string{"--all", "sudo reboot"}, |
314 | + all: true, |
315 | + commands: "sudo reboot", |
316 | + }, { |
317 | + message: "all and defined machines", |
318 | + args: []string{"--all", "--machine=1,2", "sudo reboot"}, |
319 | + errMatch: `You cannot specify --all and individual machines`, |
320 | + }, { |
321 | + message: "command to machines 1, 2, and 1/kvm/0", |
322 | + args: []string{"--machine=1,2,1/kvm/0", "sudo reboot"}, |
323 | + commands: "sudo reboot", |
324 | + machines: []string{"1", "2", "1/kvm/0"}, |
325 | + }, { |
326 | + message: "bad machine names", |
327 | + args: []string{"--machine=foo,machine-2", "sudo reboot"}, |
328 | + errMatch: "" + |
329 | + "The following run targets are not valid:\n" + |
330 | + " \"foo\" is not a valid machine id\n" + |
331 | + " \"machine-2\" is not a valid machine id", |
332 | + }, { |
333 | + message: "all and defined services", |
334 | + args: []string{"--all", "--service=wordpress,mysql", "sudo reboot"}, |
335 | + errMatch: `You cannot specify --all and individual services`, |
336 | + }, { |
337 | + message: "command to services wordpress and mysql", |
338 | + args: []string{"--service=wordpress,mysql", "sudo reboot"}, |
339 | + commands: "sudo reboot", |
340 | + services: []string{"wordpress", "mysql"}, |
341 | + }, { |
342 | + message: "bad service names", |
343 | + args: []string{"--service", "foo,2,foo/0", "sudo reboot"}, |
344 | + errMatch: "" + |
345 | + "The following run targets are not valid:\n" + |
346 | + " \"2\" is not a valid service name\n" + |
347 | + " \"foo/0\" is not a valid service name", |
348 | + }, { |
349 | + message: "all and defined units", |
350 | + args: []string{"--all", "--unit=wordpress/0,mysql/1", "sudo reboot"}, |
351 | + errMatch: `You cannot specify --all and individual units`, |
352 | + }, { |
353 | + message: "command to valid units", |
354 | + args: []string{"--unit=wordpress/0,wordpress/1,mysql/0", "sudo reboot"}, |
355 | + commands: "sudo reboot", |
356 | + units: []string{"wordpress/0", "wordpress/1", "mysql/0"}, |
357 | + }, { |
358 | + message: "bad unit names", |
359 | + args: []string{"--unit", "foo,2,foo/0", "sudo reboot"}, |
360 | + errMatch: "" + |
361 | + "The following run targets are not valid:\n" + |
362 | + " \"foo\" is not a valid unit name\n" + |
363 | + " \"2\" is not a valid unit name", |
364 | + }, { |
365 | + message: "command to mixed valid targets", |
366 | + args: []string{"--machine=0", "--unit=wordpress/0,wordpress/1", "--service=mysql", "sudo reboot"}, |
367 | + commands: "sudo reboot", |
368 | + machines: []string{"0"}, |
369 | + services: []string{"mysql"}, |
370 | + units: []string{"wordpress/0", "wordpress/1"}, |
371 | + }} { |
372 | + c.Log(fmt.Sprintf("%v: %s", i, test.message)) |
373 | + runCmd := &RunCommand{} |
374 | + testing.TestInit(c, runCmd, test.args, test.errMatch) |
375 | + if test.errMatch == "" { |
376 | + c.Check(runCmd.all, gc.Equals, test.all) |
377 | + c.Check(runCmd.machines, gc.DeepEquals, test.machines) |
378 | + c.Check(runCmd.services, gc.DeepEquals, test.services) |
379 | + c.Check(runCmd.units, gc.DeepEquals, test.units) |
380 | + c.Check(runCmd.commands, gc.Equals, test.commands) |
381 | + } |
382 | + } |
383 | +} |
384 | + |
385 | +func (*RunSuite) TestTimeoutArgParsing(c *gc.C) { |
386 | + for i, test := range []struct { |
387 | + message string |
388 | + args []string |
389 | + errMatch string |
390 | + timeout time.Duration |
391 | + }{{ |
392 | + message: "default time", |
393 | + args: []string{"--all", "sudo reboot"}, |
394 | + timeout: 5 * time.Minute, |
395 | + }, { |
396 | + message: "invalid time", |
397 | + args: []string{"--timeout=foo", "--all", "sudo reboot"}, |
398 | + errMatch: `invalid value "foo" for flag --timeout: time: invalid duration foo`, |
399 | + }, { |
400 | + message: "two hours", |
401 | + args: []string{"--timeout=2h", "--all", "sudo reboot"}, |
402 | + timeout: 2 * time.Hour, |
403 | + }, { |
404 | + message: "3 minutes 30 seconds", |
405 | + args: []string{"--timeout=3m30s", "--all", "sudo reboot"}, |
406 | + timeout: (3 * time.Minute) + (30 * time.Second), |
407 | + }} { |
408 | + c.Log(fmt.Sprintf("%v: %s", i, test.message)) |
409 | + runCmd := &RunCommand{} |
410 | + testing.TestInit(c, runCmd, test.args, test.errMatch) |
411 | + if test.errMatch == "" { |
412 | + c.Check(runCmd.timeout, gc.Equals, test.timeout) |
413 | + } |
414 | + } |
415 | +} |
416 | + |
417 | +func (s *RunSuite) TestConvertRunResults(c *gc.C) { |
418 | + for i, test := range []struct { |
419 | + message string |
420 | + results []params.RunResult |
421 | + expected interface{} |
422 | + }{{ |
423 | + message: "empty", |
424 | + expected: []interface{}{}, |
425 | + }, { |
426 | + message: "minimum is machine id and stdout", |
427 | + results: []params.RunResult{ |
428 | + makeRunResult(mockResponse{machineId: "1"}), |
429 | + }, |
430 | + expected: []interface{}{ |
431 | + map[string]interface{}{ |
432 | + "MachineId": "1", |
433 | + "Stdout": "", |
434 | + }}, |
435 | + }, { |
436 | + message: "other fields are copied if there", |
437 | + results: []params.RunResult{ |
438 | + makeRunResult(mockResponse{ |
439 | + machineId: "1", |
440 | + stdout: "stdout", |
441 | + stderr: "stderr", |
442 | + code: 42, |
443 | + unitId: "unit/0", |
444 | + error: "error", |
445 | + }), |
446 | + }, |
447 | + expected: []interface{}{ |
448 | + map[string]interface{}{ |
449 | + "MachineId": "1", |
450 | + "Stdout": "stdout", |
451 | + "Stderr": "stderr", |
452 | + "ReturnCode": 42, |
453 | + "UnitId": "unit/0", |
454 | + "Error": "error", |
455 | + }}, |
456 | + }, { |
457 | + message: "stdout and stderr are base64 encoded if not valid utf8", |
458 | + results: []params.RunResult{ |
459 | + params.RunResult{ |
460 | + RemoteResponse: cmd.RemoteResponse{ |
461 | + Stdout: []byte{0xff}, |
462 | + Stderr: []byte{0xfe}, |
463 | + }, |
464 | + MachineId: "jake", |
465 | + }, |
466 | + }, |
467 | + expected: []interface{}{ |
468 | + map[string]interface{}{ |
469 | + "MachineId": "jake", |
470 | + "Stdout": "/w==", |
471 | + "Stdout.encoding": "base64", |
472 | + "Stderr": "/g==", |
473 | + "Stderr.encoding": "base64", |
474 | + }}, |
475 | + }, { |
476 | + message: "more than one", |
477 | + results: []params.RunResult{ |
478 | + makeRunResult(mockResponse{machineId: "1"}), |
479 | + makeRunResult(mockResponse{machineId: "2"}), |
480 | + makeRunResult(mockResponse{machineId: "3"}), |
481 | + }, |
482 | + expected: []interface{}{ |
483 | + map[string]interface{}{ |
484 | + "MachineId": "1", |
485 | + "Stdout": "", |
486 | + }, |
487 | + map[string]interface{}{ |
488 | + "MachineId": "2", |
489 | + "Stdout": "", |
490 | + }, |
491 | + map[string]interface{}{ |
492 | + "MachineId": "3", |
493 | + "Stdout": "", |
494 | + }, |
495 | + }, |
496 | + }} { |
497 | + c.Log(fmt.Sprintf("%v: %s", i, test.message)) |
498 | + result := ConvertRunResults(test.results) |
499 | + c.Check(result, jc.DeepEquals, test.expected) |
500 | + } |
501 | +} |
502 | + |
503 | +func (s *RunSuite) TestRunForMachineAndUnit(c *gc.C) { |
504 | + mock := s.setupMockAPI() |
505 | + machineResponse := mockResponse{ |
506 | + stdout: "megatron\n", |
507 | + machineId: "0", |
508 | + } |
509 | + unitResponse := mockResponse{ |
510 | + stdout: "bumblebee", |
511 | + machineId: "1", |
512 | + unitId: "unit/0", |
513 | + } |
514 | + mock.setResponse("0", machineResponse) |
515 | + mock.setResponse("unit/0", unitResponse) |
516 | + |
517 | + unformatted := ConvertRunResults([]params.RunResult{ |
518 | + makeRunResult(machineResponse), |
519 | + makeRunResult(unitResponse), |
520 | + }) |
521 | + |
522 | + jsonFormatted, err := cmd.FormatJson(unformatted) |
523 | + c.Assert(err, gc.IsNil) |
524 | + |
525 | + context, err := testing.RunCommand(c, &RunCommand{}, []string{ |
526 | + "--format=json", "--machine=0", "--unit=unit/0", "hostname", |
527 | + }) |
528 | + c.Assert(err, gc.IsNil) |
529 | + |
530 | + c.Check(testing.Stdout(context), gc.Equals, string(jsonFormatted)+"\n") |
531 | +} |
532 | + |
533 | +func (s *RunSuite) TestAllMachines(c *gc.C) { |
534 | + mock := s.setupMockAPI() |
535 | + mock.setMachinesAlive("0", "1") |
536 | + response0 := mockResponse{ |
537 | + stdout: "megatron\n", |
538 | + machineId: "0", |
539 | + } |
540 | + response1 := mockResponse{ |
541 | + error: "command timed out", |
542 | + machineId: "1", |
543 | + } |
544 | + mock.setResponse("0", response0) |
545 | + |
546 | + unformatted := ConvertRunResults([]params.RunResult{ |
547 | + makeRunResult(response0), |
548 | + makeRunResult(response1), |
549 | + }) |
550 | + |
551 | + jsonFormatted, err := cmd.FormatJson(unformatted) |
552 | + c.Assert(err, gc.IsNil) |
553 | + |
554 | + context, err := testing.RunCommand(c, &RunCommand{}, []string{ |
555 | + "--format=json", "--all", "hostname", |
556 | + }) |
557 | + c.Assert(err, gc.IsNil) |
558 | + |
559 | + c.Check(testing.Stdout(context), gc.Equals, string(jsonFormatted)+"\n") |
560 | +} |
561 | + |
562 | +func (s *RunSuite) TestSingleResponse(c *gc.C) { |
563 | + mock := s.setupMockAPI() |
564 | + mock.setMachinesAlive("0") |
565 | + mockResponse := mockResponse{ |
566 | + stdout: "stdout\n", |
567 | + stderr: "stderr\n", |
568 | + code: 42, |
569 | + machineId: "0", |
570 | + } |
571 | + mock.setResponse("0", mockResponse) |
572 | + unformatted := ConvertRunResults([]params.RunResult{ |
573 | + makeRunResult(mockResponse)}) |
574 | + yamlFormatted, err := cmd.FormatYaml(unformatted) |
575 | + c.Assert(err, gc.IsNil) |
576 | + jsonFormatted, err := cmd.FormatJson(unformatted) |
577 | + c.Assert(err, gc.IsNil) |
578 | + |
579 | + for i, test := range []struct { |
580 | + message string |
581 | + format string |
582 | + stdout string |
583 | + stderr string |
584 | + errorMatch string |
585 | + }{{ |
586 | + message: "smart (default)", |
587 | + stdout: "stdout\n", |
588 | + stderr: "stderr\n", |
589 | + errorMatch: "rc: 42", |
590 | + }, { |
591 | + message: "yaml output", |
592 | + format: "yaml", |
593 | + stdout: string(yamlFormatted) + "\n", |
594 | + }, { |
595 | + message: "json output", |
596 | + format: "json", |
597 | + stdout: string(jsonFormatted) + "\n", |
598 | + }} { |
599 | + c.Log(fmt.Sprintf("%v: %s", i, test.message)) |
600 | + args := []string{} |
601 | + if test.format != "" { |
602 | + args = append(args, "--format", test.format) |
603 | + } |
604 | + args = append(args, "--all", "ignored") |
605 | + context, err := testing.RunCommand(c, &RunCommand{}, args) |
606 | + if test.errorMatch != "" { |
607 | + c.Check(err, gc.ErrorMatches, test.errorMatch) |
608 | + } else { |
609 | + c.Check(err, gc.IsNil) |
610 | + } |
611 | + c.Check(testing.Stdout(context), gc.Equals, test.stdout) |
612 | + c.Check(testing.Stderr(context), gc.Equals, test.stderr) |
613 | + } |
614 | +} |
615 | + |
616 | +func (s *RunSuite) setupMockAPI() *mockRunAPI { |
617 | + mock := &mockRunAPI{} |
618 | + s.PatchValue(&getAPIClient, func(name string) (RunClient, error) { |
619 | + return mock, nil |
620 | + }) |
621 | + return mock |
622 | +} |
623 | + |
624 | +type mockRunAPI struct { |
625 | + stdout string |
626 | + stderr string |
627 | + code int |
628 | + // machines, services, units |
629 | + machines map[string]bool |
630 | + responses map[string]params.RunResult |
631 | +} |
632 | + |
633 | +type mockResponse struct { |
634 | + stdout string |
635 | + stderr string |
636 | + code int |
637 | + error string |
638 | + machineId string |
639 | + unitId string |
640 | +} |
641 | + |
642 | +var _ RunClient = (*mockRunAPI)(nil) |
643 | + |
644 | +func (m *mockRunAPI) setMachinesAlive(ids ...string) { |
645 | + if m.machines == nil { |
646 | + m.machines = make(map[string]bool) |
647 | + } |
648 | + for _, id := range ids { |
649 | + m.machines[id] = true |
650 | + } |
651 | +} |
652 | + |
653 | +func makeRunResult(mock mockResponse) params.RunResult { |
654 | + return params.RunResult{ |
655 | + RemoteResponse: cmd.RemoteResponse{ |
656 | + Stdout: []byte(mock.stdout), |
657 | + Stderr: []byte(mock.stderr), |
658 | + Code: mock.code, |
659 | + }, |
660 | + MachineId: mock.machineId, |
661 | + UnitId: mock.unitId, |
662 | + Error: mock.error, |
663 | + } |
664 | +} |
665 | + |
666 | +func (m *mockRunAPI) setResponse(id string, mock mockResponse) { |
667 | + if m.responses == nil { |
668 | + m.responses = make(map[string]params.RunResult) |
669 | + } |
670 | + m.responses[id] = makeRunResult(mock) |
671 | +} |
672 | + |
673 | +func (*mockRunAPI) Close() error { |
674 | + return nil |
675 | +} |
676 | + |
677 | +func (m *mockRunAPI) RunOnAllMachines(commands string, timeout time.Duration) ([]params.RunResult, error) { |
678 | + var result []params.RunResult |
679 | + for machine := range m.machines { |
680 | + response, found := m.responses[machine] |
681 | + if !found { |
682 | + // Consider this a timeout |
683 | + response = params.RunResult{MachineId: machine, Error: "command timed out"} |
684 | + } |
685 | + result = append(result, response) |
686 | + } |
687 | + |
688 | + return result, nil |
689 | +} |
690 | + |
691 | +func (m *mockRunAPI) Run(runParams params.RunParams) ([]params.RunResult, error) { |
692 | + var result []params.RunResult |
693 | + // Just add in ids that match in order. |
694 | + for _, id := range runParams.Machines { |
695 | + response, found := m.responses[id] |
696 | + if found { |
697 | + result = append(result, response) |
698 | + } |
699 | + } |
700 | + // mock ignores services |
701 | + for _, id := range runParams.Units { |
702 | + response, found := m.responses[id] |
703 | + if found { |
704 | + result = append(result, response) |
705 | + } |
706 | + } |
707 | + |
708 | + return result, nil |
709 | +} |
710 | |
711 | === modified file 'testing/environ.go' |
712 | --- testing/environ.go 2013-12-10 17:17:31 +0000 |
713 | +++ testing/environ.go 2014-01-12 20:39:24 +0000 |
714 | @@ -12,6 +12,7 @@ |
715 | |
716 | "launchpad.net/juju-core/environs/config" |
717 | "launchpad.net/juju-core/juju/osenv" |
718 | + "launchpad.net/juju-core/testing/testbase" |
719 | ) |
720 | |
721 | // FakeConfig() returns an environment configuration for a |
722 | @@ -233,3 +234,14 @@ |
723 | func MakeMultipleEnvHome(c *gc.C) *FakeHome { |
724 | return MakeFakeHome(c, MultipleEnvConfig, SampleCertName, SampleCertName+"-2") |
725 | } |
726 | + |
727 | +type FakeHomeSuite struct { |
728 | + testbase.LoggingSuite |
729 | + Home *FakeHome |
730 | +} |
731 | + |
732 | +func (s *FakeHomeSuite) SetUpTest(c *gc.C) { |
733 | + s.LoggingSuite.SetUpTest(c) |
734 | + s.Home = MakeSampleHome(c) |
735 | + s.AddCleanup(func(*gc.C) { s.Home.Restore() }) |
736 | +} |
Reviewers: mp+201124_ code.launchpad. net,
Message:
Please take a look.
Description:
Add the "run" command to the command line tool.
Hook up the Client Run and RunOnAllMachines API calls
to the command line tool.
https:/ /code.launchpad .net/~thumper/ juju-core/ juju-run- cli/+merge/ 201124
Requires: /code.launchpad .net/~thumper/ juju-core/ api-juju- run/+merge/ 201103
https:/
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/50310043/
Affected files (+652, -0 lines): main_test. go run_test. go
A [revision details]
M cmd/juju/main.go
M cmd/juju/
A cmd/juju/run.go
A cmd/juju/
M testing/environ.go