juju-core

Merge lp:~thumper/juju-core/lxc-provisioner into lp:~go-bot/juju-core/trunk

  1. lxc-provisioner
  2. Merge into trunk
Proposed by Tim Penhey
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 1343
Proposed branch: lp:~thumper/juju-core/lxc-provisioner
Merge into: lp:~go-bot/juju-core/trunk
Prerequisite: lp:~thumper/juju-core/provisioner-auth-provider
Diff against target: 1313 lines (+494/-130)
14 files modified
cmd/jujud/machine.go (+19/-4)
cmd/jujud/machine_test.go (+14/-0)
container/lxc/instance.go (+5/-0)
container/lxc/lxc.go (+7/-7)
container/lxc/lxc_test.go (+19/-31)
container/lxc/mock/mock-lxc.go (+86/-3)
container/lxc/test.go (+48/-0)
juju/testing/conn.go (+1/-0)
state/state.go (+1/-0)
worker/provisioner/lxc-broker.go (+3/-5)
worker/provisioner/lxc-broker_test.go (+156/-30)
worker/provisioner/provisioner.go (+85/-8)
worker/provisioner/provisioner_task.go (+15/-15)
worker/provisioner/provisioner_test.go (+35/-27)
To merge this branch: bzr merge lp:~thumper/juju-core/lxc-provisioner
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+171034@code.launchpad.net

Commit message

Hook up the lxc-broker to the provisioner.

I noticed that the JujuConnSuite wasn't closing the APIConn that it opened.

Lots of test refactoring as well.

https://codereview.appspot.com/10489043/

Description of the change

Hook up the lxc-broker to the provisioner.

How William and I discussed this was to have a new provisioner task at the
machine agent level for the containers.

I added a string representation for the lxc instance as I was outputting some
values in logging information, and it seemed reasonable to leave this in.

The mock lxc implementation gets smarter as I needed this to test an error
condition that I was seeing. Inside the provisioner task, when we delete a
machine, we remove it from the task's map of machines. When we then look for
the unknown instances, this is done by grabbing all the instances and removing
all those we know about. Since there may be a machine that we are stopping,
this has been removed from the provisioner task's map, so the instance is
unknown. When we try to find the instance for the stopped machines, we are
looking in the task's instance map, and we find it. Since we were adding both
the stopping list and the unknown list to the "stop these instances" method,
we were trying to stop the same instance more than once, which was causing
errors. The simplest thing was to pass the stopping list of instances through
to the unknown finding method so they can be ignored.

The dummy provider was very good at ignoring this completely, so the mock lxc
implementation has a little more smarts, and now does return error conditions
if create/start/stop/destory are called on containers with unexpected states.

The mock lxc implementation now also has some event observers. New events are
created for started and stopped machines. These are used in the tests to
monitor the starting and stopping of the containers.

I noticed that the JujuConnSuite wasn't closing the APIConn that it opened.

I refactored the suites the provisioner tests used in order to get some handy
function reuse. Especially when there was other state needed. Like the lxc
containers still need a real machine to have been created, this needs state,
and a real (or dummy) provider.

The provisioner grew a few factory methods that are used to create the
watchers and brokers for the particular supported types.

https://codereview.appspot.com/10489043/

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

Reviewers: mp+171034_code.launchpad.net,

Message:
Please take a look.

Description:
Hook up the lxc-broker to the provisioner.

How William and I discussed this was to have a new provisioner task at
the
machine agent level for the containers.

I added a string representation for the lxc instance as I was outputting
some
values in logging information, and it seemed reasonable to leave this
in.

The mock lxc implementation gets smarter as I needed this to test an
error
condition that I was seeing. Inside the provisioner task, when we
delete a
machine, we remove it from the task's map of machines. When we then
look for
the unknown instances, this is done by grabbing all the instances and
removing
all those we know about. Since there may be a machine that we are
stopping,
this has been removed from the provisioner task's map, so the instance
is
unknown. When we try to find the instance for the stopped machines, we
are
looking in the task's instance map, and we find it. Since we were
adding both
the stopping list and the unknown list to the "stop these instances"
method,
we were trying to stop the same instance more than once, which was
causing
errors. The simplest thing was to pass the stopping list of instances
through
to the unknown finding method so they can be ignored.

The dummy provider was very good at ignoring this completely, so the
mock lxc
implementation has a little more smarts, and now does return error
conditions
if create/start/stop/destory are called on containers with unexpected
states.

The mock lxc implementation now also has some event observers. New
events are
created for started and stopped machines. These are used in the tests
to
monitor the starting and stopping of the containers.

I noticed that the JujuConnSuite wasn't closing the APIConn that it
opened.

I refactored the suites the provisioner tests used in order to get some
handy
function reuse. Especially when there was other state needed. Like the
lxc
containers still need a real machine to have been created, this needs
state,
and a real (or dummy) provider.

The provisioner grew a few factory methods that are used to create the
watchers and brokers for the particular supported types.

https://code.launchpad.net/~thumper/juju-core/lxc-provisioner/+merge/171034

Requires:
https://code.launchpad.net/~thumper/juju-core/provisioner-auth-provider/+merge/171006

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M cmd/jujud/machine.go
   M container/lxc/instance.go
   M container/lxc/mock/mock-lxc.go
   M juju/testing/conn.go
   M worker/provisioner/export_test.go
   M worker/provisioner/lxc-broker.go
   M worker/provisioner/lxc-broker_test.go
   M worker/provisioner/provisioner.go
   M worker/provisioner/provisioner_task.go
   M worker/provisioner/provisioner_test.go

Revision history for this message
Tim Penhey (thumper) wrote :

Please take a look.

https://codereview.appspot.com/10489043/

Revision history for this message
William Reade (fwereade) wrote :
Download full text (3.3 KiB)

LGTM, lots of comments but nothing blocking anything.

https://codereview.appspot.com/10489043/diff/3001/cmd/jujud/machine.go
File cmd/jujud/machine.go (right):

https://codereview.appspot.com/10489043/diff/3001/cmd/jujud/machine.go#newcode129
cmd/jujud/machine.go:129:
provisioner.NewProvisioner(provisioner.ENVIRON, st, a.MachineId,
dataDir),
Should these provisioner.CONSTANTs maybe move to the instance package?

https://codereview.appspot.com/10489043/diff/3001/worker/provisioner/provisioner.go
File worker/provisioner/provisioner.go (right):

https://codereview.appspot.com/10489043/diff/3001/worker/provisioner/provisioner.go#newcode32
worker/provisioner/provisioner.go:32: // While I'm debugging.
Leave this out for merging :).

