Take hook exec lock in container host init

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

https://code.launchpad.net/~axwalk/juju-core/lp1317197-containerinit-hooklock/+merge/219157

(do not edit description out of merge proposal)

[axw, 2014-05-12 09:26:38]

Please take a look.

[fwereade, 2014-05-12 13:59:28]

This'd be fine if it used the same lock as the uniter does.

I'd be happiest of all if there was one func somewhere that returned this lock,
so that we can switch to an in-process lock with minimal hassle when we
integrate the uniters into the machine agent.

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

https://codereview.appspot.com/91350044/diff/1/cmd/jujud/machine.go#newcode387
cmd/jujud/machine.go:387: lockDir := filepath.Join(agentConfig.DataDir(),
"locks")
Can we move this somewhere common please? We have exactly the same in uniter
somewhere.

https://codereview.appspot.com/91350044/diff/1/worker/uniter/uniter.go
File worker/uniter/uniter.go (right):

https://codereview.appspot.com/91350044/diff/1/worker/uniter/uniter.go#newcod...
worker/uniter/uniter.go:153: u.hookLock, err = fslock.NewLock(lockDir,
HookExecutionLock)
Huh... wait. Why *isn't* this exactly the same code? It doesn't matter what we
call it, but the main point of *this* lock is to prevent apt collisions (well --
arbitrary collisions most often exhibited in the wild when using apt).

[axw, 2014-05-13 02:21:29]

Please take a look.

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

https://codereview.appspot.com/91350044/diff/1/cmd/jujud/machine.go#newcode387
cmd/jujud/machine.go:387: lockDir := filepath.Join(agentConfig.DataDir(),
"locks")
On 2014/05/12 13:59:28, fwereade wrote:
> Can we move this somewhere common please? We have exactly the same in uniter
> somewhere.

Created a common function in cmd/jujud/agent.go; the lock is created by the
agent and passed to the uniter/container setup handler.

https://codereview.appspot.com/91350044/diff/1/worker/uniter/uniter.go
File worker/uniter/uniter.go (right):

https://codereview.appspot.com/91350044/diff/1/worker/uniter/uniter.go#newcod...
worker/uniter/uniter.go:153: u.hookLock, err = fslock.NewLock(lockDir,
HookExecutionLock)
On 2014/05/12 13:59:28, fwereade wrote:
> Huh... wait. Why *isn't* this exactly the same code? It doesn't matter what we
> call it, but the main point of *this* lock is to prevent apt collisions (well
--
> arbitrary collisions most often exhibited in the wild when using apt).

Not really sure what you meant here; I just made the lock name common because
the container setup code wants to take the same lock. I've moved this out to
common code anyway.

[wallyworld, 2014-05-13 03:02:45]

LGTM

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

https://codereview.appspot.com/91350044/diff/20001/cmd/jujud/machine.go#newco...
cmd/jujud/machine.go:385: initLock, err :=
hookExecutionLock(agentConfig.DataDir())
minor nitpick, the code in unit.go uses a var:
dataDir := agentConfig.DataDir()

Would be great to be consistent

[axw, 2014-05-13 03:06:06]

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

https://codereview.appspot.com/91350044/diff/20001/cmd/jujud/machine.go#newco...
cmd/jujud/machine.go:385: initLock, err :=
hookExecutionLock(agentConfig.DataDir())
On 2014/05/13 03:02:45, wallyworld wrote:
> minor nitpick, the code in unit.go uses a var:
> dataDir := agentConfig.DataDir()
> 
> Would be great to be consistent

Only because dataDir is used by something else in the uniter code; it's not
here. I'd prefer not to introduce an identifier unless it's actually necessary.