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
=== modified file 'cmd/jujud/machine.go'
--- cmd/jujud/machine.go 2014-01-28 04:58:43 +0000
+++ cmd/jujud/machine.go 2014-01-29 04:59:50 +0000
@@ -37,6 +37,7 @@
37 "launchpad.net/juju-core/worker/instancepoller"37 "launchpad.net/juju-core/worker/instancepoller"
38 "launchpad.net/juju-core/worker/localstorage"38 "launchpad.net/juju-core/worker/localstorage"
39 workerlogger "launchpad.net/juju-core/worker/logger"39 workerlogger "launchpad.net/juju-core/worker/logger"
40 "launchpad.net/juju-core/worker/machineenvironmentworker"
40 "launchpad.net/juju-core/worker/machiner"41 "launchpad.net/juju-core/worker/machiner"
41 "launchpad.net/juju-core/worker/minunitsworker"42 "launchpad.net/juju-core/worker/minunitsworker"
42 "launchpad.net/juju-core/worker/provisioner"43 "launchpad.net/juju-core/worker/provisioner"
@@ -183,6 +184,9 @@
183 runner.StartWorker("logger", func() (worker.Worker, error) {184 runner.StartWorker("logger", func() (worker.Worker, error) {
184 return workerlogger.NewLogger(st.Logger(), agentConfig), nil185 return workerlogger.NewLogger(st.Logger(), agentConfig), nil
185 })186 })
187 runner.StartWorker("machineenvironmentworker", func() (worker.Worker, error) {
188 return machineenvironmentworker.NewMachineEnvironmentWorker(st.Environment(), agentConfig), nil
189 })
186190
187 // If not a local provider bootstrap machine, start the worker to manage SSH keys.191 // If not a local provider bootstrap machine, start the worker to manage SSH keys.
188 providerType := agentConfig.Value(agent.ProviderType)192 providerType := agentConfig.Value(agent.ProviderType)
189193
=== modified file 'cmd/jujud/machine_test.go'
--- cmd/jujud/machine_test.go 2014-01-24 14:52:58 +0000
+++ cmd/jujud/machine_test.go 2014-01-29 04:59:50 +0000
@@ -16,10 +16,12 @@
16 "launchpad.net/juju-core/charm"16 "launchpad.net/juju-core/charm"
17 "launchpad.net/juju-core/cmd"17 "launchpad.net/juju-core/cmd"
18 lxctesting "launchpad.net/juju-core/container/lxc/testing"18 lxctesting "launchpad.net/juju-core/container/lxc/testing"
19 "launchpad.net/juju-core/environs/config"
19 envtesting "launchpad.net/juju-core/environs/testing"20 envtesting "launchpad.net/juju-core/environs/testing"
20 "launchpad.net/juju-core/errors"21 "launchpad.net/juju-core/errors"
21 "launchpad.net/juju-core/instance"22 "launchpad.net/juju-core/instance"
22 "launchpad.net/juju-core/juju"23 "launchpad.net/juju-core/juju"
24 "launchpad.net/juju-core/juju/osenv"
23 "launchpad.net/juju-core/names"25 "launchpad.net/juju-core/names"
24 "launchpad.net/juju-core/provider/dummy"26 "launchpad.net/juju-core/provider/dummy"
25 "launchpad.net/juju-core/state"27 "launchpad.net/juju-core/state"
@@ -30,15 +32,18 @@
30 statetesting "launchpad.net/juju-core/state/testing"32 statetesting "launchpad.net/juju-core/state/testing"
31 "launchpad.net/juju-core/state/watcher"33 "launchpad.net/juju-core/state/watcher"
32 coretesting "launchpad.net/juju-core/testing"34 coretesting "launchpad.net/juju-core/testing"
35 "launchpad.net/juju-core/testing"
33 jc "launchpad.net/juju-core/testing/checkers"36 jc "launchpad.net/juju-core/testing/checkers"
34 "launchpad.net/juju-core/testing/testbase"37 "launchpad.net/juju-core/testing/testbase"
35 "launchpad.net/juju-core/tools"38 "launchpad.net/juju-core/tools"
39 "launchpad.net/juju-core/utils"
36 "launchpad.net/juju-core/utils/ssh"40 "launchpad.net/juju-core/utils/ssh"
37 sshtesting "launchpad.net/juju-core/utils/ssh/testing"41 sshtesting "launchpad.net/juju-core/utils/ssh/testing"
38 "launchpad.net/juju-core/version"42 "launchpad.net/juju-core/version"
39 "launchpad.net/juju-core/worker/authenticationworker"43 "launchpad.net/juju-core/worker/authenticationworker"
40 "launchpad.net/juju-core/worker/deployer"44 "launchpad.net/juju-core/worker/deployer"
41 "launchpad.net/juju-core/worker/instancepoller"45 "launchpad.net/juju-core/worker/instancepoller"
46 "launchpad.net/juju-core/worker/machineenvironmentworker"
42)47)
4348
44type MachineSuite struct {49type MachineSuite struct {
@@ -674,6 +679,45 @@
674 })679 })
675}680}
676681
682func (s *MachineSuite) TestMachineEnvirnWorker(c *gc.C) {
683 proxyDir := c.MkDir()
684 s.PatchValue(&machineenvironmentworker.ProxyDirectory, proxyDir)
685 s.PatchValue(&utils.AptConfFile, filepath.Join(proxyDir, "juju-apt-proxy"))
686
687 s.primeAgent(c, state.JobHostUnits)
688 // Make sure there are some proxy settings to write.
689 oldConfig, err := s.State.EnvironConfig()
690 c.Assert(err, gc.IsNil)
691
692 proxySettings := osenv.ProxySettings{
693 Http: "http proxy",
694 Https: "https proxy",
695 Ftp: "ftp proxy",
696 }
697
698 envConfig, err := oldConfig.Apply(config.ProxyConfigMap(proxySettings))
699 c.Assert(err, gc.IsNil)
700
701 err = s.State.SetEnvironConfig(envConfig, oldConfig)
702 c.Assert(err, gc.IsNil)
703
704 s.assertJobWithAPI(c, state.JobHostUnits, func(conf agent.Config, st *api.State) {
705 for {
706 select {
707 case <-time.After(testing.LongWait):
708 c.Fatalf("timeout while waiting for proxy settings to change")
709 case <-time.After(10 * time.Millisecond):
710 _, err := os.Stat(utils.AptConfFile)
711 if os.IsNotExist(err) {
712 continue
713 }
714 c.Assert(err, gc.IsNil)
715 return
716 }
717 }
718 })
719}
720
677func (s *MachineSuite) TestMachineAgentUninstall(c *gc.C) {721func (s *MachineSuite) TestMachineAgentUninstall(c *gc.C) {
678 m, ac, _ := s.primeAgent(c, state.JobHostUnits)722 m, ac, _ := s.primeAgent(c, state.JobHostUnits)
679 err := m.EnsureDead()723 err := m.EnsureDead()
680724
=== modified file 'environs/cloudinit/cloudinit.go'
--- environs/cloudinit/cloudinit.go 2014-01-26 23:28:43 +0000
+++ environs/cloudinit/cloudinit.go 2014-01-29 04:59:50 +0000
@@ -250,7 +250,7 @@
250250
251 // Write out the apt proxy settings251 // Write out the apt proxy settings
252 if (cfg.AptProxySettings != osenv.ProxySettings{}) {252 if (cfg.AptProxySettings != osenv.ProxySettings{}) {
253 filename := "/etc/apt/apt.conf.d/42-juju-proxy-settings"253 filename := utils.AptConfFile
254 c.AddBootCmd(fmt.Sprintf(254 c.AddBootCmd(fmt.Sprintf(
255 `[ -f %s ] || (printf '%%s\n' %s > %s)`,255 `[ -f %s ] || (printf '%%s\n' %s > %s)`,
256 filename,256 filename,
257257
=== added directory 'state/api/environment'
=== added file 'state/api/environment/environment.go'
--- state/api/environment/environment.go 1970-01-01 00:00:00 +0000
+++ state/api/environment/environment.go 2014-01-29 04:59:50 +0000
@@ -0,0 +1,23 @@
1// Copyright 2014 Canonical Ltd.
2// Licensed under the AGPLv3, see LICENCE file for details.
3
4package environment
5
6import (
7 "launchpad.net/juju-core/state/api/base"
8 "launchpad.net/juju-core/state/api/common"
9)
10
11const apiName = "Environment"
12
13// Facade provides access to a machine environment worker's view of the world.
14type Facade struct {
15 *common.EnvironWatcher
16}
17
18// NewFacade returns a new api client facade instance.
19func NewFacade(caller base.Caller) *Facade {
20 return &Facade{
21 EnvironWatcher: common.NewEnvironWatcher(apiName, caller),
22 }
23}
024
=== added file 'state/api/environment/environment_test.go'
--- state/api/environment/environment_test.go 1970-01-01 00:00:00 +0000
+++ state/api/environment/environment_test.go 2014-01-29 04:59:50 +0000
@@ -0,0 +1,30 @@
1// Copyright 2014 Canonical Ltd.
2// Licensed under the AGPLv3, see LICENCE file for details.
3
4package environment_test
5
6import (
7 gc "launchpad.net/gocheck"
8
9 jujutesting "launchpad.net/juju-core/juju/testing"
10 commontesting "launchpad.net/juju-core/state/api/common/testing"
11)
12
13type environmentSuite struct {
14 jujutesting.JujuConnSuite
15 *commontesting.EnvironWatcherTest
16}
17
18var _ = gc.Suite(&environmentSuite{})
19
20func (s *environmentSuite) SetUpTest(c *gc.C) {
21 s.JujuConnSuite.SetUpTest(c)
22
23 stateAPI, _ := s.OpenAPIAsNewMachine(c)
24
25 environmentAPI := stateAPI.Environment()
26 c.Assert(environmentAPI, gc.NotNil)
27
28 s.EnvironWatcherTest = commontesting.NewEnvironWatcherTest(
29 environmentAPI, s.State, s.BackingState, commontesting.NoSecrets)
30}
031
=== added file 'state/api/environment/package_test.go'
--- state/api/environment/package_test.go 1970-01-01 00:00:00 +0000
+++ state/api/environment/package_test.go 2014-01-29 04:59:50 +0000
@@ -0,0 +1,14 @@
1// Copyright 2014 Canonical Ltd.
2// Licensed under the AGPLv3, see LICENCE file for details.
3
4package environment_test
5
6import (
7 stdtesting "testing"
8
9 "launchpad.net/juju-core/testing"
10)
11
12func TestAll(t *stdtesting.T) {
13 testing.MgoTestPackage(t)
14}
015
=== modified file 'state/api/state.go'
--- state/api/state.go 2014-01-22 11:21:21 +0000
+++ state/api/state.go 2014-01-29 04:59:50 +0000
@@ -7,6 +7,7 @@
7 "launchpad.net/juju-core/state/api/agent"7 "launchpad.net/juju-core/state/api/agent"
8 "launchpad.net/juju-core/state/api/charmrevisionupdater"8 "launchpad.net/juju-core/state/api/charmrevisionupdater"
9 "launchpad.net/juju-core/state/api/deployer"9 "launchpad.net/juju-core/state/api/deployer"
10 "launchpad.net/juju-core/state/api/environment"
10 "launchpad.net/juju-core/state/api/firewaller"11 "launchpad.net/juju-core/state/api/firewaller"
11 "launchpad.net/juju-core/state/api/keyupdater"12 "launchpad.net/juju-core/state/api/keyupdater"
12 "launchpad.net/juju-core/state/api/logger"13 "launchpad.net/juju-core/state/api/logger"
@@ -79,6 +80,11 @@
79 return deployer.NewState(st)80 return deployer.NewState(st)
80}81}
8182
83// Environment returns access to the Environment API
84func (st *State) Environment() *environment.Facade {
85 return environment.NewFacade(st)
86}
87
82// Logger returns access to the Logger API88// Logger returns access to the Logger API
83func (st *State) Logger() *logger.State {89func (st *State) Logger() *logger.State {
84 return logger.NewState(st)90 return logger.NewState(st)
8591
=== added directory 'state/apiserver/environment'
=== added file 'state/apiserver/environment/environment.go'
--- state/apiserver/environment/environment.go 1970-01-01 00:00:00 +0000
+++ state/apiserver/environment/environment.go 2014-01-29 04:59:50 +0000
@@ -0,0 +1,25 @@
1// Copyright 2014 Canonical Ltd.
2// Licensed under the AGPLv3, see LICENCE file for details.
3
4package environment
5
6import (
7 "launchpad.net/juju-core/state"
8 "launchpad.net/juju-core/state/apiserver/common"
9)
10
11// EnvironmentAPI implements the API used by the machine environment worker.
12type EnvironmentAPI struct {
13 *common.EnvironWatcher
14}
15
16// NewEnvironmentAPI creates a new instance of the Environment API.
17func NewEnvironmentAPI(st *state.State, resources *common.Resources, authorizer common.Authorizer) (*EnvironmentAPI, error) {
18 // Can always watch for environ changes.
19 getCanWatch := common.AuthAlways(true)
20 // Does not get the secrets.
21 getCanReadSecrets := common.AuthAlways(false)
22 return &EnvironmentAPI{
23 EnvironWatcher: common.NewEnvironWatcher(st, resources, getCanWatch, getCanReadSecrets),
24 }, nil
25}
026
=== added file 'state/apiserver/environment/environment_test.go'
--- state/apiserver/environment/environment_test.go 1970-01-01 00:00:00 +0000
+++ state/apiserver/environment/environment_test.go 2014-01-29 04:59:50 +0000
@@ -0,0 +1,53 @@
1// Copyright 2013 Canonical Ltd.
2// Licensed under the AGPLv3, see LICENCE file for details.
3
4package environment_test
5
6import (
7 gc "launchpad.net/gocheck"
8
9 "launchpad.net/juju-core/juju/testing"
10 "launchpad.net/juju-core/state"
11 "launchpad.net/juju-core/state/apiserver/common"
12 commontesting "launchpad.net/juju-core/state/apiserver/common/testing"
13 "launchpad.net/juju-core/state/apiserver/environment"
14 apiservertesting "launchpad.net/juju-core/state/apiserver/testing"
15)
16
17type environmentSuite struct {
18 testing.JujuConnSuite
19 *commontesting.EnvironWatcherTest
20
21 authorizer apiservertesting.FakeAuthorizer
22 resources *common.Resources
23
24 machine0 *state.Machine
25 api *environment.EnvironmentAPI
26}
27
28var _ = gc.Suite(&environmentSuite{})
29
30func (s *environmentSuite) SetUpTest(c *gc.C) {
31 s.JujuConnSuite.SetUpTest(c)
32
33 var err error
34 s.machine0, err = s.State.AddMachine("quantal", state.JobHostUnits, state.JobManageState, state.JobManageEnviron)
35 c.Assert(err, gc.IsNil)
36
37 s.authorizer = apiservertesting.FakeAuthorizer{
38 Tag: s.machine0.Tag(),
39 LoggedIn: true,
40 MachineAgent: true,
41 Entity: s.machine0,
42 }
43 s.resources = common.NewResources()
44
45 s.api, err = environment.NewEnvironmentAPI(
46 s.State,
47 s.resources,
48 s.authorizer,
49 )
50 c.Assert(err, gc.IsNil)
51 s.EnvironWatcherTest = commontesting.NewEnvironWatcherTest(
52 s.api, s.State, s.resources, commontesting.NoSecrets)
53}
054
=== added file 'state/apiserver/environment/package_test.go'
--- state/apiserver/environment/package_test.go 1970-01-01 00:00:00 +0000
+++ state/apiserver/environment/package_test.go 2014-01-29 04:59:50 +0000
@@ -0,0 +1,14 @@
1// Copyright 2014 Canonical Ltd.
2// Licensed under the AGPLv3, see LICENCE file for details.
3
4package environment_test
5
6import (
7 stdtesting "testing"
8
9 "launchpad.net/juju-core/testing"
10)
11
12func TestAll(t *stdtesting.T) {
13 testing.MgoTestPackage(t)
14}
015
=== modified file 'state/apiserver/root.go'
--- state/apiserver/root.go 2014-01-23 08:58:16 +0000
+++ state/apiserver/root.go 2014-01-29 04:59:50 +0000
@@ -16,6 +16,7 @@
16 "launchpad.net/juju-core/state/apiserver/client"16 "launchpad.net/juju-core/state/apiserver/client"
17 "launchpad.net/juju-core/state/apiserver/common"17 "launchpad.net/juju-core/state/apiserver/common"
18 "launchpad.net/juju-core/state/apiserver/deployer"18 "launchpad.net/juju-core/state/apiserver/deployer"
19 "launchpad.net/juju-core/state/apiserver/environment"
19 "launchpad.net/juju-core/state/apiserver/firewaller"20 "launchpad.net/juju-core/state/apiserver/firewaller"
20 "launchpad.net/juju-core/state/apiserver/keymanager"21 "launchpad.net/juju-core/state/apiserver/keymanager"
21 "launchpad.net/juju-core/state/apiserver/keyupdater"22 "launchpad.net/juju-core/state/apiserver/keyupdater"
@@ -169,6 +170,17 @@
169 return deployer.NewDeployerAPI(r.srv.state, r.resources, r)170 return deployer.NewDeployerAPI(r.srv.state, r.resources, r)
170}171}
171172
173// Environment returns an object that provides access to the Environment API
174// facade. The id argument is reserved for future use and currently needs to
175// be empty.
176func (r *srvRoot) Environment(id string) (*environment.EnvironmentAPI, error) {
177 if id != "" {
178 // Safeguard id for possible future use.
179 return nil, common.ErrBadId
180 }
181 return environment.NewEnvironmentAPI(r.srv.state, r.resources, r)
182}
183
172// Logger returns an object that provides access to the Logger API facade.184// Logger returns an object that provides access to the Logger API facade.
173// The id argument is reserved for future use and must be empty.185// The id argument is reserved for future use and must be empty.
174func (r *srvRoot) Logger(id string) (*loggerapi.LoggerAPI, error) {186func (r *srvRoot) Logger(id string) (*loggerapi.LoggerAPI, error) {
175187
=== modified file 'utils/apt.go'
--- utils/apt.go 2014-01-28 04:58:43 +0000
+++ utils/apt.go 2014-01-29 04:59:50 +0000
@@ -19,6 +19,10 @@
19var (19var (
20 aptLogger = loggo.GetLogger("juju.utils.apt")20 aptLogger = loggo.GetLogger("juju.utils.apt")
21 aptProxyRE = regexp.MustCompile(`(?im)^\s*Acquire::(?P<protocol>[a-z]+)::Proxy\s+"(?P<proxy>[^"]+)";\s*$`)21 aptProxyRE = regexp.MustCompile(`(?im)^\s*Acquire::(?P<protocol>[a-z]+)::Proxy\s+"(?P<proxy>[^"]+)";\s*$`)
22
23 // AptConfFile is the full file path for the proxy settings that are
24 // written by cloud-init and the machine environ worker.
25 AptConfFile = "/etc/apt/apt.conf.d/42-juju-proxy-settings"
22)26)
2327
24// Some helpful functions for running apt in a sane way28// Some helpful functions for running apt in a sane way
2529
=== added directory 'worker/machineenvironmentworker'
=== added file 'worker/machineenvironmentworker/machineenvironmentworker.go'
--- worker/machineenvironmentworker/machineenvironmentworker.go 1970-01-01 00:00:00 +0000
+++ worker/machineenvironmentworker/machineenvironmentworker.go 2014-01-29 04:59:50 +0000
@@ -0,0 +1,173 @@
1// Copyright 2014 Canonical Ltd.
2// Licensed under the AGPLv3, see LICENCE file for details.
3
4package machineenvironmentworker
5
6import (
7 "fmt"
8 "io/ioutil"
9 "path"
10
11 "github.com/loggo/loggo"
12
13 "launchpad.net/juju-core/agent"
14 "launchpad.net/juju-core/juju/osenv"
15 "launchpad.net/juju-core/names"
16 "launchpad.net/juju-core/provider"
17 "launchpad.net/juju-core/state/api/environment"
18 "launchpad.net/juju-core/state/api/watcher"
19 "launchpad.net/juju-core/utils"
20 "launchpad.net/juju-core/utils/exec"
21 "launchpad.net/juju-core/worker"
22)
23
24var (
25 logger = loggo.GetLogger("juju.worker.machineenvironment")
26
27 // ProxyDirectory is the directory containing the proxy file that contains
28 // the environment settings for the proxies based on the environment
29 // config values.
30 ProxyDirectory = "/home/ubuntu"
31
32 // ProxyFile is the name of the file to be stored in the ProxyDirectory.
33 ProxyFile = ".juju-proxy"
34
35 // Started is a function that is called when the worker has started.
36 Started = func() {}
37)
38
39// MachineEnvironmentWorker is responsible for monitoring the juju environment
40// configuration and making changes on the physical (or virtual) machine as
41// necessary to match the environment changes. Examples of these types of
42// changes are apt proxy configuration and the juju proxies stored in the juju
43// proxy file.
44type MachineEnvironmentWorker struct {
45 api *environment.Facade
46 aptProxy osenv.ProxySettings
47 proxy osenv.ProxySettings
48
49 writeSystemFiles bool
50 // The whole point of the first value is to make sure that the the files
51 // are written out the first time through, even if they are the same as
52 // "last" time, as the initial value for last time is the zeroed struct.
53 // There is the possibility that the files exist on disk with old
54 // settings, and the environment has been updated to now not have them. We
55 // need to make sure that the disk reflects the environment, so the first
56 // time through, even if the proxies are empty, we write the files to
57 // disk.
58 first bool
59}
60
61var _ worker.NotifyWatchHandler = (*MachineEnvironmentWorker)(nil)
62
63// NewMachineEnvironmentWorker returns a worker.Worker that uses the notify
64// watcher returned from the setup.
65func NewMachineEnvironmentWorker(api *environment.Facade, agentConfig agent.Config) worker.Worker {
66 // We don't write out system files for the local provider on machine zero
67 // as that is the host machine.
68 writeSystemFiles := !(agentConfig.Tag() == names.MachineTag("0") &&
69 agentConfig.Value(agent.JujuProviderType) == provider.Local)
70 envWorker := &MachineEnvironmentWorker{
71 api: api,
72 writeSystemFiles: writeSystemFiles,
73 first: true,
74 }
75 return worker.NewNotifyWorker(envWorker)
76}
77
78func (w *MachineEnvironmentWorker) writeEnvironmentFile() error {
79 // Writing the environment file is handled by executing the script for two
80 // primary reasons:
81 //
82 // 1: In order to have the local provider specify the environment settings
83 // for the machine agent running on the host, this worker needs to run,
84 // but it shouldn't be touching any files on the disk. If however there is
85 // an ubuntu user, it will. This shouldn't be a problem.
86 //
87 // 2: On cloud-instance ubuntu images, the ubuntu user is uid 1000, but in
88 // the situation where the ubuntu user has been created as a part of the
89 // manual provisioning process, the user will exist, and will not have the
90 // same uid/gid as the default cloud image.
91 //
92 // It is easier to shell out to check both these things, and is also the
93 // same way that the file is written in the cloud-init process, so
94 // consistency FTW.
95 filePath := path.Join(ProxyDirectory, ProxyFile)
96 result, err := exec.RunCommands(exec.RunParams{
97 Commands: fmt.Sprintf(
98 `[ -e %s ] && (printf '%%s\n' %s > %s && chown ubuntu:ubuntu %s)`,
99 ProxyDirectory,
100 utils.ShQuote(w.proxy.AsScriptEnvironment()),
101 filePath, filePath),
102 WorkingDir: ProxyDirectory,
103 })
104 if err != nil {
105 return err
106 }
107 if result.Code != 0 {
108 logger.Errorf("failed writing new proxy values: \n%s\n%s", result.Stdout, result.Stderr)
109 }
110 return nil
111}
112
113func (w *MachineEnvironmentWorker) handleProxyValues(proxySettings osenv.ProxySettings) {
114 if proxySettings != w.proxy || w.first {
115 logger.Debugf("new proxy settings %#v", proxySettings)
116 w.proxy = proxySettings
117 w.proxy.SetEnvironmentValues()
118 if w.writeSystemFiles {
119 if err := w.writeEnvironmentFile(); err != nil {
120 // It isn't really fatal, but we should record it.
121 logger.Errorf("error writing proxy environment file: %v", err)
122 }
123 }
124 }
125}
126
127func (w *MachineEnvironmentWorker) handleAptProxyValues(aptSettings osenv.ProxySettings) {
128 if w.writeSystemFiles && (aptSettings != w.aptProxy || w.first) {
129 logger.Debugf("new apt proxy settings %#v", aptSettings)
130 w.aptProxy = aptSettings
131 // Always finish with a new line.
132 content := utils.AptProxyContent(w.aptProxy) + "\n"
133 err := ioutil.WriteFile(utils.AptConfFile, []byte(content), 0644)
134 if err != nil {
135 // It isn't really fatal, but we should record it.
136 logger.Errorf("error writing apt proxy config file: %v", err)
137 }
138 }
139}
140
141func (w *MachineEnvironmentWorker) onChange() error {
142 env, err := w.api.EnvironConfig()
143 if err != nil {
144 return err
145 }
146 w.handleProxyValues(env.ProxySettings())
147 w.handleAptProxyValues(env.AptProxySettings())
148 return nil
149}
150
151// SetUp is defined on the worker.NotifyWatchHandler interface.
152func (w *MachineEnvironmentWorker) SetUp() (watcher.NotifyWatcher, error) {
153 // We need to set this up initially as the NotifyWorker sucks up the first
154 // event.
155 err := w.onChange()
156 if err != nil {
157 return nil, err
158 }
159 w.first = false
160 Started()
161 return w.api.WatchForEnvironConfigChanges()
162}
163
164// Handle is defined on the worker.NotifyWatchHandler interface.
165func (w *MachineEnvironmentWorker) Handle() error {
166 return w.onChange()
167}
168
169// TearDown is defined on the worker.NotifyWatchHandler interface.
170func (w *MachineEnvironmentWorker) TearDown() error {
171 // Nothing to cleanup, only state is the watcher
172 return nil
173}
0174
=== added file 'worker/machineenvironmentworker/machineenvironmentworker_test.go'
--- worker/machineenvironmentworker/machineenvironmentworker_test.go 1970-01-01 00:00:00 +0000
+++ worker/machineenvironmentworker/machineenvironmentworker_test.go 2014-01-29 04:59:50 +0000
@@ -0,0 +1,222 @@
1// Copyright 2014 Canonical Ltd.
2// Licensed under the AGPLv3, see LICENCE file for details.
3
4package machineenvironmentworker_test
5
6import (
7 "io/ioutil"
8 "os"
9 "path"
10 "time"
11
12 gc "launchpad.net/gocheck"
13
14 "launchpad.net/juju-core/agent"
15 "launchpad.net/juju-core/environs/config"
16 "launchpad.net/juju-core/juju/osenv"
17 jujutesting "launchpad.net/juju-core/juju/testing"
18 "launchpad.net/juju-core/names"
19 "launchpad.net/juju-core/provider"
20 "launchpad.net/juju-core/state"
21 "launchpad.net/juju-core/state/api"
22 "launchpad.net/juju-core/state/api/environment"
23 "launchpad.net/juju-core/testing"
24 jc "launchpad.net/juju-core/testing/checkers"
25 "launchpad.net/juju-core/utils"
26 "launchpad.net/juju-core/worker"
27 "launchpad.net/juju-core/worker/machineenvironmentworker"
28)
29
30type MachineEnvironmentWatcherSuite struct {
31 jujutesting.JujuConnSuite
32
33 apiRoot *api.State
34 environmentAPI *environment.Facade
35 machine *state.Machine
36
37 proxyFile string
38 started bool
39}
40
41var _ = gc.Suite(&MachineEnvironmentWatcherSuite{})
42
43func (s *MachineEnvironmentWatcherSuite) setStarted() {
44 s.started = true
45}
46
47func (s *MachineEnvironmentWatcherSuite) SetUpTest(c *gc.C) {
48 s.JujuConnSuite.SetUpTest(c)
49 s.apiRoot, s.machine = s.OpenAPIAsNewMachine(c)
50 // Create the machiner API facade.
51 s.environmentAPI = s.apiRoot.Environment()
52 c.Assert(s.environmentAPI, gc.NotNil)
53
54 proxyDir := c.MkDir()
55 s.PatchValue(&machineenvironmentworker.ProxyDirectory, proxyDir)
56 s.started = false
57 s.PatchValue(&machineenvironmentworker.Started, s.setStarted)
58 s.PatchValue(&utils.AptConfFile, path.Join(proxyDir, "juju-apt-proxy"))
59 s.proxyFile = path.Join(proxyDir, machineenvironmentworker.ProxyFile)
60}
61
62func (s *MachineEnvironmentWatcherSuite) waitForPostSetup(c *gc.C) {
63 for {
64 select {
65 case <-time.After(testing.LongWait):
66 c.Fatalf("timeout while waiting for setup")
67 case <-time.After(10 * time.Millisecond):
68 if s.started {
69 return
70 }
71 }
72 }
73}
74
75func (s *MachineEnvironmentWatcherSuite) waitProxySettings(c *gc.C, expected osenv.ProxySettings) {
76 for {
77 select {
78 case <-time.After(testing.LongWait):
79 c.Fatalf("timeout while waiting for proxy settings to change")
80 case <-time.After(10 * time.Millisecond):
81 obtained := osenv.DetectProxies()
82 if obtained != expected {
83 c.Logf("proxy settings are %#v, still waiting", obtained)
84 continue
85 }
86 return
87 }
88 }
89}
90
91func (s *MachineEnvironmentWatcherSuite) waitForFile(c *gc.C, filename, expected string) {
92 for {
93 select {
94 case <-time.After(testing.LongWait):
95 c.Fatalf("timeout while waiting for proxy settings to change")
96 case <-time.After(10 * time.Millisecond):
97 fileContent, err := ioutil.ReadFile(filename)
98 if os.IsNotExist(err) {
99 continue
100 }
101 c.Assert(err, gc.IsNil)
102 if string(fileContent) != expected {
103 c.Logf("file content not matching, still waiting")
104 continue
105 }
106 return
107 }
108 }
109}
110
111func (s *MachineEnvironmentWatcherSuite) makeWorker(c *gc.C, agentConfig agent.Config) worker.Worker {
112 return machineenvironmentworker.NewMachineEnvironmentWorker(s.environmentAPI, agentConfig)
113}
114
115func (s *MachineEnvironmentWatcherSuite) TestRunStop(c *gc.C) {
116 agentConfig := agentConfig("0", "ec2")
117 envWorker := s.makeWorker(c, agentConfig)
118 c.Assert(worker.Stop(envWorker), gc.IsNil)
119}
120
121func (s *MachineEnvironmentWatcherSuite) updateConfig(c *gc.C) (osenv.ProxySettings, osenv.ProxySettings) {
122 oldConfig, err := s.State.EnvironConfig()
123 c.Assert(err, gc.IsNil)
124
125 proxySettings := osenv.ProxySettings{
126 Http: "http proxy",
127 Https: "https proxy",
128 Ftp: "ftp proxy",
129 }
130
131 envConfig, err := oldConfig.Apply(config.ProxyConfigMap(proxySettings))
132 c.Assert(err, gc.IsNil)
133
134 // We explicitly set apt proxy settings as well to show that it is the apt
135 // settings that are used for the apt config, and not just the normal
136 // proxy settings which is what we would get if we don't explicitly set
137 // apt values.
138 aptProxySettings := osenv.ProxySettings{
139 Http: "apt http proxy",
140 Https: "apt https proxy",
141 Ftp: "apt ftp proxy",
142 }
143 envConfig, err = envConfig.Apply(config.AptProxyConfigMap(aptProxySettings))
144 c.Assert(err, gc.IsNil)
145
146 err = s.State.SetEnvironConfig(envConfig, oldConfig)
147 c.Assert(err, gc.IsNil)
148
149 return proxySettings, aptProxySettings
150}
151
152func (s *MachineEnvironmentWatcherSuite) TestInitialState(c *gc.C) {
153 proxySettings, aptProxySettings := s.updateConfig(c)
154
155 agentConfig := agentConfig("0", "ec2")
156 envWorker := s.makeWorker(c, agentConfig)
157 defer worker.Stop(envWorker)
158
159 s.waitProxySettings(c, proxySettings)
160 s.waitForFile(c, s.proxyFile, proxySettings.AsScriptEnvironment()+"\n")
161 s.waitForFile(c, utils.AptConfFile, utils.AptProxyContent(aptProxySettings)+"\n")
162}
163
164func (s *MachineEnvironmentWatcherSuite) TestRespondsToEvents(c *gc.C) {
165 agentConfig := agentConfig("0", "ec2")
166 envWorker := s.makeWorker(c, agentConfig)
167 defer worker.Stop(envWorker)
168 s.waitForPostSetup(c)
169
170 proxySettings, aptProxySettings := s.updateConfig(c)
171
172 s.waitProxySettings(c, proxySettings)
173 s.waitForFile(c, s.proxyFile, proxySettings.AsScriptEnvironment()+"\n")
174 s.waitForFile(c, utils.AptConfFile, utils.AptProxyContent(aptProxySettings)+"\n")
175}
176
177func (s *MachineEnvironmentWatcherSuite) TestInitialStateLocalMachine1(c *gc.C) {
178 proxySettings, aptProxySettings := s.updateConfig(c)
179
180 agentConfig := agentConfig("1", provider.Local)
181 envWorker := s.makeWorker(c, agentConfig)
182 defer worker.Stop(envWorker)
183
184 s.waitProxySettings(c, proxySettings)
185 s.waitForFile(c, s.proxyFile, proxySettings.AsScriptEnvironment()+"\n")
186 s.waitForFile(c, utils.AptConfFile, utils.AptProxyContent(aptProxySettings)+"\n")
187}
188
189func (s *MachineEnvironmentWatcherSuite) TestInitialStateLocalMachine0(c *gc.C) {
190 proxySettings, _ := s.updateConfig(c)
191
192 agentConfig := agentConfig("0", provider.Local)
193 envWorker := s.makeWorker(c, agentConfig)
194 defer worker.Stop(envWorker)
195 s.waitForPostSetup(c)
196
197 s.waitProxySettings(c, proxySettings)
198
199 c.Assert(utils.AptConfFile, jc.DoesNotExist)
200 c.Assert(s.proxyFile, jc.DoesNotExist)
201}
202
203type mockConfig struct {
204 agent.Config
205 tag string
206 provider string
207}
208
209func (mock *mockConfig) Tag() string {
210 return mock.tag
211}
212
213func (mock *mockConfig) Value(key string) string {
214 if key == agent.JujuProviderType {
215 return mock.provider
216 }
217 return ""
218}
219
220func agentConfig(machineId, provider string) *mockConfig {
221 return &mockConfig{tag: names.MachineTag(machineId), provider: provider}
222}
0223
=== added file 'worker/machineenvironmentworker/package_test.go'
--- worker/machineenvironmentworker/package_test.go 1970-01-01 00:00:00 +0000
+++ worker/machineenvironmentworker/package_test.go 2014-01-29 04:59:50 +0000
@@ -0,0 +1,14 @@
1// Copyright 2014 Canonical Ltd.
2// Licensed under the AGPLv3, see LICENCE file for details.
3
4package machineenvironmentworker_test
5
6import (
7 stdtesting "testing"
8
9 "launchpad.net/juju-core/testing"
10)
11
12func TestAll(t *stdtesting.T) {
13 testing.MgoTestPackage(t)
14}

Subscribers

People subscribed via source and target branches

to status/vote changes: