Jay S. Bryant [Thu, 15 Jan 2015 00:16:32 +0000 (18:16 -0600)]
Move oslo.db to oslo_db namespace
This is the second in a series of changes to move to using
the new oslo_<library> namespace that is being used for
oslo libraries.
There is currently a shim in place that is allowing the old
oslo.<library> imports to work, but we need to be prepared for
when the shims go away. Thus, we need patches like this one to
move to the new namespace.
This patch also updates our hacking check to ensure that no instances
of oslo.db sneak back in.
Dunrong Huang [Wed, 14 Jan 2015 05:56:43 +0000 (13:56 +0800)]
Fix eqlx endless loop when server closes the connection
The eqlx driver would identify one CLI command completion by looking
for the system name prompt in the output ("ARRAY_NAME>"). But some
cases where there is no "ARRAY_NAME>" in the output. The eqlx server
closes the ssh connection immediately after sends such message, which
lead to a infinite loop in _get_output() and 100% CPU utilization.
What the patch does is to check the return of chan.recv(), if the
length of returned string is zero, which means server has closed the
connection, the loop will be exited.
Curt Bruns [Thu, 15 Jan 2015 18:35:05 +0000 (11:35 -0700)]
Increase unit test coverage in hacking test
Added a few unit tests to increase the test coverage from
~60% up to 90%. Also modified the assertEqual of all tests
to be in the form of assertEqual(EXPECTED, ACTUAL) format.
Mitsuhiro Tanino [Thu, 15 Jan 2015 00:59:54 +0000 (19:59 -0500)]
Add mock for cinder-rtstool call in tests.targets.test_lio_driver
Unit tests in tests.targets.test_lio_driver are calling out to
cinder-rtstool.
change-id Ic25828361d9c30c4e1aa37b630d8ad272a78977b skips this
unit test to avoid failure of tests.
This patch reverts this change and adds mock for calling of
cinder-rtstool to reenable the unit tests.
Corey Bryant [Wed, 14 Jan 2015 18:11:30 +0000 (13:11 -0500)]
Deal with PEP-0476 certificate chaining checking
PEP-0476 introduced more thorough certificate chain verfication
for HTTPS connectivity; this was introduced in Python 2.7.9, and
breaks a number of unit tests in the cinder codebase.
Disable certificate chain verification for cinder SSL tests
using the backwards compatible SSLContext provided for this
purpose.
Jay S. Bryant [Tue, 13 Jan 2015 23:11:33 +0000 (17:11 -0600)]
Add hacking check for oslo namespace usage
We want to make sure that we don't have usage of the old
oslo.concurrency naming slipping in with new changes where
we should be using the oslo_concurrency namespace. This change
adds a hacking check to avoid use of the deprecated namespace.
As we convert more oslo libraries to the new namespace the check
will be updated to enforce use of the new namespace.
This hacking check is based upon the same N333 hacking check
in Cinder.
Anthony Lee [Fri, 12 Dec 2014 19:32:15 +0000 (11:32 -0800)]
Remove locks from LeftHand driver
By removing synchronization locks from the LeftHand driver,
support for many simultaneous volume create/deletes and
volume attach/detachs will be added. This is useful for high
availability environments where performance with locks in place
would be reduced.
Removing the locks requires the creation of a new LeftHand
client when a request is sent to the driver now. This
allows requests to no longer be blocked if a request is
already in process by the driver.
This results in a performance increase for the driver and
better stability in a high availability environment.
Twelve of the the storwize_svc unit tests started failing with the
complaint that the required configuration option 'lock_path' was not set.
This change updates cinder/test.py to not only set up a temporary
lock_path location that will be automatically cleaned up, but to also
register that temporary lock_path location using lockutils.set_defaults()
to correct the problem.
Rushil Chugh [Mon, 8 Dec 2014 22:09:36 +0000 (17:09 -0500)]
Garbage Remains when Attached Volume is Migrated with NFS Driver
Garbage volume remained when an attached NFS volume was migrated
to another NFS backend. An error message was generated as the
attached volume was not detached before being migrated. The root
cause of the issue is that status_update['status'] is only set
for retype and not for migration from one NFS backend to another
NFS backend. This patch proposes to fix the aforementioned issue
where migration fails from one NFS backend to another NFS backend.
Mitsuhiro Tanino [Sat, 10 Jan 2015 01:11:43 +0000 (20:11 -0500)]
Remove unused variables from ensure_export()
After introducing commit 9651f547147188645942466602c92cce06666483,
these three variables are obsolete in ensure_export().
By remaining these variable, ensure_export() fails to export
volumes after c-vol service is restart because the caller of
ensure_export() in lvm driver does not pass these variables.
- iscsi_name, volume_group, config
Main clean up is done by commit Ibfd1feefd72c43ef316b267e9d6645f2e67e2558.
This patch adds some tests for tgt, lio, and iser targets.
And also adds initialization of chap_auth_userid and chap_auth_password
in create_iscsi_target() because these variables might be referred
before definition.
John Griffith [Fri, 9 Jan 2015 23:58:06 +0000 (23:58 +0000)]
Fix incorrect usage of get_flow in volume.manager
The call to flows.manager.create_volume.get_flow in cinder.volume.manager's
create_volume method is passing in keyword args where the flows manager is
actually expecting positional arguments. The good thing is that most of these
seem to evaluate to boolean values and are evaluated as True, so they don't
fail; however the behavior isn't going to be exactly as expected here.
The method signature in flows.manager.create_volume:
John Griffith [Wed, 7 Jan 2015 05:47:01 +0000 (22:47 -0700)]
Fix iscsi_write_cache setting for iscsi targets
While transitioning to the new driver and target
model (change 9651f547147188645942466602c92cce06666483)
some things got lost not surprisingly. One of those
things that wasn't migrated correctly was the
iscsi_write_cache option due to ensure_export not
being called properly on service restart.
This patch cleans up the ensure/create_export methods
and their signatures. Most importantly esnure_export
should now actually work and check the iscsi_write_cache
settings.
While fixing this I also cleaned up the two methods
mentioned above to eliminate the unnecessary and
duplicate info in their arguments.
John Griffith [Fri, 9 Jan 2015 00:42:05 +0000 (17:42 -0700)]
Add debug messaging for tgt already exists
We have a race condition where the call to create an
iscsi target seems to be issued twice. This patch
adds some debug logging to try and get some more info
around where things are going wrong.
First, we add a log message to the intialize_connection call
in the cinder.volume.api and include the connector info.
Second, we add a target show output prior to the update/create
command in the target driver.
In addition, we also had the update and show commands in the same
try/except block which led us to believe the show was failing,
but that's NOT the case at all. So this patch also moves that
existing show (debug purposes as well) out of the main try block.
Note that there are some old bugs logged against TgtAdm for issues
with it "loosing count" of target id's it's issued, and in turn
reissuing the same ID multiple times. This patch is intended to
give us enough info to determine if that's what's going on or not.
Other possibility is that the initialize call is being issued more
than once.
Sean McGinnis [Fri, 9 Jan 2015 15:42:48 +0000 (09:42 -0600)]
Clean up QoSSpecManageApiTest setup
There is currently unnecessary work being performed in setUp
for QoSSpecManageApiTest and confusing comments. This cleans
that up to avoid confusion.
Test was explicitly calling a reset on a fake, but the cleanup
for each test will do this as well. Relatively harmless, but
this makes the code cleaner.
John Griffith [Wed, 7 Jan 2015 23:25:48 +0000 (16:25 -0700)]
Use cinder.utils.execute directly
When we were in the process of working on brick libs
we did quite a bit of funky stuff with setting a member
variable for each class to point to an executor that was
passed in during init.
There's no longer any reason to do this with the target drivers,
so we should simplify our lives a bit and go back to using the
good old cinder.utils wrapper.
It's also believed that the use of the member variable is
susceptible to some concurrency issue that was causing the
wrong cmd string to be executed. We'll mark this as closing
that bug and reopen if we still see the signature in Kibana.
John Griffith [Wed, 7 Jan 2015 22:04:07 +0000 (15:04 -0700)]
Deal with tgt already exists errors
So there's a major problem in the iscsi code (and has been
for quite some time). The tgt show command sometimes seems
to be corrupt and the issued command is actually a tgt create.
The result is that we already have a tgt and the call raises
which then causes the operation to fail.
An example of the issue:
Stdout: Unexpected error while running command.Command:
sudo cinder-rootwrap /etc/cinder/rootwrap.conf tgt-admin
--update iqn.2010-10.org.openstack:volume-f055d3c5-db7a-
484e-9d0d-b98495439413
Exit code: 22
Stdout:
Command:tgtadm -C 0 --lld iscsi --op new --mode target
--tid 1 -T iqn.2010-10.org.openstack:volume-f055d3c5-db7a-
484e-9d0d-b98495439413
exited with code: 22.
Stderr: u'tgtadm: this target already exists
What's disturbing however is that in that section of code
we're sending a --op show!! Could be something we're
doing with our member executor? Or maybe something to
do with the new oslo concurrency code?
Regardless, his patch intends to provide a clear marker for
ER in the case that create export fails due to the
target entry already existing.
Also this patch will enable us to go ahead and just use
the existing target rather than bomb out and fail everything.
Root cause of why we're getting a second create is still
unknown and needs addressed, but this might help in getting
more info as well as keeping things stable until we address
the root issue.
John Griffith [Thu, 8 Jan 2015 00:21:42 +0000 (00:21 +0000)]
Fix drbd driver to load without 3'rd party libs
The new drbd driver has a couple needed 3'rd party libs
that aren't in the global requirements. That means that
any time there's an attempt to import the driver things
blow up due to import errors.
The great thing is that the author took care of things in
unit tests by creating mocks, but we still have the problem
of the driver itself, and this causes us to be unable to do
things like genconfig.
This patch just wraps the 3'rd party libs into a try/catch
block, sets them to None if they're not on the system.
Also, move the global dbus.array variable into init and
make it a member variable.
Tomoki Sekiyama [Wed, 7 Jan 2015 22:33:51 +0000 (17:33 -0500)]
cinder-rtstool: should use acl.node_wwn
In newer version of rtslib module, NodeACL is now an object, not a
dict. Thus, acl.node_wwn must be used instead of acl['node_wwn'],
otherwise it will fail with exception.
Tomoki Sekiyama [Wed, 7 Jan 2015 22:39:33 +0000 (17:39 -0500)]
LVM: Add terminate_connection call for Target Objects
terminate_connection should be redirected from LVM driver to targets
object. Especially this is required in LioAdm to remove an initiator
from a target. This also fixes some mismatch of method arguments and
remove unused methods in target objects.
Having an instance and an attached volume on the same physical host
(i.e. data locality) can be desirable in some configurations, in order
to achieve high-performance disk I/O.
This patch adds an InstanceLocalityFilter filter that allow users to
request creation of volumes 'local' to an existing instance, without
specifying the hypervisor's hostname, and without any knowledge of the
underlying back-ends.
In order to work:
- At least one physical host should run both nova-compute and
cinder-volume services.
- The Extended Server Attributes extension needs to be active in Nova
(this is by default), so that the 'OS-EXT-SRV-ATTR:host' property is
returned when requesting instance info.
- The user making the call needs to have sufficient rights for the
property to be returned by Nova. This can be achieved either by
changing Nova's policy.json (the 'extended_server_attributes' option),
or by setting an account with privileged rights in Cinder conf.
For example:
Instance 01234567-89ab-cdef is running in a hypervisor on the physical
host 'my-host'.
To create a 42 GB volume in a back-end hosted by 'my-host':
cinder create --hint local_to_instance=01234567-89ab-cdef 42
Note:
Currently it is not recommended to allow instance migrations for
hypervisors where this hint will be used. In case of instance
migration, a previously locally-created volume will not be
automatically migrated. Also in case of instance migration during the
volume's scheduling, the result is unpredictable.
DocImpact: New Cinder scheduler filter
Change-Id: Id428fa2132c1afed424443083645787ee3cb0399
Anthony Lee [Mon, 8 Dec 2014 14:52:18 +0000 (06:52 -0800)]
Add driver filter and evaluator for scheduler
This patch adds a new filter for the cinder scheduler that
can interpret two new properties provided by backends,
'filter_function' and 'goodness_function'. A driver can rely
on cinder.conf entries to define these properties for a backend
or the driver can generate them some other way. An evaluator is
used by the filter to parse the properties. The 'goodness_function'
property is used to weigh qualified backends in case multiple ones
pass the filter. More details can be found in the spec:
https://review.openstack.org/#/c/129330/
Implements: blueprint filtering-weighing-with-driver-supplied-functions
DocImpact: New optional backend properties in cinder.conf.
New filter and weigher available for scheduler.
Change-Id: I38408ab49b6ed869c1faae746ee64a3bae86be58
John Griffith [Tue, 6 Jan 2015 21:40:00 +0000 (14:40 -0700)]
Remove import of private _lazy module
New version of oslo.i18n released and some things
moved around (internal private modules in the lib).
This should be fine and shouldn't matter to us, BUT
it seems we had some hackery going on in our unit tests
that were being lazy and importing and manipulating the
private library.
This patch removes those cases and fixed up the cinder.i18n
helper method for enable_lazy to accept a bool (which is how
the libraries method works to begin with).
There were several tests in test_faults that were actually
performing and comparing translations of messages. These
tests aren't quite working now because they had a number
of things they imported from private variables and methods
in the i18n module. Honestly I'm not sure of the value of
testing those things here anyway, but for now I've just
added a skip to those and we can sort out long term fixes
and plans later.
Accela Zhao [Tue, 6 Jan 2015 05:36:35 +0000 (13:36 +0800)]
Fix the continuation line indent to pass flake8
The flake8 complains "E128 continuation line under-indented for
visual indent" on cinder/tests/api/contrib/test_types_manage.py
line 270. The continuation line should be indented to the opening
parentheses. This patch fixes the indention.
Fix argument order in assertEqual: tests/test_glusterfs.py
The assertEqual in cinder/tests/test_glusterfs.py is using
incorrect argument order (observed, expected), which causes the
error message about mismatch to be reversed if the test case fails.
Change it to (expected, observed).
John Griffith [Tue, 18 Nov 2014 00:46:54 +0000 (00:46 +0000)]
Transition LVM Driver to use Target Objects
This patch refactors the LVM Driver to take a
seperate Target object instead of mixing the
control and data path implementations inside the
driver itself.
It removes the volume/iscsi.py and brick/iscsis/*
files which were duplicating code and actually
very messy in terms of where calls were actually
being implemented.
Fix handling of serialized data in filtering of volumes
Commit 4aaf40ba1aab4d7c347b05750d0fe21f8d1bcc68 has introduced a bug.
'ast.literal_eval(v)' throws exception 'SyntaxError' if 'v' is an expression
such as UUID '920da701-93c1-4178-9f1a-ef1c7a8a384d', 'd-', or 'd+'.
Catch the 'SyntaxError' exception in addition to 'ValueError' and
assume 'v' is a string. So the API can handle the request successfully
rather than returning a '500' error code.