[SRU] agent/linux/ip_lib.py does not correctly handle output from 'iproute2' command

Bug #1374663 reported by Lars Kellogg-Stedman on 2014-09-27
22
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Ubuntu Cloud Archive
Undecided
Unassigned
Icehouse
Undecided
Shane Peters
neutron
Undecided
Matthew Thode
neutron (Ubuntu)
Medium
Unassigned
Trusty
Medium
Shane Peters

Bug Description

[Impact]

 * The get_devices() method in neutron/agent/linux/ip_lib.py chokes if 'iproute2' presents interface names containing '@', such as ipip tunnels and macvlan devices:

  tunl0@NONE: <NOARP> mtu 1480 qdisc noop state DOWN mode DEFAULT group default
  clone@eth0: <BROADCAST,MULTICAST> mtu 1454 qdisc noop state DOWN mode DEFAULT

  * The mere presence of one of the above interface names will cause the
    code to fail with:

  RuntimeError:
  Command: ['ip', 'addr', 'show', 'tunl0@NONE']
  Exit code: 1
  Stdout: ''
  Stderr: 'Device "tunl0@NONE" does not exist.\n'

[Test Case]

 * This has been seen in the vpn_agent.log after upgrading to a xenial kernel (4.4.0-45-generic) in a trusty/icehouse environment.

    /var/log/neutron/vpn_agent.log:
    ERROR neutron.agent.linux.interface [-] Failed unplugging interface 'qg-e75da35-63@if10'

[Regression Potential]

  * iproute2 is still used, however, a search path was added (/sys/class/net) which returns the interfaces without the '@'.

  * This has been fixed since Kilo so there is minimal regression potential.

[ Other Info ]

  *

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

Changed in neutron:
assignee: nobody → Lars Kellogg-Stedman (larsks)
status: New → In Progress

I might try and take your fix and run with it, if you don't mind (submitting my own for review).

Change abandoned by Kyle Mestery (<email address hidden>) on branch: master
Review: https://review.openstack.org/124557
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.

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

Changed in neutron:
assignee: Lars Kellogg-Stedman (larsks) → Matthew Thode (prometheanfire)

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

commit c717a6365c1aecb0e3957f0857a06f2334f99d5d
Author: Matthew Thode <email address hidden>
Date: Mon Feb 9 11:02:58 2015 -0600

    replaces enumeration method used to get a list of interfaces

    ip_lib was parsing tunnel links incorrectly. We can create interface
    names with any character the filesystem supports (not '..', '/', ':').
    Given this we do not know what to delimit on so parsing iproute2 output
    is probably not a good idea.

    I asked the iproute2 devs what the proper way we should get interface
    names is and was told NOT to parse iproute2 output but to use something
    like sysfs instead. http://www.spinics.net/lists/netdev/msg316577.html

    This patch pulls interfaces from sysfs (/sys/class/net) and verifies them
    via checking if they are links (bonding creates files for instance and
    needs to be skipped).

    Currently it is not possible without jumping through a ton of hoops to
    access a network namespace without iproute2 or cython, so we use ip to
    run find to find the correct sysfs directory. We also only call out to
    iproute2 _ONLY_ if needed.

    Change-Id: I07d1d297f07857d216649cccf717896574aac301
    Closes-Bug: 1374663

Changed in neutron:
status: In Progress → Fix Committed
Thierry Carrez (ttx) on 2015-03-19
Changed in neutron:
milestone: none → kilo-3
status: Fix Committed → Fix Released

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

commit 78d3b40899b81dd2ecfadcc8547c8eabc6849e53
Author: YAMAMOTO Takashi <email address hidden>
Date: Wed Mar 18 13:27:15 2015 +0900

    linuxbridge UT: Fix a regression of the recent ip_lib change

    A recently merged change, I07d1d297f07857d216649cccf717896574aac301,
    changed IPWrapper.get_devices to use /sys instead of executing ip command.
    Unfortunately it broke linuxbridge unit tests, which seems to assume that
    mocking utils.execute is enough in some places. This commit fixes the
    regression.

    Closes-Bug: #1433417
    Related-Bug: #1374663
    Change-Id: I9570abe703b438a3fc358f747e25d023934d1ffd

Thierry Carrez (ttx) on 2015-04-30
Changed in neutron:
milestone: kilo-3 → 2015.1.0
Shane Peters (shaner) on 2016-10-27
Changed in neutron (Ubuntu):
assignee: nobody → Shane (shaner)
Shane Peters (shaner) on 2016-10-27
tags: added: sts
Shane Peters (shaner) on 2016-11-01
description: updated

The attachment "trusty_lp1374663.debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

tags: added: patch
Shane Peters (shaner) on 2016-11-02
Changed in neutron (Ubuntu):
status: New → In Progress
Corey Bryant (corey.bryant) wrote :

Shane,

Thanks for the patch. Just a few minor comments:
* did you mean to bump concurrency to 16? it's probably fine since it's just for running tests. just want to make sure it doesn't lock up the next dev's machine. :)
* if it's possible to cherry-pick this patch from the next higher available upstream release branch/tag (> icehouse) then I'd prefer you did that (git checkout icehouse-eol; git cherry-pick -x uuid of commit).
* also relevant patch headers should be added: http://packaging.ubuntu.com/html/patches-to-packages.html#patch-headers

Thanks,
Corey

Shane Peters (shaner) wrote :

Corey,
  I appreciate the feedback! Here's an updated patch.

Changed in cloud-archive:
status: New → Invalid
status: Invalid → Fix Released
Changed in neutron (Ubuntu Trusty):
assignee: nobody → Shane Peters (shaner)
Changed in neutron (Ubuntu):
assignee: Shane Peters (shaner) → nobody
Changed in neutron (Ubuntu Trusty):
status: New → In Progress
Changed in neutron (Ubuntu):
status: In Progress → Fix Released
summary: - agent/linux/ip_lib.py does not correctly handle output from 'iproute2'
- command
+ [SRU] agent/linux/ip_lib.py does not correctly handle output from
+ 'iproute2' command

Hello Lars, or anyone else affected,

Accepted neutron into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/neutron/1:2014.1.5-0ubuntu7 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 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 to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-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!

Changed in neutron (Ubuntu Trusty):
status: In Progress → Fix Committed
tags: added: verification-needed
Shane Peters (shaner) wrote :

I can confirm this bug is fixed in trusty/icehouse using kernel linux-image-generic-lts-xenial.

Installed: 1:2014.1.5-0ubuntu7

Shane Peters (shaner) on 2016-11-10
tags: added: verification-done
removed: verification-needed
Changed in neutron (Ubuntu):
importance: Undecided → Medium
Changed in neutron (Ubuntu Trusty):
importance: Undecided → Medium

The verification of the Stable Release Update for neutron 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) wrote :

This bug was fixed in the package neutron - 1:2014.1.5-0ubuntu7

---------------
neutron (1:2014.1.5-0ubuntu7) trusty; urgency=medium

  * agent/linux/ip_lib.py does not correctly handle output from 'iproute2'
    command. (LP: #1374663):
    - d/p/neutron-ns-iface-enum.patch: Searches /sys/class/net for the correct
      interface name instead of scraping iproute2 output.

 -- Shane Peters (Shaner) <email address hidden> Thu, 03 Nov 2016 16:53:31 -0400

Changed in neutron (Ubuntu Trusty):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers