This change add a global greenpool which is used to manage
the greenthreads created via nova.utils.spawn(_n).
A test fixture is also added to use an isolated greenpool
which will raise an exception if a greenthread is leaked.
the fixture will optionally raise if greenlets are leaked.
This is enabled by unit test by default and is configurable
for functional tests.
This change removes all greenthread leaks from the unit
and functional tests that were detected. 7 functional
tests still leak greenlets but they have no obvious
cause. as such greenlet leaks are not treated as errors
for funtional tests by default. Greenthread leaks
are always treated as errors.
Set NOVA_RAISE_ON_GREENLET_LEAK=1|true|yes when invoking
tox to make greenlet leaks an error for functional tests.
Change-Id: I73b4684744b340bfb80da08537a745167ddea106
The nova.utils.spawn and spawn_n methods transport
the context (and profiling information) to the
newly created threads. But the same isn't done
when submitting work to thread-pools in the
ComputeManager.
The code doing that for spawn and spawn_n
is extracted to a new function
and called to submit the work to the thread-pools.
Closes-Bug: #1962574
Change-Id: I9085deaa8cf0b167d87db68e4afc4a463c00569c
Currently nova.utils.tpool_execute() only works by chance. And as the
bug report shows there are env where it fails.
The nova.utils.tpool_execute() call tries to uses eventlet.tpool.execute
but the tpool module is not imported by the utils module only eventlet.
In devstack it works by chance as the wsgi init actually imports
eventlet.tpool indirectly via:
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/opt/stack/nova/nova/api/openstack/compute/__init__.py", line 21, in <module>
from nova.api.openstack.compute.routes import APIRouterV21 # noqa
File "/opt/stack/nova/nova/api/openstack/compute/routes.py", line 20, in <module>
from nova.api.openstack.compute import admin_actions
File "/opt/stack/nova/nova/api/openstack/compute/admin_actions.py", line 17, in <module>
from nova.api.openstack import common
File "/opt/stack/nova/nova/api/openstack/common.py", line 27, in <module>
from nova.compute import task_states
File "/opt/stack/nova/nova/compute/task_states.py", line 26, in <module>
from nova.objects import fields
File "/opt/stack/nova/nova/objects/fields.py", line 24, in <module>
from nova.network import model as network_model
File "/opt/stack/nova/nova/network/model.py", line 23, in <module>
from nova import utils
File "/opt/stack/nova/nova/utils.py", line 39, in <module>
from oslo_concurrency import processutils
File "/usr/local/lib/python3.8/dist-packages/oslo_concurrency/processutils.py", line 57, in <module>
from eventlet import tpool
This was broken since I8dbc579e0037969aab4f2bb500fccfbde4190726. This
patch adds the correct import statement.
Change-Id: Ic46345ceeb445164aea6ae9b35c457c6150765f6
Closes-Bug: #1915400
The commit I168fffac8002f274a905cfd53ac4f6c9abe18803 added a wrapper
around fasteners.ReaderWriterLock to fix up an issue with eventlet. But
the wrapper was added to nova.utils module that is use not only by the
nova tests but also the nova production code. This made the fixture
library a dependency of the nova production code. While the current
ReaderWriterLock usage only limited to the nova test sub tree. The
I712f88fc1b6053fe6d1f13e708f3bd8874452a8f commit fix the issue of not
having fixtures in the nova requirements.txt. However I think a better
fix is to move the wrapper to the test subtree instead. This patch does
that and restores the state of the requirements.txt
Change-Id: I6903ce53b9b91325f7268cf2ebd02e4488579560
Related-Bug: #1958075
This is a follow up change to I168fffac8002f274a905cfd53ac4f6c9abe18803
which added a hackaround to enable our tests to pass with
fasteners>=0.15 which was upgraded recently as part of a
openstack/requirements update.
The ReaderWriterLock from fasteners (and thus lockutils) cannot work
correctly with eventlet patched code, so this adds a wrapper containing
the aforementioned hackaround along with a hacking check to do our best
to ensure that future use of ReaderWriterLock will be through the
wrapper.
Change-Id: Ia7bcb40a21a804c7bc6b74f501d95ce2a88b09b5
autopep8 is a code formating tool that makes python code pep8
compliant without changing everything. Unlike black it will
not radically change all code and the primary change to the
existing codebase is adding a new line after class level doc strings.
This change adds a new tox autopep8 env to manually run it on your
code before you submit a patch, it also adds autopep8 to pre-commit
so if you use pre-commit it will do it for you automatically.
This change runs autopep8 in diff mode with --exit-code in the pep8
tox env so it will fail if autopep8 would modify your code if run
in in-place mode. This allows use to gate on autopep8 not modifying
patches that are submited. This will ensure authorship of patches is
maintianed.
The intent of this change is to save the large amount of time we spend
on ensuring style guidlines are followed automatically to make it
simpler for both new and old contibutors to work on nova and save
time and effort for all involved.
Change-Id: Idd618d634cc70ae8d58fab32f322e75bfabefb9d
When creating an instance, nova sanitizes the 'instance.name' attribute,
using the 'nova.utils.sanitize_hostname' function, and stores this as
the 'instance.hostname' attribute. If neutron has the DNS extension
enabled then this attribute will be reported as the 'dns_name' value
when attaching ports [1].
Nova does not currently remove or replace periods in the name as part of
this sanitization. This results in the hostname being identified as a
(FQDN) fully qualified domain name, with all the constraints placed on
FQDNs [2]. This can be problematic for instances with common names such
as 'ubuntu18.04' or 'test.a', which aren't valid since TLDs must start
with an alpha character and must be greater than 1 character long,
respectively. Attempting to boot instances with such names can result in
failures like the below when DNS integration is enabled in neutron:
RescheduledException: Build of instance foo was re-scheduled:
Invalid input for dns_name.
Reason: 'test-ubuntu-20.04' not a valid PQDN or FQDN.
Reason: TLD '04' must not be all numeric.
Start replacing these periods with hyphens. This is a change in behavior
and may affect users who are relying on this inadvertent support for
instance names as FQDNs when using cloud-init, however, a quick poll on
openstack-discuss [3] suggests the people that are using FQDN-like
instance names are setting hostnames in the guest explicitly and not
relying on the information provided via the nova metadata service,
meaning this will not affect them.
[1] https://docs.openstack.org/neutron/victoria/admin/config-dns-int.html
[2] https://stackoverflow.com/a/53875771/613428
[3] http://lists.openstack.org/pipermail/openstack-discuss/2020-November/019113.html
Change-Id: I2fac7f919e9ddd6b0925d3e5d04e61b2ba1b8c82
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Closes-Bug: #1581977
We have discovered that if an exception is raised at any point during
the running of the init_application WSGI script in an apache/mod_wsgi
Daemon Mode environment, it will prompt apache/mod_wsgi to re-run the
script without starting a fresh python process. Because we initialize
global data structures during app init, subsequent runs of the script
blow up as some global data do *not* support re-initialization. It is
anyway not safe to assume that init of global data is safe to run
multiple times.
This mod_wsgi behavior appears to be a special situation that does not
behave the same as a normal reload in Daemon Mode as the script file is
being reloaded upon failure instead of the daemon process being
shutdown and restarted as described in the documentation [1].
In order to handle this situation, we can move the initialization of
global data structures to a helper method that is decorated to run only
once per python interpreter instance. This way, we will not attempt to
re-initialize global data that are not safe to init more than once.
Co-Authored-By: Michele Baldessari <michele@redhat.com>
Co-Authored-By: melanie witt <melwittt@gmail.com>
Closes-Bug: #1882094
[1] https://modwsgi.readthedocs.io/en/develop/user-guides/reloading-source-code.html#reloading-in-daemon-mode
Change-Id: I2bd360dcc6501feea7baf02d4510b282205fc061
Replace six.text_type with str.
A subsequent patch will replace other six.text_type.
Change-Id: I23bb9e539d08f5c6202909054c2dd49b6c7a7a0e
Implements: blueprint six-removal
Signed-off-by: Takashi Natsume <takanattie@gmail.com>
The service level check introduced in
Ie15ec8299ae52ae8f5334d591ed3944e9585cf71 should handle the case when a
compute service is wrongly configured with DB credentials. The previous
patch did not handle this and it caused a misleading error at compute
service startup. This patch makes sure that a user friendly warning is
logged in this case then the service level check is done ignoring the DB
configuration and only checking the local cell.
A subsequent patch will add a separate check that fails the compute
service startup in such invalid configuration.
Change-Id: I89cdf3852266ed93a2ac7cd6261fe269932026ac
Related-Bug: #1871482
Nova services only support computes that are not older than
the previous major release. This patch introduces a check in the
service startup that prevents staring the service if too old computes
are detected.
Change-Id: Ie15ec8299ae52ae8f5334d591ed3944e9585cf71
md5 is not an approved algorithm in FIPS mode, and trying to
instantiate a hashlib.md5() will fail when the system is running in
FIPS mode.
md5 is allowed when in a non-security context. There is a plan to
add a keyword parameter (usedforsecurity) to hashlib.md5() to annotate
whether or not the instance is being used in a security context.
In the case where it is not, the instantiation of md5 will be allowed.
See https://bugs.python.org/issue9216 for more details.
Some downstream python versions already support this parameter. To
support these versions, a new encapsulation of md5() has been added to
oslo_utils. See https://review.opendev.org/#/c/750031/
This patch is to replace the instances of hashlib.md5() with this new
encapsulation, adding an annotation indicating whether the usage is
a security context or not.
The instances being replaced here appear to be used to provide
representations for paths. There is in fact already a sha256 version
of get_hash_str that is supposed to be used in security sensitive
usages.
With this change (and the related dependent changes), the unit and
functional tests pass when run on a FIPS enabled system.
Change-Id: If0ec11e7b7fcde4dacc57265c4dd77b0f536bfab
Depends-On: https://review.opendev.org/#/c/756432
Depends-On: https://review.opendev.org/#/c/756153
Depends-On: https://review.opendev.org/#/c/760160
We're going to gradually introduce support for the various instance
operations when using vTPM due to the complications of having to worry
about the state of the vTPM device on the host. Add in API checks to
reject all manner of requests until we get to include support for each
one. With this change, the upcoming patch to turn everything on will
allow a user to create, delete and reboot an instance with vTPM, while
evacuate, rebuild, cold migration, live migration, resize, rescue and
shelve will not be supported immediately.
While we're here, we rename two unit test files so that their names
match the files they are testing and one doesn't have to spend time
finding them.
Change-Id: I3862a06ca28b383d525bcc9dcbc6fb1d4062f193
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Remove six.PY2 and six.PY3.
Subsequent patches will replace other six usages.
Change-Id: Iccce0ab50eee515e533ab36c8e7adc10cb3f7019
Implements: blueprint six-removal
Signed-off-by: Takashi Natsume <takanattie@gmail.com>
The 'nova.block_device' module is essentially a catchall utils-like
module for all things BDM. The 'get_bdm_image_metadata' module, and
closely related 'get_image_metadata_from_volume' both fall into the
category of functions that belong here so move them. This allows us to
clean up tests and, crucially, avoid a circular reference seen when we
want to use proper type hints in the 'nova.virt.driver' module.
nova.context imports...
nova.utils, which imports...
nova.block_device, which imports...
nova.virt.driver, which tries to import...
nova.context, causing a circular dependency
Change-Id: I48177d6e93f2ff132d26b53cd682fd24a43a4b31
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Execute glance upload in a native thread as it may block the current
coroutine until it completes.
Despite the fact we use eventlet monkey_patching [1] to achieve cooperative
yielding for network IO, file IO on busy file system may still get
nova-compute hanging.
Stick those IO in a native thread using eventlet tpool.execute() [2]
avoid this issue.
[1] https://eventlet.net/doc/patching.html
[2] https://eventlet.net/doc/threading.html
Closes-Bug: #1874032
Change-Id: I8dbc579e0037969aab4f2bb500fccfbde4190726
There are only a few of these remaining in the code base. Remove them.
Change-Id: I33725e2439b0f39c1e9bec9e33a37bf3e24944fb
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
We've been slowly removing these as we go. Remove the final few '_LE'
occurrences now.
Change-Id: I75ebd2e95a0c77585d7b4329ca01e4bacc1dd7c4
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
This function extracts image metadata from a given block_device_mapping
and is extremely useful outside of the compute API. This change simply
extracts the function into nova.utils alongside
get_image_metadata_from_volume and other similar functions.
Change-Id: I8ba52f0cd877cefc1f7d3c10d8a07a2a1c21cb34
This should have been removed when we removed the placement code in
change I4181f39dea7eb10b84e6f5057938767b3e422aff.
Change-Id: If5808075d853341bf274f35b7fcf0e0712f8f77a
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
Finish the job by removing all the now-unused modules. This also allows
us to - wait for it - kill mox at long last. It's a great day in the
parish.
Partial-Implements: blueprint remove-nova-network-ussuri
Partial-Implements: blueprint mox-removal-ussuri
Change-Id: Ia33ec2604b2fc2d3b6830b596cac669cc3ad6c96
Strip out everything matching '(is|use)_neutron', except the tests for
nova-network code and two other places that these tests rely on. Along
the way, remove a whole load of apparently unnecessary mocking that
clearly wasn't caught when we switched over the bulk of testing to use
the neutron network driver.
Change-Id: Ifa9c5c468400261a5e1f66b72c575845173a4f8f
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
A possible fix [1] for bug #1709118 is in keystoneauth1 release 3.3.0.
This change removes a workaround for that bug in
nova.utils.get_endpoint.
[1] https://review.openstack.org/#/c/500956/
Change-Id: If2a56ed56ecb3402fdee1189df81f0b9fbc483b3
Related-Bug: #1709118
In version 0.35.0, openstacksdk added a strict_proxies kwarg to the
Connection constructor [1].
Without it, openstacksdk tries really hard to give us an Adapter, which
in the case of the service being down can mean we default to the catalog
endpoint without doing any discovery. This should usually work; but may
break in cases where the discovery document (at the catalog endpoint)
points to different URLs for versioned endpoints.
This commit adds a check_service bool kwarg to get_sdk_adapter which, if
True, uses strict_proxies to create the Connection, and causing
get_sdk_adapter to raise a ServiceUnavailable exception if the service
is down.
This can be used for services like Ironic, where we're set up to
tolerate connect failures on startup. But it should not be used for
services like Placement, where we expect getting the adapter to succeed,
and are instead tolerant of failures making the actual API calls.
[1] https://review.opendev.org/#/c/676837/
This dependency bumps the openstacksdk u-c in the requirements project.
Depends-On: https://review.opendev.org/678207
Change-Id: I86e038af8a96e113a754b2fdb3698acd3783c1c8
Previously openstacksdk would process the config for *every* known
service type, even though get_sdk_adapter only cares about one at a
time. Aside from just being wasteful, this also resulted in weird log
messages complaining about other conf sections that weren't set up
properly for sdk consumption (which we don't care about). And that would
happen every time we requested an sdk adapter.
openstacksdk added support for restricting a Connection to a single
service type via [1]. So this commit takes advantage of it.
[1] https://review.opendev.org/674675
Change-Id: Ib0e43f6c9e28ce4f1ac1831a3e3558c825a372ca
filter_and_format_resource_metadata seems to be the leftover in
30ce493952, so this is to clean it up.
Change-Id: I100f7a22d06e4aa7955ac9b5d58b191e1d2fa9db
Enables the use of the sdk instead of ksa adapter or python-*client.
It is provided by a get_sdk_adapter method which constructs an
authenticated SDK Connection object using provided service configuration.
This change should be transparent to operators of services which already
use ksa as get_sdk_adapter uses the same conf options from keystoneauth1.
Blueprint: openstacksdk-in-nova
Co-Authored-By: Dustin Cowles <dustin.cowles@intel.com>
Change-Id: I49f364e01e2a18de0c95674654fc72acea019e76
This one's actually important since it will be an error in future
versions of Python.
Change-Id: Ib9f735216773224f91ac7f49fbe2eee119670872
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
We were seeing infinite recursion opening an ssl socket when running
various combinations of python3, eventlet, and urllib3. It is not
clear exactly what combination of versions are affected, but for
background there is an example of this issue documented here:
https://github.com/eventlet/eventlet/issues/371
The immediate cause in nova's case was that we were calling
eventlet.monkey_patch() after importing urllib3. Specifically, change
Ie7bf5d012e2ccbcd63c262ddaf739782afcdaf56 introduced the
nova.utils.monkey_patch() method to make monkey patching common
between WSGI and non-WSGI services. Unfortunately, before executing
this method you must first import nova.utils, which imports a large
number of modules itself. Anything imported (transitively) by
nova.utils would therefore be imported before monkey patching, which
included urllib3. This triggers the infinite recursion problem
described above if you have an affected combination of library
versions.
While this specific issue may eventually be worked around or fixed in
eventlet or urllib3, it remains true that eventlet best practises are
to monkey patch as early as possible, which we were not doing. To
avoid this and hopefully future similar issues, this change ensures
that monkey patching happens as early as possible, and only a minimum
number of modules are imported first.
This change fixes monkey patching for both non-wsgi and wsgi callers:
* Non-WSGI services (nova/cmd)
This is fixed by using the new monkey_patch module, which has minimal
dependencies.
* WSGI services (nova/api/openstack)
This is fixed both by using the new monkey_patch module, and by moving
the patching point up one level so that it is done before importing
anything in nova/api/openstack/__init__.py.
This move causes issues for some external tools which load this path
from nova and now monkey patch where they previously did not. However,
it is unfortunately unavoidable to enable monkey patching for the wsgi
entry point without major restructuring. This change includes a
workaround for sphinx to avoid this issue.
This change has been through several iterations. I started with what
seemed like the simplest and most obvious change, and moved on as I
discovered more interactions which broke. It is clear that eventlet
monkey patching is extremely fragile, especially when done implicitly at
module load time as we do. I would advocate a code restructure to
improve this situation, but I think the time would be better spent
removing the eventlet dependency entirely.
Co-authored-by: Lee Yarwood <lyarwood@redhat.com>
Closes-Bug: #1808975
Closes-Bug: #1808951
Change-Id: Id46e76666b553a10ec4654d4418a9884975b5b95
Validate the combination of the flavor extra-specs and image properties
as early as possible once they're both known (since you can specify
mutually-incompatible changes in the two places). If validation failed
then synchronously return error to user. We need to do this anywhere
the flavor or image changes, so basically instance creation, rebuild,
and resize.
- Rename _check_requested_image() to _validate_flavor_image() and add
a call from the resize code path. (It's already called for create
and rebuild.)
- In _validate_flavor_image() add new checks to validate numa related
options from flavor and image including CPU policy, CPU thread
policy, CPU topology, memory topology, hugepages, CPU pinning,
serial ports, realtime mask, etc.
- Added new 400 exceptions in Server API correspondent to added
validations.
blueprint: flavor-extra-spec-image-property-validation
Change-Id: I06fad233006c7bab14749a51ffa226c3801f951b
Signed-off-by: Jack Ding <jack.ding@windriver.com>
Signed-off-by: Chris Friesen <chris.friesen@windriver.com>
(Well, except for starting privsep daemons, oh and of course
os-brick wants to know what our root helper is for some reason
that is a bit beyond me.)
Yes that's right, we now live in the future and no longer need
the run_as_root infrastructure in nova.utils. I'm sure this breaks
some out of tree drivers, but they've had several releases to
notice that we're moving in this direction and its pretty easy
for them to fix themselves these days.
Change-Id: I99c66558938db9beb0bda33d27a8e36b26b8fcac
With the extraction of placement we ended up with resource class names
being duplicated between nova and placement. To address that, the
os-resource-classes library [1] was created to provide a single
authority for standard resource classes and the format of custom
classes.
This patch changes nova to use it, removing the use of the rc_fields
module which used to have the information. A method left in it
(normalize_name) has been moved to utils.py, renamed as
normalize_rc_name, and callers and tests updated accordingly.
Because the placement code is being kept in nova for the time being,
that code's use of rc_fields is maintained, and the module too.
A note is added in the module explain that. Backporting the changes
from extracted-placement to placement-in-nova was considered but
because we no longer have placement tests in nova, that didn't seem
like the right thing to do.
requirements and lower-constraints have been updated.
os-resource-classes is already in global requirements.
For reference the related placement change is at [2].
[1] https://docs.openstack.org/os-resource-classes
[2] https://review.openstack.org/#/c/623556/
Change-Id: I8e579920c0eaca81b563a87429c930b21b3d4dc5
The first patch introduced os=False is
https://review.openstack.org/#/c/11556/
But there is no explaination why nova should not monkey_patch `os`
module. The args(os=False) make nova process has to wait while calling
os.(['fdopen', 'read', 'write', 'wait', 'waitpid', 'open'])
, which seems to be unnecessary.
For nova-compute running on Windows(Hyper-v), `os` module should not be
patched, because Windows pipes don't support non-blocking I/O. I add a
comment to note that.
[1] https://github.com/eventlet/eventlet/blob/v0.23.0/eventlet/green/os.py#L12
Change-Id: I8b456676a04b9066cb2b570060c0d95cd4fe69f8
When instances are deployed concurrently on multiple
compute nodes, nova-compute may fail on checking
direct io capabilities when instance_path is shared
across nodes. This fails on shared storage, that
doesn't support concurrent write access to the
same file, i.e. Virtuozzo Storage. To fix that we add
random string at the end of test file name.
Closes-Bug: #1727369
Change-Id: I085ade355fba0e9727a38d2dcbc9cffa735a62d1
Signed-off-by: Pavel Glushchak <pglushchak@virtuozzo.com>
- This change tries to address an edge case discovered
when running placement under mod_wsgi where
if the placement wsgi application is re-initialize the db_api
configure method attempts to reconfigure a started transaction
factory.
- This since oslo.db transaction factories do not support
reconfiguration at runtime this result in an exception being
raised preventing reloading of the Placement API without
restarting apache to force mod_wsgi to recreate the
python interpreter.
- This change introduces a run once decorator to allow annotating
functions that should only be executed once for the lifetime fo
an interpreter.
- This change applies the run_once decorator to the db_api configure
method, to suppress the attempt to reconfigure the current
TransactionFactory on application reload.
Co-Authored-By: Balazs Gibizer <balazs.gibizer@ericsson.com>
Closes-Bug: #1799246
Related-Bug: #1784155
Change-Id: I704196711d30c1124e713ac31111a8ea6fa2f1ba
This function is generally useful in supporting py2/3 cross-over code,
and should be available to non-test code.
Change-Id: I81d13418d75b46fbdb9f6d44889a207528c8d6de
Sean Mooney