Pass autospec=True while patching the function.
Create the mock object for mock_helper using mock.sentinel. This gives
us some extra protection because sentinel object is not callable. If the
code tries to use it like a function, it will raise a TypeError.
This patch also changes assertFalse() to assert_not_called().
Partial-Bug: #2004174
Change-Id: I209ff5d0fd7e3eaa0b50dcbf71ff4b0960403f96
Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
This patch updates how cinder calculates it's free capacity.
The new calculations are based off of the queens cinder specs
that describes each of the capacity factors here:
https://specs.openstack.org/openstack/cinder-specs/specs/queens/provisioning-improvements.html
This patch updates the capacity filter to use the new capacity factors
calculations, which is also used by the capacity weigher.
The new calculate_capacity_factors describes each of the factors and
returns a dictionary of each of the factors as calculated.
Change-Id: Ic1b5737281e542d2782089a369e4b7941fc3d921
File locks are never removed from the system, so they keep increasing in
the locks directory, which can become problematic.
In this patch we start trying to delete these lock files when we delete
a volume or a snapshot.
This affects the 2 type of file locks we currently have:
- Using oslo lockutils synchronized with external=True
- Using coordination.synchronized when deployed in Active-Passive and no
DLM
This will alleviate the ever increasing files in the locks directory.
Deployment tools should implement a service that runs when the host is
booting and cleans of the locks directory before the OpenStack services
are started.
Partial-Bug: #1432387
Change-Id: Ic73ee64257aeb024383c6cb79f2e8c04810aaf69
We currently only validate the existence of a project on
the default types code so that we cannot set it for non
existing projects, but there are other places where we would
benefit from having this validation, such as volume type access,
setting quotas.
To achieve this we are moving methods from quota_utils and default_types
to api_utils.
This patch also modifies the method get_project_hierarchy to get_project
and removes related hierarchy tests since with the removal of nested quota
driver we don't require the hierarchy properties anymore.
Related-Bug: #1638804
Change-Id: I20f8e500f583eb85f24a4c36f344c11c7face06c
Sometimes we get an unexpected failure on the lvs command where it exits
with code 139, which is not one of the 5 predefined ones in the code
ECMD_PROCESSED, ENO_SUCH_CMD, EINVALID_CMD_LINE, EINIT_FAILED, or
ECMD_FAILED.
We've seen this happen mostly on CI jobs when deleting a volume, on the
check to see if the volume is present (``_volume_not_present``) and
makes the whole operation unexpectedly fail.
When looking at the logs we can find other instances of this same exit
code happening in other calls, but those are fortunately to be covered
by retries for other unexpected errors such as bug #1335905 that seem to
make the call eventually succeed.
The stderr of the failure is polluted with debug and warning messages
such as:
[1] /dev/sda1: stat failed: No such file or directory
This has been removed [2] from the LVM code indicating it's
somewhat troublesome, but doesn't explain how.
[3] Path /dev/sda1 no longer valid for device(8,1)
[4] WARNING: Scan ignoring device 8:0 with no paths.
But the real error is most likely:
[5]: Device open /dev/sda 8:0 failed errno 2
On failure we see that error twice, because the code retries it in LVM
trying to workaround some kind of unknown udev race [6].
Since the LVM code indicates that a retry can help, we'll retry on error
139 when calling ``get_lv_info``.
To narrow down the retry we'll only do it on error 139, so we modify the
existing ``retry`` decorator to accept the ``retry`` parameter (same as
the tenacity library) and create our own retry if the
ProcessExecutionError fails with a specific error.
This pattern seems better than blindly retrying all
ProcessExecutionError cases.
[1]: 17f5572bc9/lib/filters/filter-persistent.c (L132)
[2]: 22c5467add
[3]: b84a9927b7/lib/device/dev-cache.c (L1396-L1402)
[4]: b84a9927b7/lib/label/label.c (L798)
[5]: b84a9927b7/lib/label/label.c (L550)
[6]: b84a9927b7/lib/label/label.c (L562-L567)
Closes-Bug: #1901783
Change-Id: I6824ba4fbcb6fd8f57f8ff86ad7132446ac6c504
This continues moving code only used by volume
drivers or the volume manager to volume_utils.
The result of this is that fewer drivers import
cinder.utils (and everything it imports), and less code
is loaded by non-volume cinder code (api service,
cinder-manage, etc.)
Closes-Bug: #1912278
Change-Id: I5aa82cf0de4c70f53ddc998bf25e64f4ad8f5774
Move code that calls out to os-brick for connectors,
encryptors, etc., into volume_utils.
This leaves cinder/utils.py more general-purpose
cinder-wide code, which reduces unnecessary binding
between things like "cinder-manage db" and calls
that load much more of cinder's code (and external
libraries like os-brick).
This also means that some drivers only need to
import volume_utils and not cinder.utils.
Partial-Bug: #1912278
Change-Id: Ib2e2960ca354a47d303e0633c7d84e6da4b55b82
CinderException can be used as a low-level exception so we don't need to
introduce one more wrapper for Exception.
TSM backup driver modified without testing. Removed code was broken
because it tried to use not existing Exception class attributes.
Change-Id: Ie67d919f8f59572268f1f07946fb2d2f14a851f1
This patch adds the backup_max_operations configuration option that
limits the number of concurrent backup and restore operations.
These operations can consume large amounts of RAM, so we should limit
them according to our system resources to avoid processes being killed
because we run out of memory.
The default is 15 concurrent operations, which could use up to 85GB
of RAM(in case of ceph which uses the maximum memory for this operation).
More details are added in the document.
Closes-Bug: #1865011
Co-authored-by: Rajat Dhasmana <rajatdhasmana@gmail.com>
Change-Id: Ica4e0cea67bc894f61a57c7898977951ce3a3633
When switching from the retrying module to tenacity, we had to do some
hacking to get the internal sleep calls for tenacity to not actually
sleep during our unit tests. This required a time critical aspect where
we had to mock sleep at the time tenacity was first loaded.
This worked fine for local changes, but when consuming library code that
also uses tenacity, like the new os-brick release, the library would end
up loading tenacity before our test code had a chance to do this
mocking.
Turns out that the tenacity devs actually did add a mechanism for
handling this after all. This simplifies all that mocking to use the
sleep kwarg into the tenacity initiator that we can then use to mock out
the sleep call in a more sane way.
Change-Id: Ida0192b2dc5b782bc2ee33746cc63da536c0a0e0
Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>
The retrying library is no longer maintained and users are encouraged to
migrate to tenacity.
This has a small behavior change in that before we were applying an
exponential backoff to the first time a retry was needed. This no longer
happens, but retries will exponentially back off with subsequent each
retry.
Change-Id: I25ca007386a7b8ca6a7d572742334ba3d7dcbf1e
Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>
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>
Much of our code renames this at import already --
just name it "volume_utils" for consistency, and
to make code that imports other modules named "utils"
less confusing.
Change-Id: I3cdf445ac9ab89b3b4c221ed2723835e09d48a53
Move code into more specific locations where it is
applicable rather than the Cinder-wide utils.py.
Change-Id: I87f80f041cec255d51145e39bc0c0781a81e6db8
This collects up utils that are only used in the API code,
and moves them from cinder/utils.py to here, for better code
organization.
Change-Id: Iecd909ef4e24d5597dcccb80a671a27f361e4b4d
The utils.trace helper is logging the args list to
the decorated function but is not masking passwords
in those args. This change adds a call to mask passwords
in the function args list.
Synchronize from os_brick.utils.trace.
Change-Id: I1c0b5d22258416b7cd2d84fafc339a1308ff34ab
This improvement allows trace_api decorators to be
passed a filter function argument, which can validate
the parameters of the object being logged to decide
to not filter it.
Change-Id: I2dbf5583b8d2c9b9afe35bc5a916f480637d79f5
Two issues prevented running our unit tests on macOS. Mocking
os.stat appears to cause issues internally when running tests,
and trying to load the Linux-only rtslib_fb package would
prevent even loading the source to find tests.
This mocks out packages to allow the code to be loaded and
skips tests that have issues on macOS to at least allow the
other tests to be run.
Change-Id: I1d2cfd0d68796e1af5344b481e610c2070e41093
This patch makes a few small changes that are required in order to
have the Cinder Backup service working on Windows.
- all physial disks must be open in byte mode. 'rb+' must be used
when writing.
- reading passed the disk size boundary will not return an empty
string, raising an IOError instead. For this reason, we're avoiding
doing it.
- we ensure that the chunk size is a multiple of the sector size.
- the chmod command is not available on Windows. Although changing
owners is possible, it is not needed. For this reason, the
'temporary_chown' helper will be a noop on Windows. It's easier to
do it here rather than do platform checks wherever this gets called.
- when connecting the volumes, we pass the 'expect_raw_disk' argument,
which provides a hint to the connector about what we expect. This
allows the SMBFS connector to return a mounted raw disk path instead
of a virtual image path.
- when the driver provides temporary snapshots to be used during
the backup process, the API is bypassed. For this reason, we need to
ensure that the snapshot state and progress gets updated accordingly.
Otherwise, this breaks the nova assisted snapshot workflow.
We're doing platform checks, ensuring that we don't break/change
the current workflow.
The Swift and Posix backup drivers are known to be working on Windows.
Implements: blueprint windows-smb-backup
Depends-On: #I20f791482fb0912772fa62d2949fa5becaec5675
Change-Id: I8769a135974240fdf7cebd4b6d74aaa439ba1f27
This is part of the effort to improve Cinder's Thin provisioning
support. As some operators have been facing problems to determinte
what is the best value for the max_over_subscription_ratio, we
add in this patch a mechanism to automatically calculate this value.
The formula used for calculation is:
max_over_subscription_ratio = 20 if provisioned_capacity_gb == 0 else:
max_over_subscription_ratio = 1 + (provisioned_capacity_gb/(
total_capacity_gb - free_capacity_gb + 1))
Using this formula, the scheduler will allow the creation of a much
bigger number of volumes at the begginning of the pool's life, and
start to restrict the creation as the free space approaces to 0 or
the reserved limit.
Drivers now can set max_over_subscription_ratio = 'auto' and take
benefit of the change. Drivers that somehow use the
max_over_subscription_ratio inside the driver to do any kind of
calculations are incompatible with this new feature and should
get fixed in order to be able to use the feature.
Implements: bp provisioning-improvements
Change-Id: If30bb6276f58532c0f78ac268544a8804008770e
Despite comments stating otherwise, we set our root logging level
to DEBUG. This causes some unwanted things to be logged to the
console and large test artifacts.
There is also some suspicion that this is causing test failures
with some kind of timing change with stestr causing subunit
parsing failures.
This changes the default level to be higher and fixes a few test
cases that had expected the previous default.
Change-Id: Ib7e45915898347549c12bc8276df556b272bb259
Closes-bug: #1728640
While Windows paths are case insensitive, some checks performed in the
SMBFS driver as well as its parent class are not, breaking the driver's
logic.
We're most affected by the fact that some of the Windows API functions
disregard path case sensitivity, always returning uppercase strings in
some situations.
This change adds a helper function that compares paths having the os
type in mind and uses it throughout the remotefs and smbfs modules.
Change-Id: Icfef1c8b9ae011d2b463aa5c1fb7f512388c8f88
Closes-Bug: #1694648
In both v2 and v3, when create/update a volume/snapshot
with metadata, if the metadata is not a dict, Cinder
will raise 500 error:
AttributeError: 'int' object has no attribute 'items'
This patch added metadata format check.
Change-Id: I0791639c781e070cc0568f2ddf50b1a44d8ef42b
Closes-bug: #1696637
This patch adds 2 new APIs for microversion 3.32, one to dynamically
change the log level of cinder services, and the other that allows
querying their current log levels.
DocImpact
APIImpact
Implements: blueprint dynamic-log-levels
Change-Id: Ia5ef81135044733f1dd3970a116f97457b0371de
We recently introduced a mechanism to short-circuit notifications when
they were disabled, unfortunately the mechanism was not backward
compatible with deprecated oslo notification setup that defaults to use
the transport mechanism of RPC.
In Mitaka oslo messaging introduced the transport_url specific for
notifications under the oslo_messaging_notifications section, but some
projects still use the default transport_url defined in the DEFAULT
section.
This patch fixes the notification short-circuit, now we don't check the
transport_url but the driver option in the oslo_messaging_notifications
section, which is a more robust way of checking, since it is backward
compatible.
Change-Id: Iccf51756701759e3727d3d989bab08bc05cac9a7
Closes-Bug: #1660928
When running Cinder with notifications disabled we are still doing all
the work and calls to Oslo message notifier and it's there, at the very
last moment when it's going to send the message that it checks that
there's no actual extension loaded in the driver manager and skips the
actual send of the data.
This is considerably wasteful considering that for some of the
notifications we are actually querying the DB to get data, for example
volume attachments and glance metadata information when notifying about
volume usage.
This patch proposes short-circuiting notification methods as much as
possible to optimize code execution when Cinder has no notification
transport mechanism configured, as is the case when deployed as a
standalone SDS service.
Closes-Bug: #1660303
Change-Id: I77f655d3ef90088ce71304da5d4ea7b543991e90
Oslo.utils provides same method is_valid_boolstr,
in oslo.utils 3.17,and now we are in oslo.utils>=3.17
,so we should use it directly.
Oslo_utils's is_valid_boolstr accepts "t" and "f" ,
"on" and "off",but Cinder didn't.
The rest api in following will be influenced by this:
API: cinder type-update Parameter: is-public
cinder type-create is-public
cinder snapshot-create force
get_bool_param() param_string
cinder backup-list all-tenants
and unimplemented v3 api:group_types update function, and
corresponding parameter is is_public.
Change-Id: I0326e586fe6c402e3425fa65656821054e560e40
Add to objects.service.Service is_up property instead of
cinder.utils service_is_up function and remove utils's
function.
Also for consistency make objects cluster's method is_up as a property.
Refactor code and tests according to these changes.
Change-Id: I12045e651e92cc9727d981777a98f1c7b82c46f3
Following OpenStack Style Guidelines[1]:
http://docs.openstack.org/developer/hacking/#unit-tests-and-assertraises
[H203] Unit test assertions tend to give better messages for more specific assertions.
As a result, assertIsNone(...) is preferred over assertIs(None, ...)
Change-Id: Ia53a42e0c6a60a9050887e1f4600ece7ab7c1328
setUp and tearDown will be automatically called around each
testcase, so this is to remove setUp and tearDown that doing
nothing additional than super to keep code clean.
Change-Id: I80ae55cc36c473e8ed7fc7f4cbd946107bf187b4
To create encrypted volume from image, or retype a volume
to different encryptions, Cinder needs to attach/detch encrypted
volume.
This patch is the first patch which adds encryptor attach/detach
functions in utils so that later it is called to resolve above problems.
Change-Id: I5b6eba7e5da54cb386bf597bf100db106c7a1b06
Implements: blueprint improve-encrypted-volume
This patch fixes an issue where the method tracing was returning the
masked password, instead of the original return result.
Closes-Bug: 1616527
Change-Id: I524347d9c9b7640d9bb5b8fe5f6a04b901e45e6c
This patch ensures that input and return parameters that are being
traced mask out passwords to the log file.
Change-Id: I71baa6ee7b2a474701a1caed99d4889fffc62eca
If a driver reports thin_provisioning_support to True and
thick_provisioning_support to True on a pool that supports
both thin and thick volumes, and the user wants to create a
thick volume, the current logic in the scheduler would use
the wrong logic which makes decisions based on
provisioned_capacity_gb and max_over_subscription_ratio.
This patch tries to address the problem.
Implements: blueprint differentiate-thick-thin-in-scheduler
Closes-Bug: #1580866
Change-Id: I8b43624b934fadf1036fafa1f5e2b94b5317f81c
Removed unused safe_minidom_parse_string() method
from cinder.utils and it's related test case.
TrivialFix
Change-Id: I6c2cde126bd65930596e42b4a63dfc6b16aab839
Khadija Kamran