Merge lp:~fwereade/pyjuju/go-unit-commands into lp:pyjuju/go
- go-unit-commands
- Merge into go
Status: | Work in progress |
---|---|
Proposed branch: | lp:~fwereade/pyjuju/go-unit-commands |
Merge into: | lp:pyjuju/go |
Diff against target: |
1328 lines (+877/-88) 21 files modified
cmd/command.go (+74/-25) cmd/juju/bootstrap.go (+2/-2) cmd/juju/bootstrap_test.go (+2/-2) cmd/juju/main.go (+1/-1) cmd/jujud/agent.go (+1/-1) cmd/jujud/initzk.go (+2/-2) cmd/jujud/initzk_test.go (+1/-1) cmd/jujud/machine.go (+4/-3) cmd/jujud/main.go (+1/-1) cmd/jujud/provisioning.go (+6/-3) cmd/jujud/unit.go (+4/-3) cmd/jujud/util_test.go (+5/-5) cmd/proxy/main.go (+42/-0) cmd/proxy/proxy.go (+103/-0) cmd/proxy/proxy_test.go (+136/-0) cmd/server/protocol.go (+86/-0) cmd/server/protocol_test.go (+38/-0) cmd/server/server.go (+149/-0) cmd/server/server_test.go (+205/-0) cmd/supercommand.go (+10/-34) cmd/supercommand_test.go (+5/-5) |
To merge this branch: | bzr merge lp:~fwereade/pyjuju/go-unit-commands |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju Engineering | Pending | ||
Review via email: mp+94885@code.launchpad.net |
Commit message
Description of the change
Rough proposal for implementation of relation-get and similar commands.
In short: every command is implemented as a symlink to a single "proxy"
executable. This proxy communicates with a cmd.server.Server running in the
unit agent process, which uses a nameless SuperCommand to select and run the
command specified by the original proxy's full command line, and
communicates stderr and stdout back to the original process.
This necessitated a change to cmd, such that a new Env interface has been
added, which is responsible for the command's interactions with its
environment; cmd.DefaultEnv implements the original functionality, while
cmd.server.
back to the proxy process. Env has ended up needing to be threaded through
much of cmd, but the impact on calling code is minimal.
- 80. By William Reade
-
add tests for command server
- 81. By William Reade
-
added tests for request/response
- 82. By William Reade
-
doc tweaks
William Reade (fwereade) wrote : | # |
On Tue, 2012-02-28 at 18:22 +0000, Gustavo Niemeyer wrote:
> Can this be broken down in smaller chunks?
>
> The past changes have all been similarly large in scope. I thought they
> might be hard to break down, but I'm getting more skeptical that every
> single change needs to touch 20 files. Reviewing, applying comments, and
> getting them through the pipeline in general is a lot more traumatic
> that way.
Fair point, and this will definitely break down cleanly into 2 parts;
more might be a little tricky, but I'll see what I can do.
Gustavo Niemeyer (niemeyer) wrote : | # |
> Fair point, and this will definitely break down cleanly into 2 parts;
> more might be a little tricky, but I'll see what I can do.
I suspect there are many more independent changes packed into that stack.
--
Gustavo Niemeyer
http://
http://
http://
http://
-- I'm not absolutely sure of anything.
Unmerged revisions
- 82. By William Reade
-
doc tweaks
- 81. By William Reade
-
added tests for request/response
- 80. By William Reade
-
add tests for command server
- 79. By William Reade
-
moved packages again; added max request size check; moved InitOutput from SuperCommand to Env, to prevent potential pollution of server process settings
- 78. By William Reade
-
rearrange packages
- 77. By William Reade
-
moved response handling next to request handling
- 76. By William Reade
-
merge parent
- 75. By William Reade
-
docs
- 74. By William Reade
-
we have sometests,and apparently they work...
- 73. By William Reade
-
added testable entry point for Main
Preview Diff
1 | === modified file 'cmd/command.go' | |||
2 | --- cmd/command.go 2012-02-15 10:29:05 +0000 | |||
3 | +++ cmd/command.go 2012-02-28 17:13:19 +0000 | |||
4 | @@ -2,8 +2,10 @@ | |||
5 | 2 | 2 | ||
6 | 3 | import ( | 3 | import ( |
7 | 4 | "fmt" | 4 | "fmt" |
8 | 5 | "io" | ||
9 | 5 | "launchpad.net/gnuflag" | 6 | "launchpad.net/gnuflag" |
10 | 6 | "launchpad.net/juju/go/log" | 7 | "launchpad.net/juju/go/log" |
11 | 8 | stdlog "log" | ||
12 | 7 | "os" | 9 | "os" |
13 | 8 | "strings" | 10 | "strings" |
14 | 9 | ) | 11 | ) |
15 | @@ -32,8 +34,7 @@ | |||
16 | 32 | return fmt.Sprintf("%s %s", i.Name, i.Args) | 34 | return fmt.Sprintf("%s %s", i.Name, i.Args) |
17 | 33 | } | 35 | } |
18 | 34 | 36 | ||
21 | 35 | // Command is implemented by types that interpret any command-line arguments | 37 | // Command is implemented by types that interpret command-line arguments. |
20 | 36 | // passed to the "juju" command. | ||
22 | 37 | type Command interface { | 38 | type Command interface { |
23 | 38 | // Info returns information about the command. | 39 | // Info returns information about the command. |
24 | 39 | Info() *Info | 40 | Info() *Info |
25 | @@ -44,40 +45,85 @@ | |||
26 | 44 | 45 | ||
27 | 45 | // ParsePositional is called by Parse to allow the Command to handle | 46 | // ParsePositional is called by Parse to allow the Command to handle |
28 | 46 | // positional command-line arguments. | 47 | // positional command-line arguments. |
30 | 47 | ParsePositional(args []string) error | 48 | ParsePositional(e Env, args []string) error |
31 | 48 | 49 | ||
32 | 49 | // Run will execute the command according to the options and positional | 50 | // Run will execute the command according to the options and positional |
33 | 50 | // arguments interpreted by a call to Parse. | 51 | // arguments interpreted by a call to Parse. |
35 | 51 | Run() error | 52 | Run(e Env) error |
36 | 53 | } | ||
37 | 54 | |||
38 | 55 | // Env represents an environment in which a Command can be run. | ||
39 | 56 | type Env interface { | ||
40 | 57 | Stderr() io.Writer | ||
41 | 58 | Stdout() io.Writer | ||
42 | 59 | Exit(int) | ||
43 | 60 | InitOutput(bool, bool, string) error | ||
44 | 61 | } | ||
45 | 62 | |||
46 | 63 | // DefaultEnv is the environment in which normal command-line commands are run. | ||
47 | 64 | type DefaultEnv struct{} | ||
48 | 65 | |||
49 | 66 | func (e *DefaultEnv) Stdout() io.Writer { | ||
50 | 67 | return os.Stdout | ||
51 | 68 | } | ||
52 | 69 | |||
53 | 70 | func (e *DefaultEnv) Stderr() io.Writer { | ||
54 | 71 | return os.Stderr | ||
55 | 72 | } | ||
56 | 73 | |||
57 | 74 | func (e *DefaultEnv) Exit(code int) { | ||
58 | 75 | os.Exit(code) | ||
59 | 76 | } | ||
60 | 77 | |||
61 | 78 | func (e *DefaultEnv) InitOutput(verbose bool, debug bool, logfile string) error { | ||
62 | 79 | if debug { | ||
63 | 80 | log.Debug = true | ||
64 | 81 | } | ||
65 | 82 | var target io.Writer | ||
66 | 83 | if logfile != "" { | ||
67 | 84 | var err error | ||
68 | 85 | target, err = os.OpenFile(logfile, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644) | ||
69 | 86 | if err != nil { | ||
70 | 87 | return err | ||
71 | 88 | } | ||
72 | 89 | } else if verbose || debug { | ||
73 | 90 | target = e.Stderr() | ||
74 | 91 | } | ||
75 | 92 | if target != nil { | ||
76 | 93 | log.Target = stdlog.New(target, "", stdlog.LstdFlags) | ||
77 | 94 | } | ||
78 | 95 | return nil | ||
79 | 52 | } | 96 | } |
80 | 53 | 97 | ||
81 | 54 | // NewFlagSet returns a FlagSet initialized for use with c. | 98 | // NewFlagSet returns a FlagSet initialized for use with c. |
85 | 55 | func NewFlagSet(c Command) *gnuflag.FlagSet { | 99 | func NewFlagSet(e Env, c Command) *gnuflag.FlagSet { |
86 | 56 | f := gnuflag.NewFlagSet(c.Info().Name, gnuflag.ExitOnError) | 100 | f := gnuflag.NewFlagSet(c.Info().Name, gnuflag.ContinueOnError) |
87 | 57 | f.Usage = func() { PrintUsage(c) } | 101 | f.SetOutput(e.Stderr()) |
88 | 102 | f.Usage = func() { PrintUsage(e, c) } | ||
89 | 58 | c.InitFlagSet(f) | 103 | c.InitFlagSet(f) |
90 | 59 | return f | 104 | return f |
91 | 60 | } | 105 | } |
92 | 61 | 106 | ||
93 | 62 | // PrintUsage prints usage information for c to stderr. | 107 | // PrintUsage prints usage information for c to stderr. |
95 | 63 | func PrintUsage(c Command) { | 108 | func PrintUsage(e Env, c Command) { |
96 | 64 | i := c.Info() | 109 | i := c.Info() |
101 | 65 | fmt.Fprintf(os.Stderr, "usage: %s\n", i.Usage()) | 110 | stderr := e.Stderr() |
102 | 66 | fmt.Fprintf(os.Stderr, "purpose: %s\n", i.Purpose) | 111 | fmt.Fprintf(stderr, "usage: %s\n", i.Usage()) |
103 | 67 | fmt.Fprintf(os.Stderr, "\noptions:\n") | 112 | fmt.Fprintf(stderr, "purpose: %s\n", i.Purpose) |
104 | 68 | NewFlagSet(c).PrintDefaults() | 113 | fmt.Fprintf(stderr, "\noptions:\n") |
105 | 114 | NewFlagSet(e, c).PrintDefaults() | ||
106 | 69 | if i.Doc != "" { | 115 | if i.Doc != "" { |
108 | 70 | fmt.Fprintf(os.Stderr, "\n%s\n", strings.TrimSpace(i.Doc)) | 116 | fmt.Fprintf(stderr, "\n%s\n", strings.TrimSpace(i.Doc)) |
109 | 71 | } | 117 | } |
110 | 72 | } | 118 | } |
111 | 73 | 119 | ||
112 | 74 | // Parse parses args on c. This must be called before c is Run. | 120 | // Parse parses args on c. This must be called before c is Run. |
115 | 75 | func Parse(c Command, args []string) error { | 121 | func Parse(e Env, c Command, args []string) error { |
116 | 76 | f := NewFlagSet(c) | 122 | f := NewFlagSet(e, c) |
117 | 77 | if err := f.Parse(c.Info().Intersperse, args); err != nil { | 123 | if err := f.Parse(c.Info().Intersperse, args); err != nil { |
118 | 78 | return err | 124 | return err |
119 | 79 | } | 125 | } |
121 | 80 | return c.ParsePositional(f.Args()) | 126 | return c.ParsePositional(e, f.Args()) |
122 | 81 | } | 127 | } |
123 | 82 | 128 | ||
124 | 83 | // CheckEmpty is a utility function that returns an error if args is not empty. | 129 | // CheckEmpty is a utility function that returns an error if args is not empty. |
125 | @@ -89,16 +135,19 @@ | |||
126 | 89 | } | 135 | } |
127 | 90 | 136 | ||
128 | 91 | // Main will Parse and Run a Command, and exit appropriately. | 137 | // Main will Parse and Run a Command, and exit appropriately. |
134 | 92 | func Main(c Command, args []string) { | 138 | func Main(e Env, c Command, args []string) { |
135 | 93 | if err := Parse(c, args[1:]); err != nil { | 139 | stderr := e.Stderr() |
136 | 94 | fmt.Fprintf(os.Stderr, "%v\n", err) | 140 | if err := Parse(e, c, args); err != nil { |
137 | 95 | PrintUsage(c) | 141 | fmt.Fprintf(stderr, "%v\n", err) |
138 | 96 | os.Exit(2) | 142 | PrintUsage(e, c) |
139 | 143 | e.Exit(2) | ||
140 | 144 | return | ||
141 | 97 | } | 145 | } |
143 | 98 | if err := c.Run(); err != nil { | 146 | if err := c.Run(e); err != nil { |
144 | 99 | log.Debugf("%s command failed: %s\n", c.Info().Name, err) | 147 | log.Debugf("%s command failed: %s\n", c.Info().Name, err) |
147 | 100 | fmt.Fprintf(os.Stderr, "%v\n", err) | 148 | fmt.Fprintf(stderr, "%v\n", err) |
148 | 101 | os.Exit(1) | 149 | e.Exit(1) |
149 | 150 | return | ||
150 | 102 | } | 151 | } |
152 | 103 | os.Exit(0) | 152 | e.Exit(0) |
153 | 104 | } | 153 | } |
154 | 105 | 154 | ||
155 | === modified file 'cmd/juju/bootstrap.go' | |||
156 | --- cmd/juju/bootstrap.go 2012-02-09 11:35:24 +0000 | |||
157 | +++ cmd/juju/bootstrap.go 2012-02-28 17:13:19 +0000 | |||
158 | @@ -30,13 +30,13 @@ | |||
159 | 30 | 30 | ||
160 | 31 | // ParsePositional checks that no unexpected extra command-line arguments have | 31 | // ParsePositional checks that no unexpected extra command-line arguments have |
161 | 32 | // been specified. | 32 | // been specified. |
163 | 33 | func (c *BootstrapCommand) ParsePositional(args []string) error { | 33 | func (c *BootstrapCommand) ParsePositional(e cmd.Env, args []string) error { |
164 | 34 | return cmd.CheckEmpty(args) | 34 | return cmd.CheckEmpty(args) |
165 | 35 | } | 35 | } |
166 | 36 | 36 | ||
167 | 37 | // Run connects to the environment specified on the command line and bootstraps | 37 | // Run connects to the environment specified on the command line and bootstraps |
168 | 38 | // a juju in that environment if none already exists. | 38 | // a juju in that environment if none already exists. |
170 | 39 | func (c *BootstrapCommand) Run() error { | 39 | func (c *BootstrapCommand) Run(e cmd.Env) error { |
171 | 40 | conn, err := juju.NewConn(c.Environment) | 40 | conn, err := juju.NewConn(c.Environment) |
172 | 41 | if err != nil { | 41 | if err != nil { |
173 | 42 | return err | 42 | return err |
174 | 43 | 43 | ||
175 | === modified file 'cmd/juju/bootstrap_test.go' | |||
176 | --- cmd/juju/bootstrap_test.go 2012-02-08 11:29:32 +0000 | |||
177 | +++ cmd/juju/bootstrap_test.go 2012-02-28 17:13:19 +0000 | |||
178 | @@ -12,7 +12,7 @@ | |||
179 | 12 | 12 | ||
180 | 13 | func checkEnv(c *C, args []string, expect string) { | 13 | func checkEnv(c *C, args []string, expect string) { |
181 | 14 | bc := &main.BootstrapCommand{} | 14 | bc := &main.BootstrapCommand{} |
183 | 15 | err := cmd.Parse(bc, args) | 15 | err := cmd.Parse(&cmd.DefaultEnv{}, bc, args) |
184 | 16 | c.Assert(err, IsNil) | 16 | c.Assert(err, IsNil) |
185 | 17 | c.Assert(bc.Environment, Equals, expect) | 17 | c.Assert(bc.Environment, Equals, expect) |
186 | 18 | } | 18 | } |
187 | @@ -20,7 +20,7 @@ | |||
188 | 20 | func (s *BootstrapSuite) TestEnvironment(c *C) { | 20 | func (s *BootstrapSuite) TestEnvironment(c *C) { |
189 | 21 | bc := &main.BootstrapCommand{} | 21 | bc := &main.BootstrapCommand{} |
190 | 22 | c.Assert(bc.Environment, Equals, "") | 22 | c.Assert(bc.Environment, Equals, "") |
192 | 23 | err := cmd.Parse(bc, []string{"hotdog"}) | 23 | err := cmd.Parse(&cmd.DefaultEnv{}, bc, []string{"hotdog"}) |
193 | 24 | c.Assert(err, ErrorMatches, `unrecognised args: \[hotdog\]`) | 24 | c.Assert(err, ErrorMatches, `unrecognised args: \[hotdog\]`) |
194 | 25 | c.Assert(bc.Environment, Equals, "") | 25 | c.Assert(bc.Environment, Equals, "") |
195 | 26 | 26 | ||
196 | 27 | 27 | ||
197 | === modified file 'cmd/juju/main.go' | |||
198 | --- cmd/juju/main.go 2012-02-14 16:42:42 +0000 | |||
199 | +++ cmd/juju/main.go 2012-02-28 17:13:19 +0000 | |||
200 | @@ -18,7 +18,7 @@ | |||
201 | 18 | func Main(args []string) { | 18 | func Main(args []string) { |
202 | 19 | jc := cmd.NewSuperCommand("juju", jujuDoc) | 19 | jc := cmd.NewSuperCommand("juju", jujuDoc) |
203 | 20 | jc.Register(&BootstrapCommand{}) | 20 | jc.Register(&BootstrapCommand{}) |
205 | 21 | cmd.Main(jc, args) | 21 | cmd.Main(&cmd.DefaultEnv{}, jc, args[1:]) |
206 | 22 | } | 22 | } |
207 | 23 | 23 | ||
208 | 24 | func main() { | 24 | func main() { |
209 | 25 | 25 | ||
210 | === modified file 'cmd/jujud/agent.go' | |||
211 | --- cmd/jujud/agent.go 2012-02-21 15:13:47 +0000 | |||
212 | +++ cmd/jujud/agent.go 2012-02-28 17:13:19 +0000 | |||
213 | @@ -36,7 +36,7 @@ | |||
214 | 36 | 36 | ||
215 | 37 | // ParsePositional checks that there are no unwanted arguments, and that all | 37 | // ParsePositional checks that there are no unwanted arguments, and that all |
216 | 38 | // required flags have been set. | 38 | // required flags have been set. |
218 | 39 | func (c *agentConf) ParsePositional(args []string) error { | 39 | func (c *agentConf) ParsePositional(e cmd.Env, args []string) error { |
219 | 40 | if c.jujuDir == "" { | 40 | if c.jujuDir == "" { |
220 | 41 | return requiredError("juju-directory") | 41 | return requiredError("juju-directory") |
221 | 42 | } | 42 | } |
222 | 43 | 43 | ||
223 | === modified file 'cmd/jujud/initzk.go' | |||
224 | --- cmd/jujud/initzk.go 2012-02-21 15:13:47 +0000 | |||
225 | +++ cmd/jujud/initzk.go 2012-02-28 17:13:19 +0000 | |||
226 | @@ -32,7 +32,7 @@ | |||
227 | 32 | 32 | ||
228 | 33 | // ParsePositional checks that there are no unwanted arguments, and that all | 33 | // ParsePositional checks that there are no unwanted arguments, and that all |
229 | 34 | // required flags have been set. | 34 | // required flags have been set. |
231 | 35 | func (c *InitzkCommand) ParsePositional(args []string) error { | 35 | func (c *InitzkCommand) ParsePositional(e cmd.Env, args []string) error { |
232 | 36 | if c.StateInfo.Addrs == nil { | 36 | if c.StateInfo.Addrs == nil { |
233 | 37 | return requiredError("zookeeper-servers") | 37 | return requiredError("zookeeper-servers") |
234 | 38 | } | 38 | } |
235 | @@ -46,7 +46,7 @@ | |||
236 | 46 | } | 46 | } |
237 | 47 | 47 | ||
238 | 48 | // Run initializes zookeeper state for an environment. | 48 | // Run initializes zookeeper state for an environment. |
240 | 49 | func (c *InitzkCommand) Run() error { | 49 | func (c *InitzkCommand) Run(e cmd.Env) error { |
241 | 50 | // TODO connect to zookeeper; call State.Initialize | 50 | // TODO connect to zookeeper; call State.Initialize |
242 | 51 | return fmt.Errorf("InitzkCommand.Run not implemented") | 51 | return fmt.Errorf("InitzkCommand.Run not implemented") |
243 | 52 | } | 52 | } |
244 | 53 | 53 | ||
245 | === modified file 'cmd/jujud/initzk_test.go' | |||
246 | --- cmd/jujud/initzk_test.go 2012-02-27 08:48:13 +0000 | |||
247 | +++ cmd/jujud/initzk_test.go 2012-02-28 17:13:19 +0000 | |||
248 | @@ -12,7 +12,7 @@ | |||
249 | 12 | 12 | ||
250 | 13 | func parseInitzkCommand(args []string) (*main.InitzkCommand, error) { | 13 | func parseInitzkCommand(args []string) (*main.InitzkCommand, error) { |
251 | 14 | c := &main.InitzkCommand{} | 14 | c := &main.InitzkCommand{} |
253 | 15 | err := cmd.Parse(c, args) | 15 | err := cmd.Parse(&cmd.DefaultEnv{}, c, args) |
254 | 16 | return c, err | 16 | return c, err |
255 | 17 | } | 17 | } |
256 | 18 | 18 | ||
257 | 19 | 19 | ||
258 | === modified file 'cmd/jujud/machine.go' | |||
259 | --- cmd/jujud/machine.go 2012-02-15 12:28:26 +0000 | |||
260 | +++ cmd/jujud/machine.go 2012-02-28 17:13:19 +0000 | |||
261 | @@ -3,6 +3,7 @@ | |||
262 | 3 | import ( | 3 | import ( |
263 | 4 | "fmt" | 4 | "fmt" |
264 | 5 | "launchpad.net/gnuflag" | 5 | "launchpad.net/gnuflag" |
265 | 6 | "launchpad.net/juju/go/cmd" | ||
266 | 6 | ) | 7 | ) |
267 | 7 | 8 | ||
268 | 8 | // MachineAgent is a cmd.Command responsible for running a machine agent. | 9 | // MachineAgent is a cmd.Command responsible for running a machine agent. |
269 | @@ -23,14 +24,14 @@ | |||
270 | 23 | 24 | ||
271 | 24 | // ParsePositional checks that there are no unwanted arguments, and that all | 25 | // ParsePositional checks that there are no unwanted arguments, and that all |
272 | 25 | // required flags have been set. | 26 | // required flags have been set. |
274 | 26 | func (a *MachineAgent) ParsePositional(args []string) error { | 27 | func (a *MachineAgent) ParsePositional(e cmd.Env, args []string) error { |
275 | 27 | if a.MachineId < 0 { | 28 | if a.MachineId < 0 { |
276 | 28 | return fmt.Errorf("--machine-id option must be set, and expects a non-negative integer") | 29 | return fmt.Errorf("--machine-id option must be set, and expects a non-negative integer") |
277 | 29 | } | 30 | } |
279 | 30 | return a.agentConf.ParsePositional(args) | 31 | return a.agentConf.ParsePositional(e, args) |
280 | 31 | } | 32 | } |
281 | 32 | 33 | ||
282 | 33 | // Run runs a machine agent. | 34 | // Run runs a machine agent. |
284 | 34 | func (a *MachineAgent) Run() error { | 35 | func (a *MachineAgent) Run(e cmd.Env) error { |
285 | 35 | return fmt.Errorf("MachineAgent.Run not implemented") | 36 | return fmt.Errorf("MachineAgent.Run not implemented") |
286 | 36 | } | 37 | } |
287 | 37 | 38 | ||
288 | === modified file 'cmd/jujud/main.go' | |||
289 | --- cmd/jujud/main.go 2012-02-15 10:10:38 +0000 | |||
290 | +++ cmd/jujud/main.go 2012-02-28 17:13:19 +0000 | |||
291 | @@ -21,7 +21,7 @@ | |||
292 | 21 | jc.Register(NewUnitAgent()) | 21 | jc.Register(NewUnitAgent()) |
293 | 22 | jc.Register(NewMachineAgent()) | 22 | jc.Register(NewMachineAgent()) |
294 | 23 | jc.Register(NewProvisioningAgent()) | 23 | jc.Register(NewProvisioningAgent()) |
296 | 24 | cmd.Main(jc, args) | 24 | cmd.Main(&cmd.DefaultEnv{}, jc, args[1:]) |
297 | 25 | } | 25 | } |
298 | 26 | 26 | ||
299 | 27 | func main() { | 27 | func main() { |
300 | 28 | 28 | ||
301 | === modified file 'cmd/jujud/provisioning.go' | |||
302 | --- cmd/jujud/provisioning.go 2012-02-15 10:29:05 +0000 | |||
303 | +++ cmd/jujud/provisioning.go 2012-02-28 17:13:19 +0000 | |||
304 | @@ -1,6 +1,9 @@ | |||
305 | 1 | package main | 1 | package main |
306 | 2 | 2 | ||
308 | 3 | import "fmt" | 3 | import ( |
309 | 4 | "fmt" | ||
310 | 5 | "launchpad.net/juju/go/cmd" | ||
311 | 6 | ) | ||
312 | 4 | 7 | ||
313 | 5 | // ProvisioningAgent is a cmd.Command responsible for running a provisioning agent. | 8 | // ProvisioningAgent is a cmd.Command responsible for running a provisioning agent. |
314 | 6 | type ProvisioningAgent struct { | 9 | type ProvisioningAgent struct { |
315 | @@ -12,6 +15,6 @@ | |||
316 | 12 | } | 15 | } |
317 | 13 | 16 | ||
318 | 14 | // Run runs a provisioning agent. | 17 | // Run runs a provisioning agent. |
321 | 15 | func (a *ProvisioningAgent) Run() error { | 18 | func (a *ProvisioningAgent) Run(e cmd.Env) error { |
322 | 16 | return fmt.Errorf("MachineAgent.Run not implemented") | 19 | return fmt.Errorf("ProvisioningAgent.Run not implemented") |
323 | 17 | } | 20 | } |
324 | 18 | 21 | ||
325 | === modified file 'cmd/jujud/unit.go' | |||
326 | --- cmd/jujud/unit.go 2012-02-17 08:03:35 +0000 | |||
327 | +++ cmd/jujud/unit.go 2012-02-28 17:13:19 +0000 | |||
328 | @@ -3,6 +3,7 @@ | |||
329 | 3 | import ( | 3 | import ( |
330 | 4 | "fmt" | 4 | "fmt" |
331 | 5 | "launchpad.net/gnuflag" | 5 | "launchpad.net/gnuflag" |
332 | 6 | "launchpad.net/juju/go/cmd" | ||
333 | 6 | "launchpad.net/juju/go/juju" | 7 | "launchpad.net/juju/go/juju" |
334 | 7 | ) | 8 | ) |
335 | 8 | 9 | ||
336 | @@ -24,17 +25,17 @@ | |||
337 | 24 | 25 | ||
338 | 25 | // ParsePositional checks that there are no unwanted arguments, and that all | 26 | // ParsePositional checks that there are no unwanted arguments, and that all |
339 | 26 | // required flags have been set. | 27 | // required flags have been set. |
341 | 27 | func (a *UnitAgent) ParsePositional(args []string) error { | 28 | func (a *UnitAgent) ParsePositional(e cmd.Env, args []string) error { |
342 | 28 | if a.UnitName == "" { | 29 | if a.UnitName == "" { |
343 | 29 | return requiredError("unit-name") | 30 | return requiredError("unit-name") |
344 | 30 | } | 31 | } |
345 | 31 | if !juju.ValidUnit.MatchString(a.UnitName) { | 32 | if !juju.ValidUnit.MatchString(a.UnitName) { |
346 | 32 | return fmt.Errorf(`--unit-name option expects "<service>/<n>" argument`) | 33 | return fmt.Errorf(`--unit-name option expects "<service>/<n>" argument`) |
347 | 33 | } | 34 | } |
349 | 34 | return a.agentConf.ParsePositional(args) | 35 | return a.agentConf.ParsePositional(e, args) |
350 | 35 | } | 36 | } |
351 | 36 | 37 | ||
352 | 37 | // Run runs a unit agent. | 38 | // Run runs a unit agent. |
354 | 38 | func (a *UnitAgent) Run() error { | 39 | func (a *UnitAgent) Run(e cmd.Env) error { |
355 | 39 | return fmt.Errorf("UnitAgent.Run not implemented") | 40 | return fmt.Errorf("UnitAgent.Run not implemented") |
356 | 40 | } | 41 | } |
357 | 41 | 42 | ||
358 | === modified file 'cmd/jujud/util_test.go' | |||
359 | --- cmd/jujud/util_test.go 2012-02-27 08:48:13 +0000 | |||
360 | +++ cmd/jujud/util_test.go 2012-02-28 17:13:19 +0000 | |||
361 | @@ -13,23 +13,23 @@ | |||
362 | 13 | // command pre-parsed with the always-required options and whatever others | 13 | // command pre-parsed with the always-required options and whatever others |
363 | 14 | // are necessary to allow parsing to succeed (specified in args). | 14 | // are necessary to allow parsing to succeed (specified in args). |
364 | 15 | func CheckAgentCommand(c *C, create acCreator, args []string) main.AgentCommand { | 15 | func CheckAgentCommand(c *C, create acCreator, args []string) main.AgentCommand { |
366 | 16 | err := cmd.Parse(create(), args) | 16 | err := cmd.Parse(&cmd.DefaultEnv{}, create(), args) |
367 | 17 | c.Assert(err, ErrorMatches, "--zookeeper-servers option must be set") | 17 | c.Assert(err, ErrorMatches, "--zookeeper-servers option must be set") |
368 | 18 | args = append(args, "--zookeeper-servers", "zk1:2181,zk2:2181") | 18 | args = append(args, "--zookeeper-servers", "zk1:2181,zk2:2181") |
369 | 19 | 19 | ||
371 | 20 | err = cmd.Parse(create(), args) | 20 | err = cmd.Parse(&cmd.DefaultEnv{}, create(), args) |
372 | 21 | c.Assert(err, ErrorMatches, "--session-file option must be set") | 21 | c.Assert(err, ErrorMatches, "--session-file option must be set") |
373 | 22 | args = append(args, "--session-file", "sf") | 22 | args = append(args, "--session-file", "sf") |
374 | 23 | 23 | ||
375 | 24 | ac := create() | 24 | ac := create() |
377 | 25 | c.Assert(cmd.Parse(ac, args), IsNil) | 25 | c.Assert(cmd.Parse(&cmd.DefaultEnv{}, ac, args), IsNil) |
378 | 26 | c.Assert(ac.StateInfo().Addrs, DeepEquals, []string{"zk1:2181", "zk2:2181"}) | 26 | c.Assert(ac.StateInfo().Addrs, DeepEquals, []string{"zk1:2181", "zk2:2181"}) |
379 | 27 | c.Assert(ac.SessionFile(), Equals, "sf") | 27 | c.Assert(ac.SessionFile(), Equals, "sf") |
380 | 28 | c.Assert(ac.JujuDir(), Equals, "/var/lib/juju") | 28 | c.Assert(ac.JujuDir(), Equals, "/var/lib/juju") |
381 | 29 | args = append(args, "--juju-directory", "jd") | 29 | args = append(args, "--juju-directory", "jd") |
382 | 30 | 30 | ||
383 | 31 | ac = create() | 31 | ac = create() |
385 | 32 | c.Assert(cmd.Parse(ac, args), IsNil) | 32 | c.Assert(cmd.Parse(&cmd.DefaultEnv{}, ac, args), IsNil) |
386 | 33 | c.Assert(ac.StateInfo().Addrs, DeepEquals, []string{"zk1:2181", "zk2:2181"}) | 33 | c.Assert(ac.StateInfo().Addrs, DeepEquals, []string{"zk1:2181", "zk2:2181"}) |
387 | 34 | c.Assert(ac.SessionFile(), Equals, "sf") | 34 | c.Assert(ac.SessionFile(), Equals, "sf") |
388 | 35 | c.Assert(ac.JujuDir(), Equals, "jd") | 35 | c.Assert(ac.JujuDir(), Equals, "jd") |
389 | @@ -44,5 +44,5 @@ | |||
390 | 44 | "--session-file", "sf", | 44 | "--session-file", "sf", |
391 | 45 | "--juju-directory", "jd", | 45 | "--juju-directory", "jd", |
392 | 46 | } | 46 | } |
394 | 47 | return cmd.Parse(ac, append(common, args...)) | 47 | return cmd.Parse(&cmd.DefaultEnv{}, ac, append(common, args...)) |
395 | 48 | } | 48 | } |
396 | 49 | 49 | ||
397 | === added directory 'cmd/proxy' | |||
398 | === added file 'cmd/proxy/main.go' | |||
399 | --- cmd/proxy/main.go 1970-01-01 00:00:00 +0000 | |||
400 | +++ cmd/proxy/main.go 2012-02-28 17:13:19 +0000 | |||
401 | @@ -0,0 +1,42 @@ | |||
402 | 1 | package main | ||
403 | 2 | |||
404 | 3 | import ( | ||
405 | 4 | "fmt" | ||
406 | 5 | "os" | ||
407 | 6 | ) | ||
408 | 7 | |||
409 | 8 | // requireEnv returns an environment variable, and calls os.Exit(2) if it's not | ||
410 | 9 | // set. | ||
411 | 10 | func requireEnv(name string) string { | ||
412 | 11 | value := os.Getenv(name) | ||
413 | 12 | if value == "" { | ||
414 | 13 | fmt.Fprintf(os.Stderr, "%s not set\n", name) | ||
415 | 14 | os.Exit(2) | ||
416 | 15 | } | ||
417 | 16 | return value | ||
418 | 17 | } | ||
419 | 18 | |||
420 | 19 | // Main implements every command that calls back into a juju unit agent by using | ||
421 | 20 | // a Proxy to ask the unit agent to execute the actual command for us and pipe | ||
422 | 21 | // back stdout/stderr. This function is not redundant with main, because it | ||
423 | 22 | // provides an entry point for testing with arbitrary command line arguments. | ||
424 | 23 | // Individual commands should be exposed by symlinking the command name to this | ||
425 | 24 | // executable. | ||
426 | 25 | func Main(args []string) { | ||
427 | 26 | p := &Proxy{ | ||
428 | 27 | AgentSock: requireEnv("JUJU_AGENT_SOCKET"), | ||
429 | 28 | ClientId: requireEnv("JUJU_CLIENT_ID"), | ||
430 | 29 | Stdout: os.Stdout, | ||
431 | 30 | Stderr: os.Stderr, | ||
432 | 31 | } | ||
433 | 32 | result, err := p.Run(args) | ||
434 | 33 | if err != nil { | ||
435 | 34 | fmt.Fprintf(os.Stderr, "%s command failed: %v\n", args[0], err) | ||
436 | 35 | os.Exit(1) | ||
437 | 36 | } | ||
438 | 37 | os.Exit(result) | ||
439 | 38 | } | ||
440 | 39 | |||
441 | 40 | func main() { | ||
442 | 41 | Main(os.Args) | ||
443 | 42 | } | ||
444 | 0 | 43 | ||
445 | === added file 'cmd/proxy/proxy.go' | |||
446 | --- cmd/proxy/proxy.go 1970-01-01 00:00:00 +0000 | |||
447 | +++ cmd/proxy/proxy.go 2012-02-28 17:13:19 +0000 | |||
448 | @@ -0,0 +1,103 @@ | |||
449 | 1 | package main | ||
450 | 2 | |||
451 | 3 | import ( | ||
452 | 4 | "fmt" | ||
453 | 5 | "io" | ||
454 | 6 | "io/ioutil" | ||
455 | 7 | "launchpad.net/juju/go/cmd/server" | ||
456 | 8 | "net" | ||
457 | 9 | "os" | ||
458 | 10 | "path/filepath" | ||
459 | 11 | ) | ||
460 | 12 | |||
461 | 13 | // Proxy is responsible for asking a running Server to execute a Command, and | ||
462 | 14 | // for propagating the Command's output and return code. | ||
463 | 15 | type Proxy struct { | ||
464 | 16 | AgentSock string | ||
465 | 17 | ClientId string | ||
466 | 18 | Stdout io.Writer | ||
467 | 19 | Stderr io.Writer | ||
468 | 20 | } | ||
469 | 21 | |||
470 | 22 | // connect listens for a connection on sock, and echoes anything received to dst. | ||
471 | 23 | func connect(dst io.Writer, sock string) (<-chan bool, error) { | ||
472 | 24 | l, err := net.Listen("unix", sock) | ||
473 | 25 | if err != nil { | ||
474 | 26 | return nil, err | ||
475 | 27 | } | ||
476 | 28 | done := make(chan bool) | ||
477 | 29 | go func() { | ||
478 | 30 | defer close(done) | ||
479 | 31 | defer os.Remove(sock) | ||
480 | 32 | defer l.Close() | ||
481 | 33 | conn, err := l.Accept() | ||
482 | 34 | if err != nil { | ||
483 | 35 | return | ||
484 | 36 | } | ||
485 | 37 | defer conn.Close() | ||
486 | 38 | _, err = io.Copy(dst, conn) | ||
487 | 39 | if err != nil { | ||
488 | 40 | return | ||
489 | 41 | } | ||
490 | 42 | done <- true | ||
491 | 43 | }() | ||
492 | 44 | return done, nil | ||
493 | 45 | } | ||
494 | 46 | |||
495 | 47 | // Run connects to a Server listening on p.AgentSock, asks it to run the Command | ||
496 | 48 | // specified by args, forwards the Command's stdout/stderr to p.Stdout/p.Stderr, | ||
497 | 49 | // and returns the Command's result code. | ||
498 | 50 | func (p *Proxy) Run(args []string) (int, error) { | ||
499 | 51 | // Create tempdir for stdout/stderr sockets. | ||
500 | 52 | sockDir, err := ioutil.TempDir("", "juju-proxy") | ||
501 | 53 | if err != nil { | ||
502 | 54 | return 0, err | ||
503 | 55 | } | ||
504 | 56 | defer os.RemoveAll(sockDir) | ||
505 | 57 | |||
506 | 58 | // Pipe data from sockets into stdout/stderr. | ||
507 | 59 | outSock := filepath.Join(sockDir, "stdout.sock") | ||
508 | 60 | outDone, err := connect(p.Stdout, outSock) | ||
509 | 61 | if err != nil { | ||
510 | 62 | return 0, err | ||
511 | 63 | } | ||
512 | 64 | errSock := filepath.Join(sockDir, "stderr.sock") | ||
513 | 65 | errDone, err := connect(p.Stderr, errSock) | ||
514 | 66 | if err != nil { | ||
515 | 67 | return 0, err | ||
516 | 68 | } | ||
517 | 69 | |||
518 | 70 | // Connect to the agent socket. | ||
519 | 71 | conn, err := net.Dial("unix", p.AgentSock) | ||
520 | 72 | if err != nil { | ||
521 | 73 | return 0, err | ||
522 | 74 | } | ||
523 | 75 | defer conn.Close() | ||
524 | 76 | |||
525 | 77 | // Tell the agent to run the command specified by args (with the command | ||
526 | 78 | // name suitably trimmed) and pipe its output back to our sockets. | ||
527 | 79 | args[0] = filepath.Base(args[0]) | ||
528 | 80 | req := &server.Request{p.ClientId, args, outSock, errSock} | ||
529 | 81 | _, err = req.WriteTo(conn) | ||
530 | 82 | if err != nil { | ||
531 | 83 | return 0, err | ||
532 | 84 | } | ||
533 | 85 | |||
534 | 86 | // Extract response code from agent connection. | ||
535 | 87 | var response server.Response | ||
536 | 88 | _, err = response.ReadFrom(conn) | ||
537 | 89 | if err != nil { | ||
538 | 90 | return 0, err | ||
539 | 91 | } | ||
540 | 92 | |||
541 | 93 | // Wait for streams to close before returning. | ||
542 | 94 | _, ok := <-outDone | ||
543 | 95 | if !ok { | ||
544 | 96 | return 0, fmt.Errorf("stdout connection failed on %s", outSock) | ||
545 | 97 | } | ||
546 | 98 | _, ok = <-errDone | ||
547 | 99 | if !ok { | ||
548 | 100 | return 0, fmt.Errorf("stderr connection failed on %s", errSock) | ||
549 | 101 | } | ||
550 | 102 | return int(response), nil | ||
551 | 103 | } | ||
552 | 0 | 104 | ||
553 | === added file 'cmd/proxy/proxy_test.go' | |||
554 | --- cmd/proxy/proxy_test.go 1970-01-01 00:00:00 +0000 | |||
555 | +++ cmd/proxy/proxy_test.go 2012-02-28 17:13:19 +0000 | |||
556 | @@ -0,0 +1,136 @@ | |||
557 | 1 | package main_test | ||
558 | 2 | |||
559 | 3 | import ( | ||
560 | 4 | "bytes" | ||
561 | 5 | "fmt" | ||
562 | 6 | "launchpad.net/gnuflag" | ||
563 | 7 | . "launchpad.net/gocheck" | ||
564 | 8 | "launchpad.net/juju/go/cmd" | ||
565 | 9 | main "launchpad.net/juju/go/cmd/proxy" | ||
566 | 10 | "launchpad.net/juju/go/cmd/server" | ||
567 | 11 | "path/filepath" | ||
568 | 12 | "strings" | ||
569 | 13 | "testing" | ||
570 | 14 | ) | ||
571 | 15 | |||
572 | 16 | func Test(t *testing.T) { TestingT(t) } | ||
573 | 17 | |||
574 | 18 | type TestCommand struct { | ||
575 | 19 | Value string | ||
576 | 20 | } | ||
577 | 21 | |||
578 | 22 | func (c *TestCommand) Info() *cmd.Info { | ||
579 | 23 | return &cmd.Info{"magic", "[options]", "do magic", "blah doc", true} | ||
580 | 24 | } | ||
581 | 25 | |||
582 | 26 | func (c *TestCommand) InitFlagSet(f *gnuflag.FlagSet) { | ||
583 | 27 | f.StringVar(&c.Value, "value", "", "doc") | ||
584 | 28 | } | ||
585 | 29 | |||
586 | 30 | func (c *TestCommand) ParsePositional(e cmd.Env, args []string) error { | ||
587 | 31 | if len(args) != 0 { | ||
588 | 32 | return fmt.Errorf("BADARGS: %s", args) | ||
589 | 33 | } | ||
590 | 34 | return nil | ||
591 | 35 | } | ||
592 | 36 | |||
593 | 37 | func (c *TestCommand) Run(e cmd.Env) error { | ||
594 | 38 | if c.Value != "zyxxy" { | ||
595 | 39 | return fmt.Errorf("insufficiently magic") | ||
596 | 40 | } | ||
597 | 41 | e.Stdout().Write([]byte("eye of newt")) | ||
598 | 42 | e.Stderr().Write([]byte("toe of frog")) | ||
599 | 43 | return nil | ||
600 | 44 | } | ||
601 | 45 | |||
602 | 46 | type ProxySuite struct { | ||
603 | 47 | sock string | ||
604 | 48 | server *server.Server | ||
605 | 49 | } | ||
606 | 50 | |||
607 | 51 | var _ = Suite(&ProxySuite{}) | ||
608 | 52 | |||
609 | 53 | func (s *ProxySuite) SetUpSuite(c *C) { | ||
610 | 54 | factory := func(clientId string) (cmd.Command, error) { | ||
611 | 55 | if clientId != "jeremiah" { | ||
612 | 56 | return nil, fmt.Errorf("unknown client") | ||
613 | 57 | } | ||
614 | 58 | sc := cmd.NewSuperCommand("", "") | ||
615 | 59 | sc.Register(&TestCommand{}) | ||
616 | 60 | return sc, nil | ||
617 | 61 | } | ||
618 | 62 | s.sock = filepath.Join(c.MkDir(), "test.sock") | ||
619 | 63 | var err error | ||
620 | 64 | s.server, err = server.NewServer(factory, s.sock) | ||
621 | 65 | c.Assert(err, IsNil) | ||
622 | 66 | } | ||
623 | 67 | |||
624 | 68 | func (s *ProxySuite) TearDownSuite(c *C) { | ||
625 | 69 | c.Assert(s.server.Stop(), IsNil) | ||
626 | 70 | } | ||
627 | 71 | |||
628 | 72 | func (s *ProxySuite) AssertCall(c *C, clientId string, args []string) (int, string, string) { | ||
629 | 73 | stdout := bytes.NewBuffer([]byte{}) | ||
630 | 74 | stderr := bytes.NewBuffer([]byte{}) | ||
631 | 75 | p := &main.Proxy{ | ||
632 | 76 | AgentSock: s.sock, | ||
633 | 77 | ClientId: clientId, | ||
634 | 78 | Stdout: stdout, | ||
635 | 79 | Stderr: stderr, | ||
636 | 80 | } | ||
637 | 81 | result, err := p.Run(args) | ||
638 | 82 | c.Assert(err, IsNil) | ||
639 | 83 | return result, stdout.String(), stderr.String() | ||
640 | 84 | } | ||
641 | 85 | |||
642 | 86 | func (s *ProxySuite) TestSuccess(c *C) { | ||
643 | 87 | result, stdout, stderr := s.AssertCall( | ||
644 | 88 | c, "jeremiah", []string{"magic", "--value", "zyxxy"}) | ||
645 | 89 | c.Assert(result, Equals, 0) | ||
646 | 90 | c.Assert(stdout, Equals, "eye of newt") | ||
647 | 91 | c.Assert(stderr, Equals, "toe of frog") | ||
648 | 92 | } | ||
649 | 93 | |||
650 | 94 | func (s *ProxySuite) TestCommandPath(c *C) { | ||
651 | 95 | result, stdout, stderr := s.AssertCall( | ||
652 | 96 | c, "jeremiah", []string{"some/path/to/magic", "--value", "zyxxy"}) | ||
653 | 97 | c.Assert(result, Equals, 0) | ||
654 | 98 | c.Assert(stdout, Equals, "eye of newt") | ||
655 | 99 | c.Assert(stderr, Equals, "toe of frog") | ||
656 | 100 | } | ||
657 | 101 | |||
658 | 102 | func (s *ProxySuite) TestBadRun(c *C) { | ||
659 | 103 | result, stdout, stderr := s.AssertCall( | ||
660 | 104 | c, "jeremiah", []string{"magic", "--value", "wrong"}) | ||
661 | 105 | c.Assert(result, Equals, 1) | ||
662 | 106 | c.Assert(stdout, Equals, "") | ||
663 | 107 | errLines := strings.Split(stderr, "\n") | ||
664 | 108 | c.Assert(errLines[0], Equals, "insufficiently magic") | ||
665 | 109 | } | ||
666 | 110 | |||
667 | 111 | func (s *ProxySuite) TestBadClient(c *C) { | ||
668 | 112 | result, stdout, stderr := s.AssertCall( | ||
669 | 113 | c, "methuselah", []string{"magic"}) | ||
670 | 114 | c.Assert(result, Equals, 1) | ||
671 | 115 | c.Assert(stdout, Equals, "") | ||
672 | 116 | errLines := strings.Split(stderr, "\n") | ||
673 | 117 | c.Assert(errLines[0], Equals, "command unavailable: unknown client") | ||
674 | 118 | } | ||
675 | 119 | |||
676 | 120 | func (s *ProxySuite) TestNonsenseArgs(c *C) { | ||
677 | 121 | result, stdout, stderr := s.AssertCall( | ||
678 | 122 | c, "jeremiah", []string{"magic", "--cheese"}) | ||
679 | 123 | c.Assert(result, Equals, 2) | ||
680 | 124 | c.Assert(stdout, Equals, "") | ||
681 | 125 | errLines := strings.Split(stderr, "\n") | ||
682 | 126 | c.Assert(errLines[0], Equals, "flag provided but not defined: --cheese") | ||
683 | 127 | } | ||
684 | 128 | |||
685 | 129 | func (s *ProxySuite) TestUnknownCommand(c *C) { | ||
686 | 130 | result, stdout, stderr := s.AssertCall( | ||
687 | 131 | c, "jeremiah", []string{"witchcraft"}) | ||
688 | 132 | c.Assert(result, Equals, 2) | ||
689 | 133 | c.Assert(stdout, Equals, "") | ||
690 | 134 | errLines := strings.Split(stderr, "\n") | ||
691 | 135 | c.Assert(errLines[0], Equals, "unrecognised command: witchcraft") | ||
692 | 136 | } | ||
693 | 0 | 137 | ||
694 | === added directory 'cmd/server' | |||
695 | === added file 'cmd/server/protocol.go' | |||
696 | --- cmd/server/protocol.go 1970-01-01 00:00:00 +0000 | |||
697 | +++ cmd/server/protocol.go 2012-02-28 17:13:19 +0000 | |||
698 | @@ -0,0 +1,86 @@ | |||
699 | 1 | package server | ||
700 | 2 | |||
701 | 3 | import ( | ||
702 | 4 | "encoding/binary" | ||
703 | 5 | "fmt" | ||
704 | 6 | "io" | ||
705 | 7 | "launchpad.net/goyaml" | ||
706 | 8 | ) | ||
707 | 9 | |||
708 | 10 | // Request contains all the information necessary for a Server to run a Command | ||
709 | 11 | // on behalf of a Proxy. | ||
710 | 12 | type Request struct { | ||
711 | 13 | ClientId string | ||
712 | 14 | Args []string | ||
713 | 15 | OutSock string | ||
714 | 16 | ErrSock string | ||
715 | 17 | } | ||
716 | 18 | |||
717 | 19 | var ( | ||
718 | 20 | bo = binary.BigEndian | ||
719 | 21 | codeS = int64(binary.Size(int32(0))) | ||
720 | 22 | sizeS = int64(binary.Size(uint32(0))) | ||
721 | 23 | maxSize = uint32(1024 * 1024) | ||
722 | 24 | ) | ||
723 | 25 | |||
724 | 26 | // ReadFrom fills in a Request's fields using data read from src (which is | ||
725 | 27 | // expected in the format written by WriteTo). | ||
726 | 28 | func (r *Request) ReadFrom(src io.Reader) (count int64, err error) { | ||
727 | 29 | var size uint32 | ||
728 | 30 | err = binary.Read(src, bo, &size) | ||
729 | 31 | if err != nil { | ||
730 | 32 | return | ||
731 | 33 | } | ||
732 | 34 | count += sizeS | ||
733 | 35 | if size > maxSize { | ||
734 | 36 | err = fmt.Errorf("unreasonably large request: %d bytes", size) | ||
735 | 37 | return | ||
736 | 38 | } | ||
737 | 39 | data := make([]byte, size) | ||
738 | 40 | n, err := io.ReadFull(src, data[:]) | ||
739 | 41 | count += int64(n) | ||
740 | 42 | if err != nil { | ||
741 | 43 | return | ||
742 | 44 | } | ||
743 | 45 | err = goyaml.Unmarshal(data, r) | ||
744 | 46 | return | ||
745 | 47 | } | ||
746 | 48 | |||
747 | 49 | // WriteTo writes a Request's data to dst, in a format suitable for | ||
748 | 50 | // reconstruction by ReadFrom. | ||
749 | 51 | func (r *Request) WriteTo(dst io.Writer) (int64, error) { | ||
750 | 52 | yaml, err := goyaml.Marshal(r) | ||
751 | 53 | if err != nil { | ||
752 | 54 | return 0, err | ||
753 | 55 | } | ||
754 | 56 | bytes := []byte(yaml) | ||
755 | 57 | err = binary.Write(dst, bo, uint32(len(bytes))) | ||
756 | 58 | if err != nil { | ||
757 | 59 | return 0, err | ||
758 | 60 | } | ||
759 | 61 | n, err := dst.Write(bytes) | ||
760 | 62 | return int64(n) + sizeS, err | ||
761 | 63 | } | ||
762 | 64 | |||
763 | 65 | // Response is a command's return code, with extra methods to make it convenient | ||
764 | 66 | // to transmit and interpret. | ||
765 | 67 | type Response int32 | ||
766 | 68 | |||
767 | 69 | // ReadFrom decodes a Response from src (expected in the format written by WriteTo). | ||
768 | 70 | func (r *Response) ReadFrom(src io.Reader) (int64, error) { | ||
769 | 71 | err := binary.Read(src, bo, r) | ||
770 | 72 | if err != nil { | ||
771 | 73 | return 0, err | ||
772 | 74 | } | ||
773 | 75 | return codeS, nil | ||
774 | 76 | } | ||
775 | 77 | |||
776 | 78 | // WriteTo encodes a Response to dst, in a format suitable for reconstruction by | ||
777 | 79 | // ReadFrom. | ||
778 | 80 | func (r *Response) WriteTo(dst io.Writer) (int64, error) { | ||
779 | 81 | err := binary.Write(dst, bo, r) | ||
780 | 82 | if err != nil { | ||
781 | 83 | return 0, err | ||
782 | 84 | } | ||
783 | 85 | return codeS, nil | ||
784 | 86 | } | ||
785 | 0 | 87 | ||
786 | === added file 'cmd/server/protocol_test.go' | |||
787 | --- cmd/server/protocol_test.go 1970-01-01 00:00:00 +0000 | |||
788 | +++ cmd/server/protocol_test.go 2012-02-28 17:13:19 +0000 | |||
789 | @@ -0,0 +1,38 @@ | |||
790 | 1 | package server_test | ||
791 | 2 | |||
792 | 3 | import ( | ||
793 | 4 | "bytes" | ||
794 | 5 | . "launchpad.net/gocheck" | ||
795 | 6 | "launchpad.net/juju/go/cmd/server" | ||
796 | 7 | ) | ||
797 | 8 | |||
798 | 9 | type ProtocolSuite struct{} | ||
799 | 10 | |||
800 | 11 | var _ = Suite(&ProtocolSuite{}) | ||
801 | 12 | |||
802 | 13 | func (s *ProtocolSuite) TestWriteReadRequest(c *C) { | ||
803 | 14 | buf := bytes.NewBuffer([]byte{}) | ||
804 | 15 | req0 := &server.Request{"client", []string{"command", "arg"}, "out", "err"} | ||
805 | 16 | count0, err := req0.WriteTo(buf) | ||
806 | 17 | c.Assert(err, IsNil) | ||
807 | 18 | var req1 server.Request | ||
808 | 19 | count1, err := req1.ReadFrom(buf) | ||
809 | 20 | c.Assert(err, IsNil) | ||
810 | 21 | c.Assert(count1, Equals, count0) | ||
811 | 22 | c.Assert(req1.ClientId, Equals, "client") | ||
812 | 23 | c.Assert(req1.Args, DeepEquals, []string{"command", "arg"}) | ||
813 | 24 | c.Assert(req1.OutSock, Equals, "out") | ||
814 | 25 | c.Assert(req1.ErrSock, Equals, "err") | ||
815 | 26 | } | ||
816 | 27 | |||
817 | 28 | func (s *ProtocolSuite) TestWriteReadResponse(c *C) { | ||
818 | 29 | buf := bytes.NewBuffer([]byte{}) | ||
819 | 30 | resp0 := server.Response(-123) | ||
820 | 31 | count0, err := resp0.WriteTo(buf) | ||
821 | 32 | c.Assert(err, IsNil) | ||
822 | 33 | var resp1 server.Response | ||
823 | 34 | count1, err := resp1.ReadFrom(buf) | ||
824 | 35 | c.Assert(err, IsNil) | ||
825 | 36 | c.Assert(count1, Equals, count0) | ||
826 | 37 | c.Assert(resp1, Equals, resp0) | ||
827 | 38 | } | ||
828 | 0 | 39 | ||
829 | === added file 'cmd/server/server.go' | |||
830 | --- cmd/server/server.go 1970-01-01 00:00:00 +0000 | |||
831 | +++ cmd/server/server.go 2012-02-28 17:13:19 +0000 | |||
832 | @@ -0,0 +1,149 @@ | |||
833 | 1 | package server | ||
834 | 2 | |||
835 | 3 | import ( | ||
836 | 4 | "fmt" | ||
837 | 5 | "io" | ||
838 | 6 | "launchpad.net/juju/go/cmd" | ||
839 | 7 | "launchpad.net/tomb" | ||
840 | 8 | "net" | ||
841 | 9 | ) | ||
842 | 10 | |||
843 | 11 | // clientEnv implements cmd.Env so as to act for the Proxy which communicated a | ||
844 | 12 | // Request to a Server. | ||
845 | 13 | type clientEnv struct { | ||
846 | 14 | stdout io.WriteCloser | ||
847 | 15 | stderr io.WriteCloser | ||
848 | 16 | ret io.WriteCloser | ||
849 | 17 | } | ||
850 | 18 | |||
851 | 19 | // newClientEnv returns a clientEnv whose stdout/stderr are connected to the | ||
852 | 20 | // sockets specified in req, and whose Exit() will write the return code to conn. | ||
853 | 21 | func newClientEnv(conn net.Conn, req *Request) (*clientEnv, error) { | ||
854 | 22 | stdout, err := net.Dial("unix", req.OutSock) | ||
855 | 23 | if err != nil { | ||
856 | 24 | return nil, err | ||
857 | 25 | } | ||
858 | 26 | stderr, err := net.Dial("unix", req.ErrSock) | ||
859 | 27 | if err != nil { | ||
860 | 28 | return nil, err | ||
861 | 29 | } | ||
862 | 30 | return &clientEnv{stdout, stderr, conn}, nil | ||
863 | 31 | } | ||
864 | 32 | |||
865 | 33 | func (e *clientEnv) Stdout() io.Writer { | ||
866 | 34 | return e.stdout | ||
867 | 35 | } | ||
868 | 36 | |||
869 | 37 | func (e *clientEnv) Stderr() io.Writer { | ||
870 | 38 | return e.stderr | ||
871 | 39 | } | ||
872 | 40 | |||
873 | 41 | func (e *clientEnv) Exit(code int) { | ||
874 | 42 | response := Response(code) | ||
875 | 43 | response.WriteTo(e.ret) | ||
876 | 44 | e.ret.Close() | ||
877 | 45 | e.stdout.Close() | ||
878 | 46 | e.stderr.Close() | ||
879 | 47 | } | ||
880 | 48 | |||
881 | 49 | func (e *clientEnv) InitOutput(verbose bool, debug bool, logfile string) error { | ||
882 | 50 | // TODO move away from global logging to enable this to do something | ||
883 | 51 | // helpful? (We don't want to override this process's logging setup with | ||
884 | 52 | // the proxy command's, but I think it's still worth exposing the flags). | ||
885 | 53 | return nil | ||
886 | 54 | } | ||
887 | 55 | |||
888 | 56 | // CmdFactory returns an appropriate Command for the context specified by | ||
889 | 57 | // clientId. | ||
890 | 58 | type CmdFactory func(clientId string) (cmd.Command, error) | ||
891 | 59 | |||
892 | 60 | // Server is a bare-bones "RPC" ("RCC"?) server which accepts serialized Request | ||
893 | 61 | // structs and runs the specified Command, piping its output to the sockets | ||
894 | 62 | // specified in the Request and writing its return code back to the original | ||
895 | 63 | // connection. | ||
896 | 64 | type Server struct { | ||
897 | 65 | cmdFactory CmdFactory | ||
898 | 66 | listener net.Listener | ||
899 | 67 | reqs chan bool | ||
900 | 68 | *tomb.Tomb | ||
901 | 69 | } | ||
902 | 70 | |||
903 | 71 | var ( | ||
904 | 72 | maxReqs = 4 | ||
905 | 73 | ) | ||
906 | 74 | |||
907 | 75 | // NewServer starts a Server listening on sock and exposing the Command | ||
908 | 76 | // returned by f. | ||
909 | 77 | func NewServer(f CmdFactory, sock string) (*Server, error) { | ||
910 | 78 | s := &Server{f, nil, make(chan bool, maxReqs), tomb.New()} | ||
911 | 79 | if err := s.serve(sock); err != nil { | ||
912 | 80 | return nil, err | ||
913 | 81 | } | ||
914 | 82 | return s, nil | ||
915 | 83 | } | ||
916 | 84 | |||
917 | 85 | // serve starts listening for Requests sent on sock. | ||
918 | 86 | func (s *Server) serve(sock string) error { | ||
919 | 87 | var err error | ||
920 | 88 | s.listener, err = net.Listen("unix", sock) | ||
921 | 89 | if err != nil { | ||
922 | 90 | return err | ||
923 | 91 | } | ||
924 | 92 | go func() { | ||
925 | 93 | <-s.Dying | ||
926 | 94 | s.listener.Close() | ||
927 | 95 | for i := 0; i < maxReqs; i++ { | ||
928 | 96 | s.reqs <- true | ||
929 | 97 | } | ||
930 | 98 | s.Done() | ||
931 | 99 | }() | ||
932 | 100 | go func() { | ||
933 | 101 | for { | ||
934 | 102 | s.reqs <- true | ||
935 | 103 | conn, err := s.listener.Accept() | ||
936 | 104 | if err != nil { | ||
937 | 105 | // if err *is* tomb.Stop, it implies we Stop~ed the listener; | ||
938 | 106 | // and that the error is an expected connection-lost complaint, | ||
939 | 107 | // which we should quietly suppress. I feel that this bit could | ||
940 | 108 | // maybe be done better..? | ||
941 | 109 | if s.Err() != tomb.Stop { | ||
942 | 110 | s.Fatal(err) | ||
943 | 111 | } | ||
944 | 112 | <-s.reqs | ||
945 | 113 | return | ||
946 | 114 | } | ||
947 | 115 | go s.handle(conn) | ||
948 | 116 | } | ||
949 | 117 | }() | ||
950 | 118 | return nil | ||
951 | 119 | } | ||
952 | 120 | |||
953 | 121 | // handle reads and handles a single Request from conn. | ||
954 | 122 | func (s *Server) handle(conn net.Conn) { | ||
955 | 123 | defer conn.Close() | ||
956 | 124 | defer func() { <-s.reqs }() | ||
957 | 125 | |||
958 | 126 | req := &Request{} | ||
959 | 127 | _, err := req.ReadFrom(conn) | ||
960 | 128 | if err != nil { | ||
961 | 129 | return | ||
962 | 130 | } | ||
963 | 131 | env, err := newClientEnv(conn, req) | ||
964 | 132 | if err != nil { | ||
965 | 133 | return | ||
966 | 134 | } | ||
967 | 135 | c, err := s.cmdFactory(req.ClientId) | ||
968 | 136 | if err != nil { | ||
969 | 137 | fmt.Fprintf(env.Stderr(), "command unavailable: %s", err) | ||
970 | 138 | env.Exit(1) | ||
971 | 139 | return | ||
972 | 140 | } | ||
973 | 141 | cmd.Main(env, c, req.Args) | ||
974 | 142 | } | ||
975 | 143 | |||
976 | 144 | // Stop stops the server from accepting new requests, and returns once all | ||
977 | 145 | // current requests have completed. | ||
978 | 146 | func (s *Server) Stop() error { | ||
979 | 147 | s.Fatal(tomb.Stop) | ||
980 | 148 | return s.Wait() | ||
981 | 149 | } | ||
982 | 0 | 150 | ||
983 | === added file 'cmd/server/server_test.go' | |||
984 | --- cmd/server/server_test.go 1970-01-01 00:00:00 +0000 | |||
985 | +++ cmd/server/server_test.go 2012-02-28 17:13:19 +0000 | |||
986 | @@ -0,0 +1,205 @@ | |||
987 | 1 | package server_test | ||
988 | 2 | |||
989 | 3 | import ( | ||
990 | 4 | "bytes" | ||
991 | 5 | "fmt" | ||
992 | 6 | "io" | ||
993 | 7 | "launchpad.net/gnuflag" | ||
994 | 8 | . "launchpad.net/gocheck" | ||
995 | 9 | "launchpad.net/juju/go/cmd" | ||
996 | 10 | proxy "launchpad.net/juju/go/cmd/proxy" | ||
997 | 11 | "launchpad.net/juju/go/cmd/server" | ||
998 | 12 | "path/filepath" | ||
999 | 13 | "strings" | ||
1000 | 14 | "testing" | ||
1001 | 15 | ) | ||
1002 | 16 | |||
1003 | 17 | func Test(t *testing.T) { TestingT(t) } | ||
1004 | 18 | |||
1005 | 19 | type TestCommand struct { | ||
1006 | 20 | Value string | ||
1007 | 21 | } | ||
1008 | 22 | |||
1009 | 23 | func (c *TestCommand) Info() *cmd.Info { | ||
1010 | 24 | return &cmd.Info{"magic", "[options]", "do magic", "blah doc", true} | ||
1011 | 25 | } | ||
1012 | 26 | |||
1013 | 27 | func (c *TestCommand) InitFlagSet(f *gnuflag.FlagSet) { | ||
1014 | 28 | f.StringVar(&c.Value, "value", "", "doc") | ||
1015 | 29 | } | ||
1016 | 30 | |||
1017 | 31 | func (c *TestCommand) ParsePositional(e cmd.Env, args []string) error { | ||
1018 | 32 | return cmd.CheckEmpty(args) | ||
1019 | 33 | } | ||
1020 | 34 | |||
1021 | 35 | func (c *TestCommand) Run(e cmd.Env) error { | ||
1022 | 36 | if c.Value != "zyxxy" { | ||
1023 | 37 | return fmt.Errorf("insufficiently magic") | ||
1024 | 38 | } | ||
1025 | 39 | e.Stdout().Write([]byte("eye of newt")) | ||
1026 | 40 | e.Stderr().Write([]byte("toe of frog")) | ||
1027 | 41 | return nil | ||
1028 | 42 | } | ||
1029 | 43 | |||
1030 | 44 | type WaitCommand struct { | ||
1031 | 45 | unblock chan bool | ||
1032 | 46 | } | ||
1033 | 47 | |||
1034 | 48 | func (c *WaitCommand) Info() *cmd.Info { | ||
1035 | 49 | return &cmd.Info{"wait", "", "wait until unblocked", "blah doc", true} | ||
1036 | 50 | } | ||
1037 | 51 | |||
1038 | 52 | func (c *WaitCommand) InitFlagSet(f *gnuflag.FlagSet) {} | ||
1039 | 53 | |||
1040 | 54 | func (c *WaitCommand) ParsePositional(e cmd.Env, args []string) error { | ||
1041 | 55 | return cmd.CheckEmpty(args) | ||
1042 | 56 | } | ||
1043 | 57 | |||
1044 | 58 | func (c *WaitCommand) Run(e cmd.Env) error { | ||
1045 | 59 | fmt.Fprintf(e.Stderr(), "waiting...\n") | ||
1046 | 60 | _, ok := <-c.unblock | ||
1047 | 61 | fmt.Fprintf(e.Stderr(), "unblocked; ok = %s\n", ok) | ||
1048 | 62 | if !ok { | ||
1049 | 63 | return fmt.Errorf("oh no, command failed") | ||
1050 | 64 | } | ||
1051 | 65 | return nil | ||
1052 | 66 | } | ||
1053 | 67 | |||
1054 | 68 | type ServerSuite struct{} | ||
1055 | 69 | |||
1056 | 70 | var _ = Suite(&ServerSuite{}) | ||
1057 | 71 | |||
1058 | 72 | func startServer(c *C, register func(*cmd.SuperCommand)) (string, *server.Server) { | ||
1059 | 73 | factory := func(clientId string) (cmd.Command, error) { | ||
1060 | 74 | if clientId != "jeremiah" { | ||
1061 | 75 | return nil, fmt.Errorf("unknown client") | ||
1062 | 76 | } | ||
1063 | 77 | sc := cmd.NewSuperCommand("", "") | ||
1064 | 78 | register(sc) | ||
1065 | 79 | return sc, nil | ||
1066 | 80 | } | ||
1067 | 81 | sock := filepath.Join(c.MkDir(), "test.sock") | ||
1068 | 82 | server, err := server.NewServer(factory, sock) | ||
1069 | 83 | c.Assert(err, IsNil) | ||
1070 | 84 | return sock, server | ||
1071 | 85 | } | ||
1072 | 86 | |||
1073 | 87 | func newProxy(sock string, stdout io.Writer, stderr io.Writer) *proxy.Proxy { | ||
1074 | 88 | if stdout == nil { | ||
1075 | 89 | stdout = bytes.NewBuffer([]byte{}) | ||
1076 | 90 | } | ||
1077 | 91 | if stderr == nil { | ||
1078 | 92 | stderr = bytes.NewBuffer([]byte{}) | ||
1079 | 93 | } | ||
1080 | 94 | return &proxy.Proxy{ | ||
1081 | 95 | AgentSock: sock, | ||
1082 | 96 | ClientId: "jeremiah", | ||
1083 | 97 | Stdout: stdout, | ||
1084 | 98 | Stderr: stderr, | ||
1085 | 99 | } | ||
1086 | 100 | } | ||
1087 | 101 | |||
1088 | 102 | func (s *ServerSuite) TestStartStop(c *C) { | ||
1089 | 103 | sock, server := startServer(c, func(sc *cmd.SuperCommand) { | ||
1090 | 104 | sc.Register(&TestCommand{}) | ||
1091 | 105 | }) | ||
1092 | 106 | |||
1093 | 107 | stdout := bytes.NewBuffer([]byte{}) | ||
1094 | 108 | stderr := bytes.NewBuffer([]byte{}) | ||
1095 | 109 | p := newProxy(sock, stdout, stderr) | ||
1096 | 110 | args := []string{"magic", "--value", "zyxxy"} | ||
1097 | 111 | result, err := p.Run(args) | ||
1098 | 112 | c.Assert(err, IsNil) | ||
1099 | 113 | c.Assert(result, Equals, 0) | ||
1100 | 114 | c.Assert(stdout.String(), Equals, "eye of newt") | ||
1101 | 115 | c.Assert(stderr.String(), Equals, "toe of frog") | ||
1102 | 116 | |||
1103 | 117 | server.Stop() | ||
1104 | 118 | stdout.Reset() | ||
1105 | 119 | stderr.Reset() | ||
1106 | 120 | _, err = p.Run(args) | ||
1107 | 121 | c.Assert(err, NotNil) | ||
1108 | 122 | } | ||
1109 | 123 | |||
1110 | 124 | func (s *ServerSuite) TestConcurrentStop(c *C) { | ||
1111 | 125 | concurrency := 4 | ||
1112 | 126 | unblockOne := make(chan bool) | ||
1113 | 127 | gotOneRequest := make(chan bool) | ||
1114 | 128 | sock, server := startServer(c, func(sc *cmd.SuperCommand) { | ||
1115 | 129 | sc.Register(&WaitCommand{unblockOne}) | ||
1116 | 130 | gotOneRequest <- true | ||
1117 | 131 | }) | ||
1118 | 132 | |||
1119 | 133 | args := []string{"wait"} | ||
1120 | 134 | oneRunComplete := make(chan bool) | ||
1121 | 135 | for i := 0; i < concurrency; i++ { | ||
1122 | 136 | go func() { | ||
1123 | 137 | result, err := newProxy(sock, nil, nil).Run(args) | ||
1124 | 138 | c.Assert(err, IsNil) | ||
1125 | 139 | c.Assert(result, Equals, 0) | ||
1126 | 140 | oneRunComplete <- true | ||
1127 | 141 | }() | ||
1128 | 142 | <-gotOneRequest | ||
1129 | 143 | } | ||
1130 | 144 | |||
1131 | 145 | stopComplete := make(chan bool) | ||
1132 | 146 | go func() { | ||
1133 | 147 | err := server.Stop() | ||
1134 | 148 | c.Assert(err, IsNil) | ||
1135 | 149 | stopComplete <- true | ||
1136 | 150 | }() | ||
1137 | 151 | |||
1138 | 152 | for i := 0; i < concurrency; i++ { | ||
1139 | 153 | select { | ||
1140 | 154 | case <-server.Dying: | ||
1141 | 155 | // Jolly good; check it's not accepting new requests. | ||
1142 | 156 | _, err := newProxy(sock, nil, nil).Run(args) | ||
1143 | 157 | c.Assert(err, NotNil) | ||
1144 | 158 | case <-server.Dead: | ||
1145 | 159 | c.Fatal("server did not finish processing requests") | ||
1146 | 160 | } | ||
1147 | 161 | |||
1148 | 162 | unblockOne <- true | ||
1149 | 163 | <-oneRunComplete | ||
1150 | 164 | } | ||
1151 | 165 | <-server.Dead | ||
1152 | 166 | <-stopComplete | ||
1153 | 167 | } | ||
1154 | 168 | |||
1155 | 169 | func (s *ServerSuite) TestIgnoresHugeRequests(c *C) { | ||
1156 | 170 | sock, server := startServer(c, func(sc *cmd.SuperCommand) { | ||
1157 | 171 | c.Fatal("should never call this") | ||
1158 | 172 | }) | ||
1159 | 173 | defer server.Stop() | ||
1160 | 174 | |||
1161 | 175 | p := newProxy(sock, nil, nil) | ||
1162 | 176 | stupidArgs := [1e6]string{} | ||
1163 | 177 | _, err := p.Run(stupidArgs[:]) | ||
1164 | 178 | c.Assert(err, NotNil) | ||
1165 | 179 | } | ||
1166 | 180 | |||
1167 | 181 | func (s *ServerSuite) TestBadCommands(c *C) { | ||
1168 | 182 | sock, server := startServer(c, func(sc *cmd.SuperCommand) { | ||
1169 | 183 | sc.Register(&TestCommand{}) | ||
1170 | 184 | }) | ||
1171 | 185 | defer server.Stop() | ||
1172 | 186 | |||
1173 | 187 | assertBadCmd := func(clientId string, args []string, expect int, stderr0 string) { | ||
1174 | 188 | stderr := bytes.NewBuffer([]byte{}) | ||
1175 | 189 | p := newProxy(sock, nil, stderr) | ||
1176 | 190 | p.ClientId = clientId | ||
1177 | 191 | result, err := p.Run(args) | ||
1178 | 192 | c.Assert(err, IsNil) | ||
1179 | 193 | c.Assert(result, Equals, expect) | ||
1180 | 194 | lines := strings.Split(stderr.String(), "\n") | ||
1181 | 195 | c.Assert(lines[0], Equals, stderr0) | ||
1182 | 196 | } | ||
1183 | 197 | assertBadCmd( | ||
1184 | 198 | "jeremiah", []string{"witchcraft"}, 2, "unrecognised command: witchcraft") | ||
1185 | 199 | assertBadCmd( | ||
1186 | 200 | "jeremiah", []string{"magic", "--ham"}, 2, "flag provided but not defined: --ham") | ||
1187 | 201 | assertBadCmd( | ||
1188 | 202 | "jeremiah", []string{"magic", "--value", "wrong"}, 1, "insufficiently magic") | ||
1189 | 203 | assertBadCmd( | ||
1190 | 204 | "jeff", []string{"magic", "--value", "zyxxy"}, 1, "command unavailable: unknown client") | ||
1191 | 205 | } | ||
1192 | 0 | 206 | ||
1193 | === modified file 'cmd/supercommand.go' | |||
1194 | --- cmd/supercommand.go 2012-02-17 08:03:35 +0000 | |||
1195 | +++ cmd/supercommand.go 2012-02-28 17:13:19 +0000 | |||
1196 | @@ -2,11 +2,7 @@ | |||
1197 | 2 | 2 | ||
1198 | 3 | import ( | 3 | import ( |
1199 | 4 | "fmt" | 4 | "fmt" |
1200 | 5 | "io" | ||
1201 | 6 | "launchpad.net/gnuflag" | 5 | "launchpad.net/gnuflag" |
1202 | 7 | "launchpad.net/juju/go/log" | ||
1203 | 8 | stdlog "log" | ||
1204 | 9 | "os" | ||
1205 | 10 | "sort" | 6 | "sort" |
1206 | 11 | "strings" | 7 | "strings" |
1207 | 12 | ) | 8 | ) |
1208 | @@ -72,7 +68,9 @@ | |||
1209 | 72 | var info *Info | 68 | var info *Info |
1210 | 73 | if c.subcmd != nil { | 69 | if c.subcmd != nil { |
1211 | 74 | info = c.subcmd.Info() | 70 | info = c.subcmd.Info() |
1213 | 75 | info.Name = fmt.Sprintf("%s %s", c.Name, info.Name) | 71 | if c.Name != "" { |
1214 | 72 | info.Name = fmt.Sprintf("%s %s", c.Name, info.Name) | ||
1215 | 73 | } | ||
1216 | 76 | return info | 74 | return info |
1217 | 77 | } | 75 | } |
1218 | 78 | return &Info{ | 76 | return &Info{ |
1219 | @@ -89,7 +87,7 @@ | |||
1220 | 89 | if c.subcmd != nil { | 87 | if c.subcmd != nil { |
1221 | 90 | c.subcmd.InitFlagSet(f) | 88 | c.subcmd.InitFlagSet(f) |
1222 | 91 | } | 89 | } |
1224 | 92 | // SuperCommand's flags are always added to subcommands./ Note that the | 90 | // SuperCommand's flags are always added to subcommands'. Note that the |
1225 | 93 | // flag defaults come from the SuperCommand itself, so that ParsePositional | 91 | // flag defaults come from the SuperCommand itself, so that ParsePositional |
1226 | 94 | // can call Parse twice on the same SuperCommand without losing information. | 92 | // can call Parse twice on the same SuperCommand without losing information. |
1227 | 95 | f.StringVar(&c.LogFile, "log-file", c.LogFile, "path to write log to") | 93 | f.StringVar(&c.LogFile, "log-file", c.LogFile, "path to write log to") |
1228 | @@ -101,9 +99,9 @@ | |||
1229 | 101 | 99 | ||
1230 | 102 | // ParsePositional selects the subcommand specified by subargs and uses it to | 100 | // ParsePositional selects the subcommand specified by subargs and uses it to |
1231 | 103 | // Parse any remaining unconsumed command-line arguments. | 101 | // Parse any remaining unconsumed command-line arguments. |
1233 | 104 | func (c *SuperCommand) ParsePositional(subargs []string) error { | 102 | func (c *SuperCommand) ParsePositional(e Env, subargs []string) error { |
1234 | 105 | if c.subcmd != nil { | 103 | if c.subcmd != nil { |
1236 | 106 | return c.subcmd.ParsePositional(subargs) | 104 | return c.subcmd.ParsePositional(e, subargs) |
1237 | 107 | } | 105 | } |
1238 | 108 | if len(subargs) == 0 { | 106 | if len(subargs) == 0 { |
1239 | 109 | return fmt.Errorf("no command specified") | 107 | return fmt.Errorf("no command specified") |
1240 | @@ -112,38 +110,16 @@ | |||
1241 | 112 | if c.subcmd, found = c.subcmds[subargs[0]]; !found { | 110 | if c.subcmd, found = c.subcmds[subargs[0]]; !found { |
1242 | 113 | return fmt.Errorf("unrecognised command: %s", subargs[0]) | 111 | return fmt.Errorf("unrecognised command: %s", subargs[0]) |
1243 | 114 | } | 112 | } |
1267 | 115 | return Parse(c, subargs[1:]) | 113 | return Parse(e, c, subargs[1:]) |
1245 | 116 | } | ||
1246 | 117 | |||
1247 | 118 | // initOutput sets up logging to a file or to stderr depending on what's been | ||
1248 | 119 | // requested on the command line. | ||
1249 | 120 | func (c *SuperCommand) initOutput() error { | ||
1250 | 121 | if c.Debug { | ||
1251 | 122 | log.Debug = true | ||
1252 | 123 | } | ||
1253 | 124 | var target io.Writer | ||
1254 | 125 | if c.LogFile != "" { | ||
1255 | 126 | var err error | ||
1256 | 127 | target, err = os.OpenFile(c.LogFile, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644) | ||
1257 | 128 | if err != nil { | ||
1258 | 129 | return err | ||
1259 | 130 | } | ||
1260 | 131 | } else if c.Verbose || c.Debug { | ||
1261 | 132 | target = os.Stderr | ||
1262 | 133 | } | ||
1263 | 134 | if target != nil { | ||
1264 | 135 | log.Target = stdlog.New(target, "", stdlog.LstdFlags) | ||
1265 | 136 | } | ||
1266 | 137 | return nil | ||
1268 | 138 | } | 114 | } |
1269 | 139 | 115 | ||
1270 | 140 | // Run executes the subcommand that was selected when Parse was called. | 116 | // Run executes the subcommand that was selected when Parse was called. |
1273 | 141 | func (c *SuperCommand) Run() error { | 117 | func (c *SuperCommand) Run(e Env) error { |
1274 | 142 | if err := c.initOutput(); err != nil { | 118 | if err := e.InitOutput(c.Verbose, c.Debug, c.LogFile); err != nil { |
1275 | 143 | return err | 119 | return err |
1276 | 144 | } | 120 | } |
1277 | 145 | if c.subcmd == nil { | 121 | if c.subcmd == nil { |
1278 | 146 | panic("Run: missing subcommand; Parse failed or not called") | 122 | panic("Run: missing subcommand; Parse failed or not called") |
1279 | 147 | } | 123 | } |
1281 | 148 | return c.subcmd.Run() | 124 | return c.subcmd.Run(e) |
1282 | 149 | } | 125 | } |
1283 | 150 | 126 | ||
1284 | === modified file 'cmd/supercommand_test.go' | |||
1285 | --- cmd/supercommand_test.go 2012-02-09 11:35:24 +0000 | |||
1286 | +++ cmd/supercommand_test.go 2012-02-28 17:13:19 +0000 | |||
1287 | @@ -31,20 +31,20 @@ | |||
1288 | 31 | f.StringVar(&c.Value, "value", "", "doc") | 31 | f.StringVar(&c.Value, "value", "", "doc") |
1289 | 32 | } | 32 | } |
1290 | 33 | 33 | ||
1292 | 34 | func (c *TestCommand) ParsePositional(args []string) error { | 34 | func (c *TestCommand) ParsePositional(e cmd.Env, args []string) error { |
1293 | 35 | if len(args) != 0 { | 35 | if len(args) != 0 { |
1294 | 36 | return fmt.Errorf("BADARGS: %s", args) | 36 | return fmt.Errorf("BADARGS: %s", args) |
1295 | 37 | } | 37 | } |
1296 | 38 | return nil | 38 | return nil |
1297 | 39 | } | 39 | } |
1298 | 40 | 40 | ||
1300 | 41 | func (c *TestCommand) Run() error { | 41 | func (c *TestCommand) Run(e cmd.Env) error { |
1301 | 42 | return fmt.Errorf("BORKEN: value is %s.", c.Value) | 42 | return fmt.Errorf("BORKEN: value is %s.", c.Value) |
1302 | 43 | } | 43 | } |
1303 | 44 | 44 | ||
1304 | 45 | func parseEmpty(args []string) (*cmd.SuperCommand, error) { | 45 | func parseEmpty(args []string) (*cmd.SuperCommand, error) { |
1305 | 46 | jc := cmd.NewSuperCommand("jujutest", "") | 46 | jc := cmd.NewSuperCommand("jujutest", "") |
1307 | 47 | err := cmd.Parse(jc, args) | 47 | err := cmd.Parse(&cmd.DefaultEnv{}, jc, args) |
1308 | 48 | return jc, err | 48 | return jc, err |
1309 | 49 | } | 49 | } |
1310 | 50 | 50 | ||
1311 | @@ -52,7 +52,7 @@ | |||
1312 | 52 | jc := cmd.NewSuperCommand("jujutest", "") | 52 | jc := cmd.NewSuperCommand("jujutest", "") |
1313 | 53 | tc := &TestCommand{Name: "defenestrate"} | 53 | tc := &TestCommand{Name: "defenestrate"} |
1314 | 54 | jc.Register(tc) | 54 | jc.Register(tc) |
1316 | 55 | err := cmd.Parse(jc, args) | 55 | err := cmd.Parse(&cmd.DefaultEnv{}, jc, args) |
1317 | 56 | return jc, tc, err | 56 | return jc, tc, err |
1318 | 57 | } | 57 | } |
1319 | 58 | 58 | ||
1320 | @@ -161,7 +161,7 @@ | |||
1321 | 161 | jc, _, err := parseDefenestrate(args) | 161 | jc, _, err := parseDefenestrate(args) |
1322 | 162 | c.Assert(err, IsNil) | 162 | c.Assert(err, IsNil) |
1323 | 163 | 163 | ||
1325 | 164 | err = jc.Run() | 164 | err = jc.Run(&cmd.DefaultEnv{}) |
1326 | 165 | c.Assert(err, ErrorMatches, "BORKEN: value is cheese.") | 165 | c.Assert(err, ErrorMatches, "BORKEN: value is cheese.") |
1327 | 166 | 166 | ||
1328 | 167 | c.Assert(log.Debug, Equals, debug) | 167 | c.Assert(log.Debug, Equals, debug) |
Can this be broken down in smaller chunks?
The past changes have all been similarly large in scope. I thought they
might be hard to break down, but I'm getting more skeptical that every
single change needs to touch 20 files. Reviewing, applying comments, and
getting them through the pipeline in general is a lot more traumatic
that way.
https:/ /codereview. appspot. com/5695082/