https://codereview.appspot.com/10489043/diff/3001/worker/provisioner/provisioner.go#newcode148
worker/provisioner/provisioner.go:148: switch p.pt {
I don't really love this but I'm happy to see where it goes.

https://codereview.appspot.com/10489043/diff/3001/worker/provisioner/provisioner.go#newcode177
worker/provisioner/provisioner.go:177: func (p *Provisioner)
getAgentTools() (*state.Tools, error) {
Again, I'm willing to chalk this up to evolution, but I'm not quite sure
it belongs here long-term.

https://codereview.appspot.com/10489043/diff/7001/juju/testing/conn.go
File juju/testing/conn.go (right):

https://codereview.appspot.com/10489043/diff/7001/juju/testing/conn.go#newcode225
juju/testing/conn.go:225: c.Assert(s.APIConn.Close(), IsNil)
Thanks for that.

/me wonders whether this might have to do with the races davecheney was
looking into... ping him maybe?

https://codereview.appspot.com/10489043/diff/7001/worker/provisioner/lxc-broker.go
File worker/provisioner/lxc-broker.go (right):

https://codereview.appspot.com/10489043/diff/7001/worker/provisioner/lxc-broker.go#newcode45
worker/provisioner/lxc-broker.go:45: lxcLogger.Infof("started lxc
container for machineId: %s, %s", machineId, inst.Id())
I'd be fine with package vars called "log" by convention.

https://codereview.appspot.com/10489043/diff/7001/worker/provisioner/provisioner.go
File worker/provisioner/provisioner.go (right):

https://codereview.appspot.com/10489043/diff/7001/worker/provisioner/provisioner.go#newcode112
worker/provisioner/provisioner.go:112: machineBroker,
instance broker? even, perhaps, instance.Broker? just a thought...

https://codereview.appspot.com/10489043/diff/7001/worker/provisioner/provisioner_task.go
File worker/provisioner/provisioner_task.go (right):

https://codereview.appspot.com/10489043/diff/7001/worker/provisioner/provisioner_task.go#newcode252
worker/provisioner/provisioner_task.go:252: // have been removed from
the task.machines map.
In a spirit purely of philosophical enquiry: would it have been
equivalent to have just removed the machines from the map a bit later?

https://codereview.appspot.com/10489043/diff/7001/worker/provisioner/provisioner_task.go#newcode338
worker/provisioner/provisioner_task.go:338: logger.Errorf("cannot start
the machine as provisioned %q: %v", machine, err)
maybe "cannot register instance for machine %v: %v"?

fwiw, machine ids have hitherto *not* been reported with quotes. This no
longe...

Read more...

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

LGTM
What William said plus I query of my own. Perhaps expand the code
comment a little?

https://codereview.appspot.com/10489043/diff/7001/cmd/jujud/machine.go
File cmd/jujud/machine.go (right):

https://codereview.appspot.com/10489043/diff/7001/cmd/jujud/machine.go#newcode172
cmd/jujud/machine.go:172: runner.StartWorker(workerName, func()
(worker.Worker, error) {
I'm confused by this. I don't understand why an lxc provisoner is used
for non-lxc containers. I know the comment says something, but it's
still not clear to me. How would an lxc provisioner task provision a kvm
container? What does not embedding lxc containers have to do with it?
Should the worker name contain the actual container? eg
"LXC-provisioner-for-KVM"?

https://codereview.appspot.com/10489043/diff/7001/worker/provisioner/lxc-broker.go
File worker/provisioner/lxc-broker.go (right):

https://codereview.appspot.com/10489043/diff/7001/worker/provisioner/lxc-broker.go#newcode45
worker/provisioner/lxc-broker.go:45: lxcLogger.Infof("started lxc
container for machineId: %s, %s", machineId, inst.Id())
On 2013/06/26 23:34:47, fwereade wrote:
> I'd be fine with package vars called "log" by convention.

+1
"logger" is the colour of my bikeshed

https://codereview.appspot.com/10489043/diff/7001/worker/provisioner/provisioner.go
File worker/provisioner/provisioner.go (right):

https://codereview.appspot.com/10489043/diff/7001/worker/provisioner/provisioner.go#newcode137
worker/provisioner/provisioner.go:137: if p.machine == nil {
This is not thread safe. Is this being used in a single threaded
context? I guess it doesn't matter so much since we are just reading
some info from state.

https://codereview.appspot.com/10489043/

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

Please take a look.

https://codereview.appspot.com/10489043/diff/7001/cmd/jujud/machine.go
File cmd/jujud/machine.go (right):

https://codereview.appspot.com/10489043/diff/7001/cmd/jujud/machine.go#newcode172
cmd/jujud/machine.go:172: runner.StartWorker(workerName, func()
(worker.Worker, error) {
On 2013/06/27 00:16:56, wallyworld wrote:
> I'm confused by this. I don't understand why an lxc provisoner is used
 for
> non-lxc containers. I know the comment says something, but it's still
 not clear
> to me. How would an lxc provisioner task provision a kvm container?
 What does
> not embedding lxc containers have to do with it? Should the worker
 name contain
> the actual container? eg "LXC-provisioner-for-KVM"?

Extended the comment somewhat to help with that confusion.

https://codereview.appspot.com/10489043/diff/7001/juju/testing/conn.go
File juju/testing/conn.go (right):

https://codereview.appspot.com/10489043/diff/7001/juju/testing/conn.go#newcode225
juju/testing/conn.go:225: c.Assert(s.APIConn.Close(), IsNil)
On 2013/06/26 23:34:47, fwereade wrote:
> Thanks for that.

> /me wonders whether this might have to do with the races davecheney
was looking
> into... ping him maybe?

Done.

https://codereview.appspot.com/10489043/diff/7001/worker/provisioner/lxc-broker.go
File worker/provisioner/lxc-broker.go (right):

https://codereview.appspot.com/10489043/diff/7001/worker/provisioner/lxc-broker.go#newcode45
worker/provisioner/lxc-broker.go:45: lxcLogger.Infof("started lxc
container for machineId: %s, %s", machineId, inst.Id())
On 2013/06/27 00:16:56, wallyworld wrote:
> On 2013/06/26 23:34:47, fwereade wrote:
> > I'd be fine with package vars called "log" by convention.

> +1
> "logger" is the colour of my bikeshed

logger is what I've used, however that one already exists at package
level, which is why I needed another. I was torn about what to do with
it, but for now, I'm going to leave this.

Perhaps we should move the lxc broker into an lxc package under this
one.

https://codereview.appspot.com/10489043/diff/7001/worker/provisioner/provisioner.go
File worker/provisioner/provisioner.go (right):

https://codereview.appspot.com/10489043/diff/7001/worker/provisioner/provisioner.go#newcode112
worker/provisioner/provisioner.go:112: machineBroker,
On 2013/06/26 23:34:47, fwereade wrote:
> instance broker? even, perhaps, instance.Broker? just a thought...

I would love to move Broker into the instance package, but right now,
the instance packages doesn't have dependencies, and the Broker would
bring in state, api and constraints.

I will change the name though to instanceBroker.

https://codereview.appspot.com/10489043/diff/7001/worker/provisioner/provisioner.go#newcode137
worker/provisioner/provisioner.go:137: if p.machine == nil {
On 2013/06/27 00:16:56, wallyworld wrote:
> This is not thread safe. Is this being used in a single threaded
 context? I
> guess it doesn't matter so much since we are just reading some info
 from state.

Yes, getMachine is used just during the start-up process in the loop()
call. It doesn't need to be thread-safe.

https://codereview.appspot.com/10489043/diff/7001/worker/provisioner/provisioner_task.go
File worker/pro...

Read more...

Revision history for this message
Go Bot (go-bot) wrote :
Download full text (45.0 KiB)

The attempt to merge lp:~thumper/juju-core/lxc-provisioner into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core/bzr 6.990s
ok launchpad.net/juju-core/cert 4.369s
ok launchpad.net/juju-core/charm 0.530s
? launchpad.net/juju-core/charm/hooks [no test files]
ok launchpad.net/juju-core/cloudinit 0.009s
ok launchpad.net/juju-core/cmd 0.239s
? launchpad.net/juju-core/cmd/builddb [no test files]
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 120.272s

----------------------------------------------------------------------
FAIL: machine_test.go:129: MachineSuite.TestDyingMachine

[LOG] 3.60901 INFO juju environs/testing: uploading FAKE tools 1.11.1-precise-amd64
[LOG] 3.60910 INFO juju environs: reading tools with major version 1
[LOG] 3.60912 DEBUG juju environs/tools: reading v1.* tools
[LOG] 3.60914 INFO juju environs: falling back to public bucket
[LOG] 3.60915 DEBUG juju environs/tools: reading v1.* tools
[LOG] 3.60920 DEBUG juju environs/tools: found 1.11.1-precise-amd64
[LOG] 3.60921 INFO juju environs: filtering tools by series: precise
[LOG] 3.60924 INFO juju environs: filtering tools by version: 1.11.1
[LOG] 3.60927 INFO juju environs/dummy: would pick tools from 1.11.1-precise-amd64
[LOG] 3.63986 INFO juju state: opening state; mongo addresses: ["localhost:39292"]; entity ""
[LOG] 3.64356 INFO juju state: connection established
[LOG] 3.67755 INFO juju state: initializing environment
[LOG] 3.70506 INFO juju state/api: listening on "127.0.0.1:32893"
[LOG] 3.73164 INFO juju state: opening state; mongo addresses: ["localhost:39292"]; entity ""
[LOG] 3.73535 INFO juju state: connection established
[LOG] 3.73594 INFO juju juju: authorization error while connecting to state server; retrying
[LOG] 3.73601 INFO juju state: opening state; mongo addresses: ["localhost:39292"]; entity ""
[LOG] 3.73965 INFO juju state: connection established
[LOG] 3.79803 INFO juju state/api: dialing "wss://127.0.0.1:32893/"
[LOG] 3.80286 INFO juju state/api: connection established
[LOG] 3.80311 INFO juju rpc: discarding action method reflect.Method{Name:"apiRootForEntity", PkgPath:"launchpad.net/juju-core/state/apiserver", Type:(*reflect.commonType)(0x93dfa0), Func:reflect.Value{typ:(*reflect.commonType)(0x93dfa0), val:(unsafe.Pointer)(0x53d0a0), flag:0x131}, Index:1}
[LOG] 3.80322 DEBUG juju rpc/jsoncodec: <- {"RequestId":1,"Type":"Admin","Request":"Login","Params":{"AuthTag":"user-admin","Password":"dummy-secret"}}
[LOG] 3.80374 INFO juju rpc: discarding obtainer method reflect.Method{Name:"AuthClient", PkgPath:"", Type:(*reflect.commonType)(0x849a98), Func:reflect.Value{typ:(*reflect.commonType)(0x849a98), val:(unsafe.Pointer)(0x53ebcf), flag:0x130}, Index:1}
[LOG] 3.80377 INFO juju rpc: discarding obtainer method reflect.Method{Name:"AuthEnvironManager", PkgPath:"", Type:(*reflect.commonType)(0x849a98), Func:reflect.Value{typ:(*reflect.commonType)(0x849a98), val:(unsafe.Pointer)(0x53eb89), flag:0x130}, Index:2}
[LOG] 3.80380 INFO juju rpc: discarding obtainer method reflect.Method{Name:"AuthMachineAgen...

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 2013-06-26 12:36:42 +0000
3+++ cmd/jujud/machine.go 2013-06-27 04:53:25 +0000
4@@ -156,23 +156,38 @@
5 m := entity.(*state.Machine)
6 // TODO(rog) use more discriminating test for errors
7 // rather than taking everything down indiscriminately.
8+ dataDir := a.Conf.DataDir
9 runner := worker.NewRunner(allFatal, moreImportant)
10 runner.StartWorker("upgrader", func() (worker.Worker, error) {
11 // TODO(rog) use id instead of *Machine (or introduce Clone method)
12- return NewUpgrader(st, m, a.Conf.DataDir), nil
13+ return NewUpgrader(st, m, dataDir), nil
14 })
15 runner.StartWorker("machiner", func() (worker.Worker, error) {
16 return machiner.NewMachiner(st, m.Id()), nil
17 })
18+ // At this stage, since we don't embed lxc containers, just start an lxc
19+ // provisioner task for non-lxc containers. Since we have only LXC
20+ // containers and normal machines, this effectively means that we only
21+ // have an LXC provisioner when we have a normally provisioned machine
22+ // (through the environ-provisioner). With the upcoming advent of KVM
23+ // containers, it is likely that we will want an LXC provisioner on a KVM
24+ // machine, and once we get nested LXC containers, we can remove this
25+ // check.
26+ if m.ContainerType() != state.LXC {
27+ workerName := fmt.Sprintf("%s-provisioner", provisioner.LXC)
28+ runner.StartWorker(workerName, func() (worker.Worker, error) {
29+ return provisioner.NewProvisioner(provisioner.LXC, st, a.MachineId, dataDir), nil
30+ })
31+ }
32 for _, job := range m.Jobs() {
33 switch job {
34 case state.JobHostUnits:
35 runner.StartWorker("deployer", func() (worker.Worker, error) {
36- return newDeployer(st, m.WatchPrincipalUnits(), a.Conf.DataDir), nil
37+ return newDeployer(st, m.WatchPrincipalUnits(), dataDir), nil
38 })
39 case state.JobManageEnviron:
40- runner.StartWorker("provisioner", func() (worker.Worker, error) {
41- return provisioner.NewProvisioner(st, a.MachineId), nil
42+ runner.StartWorker("environ-provisioner", func() (worker.Worker, error) {
43+ return provisioner.NewProvisioner(provisioner.ENVIRON, st, a.MachineId, dataDir), nil
44 })
45 runner.StartWorker("firewaller", func() (worker.Worker, error) {
46 return firewaller.NewFirewaller(st), nil
47
48=== modified file 'cmd/jujud/machine_test.go'
49--- cmd/jujud/machine_test.go 2013-06-21 17:09:58 +0000
50+++ cmd/jujud/machine_test.go 2013-06-27 04:53:25 +0000
51@@ -9,6 +9,7 @@
52 "launchpad.net/juju-core/charm"
53 "launchpad.net/juju-core/cmd"
54 "launchpad.net/juju-core/constraints"
55+ "launchpad.net/juju-core/container/lxc"
56 "launchpad.net/juju-core/environs/agent"
57 "launchpad.net/juju-core/environs/dummy"
58 envtesting "launchpad.net/juju-core/environs/testing"
59@@ -26,6 +27,7 @@
60
61 type MachineSuite struct {
62 agentSuite
63+ lxc.TestSuite
64 oldCacheDir string
65 }
66
67@@ -33,14 +35,26 @@
68
69 func (s *MachineSuite) SetUpSuite(c *C) {
70 s.agentSuite.SetUpSuite(c)
71+ s.TestSuite.SetUpSuite(c)
72 s.oldCacheDir = charm.CacheDir
73 }
74
75 func (s *MachineSuite) TearDownSuite(c *C) {
76 charm.CacheDir = s.oldCacheDir
77+ s.TestSuite.TearDownSuite(c)
78 s.agentSuite.TearDownSuite(c)
79 }
80
81+func (s *MachineSuite) SetUpTest(c *C) {
82+ s.agentSuite.SetUpTest(c)
83+ s.TestSuite.SetUpTest(c)
84+}
85+
86+func (s *MachineSuite) TearDownTest(c *C) {
87+ s.TestSuite.TearDownTest(c)
88+ s.agentSuite.TearDownTest(c)
89+}
90+
91 // primeAgent adds a new Machine to run the given jobs, and sets up the
92 // machine agent's directory. It returns the new machine, the
93 // agent's configuration and the tools currently running.
94
95=== modified file 'container/lxc/instance.go'
96--- container/lxc/instance.go 2013-06-25 21:24:49 +0000
97+++ container/lxc/instance.go 2013-06-27 04:53:25 +0000
98@@ -44,3 +44,8 @@
99 func (lxc *lxcInstance) Ports(machineId string) ([]instance.Port, error) {
100 return nil, fmt.Errorf("not implemented")
101 }
102+
103+// Add a string representation of the id.
104+func (lxc *lxcInstance) String() string {
105+ return fmt.Sprintf("lxc:%s", lxc.id)
106+}
107
108=== modified file 'container/lxc/lxc.go'
109--- container/lxc/lxc.go 2013-06-24 01:35:34 +0000
110+++ container/lxc/lxc.go 2013-06-27 04:53:25 +0000
111@@ -28,6 +28,7 @@
112 containerDir = "/var/lib/juju/containers"
113 removedContainerDir = "/var/lib/juju/removed-containers"
114 lxcContainerDir = "/var/lib/lxc"
115+ lxcObjectFactory = golxc.Factory()
116 )
117
118 // ContainerManager is responsible for starting containers, and stopping and
119@@ -49,15 +50,14 @@
120 }
121
122 type containerManager struct {
123- lxcObjectFactory golxc.ContainerFactory
124- name string
125+ name string
126 }
127
128 // NewContainerManager returns a manager object that can start and stop lxc
129 // containers. The containers that are created are namespaced by the name
130 // parameter.
131-func NewContainerManager(factory golxc.ContainerFactory, name string) ContainerManager {
132- return &containerManager{factory, name}
133+func NewContainerManager(name string) ContainerManager {
134+ return &containerManager{name}
135 }
136
137 func (manager *containerManager) StartContainer(
138@@ -74,7 +74,7 @@
139 // Note here that the lxcObjectFacotry only returns a valid container
140 // object, and doesn't actually construct the underlying lxc container on
141 // disk.
142- container := manager.lxcObjectFactory.New(name)
143+ container := lxcObjectFactory.New(name)
144
145 // Create the cloud-init.
146 directory := jujuContainerDirectory(name)
147@@ -134,7 +134,7 @@
148
149 func (manager *containerManager) StopContainer(instance instance.Instance) error {
150 name := string(instance.Id())
151- container := manager.lxcObjectFactory.New(name)
152+ container := lxcObjectFactory.New(name)
153 if err := container.Stop(); err != nil {
154 logger.Errorf("failed to stop lxc container: %v", err)
155 return err
156@@ -162,7 +162,7 @@
157 }
158
159 func (manager *containerManager) ListContainers() (result []instance.Instance, err error) {
160- containers, err := manager.lxcObjectFactory.List()
161+ containers, err := lxcObjectFactory.List()
162 if err != nil {
163 logger.Errorf("failed getting all instances: %v", err)
164 return
165
166=== modified file 'container/lxc/lxc_test.go'
167--- container/lxc/lxc_test.go 2013-06-24 01:35:34 +0000
168+++ container/lxc/lxc_test.go 2013-06-27 04:53:25 +0000
169@@ -13,7 +13,6 @@
170 . "launchpad.net/gocheck"
171 "launchpad.net/goyaml"
172 "launchpad.net/juju-core/container/lxc"
173- "launchpad.net/juju-core/container/lxc/mock"
174 "launchpad.net/juju-core/instance"
175 jujutesting "launchpad.net/juju-core/juju/testing"
176 "launchpad.net/juju-core/state"
177@@ -28,38 +27,28 @@
178
179 type LxcSuite struct {
180 testing.LoggingSuite
181- containerDir string
182- removedDir string
183- lxcDir string
184- oldContainerDir string
185- oldRemovedDir string
186- oldLxcContainerDir string
187+ lxc.TestSuite
188 }
189
190 var _ = Suite(&LxcSuite{})
191
192 func (s *LxcSuite) SetUpSuite(c *C) {
193 s.LoggingSuite.SetUpSuite(c)
194+ s.TestSuite.SetUpSuite(c)
195 }
196
197 func (s *LxcSuite) TearDownSuite(c *C) {
198+ s.TestSuite.TearDownSuite(c)
199 s.LoggingSuite.TearDownSuite(c)
200 }
201
202 func (s *LxcSuite) SetUpTest(c *C) {
203 s.LoggingSuite.SetUpTest(c)
204- s.containerDir = c.MkDir()
205- s.oldContainerDir = lxc.SetContainerDir(s.containerDir)
206- s.removedDir = c.MkDir()
207- s.oldRemovedDir = lxc.SetRemovedContainerDir(s.removedDir)
208- s.lxcDir = c.MkDir()
209- s.oldLxcContainerDir = lxc.SetLxcContainerDir(s.lxcDir)
210+ s.TestSuite.SetUpTest(c)
211 }
212
213 func (s *LxcSuite) TearDownTest(c *C) {
214- lxc.SetContainerDir(s.oldContainerDir)
215- lxc.SetLxcContainerDir(s.oldLxcContainerDir)
216- lxc.SetRemovedContainerDir(s.oldRemovedDir)
217+ s.TestSuite.TearDownTest(c)
218 s.LoggingSuite.TearDownTest(c)
219 }
220
221@@ -81,13 +70,13 @@
222 }
223
224 func (s *LxcSuite) TestStartContainer(c *C) {
225- manager := lxc.NewContainerManager(mock.MockFactory(), "")
226+ manager := lxc.NewContainerManager("")
227 instance := StartContainer(c, manager, "1/lxc/0")
228
229 name := string(instance.Id())
230 // Check our container config files.
231- c.Assert(filepath.Join(s.containerDir, name, "lxc.conf"), IsNonEmptyFile)
232- cloudInitFilename := filepath.Join(s.containerDir, name, "cloud-init")
233+ c.Assert(filepath.Join(s.ContainerDir, name, "lxc.conf"), IsNonEmptyFile)
234+ cloudInitFilename := filepath.Join(s.ContainerDir, name, "cloud-init")
235 c.Assert(cloudInitFilename, IsNonEmptyFile)
236 data, err := ioutil.ReadFile(cloudInitFilename)
237 c.Assert(err, IsNil)
238@@ -105,11 +94,11 @@
239 c.Assert(scripts[len(scripts)-1], Equals, "start jujud-machine-1-lxc-0")
240
241 // Check the mount point has been created inside the container.
242- c.Assert(filepath.Join(s.lxcDir, name, "rootfs/var/log/juju"), IsDirectory)
243+ c.Assert(filepath.Join(s.LxcDir, name, "rootfs/var/log/juju"), IsDirectory)
244 }
245
246 func (s *LxcSuite) TestStopContainer(c *C) {
247- manager := lxc.NewContainerManager(mock.MockFactory(), "")
248+ manager := lxc.NewContainerManager("")
249 instance := StartContainer(c, manager, "1/lxc/0")
250
251 err := manager.StopContainer(instance)
252@@ -117,17 +106,17 @@
253
254 name := string(instance.Id())
255 // Check that the container dir is no longer in the container dir
256- c.Assert(filepath.Join(s.containerDir, name), DoesNotExist)
257+ c.Assert(filepath.Join(s.ContainerDir, name), DoesNotExist)
258 // but instead, in the removed container dir
259- c.Assert(filepath.Join(s.removedDir, name), IsDirectory)
260+ c.Assert(filepath.Join(s.RemovedDir, name), IsDirectory)
261 }
262
263 func (s *LxcSuite) TestStopContainerNameClash(c *C) {
264- manager := lxc.NewContainerManager(mock.MockFactory(), "")
265+ manager := lxc.NewContainerManager("")
266 instance := StartContainer(c, manager, "1/lxc/0")
267
268 name := string(instance.Id())
269- targetDir := filepath.Join(s.removedDir, name)
270+ targetDir := filepath.Join(s.RemovedDir, name)
271 err := os.MkdirAll(targetDir, 0755)
272 c.Assert(err, IsNil)
273
274@@ -135,21 +124,20 @@
275 c.Assert(err, IsNil)
276
277 // Check that the container dir is no longer in the container dir
278- c.Assert(filepath.Join(s.containerDir, name), DoesNotExist)
279+ c.Assert(filepath.Join(s.ContainerDir, name), DoesNotExist)
280 // but instead, in the removed container dir with a ".1" suffix as there was already a directory there.
281- c.Assert(filepath.Join(s.removedDir, fmt.Sprintf("%s.1", name)), IsDirectory)
282+ c.Assert(filepath.Join(s.RemovedDir, fmt.Sprintf("%s.1", name)), IsDirectory)
283 }
284
285 func (s *LxcSuite) TestNamedManagerPrefix(c *C) {
286- manager := lxc.NewContainerManager(mock.MockFactory(), "eric")
287+ manager := lxc.NewContainerManager("eric")
288 instance := StartContainer(c, manager, "1/lxc/0")
289 c.Assert(string(instance.Id()), Equals, "eric-machine-1-lxc-0")
290 }
291
292 func (s *LxcSuite) TestListContainers(c *C) {
293- factory := mock.MockFactory()
294- foo := lxc.NewContainerManager(factory, "foo")
295- bar := lxc.NewContainerManager(factory, "bar")
296+ foo := lxc.NewContainerManager("foo")
297+ bar := lxc.NewContainerManager("bar")
298
299 foo1 := StartContainer(c, foo, "1/lxc/0")
300 foo2 := StartContainer(c, foo, "1/lxc/1")
301
302=== modified file 'container/lxc/mock/mock-lxc.go'
303--- container/lxc/mock/mock-lxc.go 2013-06-19 04:09:53 +0000
304+++ container/lxc/mock/mock-lxc.go 2013-06-27 04:53:25 +0000
305@@ -12,12 +12,46 @@
306 // This file provides a mock implementation of the golxc interfaces
307 // ContainerFactory and Container.
308
309+type Action int
310+
311+const (
312+ // A container has been started.
313+ Started Action = iota
314+ // A container has been stopped.
315+ Stopped
316+)
317+
318+func (action Action) String() string {
319+ switch action {
320+ case Started:
321+ return "Started"
322+ case Stopped:
323+ return "Stopped"
324+ }
325+ return "unknown"
326+}
327+
328+type Event struct {
329+ Action Action
330+ InstanceId string
331+}
332+
333+type ContainerFactory interface {
334+ golxc.ContainerFactory
335+
336+ AddListener(chan<- Event)
337+ RemoveListener(chan<- Event)
338+}
339+
340 type mockFactory struct {
341 instances map[string]golxc.Container
342+ listeners []chan<- Event
343 }
344
345-func MockFactory() golxc.ContainerFactory {
346- return &mockFactory{make(map[string]golxc.Container)}
347+func MockFactory() ContainerFactory {
348+ return &mockFactory{
349+ instances: make(map[string]golxc.Container),
350+ }
351 }
352
353 type mockContainer struct {
354@@ -35,6 +69,9 @@
355
356 // Create creates a new container based on the given template.
357 func (mock *mockContainer) Create(configFile, template string, templateArgs ...string) error {
358+ if mock.state != golxc.StateUnknown {
359+ return fmt.Errorf("container is already created")
360+ }
361 mock.state = golxc.StateStopped
362 mock.factory.instances[mock.name] = mock
363 return nil
364@@ -42,13 +79,25 @@
365
366 // Start runs the container as a daemon.
367 func (mock *mockContainer) Start(configFile, consoleFile string) error {
368+ if mock.state == golxc.StateUnknown {
369+ return fmt.Errorf("container has not been created")
370+ } else if mock.state == golxc.StateRunning {
371+ return fmt.Errorf("container is already running")
372+ }
373 mock.state = golxc.StateRunning
374+ mock.factory.notify(Started, mock.name)
375 return nil
376 }
377
378 // Stop terminates the running container.
379 func (mock *mockContainer) Stop() error {
380+ if mock.state == golxc.StateUnknown {
381+ return fmt.Errorf("container has not been created")
382+ } else if mock.state == golxc.StateStopped {
383+ return fmt.Errorf("container is already stopped")
384+ }
385 mock.state = golxc.StateStopped
386+ mock.factory.notify(Stopped, mock.name)
387 return nil
388 }
389
390@@ -76,6 +125,11 @@
391
392 // Destroy stops and removes the container.
393 func (mock *mockContainer) Destroy() error {
394+ if mock.state == golxc.StateUnknown {
395+ return fmt.Errorf("container has not been created")
396+ } else if mock.state == golxc.StateRunning {
397+ return fmt.Errorf("container is running")
398+ }
399 mock.state = golxc.StateUnknown
400 delete(mock.factory.instances, mock.name)
401 return nil
402@@ -129,8 +183,16 @@
403 mock.logLevel = level
404 }
405
406+func (mock *mockFactory) String() string {
407+ return fmt.Sprintf("mock lxc factory")
408+}
409+
410 func (mock *mockFactory) New(name string) golxc.Container {
411- container := &mockContainer{
412+ container, ok := mock.instances[name]
413+ if ok {
414+ return container
415+ }
416+ container = &mockContainer{
417 factory: mock,
418 name: name,
419 state: golxc.StateUnknown,
420@@ -145,3 +207,24 @@
421 }
422 return
423 }
424+
425+func (mock *mockFactory) notify(action Action, instanceId string) {
426+ event := Event{action, instanceId}
427+ for _, c := range mock.listeners {
428+ c <- event
429+ }
430+}
431+
432+func (mock *mockFactory) AddListener(listener chan<- Event) {
433+ mock.listeners = append(mock.listeners, listener)
434+}
435+
436+func (mock *mockFactory) RemoveListener(listener chan<- Event) {
437+ pos := 0
438+ for i, c := range mock.listeners {
439+ if c == listener {
440+ pos = i
441+ }
442+ }
443+ mock.listeners = append(mock.listeners[:pos], mock.listeners[pos+1:]...)
444+}
445
446=== modified file 'container/lxc/test.go'
447--- container/lxc/test.go 2013-06-24 01:52:54 +0000
448+++ container/lxc/test.go 2013-06-27 04:53:25 +0000
449@@ -7,6 +7,12 @@
450
451 package lxc
452
453+import (
454+ . "launchpad.net/gocheck"
455+ "launchpad.net/golxc"
456+ "launchpad.net/juju-core/container/lxc/mock"
457+)
458+
459 // SetContainerDir allows tests in other packages to override the
460 // containerDir.
461 func SetContainerDir(dir string) (old string) {
462@@ -27,3 +33,45 @@
463 old, removedContainerDir = removedContainerDir, dir
464 return
465 }
466+
467+// SetLxcFactory allows tests in other packages to override the lxcObjectFactory
468+func SetLxcFactory(factory golxc.ContainerFactory) (old golxc.ContainerFactory) {
469+ logger.Infof("lxcObjectFactory replaced with %v", factory)
470+ old, lxcObjectFactory = lxcObjectFactory, factory
471+ return
472+}
473+
474+// TestSuite replaces the lxc factory that the broker uses with a mock
475+// implementation.
476+type TestSuite struct {
477+ Factory mock.ContainerFactory
478+ oldFactory golxc.ContainerFactory
479+ ContainerDir string
480+ RemovedDir string
481+ LxcDir string
482+ oldContainerDir string
483+ oldRemovedDir string
484+ oldLxcContainerDir string
485+}
486+
487+func (s *TestSuite) SetUpSuite(c *C) {}
488+
489+func (s *TestSuite) TearDownSuite(c *C) {}
490+
491+func (s *TestSuite) SetUpTest(c *C) {
492+ s.ContainerDir = c.MkDir()
493+ s.oldContainerDir = SetContainerDir(s.ContainerDir)
494+ s.RemovedDir = c.MkDir()
495+ s.oldRemovedDir = SetRemovedContainerDir(s.RemovedDir)
496+ s.LxcDir = c.MkDir()
497+ s.oldLxcContainerDir = SetLxcContainerDir(s.LxcDir)
498+ s.Factory = mock.MockFactory()
499+ s.oldFactory = SetLxcFactory(s.Factory)
500+}
501+
502+func (s *TestSuite) TearDownTest(c *C) {
503+ SetContainerDir(s.oldContainerDir)
504+ SetLxcContainerDir(s.oldLxcContainerDir)
505+ SetRemovedContainerDir(s.oldRemovedDir)
506+ SetLxcFactory(s.oldFactory)
507+}
508
509=== modified file 'juju/testing/conn.go'
510--- juju/testing/conn.go 2013-06-26 13:19:16 +0000
511+++ juju/testing/conn.go 2013-06-27 04:53:25 +0000
512@@ -222,6 +222,7 @@
513 c.Logf("cannot reset admin password: %v", err)
514 }
515 c.Assert(s.Conn.Close(), IsNil)
516+ c.Assert(s.APIConn.Close(), IsNil)
517 dummy.Reset()
518 s.Conn = nil
519 s.State = nil
520
521=== modified file 'state/state.go'
522--- state/state.go 2013-06-26 15:38:39 +0000
523+++ state/state.go 2013-06-27 04:53:25 +0000
524@@ -534,6 +534,7 @@
525 prefix, id := tag[0:i], tag[i+1:]
526 switch prefix {
527 case "machine":
528+ id = MachineIdFromTag(tag)
529 if !IsMachineId(id) {
530 return nil, fmt.Errorf("invalid entity tag %q", tag)
531 }
532
533=== modified file 'worker/provisioner/lxc-broker.go'
534--- worker/provisioner/lxc-broker.go 2013-06-25 22:01:36 +0000
535+++ worker/provisioner/lxc-broker.go 2013-06-27 04:53:25 +0000
536@@ -4,7 +4,6 @@
537 package provisioner
538
539 import (
540- "launchpad.net/golxc"
541 "launchpad.net/juju-core/constraints"
542 "launchpad.net/juju-core/container/lxc"
543 "launchpad.net/juju-core/environs/config"
544@@ -18,17 +17,15 @@
545
546 var _ Broker = (*lxcBroker)(nil)
547
548-func NewLxcBroker(factory golxc.ContainerFactory, config *config.Config, tools *state.Tools) Broker {
549+func NewLxcBroker(config *config.Config, tools *state.Tools) Broker {
550 return &lxcBroker{
551- golxc: factory,
552- manager: lxc.NewContainerManager(factory, "juju"),
553+ manager: lxc.NewContainerManager("juju"),
554 config: config,
555 tools: tools,
556 }
557 }
558
559 type lxcBroker struct {
560- golxc golxc.ContainerFactory
561 manager lxc.ContainerManager
562 config *config.Config
563 tools *state.Tools
564@@ -42,6 +39,7 @@
565 lxcLogger.Errorf("failed to start container: %v", err)
566 return nil, nil, err
567 }
568+ lxcLogger.Infof("started lxc container for machineId: %s, %s", machineId, inst.Id())
569 return inst, nil, nil
570 }
571
572
573=== modified file 'worker/provisioner/lxc-broker_test.go'
574--- worker/provisioner/lxc-broker_test.go 2013-06-25 22:02:29 +0000
575+++ worker/provisioner/lxc-broker_test.go 2013-06-27 04:53:25 +0000
576@@ -4,13 +4,18 @@
577 package provisioner_test
578
579 import (
580+ "fmt"
581+ "io/ioutil"
582+ "os"
583 "path/filepath"
584+ "time"
585
586 . "launchpad.net/gocheck"
587- "launchpad.net/golxc"
588 "launchpad.net/juju-core/constraints"
589 "launchpad.net/juju-core/container/lxc"
590 "launchpad.net/juju-core/container/lxc/mock"
591+ "launchpad.net/juju-core/environs/agent"
592+ "launchpad.net/juju-core/environs/config"
593 "launchpad.net/juju-core/instance"
594 jujutesting "launchpad.net/juju-core/juju/testing"
595 "launchpad.net/juju-core/state"
596@@ -20,49 +25,54 @@
597 "launchpad.net/juju-core/worker/provisioner"
598 )
599
600+type lxcSuite struct {
601+ testing.LoggingSuite
602+ lxc.TestSuite
603+ events chan mock.Event
604+}
605+
606 type lxcBrokerSuite struct {
607- testing.LoggingSuite
608- golxc golxc.ContainerFactory
609- broker provisioner.Broker
610- containerDir string
611- removedDir string
612- lxcDir string
613- oldContainerDir string
614- oldRemovedDir string
615- oldLxcContainerDir string
616+ lxcSuite
617+ broker provisioner.Broker
618 }
619
620 var _ = Suite(&lxcBrokerSuite{})
621
622-func (s *lxcBrokerSuite) SetUpSuite(c *C) {
623+func (s *lxcSuite) SetUpSuite(c *C) {
624 s.LoggingSuite.SetUpSuite(c)
625+ s.TestSuite.SetUpSuite(c)
626 }
627
628-func (s *lxcBrokerSuite) TearDownSuite(c *C) {
629+func (s *lxcSuite) TearDownSuite(c *C) {
630+ s.TestSuite.TearDownSuite(c)
631 s.LoggingSuite.TearDownSuite(c)
632 }
633
634+func (s *lxcSuite) SetUpTest(c *C) {
635+ s.LoggingSuite.SetUpTest(c)
636+ s.TestSuite.SetUpTest(c)
637+ s.events = make(chan mock.Event)
638+ go func() {
639+ for event := range s.events {
640+ c.Output(3, fmt.Sprintf("lxc event: <%s, %s>", event.Action, event.InstanceId))
641+ }
642+ }()
643+ s.TestSuite.Factory.AddListener(s.events)
644+}
645+
646+func (s *lxcSuite) TearDownTest(c *C) {
647+ close(s.events)
648+ s.TestSuite.TearDownTest(c)
649+ s.LoggingSuite.TearDownTest(c)
650+}
651+
652 func (s *lxcBrokerSuite) SetUpTest(c *C) {
653- s.LoggingSuite.SetUpTest(c)
654- s.containerDir = c.MkDir()
655- s.oldContainerDir = lxc.SetContainerDir(s.containerDir)
656- s.removedDir = c.MkDir()
657- s.oldRemovedDir = lxc.SetRemovedContainerDir(s.removedDir)
658- s.lxcDir = c.MkDir()
659- s.oldLxcContainerDir = lxc.SetLxcContainerDir(s.lxcDir)
660- s.golxc = mock.MockFactory()
661+ s.lxcSuite.SetUpTest(c)
662 tools := &state.Tools{
663 Binary: version.MustParseBinary("2.3.4-foo-bar"),
664 URL: "http://tools.example.com/2.3.4-foo-bar.tgz",
665 }
666- s.broker = provisioner.NewLxcBroker(s.golxc, testing.EnvironConfig(c), tools)
667-}
668-
669-func (s *lxcBrokerSuite) TearDownTest(c *C) {
670- lxc.SetContainerDir(s.oldContainerDir)
671- lxc.SetLxcContainerDir(s.oldLxcContainerDir)
672- lxc.SetRemovedContainerDir(s.oldRemovedDir)
673- s.LoggingSuite.TearDownTest(c)
674+ s.broker = provisioner.NewLxcBroker(testing.EnvironConfig(c), tools)
675 }
676
677 func (s *lxcBrokerSuite) startInstance(c *C, machineId string) instance.Instance {
678@@ -119,9 +129,125 @@
679 }
680
681 func (s *lxcBrokerSuite) lxcContainerDir(inst instance.Instance) string {
682- return filepath.Join(s.containerDir, string(inst.Id()))
683+ return filepath.Join(s.ContainerDir, string(inst.Id()))
684 }
685
686 func (s *lxcBrokerSuite) lxcRemovedContainerDir(inst instance.Instance) string {
687- return filepath.Join(s.removedDir, string(inst.Id()))
688+ return filepath.Join(s.RemovedDir, string(inst.Id()))
689+}
690+
691+type lxcProvisionerSuite struct {
692+ CommonProvisionerSuite
693+ lxcSuite
694+ machineId string
695+ events chan mock.Event
696+}
697+
698+var _ = Suite(&lxcProvisionerSuite{})
699+
700+func (s *lxcProvisionerSuite) SetUpSuite(c *C) {
701+ s.CommonProvisionerSuite.SetUpSuite(c)
702+ s.lxcSuite.SetUpSuite(c)
703+}
704+
705+func (s *lxcProvisionerSuite) TearDownSuite(c *C) {
706+ s.lxcSuite.TearDownSuite(c)
707+ s.CommonProvisionerSuite.TearDownSuite(c)
708+}
709+
710+func (s *lxcProvisionerSuite) SetUpTest(c *C) {
711+ s.CommonProvisionerSuite.SetUpTest(c)
712+ s.lxcSuite.SetUpTest(c)
713+ // Write the tools file.
714+ toolsDir := agent.SharedToolsDir(s.DataDir(), version.Current)
715+ c.Assert(os.MkdirAll(toolsDir, 0755), IsNil)
716+ urlPath := filepath.Join(toolsDir, "downloaded-url.txt")
717+ err := ioutil.WriteFile(urlPath, []byte("http://example.com/tools"), 0644)
718+ c.Assert(err, IsNil)
719+
720+ // The lxc provisioner actually needs the machine it is being created on
721+ // to be in state, in order to get the watcher.
722+ m, err := s.State.AddMachine(config.DefaultSeries, state.JobHostUnits)
723+ c.Assert(err, IsNil)
724+ s.machineId = m.Id()
725+
726+ s.events = make(chan mock.Event, 25)
727+ s.Factory.AddListener(s.events)
728+}
729+
730+func (s *lxcProvisionerSuite) expectStarted(c *C, machine *state.Machine) {
731+ event := <-s.events
732+ c.Assert(event.Action, Equals, mock.Started)
733+ err := machine.Refresh()
734+ c.Assert(err, IsNil)
735+ s.waitInstanceId(c, machine, instance.Id(event.InstanceId))
736+}
737+
738+func (s *lxcProvisionerSuite) expectStopped(c *C, machine *state.Machine) {
739+ event := <-s.events
740+ c.Assert(event.Action, Equals, mock.Stopped)
741+ inst, ok := machine.InstanceId()
742+ c.Assert(ok, IsTrue)
743+ c.Assert(string(inst), Equals, event.InstanceId)
744+}
745+
746+func (s *lxcProvisionerSuite) expectNoEvents(c *C) {
747+ select {
748+ case event := <-s.events:
749+ c.Fatalf("unexpected event %#v", event)
750+ case <-time.After(200 * time.Millisecond):
751+ return
752+ }
753+}
754+
755+func (s *lxcProvisionerSuite) TearDownTest(c *C) {
756+ close(s.events)
757+ s.lxcSuite.TearDownTest(c)
758+ s.CommonProvisionerSuite.TearDownTest(c)
759+}
760+
761+func (s *lxcProvisionerSuite) newLxcProvisioner() *provisioner.Provisioner {
762+ return provisioner.NewProvisioner(provisioner.LXC, s.State, s.machineId, s.DataDir())
763+}
764+
765+func (s *lxcProvisionerSuite) TestProvisionerStartStop(c *C) {
766+ p := s.newLxcProvisioner()
767+ c.Assert(p.Stop(), IsNil)
768+}
769+
770+func (s *lxcProvisionerSuite) TestDoesNotStartEnvironMachines(c *C) {
771+ p := s.newLxcProvisioner()
772+ defer stop(c, p)
773+
774+ // Check that an instance is not provisioned when the machine is created.
775+ _, err := s.State.AddMachine(config.DefaultSeries, state.JobHostUnits)
776+ c.Assert(err, IsNil)
777+
778+ s.expectNoEvents(c)
779+}
780+
781+func (s *lxcProvisionerSuite) addContainer(c *C) *state.Machine {
782+ params := state.AddMachineParams{
783+ ParentId: s.machineId,
784+ ContainerType: state.LXC,
785+ Series: config.DefaultSeries,
786+ Jobs: []state.MachineJob{state.JobHostUnits},
787+ }
788+ container, err := s.State.AddMachineWithConstraints(&params)
789+ c.Assert(err, IsNil)
790+ return container
791+}
792+
793+func (s *lxcProvisionerSuite) TestContainerStartedAndStopped(c *C) {
794+ p := s.newLxcProvisioner()
795+ defer stop(c, p)
796+
797+ container := s.addContainer(c)
798+
799+ s.expectStarted(c, container)
800+
801+ // ...and removed, along with the machine, when the machine is Dead.
802+ c.Assert(container.EnsureDead(), IsNil)
803+ s.expectStopped(c, container)
804+ s.waitRemoved(c, container)
805 }
806
807=== modified file 'worker/provisioner/provisioner.go'
808--- worker/provisioner/provisioner.go 2013-06-25 21:36:48 +0000
809+++ worker/provisioner/provisioner.go 2013-06-27 04:53:25 +0000
810@@ -4,23 +4,43 @@
811 package provisioner
812
813 import (
814+ "fmt"
815 "sync"
816
817 "launchpad.net/juju-core/environs"
818+ "launchpad.net/juju-core/environs/agent"
819 "launchpad.net/juju-core/environs/config"
820 "launchpad.net/juju-core/state"
821 "launchpad.net/juju-core/state/watcher"
822+ "launchpad.net/juju-core/version"
823 "launchpad.net/juju-core/worker"
824 "launchpad.net/loggo"
825 "launchpad.net/tomb"
826 )
827
828-var logger = loggo.GetLogger("juju.provisioner")
829+type ProvisionerType string
830+
831+var (
832+ logger = loggo.GetLogger("juju.provisioner")
833+
834+ // ENVIRON provisioners create machines from the environment
835+ ENVIRON ProvisionerType = "environ"
836+ // LXC provisioners create lxc containers on their parent machine
837+ LXC ProvisionerType = "lxc"
838+)
839+
840+// While I'm debugging.
841+func init() {
842+ logger.SetLogLevel(loggo.TRACE)
843+}
844
845 // Provisioner represents a running provisioning worker.
846 type Provisioner struct {
847+ pt ProvisionerType
848 st *state.State
849 machineId string // Which machine runs the provisioner.
850+ dataDir string
851+ machine *state.Machine
852 environ environs.Environ
853 tomb tomb.Tomb
854
855@@ -44,10 +64,12 @@
856 // NewProvisioner returns a new Provisioner. When new machines
857 // are added to the state, it allocates instances from the environment
858 // and allocates them to the new machines.
859-func NewProvisioner(st *state.State, machineId string) *Provisioner {
860+func NewProvisioner(pt ProvisionerType, st *state.State, machineId, dataDir string) *Provisioner {
861 p := &Provisioner{
862+ pt: pt,
863 st: st,
864 machineId: machineId,
865+ dataDir: dataDir,
866 }
867 go func() {
868 defer p.tomb.Done()
869@@ -75,14 +97,19 @@
870
871 // Start responding to changes in machines, and to any further updates
872 // to the environment config.
873- machinesWatcher := p.st.WatchEnvironMachines()
874- environmentBroker := newEnvironBroker(p.environ)
875+ instanceBroker, err := p.getBroker()
876+ if err != nil {
877+ return err
878+ }
879+ machineWatcher, err := p.getWatcher()
880+ if err != nil {
881+ return err
882+ }
883 environmentProvisioner := NewProvisionerTask(
884- "environ provisioner for machine "+p.machineId,
885 p.machineId,
886 p.st,
887- machinesWatcher,
888- environmentBroker,
889+ machineWatcher,
890+ instanceBroker,
891 auth)
892 defer watcher.Stop(environmentProvisioner, &p.tomb)
893
894@@ -106,6 +133,56 @@
895 panic("not reached")
896 }
897
898+func (p *Provisioner) getMachine() (*state.Machine, error) {
899+ if p.machine == nil {
900+ var err error
901+ if p.machine, err = p.st.Machine(p.machineId); err != nil {
902+ logger.Errorf("machine %s is not in state", p.machineId)
903+ return nil, err
904+ }
905+ }
906+ return p.machine, nil
907+}
908+
909+func (p *Provisioner) getWatcher() (Watcher, error) {
910+ switch p.pt {
911+ case ENVIRON:
912+ return p.st.WatchEnvironMachines(), nil
913+ case LXC:
914+ machine, err := p.getMachine()
915+ if err != nil {
916+ return nil, err
917+ }
918+ return machine.WatchContainers(state.LXC), nil
919+ }
920+ return nil, fmt.Errorf("unknown provisioner type")
921+}
922+
923+func (p *Provisioner) getBroker() (Broker, error) {
924+ switch p.pt {
925+ case ENVIRON:
926+ return newEnvironBroker(p.environ), nil
927+ case LXC:
928+ config := p.environ.Config()
929+ tools, err := p.getAgentTools()
930+ if err != nil {
931+ logger.Errorf("cannot get tools from machine for lxc broker")
932+ return nil, err
933+ }
934+ return NewLxcBroker(config, tools), nil
935+ }
936+ return nil, fmt.Errorf("unknown provisioner type")
937+}
938+
939+func (p *Provisioner) getAgentTools() (*state.Tools, error) {
940+ tools, err := agent.ReadTools(p.dataDir, version.Current)
941+ if err != nil {
942+ logger.Errorf("cannot read agent tools from %q", p.dataDir)
943+ return nil, err
944+ }
945+ return tools, nil
946+}
947+
948 // setConfig updates the environment configuration and notifies
949 // the config observer.
950 func (p *Provisioner) setConfig(config *config.Config) error {
951@@ -133,7 +210,7 @@
952 }
953
954 func (p *Provisioner) String() string {
955- return "provisioning worker"
956+ return fmt.Sprintf("%s provisioning worker for machine %s", string(p.pt), p.machineId)
957 }
958
959 // Stop stops the Provisioner and returns any error encountered while
960
961=== modified file 'worker/provisioner/provisioner_task.go'
962--- worker/provisioner/provisioner_task.go 2013-06-26 13:19:16 +0000
963+++ worker/provisioner/provisioner_task.go 2013-06-27 04:53:25 +0000
964@@ -21,7 +21,6 @@
965 Stop() error
966 Dying() <-chan struct{}
967 Err() error
968- String() string
969 }
970
971 type Watcher interface {
972@@ -35,7 +34,6 @@
973 }
974
975 func NewProvisionerTask(
976- name string,
977 machineId string,
978 machineGetter MachineGetter,
979 watcher Watcher,
980@@ -43,7 +41,6 @@
981 auth AuthenticationProvider,
982 ) ProvisionerTask {
983 task := &provisionerTask{
984- name: name,
985 machineId: machineId,
986 machineGetter: machineGetter,
987 machineWatcher: watcher,
988@@ -59,7 +56,6 @@
989 }
990
991 type provisionerTask struct {
992- name string
993 machineId string
994 machineGetter MachineGetter
995 machineWatcher Watcher
996@@ -96,10 +92,6 @@
997 return task.tomb.Err()
998 }
999
1000-func (task *provisionerTask) String() string {
1001- return task.name
1002-}
1003-
1004 func (task *provisionerTask) loop() error {
1005 logger.Infof("Starting up provisioner task %s", task.machineId)
1006 defer watcher.Stop(task.machineWatcher, &task.tomb)
1007@@ -141,15 +133,15 @@
1008 return err
1009 }
1010
1011- // Find running instances that have no machines associated
1012- unknown, err := task.findUnknownInstances()
1013- if err != nil {
1014- return err
1015- }
1016-
1017 // Stop all machines that are dead
1018 stopping := task.instancesForMachines(dead)
1019
1020+ // Find running instances that have no machines associated
1021+ unknown, err := task.findUnknownInstances(stopping)
1022+ if err != nil {
1023+ return err
1024+ }
1025+
1026 // It's important that we stop unknown instances before starting
1027 // pending ones, because if we start an instance and then fail to
1028 // set its InstanceId on the machine we don't want to start a new
1029@@ -238,11 +230,13 @@
1030 logger.Infof("machine %v already started as instance %q", machine, instId)
1031 }
1032 }
1033+ logger.Tracef("pending machines: %v", pending)
1034+ logger.Tracef("dead machines: %v", dead)
1035 return
1036 }
1037
1038 // findUnknownInstances finds instances which are not associated with a machine.
1039-func (task *provisionerTask) findUnknownInstances() ([]instance.Instance, error) {
1040+func (task *provisionerTask) findUnknownInstances(stopping []instance.Instance) ([]instance.Instance, error) {
1041 // Make a copy of the instances we know about.
1042 instances := make(map[instance.Id]instance.Instance)
1043 for k, v := range task.instances {
1044@@ -254,6 +248,11 @@
1045 delete(instances, instId)
1046 }
1047 }
1048+ // Now remove all those instances that we are stopping already, as they
1049+ // have been removed from the task.machines map.
1050+ for _, i := range stopping {
1051+ delete(instances, i.Id())
1052+ }
1053 var unknown []instance.Instance
1054 for _, i := range instances {
1055 unknown = append(unknown, i)
1056@@ -336,6 +335,7 @@
1057 return nil
1058 }
1059 if err := machine.SetProvisioned(inst.Id(), nonce); err != nil {
1060+ logger.Errorf("cannot register instance for machine %v: %v", machine, err)
1061 // The machine is started, but we can't record the mapping in
1062 // state. It'll keep running while we fail out and restart,
1063 // but will then be detected by findUnknownInstances and
1064
1065=== modified file 'worker/provisioner/provisioner_test.go'
1066--- worker/provisioner/provisioner_test.go 2013-06-20 11:03:10 +0000
1067+++ worker/provisioner/provisioner_test.go 2013-06-27 04:53:25 +0000
1068@@ -30,12 +30,16 @@
1069 coretesting.MgoTestPackage(t)
1070 }
1071
1072-type ProvisionerSuite struct {
1073+type CommonProvisionerSuite struct {
1074 testing.JujuConnSuite
1075 op <-chan dummy.Operation
1076 cfg *config.Config
1077 }
1078
1079+type ProvisionerSuite struct {
1080+ CommonProvisionerSuite
1081+}
1082+
1083 var _ = Suite(&ProvisionerSuite{})
1084
1085 var veryShortAttempt = utils.AttemptStrategy{
1086@@ -45,7 +49,7 @@
1087
1088 var _ worker.Worker = (*provisioner.Provisioner)(nil)
1089
1090-func (s *ProvisionerSuite) SetUpTest(c *C) {
1091+func (s *CommonProvisionerSuite) SetUpTest(c *C) {
1092 s.JujuConnSuite.SetUpTest(c)
1093 // Create the operations channel with more than enough space
1094 // for those tests that don't listen on it.
1095@@ -74,7 +78,7 @@
1096 // invalidateEnvironment alters the environment configuration
1097 // so the Settings returned from the watcher will not pass
1098 // validation.
1099-func (s *ProvisionerSuite) invalidateEnvironment(c *C) error {
1100+func (s *CommonProvisionerSuite) invalidateEnvironment(c *C) error {
1101 admindb := s.Session.DB("admin")
1102 err := admindb.Login("admin", testing.AdminSecret)
1103 if err != nil {
1104@@ -86,7 +90,7 @@
1105 }
1106
1107 // fixEnvironment undoes the work of invalidateEnvironment.
1108-func (s *ProvisionerSuite) fixEnvironment() error {
1109+func (s *CommonProvisionerSuite) fixEnvironment() error {
1110 return s.State.SetEnvironConfig(s.cfg)
1111 }
1112
1113@@ -100,11 +104,11 @@
1114 c.Assert(s.Stop(), IsNil)
1115 }
1116
1117-func (s *ProvisionerSuite) checkStartInstance(c *C, m *state.Machine) instance.Instance {
1118+func (s *CommonProvisionerSuite) checkStartInstance(c *C, m *state.Machine) instance.Instance {
1119 return s.checkStartInstanceCustom(c, m, "pork", constraints.Value{})
1120 }
1121
1122-func (s *ProvisionerSuite) checkStartInstanceCustom(c *C, m *state.Machine, secret string, cons constraints.Value) (instance instance.Instance) {
1123+func (s *CommonProvisionerSuite) checkStartInstanceCustom(c *C, m *state.Machine, secret string, cons constraints.Value) (instance instance.Instance) {
1124 s.State.StartSync()
1125 for {
1126 select {
1127@@ -149,7 +153,7 @@
1128 }
1129
1130 // checkNoOperations checks that the environ was not operated upon.
1131-func (s *ProvisionerSuite) checkNoOperations(c *C) {
1132+func (s *CommonProvisionerSuite) checkNoOperations(c *C) {
1133 s.State.StartSync()
1134 select {
1135 case o := <-s.op:
1136@@ -160,7 +164,7 @@
1137 }
1138
1139 // checkStopInstances checks that an instance has been stopped.
1140-func (s *ProvisionerSuite) checkStopInstances(c *C, instances ...instance.Instance) {
1141+func (s *CommonProvisionerSuite) checkStopInstances(c *C, instances ...instance.Instance) {
1142 s.State.StartSync()
1143 instanceIds := set.NewStrings()
1144 for _, instance := range instances {
1145@@ -187,7 +191,7 @@
1146 }
1147 }
1148
1149-func (s *ProvisionerSuite) waitMachine(c *C, m *state.Machine, check func() bool) {
1150+func (s *CommonProvisionerSuite) waitMachine(c *C, m *state.Machine, check func() bool) {
1151 w := m.Watch()
1152 defer stop(c, w)
1153 timeout := time.After(500 * time.Millisecond)
1154@@ -208,7 +212,7 @@
1155 }
1156
1157 // waitRemoved waits for the supplied machine to be removed from state.
1158-func (s *ProvisionerSuite) waitRemoved(c *C, m *state.Machine) {
1159+func (s *CommonProvisionerSuite) waitRemoved(c *C, m *state.Machine) {
1160 s.waitMachine(c, m, func() bool {
1161 err := m.Refresh()
1162 if errors.IsNotFoundError(err) {
1163@@ -222,7 +226,7 @@
1164
1165 // waitInstanceId waits until the supplied machine has an instance id, then
1166 // asserts it is as expected.
1167-func (s *ProvisionerSuite) waitInstanceId(c *C, m *state.Machine, expect instance.Id) {
1168+func (s *CommonProvisionerSuite) waitInstanceId(c *C, m *state.Machine, expect instance.Id) {
1169 s.waitMachine(c, m, func() bool {
1170 err := m.Refresh()
1171 c.Assert(err, IsNil)
1172@@ -235,13 +239,17 @@
1173 })
1174 }
1175
1176+func (s *ProvisionerSuite) newEnvironProvisioner(machineId string) *provisioner.Provisioner {
1177+ return provisioner.NewProvisioner(provisioner.ENVIRON, s.State, machineId, "")
1178+}
1179+
1180 func (s *ProvisionerSuite) TestProvisionerStartStop(c *C) {
1181- p := provisioner.NewProvisioner(s.State, "0")
1182+ p := s.newEnvironProvisioner("0")
1183 c.Assert(p.Stop(), IsNil)
1184 }
1185
1186 func (s *ProvisionerSuite) TestSimple(c *C) {
1187- p := provisioner.NewProvisioner(s.State, "0")
1188+ p := s.newEnvironProvisioner("0")
1189 defer stop(c, p)
1190
1191 // Check that an instance is provisioned when the machine is created...
1192@@ -264,14 +272,14 @@
1193 c.Assert(err, IsNil)
1194
1195 // Start a provisioner and check those constraints are used.
1196- p := provisioner.NewProvisioner(s.State, "0")
1197+ p := s.newEnvironProvisioner("0")
1198 defer stop(c, p)
1199 s.checkStartInstanceCustom(c, m, "pork", cons)
1200 }
1201
1202 func (s *ProvisionerSuite) TestProvisionerSetsErrorStatusWhenStartInstanceFailed(c *C) {
1203 brokenMsg := breakDummyProvider(c, s.State, "StartInstance")
1204- p := provisioner.NewProvisioner(s.State, "0")
1205+ p := s.newEnvironProvisioner("0")
1206 defer stop(c, p)
1207
1208 // Check that an instance is not provisioned when the machine is created...
1209@@ -291,13 +299,13 @@
1210
1211 // Restart the PA to make sure the machine is skipped again.
1212 stop(c, p)
1213- p = provisioner.NewProvisioner(s.State, "0")
1214+ p = s.newEnvironProvisioner("0")
1215 defer stop(c, p)
1216 s.checkNoOperations(c)
1217 }
1218
1219 func (s *ProvisionerSuite) TestProvisioningDoesNotOccurForContainers(c *C) {
1220- p := provisioner.NewProvisioner(s.State, "0")
1221+ p := s.newEnvironProvisioner("0")
1222 defer stop(c, p)
1223
1224 // create a machine to host the container.
1225@@ -330,7 +338,7 @@
1226 err := s.invalidateEnvironment(c)
1227 c.Assert(err, IsNil)
1228
1229- p := provisioner.NewProvisioner(s.State, "0")
1230+ p := s.newEnvironProvisioner("0")
1231 defer stop(c, p)
1232
1233 // try to create a machine
1234@@ -345,7 +353,7 @@
1235 err := s.invalidateEnvironment(c)
1236 c.Assert(err, IsNil)
1237
1238- p := provisioner.NewProvisioner(s.State, "0")
1239+ p := s.newEnvironProvisioner("0")
1240 defer stop(c, p)
1241
1242 // try to create a machine
1243@@ -362,7 +370,7 @@
1244 }
1245
1246 func (s *ProvisionerSuite) TestProvisioningDoesOccurAfterInvalidEnvironmentPublished(c *C) {
1247- p := provisioner.NewProvisioner(s.State, "0")
1248+ p := s.newEnvironProvisioner("0")
1249 defer stop(c, p)
1250
1251 // place a new machine into the state
1252@@ -383,7 +391,7 @@
1253 }
1254
1255 func (s *ProvisionerSuite) TestProvisioningDoesNotProvisionTheSameMachineAfterRestart(c *C) {
1256- p := provisioner.NewProvisioner(s.State, "0")
1257+ p := s.newEnvironProvisioner("0")
1258 defer stop(c, p)
1259
1260 // create a machine
1261@@ -393,7 +401,7 @@
1262
1263 // restart the PA
1264 stop(c, p)
1265- p = provisioner.NewProvisioner(s.State, "0")
1266+ p = s.newEnvironProvisioner("0")
1267 defer stop(c, p)
1268
1269 // check that there is only one machine known
1270@@ -407,7 +415,7 @@
1271 }
1272
1273 func (s *ProvisionerSuite) TestProvisioningStopsInstances(c *C) {
1274- p := provisioner.NewProvisioner(s.State, "0")
1275+ p := s.newEnvironProvisioner("0")
1276 defer stop(c, p)
1277
1278 // create a machine
1279@@ -429,14 +437,14 @@
1280 c.Assert(m1.Remove(), IsNil)
1281
1282 // start a new provisioner to shut them both down
1283- p = provisioner.NewProvisioner(s.State, "0")
1284+ p = s.newEnvironProvisioner("0")
1285 defer stop(c, p)
1286 s.checkStopInstances(c, i0, i1)
1287 s.waitRemoved(c, m0)
1288 }
1289
1290 func (s *ProvisionerSuite) TestDyingMachines(c *C) {
1291- p := provisioner.NewProvisioner(s.State, "0")
1292+ p := s.newEnvironProvisioner("0")
1293 defer stop(c, p)
1294
1295 // provision a machine
1296@@ -456,7 +464,7 @@
1297 c.Assert(err, IsNil)
1298
1299 // start the provisioner and wait for it to reap the useless machine
1300- p = provisioner.NewProvisioner(s.State, "0")
1301+ p = s.newEnvironProvisioner("0")
1302 defer stop(c, p)
1303 s.checkNoOperations(c)
1304 s.waitRemoved(c, m1)
1305@@ -468,7 +476,7 @@
1306 }
1307
1308 func (s *ProvisionerSuite) TestProvisioningRecoversAfterInvalidEnvironmentPublished(c *C) {
1309- p := provisioner.NewProvisioner(s.State, "0")
1310+ p := s.newEnvironProvisioner("0")
1311 defer stop(c, p)
1312
1313 // place a new machine into the state

Subscribers

People subscribed via source and target branches

to status/vote changes: