Nova test_report_client uses nova conf when starting placement intercept, causing missing config opts

Bug #1818560 reported by Chris Dent
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Fix Released
Critical
Chris Dent

Bug Description

See: http://logs.openstack.org/98/538498/22/gate/nova-tox-functional-py35/7673d3e/testr_results.html.gz

The failing tests there are failing because the Database fixture from placement is used directly, and configuration opts are not being registered properly. This was an oversight when adding a new configuration setting.

The fix is to register the missing opt when requested to do so.

This is blocking the gate.

LATER, to clarify:

The root cause of this is that in test_report_client, a global CONF from nova was being used to create the placement wsgi-intercepts. When a new config was added on the placement side, that global CONF was no longer in sync.

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

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

Changed in nova:
assignee: nobody → Chris Dent (cdent)
status: Confirmed → In Progress
Revision history for this message
Matt Riedemann (mriedem) wrote : Re: Nova's use of the placement database fixture from test_report_client doesn't register opts
Revision history for this message
Chris Dent (cdent) wrote :

No, https://review.openstack.org/#/c/619050/ found the bug :). The fact that we were using global conf from nova in the placement wsgi interceptor was something we should have noticed sooner.

summary: - Nova's use of the placement database fixture from test_report_client
- doesn't register opts
+ Nova test_report_client uses nova conf when starting placement
+ intercept, causing missing config opts
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.openstack.org/640887

Changed in nova:
assignee: Chris Dent (cdent) → Eric Fried (efried)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (master)

Reviewed: https://review.openstack.org/640853
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=09090c8277284848007a8de1187b5cdc68c37d09
Submitter: Zuul
Branch: master

commit 09090c8277284848007a8de1187b5cdc68c37d09
Author: Chris Dent <email address hidden>
Date: Mon Mar 4 20:08:28 2019 +0000

    Use a placement conf when testing report client

    It turns out that the independent wsgi interceptors in
    test_report_client were using nova's global configuration
    when creating the intercepts using code from placement.
    This was working because until [1] placement's set of conf
    options had not diverged from nova's and nova still has
    placement_database config settings.

    This change takes advantage of new functionality in the
    PlacementFixture to allow the fixtur to manage config and
    database, but _not_ run the interceptor. This means it
    can set up a config that is later used by the independent
    interceptors that are used in the report client tests.

    [1] Ie43a69be8b75250d9deca6a911eda7b722ef8648

    Change-Id: I05326e0f917ca1b9a6ef8d3bd463f68bd00e217e
    Closes-Bug: #1818560
    Depends-On: I8c36f35dbe85b0c0db1a5b6b5389b160b68ca488

Changed in nova:
status: In Progress → Fix Released
Matt Riedemann (mriedem)
Changed in nova:
assignee: Eric Fried (efried) → Chris Dent (cdent)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/nova 19.0.0.0rc1

This issue was fixed in the openstack/nova 19.0.0.0rc1 release candidate.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (master)

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

commit f2d088b04e6e6896f0df04f03e92b32a539b6917
Author: Eric Fried <email address hidden>
Date: Mon Mar 4 16:08:39 2019 -0600

    Stop using PlacementDirect

    PlacementDirect was integrated into a functional test suite when it was
    first created as a way to prove that it worked [1] and demonstrate how
    to use it.

    However, it was a pain then, because the interceptor needs to be created
    every time you want to use it; and since extracted placement started
    diverging from in-tree placement, other problems started cropping up
    (see the associated bug).

    So this commit removes the use of PlacementDirect from nova. Details:

    - test_report_client now uses PlacementFixture. So all the `with
      interceptor` context management is gone. This accounts for the vast
      majority of the apparent change, which is just outdenting those
      contexts.
    - SchedulerReportClientTestBase, which was doing some hocus pocus to
      wrap the SchedulerReportClient such that we could do some microversion
      checks, is removed. The test suite simply instantiates the
      microversion-checking wrapper class directly as the client used by the
      test cases.
    - We were taking advantage of a PlacementDirect feature allowing us to
      default to the latest microversion if not explicitly specified in the
      request. Without this, we had to add the `version` kwarg to some of
      the calls we were making to SchedulerReportClient primitives
      (get/put/post/delete).
    - A piece of test_update_from_provider_tree was using a
      deliberately-broken interceptor to prove that the code in question
      wasn't hitting the API. We replace this with a non-callable mock on
      the Adapter's request method.
    - test_global_request_id was taking advantage of the interceptor to
      validate that the global request ID was making it to the "other side"
      of the API boundary. This was fun, but overkill. We now simply assert
      that the correct HTTP header is making it into the ksa Adapter's
      request method.
    - Functional test suite test_resource_tracker.IronicResourceTrackerTest
      was inheriting from the SchedulerReportClientTestBase class, but not
      using the interceptor anywhere. Can't tell you why that was done. So
      now it just uses the plain old test.TestCase like everyone else.

    [1] This commit does remove all of nova's testing of PlacementDirect.
    However, it is still tested in the placement repository itself:
    https://github.com/openstack/placement/blob/69b9659a457b6d715ed47e6bd6d3b923d548c620/placement/tests/functional/test_direct.py

    Change-Id: Icb889c09a69e7c5cbf9330e5d9917d6ab3ac3dc5
    Related-Bug: #1818560

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.