ip filtering short circuits based on regex

Bug #1668828 reported by zhufl
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Invalid
Medium
Matt Riedemann
tempest
Fix Released
Undecided
zhufl

Bug Description

1. Problem:
    test_list_servers_filtered_by_ip sometimes fails with:
    Traceback (most recent call last):
      File "/home/zfl/tempest_m_git_local/tempest/api/compute/servers/test_list_server_filters.py", line 284, in test_list_servers_filtered_by_ip
        self.assertNotIn(self.s2_name, map(lambda x: x['name'], servers))
      File "/usr/lib/python2.7/site-packages/testtools/testcase.py", line 392, in assertNotIn
        self.assertThat(haystack, matcher, message)
      File "/usr/lib/python2.7/site-packages/testtools/testcase.py", line 433, in assertThat
        raise mismatch_error
    MismatchError: [u'tempest-ListServerFiltersTestJSON-instance-328761420'] matches Contains('tempest-ListServerFiltersTestJSON-instance-328761420')

2. Reason:
    We can see from nova code that filter by ip is always "regexp match",
    so if server2' ip happened to be part of server1's ip, then filter by
    server1's ip will also return server2.
    https://github.com/openstack/nova/blob/63805735c25a54ad1b9b97e05080c1a6153d8e22/nova/compute/api.py
    get_all

3. Solution:
    There are 3 possibles ways,
    1) create servers with specified ips, such as 1.1.1.1, 1.1.1.2, 1.1.1.3
    2) find the longest ip in servers, and filter it by that ip, then only
       one server will be returned
    3) filter using server1's ip, and using assertIn or assertNotIn according
       to whether other server ip is part of server1's ip

zhufl (zhu-fanglei)
description: updated
Changed in tempest:
assignee: nobody → zhufl (zhu-fanglei)
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to tempest (master)

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

Revision history for this message
Matt Riedemann (mriedem) wrote : Re: test_list_servers_filtered_by_ip fails ccasionally

The bug is actually in nova here:

https://github.com/openstack/nova/blob/3a2a181796664fea7f6069d49f9cd367b8cff825/nova/compute/api.py#L2508

As demonstrated in this example:

>>> import re
>>> ipv4_f = re.compile(str('10.1.1.1'))
>>> address1 = '10.1.1.1'
>>> address2 = '10.1.1.10'
>>> addresses = (address1, address2)
>>> for addr in addresses:
... if ipv4_f.match(addr):
... print 'match found on address: %s' % addr
...
match found on address: 10.1.1.1
match found on address: 10.1.1.10
>>>

Nova should probably just attempt to do an exact match and if none found, then do the regex match but rather than return the first match, return all matches.

Revision history for this message
Matt Riedemann (mriedem) wrote :

When the ip filtering started happening in the compute API code originally, we always returned all of the matches:

https://github.com/openstack/nova/commit/ae781ee97947c33d6d43e4c21df4f338c875bf1c

That was regressed about 2 years ago here:

https://github.com/openstack/nova/commit/8fc7eee58ebd929dda75e467f93c381091da7ace

Where it short-circuits and only returns the first match. So as far as latent behavior is concerned, I think we could go back to returning all matches since it was a very old regression to begin with (regressed in kilo).

Changed in nova:
status: New → Triaged
importance: Undecided → Medium
Matt Riedemann (mriedem)
no longer affects: tempest
Matt Riedemann (mriedem)
summary: - test_list_servers_filtered_by_ip fails ccasionally
+ ip filtering short circuits based on regex
Revision history for this message
Matt Riedemann (mriedem) wrote :

I'm going to work on creating a functional test within nova that can reproduce this bug.

Changed in nova:
assignee: nobody → Matt Riedemann (mriedem)
Revision history for this message
Matt Riedemann (mriedem) wrote :

Now that I've written a recreate test for this on the nova side, I think patch set 3 on the Tempest change was actually correct and the nova api is performing as intended. The nova api code performs a regex match on the ip filter, it always has. Earlier today when I was looking at this, I thought the nova-api code was short circuiting and only returning the 10.1.1.10 match in this example, but now that I re-read the commit message on the tempest change, it's actually returning both 10.1.1.10 and 10.1.1.1 and it's tempest that's not handling that correctly.

Changed in nova:
status: Triaged → Invalid
Changed in tempest:
status: New → Confirmed
status: Confirmed → In Progress
assignee: nobody → zhufl (zhu-fanglei)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (master)

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

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

Reviewed: https://review.openstack.org/439428
Committed: https://git.openstack.org/cgit/openstack/tempest/commit/?id=7d8f7c5669f42178ae47aa1a6446eda42910a390
Submitter: Jenkins
Branch: master

commit 7d8f7c5669f42178ae47aa1a6446eda42910a390
Author: zhufl <email address hidden>
Date: Wed Mar 1 15:41:41 2017 +0800

    Correct test_list_servers_filtered_by_ip for bug 1668828"

    test_list_servers_filtered_by_ip will fail occasionally, because
    now on Nova side filter by ip is always "regexp match",
    https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/schemas/servers.py#L355
    so if server1's ip is "10.1.1.1" and servers2's ip is "10.1.1.10",
    filter by server1's ip "10.1.1.1" will get both servers in the list.

    This is to use the longest ip for the filter, i.e., if server1's
    ip is "10.1.1.1" and server2's ip is "10.1.1.10", then we should use
    "10.1.1.10" for the filter, so to ensure only one server is returned
    in the list.

    Change-Id: I87c325cb80a95861287c54fbd1b5718cfb9ef310
    Closes-Bug: #1668828

Changed in tempest:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (master)

Reviewed: https://review.openstack.org/439969
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=b6a99a390caf2c143f3211c40f2ac7f5d51c9c1d
Submitter: Jenkins
Branch: master

commit b6a99a390caf2c143f3211c40f2ac7f5d51c9c1d
Author: Matt Riedemann <email address hidden>
Date: Wed Mar 1 22:19:09 2017 -0500

    Add functional test for ip filtering with regex

    There was a bug in Tempest where it would filter the
    list of servers by fixed IP address and expected to
    only get back one server but sometimes got more than
    one. That was because the IP of one server was
    a prefix of the IP of another server, and because of
    the regex used in the _ip_filter method in the compute
    API it will return all matches.

    This is how the IP filtering has always worked, but
    it's not clear we had a solid test coverage of this
    scenario so a functional test was written to confirm
    the expected behavior and so that we don't regress it
    since it's not documented in the api-ref (yet).

    Change-Id: If0e6d7ea7ac61a8480bf7f0288b263902536dc59
    Related-Bug: #1668828

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/tempest 16.0.0

This issue was fixed in the openstack/tempest 16.0.0 release.

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.