Merge lp:~themue/juju-core/go-mstate-machine-agent-alive into lp:~juju/juju-core/trunk

Proposed by Frank Mueller
Status: Merged
Approved by: Gustavo Niemeyer
Approved revision: no longer in the source branch.
Merged at revision: 473
Proposed branch: lp:~themue/juju-core/go-mstate-machine-agent-alive
Merge into: lp:~juju/juju-core/trunk
Prerequisite: lp:~themue/juju-core/go-mstate-unit-status
Diff against target: 140 lines (+96/-0) (has conflicts)
2 files modified
mstate/machine.go (+51/-0)
mstate/machine_test.go (+45/-0)
Text conflict in mstate/state.go
To merge this branch: bzr merge lp:~themue/juju-core/go-mstate-machine-agent-alive
Reviewer Review Type Date Requested Status
The Go Language Gophers Pending
Review via email: mp+122819@code.launchpad.net

Description of the change

mstate: added agent alive methods

Machine provides AgentAlive(), WaitAgentAlive() and
SetAgentAlive() using the presence package.

https://codereview.appspot.com/6498091/

To post a comment you must log in.
Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Nice, LGTM.

https://codereview.appspot.com/6498091/diff/1/mstate/machine.go
File mstate/machine.go (right):

https://codereview.appspot.com/6498091/diff/1/mstate/machine.go#newcode82
mstate/machine.go:82: func (m *Machine) WaitAgentAlive(timeout
time.Duration) error {
Nice implementation.

https://codereview.appspot.com/6498091/diff/1/mstate/machine.go#newcode101
mstate/machine.go:101: panic(fmt.Sprintf("unexpected alive status of
machine %v", m))
"presence reported dead status twice in a row for machine %v"

https://codereview.appspot.com/6498091/diff/1/mstate/machine.go#newcode109
mstate/machine.go:109: // by starting a pinger on its presence node. It
returns the
Please drop the "by starting a pinger on its presence node" part. This
isn't entirely true anymore, and isn't really necessary given the rest
of the doc.

https://codereview.appspot.com/6498091/diff/1/mstate/machine_test.go
File mstate/machine_test.go (right):

https://codereview.appspot.com/6498091/diff/1/mstate/machine_test.go#newcode31
mstate/machine_test.go:31: defer pinger.Kill()
s/Kill/Stop/

We only care that it's not running after the test is done.

https://codereview.appspot.com/6498091/diff/1/mstate/machine_test.go#newcode50
mstate/machine_test.go:50: c.Assert(pinger, Not(IsNil))
This check isn't necessary. We'd go crazy if we had to check everything
that is *not* nil when err is nil.

https://codereview.appspot.com/6498091/diff/1/mstate/machine_test.go#newcode60
mstate/machine_test.go:60: pinger.Kill()
err := pinger.Kill()
c.Assert(err, IsNil)

https://codereview.appspot.com/6498091/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'mstate/machine.go'
--- mstate/machine.go 2012-08-29 10:05:26 +0000
+++ mstate/machine.go 2012-09-05 09:13:24 +0000
@@ -3,8 +3,10 @@
3import (3import (
4 "fmt"4 "fmt"
5 "labix.org/v2/mgo/txn"5 "labix.org/v2/mgo/txn"
6 "launchpad.net/juju-core/mstate/presence"
6 "launchpad.net/juju-core/trivial"7 "launchpad.net/juju-core/trivial"
7 "strconv"8 "strconv"
9 "time"
8)10)
911
10// Machine represents the state of a machine.12// Machine represents the state of a machine.
@@ -29,6 +31,11 @@
29 return m.doc.Id31 return m.doc.Id
30}32}
3133
34// globalKey returns the global database key for the machine.
35func (m *Machine) globalKey() string {
36 return "m#" + m.String()
37}
38
32// Life returns whether the machine is Alive, Dying or Dead.39// Life returns whether the machine is Alive, Dying or Dead.
33func (m *Machine) Life() Life {40func (m *Machine) Life() Life {
34 return m.doc.Life41 return m.doc.Life
@@ -66,6 +73,50 @@
66 return nil73 return nil
67}74}
6875
76// AgentAlive returns whether the respective remote agent is alive.
77func (m *Machine) AgentAlive() bool {
78 return m.st.presencew.Alive(m.globalKey())
79}
80
81// WaitAgentAlive blocks until the respective agent is alive.
82func (m *Machine) WaitAgentAlive(timeout time.Duration) error {
83 ch := make(chan presence.Change)
84 m.st.presencew.Add(m.globalKey(), ch)
85 defer m.st.presencew.Remove(m.globalKey(), ch)
86 // Initial check.
87 select {
88 case change := <-ch:
89 if change.Alive {
90 return nil
91 }
92 case <-time.After(timeout):
93 return fmt.Errorf("waiting for agent of machine %v: still not alive after timeout", m)
94 }
95 // Hasn't been alive, so now wait for change.
96 select {
97 case change := <-ch:
98 if change.Alive {
99 return nil
100 }
101 panic(fmt.Sprintf("unexpected alive status of machine %v", m))
102 case <-time.After(timeout):
103 return fmt.Errorf("waiting for agent of machine %v: still not alive after timeout", m)
104 }
105 panic("unreachable")
106}
107
108// SetAgentAlive signals that the agent for machine m is alive
109// by starting a pinger on its presence node. It returns the
110// started pinger.
111func (m *Machine) SetAgentAlive() (*presence.Pinger, error) {
112 p := presence.NewPinger(m.st.presence, m.globalKey())
113 err := p.Start()
114 if err != nil {
115 return nil, err
116 }
117 return p, nil
118}
119
69// InstanceId returns the provider specific machine id for this machine.120// InstanceId returns the provider specific machine id for this machine.
70func (m *Machine) InstanceId() (string, error) {121func (m *Machine) InstanceId() (string, error) {
71 if m.doc.InstanceId == "" {122 if m.doc.InstanceId == "" {
72123
=== modified file 'mstate/machine_test.go'
--- mstate/machine_test.go 2012-08-29 10:05:26 +0000
+++ mstate/machine_test.go 2012-09-05 09:13:24 +0000
@@ -4,6 +4,7 @@
4 . "launchpad.net/gocheck"4 . "launchpad.net/gocheck"
5 state "launchpad.net/juju-core/mstate"5 state "launchpad.net/juju-core/mstate"
6 "sort"6 "sort"
7 "time"
7)8)
89
9type MachineSuite struct {10type MachineSuite struct {
@@ -20,6 +21,50 @@
20 c.Assert(err, IsNil)21 c.Assert(err, IsNil)
21}22}
2223
24func (s *MachineSuite) TestMachineSetAgentAlive(c *C) {
25 alive := s.machine.AgentAlive()
26 c.Assert(alive, Equals, false)
27
28 pinger, err := s.machine.SetAgentAlive()
29 c.Assert(err, IsNil)
30 c.Assert(pinger, Not(IsNil))
31 defer pinger.Kill()
32
33 s.State.ForcePresenceRefresh()
34 alive = s.machine.AgentAlive()
35 c.Assert(alive, Equals, true)
36}
37
38func (s *MachineSuite) TestMachineWaitAgentAlive(c *C) {
39 // test -gocheck.f TestMachineWaitAgentAlive
40 timeout := 5 * time.Second
41 alive := s.machine.AgentAlive()
42 c.Assert(alive, Equals, false)
43
44 s.State.ForcePresenceRefresh()
45 err := s.machine.WaitAgentAlive(timeout)
46 c.Assert(err, ErrorMatches, `waiting for agent of machine 0: still not alive after timeout`)
47
48 pinger, err := s.machine.SetAgentAlive()
49 c.Assert(err, IsNil)
50 c.Assert(pinger, Not(IsNil))
51
52 s.State.ForcePresenceRefresh()
53 err = s.machine.WaitAgentAlive(timeout)
54 c.Assert(err, IsNil)
55
56 alive = s.machine.AgentAlive()
57 c.Assert(err, IsNil)
58 c.Assert(alive, Equals, true)
59
60 pinger.Kill()
61
62 s.State.ForcePresenceRefresh()
63 alive = s.machine.AgentAlive()
64 c.Assert(err, IsNil)
65 c.Assert(alive, Equals, false)
66}
67
23func (s *MachineSuite) TestMachineInstanceId(c *C) {68func (s *MachineSuite) TestMachineInstanceId(c *C) {
24 machine, err := s.State.AddMachine()69 machine, err := s.State.AddMachine()
25 c.Assert(err, IsNil)70 c.Assert(err, IsNil)

Subscribers

People subscribed via source and target branches