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
Building on the ``[workarounds]/disable_native_luksv1``
configurable introduced in Ia500eb614cf575ab846f64f4b69c9068274c8c1f
this change introduces another workaround configurable that when enabled
will connect RBD volumes to the compute host as block devices using
os-brick.
When used togther both options allow operators to workaround recently
discovered performance issues in the libgcrypt library used by QEMU when
natively decrypting LUKSv1 encrypted disks.
For now the extend_volume method raises a NotImplemented error in-line
with the underlying method in os-brick. Future work will be required to
both support this in os-brick and wire up the required calls in the
volume driver.
This workaround is temporary and will be removed during the W release
once all impacted distributions have been able to update their versions
of the libgcrypt library.
Finally os-brick 3.0.1 is now required as it provides the
Id507109df80391699074773f4787f74507c4b882 fix when attempting to
diconnect locally attached RBD volumes.
Closes-Bug: #1869184
Change-Id: Ied3732042738a6194b635c55e0304d71a6fb66e3
Ieb9a03d308495be4e8c54b5c6c0ff781ea7f0559 introduced support for using
QEMU's native iSCSI initiator support way back in Kilo. However this was
only enabled when the LibvirtNetVolumeDriver class was configured as the
``iscsi`` volume driver via the ``[libvirt]/volume_drivers`` configurable.
Unfortunately this configurable was removed in Liberty by
I832820499ec3304132379ad9b9d1ee92c5a75b61 essentially rendering this
``iscsi`` based code path dead ever since unless operators manually
hacked the now static ``libvirt_volume_drivers`` list within driver.py.
As a result of this and a complete lack of any test coverage in the gate
we can now remove this unreachable code from Nova. It might be desirable
to reintroduce this support later but this should take the form of an
extracted volume driver and a new configurable within nova.conf to
switch between the two available drivers.
Closes-Bug: #1501447
Change-Id: I1043287fe8063c4b2af07c997a931a7097518ca9
Implement support for extending RBD attached volumes using the libvirt
network volume driver.
This adds a new parameter "requested_size" to the extend_volume method.
This is necessary because the new volume size can not be detected by
libvirt for network volumes. All other volume types currently
implementing the extend_volume call have a block device on the
hypervisor which needs to be updated and can be polled for it's new
size. For network volumes no such block device exists.
Alternatively this could be implemented without a new parameter by
calling into Ceph using os_brick to get the new size of the volume.
This would make the LibvirtNetVolumeDriver Ceph specific.
This also extends the logic to get the device_path for extending volumes
in the libvirt driver. This is necessary as network volumes don't have
the device path in the connection_info. The device_path is retrieved by
matching the connection_info serial (= volume UUID) against all guest
disks.
Co-Authored-By: Jose Castro Leon <jose.castro.leon@cern.ch>
Blueprint: extend-in-use-rbd-volumes
Change-Id: I5698e451861828a8b1240d046d1610d8d37ca5a2
The warning being modified here was added in Pike:
I6fc7108817fcd9df4a342c9dabbf14ab7911d06a
And then updated to say we would remove the fallback in
Queens:
I006f41383dcef68d63a646665a56cbd4c3f93c4b
And here we're long past Queens and still have this old
warning with no immediate plans to do data migrations to
drop the fallback code, so this change simply makes the
warning message more generic.
Change-Id: Icdc02d0bbd023e916bd39945fe7afc52b7a3942c
As of Ocata, cinder config is preferred for rbd auth values with a
fallback to nova values [1]. The fallback path, for the case when
rbd_user is configured in cinder.conf and rbd_secret_uuid is not
configured in cinder.conf, results in the mismatched use of cinder
rbd_user with nova rbd_secret_uuid.
This fixes that fallback path to use nova rbd_user from nova.conf
with rbd_secret_uuid from nova.conf.
[1] See commit f2d27f6a8a
Thanks to David Ames for this fix.
Change-Id: Ieba216275c07ab16414065ee47e66915e9e9477d
Co-Authored-By: David Ames <david.ames@canonical.com>
Closes-Bug: #1809454
_disconnect_volume was being passed disk_dev, which was is name of the
disk as presented to the gust. However, _disconnect_volume is only
concerned with unmounting the volume from the host, so this isn't
relevant.
A couple of volume drivers were using it for logging. We remove these
because it wasn't done consistently, is better done by the caller, and
isn't required as the information is available from other log
messages.
In the very minor associated debug log cleanup we also remove logging
of connection_info from one volume driver, which is potential security
bug.
We also do some associated cleanup in a few volume driver tests which
were assuming that disconnect_volume was being passed disk_info rather
than disk_dev, and that connection_info['data'] is related to
disk_info, which it isn't.
Change-Id: I61a0bee9e71e9a67f6a7c04a7bfd6e77fe818a77
it's apparently too late to make this change in Pike rc stage
so defer to Queens by changing the release version.
Change-Id: I006f41383dcef68d63a646665a56cbd4c3f93c4b
The i18n team has decided not to translate the logs because it
seems like it not very useful; operators prefer to have them in
English so that they can search for those strings on the internet.
Partially fix on nova/virt other paths will be fixed on next commits
Change-Id: Ie7821aa4a5147cdb0616741bd1a1b1fc22080440
Change Idcbada705c1d38ac5fd7c600141c2de7020eae25 in Ocata
started preferring Cinder connection info for getting RBD auth
values since Nova needs to be using the same settings as Cinder
for volume auth.
However, that introduced a problem for guest connections made
before that change, where the secret_uuid might not have been
configured on the Cinder side and that's what is stored in the
block_device_mappings.connection_info column and is what we're
checking in _set_auth_config_rbd. Before Ocata this wasn't a
problem because we'd use the Nova configuration values for the
rbd_secret_uuid if set. But since Ocata it is a problem since
we don't consult nova.conf if auth was enabled, but not completely
configured, on the Cinder side.
So this adds a fallback check to set the secret_uuid from
nova.conf if it wasn't set in the connection_info via Cinder
originally. A note is also added to caution about removing
any fallback mechanism on the nova side - something we'd
need to consider before we could likely drop this code.
Co-Authored-By: Tadas Ustinavičius <tadas@ring.lt>
Change-Id: I6fc7108817fcd9df4a342c9dabbf14ab7911d06a
Closes-Bug: #1687581
When we support multi-attach volumes, for volume drivers which must
make host state changes (eg mount/unmount) it is no longer enough to
know only which volume is being connected; we must also know which
instance it is being attached to. Consider the following sequence of
calls, and the expected behaviour of the volume driver:
* connect_volume(conn_info)
connect the volume
* connect_volume(conn_info)
do nothing (volume is already connected)
* disconnect_volume(conn_info)
disconnect the volume
* disconnect_volume(conn_info)
do nothing (volume is already disconnected)
Now consider these were actually connections to different instances:
* connect_volume(conn_info, A)
connect the volume
* connect_volume(conn_info, B)
do nothing (volume is already connected)
* disconnect_volume(conn_info, A)
++ do nothing (volume is still connected to B)
* disconnect_volume(conn_info, B)
disconnect the volume
IOW, it is not possible for the driver to determine the correct
behaviour unless it also knows which instance is being attached.
This is a non functional change which simply adds instance as an
argument to all connect_volume and disconnect_volume calls in libvirt
volume drivers. It is effectively a part of change I3155984d. I have
split it as it is mechanical, it touches a large number of files
which make the substantive change harder to read, and to isolate the
substantive change from merge conflicts caused by this change.
We also add a utility test class: SubclassSignatureTestCase. This is
used in this patch to ensure we have found all connect_volume and
disconnect_volume methods which need to be updated. We implement it in
the top level tests module as we expect it to be useful elsewhere.
This patch was previously submitted as change I658d7ab5 but had to be
reverted as it failed to update the signatures of several volume
drivers. This is also what has prompted the addition of
SubclassSignatureTestCase, which ensures this hasn't happened again.
Change-Id: I01c908add1312063f0db724110f7e5927ff281ff
This reverts commit f0153fa4c8.
This broke a whole set of FS-based volume drivers since it
didn't change the method signatures in those drivers to take
the new instance argument.
I was working on adding them but then just decided this
isn't even used right now so it's faster to revert.
When this is added back in, I want to see the
LibvirtBaseVolumeDriver class turned into an ABC and
the connect_volume and disconnect_volume methods turned
into abstractmethods in LibvirtBaseVolumeDriver so this
kind of thing does not happen again.
Change-Id: If1ee6d6e4004750f80424ec54d7dabc564ec9e06
Closes-Bug: #1675584
When we support multi-attach volumes, for volume drivers which must
make host state changes (eg mount/unmount) it is no longer enough to
know only which volume is being connected; we must also know which
instance it is being attached to. Consider the following sequence of
calls, and the expected behaviour of the volume driver:
* connect_volume(conn_info)
connect the volume
* connect_volume(conn_info)
do nothing (volume is already connected)
* disconnect_volume(conn_info)
disconnect the volume
* disconnect_volume(conn_info)
do nothing (volume is already disconnected)
Now consider these were actually connections to different instances:
* connect_volume(conn_info, A)
connect the volume
* connect_volume(conn_info, B)
do nothing (volume is already connected)
* disconnect_volume(conn_info, A)
++ do nothing (volume is still connected to B)
* disconnect_volume(conn_info, B)
disconnect the volume
IOW, it is not possible for the driver to determine the correct
behaviour unless it also knows which instance is being attached.
This is a non functional change which simply adds instance as an
argument to all connect_volume and disconnect_volume calls in libvirt
volume drivers. It is effectively a part of change I3155984d. I have
split it as it is mechanical, it touches a large number of files
which make the substantive change harder to read, and to isolate the
substantive change from merge conflicts caused by this change.
Change-Id: I658d7ab503cb17ae6750fd301d49e2c46085a0c0
This is not intended to be used directly. Rather, it should be
subclassed and these subclasses used. InternalError is the generic
exception of choice, thus, this is used in place of NovaException.
Change-Id: I0091959c7c23df32e83579984378c7ca98620c5d
In the case that the ceph storage backing volumes is different
from the one backing ephemeral storage in nova, the auth values
in the rbd connection_info could be different and not work if
we are using the nova.conf values for ephemeral storage.
This change makes the volume connection config code for rbd
prefer the cinder connection_info values if they exist, and
only falls back to nova config values if cinder doesn't have
anything set.
Depends-On: I4655cae3212d589177d2570403b563a83aad529a
Change-Id: Idcbada705c1d38ac5fd7c600141c2de7020eae25
Closes-Bug: #1635008
The LibvirtNetVolumeDriver is handling both rbd and
sheepdog (iscsi) connections. The auth config logic
is mingling both backends which is really confusing.
For example, the iscsi protocol only defines auth_method,
auth_username and auth_password. It does not set an
auth_enabled value in the connection_info['data'] dict.
This change simplifies the logic involved for setting
the auth config by decoupling the rbd/iscsi handlers.
A follow-up change will build on this to fix the
rbd auth config to prefer the cinder volume connection_info
auth data over the local config for nova in the case
that different cinder backends are used for ephemeral
and block storage.
Change-Id: I8a55d87f75ecad757ce81b1f5f77c3a551154a17
Partial-Bug: #1635008
We were initialising libvirt volume drivers by passing the
LibvirtDriver object. However, this is only ever used to fetch the
Host. We update the interface to pass the Host instead. This is
cleaner, but also better represents the intent of the interface. They
should not be able to access arbitrary attributes of the LibvirtDriver
object.
Change-Id: I87ceeee1ec46dc22754321574b16e6c47b27a848
The config options of the "nova.conf" section "libvirt" got
moved to the new central location "nova/conf/libvirt.py".
Subsequent patches will then move another options in libvirt section.
This is the 12th patch in a long-chain patchs.
Change-Id: I5cb03ce7082e9f71ee584c4cdb2cc9a658882a9e
Co-Authored-by: Markus Zoeller <mzoeller@de.ibm.com>
Implements: blueprint centralize-config-options-newton
Introduces the LibvirtISCSIVolumeBaseTestCase class for the iSCSI-based
volume driver tests like iSCSI and the Network volume driver (iSCSI volume
driver changes yet to come).
There is some additional refactor cleanup that can be done here since
this code is a bit hacky with the protocol checks and secret CRUD ops in
the libvirt host module, but that's reserved for later changes.
Part of blueprint consolidate-libvirt-fs-volume-drivers
Change-Id: I31404d057d596473a9c0960e2696affb1e35e132
Sean Mooney