juju-core

Merge lp:~themue/juju-core/go-provisioning-state into lp:~juju/juju-core/trunk

  1. go-provisioning-state
  2. Merge into trunk
Proposed by Frank Mueller
Status: Merged
Approved by: Gustavo Niemeyer
Approved revision: 334
Merged at revision: 336
Proposed branch: lp:~themue/juju-core/go-provisioning-state
Merge into: lp:~juju/juju-core/trunk
Diff against target: 229 lines (+25/-47)
3 files modified
cmd/jujud/provisioning.go (+10/-1)
worker/provisioner/provisioner.go (+1/-7)
worker/provisioner/provisioner_test.go (+14/-39)
To merge this branch: bzr merge lp:~themue/juju-core/go-provisioning-state
Reviewer Review Type Date Requested Status
The Go Language Gophers Pending
Review via email: mp+116832@code.launchpad.net

Description of the change

provisioning: provisioner gets state as argument

As a preparation for the integration of the firewaller
into the provisioning agent provisioner has been changed
to get a state instead an info as argument. So both
later can share the same state.

https://codereview.appspot.com/6452049/

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

https://codereview.appspot.com/6452049/diff/1/cmd/jujud/provisioning.go
File cmd/jujud/provisioning.go (right):

https://codereview.appspot.com/6452049/diff/1/cmd/jujud/provisioning.go#newcode39
cmd/jujud/provisioning.go:39: st, err := state.Open(&a.Conf.StateInfo)
Hmm.. I think we should Close and re-open the state connection with the
state in case of errors.

https://codereview.appspot.com/6452049/

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

LGTM

https://codereview.appspot.com/6452049/

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

LGTM

https://codereview.appspot.com/6452049/

lp:~themue/juju-core/go-provisioning-state updated
335. By Frank Mueller

