port list / get_ports() fails when filtering and limiting at the same time

Bug #1826186 reported by Sebastian Lohff
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
neutron
Fix Released
Medium
Gabriele Cerami

Bug Description

When doing a openstack port list that filters for a fixed-ip/subnet and at the same time limits the amount of results neutron returns a 500 internal server error.

Example command: openstack port list --fixed-ip ip-address=192.0.2.23
Limits should be applied automatically with a recent version of the openstacksdk with pagination turned on by default. Additionally, I attached a testcase that triggers this bug. This bug was found on neutron-queens, but the test-case also breaks current master (tested on commit id 1214e59cc2d818f6fde9c3e24c7f26c50d2a8a74).

It looks like _get_ports_query() gets a query with pre-applied limits by calling model_query.get_collection_query() and then tries to filter the results, which triggers a sqlalchemy assertion that disallows filtering after a limit has been applied.

The corresponding exception neutron exception would be the following:
InvalidRequestError: Query.filter() being called on a Query which already has LIMIT or OFFSET applied. To modify the row-limited results of a Query, call from_self() first. Otherwise, call filter() before limit() or offset() are applied.
  File "pecan/core.py", line 683, in __call__
    self.invoke_controller(controller, args, kwargs, state)
  File "pecan/core.py", line 574, in invoke_controller
    result = controller(*args, **kwargs)
  File "neutron/db/api.py", line 91, in wrapped
    setattr(e, '_RETRY_EXCEEDED', True)
  File "oslo_utils/excutils.py", line 220, in __exit__
    self.force_reraise()
  File "oslo_utils/excutils.py", line 196, in force_reraise
    six.reraise(self.type_, self.value, self.tb)
  File "neutron/db/api.py", line 87, in wrapped
    return f(*args, **kwargs)
  File "oslo_db/api.py", line 147, in wrapper
    ectxt.value = e.inner_exc
  File "oslo_utils/excutils.py", line 220, in __exit__
    self.force_reraise()
  File "oslo_utils/excutils.py", line 196, in force_reraise
    six.reraise(self.type_, self.value, self.tb)
  File "oslo_db/api.py", line 135, in wrapper
    return f(*args, **kwargs)
  File "neutron/db/api.py", line 126, in wrapped
    LOG.debug("Retry wrapper got retriable exception: %s", e)
  File "oslo_utils/excutils.py", line 220, in __exit__
    self.force_reraise()
  File "oslo_utils/excutils.py", line 196, in force_reraise
    six.reraise(self.type_, self.value, self.tb)
  File "neutron/db/api.py", line 122, in wrapped
    return f(*dup_args, **dup_kwargs)
  File "neutron/pecan_wsgi/controllers/utils.py", line 76, in wrapped
    return f(*args, **kwargs)
  File "neutron/pecan_wsgi/controllers/resource.py", line 131, in index
    return self.get(*args, **kwargs)
  File "neutron/pecan_wsgi/controllers/resource.py", line 141, in get
    **query_params)}
  File "neutron/db/api.py", line 161, in wrapped
    return method(*args, **kwargs)
  File "neutron/db/api.py", line 91, in wrapped
    setattr(e, '_RETRY_EXCEEDED', True)
  File "oslo_utils/excutils.py", line 220, in __exit__
    self.force_reraise()
  File "oslo_utils/excutils.py", line 196, in force_reraise
    six.reraise(self.type_, self.value, self.tb)
  File "neutron/db/api.py", line 87, in wrapped
    return f(*args, **kwargs)
  File "oslo_db/api.py", line 147, in wrapper
    ectxt.value = e.inner_exc
  File "oslo_utils/excutils.py", line 220, in __exit__
    self.force_reraise()
  File "oslo_utils/excutils.py", line 196, in force_reraise
    six.reraise(self.type_, self.value, self.tb)
  File "oslo_db/api.py", line 135, in wrapper
    return f(*args, **kwargs)
  File "neutron/db/api.py", line 126, in wrapped
    LOG.debug("Retry wrapper got retriable exception: %s", e)
  File "oslo_utils/excutils.py", line 220, in __exit__
    self.force_reraise()
  File "oslo_utils/excutils.py", line 196, in force_reraise
    six.reraise(self.type_, self.value, self.tb)
  File "neutron/db/api.py", line 122, in wrapped
    return f(*dup_args, **dup_kwargs)
  File "neutron/db/db_base_plugin_v2.py", line 1417, in get_ports
    page_reverse=page_reverse)
  File "neutron/plugins/ml2/plugin.py", line 1941, in _get_ports_query
    query = query.filter(substr_filter)
  File "<string>", line 2, in filter
  File "sqlalchemy/orm/base.py", line 200, in generate
    assertion(self, fn.__name__)
  File "sqlalchemy/orm/query.py", line 435, in _no_limit_offset
    % (meth, meth)

Revision history for this message
Sebastian Lohff (sebageek) wrote :
Boden R (boden)
Changed in neutron:
status: New → Confirmed
Revision history for this message
Gabriele Cerami (gcerami) wrote :

Newbie here. Expect slow resolution. If you think this bug is too urgent to be assigned to me, please tell me.

Changed in neutron:
assignee: nobody → Gabriele Cerami (gcerami)
Revision history for this message
Slawek Kaplonski (slaweq) wrote :

@Gabriele: thx for taking care of it. If You will have any questions, You can ping me on irc. My nick is "slaweq".

Changed in neutron:
importance: Undecided → Medium
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/655664

Changed in neutron:
status: Confirmed → In Progress
Revision history for this message
Gabriele Cerami (gcerami) wrote :

Suggestion to use from_self() was interesting, but it seemed wrong to apply filter after limiting results. If I limit to 5 usable records out of 10 total, and following filters remove all 5 records, user may be led to believe there are no ports with the filter specified.

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

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

