placement inadvertently imports many python modules it does not need
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
| OpenStack Compute (nova) |
Low
|
Chris Dent |
Bug Description
The placement WSGI app has its own WSGI framework and is the only script/binary that placement uses. From the outset, it has been intended that a deployment could run multiple placement servers, distributed across multiple hosts/pods/
In that model, it would be ideal for the app to require as few python module as possible, and have as small a footprint, at least starting up, as possible.
Currently, that's not the case, for two reasons:
* Placement makes use of the FaultWrapper WSGI middleware to catch unexpected exceptions and turn them into status 500 HTTP responses. FaultWrapper imports nova.utils and nova.utils imports a lot of stuff.
* Placement is within the nova/api/openstack package hierarchy and nova/api/
These problems can be addressed:
* FaultWrapper is overkill for the placement WSGI app. FaultWrapper is capable of decoding a variety of exceptions into a variety of status responses. This is not required by placement. The only exceptions that can make it to FaultWrapper in placement are those that would result in a 500. A simpler middleware can handle this.
* nova/api/
Note that this is not the only case of inadvertent imports, but is one of the main vectors. Others will be identified and additional bugs created.
Changed in nova: | |
status: | Triaged → In Progress |
Chris Dent (cdent) wrote : | #2 |
This going to be fairly complicated to make much headway on, unless the resource_provider objects are moved out from under nova/objects. __init__.py there registers all those objects, each of which imports things from all over the nova hierarchy. One notable example are the ec2 objects which import nova/api/
Also, nova/__init__.py imports oslo_service and eventlet, neither of which placement wants to see. This could perhaps be moved back to nova/cmd and wherever else is actually required.
Chris Dent (cdent) wrote : | #3 |
I messed up the bug reference, so there's also:
https:/
Reviewed: https:/
Committed: https:/
Submitter: Zuul
Branch: master
commit 5f881ff030c119b
Author: Chris Dent <email address hidden>
Date: Sat Jan 13 16:11:51 2018 +0000
[placement] use simple FaultWrapper
Prior to this patch placement was using the nova FaultWrapper
middleware as a final guard against unexpected exceptions. That
middleware does more than required and also imports stuff that
placement does not otherwise require.
The new middleware is only for exceptions that have fallen through
previous handling and need to be turned into 500 responses, formatted
to JSON, and logged.
While doing this it became clear that the limited Exception catching
in handler.py was redundant if FaultWrapper and the Microversion
middlewares (both required for the operation of the system) are
involved, so that has been removed, with a comment.
This should not require a microversion as the structure of successful
responses and expected error responses does not change. What does
change is that a 500 response will be formatted more as the api-wg
errors guideline[1] desires.
[1] http://
Change-Id: Ib2c3250b964a30
Partial-Bug: #1743120
OpenStack Infra (hudson-openstack) wrote : | #5 |
Reviewed: https:/
Committed: https:/
Submitter: Zuul
Branch: master
commit ef6f4e4c8ec82e2
Author: Chris Dent <email address hidden>
Date: Mon Jan 15 20:58:48 2018 +0000
Refactor WSGI apps and utils to limit imports
The file nova/api/
modules, notably nova.utils. This means that any code which
runs within that package, notably the placement service, imports
all those modules, even if it is not going to use them. This
results in scripts/binaries that are heavier than they need
to be and in some cases including modules, like eventlet, that
it would feel safe to not have in the stack.
Unfortunately we cannot sinply rename nova/api/
to another name because it contains FaultWrapper and FaultWrapper
is referred to, by package path, from the paste.ini file and that
file is out there in config land, and something we prefer not to
change. Therefore alternate methods of cleaning up were explored
and this has led to some useful changes:
Fault wrapper is the only consumer of walk_class_
there is no reason for it it to be in nova.utils.
nova.wsgi contains a mismash of WSGI middleware and applications,
which need only a small number of imports, and Server classes
which are more complex and not required by the WSGI wares.
Therefore nova.wsgi was split into nova.wsgi and nova.api.wsgi.
The name choices may not be ideal, but they were chosen to limit
the cascades of changes that are needed across code and tests.
Where utils.utf8 was used it has been replaced with the similar (but not
exactly equivalient) method from oslo_utils.
Change-Id: I297f30aa6eb01f
Partial-Bug: #1743120
Chris Dent (cdent) wrote : | #6 |
We can call this done now that openstack/placement exists
Changed in nova: | |
status: | In Progress → Fix Committed |
Fix proposed to branch: master /review. openstack. org/533752
Review: https:/