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
Move these to the central place. There's a large amount of test damage
but it's pretty trivial.
Change-Id: If581eb7aa463c9dde13714f34f0f1b41549a7130
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
This rather beefy (but also quite simple) patch replaces the
'stub_out_image_service' call and associated cleanup in all functional
tests with a new 'GlanceFixture', based on the old 'FakeImageService'.
The use of a fixture means we don't have to worry about teardown and
allows us to stub Glance in the same manners as Cinder, Neutron,
Placement etc.
Unit test cleanup is handled in a later patch.
Change-Id: I6daea47988181dfa6dde3d9c42004c0ecf6ae87a
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
'_IntegratedTestBase' has subclassed 'InstanceHelperMixin' since change
I0d21cb94c932e6e556eca964c57868c705b2d120, which means both now provide
a '_build_minimal_create_server_request' function. However, only
'_IntegratedTestBase' provides a '_build_server' function. The
'_build_minimal_create_server_request' and '_build_server' functions do
pretty much the same thing but there are some differences. Combine these
under the '_build_server' alias.
Change-Id: I91fa2f73185fef48e9aae9b7f61389c374e06676
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
Pretty much every test case in 'nova.tests.functional' defines an 'api'
attribute, and many define an 'admin_api' attribute. We can pull these
from the class rather than explicitly passing them to helpers. Rework
things so this happens.
Note that the bulk of the changes here are in the
'nova/tests/functional/integrated_helpers.py' file. The rest of the
changes were auto-generated using the following script (my sed-fu is
non-existent):
$ cd nova/tests/functional
$ python3
>>> import glob
>>> import re
>>> pattern = r'_state_change\((\n\s+)?self\.(admin_)?api,\s+'
>>> replace = r'_state_change(\1'
>>> for path in glob.glob('*.py') + glob.glob('*/*.py'):
... with open(path) as fh:
... data = fh.read()
... new = re.sub(pattern, replace, data, flags=re.MULTILINE)
... if new != data:
... with open(path, 'w') as fh:
... fh.write(new)
...
>>> quit()
(ditto for the other substitutions)
Some manual fixups were required after, which pre-commit highlighted :)
Change-Id: I8c96b337f32148f8f5899c9b87af331b1fa41424
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
Multiple functional tests used its own copy-pasted implementation of a
weigher to make the scheduling deterministic based on host names.
This patch moves the HostNameWeigher implementation to a common place
and adds a fixtures to set up the weigher in the conf.
Change-Id: I4c23a3c4f8963c42379e7a8d63c6c1e089350eb3
The virt driver FakeDriver used in both the functional and in the unit
test used a global state to configure the host and node names the driver
reports. This was hard to use when more then one compute service is started.
Also global state is dangerous.
It turned out that only a set of unit tests are using multiple nodes per
compute the rest of the tests can simply use host=<hostname>,
nodes=[<hostname>] setup.
So this removes the global state.
Change-Id: I2cf2fcbaebc706f897ce5dfbff47d32117064f9c
This hasn't been needed since we moved console authentication to the
database in Rocky.
Part of blueprint remove-consoleauth
Change-Id: Ie5e7d70f9d6af77edd22756128937f06d0b961a9
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
It is not necessary to call the following statement multiple times in
some functional tests.
self.addCleanup(fake.restore_nodes)
So remove duplicate calls in the tests.
TrivalFix
Change-Id: Iaae6fc4a66145576f4a4fc1cea452ef6acbadb15
Adjust the fixtures used by the functional tests so they
use placement database and web fixtures defined by placement
code. To avoid making redundant changes, the solely placement-
related unit and functional tests are removed, but the placement
code itself is not (yet).
openstack-placement is required by the functional tests. It is not
added to test-requirements as we do not want unit tests to depend
on placement in any way, and we enforce this by not having placement
in the test env.
The concept of tox-siblings is used to ensure that the
placement requirement will be satisfied correctly if there is a
depends-on. To make this happen, the functional jobs defined in
.zuul.yaml are updated to require openstack/placement.
tox.ini has to be updated to use a envdir that is the same
name as job. Otherwise the tox siblings role in ansible cannot work.
The handling of the placement fixtures is moved out of nova/test.py
into the functional tests that actually use it because we do not
want unit tests (which get the base test class out of test.py) to
have anything to do with placement. This requires adjusting some
test files to use absolute import.
Similarly, a test of the comparison function for the api samples tests
is moved into functional, because it depends on placement functionality,
TestUpgradeCheckResourceProviders in unit.cmd.test_status is moved into
a new test file: nova/tests/functional/test_nova_status.py. This is done
because it requires the PlacementFixture, which is only available to
functional tests. A MonkeyPatch is required in the test to make sure that
the right context managers are used at the right time in the command
itself (otherwise some tables do no exist). In the test itself, to avoid
speaking directly to the placement database, which would require
manipulating the RequestContext objects, resource providers are now
created over the API.
Co-Authored-By: Balazs Gibizer <balazs.gibizer@ericsson.com>
Change-Id: Idaed39629095f86d24a54334c699a26c218c6593
The assertion in the test that the migration status is 'completed'
is flawed in that it assumes when the instance status is 'ACTIVE'
the migration is completed, which isn't True, since the instance
status gets changed before the migration is completed, but they are
very close in time so there is a race, which is how this test slipped
by. This fixes the issue by polling the migration status until it
is actually completed or we timeout.
Change-Id: I61f745667f4c003d7e3ca6f2f9a99194930ac892
Closes-Bug: #1762876
During a resize, the RequestSpec.flavor is updated
to the new flavor. If the resize failed on one host
and was rescheduled, the RequestSpec.retry is updated
for that failed host and mistakenly persisted, which
can affect later move operations, like if an admin
targets one of those previously failed hosts for a
live migration or evacuate operation.
This change fixes the problem by not ever persisting
the RequestSpec.retry field to the database, since
retries are per-request/operation and not something
that needs to be persisted.
Alternative to this, we could reset the retry field
in the RequestSpec.reset_forced_destinations method
but that would be slightly overloading the meaning
of that method, and the approach taken in this patch
is arguably cleaner since retries shouldn't ever be
persisted. It should be noted, however, that one
advantage to resetting the 'retry' field in the
RequestSpec.reset_forced_destinations method would
be to avoid this issue for any existing DB entries
that have this problem.
The related functional regression test is updated
to show the bug is now fixed.
Change-Id: Iadbf8ec935565a6d4ccf6f36ef630ab6bf1bea5d
Closes-Bug: #1718512
Commit 74ab427d47 in Newton added
code to persist changes to the RequestSpec during a resize since
the flavor changes.
That change inadvertantly also persisted any failed hosts during
the resize that are stored in the RequestSpec.retry field during
a reschedule.
The problem is that later those persisted failed hosts are rejected
by the RetryFilter, which can be confusing if an admin is trying
to live migrate or evacate the instance to one of those specific
hosts.
This adds a functional regression test to show the failure, which
will be fixed in a separate change that then modifies the assertions.
Change-Id: Ib8a23db838b0bbf2cfb8123cf6aaa39d00ff0640
Related-Bug: #1718512
Sean Mooney