Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[FLOC-4489] Use detailed volume list and update to a version of mimic that supports that endpoint #2892

Merged
merged 5 commits into from
Sep 8, 2016
Merged

[FLOC-4489] Use detailed volume list and update to a version of mimic that supports that endpoint #2892

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cbf4eb2
Install ClusterHQ fork of mimic
wallrj Aug 30, 2016
c3718cd
Merge remote-tracking branch 'origin/flaky-mimic-tests-FLOC-4491' int…
wallrj Sep 2, 2016
764a226
Merge remote-tracking branch 'origin/rackspace-libcloud-tls-FLOC-4490…
wallrj Sep 2, 2016
84bb3b7
The updated mimic is detected as a cinder v2 endpoint so we need to u…
wallrj Sep 2, 2016
fe8f1a3
Merge remote-tracking branch 'origin/master' into cinder-volume-detai…
wallrj Sep 7, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion flocker/node/agents/cinder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ def list_volumes(self):
http://docs.rackspace.com/cbs/api/v1.0/cbs-devguide/content/GET_getVolumesDetail_v1__tenant_id__volumes_detail_volumes.html
"""
flocker_volumes = []
for cinder_volume in self.cinder_volume_manager.list(detailed=False):
for cinder_volume in self.cinder_volume_manager.list(detailed=True):
Copy link
Contributor

@wallnerryan wallnerryan Sep 6, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a case where we do not always need detailed=True? I seem to remember the discussion that Cinder v1 would pass with detailed=False, thus the detailed information is not needed? maybe im making this up, but making sure we're not requested uneeded information that may slow down the process being that every volume may have more information.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that's right.

That's what led to this bug. I tested against Rackspace and saw the tests pass.
On OpenStack the detailed list doesn't include the attachment info.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be the case where we use False for v1 instead of always True for detailed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be possible but probably more trouble than it's worth.
At this point the cinder version detection has already taken place and the CinderBlockDeviceAPI has an object conforming to an interface that we've defined ICinderVolumeManager.
Since we're aiming to drop support for CinderV1 (Rackspace) I'm going to leave it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. I wonder if we should drop v1 though, there may be private deployments that may want to use v1, but not sure.

if _is_cluster_volume(self.cluster_id, cinder_volume):
flocker_volume = _blockdevicevolume_from_cinder_volume(
cinder_volume
Expand Down
1 change: 1 addition & 0 deletions flocker/node/agents/functional/test_cinder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -868,6 +868,7 @@ def _build_and_test_api(self, listening_port):
(api
.cinder_volume_manager
._original
._client_v2
._cinder_volumes
.api
.client
Expand Down
3 changes: 2 additions & 1 deletion requirements/flocker-dev.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
mimic==2.1.0
--find-links git+https://github.com/ClusterHQ/mimic@2.1.0+chq2#egg=mimic-2.1.0+chq2
mimic==2.1.0+chq2
# The test suite uses network namespaces
# nomenclature can only be installed on Linux
# The "linux2" marker value is specific to Python2
Expand Down
1 change: 1 addition & 0 deletions requirements/flocker-dev.txt.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
--find-links git+https://github.com/ClusterHQ/mimic@2.1.0+chq2#egg=mimic-2.1.0+chq2
mimic
# The test suite uses network namespaces
# nomenclature can only be installed on Linux
Expand Down