AllWatcher does not remove last closed port for a unit, last removed service config

Bug #1428430 reported by Dimiter Naydenov
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
juju-core
Fix Released
High
Frank Mueller

Bug Description

I've discovered the AllWatcher reports stale changes in some cases - already closed ports for units and already unset service config settings.

Steps to reproduce (with ports):

1. Bootstrap a local environment (latest trunk)
2. Deploy juju-gui
3. Deploy ubuntu
4. Wait for both of them to start (not that important, just need access to the GUI)
5. Login into the GUI, click on the unit ubuntu/0 - there are no ports, just addresses. No need to touch the GUI until the last step.
6. Run juju run --unit ubuntu/0 "opened-ports" - no results.
7. Again with juju run, execute the following: "open-port 1234/tcp", "open-port 25/udp".
8. Running "opened-ports" now reports 1234/tcp and 25/udp.
9. Run the reverse commands: "close-port 1234/tcp", "close-port 25/udp" (order does not matter), "opened-ports" - no results.
10. Click on the unit ubuntu/0 in the GUI and observe the last removed port (e.g. 25/udp above) still shows.

Looking at the logs even at trace level didn't help until I started adding more logging - http://paste.ubuntu.com/10533228/
I've realized removed() was empty for backingOpenedPorts in state/megawatcher.go, so even though it gets called as needed (see the added trace logs) it doesn't do anything. End result of the issue is the GUI and any other client using the AllWatcher will continue to see stale info until updated again (e.g. open another port).

The other case (stale service config setttings) is similar and due to the same cause (missing removed() handling for backingSettings). To reproduce, in addition to the above:

11. Deploy nrpe (or any service with config settings defined I guess - not confirmed it has to be a subordinate, but the ubuntu charm does not have any settings to set).
12. add-relation ubuntu nrpe, juju get nrpe - the usual output with only the default value for setting "monitors".
13. juju set nrpe monitors="bar", click on the service config tab in the GUI inspector for nrpe - "bar" is there, also running juju get nrpe "value: bar" is displayed.
14. juju unset nrpe, juju get nrpe - again the default value is used for "monitors", but clicking in the GUI again we can see "bar" is still in the config field for the service.

Also the logs show the last AllWatcher changes (after the unset command was run) still contains "Config": {"monitors": "bar"} instead of the expected {}.

I don't think this is a regression, as the code is pretty much unchanged from 1.20 in the megawatcher.
A few other entities also have an empty removed() method, but they are removed very soon after their parent entity and I couldn't find a way to abuse that.

Tags: api
summary: - AllWatcher does not remove last closed port for a unit
+ AllWatcher does not remove last closed port for a unit, last removed
+ service config
Frank Mueller (themue)
Changed in juju-core:
assignee: nobody → Frank Mueller (themue)
Revision history for this message
Frank Mueller (themue) wrote :

Fix is proposed and waits for second review after some changes.

Ian Booth (wallyworld)
Changed in juju-core:
status: Triaged → Fix Committed
Curtis Hovey (sinzui)
Changed in juju-core:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.