provisioning: merged trunk after lgtm

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cmd/jujud/provisioning.go'
2--- cmd/jujud/provisioning.go 2012-07-11 05:10:11 +0000
3+++ cmd/jujud/provisioning.go 2012-07-26 12:55:21 +0000
4@@ -6,6 +6,7 @@
5 "launchpad.net/gnuflag"
6 "launchpad.net/juju-core/cmd"
7 "launchpad.net/juju-core/log"
8+ "launchpad.net/juju-core/state"
9 "launchpad.net/juju-core/worker/provisioner"
10
11 // register providers
12@@ -36,7 +37,11 @@
13 // Run runs a provisioning agent.
14 func (a *ProvisioningAgent) Run(_ *cmd.Context) error {
15 for {
16- p, err := provisioner.NewProvisioner(&a.Conf.StateInfo)
17+ st, err := state.Open(&a.Conf.StateInfo)
18+ if err != nil {
19+ return err
20+ }
21+ p, err := provisioner.NewProvisioner(st)
22 if err == nil {
23 if err = p.Wait(); err == nil {
24 // if Wait returns nil then we consider that a signal
25@@ -44,6 +49,10 @@
26 return nil
27 }
28 }
29+ err = st.Close()
30+ if err != nil {
31+ return err
32+ }
33 log.Printf("restarting provisioner after error: %v", err)
34 time.Sleep(retryDuration)
35 }
36
37=== modified file 'worker/provisioner/provisioner.go'
38--- worker/provisioner/provisioner.go 2012-07-25 08:59:06 +0000
39+++ worker/provisioner/provisioner.go 2012-07-26 12:55:21 +0000
40@@ -22,14 +22,9 @@
41 }
42
43 // NewProvisioner returns a Provisioner.
44-func NewProvisioner(info *state.Info) (*Provisioner, error) {
45- st, err := state.Open(info)
46- if err != nil {
47- return nil, err
48- }
49+func NewProvisioner(st *state.State) (*Provisioner, error) {
50 p := &Provisioner{
51 st: st,
52- info: info,
53 instances: make(map[int]environs.Instance),
54 machines: make(map[string]*state.Machine),
55 }
56@@ -39,7 +34,6 @@
57
58 func (p *Provisioner) loop() {
59 defer p.tomb.Done()
60- defer p.st.Close()
61 environWatcher := p.st.WatchEnvironConfig()
62 defer watcher.Stop(environWatcher, &p.tomb)
63
64
65=== modified file 'worker/provisioner/provisioner_test.go'
66--- worker/provisioner/provisioner_test.go 2012-07-17 01:34:28 +0000
67+++ worker/provisioner/provisioner_test.go 2012-07-26 12:55:21 +0000
68@@ -10,7 +10,6 @@
69 "launchpad.net/juju-core/state/testing"
70 coretesting "launchpad.net/juju-core/testing"
71 "launchpad.net/juju-core/worker/provisioner"
72- "strings"
73 stdtesting "testing"
74 )
75
76@@ -60,7 +59,6 @@
77
78 func (s *ProvisionerSuite) TearDownTest(c *C) {
79 dummy.Reset()
80- s.StateSuite.TearDownTest(c)
81 s.LoggingSuite.TearDownTest(c)
82 }
83
84@@ -181,36 +179,13 @@
85 }
86
87 func (s *ProvisionerSuite) TestProvisionerStartStop(c *C) {
88- p, err := provisioner.NewProvisioner(s.StateInfo(c))
89+ p, err := provisioner.NewProvisioner(s.State)
90 c.Assert(err, IsNil)
91 c.Assert(p.Stop(), IsNil)
92 }
93
94-func (s *ProvisionerSuite) TestProvisionerDifferentStateInfo(c *C) {
95- info := *s.StateInfo(c)
96-
97- // Use an equivalent but textually different address and check
98- // that the info when new instances are started is that returned
99- // from the environment, not the one passed in originally.
100- info.Addrs = append([]string{}, info.Addrs...)
101- if !strings.HasPrefix(info.Addrs[0], "127.0.0.1") {
102- c.Fatalf("local address %q not as expected", info.Addrs)
103- }
104- info.Addrs[0] = "localhost" + info.Addrs[0][len("127.0.0.1"):]
105-
106- p, err := provisioner.NewProvisioner(&info)
107- c.Assert(err, IsNil)
108- defer s.stopProvisioner(c, p)
109-
110- // place a new machine into the state
111- m, err := s.State.AddMachine()
112- c.Assert(err, IsNil)
113-
114- s.checkStartInstance(c, m)
115-}
116-
117 func (s *ProvisionerSuite) TestProvisionerEnvironmentChange(c *C) {
118- p, err := provisioner.NewProvisioner(s.StateInfo(c))
119+ p, err := provisioner.NewProvisioner(s.State)
120 c.Assert(err, IsNil)
121 defer s.stopProvisioner(c, p)
122 // Twiddle with the environment configuration.
123@@ -224,7 +199,7 @@
124 }
125
126 func (s *ProvisionerSuite) TestProvisionerStopOnStateClose(c *C) {
127- p, err := provisioner.NewProvisioner(s.StateInfo(c))
128+ p, err := provisioner.NewProvisioner(s.State)
129 c.Assert(err, IsNil)
130
131 p.CloseState()
132@@ -236,7 +211,7 @@
133
134 // Start and stop one machine, watch the PA.
135 func (s *ProvisionerSuite) TestSimple(c *C) {
136- p, err := provisioner.NewProvisioner(s.StateInfo(c))
137+ p, err := provisioner.NewProvisioner(s.State)
138 c.Assert(err, IsNil)
139 defer s.stopProvisioner(c, p)
140
141@@ -258,7 +233,7 @@
142 err := s.invalidateEnvironment()
143 c.Assert(err, IsNil)
144
145- p, err := provisioner.NewProvisioner(s.StateInfo(c))
146+ p, err := provisioner.NewProvisioner(s.State)
147 c.Assert(err, IsNil)
148 defer s.stopProvisioner(c, p)
149
150@@ -274,7 +249,7 @@
151 err := s.invalidateEnvironment()
152 c.Assert(err, IsNil)
153
154- p, err := provisioner.NewProvisioner(s.StateInfo(c))
155+ p, err := provisioner.NewProvisioner(s.State)
156 c.Assert(err, IsNil)
157 defer s.stopProvisioner(c, p)
158
159@@ -292,7 +267,7 @@
160 }
161
162 func (s *ProvisionerSuite) TestProvisioningDoesOccurAfterInvalidEnvironmentPublished(c *C) {
163- p, err := provisioner.NewProvisioner(s.StateInfo(c))
164+ p, err := provisioner.NewProvisioner(s.State)
165 c.Assert(err, IsNil)
166 defer s.stopProvisioner(c, p)
167
168@@ -314,7 +289,7 @@
169 }
170
171 func (s *ProvisionerSuite) TestProvisioningDoesNotProvisionTheSameMachineAfterRestart(c *C) {
172- p, err := provisioner.NewProvisioner(s.StateInfo(c))
173+ p, err := provisioner.NewProvisioner(s.State)
174 c.Check(err, IsNil)
175 // we are not using defer s.stopProvisioner(c, p) because we need to control when
176 // the PA is restarted in this test. tf. Methods like Fatalf and Assert should not be used.
177@@ -328,7 +303,7 @@
178 // restart the PA
179 c.Check(p.Stop(), IsNil)
180
181- p, err = provisioner.NewProvisioner(s.StateInfo(c))
182+ p, err = provisioner.NewProvisioner(s.State)
183 c.Check(err, IsNil)
184
185 // check that there is only one machine known
186@@ -344,7 +319,7 @@
187 }
188
189 func (s *ProvisionerSuite) TestProvisioningStopsUnknownInstances(c *C) {
190- p, err := provisioner.NewProvisioner(s.StateInfo(c))
191+ p, err := provisioner.NewProvisioner(s.State)
192 c.Check(err, IsNil)
193 // we are not using defer s.stopProvisioner(c, p) because we need to control when
194 // the PA is restarted in this test. Methods like Fatalf and Assert should not be used.
195@@ -369,7 +344,7 @@
196 c.Check(err, IsNil)
197
198 // start a new provisioner
199- p, err = provisioner.NewProvisioner(s.StateInfo(c))
200+ p, err = provisioner.NewProvisioner(s.State)
201 c.Check(err, IsNil)
202
203 s.checkStopInstance(c)
204@@ -381,7 +356,7 @@
205 // where the final machine has been removed from the state while the PA was
206 // not running.
207 func (s *ProvisionerSuite) TestProvisioningStopsOnlyUnknownInstances(c *C) {
208- p, err := provisioner.NewProvisioner(s.StateInfo(c))
209+ p, err := provisioner.NewProvisioner(s.State)
210 c.Check(err, IsNil)
211 // we are not using defer s.stopProvisioner(c, p) because we need to control when
212 // the PA is restarted in this test. Methods like Fatalf and Assert should not be used.
213@@ -404,7 +379,7 @@
214 c.Check(len(machines), Equals, 0) // it's really gone
215
216 // start a new provisioner
217- p, err = provisioner.NewProvisioner(s.StateInfo(c))
218+ p, err = provisioner.NewProvisioner(s.State)
219 c.Check(err, IsNil)
220
221 s.checkStopInstance(c)
222@@ -413,7 +388,7 @@
223 }
224
225 func (s *ProvisionerSuite) TestProvisioningRecoversAfterInvalidEnvironmentPublished(c *C) {
226- p, err := provisioner.NewProvisioner(s.StateInfo(c))
227+ p, err := provisioner.NewProvisioner(s.State)
228 c.Assert(err, IsNil)
229 defer s.stopProvisioner(c, p)
230

Subscribers

People subscribed via source and target branches