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