InstancePoller compares wrong Address list and always requests updated state Addresses
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
juju-core |
Fix Released
|
High
|
Tim Penhey | ||
1.22 |
Fix Released
|
Critical
|
Tim Penhey | ||
1.23 |
Fix Released
|
High
|
Tim Penhey | ||
1.24 |
Fix Released
|
High
|
Tim Penhey |
Bug Description
The Transaction log is full of entries that look like:
{ "_id": ObjectId(
"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:/
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:/
description: | updated |
tags: | added: cpec |
tags: |
added: network removed: cpec |
tags: | added: cpec |
summary: |
- Machine.SetAddresses should not generate transactions for noop changes + InstancePoller compares wrong Address list and always requests updated + state Addresses |
Changed in juju-core: | |
assignee: | nobody → Tim Penhey (thumper) |
assignee: | Tim Penhey (thumper) → nobody |
Changed in juju-core: | |
assignee: | nobody → Tim Penhey (thumper) |
status: | Triaged → In Progress |
milestone: | none → 1.25.0 |
Changed in juju-core: | |
status: | In Progress → Fix Committed |
Changed in juju-core: | |
status: | Fix Committed → Fix Released |
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.