openstack: fix bootstrap and status

openstack: fix bootstrap and status

Bootstrap had issues with incorrectly encoded base64 userdata,
which will be fixed with a follow-up CL in goose.
Status is now fully working.

https://code.launchpad.net/~dimitern/juju-core/openstack-status/+merge/145619

(do not edit description out of merge proposal)

[dimitern, 2013-01-30 14:41:35]

Please take a look.

[TheMue, 2013-01-30 14:48:15]

LGTM, only few comments.

https://codereview.appspot.com/7223059/diff/1/environs/openstack/provider.go
File environs/openstack/provider.go (right):

https://codereview.appspot.com/7223059/diff/1/environs/openstack/provider.go#...
environs/openstack/provider.go:121: func getInstanceAddress(addresses
map[string][]nova.IPAddress) (string, error) {
Is the get-prefix needed? More typical is simply instanceAddress().

https://codereview.appspot.com/7223059/diff/1/environs/openstack/provider.go#...
environs/openstack/provider.go:122: var private, public, privatenet string
s/privatenet/privateNet/

[gz, 2013-01-30 14:48:34]

LGTM, a few comments:

https://codereview.appspot.com/7223059/diff/1/environs/openstack/provider.go
File environs/openstack/provider.go (right):

https://codereview.appspot.com/7223059/diff/1/environs/openstack/provider.go#...
environs/openstack/provider.go:32: var apiPortSuffix = fmt.Sprintf(":%d",
apiPort)
Could these be const as well?

https://codereview.appspot.com/7223059/diff/1/environs/openstack/provider.go#...
environs/openstack/provider.go:310: func (e *environ) StateInfo() (*state.Info,
*api.Info, error) {
I presume this is mostly copied from the ec2 one, so will skip over.

https://codereview.appspot.com/7223059/diff/1/environs/openstack/provider.go#...
environs/openstack/provider.go:451: InstanceIdAccessor: `$(curl
http://169.254.169.254/openstack/2012-08-10/meta_data.json|python -c 'import
json,sys;print json.loads(sys.stdin.read())["uuid"]')`,
I'd like a comment above this noting it's a horrible hack, only works in folsom
or later, and really we'd like to do the whole thing a better way. :)

https://codereview.appspot.com/7223059/diff/1/environs/openstack/provider.go#...
environs/openstack/provider.go:468: log.Debugf("environs/openstack: openstack
user data; %d bytes: %q", len(cdata), data)
I'd lose this debug line, it's not useful in general.

[dimitern, 2013-01-30 15:01:34]

Please take a look.

https://codereview.appspot.com/7223059/diff/1/environs/openstack/provider.go
File environs/openstack/provider.go (right):

https://codereview.appspot.com/7223059/diff/1/environs/openstack/provider.go#...
environs/openstack/provider.go:32: var apiPortSuffix = fmt.Sprintf(":%d",
apiPort)
On 2013/01/30 14:48:34, gz wrote:
> Could these be const as well?

I just copied EC2 ones. But probably you're right.

https://codereview.appspot.com/7223059/diff/1/environs/openstack/provider.go#...
environs/openstack/provider.go:121: func getInstanceAddress(addresses
map[string][]nova.IPAddress) (string, error) {
On 2013/01/30 14:48:16, TheMue wrote:
> Is the get-prefix needed? More typical is simply instanceAddress().

Done.

https://codereview.appspot.com/7223059/diff/1/environs/openstack/provider.go#...
environs/openstack/provider.go:122: var private, public, privatenet string
On 2013/01/30 14:48:16, TheMue wrote:
> s/privatenet/privateNet/

Done.

https://codereview.appspot.com/7223059/diff/1/environs/openstack/provider.go#...
environs/openstack/provider.go:310: func (e *environ) StateInfo() (*state.Info,
*api.Info, error) {
On 2013/01/30 14:48:34, gz wrote:
> I presume this is mostly copied from the ec2 one, so will skip over.

It is, except for the line with DNSName().

https://codereview.appspot.com/7223059/diff/1/environs/openstack/provider.go#...
environs/openstack/provider.go:451: InstanceIdAccessor: `$(curl
http://169.254.169.254/openstack/2012-08-10/meta_data.json|python -c 'import
json,sys;print json.loads(sys.stdin.read())["uuid"]')`,
On 2013/01/30 14:48:34, gz wrote:
> I'd like a comment above this noting it's a horrible hack, only works in
folsom
> or later, and really we'd like to do the whole thing a better way. :)

Done.

https://codereview.appspot.com/7223059/diff/1/environs/openstack/provider.go#...
environs/openstack/provider.go:468: log.Debugf("environs/openstack: openstack
user data; %d bytes: %q", len(cdata), data)
On 2013/01/30 14:48:34, gz wrote:
> I'd lose this debug line, it's not useful in general.
It is I think, because it tells us how long is the actual cloudinit (if it gets
too big > 64K will catch it easily).

[TheMue, 2013-01-30 15:03:55]

LGTM

[gz, 2013-01-30 15:09:51]

LGTM with a query, land after that.

https://codereview.appspot.com/7223059/diff/1/environs/openstack/provider.go
File environs/openstack/provider.go (right):

https://codereview.appspot.com/7223059/diff/1/environs/openstack/provider.go#...
environs/openstack/provider.go:468: log.Debugf("environs/openstack: openstack
user data; %d bytes: %q", len(cdata), data)
On 2013/01/30 15:01:34, dimitern wrote:
> On 2013/01/30 14:48:34, gz wrote:
> > I'd lose this debug line, it's not useful in general.
> It is I think, because it tells us how long is the actual cloudinit (if it
gets
> too big > 64K will catch it easily).

In which case, only log the length. Logging the whole (binary compressed)
cloud-init data is ugly, and potentially an information leak when we ask people
to paste logs.

[dimitern, 2013-01-30 15:12:24]

Please take a look.

[dimitern, 2013-01-30 15:14:21]

*** Submitted:

openstack: fix bootstrap and status

Bootstrap had issues with incorrectly encoded base64 userdata,
which will be fixed with a follow-up CL in goose.
Status is now fully working.

R=TheMue, gz
CC=
https://codereview.appspot.com/7223059