rejecting move operations with qos ports does not work if the operation is called with non admin user

Bug #1850280 reported by Balazs Gibizer
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Fix Released
Medium
Balazs Gibizer
Stein
Fix Committed
Medium
Balazs Gibizer
Train
Fix Committed
Medium
Balazs Gibizer

Bug Description

At the start of the move operation the nova-api checks that the server has ports with resource request as some of the move operations are not supported for such servers yet. Unfortunately if move the operation is called by a non-admin user (either it is a resize, or another move operation with explicit policy change) then nova uses the non-admin token to query neutron. If the neutron port is queried with a non admin token then neutron does not return the resource_request to nova in the port response. Therefore nova thinks that the port ha no resource request and allows the operation.

Reproduce in Ussuri
===================

* Boot a server with qos port.
* Change the nova policy to allow evacuate to be called by the owner of the server "os_compute_api:os-evacuate": "rule:admin_or_owner"
* stop the nova-compute service on the host where the server currently running and wait until the controller decides that the compute is done
* with the non-admin owner initiate the evacuate of the server

Expected:
* evacuate rejected

Actual:
* evacuate accepted (and later fail due to missing implementation)

Triage
======

Due to [1] not using an admin client nova does not get the resource requests of the attached ports.

Affected versions and operations
================================
The resize, migrate, live migrate, evacuate, unshelve opertaions are affected on master, Train, Stein.

[1] https://github.com/openstack/nova/blob/9742a64403c0a0ae5e0b37df5b0bf3ba14ac4626/nova/api/openstack/common.py#L576

Tags: neutron
Changed in nova:
assignee: nobody → Balazs Gibizer (balazs-gibizer)
status: New → Triaged
importance: Undecided → Medium
tags: added: neutron
description: updated
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (master)

Fix proposed to branch: master
Review: https://review.opendev.org/691900

Changed in nova:
status: Triaged → In Progress
Changed in nova:
assignee: Balazs Gibizer (balazs-gibizer) → Matt Riedemann (mriedem)
Matt Riedemann (mriedem)
Changed in nova:
assignee: Matt Riedemann (mriedem) → Balazs Gibizer (balazs-gibizer)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (master)

Reviewed: https://review.opendev.org/691900
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=899976960503524b8e5c6588e339742ca4bf8158
Submitter: Zuul
Branch: master

commit 899976960503524b8e5c6588e339742ca4bf8158
Author: Balazs Gibizer <email address hidden>
Date: Tue Oct 29 16:43:04 2019 +0100

    Use admin neutron client to see if instance has qos ports

    The nova-api checks at each move* operation if the instance has qos port
    attached as not all the move operations are supported for such servers.
    Nova uses the request context to initialize the neutron client for the
    port query. However neutron does not return the value of the
    resource_request of the port if it is queried with a non admin client.
    This causes that if the move operation is initiated by a non admin
    then nova thinks that the ports do not have resource request.

    This patch creates an admin context for this neutron query.

    The new functional tests are not added before this patch in a regression
    test like way as existing functional tests are reused with different
    setup and doing that without the fix causes a lot of different failure
    scenarios.

    Note that neutron fixture is changed to simulate the different behavior
    in case of different request context are used to initialize the client.

    *: Note that Id5f2f4f22b856c989e2eef8ed56b9829d1bcefb6 removed the check
       for evacuate in Ussuri but exists in Train and Stein.

    Change-Id: I3cf6eb4654663865d9258c38f05cd05974ffcf9d
    Closes-Bug: #1850280

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

Fix proposed to branch: stable/train
Review: https://review.opendev.org/694018

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

Reviewed: https://review.opendev.org/694018
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=4cb493fe4f79e736ae4fd7874b02d1d5ceed02c1
Submitter: Zuul
Branch: stable/train

commit 4cb493fe4f79e736ae4fd7874b02d1d5ceed02c1
Author: Balazs Gibizer <email address hidden>
Date: Tue Oct 29 16:43:04 2019 +0100

    Use admin neutron client to see if instance has qos ports

    The nova-api checks at each move* operation if the instance has qos port
    attached as not all the move operations are supported for such servers.
    Nova uses the request context to initialize the neutron client for the
    port query. However neutron does not return the value of the
    resource_request of the port if it is queried with a non admin client.
    This causes that if the move operation is initiated by a non admin
    then nova thinks that the ports do not have resource request.

    This patch creates an admin context for this neutron query.

    The new functional tests are not added before this patch in a regression
    test like way as existing functional tests are reused with different
    setup and doing that without the fix causes a lot of different failure
    scenarios.

    Note that neutron fixture is changed to simulate the different behavior
    in case of different request context are used to initialize the client.

    *: Note that Id5f2f4f22b856c989e2eef8ed56b9829d1bcefb6 removed the check
       for evacuate in Ussuri but exists in Train and Stein.

    Conflicts:
          nova/tests/fixtures.py
    Due to d2d4317e1a31f6ac40c5f29b57cd4ea196dc8780 is missing from
    stable/train

    Another difference from the original patch is in
    nova/api/openstack/compute/evacuate.py due to
    Id5f2f4f22b856c989e2eef8ed56b9829d1bcefb6 is only merged in Ussuri.

    Change-Id: I3cf6eb4654663865d9258c38f05cd05974ffcf9d
    Closes-Bug: #1850280
    (cherry picked from commit 899976960503524b8e5c6588e339742ca4bf8158)

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

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

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

Reviewed: https://review.opendev.org/694668
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=65ada9745cb01c9004eb4f6e7721a64b3a6e3ca5
Submitter: Zuul
Branch: stable/stein

commit 65ada9745cb01c9004eb4f6e7721a64b3a6e3ca5
Author: Balazs Gibizer <email address hidden>
Date: Tue Oct 29 16:43:04 2019 +0100

    Use admin neutron client to see if instance has qos ports

    The nova-api checks at each move* operation if the instance has qos port
    attached as not all the move operations are supported for such servers.
    Nova uses the request context to initialize the neutron client for the
    port query. However neutron does not return the value of the
    resource_request of the port if it is queried with a non admin client.
    This causes that if the move operation is initiated by a non admin
    then nova thinks that the ports do not have resource request.

    This patch creates an admin context for this neutron query.

    The new functional tests are not added before this patch in a regression
    test like way as existing functional tests are reused with different
    setup and doing that without the fix causes a lot of different failure
    scenarios.

    Note that neutron fixture is changed to simulate the different behavior
    in case of different request context are used to initialize the client.

    *: Note that Id5f2f4f22b856c989e2eef8ed56b9829d1bcefb6 removed the check
       for evacuate in Ussuri but exists in Train and Stein.

    Conflicts:
      nova/api/openstack/common.py due to
      I6fc3cb0651f65b2448fbbb58989c920974f076c3 is missing

      nova/api/openstack/compute/migrate_server.py
      nova/tests/unit/api/openstack/compute/test_migrate_server.py
      due to I09cac780b9ee5b5726874d4e6f895fd0cd4bff8c is missing

      nova/api/openstack/compute/servers.py
      due to Ib1a73f5e20b6f9a325d8b24d9253a18f2a46db1f and
      Id0ee10e8d323786f4d79c82e3f05b48e76bd0956 is missing

      nova/tests/unit/api/openstack/compute/test_server_actions.py
      due to Ib1a73f5e20b6f9a325d8b24d9253a18f2a46db1f is missing

      nova/api/openstack/compute/shelve.py due to
      I6381365ff882cf23808e8dabfce41143c5e35192 is missing

    Also integrated_helpers.py and test_servers.py needed to be changed due
    to Id0ee10e8d323786f4d79c82e3f05b48e76bd0956 is missing. As well as
    NonAdminUnsupportedPortResourceRequestBasedSchedulingTest.setUp() needed
    to be extended to allow calling migrate and resize for the non admin api
    client.

    Change-Id: I3cf6eb4654663865d9258c38f05cd05974ffcf9d
    Closes-Bug: #1850280
    (cherry picked from commit 899976960503524b8e5c6588e339742ca4bf8158)
    (cherry picked from commit 4cb493fe4f79e736ae4fd7874b02d1d5ceed02c1)

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

This issue was fixed in the openstack/nova 20.1.0 release.

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

This issue was fixed in the openstack/nova 19.1.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.