Dynamic routing: adding speaker to agent fails

[Dr. Jens Harbott]

SRU details for Ubuntu
----------------------
[Impact]
See "Original description" below.

[Test Case]
See "Original description" below.

[Regression Potential]
Low. This is fixed upstream in corresponding stable branches.

Original description
--------------------
When following https://docs.openstack.org/neutron-dynamic-routing/latest/contributor/testing.html everything works fine because the speaker is scheduled to the agent automatically (in contrast to what the docs say). But if I remove the speaker from the agent and add it again with

$ neutron bgp-dragent-speaker-remove 0159fc0a-22de-4995-8fad-8fb8835a4d86 bgp-speaker
$ neutron bgp-dragent-speaker-add 0159fc0a-22de-4995-8fad-8fb8835a4d86 bgp-speaker

the following error is seen in the log:

Feb 17 10:56:30 test-node01 neutron-bgp-dragent[18999]: ERROR neutron_dynamic_routing.services.bgp.agent.bgp_dragent [None req-da9a22ae-52a2-4be7-a3e8-2dc2dc970fdd admin admin] Call to driver for BGP Speaker d2aa5935-30c2-4369-83ee-b3a0ff77cc49 add_bgp_peer has failed with exception 'auth_type'.

The same thing happens when there are multiple agents and one tries to add the speaker to one of the other agents.

[OpenStack Infra (hudson-openstack)]

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

[Tim Way (tim.way)]

This makes BGP essentially useless after the Neutron agent (container in my case) reboots in stable/pike installed by Openstack Ansible. Please merge the patch into Neutron so it can be cherry-picked to Neutron's stable/pike branch so the sha's can be bumped to snag it.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/queens
Review: https://review.openstack.org/555161

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/pike
Review: https://review.openstack.org/555162

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/545783
Committed: https://git.openstack.org/cgit/openstack/neutron-dynamic-routing/commit/?id=fd497a491afec89d4a15cf061b5d5610fb7ad6d2
Submitter: Zuul
Branch: master

commit fd497a491afec89d4a15cf061b5d5610fb7ad6d2
Author: Jens Harbott <email address hidden>
Date: Mon Feb 19 10:53:04 2018 +0100

    Fix failure when adding a speaker to an agent

    When get_bgp_speaker_with_advertised_routes() collects the data for BGP
    peers, it needs to include the auth_type field, otherwise adding the
    speaker to an agent will fail.

    Change-Id: I668397be4a531ba5c0628eb23df9a84d0de8c28c
    Closes-Bug: 1750121

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/555162
Committed: https://git.openstack.org/cgit/openstack/neutron-dynamic-routing/commit/?id=c1914078fc2d75ca18e875cdc55a3ee49a9bcd9c
Submitter: Zuul
Branch: stable/pike

commit c1914078fc2d75ca18e875cdc55a3ee49a9bcd9c
Author: Jens Harbott <email address hidden>
Date: Mon Feb 19 10:53:04 2018 +0100

    Fix failure when adding a speaker to an agent

    When get_bgp_speaker_with_advertised_routes() collects the data for BGP
    peers, it needs to include the auth_type field, otherwise adding the
    speaker to an agent will fail.

    (cherry picked from commit fd497a491afec89d4a15cf061b5d5610fb7ad6d2)
    Change-Id: I668397be4a531ba5c0628eb23df9a84d0de8c28c
    Closes-Bug: 1750121

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/555161
Committed: https://git.openstack.org/cgit/openstack/neutron-dynamic-routing/commit/?id=a29018ba87b18828c52326030413bc5e55f3668b
Submitter: Zuul
Branch: stable/queens

commit a29018ba87b18828c52326030413bc5e55f3668b
Author: Jens Harbott <email address hidden>
Date: Mon Feb 19 10:53:04 2018 +0100

    Fix failure when adding a speaker to an agent

    When get_bgp_speaker_with_advertised_routes() collects the data for BGP
    peers, it needs to include the auth_type field, otherwise adding the
    speaker to an agent will fail.

    (cherry picked from commit fd497a491afec89d4a15cf061b5d5610fb7ad6d2)
    Change-Id: I668397be4a531ba5c0628eb23df9a84d0de8c28c
    Closes-Bug: 1750121

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron-dynamic-routing 13.0.0.0b1 development milestone.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron-dynamic-routing 11.0.1 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron-dynamic-routing 12.0.1 release.

[Corey Bryant (corey.bryant)]

I've uploaded new versions of neutron-dynamic-routing with the cherry-picked patch to cosmic, bionic, and artful where the package is awaiting acceptance from the unapproved queues:

https://launchpad.net/ubuntu/cosmic/+queue?queue_state=1&queue_text=
https://launchpad.net/ubuntu/bionic/+queue?queue_state=1&queue_text=
https://launchpad.net/ubuntu/artful/+queue?queue_state=1&queue_text=

[Launchpad Janitor (janitor)]

This bug was fixed in the package neutron-dynamic-routing - 2:12.0.0-0ubuntu2

---------------
neutron-dynamic-routing (2:12.0.0-0ubuntu2) cosmic; urgency=medium

  * d/p/fix-failure-when-adding-a-speaker-to-an-agent.patch: Cherry-picked
    from upstream stable/queens branch to ensure adding speaker to agent
    doesn't fail (LP: #1750121).

 -- Corey Bryant <email address hidden> Wed, 25 Apr 2018 09:16:30 -0400

[Brian Murray (brian-murray)]

Hello Dr., or anyone else affected,

Accepted neutron-dynamic-routing into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/neutron-dynamic-routing/2:12.0.0-0ubuntu1.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Brian Murray (brian-murray)]

Hello Dr., or anyone else affected,

Accepted neutron-dynamic-routing into artful-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/neutron-dynamic-routing/2:11.0.0-0ubuntu2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-artful to verification-done-artful. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-artful. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Dr. Jens Harbott (j-harbott)]

@Corey: There are releases 11.0.1 and 12.0.1 containing the patch, so I don't understand why you cherry-picked it instead of using the new releases.

[Corey Bryant (corey.bryant)]

Hello Dr., or anyone else affected,

Accepted neutron-dynamic-routing into queens-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:queens-proposed
  sudo apt-get update

Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-queens-needed to verification-queens-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-queens-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Corey Bryant (corey.bryant)]

Hello Dr., or anyone else affected,

Accepted neutron-dynamic-routing into pike-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:pike-proposed
  sudo apt-get update

Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-pike-needed to verification-pike-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-pike-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Corey Bryant (corey.bryant)]

@Jens, we just don't have full test coverage for neutron-dynamic-routing quite yet on our end so I was trying to keep the stable delta to a minimum. If by any chance you could test corresponding releases for us then I can prepare some stable point releases.

[Dr. Jens Harbott (j-harbott)]

Ah, o.k., well for stable/pike this is actually the only patch from 11.0.0 to 11.0.1, so maybe we can keep your version there, except that it may be confusing to users.

In 12.0.1 there is only https://review.openstack.org/544866 which I think may be relevant for upgrades.

Anyway if you build new packages I sure can test them.

[David Ames (thedac)]

I still see this failure when the neutron-bgp-dragent is restarted.

Bionic proposed: 12.0.0-0ubuntu1.1

