Implement DestroyEnvironment API

Implement DestroyEnvironment API

We introduce a new DestroyEnvironment client API
method, which:
 - checks that there are no manual non-manager machines in state
 - sets the environment to Dying, scheduling
   cleanups for services and preventing addition
   of new services and machines.
 - destroys non-manager instances

"juju destroy-environment" will call this new method,
ignoring the "no such method" error returned by old
API servers. If the API method succeeds, or it doesn't
exist, the provider.Destroy() method is called.

https://code.launchpad.net/~axwalk/juju-core/destroy-environment-api/+merge/198698

(do not edit description out of merge proposal)

[axw, 2013-12-12 10:07:38]

Please take a look.

[fwereade, 2013-12-18 12:52:20]

LGTM with more-eager instance-killing, or a good reason not to, plus the other
trivials below

https://codereview.appspot.com/41280043/diff/1/state/apiserver/client/destroy.go
File state/apiserver/client/destroy.go (right):

https://codereview.appspot.com/41280043/diff/1/state/apiserver/client/destroy...
state/apiserver/client/destroy.go:54: if err := checkManualMachines(machines);
err != nil {
Hmm, can we consider the ordering of the check-and-abort vs the
stop-all-instances? I feel like it's probably best to kill what we can, and then
return an error like "can't stop state server yet, manual machines x, y, z are
still using it, please destroy-machine".

Also... it crosses my mind that the environ provisioner might take some time to
get the memo about the environment dying. Do we have any way to block racy
*provisioning* of recently-added machines? That can be a followup, though.

https://codereview.appspot.com/41280043/diff/1/state/apiserver/client/destroy...
state/apiserver/client/destroy.go:110: if inst == nil {
log the failed instances?

https://codereview.appspot.com/41280043/diff/1/state/apiserver/client/destroy...
state/apiserver/client/destroy.go:145: return strings.HasPrefix(string(iid),
"manual:")
Yeah, use the nonce -- if you need to make this a method on machine, so be it.

[axw, 2013-12-19 05:16:42]

Please take a look.

https://codereview.appspot.com/41280043/diff/1/state/apiserver/client/destroy.go
File state/apiserver/client/destroy.go (right):

https://codereview.appspot.com/41280043/diff/1/state/apiserver/client/destroy...
state/apiserver/client/destroy.go:54: if err := checkManualMachines(machines);
err != nil {
On 2013/12/18 12:52:20, fwereade wrote:
> Hmm, can we consider the ordering of the check-and-abort vs the
> stop-all-instances? I feel like it's probably best to kill what we can, and
then
> return an error like "can't stop state server yet, manual machines x, y, z are
> still using it, please destroy-machine".

Moved the checkManualMachines after destroyInstances, and updated
destroyInstances to ignore manual machines.

> Also... it crosses my mind that the environ provisioner might take some time
to
> get the memo about the environment dying. Do we have any way to block racy
> *provisioning* of recently-added machines? That can be a followup, though.

I don't think we have any way of doing that yet.

https://codereview.appspot.com/41280043/diff/1/state/apiserver/client/destroy...
state/apiserver/client/destroy.go:110: if inst == nil {
On 2013/12/18 12:52:20, fwereade wrote:
> log the failed instances?

Done.

https://codereview.appspot.com/41280043/diff/1/state/apiserver/client/destroy...
state/apiserver/client/destroy.go:145: return strings.HasPrefix(string(iid),
"manual:")
On 2013/12/18 12:52:20, fwereade wrote:
> Yeah, use the nonce -- if you need to make this a method on machine, so be it.

Done.

[axw, 2013-12-19 14:25:38]

Please take a look.

[axw, 2013-12-19 14:34:00]

On 2013/12/19 14:25:38, axw wrote:
> Please take a look.

FYI, I added a --force flag to destroy-environment which skips the API call.
This is necessary for destroying broken environments, where the API server is
inaccessible or otherwise broken. There's also a drive-by fix to add-machine,
which was returning spurious errors due to nil interface assignment.

[axw, 2013-12-19 14:34:00]

On 2013/12/19 14:25:38, axw wrote:
> Please take a look.

FYI, I added a --force flag to destroy-environment which skips the API call.
This is necessary for destroying broken environments, where the API server is
inaccessible or otherwise broken. There's also a drive-by fix to add-machine,
which was returning spurious errors due to nil interface assignment.