Extend the mega-watcher to report opened port changes.

[frankban]

This fixes a regression causing opened ports not being
reported by the mega-watcher for units. A change in the
ports now generates a change in the unitInfo as expected
by the Juju API clients.

Also fix another backward incompatible API change in the way
empty ports are reported in the mega-watcher JSON: legacy
clients (e.g. python-jujuclient) expect an empty list, not
the null value resulting from marshaling a nil slice.

QA:

• bootstrap an environment;
• deploy and expose the juju-gui charm;
• while the unit is installed, the mega-watcher should
  report an empty list in both ports and port ranges;
• when the deployment completes and the unit is in
  a started state, the mega-watcher API should correctly
  report the juju-gui unit opened ports (80 and 443).

(Review request: http://reviews.vapour.ws/r/919/)

[voidspace]

LGTM

[rogpeppe]

or, given that port ranges are usually single numbers in practice make([]network.Port, 0, len(portRanges))

[dimitern]

Commonly yes, but the whole point of having ranges is the ability to open e.g. a 1000 ports by specifying a single 5000-6000/tcp range, but OTOH having len(portRanges) as capacity by default is will perhaps be slightly more efficient and requiring less reallocations in 80% of the cases.

[frankban]

Done.

[rogpeppe]

It might be worth investigating if we can reasonably avoid this round-trip,
as all Machine.Units actually requires is the machine id, which we already have.

[dimitern]

We could, if we had st.UnitsFor(machineId) hiding that dirty return &Machine{st:st, doc:machineDoc{Id:machineId}}}.Units() to save the round-trip into the state package where it belongs.

[frankban]

Done.

[rogpeppe]

Looks like this default case can be removed.

[frankban]

Indeed, done.

[rogpeppe]

just:

 switch info := store.Get(parentId).(type) {

should be fine, I think.

[frankban]

Done.

[rogpeppe]

switch oldInfo := store.Get(eid).(type) {

[frankban]

Done.

[dimitern]

Since you're in the state package, how about this instead?

machineId, err := extractPortsIdParts(key)
if err != nil {
    logger.Debugf("cannot parse ports key %q: %v", key, err)
    return multiwatcher.EntityId{}, false
}
return backingEntityIdForGlobalKey(machineGlobalKey(machineId))

[frankban]

Done in a way similar to what you suggested.

[dimitern]

FWIW, this test is becoming huge and unmanageable, we should split it to smaller (saner?) blocks.

[frankban]

I'd leave this for another branch, focused on refactoring this long test.

[dimitern]

@frankban #1560 is soon to be reverted by #1589 to unblock master, you'll need to integrate the original fix into this PR as well.

[frankban]

Thanks for the reviews!
$$merge$$

[jujubot]

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju