L3 HA state transitions raceful; Metadata proxy not always opened and closed correctly

[Assaf Muller]

L3 HA configures keepalived to invoke a bash script asynchronously whenever it performs a state transition. This script writes the new state to disk and spawns or destroys the metadata proxy depending on the new state. Manual testing has revealed that when keepalived changes state twice consecutively, for example, to standby and then to master, the standby and master scripts are called in the correct order, but may then execute code in their scripts out of order.

For example, keepalived changes state to standby and then immediately to master.
notify_standby is executed, followed by notify_master. However, notify_master writes 'master' to disk first, then notify_standby writes 'standby'. Spawning and destroying the metadata proxy may also be performed out of order.

Currently, the state is written to disk for debugging purposes and so the effect of the bug is that a new master may not have the metadata proxy up, and that routers going to standby may not shut down the proxy.

Note that the bash notifier scripts will be replaced by Python scripts during the Kilo cycle. The Python script will write the new state to disk, then inform the agent of a state transition via a Unix domain socket. The agent will then manage the metadata proxy and perform additional actions. Since bp/report-ha-router-master has not yet been merged, this bug will be fixed in the yet-unmerged Python script pre-merge as would be expected. Additionally the bug will be fixed in the current Bash scripts, and back ported to Juno.

How to reproduce:
Three L3 agents, max_l3_agents_per_router = 3. Cause all three router instances to fail by shutting down the HA interface in the router namespaces, then bring two back simultaneously. I successfully reproduce the issue after 5 or 6 times. Perhaps there's an easier reproduction. I haven't been able to automate a failing functional test at this time.

[Assaf Muller (amuller)]

About the severity of the bug, apart from what's outlined above, the bug becomes very visible with bp/report-ha-router-master, because at that point the router states are reported to the controller and via the API. Once the bug reproduces I see two masters and the like.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/141645

[OpenStack Infra (hudson-openstack)]

Change abandoned by Assaf Muller (<email address hidden>) on branch: master
Review: https://review.openstack.org/141645
Reason: Abandoning in favor of an entirely different approach.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/149647

[Assaf Muller (amuller)]

I originally thought to solve this by somehow serializing the keepalived notifier scripts, but after spending an enormous amount of time trying I determined that that's impossible. Instead, I intend to abandon the keepalived notifier scripts altogether and instead monitor the IP addresses in the router namespaces. Since keepalived configures IP addresses only on the master instance, that's a good way of knowing the state of an HA router. This approach is technically backportable (No DB upgrades, no RPC or API changes, etc) but it might just be too large / risky. We can determine that once the complete solution is up for review.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/158097

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/158097
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=7907d40075de7528122b0e900a5699b280a04901
Submitter: Jenkins
Branch: master

commit 7907d40075de7528122b0e900a5699b280a04901
Author: Assaf Muller <email address hidden>
Date: Sat Feb 21 19:27:44 2015 -0500

    Allow AsyncProcess to block on process start and stop

    * Move utility functions from the test tree to the non-test tree
    * Add tests for the newly moved functions
    * Use these functions in AsyncProcess

    This will allow the ip monitor in the following patch to start
    and stop in a synchronous manner.

    Related-Bug: #1402010
    Change-Id: I03727d8acc17e561d3473b0ebecfbe49cb5523b1

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/149647
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=aec3a94cd360be94a20d095241e5b2a6dd5ec21f
Submitter: Jenkins
Branch: master

commit aec3a94cd360be94a20d095241e5b2a6dd5ec21f
Author: Assaf Muller <email address hidden>
Date: Sat Feb 21 19:28:54 2015 -0500

    Introduce ip address monitor

    In Juno, we used keepalived notifier scripts to report the local
    state of an HA router's state. These have been found to be
    unreliable. The proposed approach is to not use them altogether.
    Instead, monitor the omnipresent VIP on the HA device - It is
    only configured on the master instance. In order to do that,
    we'll use the 'ip monitor address' wrapper introduced in this patch
    to get address change events as they happen to avoid polling.

    Related-Bug: #1402010
    Change-Id: Icc2c07efb7e20008ff5b07d7df2104e6099091d7

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/155058
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=287754bd40393ff12e91ccb81287364fcbae13ed
Submitter: Jenkins
Branch: master

commit 287754bd40393ff12e91ccb81287364fcbae13ed
Author: Assaf Muller <email address hidden>
Date: Wed Feb 11 16:11:33 2015 -0500

    Change metadata driver unit tests to use monitored spawn

    The modified unit test is currently the sole user of the
    unmonitored _spawn_metadata_proxy. We must remove this usage so that
    when the L3 agent manages the metadata proxy for HA routers,
    the unmanaged methods may be removed.

    Related-Bug: #1402010
    Change-Id: Ib57e63dcadc378ac51702949ed24901d403002dd

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/125384
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=9bae3b1832798a274bef4a77e5acda1abaa51f2a
Submitter: Jenkins
Branch: master

commit 9bae3b1832798a274bef4a77e5acda1abaa51f2a
Author: Assaf Muller <email address hidden>
Date: Thu Mar 12 19:50:43 2015 -0400

    Replace keepalived notifier bash script with Python ip monitor

    Previously L3 HA generated a bash script and copied it to a per-router
    configuration directory that was visible to that router's keepalived
    instance. This patch changes the in-line generated Bash script to a
    Python script that can be maintained in the repository.
    The bash script was used as a keepalived notifier script, that was invoked
    by keepalived whenever a state transition occured. These notifier scripts
    may be invoked by keepalived out of order in case it transitions quickly
    twice. For example, if the master failed and two slaves fight for the new
    master role. One will transition to master, and the other will often
    transition to master and then immidiately back to standby. In this case,
    the transition scripts were often fired out of order, resulting in the
    wrong state being reported.

    The proposed approach is to get rid of the keepalived notifier scripts
    entirely. Instead, monitor IP changes on the HA device. If the omnipresent
    IP address was configured on the HA device, it means that we're looking
    at a master instance. If it was deleted, the router transition to standby
    or fault.

    In order to keep the L3 agent CPU usage down, it will spawn a process
    per HA router. That process will start the ip address monitor.
    Whenever it gets an IP address change event, it will notify the L3 agent
    via a unix domain socket.

    Partially-Implements: blueprint report-ha-router-master
    Change-Id: I2022bced330d5f108fbedd40548a901225d7ea1c
    Closes-Bug: #1402010
    Closes-Bug: #1367705