apiserver/common: DeadEnsurer + Machiner use it

apiserver/common: DeadEnsurer + Machiner use it

Next step of extracing common API code, used by
the machiner and will be used by the uniter.
This introduces DeadEnsurer for entities with
EnsureDead method.

https://code.launchpad.net/~dimitern/juju-core/089-api-ensuredeader/+merge/178545

Requires: https://code.launchpad.net/~dimitern/juju-core/088-api-statussetter/+merge/178529

(do not edit description out of merge proposal)

[dimitern, 2013-08-05 12:37:14]

Please take a look.

[rog, 2013-08-05 14:52:04]

On 2013/08/05 12:37:14, dimitern wrote:
> Please take a look.

LGTM

[dimitern, 2013-08-05 14:56:44]

Please take a look.

[gz, 2013-08-05 15:57:44]

LGTM.

https://codereview.appspot.com/12461043/diff/5001/state/apiserver/common/ensu...
File state/apiserver/common/ensuredead.go (right):

https://codereview.appspot.com/12461043/diff/5001/state/apiserver/common/ensu...
state/apiserver/common/ensuredead.go:18: type DeadEnsurerer interface {
This is absurd. :)

https://codereview.appspot.com/12461043/diff/5001/state/apiserver/common/ensu...
state/apiserver/common/ensuredead.go:45: // will fail if the entity is not
present, of if it's not Alive or
*or if it's not Dead?

I also find the circular nature of all of this confusing...
DeadEnsurer.EnsureDead() calls DeadEnsurer.ensureEntityDead() with each entity's
tag, which calls DeadEnsurer.EnsureDead() with no arguments on a different
deadEnsurer constructed with that tag...

https://codereview.appspot.com/12461043/diff/5001/state/apiserver/common/ensu...
File state/apiserver/common/ensuredead_test.go (right):

https://codereview.appspot.com/12461043/diff/5001/state/apiserver/common/ensu...
state/apiserver/common/ensuredead_test.go:42: panic("not needed")
I prefer errors that will make sense if the test code ever incorrectly reaches
them (though panics are somewhat painful regardless), eg "fakeDeadEnsurer.Tag
should not be called" or similar.

[dimitern, 2013-08-05 16:20:27]

Please take a look.

https://codereview.appspot.com/12461043/diff/5001/state/apiserver/common/ensu...
File state/apiserver/common/ensuredead.go (right):

https://codereview.appspot.com/12461043/diff/5001/state/apiserver/common/ensu...
state/apiserver/common/ensuredead.go:18: type DeadEnsurerer interface {
On 2013/08/05 15:57:44, gz wrote:
> This is absurd. :)

It is! We can change it later, if suggestions for better names arise.

https://codereview.appspot.com/12461043/diff/5001/state/apiserver/common/ensu...
state/apiserver/common/ensuredead.go:45: // will fail if the entity is not
present, of if it's not Alive or
On 2013/08/05 15:57:44, gz wrote:
> *or if it's not Dead?
> 
> I also find the circular nature of all of this confusing...
> DeadEnsurer.EnsureDead() calls DeadEnsurer.ensureEntityDead() with each
entity's
> tag, which calls DeadEnsurer.EnsureDead() with no arguments on a different
> deadEnsurer constructed with that tag...

I'll fix the comment. It actually won't fail if the entity is alive, it just
won't do anything - that's how EnsureDead works.

It is confusing, but it's also not easy to have mockable generic types, like an
"entity" which has EnsureDead and not bringing up the entire state package just
for that.

https://codereview.appspot.com/12461043/diff/5001/state/apiserver/common/ensu...
File state/apiserver/common/ensuredead_test.go (right):

https://codereview.appspot.com/12461043/diff/5001/state/apiserver/common/ensu...
state/apiserver/common/ensuredead_test.go:42: panic("not needed")
On 2013/08/05 15:57:44, gz wrote:
> I prefer errors that will make sense if the test code ever incorrectly reaches
> them (though panics are somewhat painful regardless), eg "fakeDeadEnsurer.Tag
> should not be called" or similar.

I'll change the message.