Merge lp:~thumper/juju-core/machine-worker into lp:~go-bot/juju-core/trunk
- machine-worker
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Tim Penhey |
Approved revision: | no longer in the source branch. |
Merged at revision: | 2269 |
Proposed branch: | lp:~thumper/juju-core/machine-worker |
Merge into: | lp:~go-bot/juju-core/trunk |
Prerequisite: | lp:~thumper/juju-core/fix-uniter-proxy-env-vars |
Diff against target: |
801 lines (+639/-1) 15 files modified
cmd/jujud/machine.go (+4/-0) cmd/jujud/machine_test.go (+44/-0) environs/cloudinit/cloudinit.go (+1/-1) state/api/environment/environment.go (+23/-0) state/api/environment/environment_test.go (+30/-0) state/api/environment/package_test.go (+14/-0) state/api/state.go (+6/-0) state/apiserver/environment/environment.go (+25/-0) state/apiserver/environment/environment_test.go (+53/-0) state/apiserver/environment/package_test.go (+14/-0) state/apiserver/root.go (+12/-0) utils/apt.go (+4/-0) worker/machineenvironmentworker/machineenvironmentworker.go (+173/-0) worker/machineenvironmentworker/machineenvironmentworker_test.go (+222/-0) worker/machineenvironmentworker/package_test.go (+14/-0) |
To merge this branch: | bzr merge lp:~thumper/juju-core/machine-worker |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju Engineering | Pending | ||
Review via email: mp+203460@code.launchpad.net |
Commit message
Add a worker to keep disk files up to date
Added the machine environment worker which watches
the environment config and will write changes to the
disk as needed.
A new end point was added on the client and server side
that is just called "Environment", which provides the common
environment watcher.
The only gotcha in this is that if the worker is running on
machine 0 for a local provider, it shouldn't attempt to modify
system files. There are tests for this.
The two files on disk that are currently being kept up
to date with this are:
/home/
/etc/apt/
Description of the change
Add a worker to keep disk files up to date
Added the machine environment worker which watches
the environment config and will write changes to the
disk as needed.
A new end point was added on the client and server side
that is just called "Environment", which provies the common
environment watcher.
The only gotcha in this is that if the worker is running on
machine 0 for a local provider, it shouldn't attempt to modify
system files. There are tests for this.
The two files on disk that are currently being kept up
to date with this are:
/home/
/etc/apt/
Tim Penhey (thumper) wrote : | # |
Ian Booth (wallyworld) wrote : | # |
Looks pretty good but we need a juju/machine_test to test that the jujud
has started the worker properly. There's a few examples in machine_test
to look at.
https:/
File worker/
(right):
https:/
worker/
ProxyDirectory = "/home/ubuntu"
Not sure if other workers use ~ubuntu and/or /home/ubuntu, but if they
do , would be good to make this a const or method call.
https:/
worker/
bool
Do other workers use first? Don't they just suck up the initial event
and deal with it? It seems a little complicated to use it here. If we do
need to use it, perhaps a comment saying why we need it here.
https:/
worker/
ubuntu user, it will. This shouldn't be a problem.
I read the above a few times and am still not sure I've parsed it
correctly. I think "should" might need to be "shouldn't"??
https:/
worker/
way that the file is writting in the cloud-init process, so
s/writting/written ?
https:/
worker/
aptSettings := env.AptProxySet
not sure if we want the 2 lots of logic in this method in separate sub
methods so onChange is dead easy to ready
https:/
worker/
*MachineEnviron
Typically we add a doc string saying what interface SetUp() comes from
https:/
worker/
*MachineEnviron
ditto
https:/
worker/
*MachineEnviron
ditto
https:/
File worker/
(right):
Tim Penhey (thumper) wrote : | # |
Please take a look.
https:/
File worker/
(right):
https:/
worker/
ProxyDirectory = "/home/ubuntu"
On 2014/01/28 04:44:50, wallyworld wrote:
> Not sure if other workers use ~ubuntu and/or /home/ubuntu, but if they
do ,
> would be good to make this a const or method call.
The whole point it is a variable is so we can mock it out during
testing.
https:/
worker/
bool
On 2014/01/28 04:44:50, wallyworld wrote:
> Do other workers use first? Don't they just suck up the initial event
and deal
> with it? It seems a little complicated to use it here. If we do need
to use it,
> perhaps a comment saying why we need it here.
I'll add a comment. It is only used for testing in order to call the
Started function (which is by default a no-op). The Started function is
mocked out in the tests so I can demonstrate that not only do the
changes happen when it starts up, but that they happen once it is in the
main loop.
https:/
worker/
ubuntu user, it will. This shouldn't be a problem.
On 2014/01/28 04:44:50, wallyworld wrote:
> I read the above a few times and am still not sure I've parsed it
correctly. I
> think "should" might need to be "shouldn't"??
Yes, you are right. Changed.
https:/
worker/
way that the file is writting in the cloud-init process, so
On 2014/01/28 04:44:50, wallyworld wrote:
> s/writting/written ?
Done.
https:/
worker/
aptSettings := env.AptProxySet
On 2014/01/28 04:44:50, wallyworld wrote:
> not sure if we want the 2 lots of logic in this method in separate sub
methods
> so onChange is dead easy to ready
Yeah I was tossing up on this one as I was writing it.
I'll split it up.
https:/
worker/
*MachineEnviron
On 2014/01/28 04:44:50, wallyworld wrote:
> Typically we add a doc string saying what interface SetUp() comes from
cleaner, logger and machiner all didn't. I looked at your one for the
boilerplate.
Ian Booth (wallyworld) wrote : | # |
LGTM, thanks for the extra code comments, especially for first. Makes it
much clearer for the casual reader.
https:/
File cmd/jujud/
https:/
cmd/jujud/
filepath.
Small nitpick, feel free to ignore - these set up steps are only for one
specific test and not for the tests in general. My preference would be
just to have them in the one test that needs them.
Preview Diff
1 | === modified file 'cmd/jujud/machine.go' |
2 | --- cmd/jujud/machine.go 2014-01-28 04:58:43 +0000 |
3 | +++ cmd/jujud/machine.go 2014-01-29 04:59:50 +0000 |
4 | @@ -37,6 +37,7 @@ |
5 | "launchpad.net/juju-core/worker/instancepoller" |
6 | "launchpad.net/juju-core/worker/localstorage" |
7 | workerlogger "launchpad.net/juju-core/worker/logger" |
8 | + "launchpad.net/juju-core/worker/machineenvironmentworker" |
9 | "launchpad.net/juju-core/worker/machiner" |
10 | "launchpad.net/juju-core/worker/minunitsworker" |
11 | "launchpad.net/juju-core/worker/provisioner" |
12 | @@ -183,6 +184,9 @@ |
13 | runner.StartWorker("logger", func() (worker.Worker, error) { |
14 | return workerlogger.NewLogger(st.Logger(), agentConfig), nil |
15 | }) |
16 | + runner.StartWorker("machineenvironmentworker", func() (worker.Worker, error) { |
17 | + return machineenvironmentworker.NewMachineEnvironmentWorker(st.Environment(), agentConfig), nil |
18 | + }) |
19 | |
20 | // If not a local provider bootstrap machine, start the worker to manage SSH keys. |
21 | providerType := agentConfig.Value(agent.ProviderType) |
22 | |
23 | === modified file 'cmd/jujud/machine_test.go' |
24 | --- cmd/jujud/machine_test.go 2014-01-24 14:52:58 +0000 |
25 | +++ cmd/jujud/machine_test.go 2014-01-29 04:59:50 +0000 |
26 | @@ -16,10 +16,12 @@ |
27 | "launchpad.net/juju-core/charm" |
28 | "launchpad.net/juju-core/cmd" |
29 | lxctesting "launchpad.net/juju-core/container/lxc/testing" |
30 | + "launchpad.net/juju-core/environs/config" |
31 | envtesting "launchpad.net/juju-core/environs/testing" |
32 | "launchpad.net/juju-core/errors" |
33 | "launchpad.net/juju-core/instance" |
34 | "launchpad.net/juju-core/juju" |
35 | + "launchpad.net/juju-core/juju/osenv" |
36 | "launchpad.net/juju-core/names" |
37 | "launchpad.net/juju-core/provider/dummy" |
38 | "launchpad.net/juju-core/state" |
39 | @@ -30,15 +32,18 @@ |
40 | statetesting "launchpad.net/juju-core/state/testing" |
41 | "launchpad.net/juju-core/state/watcher" |
42 | coretesting "launchpad.net/juju-core/testing" |
43 | + "launchpad.net/juju-core/testing" |
44 | jc "launchpad.net/juju-core/testing/checkers" |
45 | "launchpad.net/juju-core/testing/testbase" |
46 | "launchpad.net/juju-core/tools" |
47 | + "launchpad.net/juju-core/utils" |
48 | "launchpad.net/juju-core/utils/ssh" |
49 | sshtesting "launchpad.net/juju-core/utils/ssh/testing" |
50 | "launchpad.net/juju-core/version" |
51 | "launchpad.net/juju-core/worker/authenticationworker" |
52 | "launchpad.net/juju-core/worker/deployer" |
53 | "launchpad.net/juju-core/worker/instancepoller" |
54 | + "launchpad.net/juju-core/worker/machineenvironmentworker" |
55 | ) |
56 | |
57 | type MachineSuite struct { |
58 | @@ -674,6 +679,45 @@ |
59 | }) |
60 | } |
61 | |
62 | +func (s *MachineSuite) TestMachineEnvirnWorker(c *gc.C) { |
63 | + proxyDir := c.MkDir() |
64 | + s.PatchValue(&machineenvironmentworker.ProxyDirectory, proxyDir) |
65 | + s.PatchValue(&utils.AptConfFile, filepath.Join(proxyDir, "juju-apt-proxy")) |
66 | + |
67 | + s.primeAgent(c, state.JobHostUnits) |
68 | + // Make sure there are some proxy settings to write. |
69 | + oldConfig, err := s.State.EnvironConfig() |
70 | + c.Assert(err, gc.IsNil) |
71 | + |
72 | + proxySettings := osenv.ProxySettings{ |
73 | + Http: "http proxy", |
74 | + Https: "https proxy", |
75 | + Ftp: "ftp proxy", |
76 | + } |
77 | + |
78 | + envConfig, err := oldConfig.Apply(config.ProxyConfigMap(proxySettings)) |
79 | + c.Assert(err, gc.IsNil) |
80 | + |
81 | + err = s.State.SetEnvironConfig(envConfig, oldConfig) |
82 | + c.Assert(err, gc.IsNil) |
83 | + |
84 | + s.assertJobWithAPI(c, state.JobHostUnits, func(conf agent.Config, st *api.State) { |
85 | + for { |
86 | + select { |
87 | + case <-time.After(testing.LongWait): |
88 | + c.Fatalf("timeout while waiting for proxy settings to change") |
89 | + case <-time.After(10 * time.Millisecond): |
90 | + _, err := os.Stat(utils.AptConfFile) |
91 | + if os.IsNotExist(err) { |
92 | + continue |
93 | + } |
94 | + c.Assert(err, gc.IsNil) |
95 | + return |
96 | + } |
97 | + } |
98 | + }) |
99 | +} |
100 | + |
101 | func (s *MachineSuite) TestMachineAgentUninstall(c *gc.C) { |
102 | m, ac, _ := s.primeAgent(c, state.JobHostUnits) |
103 | err := m.EnsureDead() |
104 | |
105 | === modified file 'environs/cloudinit/cloudinit.go' |
106 | --- environs/cloudinit/cloudinit.go 2014-01-26 23:28:43 +0000 |
107 | +++ environs/cloudinit/cloudinit.go 2014-01-29 04:59:50 +0000 |
108 | @@ -250,7 +250,7 @@ |
109 | |
110 | // Write out the apt proxy settings |
111 | if (cfg.AptProxySettings != osenv.ProxySettings{}) { |
112 | - filename := "/etc/apt/apt.conf.d/42-juju-proxy-settings" |
113 | + filename := utils.AptConfFile |
114 | c.AddBootCmd(fmt.Sprintf( |
115 | `[ -f %s ] || (printf '%%s\n' %s > %s)`, |
116 | filename, |
117 | |
118 | === added directory 'state/api/environment' |
119 | === added file 'state/api/environment/environment.go' |
120 | --- state/api/environment/environment.go 1970-01-01 00:00:00 +0000 |
121 | +++ state/api/environment/environment.go 2014-01-29 04:59:50 +0000 |
122 | @@ -0,0 +1,23 @@ |
123 | +// Copyright 2014 Canonical Ltd. |
124 | +// Licensed under the AGPLv3, see LICENCE file for details. |
125 | + |
126 | +package environment |
127 | + |
128 | +import ( |
129 | + "launchpad.net/juju-core/state/api/base" |
130 | + "launchpad.net/juju-core/state/api/common" |
131 | +) |
132 | + |
133 | +const apiName = "Environment" |
134 | + |
135 | +// Facade provides access to a machine environment worker's view of the world. |
136 | +type Facade struct { |
137 | + *common.EnvironWatcher |
138 | +} |
139 | + |
140 | +// NewFacade returns a new api client facade instance. |
141 | +func NewFacade(caller base.Caller) *Facade { |
142 | + return &Facade{ |
143 | + EnvironWatcher: common.NewEnvironWatcher(apiName, caller), |
144 | + } |
145 | +} |
146 | |
147 | === added file 'state/api/environment/environment_test.go' |
148 | --- state/api/environment/environment_test.go 1970-01-01 00:00:00 +0000 |
149 | +++ state/api/environment/environment_test.go 2014-01-29 04:59:50 +0000 |
150 | @@ -0,0 +1,30 @@ |
151 | +// Copyright 2014 Canonical Ltd. |
152 | +// Licensed under the AGPLv3, see LICENCE file for details. |
153 | + |
154 | +package environment_test |
155 | + |
156 | +import ( |
157 | + gc "launchpad.net/gocheck" |
158 | + |
159 | + jujutesting "launchpad.net/juju-core/juju/testing" |
160 | + commontesting "launchpad.net/juju-core/state/api/common/testing" |
161 | +) |
162 | + |
163 | +type environmentSuite struct { |
164 | + jujutesting.JujuConnSuite |
165 | + *commontesting.EnvironWatcherTest |
166 | +} |
167 | + |
168 | +var _ = gc.Suite(&environmentSuite{}) |
169 | + |
170 | +func (s *environmentSuite) SetUpTest(c *gc.C) { |
171 | + s.JujuConnSuite.SetUpTest(c) |
172 | + |
173 | + stateAPI, _ := s.OpenAPIAsNewMachine(c) |
174 | + |
175 | + environmentAPI := stateAPI.Environment() |
176 | + c.Assert(environmentAPI, gc.NotNil) |
177 | + |
178 | + s.EnvironWatcherTest = commontesting.NewEnvironWatcherTest( |
179 | + environmentAPI, s.State, s.BackingState, commontesting.NoSecrets) |
180 | +} |
181 | |
182 | === added file 'state/api/environment/package_test.go' |
183 | --- state/api/environment/package_test.go 1970-01-01 00:00:00 +0000 |
184 | +++ state/api/environment/package_test.go 2014-01-29 04:59:50 +0000 |
185 | @@ -0,0 +1,14 @@ |
186 | +// Copyright 2014 Canonical Ltd. |
187 | +// Licensed under the AGPLv3, see LICENCE file for details. |
188 | + |
189 | +package environment_test |
190 | + |
191 | +import ( |
192 | + stdtesting "testing" |
193 | + |
194 | + "launchpad.net/juju-core/testing" |
195 | +) |
196 | + |
197 | +func TestAll(t *stdtesting.T) { |
198 | + testing.MgoTestPackage(t) |
199 | +} |
200 | |
201 | === modified file 'state/api/state.go' |
202 | --- state/api/state.go 2014-01-22 11:21:21 +0000 |
203 | +++ state/api/state.go 2014-01-29 04:59:50 +0000 |
204 | @@ -7,6 +7,7 @@ |
205 | "launchpad.net/juju-core/state/api/agent" |
206 | "launchpad.net/juju-core/state/api/charmrevisionupdater" |
207 | "launchpad.net/juju-core/state/api/deployer" |
208 | + "launchpad.net/juju-core/state/api/environment" |
209 | "launchpad.net/juju-core/state/api/firewaller" |
210 | "launchpad.net/juju-core/state/api/keyupdater" |
211 | "launchpad.net/juju-core/state/api/logger" |
212 | @@ -79,6 +80,11 @@ |
213 | return deployer.NewState(st) |
214 | } |
215 | |
216 | +// Environment returns access to the Environment API |
217 | +func (st *State) Environment() *environment.Facade { |
218 | + return environment.NewFacade(st) |
219 | +} |
220 | + |
221 | // Logger returns access to the Logger API |
222 | func (st *State) Logger() *logger.State { |
223 | return logger.NewState(st) |
224 | |
225 | === added directory 'state/apiserver/environment' |
226 | === added file 'state/apiserver/environment/environment.go' |
227 | --- state/apiserver/environment/environment.go 1970-01-01 00:00:00 +0000 |
228 | +++ state/apiserver/environment/environment.go 2014-01-29 04:59:50 +0000 |
229 | @@ -0,0 +1,25 @@ |
230 | +// Copyright 2014 Canonical Ltd. |
231 | +// Licensed under the AGPLv3, see LICENCE file for details. |
232 | + |
233 | +package environment |
234 | + |
235 | +import ( |
236 | + "launchpad.net/juju-core/state" |
237 | + "launchpad.net/juju-core/state/apiserver/common" |
238 | +) |
239 | + |
240 | +// EnvironmentAPI implements the API used by the machine environment worker. |
241 | +type EnvironmentAPI struct { |
242 | + *common.EnvironWatcher |
243 | +} |
244 | + |
245 | +// NewEnvironmentAPI creates a new instance of the Environment API. |
246 | +func NewEnvironmentAPI(st *state.State, resources *common.Resources, authorizer common.Authorizer) (*EnvironmentAPI, error) { |
247 | + // Can always watch for environ changes. |
248 | + getCanWatch := common.AuthAlways(true) |
249 | + // Does not get the secrets. |
250 | + getCanReadSecrets := common.AuthAlways(false) |
251 | + return &EnvironmentAPI{ |
252 | + EnvironWatcher: common.NewEnvironWatcher(st, resources, getCanWatch, getCanReadSecrets), |
253 | + }, nil |
254 | +} |
255 | |
256 | === added file 'state/apiserver/environment/environment_test.go' |
257 | --- state/apiserver/environment/environment_test.go 1970-01-01 00:00:00 +0000 |
258 | +++ state/apiserver/environment/environment_test.go 2014-01-29 04:59:50 +0000 |
259 | @@ -0,0 +1,53 @@ |
260 | +// Copyright 2013 Canonical Ltd. |
261 | +// Licensed under the AGPLv3, see LICENCE file for details. |
262 | + |
263 | +package environment_test |
264 | + |
265 | +import ( |
266 | + gc "launchpad.net/gocheck" |
267 | + |
268 | + "launchpad.net/juju-core/juju/testing" |
269 | + "launchpad.net/juju-core/state" |
270 | + "launchpad.net/juju-core/state/apiserver/common" |
271 | + commontesting "launchpad.net/juju-core/state/apiserver/common/testing" |
272 | + "launchpad.net/juju-core/state/apiserver/environment" |
273 | + apiservertesting "launchpad.net/juju-core/state/apiserver/testing" |
274 | +) |
275 | + |
276 | +type environmentSuite struct { |
277 | + testing.JujuConnSuite |
278 | + *commontesting.EnvironWatcherTest |
279 | + |
280 | + authorizer apiservertesting.FakeAuthorizer |
281 | + resources *common.Resources |
282 | + |
283 | + machine0 *state.Machine |
284 | + api *environment.EnvironmentAPI |
285 | +} |
286 | + |
287 | +var _ = gc.Suite(&environmentSuite{}) |
288 | + |
289 | +func (s *environmentSuite) SetUpTest(c *gc.C) { |
290 | + s.JujuConnSuite.SetUpTest(c) |
291 | + |
292 | + var err error |
293 | + s.machine0, err = s.State.AddMachine("quantal", state.JobHostUnits, state.JobManageState, state.JobManageEnviron) |
294 | + c.Assert(err, gc.IsNil) |
295 | + |
296 | + s.authorizer = apiservertesting.FakeAuthorizer{ |
297 | + Tag: s.machine0.Tag(), |
298 | + LoggedIn: true, |
299 | + MachineAgent: true, |
300 | + Entity: s.machine0, |
301 | + } |
302 | + s.resources = common.NewResources() |
303 | + |
304 | + s.api, err = environment.NewEnvironmentAPI( |
305 | + s.State, |
306 | + s.resources, |
307 | + s.authorizer, |
308 | + ) |
309 | + c.Assert(err, gc.IsNil) |
310 | + s.EnvironWatcherTest = commontesting.NewEnvironWatcherTest( |
311 | + s.api, s.State, s.resources, commontesting.NoSecrets) |
312 | +} |
313 | |
314 | === added file 'state/apiserver/environment/package_test.go' |
315 | --- state/apiserver/environment/package_test.go 1970-01-01 00:00:00 +0000 |
316 | +++ state/apiserver/environment/package_test.go 2014-01-29 04:59:50 +0000 |
317 | @@ -0,0 +1,14 @@ |
318 | +// Copyright 2014 Canonical Ltd. |
319 | +// Licensed under the AGPLv3, see LICENCE file for details. |
320 | + |
321 | +package environment_test |
322 | + |
323 | +import ( |
324 | + stdtesting "testing" |
325 | + |
326 | + "launchpad.net/juju-core/testing" |
327 | +) |
328 | + |
329 | +func TestAll(t *stdtesting.T) { |
330 | + testing.MgoTestPackage(t) |
331 | +} |
332 | |
333 | === modified file 'state/apiserver/root.go' |
334 | --- state/apiserver/root.go 2014-01-23 08:58:16 +0000 |
335 | +++ state/apiserver/root.go 2014-01-29 04:59:50 +0000 |
336 | @@ -16,6 +16,7 @@ |
337 | "launchpad.net/juju-core/state/apiserver/client" |
338 | "launchpad.net/juju-core/state/apiserver/common" |
339 | "launchpad.net/juju-core/state/apiserver/deployer" |
340 | + "launchpad.net/juju-core/state/apiserver/environment" |
341 | "launchpad.net/juju-core/state/apiserver/firewaller" |
342 | "launchpad.net/juju-core/state/apiserver/keymanager" |
343 | "launchpad.net/juju-core/state/apiserver/keyupdater" |
344 | @@ -169,6 +170,17 @@ |
345 | return deployer.NewDeployerAPI(r.srv.state, r.resources, r) |
346 | } |
347 | |
348 | +// Environment returns an object that provides access to the Environment API |
349 | +// facade. The id argument is reserved for future use and currently needs to |
350 | +// be empty. |
351 | +func (r *srvRoot) Environment(id string) (*environment.EnvironmentAPI, error) { |
352 | + if id != "" { |
353 | + // Safeguard id for possible future use. |
354 | + return nil, common.ErrBadId |
355 | + } |
356 | + return environment.NewEnvironmentAPI(r.srv.state, r.resources, r) |
357 | +} |
358 | + |
359 | // Logger returns an object that provides access to the Logger API facade. |
360 | // The id argument is reserved for future use and must be empty. |
361 | func (r *srvRoot) Logger(id string) (*loggerapi.LoggerAPI, error) { |
362 | |
363 | === modified file 'utils/apt.go' |
364 | --- utils/apt.go 2014-01-28 04:58:43 +0000 |
365 | +++ utils/apt.go 2014-01-29 04:59:50 +0000 |
366 | @@ -19,6 +19,10 @@ |
367 | var ( |
368 | aptLogger = loggo.GetLogger("juju.utils.apt") |
369 | aptProxyRE = regexp.MustCompile(`(?im)^\s*Acquire::(?P<protocol>[a-z]+)::Proxy\s+"(?P<proxy>[^"]+)";\s*$`) |
370 | + |
371 | + // AptConfFile is the full file path for the proxy settings that are |
372 | + // written by cloud-init and the machine environ worker. |
373 | + AptConfFile = "/etc/apt/apt.conf.d/42-juju-proxy-settings" |
374 | ) |
375 | |
376 | // Some helpful functions for running apt in a sane way |
377 | |
378 | === added directory 'worker/machineenvironmentworker' |
379 | === added file 'worker/machineenvironmentworker/machineenvironmentworker.go' |
380 | --- worker/machineenvironmentworker/machineenvironmentworker.go 1970-01-01 00:00:00 +0000 |
381 | +++ worker/machineenvironmentworker/machineenvironmentworker.go 2014-01-29 04:59:50 +0000 |
382 | @@ -0,0 +1,173 @@ |
383 | +// Copyright 2014 Canonical Ltd. |
384 | +// Licensed under the AGPLv3, see LICENCE file for details. |
385 | + |
386 | +package machineenvironmentworker |
387 | + |
388 | +import ( |
389 | + "fmt" |
390 | + "io/ioutil" |
391 | + "path" |
392 | + |
393 | + "github.com/loggo/loggo" |
394 | + |
395 | + "launchpad.net/juju-core/agent" |
396 | + "launchpad.net/juju-core/juju/osenv" |
397 | + "launchpad.net/juju-core/names" |
398 | + "launchpad.net/juju-core/provider" |
399 | + "launchpad.net/juju-core/state/api/environment" |
400 | + "launchpad.net/juju-core/state/api/watcher" |
401 | + "launchpad.net/juju-core/utils" |
402 | + "launchpad.net/juju-core/utils/exec" |
403 | + "launchpad.net/juju-core/worker" |
404 | +) |
405 | + |
406 | +var ( |
407 | + logger = loggo.GetLogger("juju.worker.machineenvironment") |
408 | + |
409 | + // ProxyDirectory is the directory containing the proxy file that contains |
410 | + // the environment settings for the proxies based on the environment |
411 | + // config values. |
412 | + ProxyDirectory = "/home/ubuntu" |
413 | + |
414 | + // ProxyFile is the name of the file to be stored in the ProxyDirectory. |
415 | + ProxyFile = ".juju-proxy" |
416 | + |
417 | + // Started is a function that is called when the worker has started. |
418 | + Started = func() {} |
419 | +) |
420 | + |
421 | +// MachineEnvironmentWorker is responsible for monitoring the juju environment |
422 | +// configuration and making changes on the physical (or virtual) machine as |
423 | +// necessary to match the environment changes. Examples of these types of |
424 | +// changes are apt proxy configuration and the juju proxies stored in the juju |
425 | +// proxy file. |
426 | +type MachineEnvironmentWorker struct { |
427 | + api *environment.Facade |
428 | + aptProxy osenv.ProxySettings |
429 | + proxy osenv.ProxySettings |
430 | + |
431 | + writeSystemFiles bool |
432 | + // The whole point of the first value is to make sure that the the files |
433 | + // are written out the first time through, even if they are the same as |
434 | + // "last" time, as the initial value for last time is the zeroed struct. |
435 | + // There is the possibility that the files exist on disk with old |
436 | + // settings, and the environment has been updated to now not have them. We |
437 | + // need to make sure that the disk reflects the environment, so the first |
438 | + // time through, even if the proxies are empty, we write the files to |
439 | + // disk. |
440 | + first bool |
441 | +} |
442 | + |
443 | +var _ worker.NotifyWatchHandler = (*MachineEnvironmentWorker)(nil) |
444 | + |
445 | +// NewMachineEnvironmentWorker returns a worker.Worker that uses the notify |
446 | +// watcher returned from the setup. |
447 | +func NewMachineEnvironmentWorker(api *environment.Facade, agentConfig agent.Config) worker.Worker { |
448 | + // We don't write out system files for the local provider on machine zero |
449 | + // as that is the host machine. |
450 | + writeSystemFiles := !(agentConfig.Tag() == names.MachineTag("0") && |
451 | + agentConfig.Value(agent.JujuProviderType) == provider.Local) |
452 | + envWorker := &MachineEnvironmentWorker{ |
453 | + api: api, |
454 | + writeSystemFiles: writeSystemFiles, |
455 | + first: true, |
456 | + } |
457 | + return worker.NewNotifyWorker(envWorker) |
458 | +} |
459 | + |
460 | +func (w *MachineEnvironmentWorker) writeEnvironmentFile() error { |
461 | + // Writing the environment file is handled by executing the script for two |
462 | + // primary reasons: |
463 | + // |
464 | + // 1: In order to have the local provider specify the environment settings |
465 | + // for the machine agent running on the host, this worker needs to run, |
466 | + // but it shouldn't be touching any files on the disk. If however there is |
467 | + // an ubuntu user, it will. This shouldn't be a problem. |
468 | + // |
469 | + // 2: On cloud-instance ubuntu images, the ubuntu user is uid 1000, but in |
470 | + // the situation where the ubuntu user has been created as a part of the |
471 | + // manual provisioning process, the user will exist, and will not have the |
472 | + // same uid/gid as the default cloud image. |
473 | + // |
474 | + // It is easier to shell out to check both these things, and is also the |
475 | + // same way that the file is written in the cloud-init process, so |
476 | + // consistency FTW. |
477 | + filePath := path.Join(ProxyDirectory, ProxyFile) |
478 | + result, err := exec.RunCommands(exec.RunParams{ |
479 | + Commands: fmt.Sprintf( |
480 | + `[ -e %s ] && (printf '%%s\n' %s > %s && chown ubuntu:ubuntu %s)`, |
481 | + ProxyDirectory, |
482 | + utils.ShQuote(w.proxy.AsScriptEnvironment()), |
483 | + filePath, filePath), |
484 | + WorkingDir: ProxyDirectory, |
485 | + }) |
486 | + if err != nil { |
487 | + return err |
488 | + } |
489 | + if result.Code != 0 { |
490 | + logger.Errorf("failed writing new proxy values: \n%s\n%s", result.Stdout, result.Stderr) |
491 | + } |
492 | + return nil |
493 | +} |
494 | + |
495 | +func (w *MachineEnvironmentWorker) handleProxyValues(proxySettings osenv.ProxySettings) { |
496 | + if proxySettings != w.proxy || w.first { |
497 | + logger.Debugf("new proxy settings %#v", proxySettings) |
498 | + w.proxy = proxySettings |
499 | + w.proxy.SetEnvironmentValues() |
500 | + if w.writeSystemFiles { |
501 | + if err := w.writeEnvironmentFile(); err != nil { |
502 | + // It isn't really fatal, but we should record it. |
503 | + logger.Errorf("error writing proxy environment file: %v", err) |
504 | + } |
505 | + } |
506 | + } |
507 | +} |
508 | + |
509 | +func (w *MachineEnvironmentWorker) handleAptProxyValues(aptSettings osenv.ProxySettings) { |
510 | + if w.writeSystemFiles && (aptSettings != w.aptProxy || w.first) { |
511 | + logger.Debugf("new apt proxy settings %#v", aptSettings) |
512 | + w.aptProxy = aptSettings |
513 | + // Always finish with a new line. |
514 | + content := utils.AptProxyContent(w.aptProxy) + "\n" |
515 | + err := ioutil.WriteFile(utils.AptConfFile, []byte(content), 0644) |
516 | + if err != nil { |
517 | + // It isn't really fatal, but we should record it. |
518 | + logger.Errorf("error writing apt proxy config file: %v", err) |
519 | + } |
520 | + } |
521 | +} |
522 | + |
523 | +func (w *MachineEnvironmentWorker) onChange() error { |
524 | + env, err := w.api.EnvironConfig() |
525 | + if err != nil { |
526 | + return err |
527 | + } |
528 | + w.handleProxyValues(env.ProxySettings()) |
529 | + w.handleAptProxyValues(env.AptProxySettings()) |
530 | + return nil |
531 | +} |
532 | + |
533 | +// SetUp is defined on the worker.NotifyWatchHandler interface. |
534 | +func (w *MachineEnvironmentWorker) SetUp() (watcher.NotifyWatcher, error) { |
535 | + // We need to set this up initially as the NotifyWorker sucks up the first |
536 | + // event. |
537 | + err := w.onChange() |
538 | + if err != nil { |
539 | + return nil, err |
540 | + } |
541 | + w.first = false |
542 | + Started() |
543 | + return w.api.WatchForEnvironConfigChanges() |
544 | +} |
545 | + |
546 | +// Handle is defined on the worker.NotifyWatchHandler interface. |
547 | +func (w *MachineEnvironmentWorker) Handle() error { |
548 | + return w.onChange() |
549 | +} |
550 | + |
551 | +// TearDown is defined on the worker.NotifyWatchHandler interface. |
552 | +func (w *MachineEnvironmentWorker) TearDown() error { |
553 | + // Nothing to cleanup, only state is the watcher |
554 | + return nil |
555 | +} |
556 | |
557 | === added file 'worker/machineenvironmentworker/machineenvironmentworker_test.go' |
558 | --- worker/machineenvironmentworker/machineenvironmentworker_test.go 1970-01-01 00:00:00 +0000 |
559 | +++ worker/machineenvironmentworker/machineenvironmentworker_test.go 2014-01-29 04:59:50 +0000 |
560 | @@ -0,0 +1,222 @@ |
561 | +// Copyright 2014 Canonical Ltd. |
562 | +// Licensed under the AGPLv3, see LICENCE file for details. |
563 | + |
564 | +package machineenvironmentworker_test |
565 | + |
566 | +import ( |
567 | + "io/ioutil" |
568 | + "os" |
569 | + "path" |
570 | + "time" |
571 | + |
572 | + gc "launchpad.net/gocheck" |
573 | + |
574 | + "launchpad.net/juju-core/agent" |
575 | + "launchpad.net/juju-core/environs/config" |
576 | + "launchpad.net/juju-core/juju/osenv" |
577 | + jujutesting "launchpad.net/juju-core/juju/testing" |
578 | + "launchpad.net/juju-core/names" |
579 | + "launchpad.net/juju-core/provider" |
580 | + "launchpad.net/juju-core/state" |
581 | + "launchpad.net/juju-core/state/api" |
582 | + "launchpad.net/juju-core/state/api/environment" |
583 | + "launchpad.net/juju-core/testing" |
584 | + jc "launchpad.net/juju-core/testing/checkers" |
585 | + "launchpad.net/juju-core/utils" |
586 | + "launchpad.net/juju-core/worker" |
587 | + "launchpad.net/juju-core/worker/machineenvironmentworker" |
588 | +) |
589 | + |
590 | +type MachineEnvironmentWatcherSuite struct { |
591 | + jujutesting.JujuConnSuite |
592 | + |
593 | + apiRoot *api.State |
594 | + environmentAPI *environment.Facade |
595 | + machine *state.Machine |
596 | + |
597 | + proxyFile string |
598 | + started bool |
599 | +} |
600 | + |
601 | +var _ = gc.Suite(&MachineEnvironmentWatcherSuite{}) |
602 | + |
603 | +func (s *MachineEnvironmentWatcherSuite) setStarted() { |
604 | + s.started = true |
605 | +} |
606 | + |
607 | +func (s *MachineEnvironmentWatcherSuite) SetUpTest(c *gc.C) { |
608 | + s.JujuConnSuite.SetUpTest(c) |
609 | + s.apiRoot, s.machine = s.OpenAPIAsNewMachine(c) |
610 | + // Create the machiner API facade. |
611 | + s.environmentAPI = s.apiRoot.Environment() |
612 | + c.Assert(s.environmentAPI, gc.NotNil) |
613 | + |
614 | + proxyDir := c.MkDir() |
615 | + s.PatchValue(&machineenvironmentworker.ProxyDirectory, proxyDir) |
616 | + s.started = false |
617 | + s.PatchValue(&machineenvironmentworker.Started, s.setStarted) |
618 | + s.PatchValue(&utils.AptConfFile, path.Join(proxyDir, "juju-apt-proxy")) |
619 | + s.proxyFile = path.Join(proxyDir, machineenvironmentworker.ProxyFile) |
620 | +} |
621 | + |
622 | +func (s *MachineEnvironmentWatcherSuite) waitForPostSetup(c *gc.C) { |
623 | + for { |
624 | + select { |
625 | + case <-time.After(testing.LongWait): |
626 | + c.Fatalf("timeout while waiting for setup") |
627 | + case <-time.After(10 * time.Millisecond): |
628 | + if s.started { |
629 | + return |
630 | + } |
631 | + } |
632 | + } |
633 | +} |
634 | + |
635 | +func (s *MachineEnvironmentWatcherSuite) waitProxySettings(c *gc.C, expected osenv.ProxySettings) { |
636 | + for { |
637 | + select { |
638 | + case <-time.After(testing.LongWait): |
639 | + c.Fatalf("timeout while waiting for proxy settings to change") |
640 | + case <-time.After(10 * time.Millisecond): |
641 | + obtained := osenv.DetectProxies() |
642 | + if obtained != expected { |
643 | + c.Logf("proxy settings are %#v, still waiting", obtained) |
644 | + continue |
645 | + } |
646 | + return |
647 | + } |
648 | + } |
649 | +} |
650 | + |
651 | +func (s *MachineEnvironmentWatcherSuite) waitForFile(c *gc.C, filename, expected string) { |
652 | + for { |
653 | + select { |
654 | + case <-time.After(testing.LongWait): |
655 | + c.Fatalf("timeout while waiting for proxy settings to change") |
656 | + case <-time.After(10 * time.Millisecond): |
657 | + fileContent, err := ioutil.ReadFile(filename) |
658 | + if os.IsNotExist(err) { |
659 | + continue |
660 | + } |
661 | + c.Assert(err, gc.IsNil) |
662 | + if string(fileContent) != expected { |
663 | + c.Logf("file content not matching, still waiting") |
664 | + continue |
665 | + } |
666 | + return |
667 | + } |
668 | + } |
669 | +} |
670 | + |
671 | +func (s *MachineEnvironmentWatcherSuite) makeWorker(c *gc.C, agentConfig agent.Config) worker.Worker { |
672 | + return machineenvironmentworker.NewMachineEnvironmentWorker(s.environmentAPI, agentConfig) |
673 | +} |
674 | + |
675 | +func (s *MachineEnvironmentWatcherSuite) TestRunStop(c *gc.C) { |
676 | + agentConfig := agentConfig("0", "ec2") |
677 | + envWorker := s.makeWorker(c, agentConfig) |
678 | + c.Assert(worker.Stop(envWorker), gc.IsNil) |
679 | +} |
680 | + |
681 | +func (s *MachineEnvironmentWatcherSuite) updateConfig(c *gc.C) (osenv.ProxySettings, osenv.ProxySettings) { |
682 | + oldConfig, err := s.State.EnvironConfig() |
683 | + c.Assert(err, gc.IsNil) |
684 | + |
685 | + proxySettings := osenv.ProxySettings{ |
686 | + Http: "http proxy", |
687 | + Https: "https proxy", |
688 | + Ftp: "ftp proxy", |
689 | + } |
690 | + |
691 | + envConfig, err := oldConfig.Apply(config.ProxyConfigMap(proxySettings)) |
692 | + c.Assert(err, gc.IsNil) |
693 | + |
694 | + // We explicitly set apt proxy settings as well to show that it is the apt |
695 | + // settings that are used for the apt config, and not just the normal |
696 | + // proxy settings which is what we would get if we don't explicitly set |
697 | + // apt values. |
698 | + aptProxySettings := osenv.ProxySettings{ |
699 | + Http: "apt http proxy", |
700 | + Https: "apt https proxy", |
701 | + Ftp: "apt ftp proxy", |
702 | + } |
703 | + envConfig, err = envConfig.Apply(config.AptProxyConfigMap(aptProxySettings)) |
704 | + c.Assert(err, gc.IsNil) |
705 | + |
706 | + err = s.State.SetEnvironConfig(envConfig, oldConfig) |
707 | + c.Assert(err, gc.IsNil) |
708 | + |
709 | + return proxySettings, aptProxySettings |
710 | +} |
711 | + |
712 | +func (s *MachineEnvironmentWatcherSuite) TestInitialState(c *gc.C) { |
713 | + proxySettings, aptProxySettings := s.updateConfig(c) |
714 | + |
715 | + agentConfig := agentConfig("0", "ec2") |
716 | + envWorker := s.makeWorker(c, agentConfig) |
717 | + defer worker.Stop(envWorker) |
718 | + |
719 | + s.waitProxySettings(c, proxySettings) |
720 | + s.waitForFile(c, s.proxyFile, proxySettings.AsScriptEnvironment()+"\n") |
721 | + s.waitForFile(c, utils.AptConfFile, utils.AptProxyContent(aptProxySettings)+"\n") |
722 | +} |
723 | + |
724 | +func (s *MachineEnvironmentWatcherSuite) TestRespondsToEvents(c *gc.C) { |
725 | + agentConfig := agentConfig("0", "ec2") |
726 | + envWorker := s.makeWorker(c, agentConfig) |
727 | + defer worker.Stop(envWorker) |
728 | + s.waitForPostSetup(c) |
729 | + |
730 | + proxySettings, aptProxySettings := s.updateConfig(c) |
731 | + |
732 | + s.waitProxySettings(c, proxySettings) |
733 | + s.waitForFile(c, s.proxyFile, proxySettings.AsScriptEnvironment()+"\n") |
734 | + s.waitForFile(c, utils.AptConfFile, utils.AptProxyContent(aptProxySettings)+"\n") |
735 | +} |
736 | + |
737 | +func (s *MachineEnvironmentWatcherSuite) TestInitialStateLocalMachine1(c *gc.C) { |
738 | + proxySettings, aptProxySettings := s.updateConfig(c) |
739 | + |
740 | + agentConfig := agentConfig("1", provider.Local) |
741 | + envWorker := s.makeWorker(c, agentConfig) |
742 | + defer worker.Stop(envWorker) |
743 | + |
744 | + s.waitProxySettings(c, proxySettings) |
745 | + s.waitForFile(c, s.proxyFile, proxySettings.AsScriptEnvironment()+"\n") |
746 | + s.waitForFile(c, utils.AptConfFile, utils.AptProxyContent(aptProxySettings)+"\n") |
747 | +} |
748 | + |
749 | +func (s *MachineEnvironmentWatcherSuite) TestInitialStateLocalMachine0(c *gc.C) { |
750 | + proxySettings, _ := s.updateConfig(c) |
751 | + |
752 | + agentConfig := agentConfig("0", provider.Local) |
753 | + envWorker := s.makeWorker(c, agentConfig) |
754 | + defer worker.Stop(envWorker) |
755 | + s.waitForPostSetup(c) |
756 | + |
757 | + s.waitProxySettings(c, proxySettings) |
758 | + |
759 | + c.Assert(utils.AptConfFile, jc.DoesNotExist) |
760 | + c.Assert(s.proxyFile, jc.DoesNotExist) |
761 | +} |
762 | + |
763 | +type mockConfig struct { |
764 | + agent.Config |
765 | + tag string |
766 | + provider string |
767 | +} |
768 | + |
769 | +func (mock *mockConfig) Tag() string { |
770 | + return mock.tag |
771 | +} |
772 | + |
773 | +func (mock *mockConfig) Value(key string) string { |
774 | + if key == agent.JujuProviderType { |
775 | + return mock.provider |
776 | + } |
777 | + return "" |
778 | +} |
779 | + |
780 | +func agentConfig(machineId, provider string) *mockConfig { |
781 | + return &mockConfig{tag: names.MachineTag(machineId), provider: provider} |
782 | +} |
783 | |
784 | === added file 'worker/machineenvironmentworker/package_test.go' |
785 | --- worker/machineenvironmentworker/package_test.go 1970-01-01 00:00:00 +0000 |
786 | +++ worker/machineenvironmentworker/package_test.go 2014-01-29 04:59:50 +0000 |
787 | @@ -0,0 +1,14 @@ |
788 | +// Copyright 2014 Canonical Ltd. |
789 | +// Licensed under the AGPLv3, see LICENCE file for details. |
790 | + |
791 | +package machineenvironmentworker_test |
792 | + |
793 | +import ( |
794 | + stdtesting "testing" |
795 | + |
796 | + "launchpad.net/juju-core/testing" |
797 | +) |
798 | + |
799 | +func TestAll(t *stdtesting.T) { |
800 | + testing.MgoTestPackage(t) |
801 | +} |
Reviewers: mp+203460_ code.launchpad. net,
Message:
Please take a look.
Description:
Add a worker to keep disk files up to date
Added the machine environment worker which watches
the environment config and will write changes to the
disk as needed.
A new end point was added on the client and server side
that is just called "Environment", which provies the common
environment watcher.
The only gotcha in this is that if the worker is running on
machine 0 for a local provider, it shouldn't attempt to modify
system files. There are tests for this.
The two files on disk that are currently being kept up ubuntu/ .juju-proxy apt/apt. conf.d/ 42-juju- proxy-settings
to date with this are:
/home/
/etc/
https:/ /code.launchpad .net/~thumper/ juju-core/ machine- worker/ +merge/ 203460
Requires: /code.launchpad .net/~thumper/ juju-core/ fix-uniter- proxy-env- vars/+merge/ 203459
https:/
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/57600043/
Affected files (+580, -1 lines): machine. go cloudinit/ cloudinit. go environment/ environment. go environment/ environment_ test.go environment/ package_ test.go /environment/ environment. go /environment/ environment_ test.go /environment/ package_ test.go /root.go machineenvironm entworker/ machineenvironm entworker. go machineenvironm entworker/ machineenvironm entworker_ test.go machineenvironm entworker/ package_ test.go
A [revision details]
M cmd/jujud/
M environs/
A state/api/
A state/api/
A state/api/
M state/api/state.go
A state/apiserver
A state/apiserver
A state/apiserver
M state/apiserver
M utils/apt.go
A worker/
A worker/
A worker/