Code review comment for lp:~thumper/juju-core/fast-lxc-no-apt-upgrade

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

I'm not fully across the finer details about lxc cloning. I think this
looks good. Perhaps a little more test coverage though. Specifically:
1. EnsureCloneTemplate behaves correctly if called when a template
already exists
2. The cloud init contents are devoid of apt stuff

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

https://codereview.appspot.com/74370044/diff/1/container/lxc/lxc.go#newcode92
container/lxc/lxc.go:92: if conf["use-clone"] == "true" {
Is there a helper function for str -> bool?

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

https://codereview.appspot.com/74370044/diff/1/container/lxc/lxc_test.go#newcode165
container/lxc/lxc_test.go:165: func (s *LxcSuite) AssertEvent(c *gc.C,
event mock.Event, expected mock.Action, id string) {
does this need to be exported?

https://codereview.appspot.com/74370044/diff/1/container/lxc/lxc_test.go#newcode203
container/lxc/lxc_test.go:203: c.Assert(template.Name(), gc.Equals,
name)
Should we be checking (some key aspects of) the contents of the
template?

https://codereview.appspot.com/74370044/

« Back to merge proposal