Looking good in general, but I think it needs some polishing. Some
suggestions below.


https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc.go
File worker/deployer/lxc.go (right):

https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc.go#newcode23
worker/deployer/lxc.go:23: // EnvironName identifies the environment the
manager is running in.
s/manager/context/ ?

https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc.go#newcode68
worker/deployer/lxc.go:68: dataDir = "/var/lib/juju"
s/dataDir/hostDataDir/

https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc.go#newcode82
worker/deployer/lxc.go:82: // Deploy unit creates a container for a unit
and installs the
// DeployUnit creates a container for a unit and installs the required
upstart job in it.

https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc.go#newcode92
worker/deployer/lxc.go:92: err := ctx.createContainer(containerName)
if err := ctx.createContainer(); err != nil { ...

https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc.go#newcode103
worker/deployer/lxc.go:103: // RecallUnit destroys the container and
removes the autostart link.
s/autostart/upstart/ ?

https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc.go#newcode108
worker/deployer/lxc.go:108: err := ctx.destroyConfigLink(unitName)
if err := ..; err != nil { ...

https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc.go#newcode120
worker/deployer/lxc.go:120: // DeployedUnits checks which units are
deployed right now.
s/checks/returns/ ?

https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc.go#newcode165
worker/deployer/lxc.go:165: if err != nil {
if _, err := ..; err != nil { ...

https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc.go#newcode196
worker/deployer/lxc.go:196: err = copyTools(hostToolsDir, toolsDir)
ditto

https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc.go#newcode218
worker/deployer/lxc.go:218: defer removeOnErr(&err, conf.Dir())
did you mean the err from line 200 or 215 here?

https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc.go#newcode227
worker/deployer/lxc.go:227: "--debug", // TODO: propagate debug state
sensibly
what does this mean?

https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc.go#newcode242
worker/deployer/lxc.go:242: return fmt.Errorf("container %q is not yet
created", containerName)
s/is not yet created/does not exist/ seems better

https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc.go#newcode247
worker/deployer/lxc.go:247: _, err := lxcDo("lxc-stop", args...)
next 3 ifs: if err := ..; err != nil { return err } ?

https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc.go#newcode262
worker/deployer/lxc.go:262: // containerName creates a unique name for
the container and its upstart
creates or just returns?

https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc.go#newcode273
worker/deployer/lxc.go:273: _, err := os.Stat(autoConfigPath)
shouldn't we at least log the error?

https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc.go#newcode278
worker/deployer/lxc.go:278: func (ctx *LxcContext)
createConfigLink(unitName string) error {
combine common parts of this, previous and next method in a single
helper please.
Maybe even make container its own type with methods?

https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc.go#newcode308
worker/deployer/lxc.go:308: // lxcDo executes the passed command and
returns the out.
s/out/output/

Also maybe rename it to lxcRun ?

https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc.go#newcode328
worker/deployer/lxc.go:328: fcopy := func(fi os.FileInfo) error {
it might be better to extract this into a separate free func.

https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc_test.go
File worker/deployer/lxc_test.go (right):

https://codereview.appspot.com/7637045/diff/1/worker/deployer/lxc_test.go#newcode10
worker/deployer/lxc_test.go:10:
d

https://codereview.appspot.com/7637045/