Code review - Issue 8630043: worker/machiner: set status and agent alivehttps://codereview.appspot.com/2013-04-11T09:11:59+00:00rietveld
Message from unknown
2013-04-10T16:19:48+00:00dimiternurn:md5:42a4f217d0f72ca24443cfb83950ba38
Message from dimiter.naydenov@canonical.com
2013-04-10T16:19:52+00:00dimiternurn:md5:1b17131b30f73dcf656d6f5f566f42b4
Please take a look.
Message from fwereade@gmail.com
2013-04-10T16:37:15+00:00fwereadeurn:md5:03e89dce7c92433d3567890befb6e8e3
LGTM modulo missing test
https://codereview.appspot.com/8630043/diff/1/worker/machiner/machiner.go
File worker/machiner/machiner.go (right):
https://codereview.appspot.com/8630043/diff/1/worker/machiner/machiner.go#newcode59
worker/machiner/machiner.go:59:
Reverse the order of these, I think
https://codereview.appspot.com/8630043/diff/1/worker/machiner/machiner_test.go
File worker/machiner/machiner_test.go (right):
https://codereview.appspot.com/8630043/diff/1/worker/machiner/machiner_test.go#newcode76
worker/machiner/machiner_test.go:76: s.waitMachineStatus(c, m, params.MachineStarted)
I don't see a test for AgentAlive here.
Message from rogpeppe@gmail.com
2013-04-11T07:49:08+00:00rogurn:md5:fd12e68c170e920a68a94c4a319c0403
LGTM modulo william's comments.
https://codereview.appspot.com/8630043/diff/1/worker/machiner/machiner.go
File worker/machiner/machiner.go (right):
https://codereview.appspot.com/8630043/diff/1/worker/machiner/machiner.go#newcode59
worker/machiner/machiner.go:59:
On 2013/04/10 16:37:15, fwereade wrote:
> Reverse the order of these, I think
+1.
if we can't SetAgentAlive, then we shouldn't say we're started.
https://codereview.appspot.com/8630043/diff/1/worker/machiner/machiner_test.go
File worker/machiner/machiner_test.go (right):
https://codereview.appspot.com/8630043/diff/1/worker/machiner/machiner_test.go#newcode76
worker/machiner/machiner_test.go:76: s.waitMachineStatus(c, m, params.MachineStarted)
On 2013/04/10 16:37:15, fwereade wrote:
> I don't see a test for AgentAlive here.
+1
Message from unknown
2013-04-11T09:11:28+00:00dimiternurn:md5:80d5ca4470b595031ae51d06f19392e8
Message from dimiter.naydenov@canonical.com
2013-04-11T09:11:59+00:00dimiternurn:md5:f1775b7e42ffa876f00b43e9c2299f61
*** Submitted:
worker/machiner: set status and agent alive
The machiner will now set the machine status
as needed (started/stopped), log the actions
with Noticef, and SetAgentAlive(), as required.
R=fwereade, rog
CC=
https://codereview.appspot.com/8630043
https://codereview.appspot.com/8630043/diff/1/worker/machiner/machiner.go
File worker/machiner/machiner.go (right):
https://codereview.appspot.com/8630043/diff/1/worker/machiner/machiner.go#newcode59
worker/machiner/machiner.go:59:
On 2013/04/10 16:37:15, fwereade wrote:
> Reverse the order of these, I think
Done.
https://codereview.appspot.com/8630043/diff/1/worker/machiner/machiner_test.go
File worker/machiner/machiner_test.go (right):
https://codereview.appspot.com/8630043/diff/1/worker/machiner/machiner_test.go#newcode76
worker/machiner/machiner_test.go:76: s.waitMachineStatus(c, m, params.MachineStarted)
On 2013/04/10 16:37:15, fwereade wrote:
> I don't see a test for AgentAlive here.
Done.