Code review comment for lp:~sidnei/juju-core/lxc-clone-with-overlayfs

Ian Booth (wallyworld) wrote :

Looks like a nice change.

I'd like to see some tests to ensure the new template stuff is processed
correctly. eg is the container template created as expected, is it only
done once, is the filename done correctly if manager name is "" etc etc.

Also, maybe a comment as to why the newly added HUP is needed in the
upstart script.

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

https://codereview.appspot.com/12801044/diff/1/container/lxc/lxc.go#newcode133
container/lxc/lxc.go:133: template := lxcObjectFactory.New(templateName)

What if manager.name is ""? There's a few lines of code above this
accounts for the fact that manager name might be empty so I think
something similar is needed for making the template name also? Otherwise
we will end up with a template name like "-precise-template" if
manager.name is ""

https://codereview.appspot.com/12801044/

« Back to merge proposal