provider/common: Destroy, and tests

provider/common: Destroy, and tests

Added a common implementation of Destroy, and tests for Bootstrap which had
somehow never been written. Also drive-by removed the machineId arg from
consideration when bootstrapping, because, WTF... a non-0 bootstrap machine
would break *everything*.

https://code.launchpad.net/~fwereade/juju-core/provider-common-destroy/+merge/188750

(do not edit description out of merge proposal)

[fwereade, 2013-10-02 00:50:26]

Please take a look.

[axw1, 2013-10-02 08:38:59]

LGTM modulo below concern. Let me know what you think.

https://codereview.appspot.com/14254043/diff/1/provider/common/destroy.go
File provider/common/destroy.go (right):

https://codereview.appspot.com/14254043/diff/1/provider/common/destroy.go#new...
provider/common/destroy.go:21: fallthrough
Hrm, so we'll clean up the bootstrap-state file even if we fail to stop some
instances now. This is a departure from ec2.

Is this really desirable? It could be easy to miss that error log message, and
not notice that you've left some instances running. Particularly if you ran
destroy-environment from a script.

[fwereade, 2013-10-02 10:15:16]

https://codereview.appspot.com/14254043/diff/1/provider/common/destroy.go
File provider/common/destroy.go (right):

https://codereview.appspot.com/14254043/diff/1/provider/common/destroy.go#new...
provider/common/destroy.go:21: fallthrough
On 2013/10/02 08:38:59, axw1 wrote:
> Hrm, so we'll clean up the bootstrap-state file even if we fail to stop some
> instances now. This is a departure from ec2.
> 
> Is this really desirable? It could be easy to miss that error log message, and
> not notice that you've left some instances running. Particularly if you ran
> destroy-environment from a script.

If StopInstances fails, we return the error and leave the storage alone... don't
we? TestCannotStopInstances doesn't configure storage, so it'll panic if we hit
it, I think.

[axw1, 2013-10-02 10:42:01]

https://codereview.appspot.com/14254043/diff/1/provider/common/destroy.go
File provider/common/destroy.go (right):

https://codereview.appspot.com/14254043/diff/1/provider/common/destroy.go#new...
provider/common/destroy.go:21: fallthrough
On 2013/10/02 10:15:16, fwereade wrote:
> On 2013/10/02 08:38:59, axw1 wrote:
> > Hrm, so we'll clean up the bootstrap-state file even if we fail to stop some
> > instances now. This is a departure from ec2.
> > 
> > Is this really desirable? It could be easy to miss that error log message,
and
> > not notice that you've left some instances running. Particularly if you ran
> > destroy-environment from a script.
> 
> If StopInstances fails, we return the error and leave the storage alone...
don't
> we? TestCannotStopInstances doesn't configure storage, so it'll panic if we
hit
> it, I think.

Yep, I'm an idiot. Forget that you ever saw this ;)