Merge lp:~thumper/juju-core/machine-worker into lp:~go-bot/juju-core/trunk

Proposed by Tim Penhey
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
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/ubuntu/.juju-proxy
 /etc/apt/apt.conf.d/42-juju-proxy-settings

https://codereview.appspot.com/57600043/

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/ubuntu/.juju-proxy
 /etc/apt/apt.conf.d/42-juju-proxy-settings

https://codereview.appspot.com/57600043/

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

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
to date with this are:
  /home/ubuntu/.juju-proxy
  /etc/apt/apt.conf.d/42-juju-proxy-settings

https://code.launchpad.net/~thumper/juju-core/machine-worker/+merge/203460

Requires:
https://code.launchpad.net/~thumper/juju-core/fix-uniter-proxy-env-vars/+merge/203459

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/57600043/

Affected files (+580, -1 lines):
   A [revision details]
   M cmd/jujud/machine.go
   M environs/cloudinit/cloudinit.go
   A state/api/environment/environment.go
   A state/api/environment/environment_test.go
   A state/api/environment/package_test.go
   M state/api/state.go
   A state/apiserver/environment/environment.go
   A state/apiserver/environment/environment_test.go
   A state/apiserver/environment/package_test.go
   M state/apiserver/root.go
   M utils/apt.go
   A worker/machineenvironmentworker/machineenvironmentworker.go
   A worker/machineenvironmentworker/machineenvironmentworker_test.go
   A worker/machineenvironmentworker/package_test.go

Revision history for this message
Ian Booth (wallyworld) wrote :
Download full text (3.6 KiB)

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://codereview.appspot.com/57600043/diff/1/worker/machineenvironmentworker/machineenvironmentworker.go
File worker/machineenvironmentworker/machineenvironmentworker.go
(right):

https://codereview.appspot.com/57600043/diff/1/worker/machineenvironmentworker/machineenvironmentworker.go#newcode30
worker/machineenvironmentworker/machineenvironmentworker.go:30:
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://codereview.appspot.com/57600043/diff/1/worker/machineenvironmentworker/machineenvironmentworker.go#newcode50
worker/machineenvironmentworker/machineenvironmentworker.go:50: first
         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://codereview.appspot.com/57600043/diff/1/worker/machineenvironmentworker/machineenvironmentworker.go#newcode77
worker/machineenvironmentworker/machineenvironmentworker.go:77: // an
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://codereview.appspot.com/57600043/diff/1/worker/machineenvironmentworker/machineenvironmentworker.go#newcode85
worker/machineenvironmentworker/machineenvironmentworker.go:85: // same
way that the file is writting in the cloud-init process, so
s/writting/written ?

https://codereview.appspot.com/57600043/diff/1/worker/machineenvironmentworker/machineenvironmentworker.go#newcode122
worker/machineenvironmentworker/machineenvironmentworker.go:122:
aptSettings := env.AptProxySettings()
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://codereview.appspot.com/57600043/diff/1/worker/machineenvironmentworker/machineenvironmentworker.go#newcode137
worker/machineenvironmentworker/machineenvironmentworker.go:137: func (w
*MachineEnvironmentWorker) SetUp() (watcher.NotifyWatcher, error) {
Typically we add a doc string saying what interface SetUp() comes from

https://codereview.appspot.com/57600043/diff/1/worker/machineenvironmentworker/machineenvironmentworker.go#newcode149
worker/machineenvironmentworker/machineenvironmentworker.go:149: func (w
*MachineEnvironmentWorker) Handle() error {
ditto

https://codereview.appspot.com/57600043/diff/1/worker/machineenvironmentworker/machineenvironmentworker.go#newcode153
worker/machineenvironmentworker/machineenvironmentworker.go:153: func (w
*MachineEnvironmentWorker) TearDown() error {
ditto

https://codereview.appspot.com/57600043/diff/1/worker/machineenvironmentworker/machineenvironmentworker_test.go
File worker/machineenvironmentworker/machineenvironmentworker_test.go
(right):

https://codereview.appspot.com/57600043/diff/1/worker/machineenvironmentworker/machineenvironmentworker_test.go#newcode41
worker/ma...

Read more...

Revision history for this message
Tim Penhey (thumper) wrote :
Download full text (4.7 KiB)

Please take a look.

https://codereview.appspot.com/57600043/diff/1/worker/machineenvironmentworker/machineenvironmentworker.go
File worker/machineenvironmentworker/machineenvironmentworker.go
(right):

https://codereview.appspot.com/57600043/diff/1/worker/machineenvironmentworker/machineenvironmentworker.go#newcode30
worker/machineenvironmentworker/machineenvironmentworker.go:30:
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://codereview.appspot.com/57600043/diff/1/worker/machineenvironmentworker/machineenvironmentworker.go#newcode50
worker/machineenvironmentworker/machineenvironmentworker.go:50: first
         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://codereview.appspot.com/57600043/diff/1/worker/machineenvironmentworker/machineenvironmentworker.go#newcode77
worker/machineenvironmentworker/machineenvironmentworker.go:77: // an
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://codereview.appspot.com/57600043/diff/1/worker/machineenvironmentworker/machineenvironmentworker.go#newcode85
worker/machineenvironmentworker/machineenvironmentworker.go:85: // same
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://codereview.appspot.com/57600043/diff/1/worker/machineenvironmentworker/machineenvironmentworker.go#newcode122
worker/machineenvironmentworker/machineenvironmentworker.go:122:
aptSettings := env.AptProxySettings()
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://codereview.appspot.com/57600043/diff/1/worker/machineenvironmentworker/machineenvironmentworker.go#newcode137
worker/machineenvironmentworker/machineenvironmentworker.go:137: func (w
*MachineEnvironmentWorker) SetUp() (watcher.NotifyWatcher, error) {
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.

https://codereview.appspot.com/57600043/diff/1/worker/machineenvironmentworker/machineenvironmentworker.go#...

Read more...

Revision history for this message
Ian Booth (wallyworld) wrote :

LGTM, thanks for the extra code comments, especially for first. Makes it
much clearer for the casual reader.

https://codereview.appspot.com/57600043/diff/20001/cmd/jujud/machine_test.go
File cmd/jujud/machine_test.go (right):

https://codereview.appspot.com/57600043/diff/20001/cmd/jujud/machine_test.go#newcode80
cmd/jujud/machine_test.go:80: s.PatchValue(&utils.AptConfFile,
filepath.Join(s.proxyDir, "juju-apt-proxy"))
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.

https://codereview.appspot.com/57600043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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+}

Subscribers

People subscribed via source and target branches

to status/vote changes: