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

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

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 ]

  *

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to neutron (master)

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

Changed in neutron:
assignee: nobody → Lars Kellogg-Stedman (larsks)
status: New → In Progress
Revision history for this message
Matthew Thode (prometheanfire) wrote : Re: agent/linux/ip_lib.py does not correctly handle output from 'iproute2' command

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on neutron (master)

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.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to neutron (master)

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

Changed in neutron:
assignee: Lars Kellogg-Stedman (larsks) → Matthew Thode (prometheanfire)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (master)

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
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to neutron (master)

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

Thierry Carrez (ttx)
Changed in neutron:
milestone: none → kilo-3
status: Fix Committed → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to neutron (master)

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)
Changed in neutron:
milestone: kilo-3 → 2015.1.0
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to neutron (neutron-pecan)

Related fix proposed to branch: neutron-pecan
Review: https://review.openstack.org/185072

Shane Peters (shaner)
Changed in neutron (Ubuntu):
assignee: nobody → Shane (shaner)
Shane Peters (shaner)
tags: added: sts
Shane Peters (shaner)
description: updated
Revision history for this message
Shane Peters (shaner) wrote : Re: agent/linux/ip_lib.py does not correctly handle output from 'iproute2' command

Patch for neutron_2014.1.5-0ubuntu6

Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

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)
Changed in neutron (Ubuntu):
status: New → In Progress
Revision history for this message
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

Revision history for this message
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
Revision history for this message
Brian Murray (brian-murray) wrote : Please test proposed package

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
Revision history for this message
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)
tags: added: verification-done
removed: verification-needed
Mathew Hodson (mhodson)
Changed in neutron (Ubuntu):
importance: Undecided → Medium
Changed in neutron (Ubuntu Trusty):
importance: Undecided → Medium
Revision history for this message
Martin Pitt (pitti) wrote : Update Released

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.

Revision history for this message
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
Shane Peters (shaner)
Changed in neutron (Ubuntu Trusty):
assignee: Shane Peters (shaner) → nobody
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers