juju-core

Merge lp:~axwalk/juju-core/lp1317197-containerinit-hooklock-1.18 into lp:juju-core/1.18

  1. lp1317197-containerinit-hooklock-1.18
  2. Merge into 1.18
Proposed by Andrew Wilkins
Status: Merged
Approved by: Andrew Wilkins
Approved revision: no longer in the source branch.
Merged at revision: 2293
Proposed branch: lp:~axwalk/juju-core/lp1317197-containerinit-hooklock-1.18
Merge into: lp:juju-core/1.18
Diff against target: 317 lines (+82/-25)
7 files modified
cmd/jujud/agent.go (+10/-0)
cmd/jujud/machine.go (+9/-5)
cmd/jujud/unit.go (+6/-2)
worker/provisioner/container_initialisation.go (+15/-3)
worker/provisioner/container_initialisation_test.go (+34/-6)
worker/uniter/uniter.go (+4/-8)
worker/uniter/uniter_test.go (+4/-1)
To merge this branch: bzr merge lp:~axwalk/juju-core/lp1317197-containerinit-hooklock-1.18
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+219283@code.launchpad.net

Commit message

Take hook exec lock in container host init

When initialising the host for a container type,
we will install packages (lxc, kvm, etc.); this can
conflict with hook execution. This CL modifies the
container initialisation code to acquire the hook
execution lock when initialisting the host to avoid
the conflict.

Fixes lp:1317197

Description of the change

Take hook exec lock in container host init

When initialising the host for a container type,
we will install packages (lxc, kvm, etc.); this can
conflict with hook execution. This CL modifies the
container initialisation code to acquire the hook
execution lock when initialisting the host to avoid
the conflict.

Fixes lp:1317197

