apiserver: Start Pinger on MA connection

apiserver: Start Pinger on MA connection

Machine agent facade now starts a presence.Pinger
automatically when logging in successfully.
This allows us to skip implementing SetAgentAlive
in the API and handles better the concept of an
agent being "alive": when it has an active API
connection.

Next in line is the implementation of the still
missing API call Machiner.Watch, needed by the
worker.

https://code.launchpad.net/~dimitern/juju-core/073-apiserver-pinger-on-ma-connection/+merge/175833

Requires: https://code.launchpad.net/~dimitern/juju-core/072-machineagent-login-nonce/+merge/175791

(do not edit description out of merge proposal)

[dimitern, 2013-07-19 13:46:46]

Please take a look.

[dimitern, 2013-07-19 15:42:41]

Please take a look.

[dimitern, 2013-07-19 15:55:41]

Please take a look.

[rog, 2013-07-19 16:00:05]

LGTM with one simplification below

https://codereview.appspot.com/11572043/diff/8001/state/apiserver/admin.go
File state/apiserver/admin.go (right):

https://codereview.appspot.com/11572043/diff/8001/state/apiserver/admin.go#ne...
state/apiserver/admin.go:102: if ok && !machine.CheckProvisioned(c.MachineNonce)
{
if ok {
    if !machine.CheckProvisioned(c.MachineNonce) {
        return common.ErrNotProvisioned
    }
    start pinger
}

or am i missing something here?

https://codereview.appspot.com/11572043/diff/8001/state/apiserver/machine/com...
File state/apiserver/machine/common_test.go (right):

https://codereview.appspot.com/11572043/diff/8001/state/apiserver/machine/com...
state/apiserver/machine/common_test.go:40: Tag:          s.machine1.Tag(),
heh

[dimitern, 2013-07-19 16:06:55]

Please take a look.

https://codereview.appspot.com/11572043/diff/8001/state/apiserver/admin.go
File state/apiserver/admin.go (right):

https://codereview.appspot.com/11572043/diff/8001/state/apiserver/admin.go#ne...
state/apiserver/admin.go:102: if ok && !machine.CheckProvisioned(c.MachineNonce)
{
On 2013/07/19 16:00:05, rog wrote:
> if ok {
>     if !machine.CheckProvisioned(c.MachineNonce) {
>         return common.ErrNotProvisioned
>     }
>     start pinger
> }
> 
> or am i missing something here?

Yeah, good point. Wasn't happy how ugly it became. Done.

[fwereade, 2013-07-19 17:08:09]

LGTM with one more test, just to check it's really a resource..?

https://codereview.appspot.com/11572043/diff/13001/state/apiserver/server_tes...
File state/apiserver/server_test.go (right):

https://codereview.appspot.com/11572043/diff/13001/state/apiserver/server_tes...
state/apiserver/server_test.go:145: c.Assert(alive, Equals, true)
I think we should probably test that it's stopped when the connection ends too.

And (not this CL) we should probably have a followup that Kills instead of Stops
when the machine's actually dead...

[dimitern, 2013-07-19 17:59:47]

Please take a look.

https://codereview.appspot.com/11572043/diff/13001/state/apiserver/server_tes...
File state/apiserver/server_test.go (right):

https://codereview.appspot.com/11572043/diff/13001/state/apiserver/server_tes...
state/apiserver/server_test.go:145: c.Assert(alive, Equals, true)
On 2013/07/19 17:08:09, fwereade wrote:
> I think we should probably test that it's stopped when the connection ends
too.
> 
> And (not this CL) we should probably have a followup that Kills instead of
Stops
> when the machine's actually dead...

Ah, good point. Done.
I had to wrap the pinger to redefine its Stop method to call Pinger.Kill
instead.