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.
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.
Adrien Vergé [Fri, 5 Dec 2014 15:09:10 +0000 (16:09 +0100)]
Add an instance-locality filter
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.
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.
Adrien Vergé [Tue, 28 Oct 2014 19:35:54 +0000 (20:35 +0100)]
Add a privileged user for OpenStack services
Currently Cinder makes all requests to other services (Nova, Swift,
etc.) with current user context. Sometimes Cinder needs privileged
rights for external queries (e.g. asking Nova where an instance is
hosted); there is no way to do it yet.
This patch adds to ability to configure an account with special rights
in the configuration ('os_privileged_user_name',
'os_privileged_user_password' and 'os_privileged_user_tenant' options).
Then, requests that need special permissions can be achieved by creating
a client(privileged_user=True).
Note: This user does not necessarily need to have an admin role
associated with it. For instance, policies can be changed to allow a
specific user (without any roles) to perform special actions.
DocImpact: New configuration options to set a privileged user account
Change-Id: I61d8a6de1c5db5ee2ecce124997f9b6447b04e47
Patrick East [Fri, 5 Dec 2014 02:51:06 +0000 (18:51 -0800)]
Add support to PureISCSIDriver for Consistency Groups
This change adds implementations for the required driver methods for
Consistency Groups to be supported with the PureISCSIDriver. There is a
nice direct mapping between Consistency Groups and Purity Protection
Groups which makes the implementation pretty straightforward.
Tom Fifield [Wed, 24 Dec 2014 14:38:32 +0000 (23:38 +0900)]
Expand the description of extra_capabilities
The existing description of extra_capabilities was a bit terse,
meaning deployers couldn't easily gauge the power of this option.
This patch expands the textual description using information from
the blueprint and original commit message, so it can be picked
up automatically by the documentation.
John Griffith [Tue, 23 Dec 2014 00:05:01 +0000 (17:05 -0700)]
Fix broken StorPool driver
The newly added StorPool driver has a number of issues;
It imports it's own 3'rd party libs which are not in the
requirements file, it declares conf options but never registers
them, and as a result of these two things it break the ability
to generate a configuration file.
This patch adds a try_import around the import storpool calls
like we do in other drivers, and it registers the config options
properly.
We also move the api setting out of init and into the check_setup
so the service doesn't crash if somebody tries to load the driver
without the required storpool modules.
Eric Harney [Mon, 17 Nov 2014 20:01:02 +0000 (15:01 -0500)]
Brick LVM: Remove self.lv_list
This member isn't used anywhere, and now that requests for
specific LVs are overwriting self.lv_list with a subset of
the LVs that are present, it doesn't contain coherent info
anyway.
This commit broke the ability to create Iscsi targets w/
cinder-rtstool. Everything seems to be working fine except there
are issues with regards to how rtstool output is parsed in that
any output (regardless of exit status) causes the command
to be marked as failed.
Suggesting this as a quick revert until we can investigate
the proper fix for error handling with cinder-rtstool.
Clinton Knight [Fri, 12 Dec 2014 20:30:26 +0000 (15:30 -0500)]
Replace the NetApp driver proxy layer with a proper factory.
The proxy implementation required a hack in the Abstract Base
Classes project in Cinder core. This factory implementation
removes the need for the hack. It also improves logging, as
the actual driver object is provided to the volume manager
instead of the driver proxy.
Xing Yang [Fri, 19 Dec 2014 17:45:45 +0000 (12:45 -0500)]
Handle the volume not found case in the VMAX driver
When a volume cannot be found on the array, VMAX driver throws
an exception. This brings a volume into error_deleting state during
the volume deletion operation. This patch fixed it by returning None
when a volume cannot be found.
John Griffith [Fri, 19 Dec 2014 19:25:30 +0000 (19:25 +0000)]
Fix format errors in brick/iscsi LOG messages
Currently in cinder.brick.iscsi.iscs:LioADM.create_iscsi_target
The Log message for the exception is:
LOG.error("%s" % e),
This rightfully results in:
*** UnicodeError: UnicodeError(u'Message objects
do not support str() because they may contain
non-ascii characters. Please use unicode() or
translate() instead.',)
In some cases this causes the Volume service to stop and
doesn't help a ton with debug. While looking at this also
noticed a number of other similar cases where invalid LOG
messages were set up. Following the i8n guidelines here:
http://docs.openstack.org/developer/oslo.i18n/guidelines.html
Went ahead and cleaned the bulk of the LOG messages in this
file up to adhere to the guidelines as well as fixing the
UnicodeError described in the bug.
Andrew Kerr [Tue, 25 Nov 2014 20:47:01 +0000 (15:47 -0500)]
Add unit tests for NetApp do_setup methods
We need to ensure that our do_setup methods are properly calling check_flags.
This patch adds tests to that effect. We also need to ensure that our config
documentation is accurate; this also fixes a minor issue for that.
John Griffith [Sat, 20 Dec 2014 00:45:16 +0000 (17:45 -0700)]
Set iet_conf to nonexistent file in unit test
The Iet test in test_iscsi isn't setup to deal with
an actual iet.conf file. The result is that if you happen
to be on a system that has an iet.conf file the test will fail
when it gets to the os.stat check.
This patch just uses a tempdir with a bogus file name that is
set as the iet_conf option to make sure the test never runs into
a situation where the file is actually there.
Avishay Traeger [Thu, 18 Dec 2014 13:55:26 +0000 (14:55 +0100)]
Fix issue with passing lists in filters
The Cinder db code already allows searching for a value in a list,
for example, id in ['id1', 'id2', 'id3']. The problem is when we
get the value at the API it comes as a string (e.g.
"['id1', 'id2', 'id3']", and therefore the filter fails. This was
already fixed for metadata filters, so just apply it to all filters.
We do a try/except because if it really is a string, then the
literal_eval will fail because we don't pass double quotes, such as
"'string'". The current code therefore will now cover lists and
fall back to the original behavior for strings.
Code Review / openstack-build / cinder-build.git / log