Sean McGinnis [Tue, 5 May 2015 20:57:30 +0000 (15:57 -0500)]
Logging not using oslo.i18n guidelines (zonemgr)
Multi-patch set for easier chunks. This one addresses
the zonemanager cinder directory.
There have been quite a few instances found where the
i18n guidelines are not being followed. I believe this
has helped lead to some of the confusion around how to
correctly do this. Other developers see this code and
assume it is an example of the correct usage.
This patch attempts to clean up most of those violations
in the existing codebase to hopefully help avoid some of
that confusion in reviews.
Some issues address:
* Correct log translation markers for different log levels
* Passing format values as arguments to call, not preformatting
* Not forcing translation via six.text_type and others
Guidelines can be found here:
http://docs.openstack.org/developer/oslo.i18n/guidelines.html
Hacking checks will not be able to identify all violations of
the guidelines, but it could be useful for catching obvious
one such as LOG.info("No markers!").
Some of unit tests print logs to stdout. Tests output should be clear
and contain only necessary information. Debug output should not be mixed
with tests output.
Gorka Eguileor [Mon, 9 Mar 2015 18:39:11 +0000 (19:39 +0100)]
Preserve usage and reservations on quota deletion
Current API deletes quota usage and reservations on quota limit
deletion.
According to API documentation what should only happen is that quotas
limits revert to default values by deleting tenant/user limits.
This patch fixes this issue.
APIImpact: Delete on os-quota-sets will no longer remove usage and
reservation quotas. Those quotas are handled by Cinder
service.
UpgradeImpact: There is no upgrade impact afaik.
Closes-Bug: #1410034
Change-Id: I9340b6f78623cfa5b505886ad75b8e4d3cd6131b
Jamie Lennox [Tue, 5 May 2015 01:50:21 +0000 (11:50 +1000)]
Catch additional type conversion errors
When converting strings to integers in quota we catch ValueError which
will handle an input which is not a valid number, eg 'abc'. Extend this
to covert TypeErrors which will handle things like a None value being
passed.
Currently vCenter certificate is not verified during connection
establishment. This patch adds a config option to specify a CA
bundle file to verify vCenter server certificate.
John Griffith [Fri, 1 May 2015 16:35:30 +0000 (10:35 -0600)]
Fake out sleeps in unit tests
Just a minor cleanup, had a few tests that were doing sleeps
(or looping calls for those that prefer it) that were being
executed to test timeouts in the unit tests. No value in
actually *waiting* for these, so this patch just mocks them
out so we still test the procedure but don't actually have
to wait.
This shaves about 30 seconds or so off of the unit test run,
not a huge deal, but every little bit helps IMO.
Also noticed a LOG.debug message in the test_extensions file
that wasn't picked up in other cleanup work so I removed that
debug log call as well.
Tom Barron [Sat, 18 Apr 2015 13:42:10 +0000 (09:42 -0400)]
Fix range check for NFS used ratio
The nfs_used_ratio configuration parameter is supposed to be
greater than zero and less than or equal to one. However,
current configuration checks actually allow values greater
than one because of a logic error. Unit tests fail to detect
this issue because they find the exception that they expect
to find, but it is actually being thrown for another reason.
This commit fixes the logic error in the nfs driver code.
It also reworks all the unit tests for the do_setup method since
they suffer from the same issue that led to failure to detect
this error in the first place - they fail to do proper mocks of
submethods, and when these run the expected exception gets triggered
for the wrong reason. We have moved these tests into their own test
class, where mock is used rather than mox. Over time it is likely
that new tests will be implemented, or old tests reworked, with mock
instead of mox. These can go in the new test class, which can be
renamed if that is appropriate.
When attach is called twice for the same volume and instance the
attach_volume checks if the attachment already exists and returns.
This leaves the volume in 'Attaching' state. The volume status should
be reset to 'in-use' if we see the attachment already exists.
John Griffith [Wed, 29 Apr 2015 23:25:12 +0000 (17:25 -0600)]
Add retry to lvm delete
Seems we have another issue related to lvm and
snapshots, but this time it's on the delete side.
We used a simple retry mechanism for snapshot create
here:
https://review.openstack.org/#/c/149360/8
I'm hesitant to just add another retry without looking
at dm issues, but this should address the problem
temporarily and give us a chance to collect some
data on the issue.
wuyuting [Fri, 13 Feb 2015 02:16:24 +0000 (10:16 +0800)]
Admin extends tenant's volume but change admin's quota
When admin extends a tenant's volume, the tenant's
quota-usage doesn't change. However, admin's quota-usage
has changed. The right practice is: extend whose volume,
whose quota-usage should be changed.
The Oslo libraries have moved all of their code out of the 'oslo'
namespace package into per-library packages. The namespace package was
retained during kilo for backwards compatibility, but will be removed by
the liberty-2 milestone. This change removes the use of the namespace
package, replacing it with the new package names.
The patches in the libraries will be put on hold until application
patches have landed, or L2, whichever comes first. At that point, new
versions of the libraries without namespace packages will be released as
a major version update.
Please merge this patch, or an equivalent, before L2 to avoid problems
with those library releases.
Jay S. Bryant [Mon, 20 Apr 2015 23:05:47 +0000 (18:05 -0500)]
Add hacking check for str and unicode in exceptions
One of the comments we are frequently having to make
on reviews is with regards to the use of str() or
unicode() on exceptions. This hacking check pulled
from Nova will catch this problem earlier and avoid
conflicting comments being made in reviews.
Joel Coffman [Thu, 26 Mar 2015 22:14:01 +0000 (18:14 -0400)]
Add test case for volume_encryption_metadata_get
This change adds unit tests for the volume_encryption_metadata_get
function. The unit tests provide protection against regressions when
refactoring this code as part of follow-up patches.
Writing the unit tests also exposed a minor issue with the existing
implementation of the volume_encryption_metadata_get function. If the
volume type is not encrypted, then the existing implementation would
raise an exception due to volume_type_encryption_get returning None.
In practice, this issue would not be encountered due to separate
checks to ensure that the volume type is encrypted, but a small
refactoring obviates the need for these checks and allows the
volume_encryption_metadata_get function to be invoked for both
encrypted and "normal" volumes.
A separate patch will clean up the unnecessary checks to ensure that
the volume type was encrypted prior to calling this function.
Deliang Fan [Fri, 24 Apr 2015 03:16:46 +0000 (11:16 +0800)]
Don't truncate osapi_volume_link prefixes
When osapi_volume_link_prefix is defined and used to update the
links return in API responses, do not drop the path component of
the overriding link prefix.
After the 3PAR drivers were refactored to remove the local
file locks, the login mechanism was logging the common and
client version numbers on every driver entry point.
This patch removes that logging except at driver startup.
John Griffith [Thu, 23 Apr 2015 15:05:57 +0000 (09:05 -0600)]
Sync oslo service module
This does a full sync of the oslo.service module. Note
that we've cherry picked some critical bug-fix changes
to this module already, this commit just syncs the full
module properly and gets us up to date where we should be.
Current HEAD in OSLO:
-----------------------
commit: d5edda00b4eca65d57f94bd0ac1b790e6d1f732e
Date: Wed Apr 22 19:49:00 2015 +0000
Merge "service child process normal SIGTERM exit"
Changes merged with this patch:
--------------------------------- d5edda00 - Merge "service child process normal SIGTERM exit" 702bc569 - service child process normal SIGTERM exit 64b5819e - Revert "Revert "Revert "Optimization of waiting subprocesses
in ProcessLauncher f5646edc - Revert "Revert "Optimization of waiting subprocesses
in ProcessLauncher d23b6589 - Revert "Optimization of waiting subprocesses in ProcessLauncher" 593005b7 - ProcessLauncher: reload config file in parent process on SIGHUP f29e865d - Store ProcessLauncher signal handlers on class level bf92010c - Optimization of waiting subprocesses in ProcessLauncher
NOTE: Commit 702bc569 was actually pulled in with commit d73ac96d .
We shouldn't have merged that individual commit. I include the
commit here to document Oslo level that the service module is at
cumulatively between that commit and patch.
John Griffith [Thu, 23 Apr 2015 18:07:12 +0000 (12:07 -0600)]
Add external genconfig calls
After moving to oslo.config we still were using
incubator config generator. This was ok, but the
problem is we haven't been pulling config options
from the oslo libs.
This is a hack that just appends external lib calls
and appends those options to the sample file being built.
Daniel Wilson [Wed, 22 Apr 2015 21:07:02 +0000 (14:07 -0700)]
Enable use of filter_function in PureISCIDriver
Adding filter_function to capabilities of PureISCIDriver. Return
total_volumes as well for use with filter_function.
This will open a way for admins to prevent Cinder from scheduling
volumes on a Pure backend once it has reached the array’s limits. The
primary use case for this is volume limits. An example of a
filter_function that will prevent this issue would be:
This will prevent any issues with Purity versions that are limited to
500 volumes.
Without that filter_function the scheduler will continue to try and
schedule volume creation if space is available instead of selecting
another backend. The end result would be volume creation fails and the
volume is in an error state even though there are backends available
that could have handled it.
DocImpact: This means the PureISCSIDriver will now respect the
filter_function config option. Special Pure specific fields to filter
off of need to be added to the driver config manual.
Alex Meade [Tue, 24 Feb 2015 21:22:58 +0000 (16:22 -0500)]
NetApp E-Series: Fix instance live-migration with attached volumes
Currently, live migrations of instances with attached volumes that live
on a NetApp E-Series backend will fail and break connectivity to the
guest. This patch adds the 'netapp_enable_multiattach' configuration
option that enables multiattach operations with the E-Series driver.
It defaults to allowing these operations but needs to be configurable
since allowing for multiple attachments imposes a limit of 256 volumes
on the backend due to how multiple attachments must be managed by
E-Series.
Multiattach operations are enabled by mapping volumes to an E-Series
host group on the backend called 'cinder-host-group'. Host groups can
only have 256 mappings at a time and so we must limit the number of
volumes in order to guarantee any volume created could then be attached.
John Griffith [Sat, 18 Apr 2015 00:19:37 +0000 (00:19 +0000)]
Add resource tag to logging in volume.manager.py
We now have resource tag support in oslo logging,
and our logging is pretty inconsistent and down right
ugly in places. Let's clean things up based on the
standard logging guidelines and use the fancy new
resource tag.
To use set the following in cinder.conf:
logging_context_format_string = \
%(asctime)s.%(msecs)03d %(levelname)s %(name)s [%(request_id)s \
%(project_name)s] %(resource)s%(message)s
This change hits the majority of the code in manager and
should be used as an example/guide. There are some exceptions
around migration and replication where things are kinda ugly,
those should be fixed up as follow up work.
Julien Danjou [Thu, 19 Mar 2015 12:24:00 +0000 (13:24 +0100)]
Leverage timeutils, drop strtime() usage
This patch remove some code that tried to parse time in different way by
leveraging timeutils instead. It also drops strtime() usage as it's
going to be deprecated in oslo_utils (see
I8b5119e64369ccac3423dccc04421f99912df733).
The logic on read-only volume in cinder/volume/drivers/emc/emc_vnx_cli.py
is duplicate to the logic in cinder/volume/manager.py. This patch is to
remove it.
John Griffith [Wed, 22 Apr 2015 04:28:34 +0000 (04:28 +0000)]
Remove force check from copy_volume_to_image
The upload_volume_to_image method allows a force parameter that will
upload a volume even though the volume is attached/in-use. A user can
get away with this with the LVM driver because the LVM backing is
local to the Cinder worker node that's pushing the bits to Glance.
The problem is, this only works for local storage, it won't work
with any iSCSI devices because they can't do multi-attach. Also,
the reason we required that a volume NOT be in-use for this
operation is because we have no way of keeping the guest Instance
from writing to the volume while we're uploading, and corrupting the data.
This has been exposed like this for several releases, so removing it
now likely would not be a good user experience. Instead, this
patch add a config option to enable/disable it (default is to
disable), and deployers can choose whether they would like to
allow the use of --force True or not.
DocImpact Disables the --force option to copy-volume-to-image and
introduces "allow_force_upload" boolean option to re-enable
Sean McGinnis [Tue, 14 Apr 2015 15:02:22 +0000 (10:02 -0500)]
Logging not using oslo.i18n guidelines (scheduler)
Multi-patch set for easier chunks. This one addresses
the scheduler cinder directory.
There have been quite a few instances found where the
i18n guidelines are not being followed. I believe this
has helped lead to some of the confusion around how to
correctly do this. Other developers see this code and
assume it is an example of the correct usage.
This patch attempts to clean up most of those violations
in the existing codebase to hopefully help avoid some of
that confusion in reviews.
Some issues address:
* Correct log translation markers for different log levels
* Passing format values as arguments to call, not preformatting
* Not forcing translation via six.text_type and others
Guidelines can be found here:
http://docs.openstack.org/developer/oslo.i18n/guidelines.html
Hacking checks will not be able to identify all violations of
the guidelines, but it could be useful for catching obvious
one such as LOG.info("No markers!").
service.py had some code where the child process would catch the
SIGTERM from the parent just so it could exit with 1 status rather
than with an indication that it exited due to SIGTERM. When
shutting down the parent doesn't care in what way the child ended,
only that they're all gone, so this code is unnecessary.
Also, for some reason this caused the child to never exit while
there was an open connection from a client. Probably something
with eventlet and signal handling.
John Griffith [Mon, 20 Apr 2015 21:38:22 +0000 (15:38 -0600)]
Move unit tests into dedicated directory
This patch moves all of the existing cinder/tests into
cinder unit tests. This is being done to make way for
the addition of cinder/tests/functional.
Yes, this is going to cause significant pain with
any changes that haven't merged behind it in terms
of rebase, but there's no real alternative. We have
to rip the band-aid off at some point, and early in L
seems like a great time to do it.
Ivan Kolodyazhny [Mon, 20 Apr 2015 19:53:14 +0000 (22:53 +0300)]
Move RBD calls to a separate threads
RBD is a python binding for librados which isn't patched by eventlet.
Making long-running tasks like removing big (~100GB, ~1TB) volumes
blocks eventlet loop and all cinder-volume service hangs
until it finished when rados_connect_timeout is disabled. It makes
cinder-volume services unavailable for a while.
This patch moves all rados calls to a separate python thread which
doesn't block eventlet loop.
RBD: Add missing Ceph customized cluster name support
It turns out '--cluster' is also needed when RBD driver talks to
ceph cluster using 'ceph' command (not via librados). This change
appends RBDDriver._ceph_args with '--cluster' when 'rbd_cluster_name'
config option is not None.
John Griffith [Fri, 17 Apr 2015 20:51:06 +0000 (20:51 +0000)]
Standardize logging in volume.api.py
We now have resource tag support in oslo logging,
and our logging is pretty inconsistent and down right
ugly in places. Let's clean things up based on the
standard logging guidelines and use the fancy new
resource tag.
This patch starts with the volume.api file as that's
'easy', so we can enforce things going forward and start
working out other files in future patches.
To use set the following in cinder.conf:
logging_context_format_string = \
%(asctime)s.%(msecs)03d %(levelname)s %(name)s [%(request_id)s \
%(project_name)s] %(resource)s%(message)s
VolMgr: reschedule only when filter_properties has retry
In the task flow for volume manager, create_volume tasks, volume gets
reschedule even when scheduler doesn't indicate so. The problem is
the flow should not only check 'allow_reschedule' and 'request_specs',
but also (more importantly) filter_properties['retry'], which is
populated by scheduler if schedule_max_attempts is set to > 1. This
checks was there before taskflow was introduced, but somehow the
migration missed the check for filter_properties['retry'].
This change adds back the check, so scheduler_max_attempts won't be
treated like scheduler_max_attempts = infinite.
This patch adds the fix that exists in the nova
libvirt volume code to use oslo_utils strutils
to mask passwords that might show up in debug log
messages.
Current RBD driver assumes ceph cluster name to be 'ceph', for
cluster has a different name, the driver won't be able to connect
to the cluster. This change add a new config option
'rbd_cluster_name' to address this issue.
This allows operations that do not conflict with each
other (i.e. are on different volumes) to run concurrently.
The prior locking scheme was too coarse and essentially
made the driver single-threaded.
This patch moves the implementation of the GlusterFS driver locking
scheme to the RemoteFS base driver so that other similar volume
drivers can use it.
Lucian Petrut [Fri, 27 Mar 2015 12:15:25 +0000 (14:15 +0200)]
Windows SMBFS: fix volume extend
The Windows SMBFS driver inherits the Linux SMBFS driver,
overriding Windows specific methods.
This commit Ic89cffc93940b7b119cfcde3362f304c9f2875df added the
volume name as an extra argument to the _do_extend_volume in order
to check if differencing images are pointing to backing files other
than the according volume disks.
Although this is not required on Windows, this method should accept
this extra argument in order to have the same signature as the
method it overrides. At the moment, this raises the following
exception:
Code Review / openstack-build / cinder-build.git / log