environs/cloudinit: bootstrap with MongoDB

environs/cloudinit: bootstrap with MongoDB



https://code.launchpad.net/~niemeyer/juju-core/mgo-cloudinit/+merge/125734

(do not edit description out of merge proposal)

[niemeyer, 2012-09-21 14:45:36]

Please take a look.

[dave_cheney.net, 2012-09-21 14:51:30]

LGTM with minors.

https://codereview.appspot.com/6543050/diff/2001/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/6543050/diff/2001/environs/cloudinit/cloudinit...
environs/cloudinit/cloudinit.go:97: if cfg.StateServer {
Remove this whole condition ?

https://codereview.appspot.com/6543050/diff/2001/environs/cloudinit/cloudinit...
environs/cloudinit/cloudinit.go:125: fmt.Sprintf("wget -O - %s | tar xz -C
/opt", shquote(url)),
curl ? Is wget installed by default ?

https://codereview.appspot.com/6543050/diff/2001/environs/cloudinit/cloudinit...
environs/cloudinit/cloudinit.go:165: addScripts(c, fmt.Sprintf("ln -s %v %s",
cfg.Tools.Binary, toolsDir))
ln -nfs, so it doesn't fail if for some reason that the target already exists

https://codereview.appspot.com/6543050/diff/2001/environs/cloudinit/cloudinit...
environs/cloudinit/cloudinit.go:198: Cmd:     "/opt/mongo/bin/mongod --port
37017 --bind_ip 127.0.0.1 --dbpath=/var/lib/juju/db",
Hmm, binding to 127.0.0.1 is fine, but it means we should drop, or force UseSSH
to true for everyone.

[niemeyer, 2012-09-21 14:58:55]

Please take a look.

https://codereview.appspot.com/6543050/diff/2001/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/6543050/diff/2001/environs/cloudinit/cloudinit...
environs/cloudinit/cloudinit.go:97: if cfg.StateServer {
On 2012/09/21 14:51:31, dfc wrote:
> Remove this whole condition ?

Duh.

https://codereview.appspot.com/6543050/diff/2001/environs/cloudinit/cloudinit...
environs/cloudinit/cloudinit.go:125: fmt.Sprintf("wget -O - %s | tar xz -C
/opt", shquote(url)),
On 2012/09/21 14:51:31, dfc wrote:
> curl ? Is wget installed by default ?

This is a copy & paste from previous code above.

https://codereview.appspot.com/6543050/diff/2001/environs/cloudinit/cloudinit...
environs/cloudinit/cloudinit.go:165: addScripts(c, fmt.Sprintf("ln -s %v %s",
cfg.Tools.Binary, toolsDir))
On 2012/09/21 14:51:31, dfc wrote:
> ln -nfs, so it doesn't fail if for some reason that the target already exists

I haven't touched that in this branch. I'd prefer to not fiddle around for the
moment if that's ok. I've added a TODO.

https://codereview.appspot.com/6543050/diff/2001/environs/cloudinit/cloudinit...
environs/cloudinit/cloudinit.go:198: Cmd:     "/opt/mongo/bin/mongod --port
37017 --bind_ip 127.0.0.1 --dbpath=/var/lib/juju/db",
On 2012/09/21 14:51:31, dfc wrote:
> Hmm, binding to 127.0.0.1 is fine, but it means we should drop, or force
UseSSH
> to true for everyone.

Good point. Changed.

[niemeyer, 2012-09-21 14:59:38]

Please take a look.

[rog, 2012-09-21 15:01:23]

On 2012/09/21 14:59:38, niemeyer wrote:
> Please take a look.

LGTM

[dave_cheney.net, 2012-09-21 15:01:56]

LGTM, with one comment.

https://codereview.appspot.com/6543050/diff/7001/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/6543050/diff/7001/environs/cloudinit/cloudinit...
environs/cloudinit/cloudinit.go:127: " --state-servers localhost"+zkPortSuffix+
zkPortSuffix ?

[niemeyer, 2012-09-21 15:46:27]

*** Submitted:

environs/cloudinit: bootstrap with MongoDB

R=dfc, rog
CC=
https://codereview.appspot.com/6543050

https://codereview.appspot.com/6543050/diff/7001/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/6543050/diff/7001/environs/cloudinit/cloudinit...
environs/cloudinit/cloudinit.go:127: " --state-servers localhost"+zkPortSuffix+
On 2012/09/21 15:01:56, dfc wrote:
> zkPortSuffix ?

Done.