"send_ip_addr_adv_notif" can't use eventlet when called from "keepalived_state_change"

Bug #1870313 reported by Rodolfo Alonso
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
neutron
Fix Released
High
Rodolfo Alonso

Bug Description

"keepalived_state_change" monitor does not use eventlet but normal Python threads. When "send_ip_addr_adv_notif" is called from inside the monitor, the arping command is never sent because the eventlet thread does not start. In order to be able to be called from this process, this method should also have an alternative implementation using "threading".

This should have been captured by "TestMonitorDaemon.test_new_fip_sends_garp", but there is a problem in the implementation of this test. This test created two namespaces with a veth interface connecting both. The "keepalived_state_change" monitor is set to capture the events in the first namespace and the interface created inside. The test expects the monitor to send a GARP when a new IP address is added to the monitored interface. But this agent does not send a GARP from the monitored interface if a new IP address is set but from any other interface in this namespace [1].

This test use to pass because when the "ip neigh" is checked the second time, the "expected_ip" address is in the second namespace ARP table. This is because when the test asserts there is no ping and then sets the IP address on the interface, the last ping is replied by the interface:
http://paste.openstack.org/show/791516/

This will, not intentionally, populate the ARP table in the second namespace but the GARP was not sent by the "keepalived_state_change" monitor.

[1] https://github.com/openstack/neutron/blob/8ee34655b8757086c03feecfda100333f47ed810/neutron/agent/l3/keepalived_state_change.py#L90

Changed in neutron:
assignee: nobody → Rodolfo Alonso (rodolfo-alonso-hernandez)
description: updated
description: updated
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to neutron (master)

Fix proposed to branch: master
Review: https://review.opendev.org/716944

Changed in neutron:
status: New → In Progress
tags: added: l3-ha
Changed in neutron:
importance: Undecided → High
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (master)

Reviewed: https://review.opendev.org/716944
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=21935365f29cce3fa95f032d9778786519461521
Submitter: Zuul
Branch: master

commit 21935365f29cce3fa95f032d9778786519461521
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Thu Apr 2 11:00:35 2020 +0000

    "keepalived_state_change" needs to use threading to send arping

    "keepalived_state_change" monitor does not use eventlet but normal
    Python threads. When "send_ip_addr_adv_notif" is called from inside
    the monitor, the arping command is never sent because the eventlet
    thread does not start. In order to be able to be called from this
    process, this method should also have an alternative implementation
    using "threading".

    "TestMonitorDaemon.test_new_fip_sends_garp" is also modified to
    actually test the GARP sent. The test was originally implemented with
    only one interface in the monitored namespace.
    "keepalived_state_change" sends a GARP when a new IP address is added
    in a interface other than the monitored one. That's why this patch
    creates a new interface and sets it as the monitor interface. When
    a new IP address is added to the other interface, the monitor populates
    it by sending a GARP through the modified interface [1].

    [1] https://github.com/openstack/neutron/blob/8ee34655b8757086c03feecfda100333f47ed810/neutron/agent/l3/keepalived_state_change.py#L90

    Change-Id: Ib69e21b4645cef71db07595019fac9af77fefaa1
    Closes-Bug: #1870313

Changed in neutron:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to neutron (stable/train)

Fix proposed to branch: stable/train
Review: https://review.opendev.org/717805

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (stable/train)

Reviewed: https://review.opendev.org/717805
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=476bb780671e338d39e5eb776254acca59ddce16
Submitter: Zuul
Branch: stable/train

commit 476bb780671e338d39e5eb776254acca59ddce16
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Thu Apr 2 11:00:35 2020 +0000

    "keepalived_state_change" needs to use threading to send arping

    "keepalived_state_change" monitor does not use eventlet but normal
    Python threads. When "send_ip_addr_adv_notif" is called from inside
    the monitor, the arping command is never sent because the eventlet
    thread does not start. In order to be able to be called from this
    process, this method should also have an alternative implementation
    using "threading".

    "TestMonitorDaemon.test_new_fip_sends_garp" is also modified to
    actually test the GARP sent. The test was originally implemented with
    only one interface in the monitored namespace.
    "keepalived_state_change" sends a GARP when a new IP address is added
    in a interface other than the monitored one. That's why this patch
    creates a new interface and sets it as the monitor interface. When
    a new IP address is added to the other interface, the monitor populates
    it by sending a GARP through the modified interface [1].

    [1] https://github.com/openstack/neutron/blob/8ee34655b8757086c03feecfda100333f47ed810/neutron/agent/l3/keepalived_state_change.py#L90

    Change-Id: Ib69e21b4645cef71db07595019fac9af77fefaa1
    Closes-Bug: #1870313
    (cherry picked from commit 21935365f29cce3fa95f032d9778786519461521)

tags: added: in-stable-train
tags: added: neutron-proactive-backport-potential
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.