Code review comment for lp:~thumper/juju-core/lxc-broker

Looks good with a few suggestions.

https://codereview.appspot.com/10409049/diff/1/container/lxc/test.go
File container/lxc/test.go (right):

https://codereview.appspot.com/10409049/diff/1/container/lxc/test.go#newcode4
container/lxc/test.go:4: package lxc
This limitation of Go really annoys me.
Could we add a big file level comment that these exported methods must
only be used for tests and not for normal business logic. I know each
method mentions the use of tests but I think a global comment would be
good too.

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

https://codereview.appspot.com/10409049/diff/1/worker/provisioner/lxc-broker.go#newcode27
worker/provisioner/lxc-broker.go:27:
To help with understanding what interface lxcBroker implements, and to
cause a compile time error if the contract is not met, the idiom we have
been using for other things is to add a line like this above the struct
definition:

var _ Broker = (*lxcBroker)(nil)

(rinse and repeat for any other implemented interfaces).

https://codereview.appspot.com/10409049/diff/1/worker/provisioner/lxc-broker_test.go
File worker/provisioner/lxc-broker_test.go (right):

https://codereview.appspot.com/10409049/diff/1/worker/provisioner/lxc-broker_test.go#newcode80
worker/provisioner/lxc-broker_test.go:80: c.Assert(lxc.Id(), Equals,
instance.Id("juju-machine-1-lxc-0"))
Can we test more than this? Perhaps ensure the started instance is in
the list of running instances? Anything else we can check? Like
files/dirs created to show the instance is started?

https://codereview.appspot.com/10409049/diff/1/worker/provisioner/lxc-broker_test.go#newcode88
worker/provisioner/lxc-broker_test.go:88: c.Assert(err, IsNil)
There's more we could do here - we haven't verified the stop actually
worked. The method could be empty and just return nil and the test would
still pass. Could we perhaps list the running instances and verify it's
empty? Oh, and invoke stop on just one instance and see that the others
remain.

https://codereview.appspot.com/10409049/