Add more info to the mega-watcher for machines.

Add more info to the mega-watcher for machines.

Include life, series, supported containers, hardware
characteristics, jobs and addresses in MachineInfo.

This will be required by the Juju GUI.

https://code.launchpad.net/~frankban/juju-core/machine-info/+merge/211527

(do not edit description out of merge proposal)

[frankban, 2014-03-18 13:38:43]

Please take a look.

[rog, 2014-03-21 09:22:16]

Looks great in general, very nearly there. Some suggestions below.

https://codereview.appspot.com/77420043/diff/1/state/api/params/params.go
File state/api/params/params.go (right):

https://codereview.appspot.com/77420043/diff/1/state/api/params/params.go#new...
state/api/params/params.go:457: HardwareCharacteristics
*instance.HardwareCharacteristics
perhaps this should be:

`json:",omitempty"`

https://codereview.appspot.com/77420043/diff/1/state/api/params/params.go#new...
state/api/params/params.go:458: Jobs                    []MachineJob
I'm not entirely sure we want to be exporting this to the client, as it's really
an agent-related thing.

That said, it does provide information that
a client can find useful, so perhaps this is ok.

Perhaps worth checking with fwereade.

https://codereview.appspot.com/77420043/diff/1/state/api/params/params_test.go
File state/api/params/params_test.go (right):

https://codereview.appspot.com/77420043/diff/1/state/api/params/params_test.g...
state/api/params/params_test.go:42: Life:                   
params.Life(state.Alive.String()),
Life: params.Alive,

https://codereview.appspot.com/77420043/diff/1/state/api/params/params_test.g...
state/api/params/params_test.go:50: json:
`["machine","change",{"Id":"Benji","InstanceId":"Shazam","Status":"error","StatusInfo":"foo","StatusData":null,"Life":"alive","Series":"trusty","SupportedContainers":["lxc"],"Jobs":["JobManageEnviron"],"Addresses":[],"HardwareCharacteristics":{"Arch":null,"Mem":null,"RootDisk":null,"CpuCores":null,"CpuPower":null,"Tags":null}}]`,
Please add json tags to instance.HardwareCharacteristics so it serialises
similarly to the yaml. This *should* serialise as "HardwareCharacteristics":{}.

