cmd/juju: status: gather basic machine properties

cmd/juju: status: gather basic machine properties

This proposal adds the ability to gather basic machine
properties. 

% juju status 
machines:
  "0":
    dns-name: ec2-50-16-84-143.compute-1.amazonaws.com
    instance-id: i-0ea58c76
services: {}

nb. The key of the machines map is a string, not an int, which
is an incompatability with the current pythonic output. This
will be addressed shortly.

https://code.launchpad.net/~dave-cheney/juju-core/go-cmd-juju-status-part-two/+merge/116341

(do not edit description out of merge proposal)

[dave_cheney.net, 2012-07-23 18:41:07]

Please take a look.

[dave_cheney.net, 2012-07-24 08:23:50]

Please take a look.

[dave_cheney.net, 2012-07-24 09:30:16]

Please take a look.

[fwereade, 2012-07-24 10:28:57]

Good start, LGTM.

https://codereview.appspot.com/6419069/diff/2002/cmd/juju/status.go
File cmd/juju/status.go (right):

https://codereview.appspot.com/6419069/diff/2002/cmd/juju/status.go#newcode138
cmd/juju/status.go:138: dnsname, err := instance.DNSName()
As I understand our conventions, DNSName should return an error if the instance
doesn't have one, but we should handle and ignore that specific error here. May
be a fix for another CL.

[dave_cheney.net, 2012-07-24 10:56:23]

Thank you for your comments.

https://codereview.appspot.com/6419069/diff/2002/cmd/juju/status.go
File cmd/juju/status.go (right):

https://codereview.appspot.com/6419069/diff/2002/cmd/juju/status.go#newcode138
cmd/juju/status.go:138: dnsname, err := instance.DNSName()
On 2012/07/24 10:28:57, fwereade wrote:
> As I understand our conventions, DNSName should return an error if the
instance
> doesn't have one, but we should handle and ignore that specific error here.
May
> be a fix for another CL.

Looking in the python code, if the InstanceId is available, then the machine is
consider to be running, so DNSName is assumed to be valid.

There is an argument to be made to skip individual machines if they error rather
than letting the error bubble up to the top.

[niemeyer, 2012-07-24 11:25:29]

LGTM

https://codereview.appspot.com/6419069/diff/2002/cmd/juju/status.go
File cmd/juju/status.go (right):

https://codereview.appspot.com/6419069/diff/2002/cmd/juju/status.go#newcode111
cmd/juju/status.go:111: machineid := strconv.Itoa(m.Id())
This will need to be fixed in a future branch as we discussed, but loks fine for
the moment.

https://codereview.appspot.com/6419069/diff/2002/cmd/juju/status.go#newcode138
cmd/juju/status.go:138: dnsname, err := instance.DNSName()
On 2012/07/24 10:56:23, dfc wrote:
> On 2012/07/24 10:28:57, fwereade wrote:
> > As I understand our conventions, DNSName should return an error if the
> instance
> > doesn't have one, but we should handle and ignore that specific error here.
> May
> > be a fix for another CL.
> 
> Looking in the python code, if the InstanceId is available, then the machine
is
> consider to be running, so DNSName is assumed to be valid.
> 
> There is an argument to be made to skip individual machines if they error
rather
> than letting the error bubble up to the top.

I suggest just keeping the error for the moment. This is the kind of thing we'll
get reports about if they are issues in practice. It'll be in the user face and
not affect the running environment.

https://codereview.appspot.com/6419069/diff/2002/cmd/juju/status_test.go
File cmd/juju/status_test.go (right):

https://codereview.appspot.com/6419069/diff/2002/cmd/juju/status_test.go#newc...
cmd/juju/status_test.go:66: "bootstrap/pending",
I suggest turning the comment onto the title, here and below:

"simulate juju bootstrap by adding machine/0 to the state",

https://codereview.appspot.com/6419069/diff/2002/cmd/juju/status_test.go#newc...
cmd/juju/status_test.go:88: inst, err := conn.Environ.StartInstance(m.Id(), nil)
Love those tests.

[dave_cheney.net, 2012-07-24 11:31:43]

Thank you for your review.

https://codereview.appspot.com/6419069/diff/2002/cmd/juju/status.go
File cmd/juju/status.go (right):

https://codereview.appspot.com/6419069/diff/2002/cmd/juju/status.go#newcode111
cmd/juju/status.go:111: machineid := strconv.Itoa(m.Id())
On 2012/07/24 11:25:29, niemeyer wrote:
> This will need to be fixed in a future branch as we discussed, but loks fine
for
> the moment.

I'm looking at that now. I wish I could unsee it.

https://codereview.appspot.com/6419069/diff/2002/cmd/juju/status.go#newcode138
cmd/juju/status.go:138: dnsname, err := instance.DNSName()
On 2012/07/24 11:25:29, niemeyer wrote:
> On 2012/07/24 10:56:23, dfc wrote:
> > On 2012/07/24 10:28:57, fwereade wrote:
> > > As I understand our conventions, DNSName should return an error if the
> > instance
> > > doesn't have one, but we should handle and ignore that specific error
here.
> > May
> > > be a fix for another CL.
> > 
> > Looking in the python code, if the InstanceId is available, then the machine
> is
> > consider to be running, so DNSName is assumed to be valid.
> > 
> > There is an argument to be made to skip individual machines if they error
> rather
> > than letting the error bubble up to the top.
> 
> I suggest just keeping the error for the moment. This is the kind of thing
we'll
> get reports about if they are issues in practice. It'll be in the user face
and
> not affect the running environment.

Done.

https://codereview.appspot.com/6419069/diff/2002/cmd/juju/status_test.go
File cmd/juju/status_test.go (right):

https://codereview.appspot.com/6419069/diff/2002/cmd/juju/status_test.go#newc...
cmd/juju/status_test.go:66: "bootstrap/pending",
On 2012/07/24 11:25:29, niemeyer wrote:
> I suggest turning the comment onto the title, here and below:
> 
> "simulate juju bootstrap by adding machine/0 to the state",

Done.

https://codereview.appspot.com/6419069/diff/2002/cmd/juju/status_test.go#newc...
cmd/juju/status_test.go:88: inst, err := conn.Environ.StartInstance(m.Id(), nil)
On 2012/07/24 11:25:29, niemeyer wrote:
> Love those tests.

ta.

[dave_cheney.net, 2012-07-24 11:32:41]

*** Submitted:

cmd/juju: status: gather basic machine properties

This proposal adds the ability to gather basic machine
properties. 

% juju status 
machines:
  "0":
    dns-name: ec2-50-16-84-143.compute-1.amazonaws.com
    instance-id: i-0ea58c76
services: {}

nb. The key of the machines map is a string, not an int, which
is an incompatability with the current pythonic output. This
will be addressed shortly.

R=fwereade, niemeyer
CC=
https://codereview.appspot.com/6419069