To post a comment you must log in.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cmd/jujud/agent.go'
2--- cmd/jujud/agent.go 2014-03-12 02:28:30 +0000
3+++ cmd/jujud/agent.go 2014-05-13 03:52:24 +0000
4@@ -6,6 +6,7 @@
5 import (
6 "fmt"
7 "io"
8+ "path/filepath"
9 "time"
10
11 "launchpad.net/gnuflag"
12@@ -21,6 +22,7 @@
13 apideployer "launchpad.net/juju-core/state/api/deployer"
14 "launchpad.net/juju-core/state/api/params"
15 apirsyslog "launchpad.net/juju-core/state/api/rsyslog"
16+ "launchpad.net/juju-core/utils/fslock"
17 "launchpad.net/juju-core/version"
18 "launchpad.net/juju-core/worker"
19 "launchpad.net/juju-core/worker/deployer"
20@@ -272,3 +274,11 @@
21 }
22 return rsyslog.NewRsyslogConfigWorker(st, mode, tag, namespace, addrs)
23 }
24+
25+// hookExecutionLock returns an *fslock.Lock suitable for use as a unit
26+// hook execution lock. Other workers may also use this lock if they
27+// require isolation from hook execution.
28+func hookExecutionLock(dataDir string) (*fslock.Lock, error) {
29+ lockDir := filepath.Join(dataDir, "locks")
30+ return fslock.NewLock(lockDir, "uniter-hook-execution")
31+}
32
33=== modified file 'cmd/jujud/machine.go'
34--- cmd/jujud/machine.go 2014-03-20 09:34:29 +0000
35+++ cmd/jujud/machine.go 2014-05-13 03:52:24 +0000
36@@ -230,7 +230,7 @@
37 }
38
39 // Perform the operations needed to set up hosting for containers.
40- if err := a.setupContainerSupport(runner, st, entity); err != nil {
41+ if err := a.setupContainerSupport(runner, st, entity, agentConfig); err != nil {
42 return nil, fmt.Errorf("setting up container support: %v", err)
43 }
44 for _, job := range entity.Jobs() {
45@@ -266,7 +266,7 @@
46
47 // setupContainerSupport determines what containers can be run on this machine and
48 // initialises suitable infrastructure to support such containers.
49-func (a *MachineAgent) setupContainerSupport(runner worker.Runner, st *api.State, entity *apiagent.Entity) error {
50+func (a *MachineAgent) setupContainerSupport(runner worker.Runner, st *api.State, entity *apiagent.Entity, agentConfig agent.Config) error {
51 var supportedContainers []instance.ContainerType
52 // We don't yet support nested lxc containers but anything else can run an LXC container.
53 if entity.ContainerType() != instance.LXC {
54@@ -279,7 +279,7 @@
55 if err == nil && supportsKvm {
56 supportedContainers = append(supportedContainers, instance.KVM)
57 }
58- return a.updateSupportedContainers(runner, st, entity.Tag(), supportedContainers)
59+ return a.updateSupportedContainers(runner, st, entity.Tag(), supportedContainers, agentConfig)
60 }
61
62 // updateSupportedContainers records in state that a machine can run the specified containers.
63@@ -287,7 +287,7 @@
64 // the watcher is killed, the machine is set up to be able to start containers of the given type,
65 // and a suitable provisioner is started.
66 func (a *MachineAgent) updateSupportedContainers(runner worker.Runner, st *api.State,
67- tag string, containers []instance.ContainerType) error {
68+ tag string, containers []instance.ContainerType, agentConfig agent.Config) error {
69
70 var machine *apiprovisioner.Machine
71 var err error
72@@ -304,9 +304,13 @@
73 if err := machine.SetSupportedContainers(containers...); err != nil {
74 return fmt.Errorf("setting supported containers for %s: %v", tag, err)
75 }
76+ initLock, err := hookExecutionLock(agentConfig.DataDir())
77+ if err != nil {
78+ return err
79+ }
80 // Start the watcher to fire when a container is first requested on the machine.
81 watcherName := fmt.Sprintf("%s-container-watcher", machine.Id())
82- handler := provisioner.NewContainerSetupHandler(runner, watcherName, containers, machine, pr, a.Conf.config)
83+ handler := provisioner.NewContainerSetupHandler(runner, watcherName, containers, machine, pr, a.Conf.config, initLock)
84 a.startWorkerAfterUpgrade(runner, watcherName, func() (worker.Worker, error) {
85 return worker.NewStringsWorker(handler), nil
86 })
87
88=== modified file 'cmd/jujud/unit.go'
89--- cmd/jujud/unit.go 2014-03-20 06:16:58 +0000
90+++ cmd/jujud/unit.go 2014-05-13 03:52:24 +0000
91@@ -82,11 +82,15 @@
92
93 func (a *UnitAgent) APIWorkers() (worker.Worker, error) {
94 agentConfig := a.Conf.config
95+ dataDir := agentConfig.DataDir()
96+ hookLock, err := hookExecutionLock(dataDir)
97+ if err != nil {
98+ return nil, err
99+ }
100 st, entity, err := openAPIState(agentConfig, a)
101 if err != nil {
102 return nil, err
103 }
104- dataDir := a.Conf.dataDir
105 runner := worker.NewRunner(connectionIsFatal(st), moreImportant)
106 runner.StartWorker("upgrader", func() (worker.Worker, error) {
107 return upgrader.NewUpgrader(st.Upgrader(), agentConfig), nil
108@@ -95,7 +99,7 @@
109 return workerlogger.NewLogger(st.Logger(), agentConfig), nil
110 })
111 runner.StartWorker("uniter", func() (worker.Worker, error) {
112- return uniter.NewUniter(st.Uniter(), entity.Tag(), dataDir), nil
113+ return uniter.NewUniter(st.Uniter(), entity.Tag(), dataDir, hookLock), nil
114 })
115 runner.StartWorker("rsyslog", func() (worker.Worker, error) {
116 return newRsyslogConfigWorker(st.Rsyslog(), agentConfig, rsyslog.RsyslogModeForwarding)
117
118=== modified file 'worker/provisioner/container_initialisation.go'
119--- worker/provisioner/container_initialisation.go 2014-05-09 03:54:53 +0000
120+++ worker/provisioner/container_initialisation.go 2014-05-13 03:52:24 +0000
121@@ -17,6 +17,7 @@
122 "launchpad.net/juju-core/state/api/params"
123 apiprovisioner "launchpad.net/juju-core/state/api/provisioner"
124 "launchpad.net/juju-core/state/api/watcher"
125+ "launchpad.net/juju-core/utils/fslock"
126 "launchpad.net/juju-core/worker"
127 )
128
129@@ -29,6 +30,7 @@
130 provisioner *apiprovisioner.State
131 machine *apiprovisioner.Machine
132 config agent.Config
133+ initLock *fslock.Lock
134
135 // Save the workerName so the worker thread can be stopped.
136 workerName string
137@@ -44,7 +46,7 @@
138 // containers are created on the given machine.
139 func NewContainerSetupHandler(runner worker.Runner, workerName string, supportedContainers []instance.ContainerType,
140 machine *apiprovisioner.Machine, provisioner *apiprovisioner.State,
141- config agent.Config) worker.StringsWatchHandler {
142+ config agent.Config, initLock *fslock.Lock) worker.StringsWatchHandler {
143
144 return &ContainerSetup{
145 runner: runner,
146@@ -53,6 +55,7 @@
147 provisioner: provisioner,
148 config: config,
149 workerName: workerName,
150+ initLock: initLock,
151 }
152 }
153
154@@ -122,13 +125,22 @@
155 if initialiser, broker, err := cs.getContainerArtifacts(containerType); err != nil {
156 return fmt.Errorf("initialising container infrastructure on host machine: %v", err)
157 } else {
158- if err := initialiser.Initialise(); err != nil {
159- return fmt.Errorf("setting up container dependnecies on host machine: %v", err)
160+ if err := cs.runInitialiser(containerType, initialiser); err != nil {
161+ return fmt.Errorf("setting up container dependencies on host machine: %v", err)
162 }
163 return StartProvisioner(cs.runner, containerType, cs.provisioner, cs.config, broker)
164 }
165 }
166
167+// runInitialiser runs the container initialiser with the initialisation hook held.
168+func (cs *ContainerSetup) runInitialiser(containerType instance.ContainerType, initialiser container.Initialiser) error {
169+ if err := cs.initLock.Lock(fmt.Sprintf("initialise-%s", containerType)); err != nil {
170+ return fmt.Errorf("failed to acquire initialization lock: %v", err)
171+ }
172+ defer cs.initLock.Unlock()
173+ return initialiser.Initialise()
174+}
175+
176 // TearDown is defined on the StringsWatchHandler interface.
177 func (cs *ContainerSetup) TearDown() error {
178 // Nothing to do here.
179
180=== modified file 'worker/provisioner/container_initialisation_test.go'
181--- worker/provisioner/container_initialisation_test.go 2014-04-30 19:14:13 +0000
182+++ worker/provisioner/container_initialisation_test.go 2014-05-13 03:52:24 +0000
183@@ -5,6 +5,7 @@
184
185 import (
186 "fmt"
187+ "os"
188 "os/exec"
189
190 jc "github.com/juju/testing/checkers"
191@@ -17,6 +18,7 @@
192 apiprovisioner "launchpad.net/juju-core/state/api/provisioner"
193 coretesting "launchpad.net/juju-core/testing"
194 "launchpad.net/juju-core/utils"
195+ "launchpad.net/juju-core/utils/fslock"
196 "launchpad.net/juju-core/version"
197 "launchpad.net/juju-core/worker"
198 "launchpad.net/juju-core/worker/provisioner"
199@@ -26,7 +28,9 @@
200 CommonProvisionerSuite
201 p provisioner.Provisioner
202 // Record the apt commands issued as part of container initialisation
203- aptCmdChan <-chan *exec.Cmd
204+ aptCmdChan <-chan *exec.Cmd
205+ initLockDir string
206+ initLock *fslock.Lock
207 }
208
209 var _ = gc.Suite(&ContainerSetupSuite{})
210@@ -56,6 +60,12 @@
211 // Set up provisioner for the state machine.
212 agentConfig := s.AgentConfigForTag(c, "machine-0")
213 s.p = provisioner.NewEnvironProvisioner(s.provisioner, agentConfig)
214+
215+ // Create a new container initialisation lock.
216+ s.initLockDir = c.MkDir()
217+ initLock, err := fslock.NewLock(s.initLockDir, "container-init")
218+ c.Assert(err, gc.IsNil)
219+ s.initLock = initLock
220 }
221
222 func (s *ContainerSetupSuite) TearDownTest(c *gc.C) {
223@@ -63,7 +73,7 @@
224 s.CommonProvisionerSuite.TearDownTest(c)
225 }
226
227-func (s *ContainerSetupSuite) setupContainerWorker(c *gc.C, tag string) {
228+func (s *ContainerSetupSuite) setupContainerWorker(c *gc.C, tag string) worker.StringsWatchHandler {
229 runner := worker.NewRunner(allFatal, noImportance)
230 pr := s.st.Provisioner()
231 machine, err := pr.Machine(tag)
232@@ -73,13 +83,11 @@
233 cfg := s.AgentConfigForTag(c, tag)
234
235 watcherName := fmt.Sprintf("%s-container-watcher", machine.Id())
236- handler := provisioner.NewContainerSetupHandler(runner, watcherName, instance.ContainerTypes, machine, pr, cfg)
237+ handler := provisioner.NewContainerSetupHandler(runner, watcherName, instance.ContainerTypes, machine, pr, cfg, s.initLock)
238 runner.StartWorker(watcherName, func() (worker.Worker, error) {
239 return worker.NewStringsWorker(handler), nil
240 })
241- go func() {
242- runner.Wait()
243- }()
244+ return handler
245 }
246
247 func (s *ContainerSetupSuite) createContainer(c *gc.C, host *state.Machine, ctype instance.ContainerType) {
248@@ -186,3 +194,23 @@
249 s.assertContainerInitialised(c, test.ctype, test.packages)
250 }
251 }
252+
253+func (s *ContainerSetupSuite) TestContainerInitLockError(c *gc.C) {
254+ m, err := s.BackingState.AddOneMachine(state.MachineTemplate{
255+ Series: coretesting.FakeDefaultSeries,
256+ Jobs: []state.MachineJob{state.JobHostUnits},
257+ Constraints: s.defaultConstraints,
258+ })
259+ c.Assert(err, gc.IsNil)
260+ err = m.SetAgentVersion(version.Current)
261+ c.Assert(err, gc.IsNil)
262+
263+ err = os.RemoveAll(s.initLockDir)
264+ c.Assert(err, gc.IsNil)
265+ handler := s.setupContainerWorker(c, m.Tag())
266+ _, err = handler.SetUp()
267+ c.Assert(err, gc.IsNil)
268+ err = handler.Handle([]string{"0/lxc/0"})
269+ c.Assert(err, gc.ErrorMatches, ".*failed to acquire initialization lock:.*")
270+
271+}
272
273=== modified file 'worker/uniter/uniter.go'
274--- worker/uniter/uniter.go 2014-04-10 22:59:23 +0000
275+++ worker/uniter/uniter.go 2014-05-13 03:52:24 +0000
276@@ -92,10 +92,11 @@
277 // NewUniter creates a new Uniter which will install, run, and upgrade
278 // a charm on behalf of the unit with the given unitTag, by executing
279 // hooks and operations provoked by changes in st.
280-func NewUniter(st *uniter.State, unitTag string, dataDir string) *Uniter {
281+func NewUniter(st *uniter.State, unitTag string, dataDir string, hookLock *fslock.Lock) *Uniter {
282 u := &Uniter{
283- st: st,
284- dataDir: dataDir,
285+ st: st,
286+ dataDir: dataDir,
287+ hookLock: hookLock,
288 }
289 go func() {
290 defer u.tomb.Done()
291@@ -143,11 +144,6 @@
292 }
293
294 func (u *Uniter) setupLocks() (err error) {
295- lockDir := filepath.Join(u.dataDir, "locks")
296- u.hookLock, err = fslock.NewLock(lockDir, "uniter-hook-execution")
297- if err != nil {
298- return err
299- }
300 if message := u.hookLock.Message(); u.hookLock.IsLocked() && message != "" {
301 // Look to see if it was us that held the lock before. If it was, we
302 // should be safe enough to break it, as it is likely that we died
303
304=== modified file 'worker/uniter/uniter_test.go'
305--- worker/uniter/uniter_test.go 2014-04-10 22:59:23 +0000
306+++ worker/uniter/uniter_test.go 2014-05-13 03:52:24 +0000
307@@ -1363,7 +1363,10 @@
308 if ctx.s.uniter == nil {
309 panic("API connection not established")
310 }
311- ctx.uniter = uniter.NewUniter(ctx.s.uniter, s.unitTag, ctx.dataDir)
312+ locksDir := filepath.Join(ctx.dataDir, "locks")
313+ lock, err := fslock.NewLock(locksDir, "uniter-hook-execution")
314+ c.Assert(err, gc.IsNil)
315+ ctx.uniter = uniter.NewUniter(ctx.s.uniter, s.unitTag, ctx.dataDir, lock)
316 uniter.SetUniterObserver(ctx.uniter, ctx)
317 }
318

Subscribers

People subscribed via source and target branches

to all changes: