This enables use of ephemeral encryption for swap disks.
The 'encrypted' attribute is intended to be read-only, so we will raise
an error if something attempts to save it later.
DriverImageBlockDevice and DriverEphemeralBlockDevice are already not
saving updates to their 'encrypted' attributes, so the same error
checking is added to them as well for consistency.
Related to blueprint ephemeral-encryption-libvirt
Change-Id: Id30577699928180eff6a5b78390ce9e3efa28b16
RDP console was only for HyperV driver so removing the
API. As API url stay same (because same used for other
console types API), RDP console API will return 400.
Cleaning up the related config options as well as moving its
API ref to obsolete seciton.
Keeping RPC method to avoid error when old controller is used
with new compute. It can be removed in next RPC version bump.
Change-Id: I8f5755009da4af0d12bda096d7a8e85fd41e1a8c
In the previous patch we changed the ordering of operations during
post_live_migration() to minimize guest networking downtime by
activating destination host port bindings as soon as possible.
Review of that patch led to the realization that exceptions during
notification sending can prevent the port binding activation from
happening. Instead of handling that in a localized try/catch, this
patch implements a general best_effort kwarg to our two notification
sending helpers to allow callers to indicate that any exceptions
during notification sending should not be fatal.
Change-Id: I01a15d6fffe98816ae019e67dc72784299fedfd3
This bumps the version of flake8 and resolves some erroneous failures in
f-strings. A number of new E721 (do not compare types) class errors are
picked up, which are all addressed.
Change-Id: I7a1937b107ff3af8d1e5fe23fc32b120ef4697f7
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
this mainly fixes typos in the tests and
one type in an exception message.
some addtional items are added to the dict based on
our usage of vars in test but we could remove them later
by doing minor test updates. They are intentionally not
fixed in this commit to limit scope creep.
Change-Id: Iacfbb0a5dc8ffb0857219c8d7c7a7d6e188f5980
This change ensure we only try to clean up dangling bdms if
cinder is installed and reachable.
Closes-Bug: #2033752
Change-Id: I0ada59d8901f8620fd1f3dc20d6be303aa7dabca
On reboot, check the instance volume status on the cinder side.
Verify if volume exists and cinder has an attachment ID, else
delete its BDMS data from nova DB and vice versa.
Updated existing test cases to use CinderFixture while rebooting as
reboot calls get_all_attachments
Implements: blueprint https://blueprints.launchpad.net/nova/+spec/cleanup-dangling-volume-attachments
Closes-Bug: 2019078
Change-Id: Ieb619d4bfe0a6472aefb118b58283d7ad8d24c29
This adds the compute_id field to the Instance object and adds
checks in save() and create() to make sure we no longer update node
without also updating compute_id.
Related to blueprint compute-object-ids
Change-Id: I0740a2e4e09a526da8565a18e6761b4dbdc4ec0b
This makes us store the compute_id of the destination node in the
Migration object. Since resize/cold-migration changes the node
affiliation of an instance *to* the destination node *from* the source
node, we need a positive record of the node id to be used. The
destination node set this to its own node when creating the migration,
and it is used by the source node when the switchover happens.
Because the migration may be backleveled for an older node involved
in that process and thus saved or passed without this field, this
adds a compatibility routine that falls back to looking up the node
by host/nodename.
Related to blueprint compute-object-ids
Change-Id: I362a40403d1094be36412f5f7afba00da8af8301
The ComputeNode object already has a service_id field that we stopped
using a while ago. This moves us back to the point where we set it when
creating new ComputeNode records, and also migrates existing records
when they are loaded.
The resource tracker is created before we may have created the
service record, but is updated afterwards in the pre_start_hook().
So this adds a way for us to pass the service_ref to the resource
tracker during that hook so that it is present before the first time
we update all of our ComputeNode records. It also makes sure to pass
the Service through from the actual Service manager instead of looking
it up again to make sure we maintain the tight relationship and avoid
any name-based ambiguity.
Related to blueprint compute-object-ids
Change-Id: I5e060d674b6145c9797c2251a2822106fc6d4a71
Previously, in numa_usage_from_instance_numa(), any new NUMACell
objects we created did not have the `socket` attribute. In some cases
this was persisted all the way down to the database. Fix this by
copying `socket` from the old_cell.
Change-Id: I9ed3c31ccd3220b02d951fc6dbc5ea049a240a68
Closes-Bug: 1995153
We have been ignoring the case where a rebuild or evacuate is triggered
and we fail to find *any* node for our host. This appears to be a very
old behavior, which I traced back ten years to this:
https://review.opendev.org/c/openstack/nova/+/35851
which was merely fixing the failure to reset instance.node during an
evacuate (which re-uses rebuild, which before that was a single-host
operation). That patch intended to make a failure to find a node for
our host a non-fatal error, but it just means we fall through that
check with no node selected, which means we never update instance.node
*and* run ResourceTracker code that will fail to find the node later.
So, this makes it an explicit error, where we stop further processing,
set the migration for the evacuation to 'failed', and send a
notification for it. This is the same behavior as happens further
down if we find that the instance has been deleted underneath us.
Change-Id: I88b962aaeaa0554da4ab00906ac4d9e6deb43589
Related to the bp/allowing-target-state-for-evacuate. This change
is extending compute API to accept a new argument targetState.
The targetState argument when set will force state of an evacuated
instance to the destination host.
Signed-off-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@industrialdiscipline.com>
Change-Id: I9660d42937ad62d647afc6be965f166cc5631392
After the scheduler selected a target host and allocated an allocation
candidate that is passed the filters nova need to make sure that PCI
claim will allocate the real PCI devices from the RP which is allocated
in placement. Placement returns the request group - provider mapping for
each allocation candidate so nova can map which InstancePCIRequest was
fulfilled from which RP in the selected allocation candidate. This
mapping is then recorded in the InstancePCIRequest object and used
during the PCI claim to filter for PCI pools that can be used to claim
PCI devices from.
blueprint: pci-device-tracking-in-placement
Change-Id: I18bb31e23cc014411db68c31317ed983886d1a8e
This patch adds the plumbing for rebuilding a volume backed
instance in compute code. This functionality will be enabled
in a subsequent patch which adds a new microversion and the
external support for requesting it.
The flow of the operation is as follows:
1) Create an empty attachment
2) Detach the volume
3) Request cinder to reimage the volume
4) Wait for cinder to notify success to nova (via external events)
5) Update and complete the attachment
Related blueprint volume-backed-server-rebuild
Change-Id: I0d889691de1af6875603a9f0f174590229e7be18
The PciPassthroughFilter logic checks each InstancePCIRequest
individually against the available PCI pools of a given host and given
boot request. So it is possible that the scheduler accepts a host that
has a single PCI device available even if two devices are requested for
a single instance via two separate PCI aliases. Then the PCI claim on
the compute detects this but does not stop the boot just logs an ERROR.
This results in the instance booted without any PCI device.
This patch does two things:
1) changes the PCI claim to fail with an exception and trigger a
re-schedule instead of just logging an ERROR.
2) change the PciDeviceStats.support_requests that is called during
scheduling to not just filter pools for individual requests but also
consume the request from the pool within the scope of a single boot
request.
The fix in #2) would not be enough alone as two parallel scheduling
request could race for a single device on the same host. #1) is the
ultimate place where we consume devices under a compute global lock so
we need the fix there too.
Closes-Bug: #1986838
Change-Id: Iea477be57ae4e95dfc03acc9368f31d4be895343
We have many places where we implement singleton behavior for the
placement client. This unifies them into a single place and
implementation. Not only does this DRY things up, but may cause us
to initialize it fewer times and also allows for emitting a common
set of error messages about expected failures for better
troubleshooting.
Change-Id: Iab8a791f64323f996e1d6e6d5a7e7a7c34eb4fb3
Related-Bug: #1846820
The PowerVM driver was deprecated in November 2021 as part of change
Icdef0a03c3c6f56b08ec9685c6958d6917bc88cb. As noted there, all
indications suggest that this driver is no longer maintained and may be
abandonware. It's been some time and there's still no activity here so
it's time to abandon this for real.
This isn't as tied into the codebase as the old XenAPI driver was, so
removal is mostly a case of deleting large swathes of code. Lovely.
Change-Id: Ibf4f36136f2c65adad64f75d665c00cf2de4b400
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
In py310 unittest.mock does not allow to mock the same function twice as
the second mocking will fail to autospec the Mock object created by the
first mocking.
This patch manually fixes the double mocking.
Fixed cases:
1) one of the mock was totally unnecessary so it was removed
2) the second mock specialized the behavior of the first generic mock.
In this case the second mock is replaced with the configuration of
the first mock
3) a test case with two test steps mocked the same function for each
step with overlapping mocks. Here the overlap was removed to have
the two mock exists independently
The get_connection injection in the libvirt functional test needed a
further tweak (yeah I know it has many already) to act like a single
mock (basically case #2) instead of a temporary re-mocking. Still the
globalness of the get_connection mocking warrant the special set / reset
logic there.
Change-Id: I3998d0d49583806ac1c3ae64f1b1fe343cefd20d
Now that we no longer support py27, we can use the standard library
unittest.mock module instead of the third party mock lib. Most of this
is autogenerated, as described below, but there is one manual change
necessary:
nova/tests/functional/regressions/test_bug_1781286.py
We need to avoid using 'fixtures.MockPatch' since fixtures is using
'mock' (the library) under the hood and a call to 'mock.patch.stop'
found in that test will now "stop" mocks from the wrong library. We
have discussed making this configurable but the option proposed isn't
that pretty [1] so this is better.
The remainder was auto-generated with the following (hacky) script, with
one or two manual tweaks after the fact:
import glob
for path in glob.glob('nova/tests/**/*.py', recursive=True):
with open(path) as fh:
lines = fh.readlines()
if 'import mock\n' not in lines:
continue
import_group_found = False
create_first_party_group = False
for num, line in enumerate(lines):
line = line.strip()
if line.startswith('import ') or line.startswith('from '):
tokens = line.split()
for lib in (
'ddt', 'six', 'webob', 'fixtures', 'testtools'
'neutron', 'cinder', 'ironic', 'keystone', 'oslo',
):
if lib in tokens[1]:
create_first_party_group = True
break
if create_first_party_group:
break
import_group_found = True
if not import_group_found:
continue
if line.startswith('import ') or line.startswith('from '):
tokens = line.split()
if tokens[1] > 'unittest':
break
elif tokens[1] == 'unittest' and (
len(tokens) == 2 or tokens[4] > 'mock'
):
break
elif not line:
break
if create_first_party_group:
lines.insert(num, 'from unittest import mock\n\n')
else:
lines.insert(num, 'from unittest import mock\n')
del lines[lines.index('import mock\n')]
with open(path, 'w+') as fh:
fh.writelines(lines)
Note that we cannot remove mock from our requirements files yet due to
importing pypowervm unit test code in nova unit tests. This library
still uses the mock lib, and since we are importing test code and that
lib (correctly) only declares mock in its test-requirements.txt, mock
would not otherwise be installed and would cause errors while loading
nova unit test code.
[1] https://github.com/testing-cabal/fixtures/pull/49
Change-Id: Id5b04cf2f6ca24af8e366d23f15cf0e5cac8e1cc
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Today Nova updates the mac_address of a direct-physical port to reflect
the MAC address of the physical device the port is bound to. But this
can only be done before the port is bound. However during migration Nova
does not update the MAC when the port is bound to a different physical
device on the destination host.
This patch extends the libvirt virt driver to provide the MAC address of
the PF in the pci_info returned to the resource tracker. This
information will be then persisted in the extra_info field of the
PciDevice object.
Then the port update logic during migration, resize, live
migration, evacuation and unshelve is also extended to record the MAC of
physical device in the port binding profile according to the device on
the destination host.
The related neutron change Ib0638f5db69cb92daf6932890cb89e83cf84f295
uses this info from the binding profile to update the mac_address field
of the port when the binding is activated.
Closes-Bug: #1942329
Change-Id: Iad5e70b43a65c076134e1874cb8e75d1ba214fde
Allow instances to be created with VNIC_TYPE_REMOTE_MANAGED ports.
Those ports are assumed to require remote-managed PCI devices which
means that operators need to tag those as "remote_managed" in the PCI
whitelist if this is the case (there is no meta information or standard
means of querying this information).
The following changes are introduced:
* Handling for VNIC_TYPE_REMOTE_MANAGED ports during allocation of
resources for instance creation (remote_managed == true in
InstancePciRequests);
* Usage of the noop os-vif plugin for VNIC_TYPE_REMOTE_MANAGED ports
in order to avoid the invocation of the local representor plugging
logic since a networking backend is responsible for that in this
case;
* Expectation of bind time events for ports of VNIC_TYPE_REMOTE_MANAGED.
Events for those arrive early from Neutron after a port update (before
Nova begins to wait in the virt driver code, therefore, Nova is set
to avoid waiting for plug events for VNIC_TYPE_REMOTE_MANAGED ports;
* Making sure the service version is high enough on all compute services
before creating instances with ports that have VNIC type
VNIC_TYPE_REMOTE_MANAGED. Network requests are examined for the presence
of port ids to determine the VNIC type via Neutron API. If
remote-managed ports are requested, a compute service version check
is performed across all cells.
Change-Id: Ica09376951d49bc60ce6e33147477e4fa38b9482
Implements: blueprint integration-with-off-path-network-backends
For some reason, we have two lineages of quota-related exceptions in
Nova. We have QuotaError (which sounds like an actual error), from
which all of our case-specific "over quota" exceptions inhert, such
as KeypairLimitExceeded, etc. In contrast, we have OverQuota which
lives outside that hierarchy and is unrelated. In a number of places,
we raise one and translate to the other, or raise the generic
QuotaError to signal an overquota situation, instead of OverQuota.
This leads to places where we have to catch both, signaling the same
over quota situation, but looking like there could be two different
causes (i.e. an error and being over quota).
This joins the two cases, by putting OverQuota at the top of the
hierarchy of specific exceptions and removing QuotaError. The latter
was only used in a few situations, so this isn't actually much change.
Cleaning this up will help with the unified limits work, reducing the
number of potential exceptions that mean the same thing.
Related to blueprint bp/unified-limits-nova
Change-Id: I17a3e20b8be98f9fb1a04b91fcf1237d67165871
This reverts commit 7a7a223602.
That commit was added because - tl'dr - upon revert resize, Neutron
with the OVS backend and the iptables security group driver would send
us the network-vif-plugged event as soon as we updated the port
binding.
That behaviour has changed with commit 66c7f00e1d. With that commit,
we started unplugging the vifs on the source compute host when doing a
resize. When reverting the resize, the vifs had to be re-plugged again,
regarldess of the networking backend in use. This renders commit
7a7a223602. pointless, and it can be
reverted.
Conflicts - most have to do with context around this commit's code:
nova/compute/manager.py
a2984b647a added provider_mappings to
_finish_revert_resize_network_migrate_finish()'s signature
750aef54b1 started using
_finish_revert_resize_network_migrate_finish() in
_finish_revert_snapshot_based_resize_at_source()
nova/network/model.py
8b33ac0644 added get_live_migration_plug_time_events() and
has_live_migration_plug_time_event()
7da94440db added has_port_with_allocation()
nova/objects/migration.py
f203da3838 added is_resize() and is_live_migration()
nova/tests/unit/compute/test_compute.py
a0e60feb3e added request_spec to the test
nova/tests/unit/compute/test_compute_mgr.py
be278006a5 added unit tests below ours
nova/tests/unit/network/test_network_info.py
7da94440db (again) added tests for has_port_with_allocation()
nova/tests/unit/virt/libvirt/test_driver.py and
nova/virt/libvirt/driver.py are different in that attempting to
identify individual conflicts is a pointless exercise, as so much has
changed (mdev, vtmp, the recent wait for events during hard reboot
workaround config option, etc). They can be treated as
manual removal of any code that had to do with the bind-time events
logic (though guided by the conflict markers in git).
TODO(artom) There was a follow up commit,
78a08d44ea, that added the migration
parameter to finish_revert_migration(). This is no longer needed, as
the migration was only used to obtain plug-time events. We'll have to
undo that as well.
Closes-bug: 1952003
Change-Id: I3cb39a9ec2c260f422b3c48122b9db512cdd799b
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
The signature of the lock() function gained a new kwargs in
oslo.concurrency 4.5 which makes some nova unit test failing. This patch
changes the mocking the unit test to mock in a higher level in the
locking infra to avoid depending on the exact signature.
This will enable
https://review.opendev.org/c/openstack/requirements/+/814889/ to land.
Change-Id: Icecb75d9a374b29f2cf70d3aa155dc6c92bf715f
There are couple of changes we can make here:
- Always attempt to refresh the cache before checking if an extension is
enabled.
- Using extension slugs as our reference point rather than extension
names. They seem like a better thing to use as a constant and are
similarly fixed.
- Be consistent in how we name and call the extension check functions
- Add documentation for what each extension doing/used for
There's a TODO here to remove some code that relies on an out-of-tree
extension that I can't see. That's done separately since this is already
big enough.
Change-Id: I8058902df167239fa455396d3595a56bcf472b2b
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
As the bug and fix If71620e808744736cb4fe3abda76d81a6335311b showed
it is dangerous to forget instantiating the Mock class before it is
used in the test as changes on the class directly leaks out from the
test. In almost all the cases using Mock class directly is a bug and the
author original intention is to use an instance instead, just forgot
about the parents. So this patch adds two new hacking rules:
N367: catches the case when Mock class is aliased in the test:
self.mock_mystuff = mock.Mock
N368: catches when mock.patch instructed to use the Mock class as
replacement value during patching:
mock.patch('Bar.foo', new=mock.Mock)
For N367 the previous patch removed the last hit. For N368 this patch
removes the two hits exists.
Change-Id: Id42ca571b1569886ef47aa350369e9d2068e77bc
Related-Bug: #1936849
Nova re-generates the resource request of an instance for each server
move operation (migrate, resize, evacuate, live-migrate, unshelve) to
find (or validate) a target host for the instance move. This patch
extends the this logic to support the extended resource request from
neutron.
As the changes in the neutron interface code is called from nova-compute
service during the port binding the compute service version is bumped.
And a check is added to the compute-api to reject the move operations
with ports having extended resource request if there are old computes
in the cluster.
blueprint: qos-minimum-guaranteed-packet-rate
Change-Id: Ibcf703e254e720b9a6de17527325758676628d48
This adds the final missing pieces to support creating servers with
ports having extended resource request. As the changes in the neutron
interface code is called from nova-compute service during the port
binding the compute service version is bumped. And a check is added to
the compute-api to reject such server create requests if there are old
computes in the cluster.
Note that some of the negative and SRIOV related interface attach
tests are also started to pass as they are not dependent on any of the
interface attach specific implementation. Still interface attach is
broken here as the failing of the positive tests show.
blueprint: qos-minimum-guaranteed-packet-rate
Change-Id: I9060cc9cb9e0d5de641ade78c5fd7e1cc77ade46
Zuul