Definitely do the formatters, but you might want to hold off on hitting
the API too hard until nate's had a chance to comment. Ping me if
anything makes little sense.


https://codereview.appspot.com/99670044/diff/20001/cmd/juju/ensureavailability.go
File cmd/juju/ensureavailability.go (right):

https://codereview.appspot.com/99670044/diff/20001/cmd/juju/ensureavailability.go#newcode91
cmd/juju/ensureavailability.go:91: }
I feel like this ought to be controlled by formatters with a cmd.Out --
so you can use a default "--format simple" to get output like the above,
and json/yaml for the use of machines. See
https://codereview.appspot.com/92610043/ for a bit more discussion
around this topic.

https://codereview.appspot.com/99670044/diff/20001/cmd/juju/ensureavailability.go#newcode97
cmd/juju/ensureavailability.go:97: ln := len(machines)
I'd prefer "count" to "ln".

https://codereview.appspot.com/99670044/diff/20001/cmd/juju/ensureavailability.go#newcode104
cmd/juju/ensureavailability.go:104: result += fmt.Sprintf("\"%s\"",
machineId)
%q?

https://codereview.appspot.com/99670044/diff/20001/cmd/juju/ensureavailability.go#newcode107
cmd/juju/ensureavailability.go:107: }
strings.Join()?

https://codereview.appspot.com/99670044/diff/20001/state/api/params/params.go
File state/api/params/params.go (right):

https://codereview.appspot.com/99670044/diff/20001/state/api/params/params.go#newcode738
state/api/params/params.go:738: type EnsureAvailabilityResult struct {
Surely, this is not an anythingResult? It's a StateServerChange, or
something, isn't it?

While you're at it, would you change the name of EnsureAvailability
please? That's a verb, not a noun; I'm drawing a bit of a blank on a
better name at the moment, but things like StateServerRequest and
StateServersSpec are floating around my head.

Regardless, I'd expect us to be accepting/returning arrays of same; and
the structures in play to look something like:

type StateServerChange struct { ... }
type StateServerChangeResult struct {
     Result *StateServerChangeResult
     Error  *Error
}
type StateServerChangeResults struct {
     Results []StateServerChangeResult
}

https://codereview.appspot.com/99670044/diff/20001/state/apiserver/client/client.go
File state/apiserver/client/client.go (right):

https://codereview.appspot.com/99670044/diff/20001/state/apiserver/client/client.go#newcode1085
state/apiserver/client/client.go:1085: func
ensureAvailabilityResultFromSSI(SSIBefore, SSIAfter
*state.StateServerInfo) params.EnsureAvailabilityResult {
it's jarring to have vars with uppercase at the beginning. ssiBefore,
ssiAfter?

https://codereview.appspot.com/99670044/diff/20001/state/apiserver/client/client.go#newcode1128
state/apiserver/client/client.go:1128: return result
I feel we could probably do this more easily with some set.Strings~s?

https://codereview.appspot.com/99670044/diff/20001/state/apiserver/client/client.go#newcode1132
state/apiserver/client/client.go:1132: func (c *Client)
EnsureAvailability(args params.EnsureAvailability)
(params.EnsureAvailabilityResult, error) {
Grar. Nate, have we yet released a stable with this API? It ought to be
both accepting and returning bulk args.

(note: I'm easy on how we indicate "the current environment", I'd prefer
a special magic string that can't be confused for an environ tag, but I
think we still want to specify it)

https://codereview.appspot.com/99670044/diff/20001/state/apiserver/client/client_test.go
File state/apiserver/client/client_test.go (right):

https://codereview.appspot.com/99670044/diff/20001/state/apiserver/client/client_test.go#newcode2388
state/apiserver/client/client_test.go:2388: var
ensureAvailabilityResultTests = []ensureAvailabilityResultTest{
If these are used in just one method, please put them inside rather than
making them global. Given the opportunity, people can and will abuse
even these unassuming test0data sorts of globals :(.

https://codereview.appspot.com/99670044/diff/20001/state/state.go
File state/state.go (right):

https://codereview.appspot.com/99670044/diff/20001/state/state.go#newcode385
state/state.go:385: func MachineIdLessThan(id1, id2 string) bool {
If you're exporting this, I think it would probably live most happily in
the names package.

https://codereview.appspot.com/99670044/