Migrate add-machine to api

Migrate add-machine to api

add-machine command is migrated to api.
This command is really dual personality - it can 
add a machine to be provisioned by a cloud provider
or it can add an exisitng machine using ssh. The first
case is fully migrated. The second case has been
refactored to move all the direct state/mgo connection
stuff into the manual provisioning code. A more
substantial refactoring is needed to fully migrate the
manual provisioning case.

https://code.launchpad.net/~wallyworld/juju-core/cli-api-addmachine/+merge/194259

(do not edit description out of merge proposal)

[wallyworld, 2013-11-06 23:57:14]

Please take a look.

[wallyworld, 2013-11-07 00:27:45]

Please take a look.

[axw, 2013-11-07 02:36:10]

LGTM, modulo below

https://codereview.appspot.com/22580043/diff/20001/cmd/juju/addmachine.go
File cmd/juju/addmachine.go (left):

https://codereview.appspot.com/22580043/diff/20001/cmd/juju/addmachine.go#old...
cmd/juju/addmachine.go:128: conf, err := conn.State.EnvironConfig()
Just a thought, but why don't we do this on the server side? i.e. just pass
series="" along, and have the server deal with that.

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

https://codereview.appspot.com/22580043/diff/20001/state/apiserver/client/cli...
state/apiserver/client/client.go:472: return results, nil
This function never fails? I would expect it to return an error if any of the
AddMachines failed, and then I can go grovelling through the individual entries
to find which ones failed.

[wallyworld, 2013-11-07 02:42:19]

https://codereview.appspot.com/22580043/diff/20001/cmd/juju/addmachine.go
File cmd/juju/addmachine.go (left):

https://codereview.appspot.com/22580043/diff/20001/cmd/juju/addmachine.go#old...
cmd/juju/addmachine.go:128: conf, err := conn.State.EnvironConfig()
On 2013/11/07 02:36:10, axw wrote:
> Just a thought, but why don't we do this on the server side? i.e. just pass
> series="" along, and have the server deal with that.

I think that is a fair point. I'll look to do that.

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

https://codereview.appspot.com/22580043/diff/20001/state/apiserver/client/cli...
state/apiserver/client/client.go:472: return results, nil
On 2013/11/07 02:36:10, axw wrote:
> This function never fails? I would expect it to return an error if any of the
> AddMachines failed, and then I can go grovelling through the individual
entries
> to find which ones failed.

My understanding of the API is that for bulk operations, the results object
contains a result and error for each object acted on. If the call to initiate
the operations succeeds, nil is returned. The caller can then iterate of the
results object to see individually whether each operation was successful or not.

[axw, 2013-11-07 03:03:32]

On 2013/11/07 02:42:19, wallyworld wrote:
> https://codereview.appspot.com/22580043/diff/20001/cmd/juju/addmachine.go
> File cmd/juju/addmachine.go (left):
> 
>
https://codereview.appspot.com/22580043/diff/20001/cmd/juju/addmachine.go#old...
> cmd/juju/addmachine.go:128: conf, err := conn.State.EnvironConfig()
> On 2013/11/07 02:36:10, axw wrote:
> > Just a thought, but why don't we do this on the server side? i.e. just pass
> > series="" along, and have the server deal with that.
> 
> I think that is a fair point. I'll look to do that.
> 
>
https://codereview.appspot.com/22580043/diff/20001/state/apiserver/client/cli...
> File state/apiserver/client/client.go (right):
> 
>
https://codereview.appspot.com/22580043/diff/20001/state/apiserver/client/cli...
> state/apiserver/client/client.go:472: return results, nil
> On 2013/11/07 02:36:10, axw wrote:
> > This function never fails? I would expect it to return an error if any of
the
> > AddMachines failed, and then I can go grovelling through the individual
> entries
> > to find which ones failed.
> 
> My understanding of the API is that for bulk operations, the results object
> contains a result and error for each object acted on. If the call to initiate
> the operations succeeds, nil is returned. The caller can then iterate of the
> results object to see individually whether each operation was successful or
not.

I think that makes sense when you're talking about asynchronous bulk calls, when
you have to poll the result of individual operations. *shrug* It's not a big
deal, just surprised me a little.