container: new package

container: new package



https://code.launchpad.net/~rogpeppe/juju-core/container-package2/+merge/110824

(do not edit description out of merge proposal)

[rog, 2012-06-18 16:42:43]

Please take a look.

[niemeyer, 2012-06-19 14:39:58]

A couple of ideas.

https://codereview.appspot.com/6312044/diff/5001/container/container.go
File container/container.go (right):

https://codereview.appspot.com/6312044/diff/5001/container/container.go#newco...
container/container.go:33: func Simple(unit *state.Unit) Container {
As we talked, this should probably not have the unit, and instead we should have
it in Deploy.

https://codereview.appspot.com/6312044/diff/5001/container/container.go#newco...
container/container.go:62: // TODO what, if any, directory do we need to delete?
The whole container, within /var/lib/juju. We must check the Python code to see
the convention in use.

The same directory should probably be created above on Deploy, and passed onto
the unit agent for use.

[rog, 2012-06-19 15:56:35]

Please take a look.

https://codereview.appspot.com/6312044/diff/5001/container/container.go
File container/container.go (right):

https://codereview.appspot.com/6312044/diff/5001/container/container.go#newco...
container/container.go:33: func Simple(unit *state.Unit) Container {
On 2012/06/19 14:39:58, niemeyer wrote:
> As we talked, this should probably not have the unit, and instead we should
have
> it in Deploy.

Done.

[rog, 2012-06-19 16:03:07]

Please take a look.

[niemeyer, 2012-06-19 17:21:39]

LGTM assuming a safer mode:

https://codereview.appspot.com/6312044/diff/11001/container/container.go
File container/container.go (right):

https://codereview.appspot.com/6312044/diff/11001/container/container.go#newc...
container/container.go:65: if err := os.MkdirAll(dir, 0777); err != nil {
0755 is a better default. There's really no reason why we'd want to give writing
access to everybody.

[rog, 2012-06-20 13:13:36]

*** Submitted:

container: new package

R=niemeyer
CC=
https://codereview.appspot.com/6312044

https://codereview.appspot.com/6312044/diff/11001/container/container.go
File container/container.go (right):

https://codereview.appspot.com/6312044/diff/11001/container/container.go#newc...
container/container.go:65: if err := os.MkdirAll(dir, 0777); err != nil {
On 2012/06/19 17:21:39, niemeyer wrote:
> 0755 is a better default. There's really no reason why we'd want to give
writing
> access to everybody.

Done.