https://codereview.appspot.com/77420043/diff/1/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/77420043/diff/1/state/machine.go#newcode77
state/machine.go:77: func jobsToParamsJobs(jobs []MachineJob)
[]params.MachineJob {
Perhaps machineJobsFromParams might be more consistent
with the naming above.

https://codereview.appspot.com/77420043/diff/1/state/megawatcher.go
File state/megawatcher.go (right):

https://codereview.appspot.com/77420043/diff/1/state/megawatcher.go#newcode39
state/megawatcher.go:39: info.SupportedContainers = m.SupportedContainers
won't m.SupportedContainers still be the expected (nil) value if
SupportedContainersKnown is false?

https://codereview.appspot.com/77420043/diff/1/state/megawatcher.go#newcode53
state/megawatcher.go:53: // The entry already exists, so preserve the current
status, instance
// The entry already exists, so preserve the current
// status and instance data.

?

https://codereview.appspot.com/77420043/diff/1/state/megawatcher_internal_tes...
File state/megawatcher_internal_test.go (right):

https://codereview.appspot.com/77420043/diff/1/state/megawatcher_internal_tes...
state/megawatcher_internal_test.go:330: Life:                   
params.Life(Alive.String()),
params.Alive

https://codereview.appspot.com/77420043/diff/1/state/megawatcher_internal_tes...
state/megawatcher_internal_test.go:968: alive := params.Life(Alive.String())
d

https://codereview.appspot.com/77420043/diff/1/state/megawatcher_internal_tes...
state/megawatcher_internal_test.go:973: Life:      alive,
params.Alive

https://codereview.appspot.com/77420043/diff/1/state/megawatcher_internal_tes...
state/megawatcher_internal_test.go:982: Life:      alive,
params.Alive

and below.

[frankban, 2014-03-21 11:23:06]

Please take a look.

https://codereview.appspot.com/77420043/diff/1/state/api/params/params.go
File state/api/params/params.go (right):

https://codereview.appspot.com/77420043/diff/1/state/api/params/params.go#new...
state/api/params/params.go:457: HardwareCharacteristics
*instance.HardwareCharacteristics
On 2014/03/21 09:22:16, rog wrote:
> perhaps this should be:
> 
> `json:",omitempty"`

Done.

https://codereview.appspot.com/77420043/diff/1/state/api/params/params.go#new...
state/api/params/params.go:458: Jobs                    []MachineJob
On 2014/03/21 09:22:16, rog wrote:
> I'm not entirely sure we want to be exporting this to the client, as it's
really
> an agent-related thing.
> 
> That said, it does provide information that
> a client can find useful, so perhaps this is ok.
> 
> Perhaps worth checking with fwereade.

Done.

https://codereview.appspot.com/77420043/diff/1/state/api/params/params_test.go
File state/api/params/params_test.go (right):

https://codereview.appspot.com/77420043/diff/1/state/api/params/params_test.g...
state/api/params/params_test.go:42: Life:                   
params.Life(state.Alive.String()),
On 2014/03/21 09:22:16, rog wrote:
> Life: params.Alive,

Done.

https://codereview.appspot.com/77420043/diff/1/state/api/params/params_test.g...
state/api/params/params_test.go:50: json:
`["machine","change",{"Id":"Benji","InstanceId":"Shazam","Status":"error","StatusInfo":"foo","StatusData":null,"Life":"alive","Series":"trusty","SupportedContainers":["lxc"],"Jobs":["JobManageEnviron"],"Addresses":[],"HardwareCharacteristics":{"Arch":null,"Mem":null,"RootDisk":null,"CpuCores":null,"CpuPower":null,"Tags":null}}]`,
On 2014/03/21 09:22:16, rog wrote:
> Please add json tags to instance.HardwareCharacteristics so it serialises
> similarly to the yaml. This *should* serialise as
"HardwareCharacteristics":{}.

Done.

https://codereview.appspot.com/77420043/diff/1/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/77420043/diff/1/state/machine.go#newcode77
state/machine.go:77: func jobsToParamsJobs(jobs []MachineJob)
[]params.MachineJob {
On 2014/03/21 09:22:16, rog wrote:
> Perhaps machineJobsFromParams might be more consistent
> with the naming above.

Renamed to paramsJobsFromJobs.

https://codereview.appspot.com/77420043/diff/1/state/megawatcher.go
File state/megawatcher.go (right):

https://codereview.appspot.com/77420043/diff/1/state/megawatcher.go#newcode39
state/megawatcher.go:39: info.SupportedContainers = m.SupportedContainers
On 2014/03/21 09:22:16, rog wrote:
> won't m.SupportedContainers still be the expected (nil) value if
> SupportedContainersKnown is false?

You suggested we should not rely on the difference between nil and [] here. FOr
this reason, we decided to include SupportedContainersKnown in the MachineInfo.

https://codereview.appspot.com/77420043/diff/1/state/megawatcher.go#newcode53
state/megawatcher.go:53: // The entry already exists, so preserve the current
status, instance
On 2014/03/21 09:22:16, rog wrote:
> // The entry already exists, so preserve the current
> // status and instance data.
> 
> ?

Done.

https://codereview.appspot.com/77420043/diff/1/state/megawatcher_internal_tes...
File state/megawatcher_internal_test.go (right):

https://codereview.appspot.com/77420043/diff/1/state/megawatcher_internal_tes...
state/megawatcher_internal_test.go:330: Life:                   
params.Life(Alive.String()),
On 2014/03/21 09:22:16, rog wrote:
> params.Alive

Done.

https://codereview.appspot.com/77420043/diff/1/state/megawatcher_internal_tes...
state/megawatcher_internal_test.go:968: alive := params.Life(Alive.String())
On 2014/03/21 09:22:16, rog wrote:
> d

Done.

https://codereview.appspot.com/77420043/diff/1/state/megawatcher_internal_tes...
state/megawatcher_internal_test.go:973: Life:      alive,
On 2014/03/21 09:22:16, rog wrote:
> params.Alive

Done.

https://codereview.appspot.com/77420043/diff/1/state/megawatcher_internal_tes...
state/megawatcher_internal_test.go:982: Life:      alive,
On 2014/03/21 09:22:16, rog wrote:
> params.Alive
> 
> and below.

Done.

[rog, 2014-03-21 13:14:18]

LGTM