InstancePoller compares wrong Address list and always requests updated state Addresses

[John A Meinel]

The Transaction log is full of entries that look like:

{ "_id": ObjectId("553deadbeef"), "n": "deadbeef",
 "o": [ {
   "c": "machines",
   "d": "6",
   "a": {
      "addresses": [
 {"value": "abcd.foo.com", "addresstype": "hostname", "networkscope": "public" },
    {"value": "abcd.foo.com", "addresstype": "hostname", "networkscope": "local-cloud" },
        ],
      "life": {"$ne": 2}
   }} ],
 "s": 2
}

However, that is simply asserting that the values in the DB are the values we currently have.
If we want a transaction, then it *should* just be setting the values to the
values we have rather than Asserting them. (SetAddresses is always going to be the official arbiter of what the addresses should be now).

Looking at this code:
  https://github.com/juju/juju/blob/1.20/state/machine.go#L986

It would look like we are only asserting notDeadDoc, but not asserting anything
about the value of addresses. But that doesn't appear to be what is in the
content of the database TXN collection.
I actually would offer that at most we should Refresh the object, and then
in-memory assert the addresses haven't changed, and in the changed=false case
we shouldn't be calling m.st.run() at all.

See also https://bugs.launchpad.net/juju-core/+bug/1454059 which is about other Transactions that seem to be filling up the txn collection.

[John A Meinel (jameinel)]

Note that https://github.com/juju/juju/blob/juju-1.20.11/state/machine.go#L973 still seems to have the same code path for SetAddresses, so we still need to figure out where this assert is comming from.

[John A Meinel (jameinel)]

So looking at: https://github.com/juju/juju/blob/juju-1.20.11/worker/instancepoller/updater.go#L264 it looks like instance poller is trying to not call SetAddresses if the addresses haven't changed.

However the bug appears to be that Machine.Addresses() is returning the union of cloud addresses and machine addresses, while SetAddresses is only updating the former.
https://github.com/juju/juju/blob/juju-1.20.11/state/machine.go#L940

Thus every loop on the instance poller is seeing that the addresses don't exactly match, so tries to force the update, which then does nothing because the cloud side of the address list already matches.

The big problem is that SetAddresses() and Addresses() do not round trip data, which breaks developer's models of how this works.

We could potentially rename the functions SetCloudAddresses (vs SetMachineAddresses), and Addresses() continues to return all addresses but we add a CloudAddresses() (ProviderAddresses?) function to retrieve just the values we are about to set.