commit cf8f3326be14d30456b20e4d1a55a36762daa550
Author: Gabriele Cerami <email address hidden>
Date: Thu Apr 25 11:28:17 2019 +0100

    Get ports query: extract limit and use it only at the end.

    _get_ports_query add filters after a get_collection. If limits are
    passed to applied to the query, then no additional filter is allowed.
    This patch extracts an eventual limit argument, to apply it only after
    the additional filters.

    Change-Id: I83394394860d10e27379efe0356d0fa9c567140e
    Closes-Bug: #1826186

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

Fix proposed to branch: stable/stein
Review: https://review.opendev.org/656063

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

Fix proposed to branch: stable/rocky
Review: https://review.opendev.org/656064

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

Fix proposed to branch: stable/queens
Review: https://review.opendev.org/656066

Revision history for this message
Sebastian Lohff (sebageek) wrote :

Thanks for taking care of this! :)

While running tempest against an installation I came across a very similar looking case of this, which also seems to be present in current master. This would be in neutron/plugins/ml2/plugin.py in _get_ports_query() where filter is again called onto the result of an already limited query.

See https://github.com/openstack/neutron/blob/6f4962dcf89aebf2552ee8ec0993c6389a953024/neutron/plugins/ml2/plugin.py#L2206

InvalidRequestError: Query.filter() being called on a Query which already has LIMIT or OFFSET applied. To modify the row-limited results of a Query, call from_self() first. Otherwise, call filter() before limit() or offset() are applied.
  File "pecan/core.py", line 683, in __call__
    self.invoke_controller(controller, args, kwargs, state)
[...]
  File "neutron/db/db_base_plugin_v2.py", line 1417, in get_ports
    page_reverse=page_reverse)
  File "neutron/plugins/ml2/plugin.py", line 1941, in _get_ports_query
    query = query.filter(substr_filter)
  File "<string>", line 2, in filter
  File "sqlalchemy/orm/base.py", line 200, in generate
    assertion(self, fn.__name__)
  File "sqlalchemy/orm/query.py", line 435, in _no_limit_offset
    % (meth, meth)

When doing a grep for _get_ports_query() in the neutron codebase I find a function with this name being called in neutron/db/dvr_mac_db.py in get_ports_on_host_by_subnet(), I do not have a stacktrace for that though.

See https://github.com/openstack/neutron/blob/6f4962dcf89aebf2552ee8ec0993c6389a953024/neutron/db/dvr_mac_db.py#L162

Would this be something that could be also included in the already proposed fix or would this be something for a separate bug report?

Revision history for this message
Nate Johnston (nate-johnston) wrote :

Sebastian, given that the current bug has already landed in master, just from a process perspective it might be easier to get attention on your issue if you file a new bug.

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

Reviewed: https://review.opendev.org/656063
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=fe09b40d57d239aa4a54b6660fc28c940a3306c5
Submitter: Zuul
Branch: stable/stein

commit fe09b40d57d239aa4a54b6660fc28c940a3306c5
Author: Gabriele Cerami <email address hidden>
Date: Thu Apr 25 11:28:17 2019 +0100

    Get ports query: extract limit and use it only at the end.

    _get_ports_query add filters after a get_collection. If limits are
    passed to applied to the query, then no additional filter is allowed.
    This patch extracts an eventual limit argument, to apply it only after
    the additional filters.

    Change-Id: I83394394860d10e27379efe0356d0fa9c567140e
    Closes-Bug: #1826186
    (cherry picked from commit cf8f3326be14d30456b20e4d1a55a36762daa550)

tags: added: in-stable-stein
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (stable/queens)

Reviewed: https://review.opendev.org/656066
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=2c6f7339330ceeea30b9cb673de2c8b61d8529af
Submitter: Zuul
Branch: stable/queens

commit 2c6f7339330ceeea30b9cb673de2c8b61d8529af
Author: Gabriele Cerami <email address hidden>
Date: Thu Apr 25 11:28:17 2019 +0100

    Get ports query: extract limit and use it only at the end.

    _get_ports_query add filters after a get_collection. If limits are
    passed to applied to the query, then no additional filter is allowed.
    This patch extracts an eventual limit argument, to apply it only after
    the additional filters.

    Change-Id: I83394394860d10e27379efe0356d0fa9c567140e
    Closes-Bug: #1826186
    (cherry picked from commit cf8f3326be14d30456b20e4d1a55a36762daa550)

tags: added: in-stable-queens
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (stable/rocky)

Reviewed: https://review.opendev.org/656064
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=d9ff4aee2034d676af06cc7bceb841bae3465bef
Submitter: Zuul
Branch: stable/rocky

commit d9ff4aee2034d676af06cc7bceb841bae3465bef
Author: Gabriele Cerami <email address hidden>
Date: Thu Apr 25 11:28:17 2019 +0100

    Get ports query: extract limit and use it only at the end.

    _get_ports_query add filters after a get_collection. If limits are
    passed to applied to the query, then no additional filter is allowed.
    This patch extracts an eventual limit argument, to apply it only after
    the additional filters.

    Change-Id: I83394394860d10e27379efe0356d0fa9c567140e
    Closes-Bug: #1826186
    (cherry picked from commit cf8f3326be14d30456b20e4d1a55a36762daa550)

tags: added: in-stable-rocky
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/neutron 13.0.4

This issue was fixed in the openstack/neutron 13.0.4 release.

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

This issue was fixed in the openstack/neutron 14.0.2 release.

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

This issue was fixed in the openstack/neutron 12.1.0 release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/neutron 15.0.0.0b1

This issue was fixed in the openstack/neutron 15.0.0.0b1 development milestone.

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.