The initial setup works fine:
2018-05-07 17:15:59.099 17215 INFO bgpspeaker.api.base [req-9692824c-b285-4304-86d2-00f46df8a216 - - - - -] API method core.start called with args: {'router_id': '10.5.0.82', 'label_range': (100, 100000), 'waiter'
: <ryu.lib.hub.Event object at 0x7f7cba5529d0>, 'bgp_server_port': 0, 'local_as': 12345, 'allow_local_as_in_count': 0, 'refresh_stalepath_time': 0, 'cluster_id': None, 'local_pref': 100, 'refresh_max_eor_time': 0}
2018-05-07 17:15:59.199 17215 INFO neutron_dynamic_routing.services.bgp.agent.driver.ryu.driver [req-9692824c-b285-4304-86d2-00f46df8a216 - - - - -] Added BGP Speaker for local_as=12345 with router_id= 10.5.0.82.
2018-05-07 17:16:00.689 17215 INFO bgpspeaker.api.base [req-6905ebee-595e-4a72-b2ac-d0372116f310 - - - - -] API method network.add called with args: {'prefix': u'192.168.0.0/24', 'next_hop': u'10.5.150.0'}
2018-05-07 17:16:00.691 17215 INFO neutron_dynamic_routing.services.bgp.agent.driver.ryu.driver [req-6905ebee-595e-4a72-b2ac-d0372116f310 - - - - -] Route cidr=192.168.0.0/24, nexthop=10.5.150.0 is advertised for
BGP Speaker running for local_as=12345.
2018-05-07 17:16:02.152 17215 INFO bgpspeaker.api.base [req-0f7a3721-db7e-418d-8ee7-0b950b4ddc88 103842446d8b4a029c1892ffb576d57d 15205edcf62643d7a3723ff7e23b74fc - - -] API method neighbor.create called with args
: {'connect_mode': 'active', 'cap_mbgp_evpn': False, 'remote_as': 10000, 'cap_mbgp_vpnv6': False, 'cap_mbgp_l2vpnfs': False, 'cap_four_octet_as_number': True, 'cap_mbgp_ipv6': False, 'is_next_hop_self': False, 'cap_mbgp_ipv4': True, 'cap_mbgp_ipv4fs': False, 'is_route_reflector_client': False, 'cap_mbgp_ipv6fs': False, 'is_route_server_client': False, 'cap_enhanced_refresh': False, 'peer_next_hop': None, 'password': None,
'ip_address': u'10.5.0.79', 'cap_mbgp_vpnv4fs': False, 'cap_mbgp_vpnv4': False, 'cap_mbgp_vpnv6fs': False}
2018-05-07 17:16:02.153 17215 INFO neutron_dynamic_routing.services.bgp.agent.driver.ryu.driver [req-0f7a3721-db7e-418d-8ee7-0b950b4ddc88 103842446d8b4a029c1892ffb576d57d 15205edcf62643d7a3723ff7e23b74fc - - -] A$ded BGP Peer 10.5.0.79 for remote_as=10000 to BGP Speaker running for local_as=12345.
2018-05-07 17:16:03.158 17215 INFO bgpspeaker.peer [-] Connection to peer: 10.5.0.79 established
2018-05-07 17:16:03.159 17215 INFO neutron_dynamic_routing.services.bgp.agent.driver.ryu.driver [-] BGP Peer 10.5.0.79 for remote_as=10000 is UP.
2018-05-07 17:16:04.167 17215 INFO neutron_dynamic_routing.services.bgp.agent.driver.ryu.driver [-] Best path change observed. cidr=10.5.0.0/16, nexthop=10.5.0.79, remote_as=10000, is_withdraw=False
2018-05-07 17:16:04.169 17215 INFO neutron_dynamic_routing.services.bgp.agent.driver.ryu.driver [-] Best path change observed. cidr=252.0.0.0/8, nexthop=10.5.0.79, remote_as=10000, is_withdraw=False
2018-05-07 17:16:08.756 17215 INFO bgpspeaker.api.base [req-55216404-ace9-4f46-8915-c952549a61...

[Corey Bryant (corey.bryant)]

I've tagged the verification of this bug as failed since restart of neutron-bgp-dragent results in a missing auth_type when attempting to recreate the peer. add_bgp_peer_to_bgp_speaker() gets a KeyError exception for 'auth_type' as can be seen in David's results in the previous comment.

I cross-checked the cherry-picked patch in 12.0.0-0ubuntu1.1 and it's the same as what upstream stable/queens has.

[Dr. Jens Harbott (j-harbott)]

@David: I have not been able to reproduce the error on xenial with queens UCA. Your logs look like you are running without the patch applied. Can you doublecheck that you did install the patched packages? Can you set "debug=true" in /etc/neutron/bgp_dragent.ini and post logs from that?

From my POV the pkg works as expected, so I'd set "verification-queens-done" but I don't want to override your report without your feedback. I'll do another test on bionic tomorrow.

[Dr. Jens Harbott (j-harbott)]

@David: Did you also restart neutron-server after upgrading the neutron-dynamic-routing packages? Omitting that step was the only way I could reproduce your error during my testing on bionic.

@Corey: Do we need to place that restart into the postinst script to make it automatic?

[Corey Bryant (corey.bryant)]

@Dr. Jens, Thanks for testing. I'd prefer the user manually restart neutron-server. It doesn't feel right to have neutron-dynamic-routing postinst do a restart of neutron-server.

[David Ames (thedac)]

@Jens,

Some points of clarification. It is possible this is a different but related bug. First off, no upgrade is occuring. This is a CI environment [0] in which I do a fresh deployment of neutron and neutron-bgp-dragent on Xenial with the Queens UCA. The deployment and initial configuration, including the bgp peer setup, works as expected. The config roughly follows [1].

Subsequent to the working deployment, I see the "KeyError: 'auth_type' Unable to sync BGP speaker state" after a simple restart of the neutron-bgp-dragent.

I confirmed the version of the dynamic routing code includes the changes in this bug report.
ii neutron-dynamic-routing-common 2:12.0.0-0ubuntu1.1~cloud0 all OpenStack Neutron Dynamic Routing - common files
ii python-neutron-dynamic-routing 2:12.0.0-0ubuntu1.1~cloud0 all OpenStack Neutron Dynamic Routing - Python 2.7 library

Steps to reproduce are as follows:

Deploy stack including neutron, dragent and quagga
Configure networking and dynamic routing [1]
Validate BGP peering relations via quagga (vtysh -c "show ip route bgp")

Note: All is working at this point.

Restart neutron-bgp-dragent
See "KeyError: 'auth_type' Unable to sync BGP speaker state" in the neutron-bgp-dragent.log

Peering relationship is dead on quagga.

I have uploaded the neutron-bgp-dragent log with debug=True. I added a note where the dragent restart takes place in the log.

Regardless if this is the same bug or a different one it is critical as it means dragent cannot be used in production. Let me know what else I can do to provide information.

[0] https://github.com/openstack/charm-neutron-dynamic-routing/blob/master/src/tox.ini#L29
[1] https://docs.openstack.org/neutron-dynamic-routing/latest/contributor/testing.html

[Dr. Jens Harbott (j-harbott)]

I still fail to reproduce this locally. Can you describe the steps that are needed to setup your CI environment? Do you see the error only in your CI or also when you follow the steps in [1] manually?
Is this an aio setup or are neutron-api and neutron-bgp-dragent running in different environments? Can you also please check what happens in your setup when using the original 12.0.0 pkgs?

[David Ames (thedac)]

@Jens,

Apologies for the delay. Of course, you were correct all along. The neutron-server node had the older version of the package. Though I looked at that 10 times I failed to process it. My further apologies for dragging you along for this process. This bug is resolved as far as I am concerned.

Thank you for your work on this.

[Chris Halse Rogers (raof)]

It looks like this might be mis-tagged verification-failed-bionic, but it's not entirely clear to me. Can someone please confirm that this *does* work on bionic, and then we can release the artful and bionic packages?

Thanks!

[Dr. Jens Harbott (j-harbott)]

@Chris: The test were done successfully, I just missed that tag when updating yesterday, sorry. Should be ready to release both artful and bionic, as well as the corresponding UCA updates.

[Robie Basak (racb)]

For Ubuntu, could somebody please confirm the package version strings that were tested for Artful and Bionic? Before I release the tested packages in Artful and Bionic, I want to make sure that what I'm releasing is definitely what was tested, and specifying the package version strings avoids any confusions between UCA, Ubuntu, upstream, etc. You can get the string using the command "dpkg-query -W neutron-bgp-dragent" on the test system for example.

[Dr. Jens Harbott (j-harbott)]

I did test 2:11.0.0-0ubuntu2~cloud0 and 2:12.0.0-0ubuntu1.1~cloud0 on Xenial. The packages seem to be identical to those in Artful and Bionic.

[Łukasz Zemczak (sil2100)]

For the SRU verification to be valid, we need testers to test the exact same binaries that are supposed to be released into -updates. The reason for that is that the build environment can be different for different binaries, even though the sources might be the same. The build environment can then influence the actual behavior and cause regressions, which already happened a few times before.

Could you please perform validation on versions present in -proposed?

[Malte Swart (malte.swart)]

I successfully use neutron-dynamic-routing-common, python-neutron-dynamic-routing and neutron-bgp-dragent in version 2:12.0.0-0ubuntu1.1 (from bionic-proposed) on bionic to provide BGP peerings with Neutron network routes.

Host with bgp-dragent:

> lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description: Ubuntu 18.04 LTS
Release: 18.04
Codename: bionic

> dpkg -l | grep Dynamic\ Routing
ii neutron-bgp-dragent 2:12.0.0-0ubuntu1.1 all OpenStack Neutron Dynamic Routing - Agent
ii neutron-dynamic-routing-common 2:12.0.0-0ubuntu1.1 all OpenStack Neutron Dynamic Routing - common files
ii python-neutron-dynamic-routing 2:12.0.0-0ubuntu1.1 all OpenStack Neutron Dynamic Routing - Python 2.7 library

Host with neutron-server:
> lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description: Ubuntu 18.04 LTS
Release: 18.04
Codename: bionic

> dpkg -l | grep Dynamic\ Routing
ii python-neutron-dynamic-routing 2:12.0.0-0ubuntu1.1 all OpenStack Neutron Dynamic Routing - Python 2.7 library

[Robie Basak (racb)]

Thank you for the testing. I will release 2:12.0.0-0ubuntu1.1 from bionic-proposed to bionic-updates based on this result.

2:11.0.0-0ubuntu2 in artful-proposed still needs verifying, so I'll reset the tag for that. We can release it to artful-updates after someone can confirm testing for 2:11.0.0-0ubuntu2 in Artful. Once someone has tested 2:11.0.0-0ubuntu2 in Artful please change the tag back.

[Robie Basak (racb)]

The verification of the Stable Release Update for neutron-dynamic-routing has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package neutron-dynamic-routing - 2:12.0.0-0ubuntu1.1

---------------
neutron-dynamic-routing (2:12.0.0-0ubuntu1.1) bionic; urgency=medium

  * d/p/fix-failure-when-adding-a-speaker-to-an-agent.patch: Cherry-picked
    from upstream stable/queens branch to ensure adding speaker to agent
    doesn't fail (LP: #1750121).
  * d/gbp.conf: Create stable/queens branch.

 -- Corey Bryant <email address hidden> Wed, 25 Apr 2018 09:16:30 -0400

[Dr. Jens Harbott (j-harbott)]

If it needs testing in artful in order to get fixes into Pike UCA, then IMO your process is broken. I think I spent more than enough time on this, if you insist to prefer keeping to distribute faulty packages, so be it.

[Corey Bryant (corey.bryant)]

The verification of the Stable Release Update for neutron-dynamic-routing has completed successfully and the package has now been released to -updates. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Corey Bryant (corey.bryant)]

This bug was fixed in the package neutron-dynamic-routing - 2:12.0.0-0ubuntu1.1~cloud0
---------------

 neutron-dynamic-routing (2:12.0.0-0ubuntu1.1~cloud0) xenial-queens; urgency=medium
 .
   * New update for the Ubuntu Cloud Archive.
 .
 neutron-dynamic-routing (2:12.0.0-0ubuntu1.1) bionic; urgency=medium
 .
   * d/p/fix-failure-when-adding-a-speaker-to-an-agent.patch: Cherry-picked
     from upstream stable/queens branch to ensure adding speaker to agent
     doesn't fail (LP: #1750121).
   * d/gbp.conf: Create stable/queens branch.

[Corey Bryant (corey.bryant)]

Marking Artful as Won't Fix since it's nearly EOL.

[Corey Bryant (corey.bryant)]

The verification of the Stable Release Update for neutron-dynamic-routing has completed successfully and the package has now been released to -updates. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Corey Bryant (corey.bryant)]

This bug was fixed in the package neutron-dynamic-routing - 2:11.0.0-0ubuntu2~cloud0
---------------

 neutron-dynamic-routing (2:11.0.0-0ubuntu2~cloud0) xenial-pike; urgency=medium
 .
   * New update for the Ubuntu Cloud Archive.
 .
 neutron-dynamic-routing (2:11.0.0-0ubuntu2) artful; urgency=medium
 .
   * d/gbp.conf: Create stable/pike branch.
   * d/p/fix-failure-when-adding-a-speaker-to-an-agent.patch: Cherry-picked
     from upstream stable/pike branch to ensure adding speaker to agent
     doesn't fail (LP: #1750121).