test.py was previously not in the tests
directory. This means that downstream packagers
of Cinder have to specifically exclude it from
the main Cinder package (which does not typically
include unit tests).
Move it under the cinder/tests/unit/ dir where it
should be, to clean this up.
Change-Id: I65c50722f5990f540d84fa361b997302bbc935c5
This adds usage of the flake8-import-order extension to our flake8
checks to enforce consistency on our import ordering to follow the
overall OpenStack code guidelines.
Since we have now dropped Python 2, this also cleans up a few cases for
things that were third party libs but became part of the standard
library such as mock, which is now a standard part of unittest.
Some questions, in order of importance:
Q: Are you insane?
A: Potentially.
Q: Why should we touch all of these files?
A: This adds consistency to our imports. The extension makes sure that
all imports follow our published guidelines of having imports ordered
by standard lib, third party, and local. This will be a one time
churn, then we can ensure consistency over time.
Q: Why bother. this doesn't really matter?
A: I agree - but...
We have the issue that we have less people actively involved and less
time to perform thorough code reviews. This will make it objective and
automated to catch these kinds of issues.
But part of this, even though it maybe seems a little annoying, is for
making it easier for contributors. Right now, we may or may not notice
if something is following the guidelines or not. And we may or may not
comment in a review to ask for a contributor to make adjustments to
follow the guidelines.
But then further along into the review process, someone decides to be
thorough, and after the contributor feels like they've had to deal with
other change requests and things are in really good shape, they get a -1
on something mostly meaningless as far as the functionality of their
code. It can be a frustrating and disheartening thing.
I believe this actually helps avoid that by making it an objective thing
that they find out right away up front - either the code is following
the guidelines and everything is happy, or it's not and running local
jobs or the pep8 CI job will let them know right away and they can fix
it. No guessing on whether or not someone is going to take a stand on
following the guidelines or not.
This will also make it easier on the code reviewers. The more we can
automate, the more time we can spend in code reviews making sure the
logic of the change is correct and less time looking at trivial coding
and style things.
Q: Should we use our hacking extensions for this?
A: Hacking has had to keep back linter requirements for a long time now.
Current versions of the linters actually don't work with the way
we've been hooking into them for our hacking checks. We will likely
need to do away with those at some point so we can move on to the
current linter releases. This will help ensure we have something in
place when that time comes to make sure some checks are automated.
Q: Didn't you spend more time on this than the benefit we'll get from
it?
A: Yeah, probably.
Change-Id: Ic13ba238a4a45c6219f4de131cfe0366219d722f
Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>
when cloning volume in DS8K, driver doesn't need to wait until
flashcopy finished in some cases, so this patch provides an option
in volume metadata that can be used to tell driver to wait for
flashcopy completed or not.
Implements: blueprint ds8k-async-clone
Change-Id: I51829e7667d3ac148978e1799cecfea1920f838f
Python's unittest framework assertTrue and assertFalse methods don't
check that the provided value is False or True, like the documentation
says:
Note that this is equivalent to bool(expr) is True and not to expr is
True (use assertIs(expr, True) for the latter).
According to the documentation we should try to avoid using it as much
as possible:
This method should also be avoided when more specific methods are
available (e.g. assertEqual(a, b) instead of assertTrue(a == b)),
because they provide a better error message in case of failure.
But in our current code we keep abusing its use in many ways, among
which we can easily find the following:
- Thinking it only matches False/True
- assertTrue(expected, actual) --> This will always be true as long as
bool(expected) is True, because actual is interpreted as the message to
return on failure.
- Incorrectly testing values, like checking for a tuple of the form
(True, {...}) instead of testing for True in the first position.
This patch fixes incorrect some of the incorrect uses of the methods and
overwrites default methods so that they behave the way many people think
they do:
- assertTrue(x) == assertIs(True,x)
- assertFalse(x) == assertIs(False,x)
This will hopefully help us prevent incorrect usage of the methods.
Change-Id: I1e8c82702932e6b6d940bd83d0a2d4494576a81b
failover_host is the interface for Cheesecake.
Currently it passes volumes to the failover_host
interface in the driver. If a backend supports both
Cheesecase and Tiramisu, it makes sense for the driver
to failover a group instead of individual volumes if a
volume is in a replication group. So this patch passes
groups to the failover_host interface in the driver in
addition to volumes so driver can decide whether to
failover a replication group.
Change-Id: I9842eec1a50ffe65a9490e2ac0c00b468f18b30a
Partially-Implements: blueprint replication-cg
This patch adds replication group capabilities APIs for
IBM-Storage drivers.
Add the following drivers APIs:
* Enable replication on group
* Disable replication on group
* Fail over replication
* Get replication error status
Depends-On: I4d488252bd670b3ebabbcc9f5e29e0e4e913765a
Implements: blueprint replication-cg-ibm-storage
Change-Id: Ia4af4dd011d569a3ac84387b37dcf2606da48fba
self.assertTrue(string) is fairly dangerous because
things like self.assertTrue('False') will pass.
Require a more specific assertion when passing a
string to assertTrue, which should help prevent mistakes.
Fix up tests that do this currently, some of which
appear to not be testing what they intended.
Change-Id: I0cd29b9f3247ae5b7b3c478b96970b73d69a72c0
For Pike, IBM storage driver needs to switch to generic volume group
from consistency group. Because IBM storage driver manages both
Spectrum Accelerate Family and DS8000 Family, this change will only
change the common code shared by these storages. Changes for these
storage systems will be in separate reviews.
Implements: blueprint ibm-storage-vg
Change-Id: Id9964f868c2dc14aaee37e4bcbba330fb4575492
Opening up the source code of ibm_storage driver. The driver supports
the IBM Spectrum Accelerate family (XIV, Spectrum Accelerate, A9000
and A9000R) and DS8000.
Change-Id: I1abc6062dd84eeb6868ebedfc7032ff41030741f
The xiv_ds8k driver now supports IBM XIV, Spectrum Accelerate,
FlashSystem A9000, FlashSystem A9000R and DS8000 storage systems.
Therefore it should be renamed to IBM Storage driver.
The actual change includes:
- text changes in comments and documentation
- changes in fields in cinder.conf
- change in variable names
- change in file names
Implements: blueprint rename-xiv-ds8k-driver
Change-Id: Ia53a60e865ddac21029ca8518b6a506cb1682c51
DocImpact
Eric Harney