Skip to content
This repository has been archived by the owner on Nov 27, 2023. It is now read-only.

Using proper volume size for volume creating #162

Closed
wants to merge 5 commits into from
Closed

Using proper volume size for volume creating #162

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4323d56
Merge pull request #1 from softlayer/master
zhiyanliu Aug 13, 2014
84f600e
Merge pull request #2 from softlayer/master
zhiyanliu Aug 14, 2014
f6988c8
Merge pull request #3 from softlayer/master
zhiyanliu Nov 17, 2014
616e1fd
Merge pull request #4 from softlayer/master
zhiyanliu Dec 2, 2014
6d28dc9
Using proper volume size for volume creating
Nov 17, 2014
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
46 changes: 27 additions & 19 deletions jumpgate/volume/drivers/sl/volumes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,8 @@ def on_post(self, req, resp, tenant_id):
"SoftLayerAPIError",
e.faultString,
code=e.faultCode)
except IndexError as e:
return error_handling.bad_request(resp, str(e))
Copy link
Member

Choose a reason for hiding this comment

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

Just curious -- does this 400 response match upstream behavior if I were to give normal Cinder some absurd value for volume size?

Copy link
Contributor

Choose a reason for hiding this comment

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

For cinder volume create, there are different sources for volume create, such as image, snapshot, volume,.... The case
jumpgate volume can handle is only the plain volume create from the backend. In such case, cinder volume API will return 202 upon the REST api, the scheduler and volume driver will find the host, check the quota, and create the volume. If everything goes well, the volume status will be 'active'. Otherwise, it will be marked as 'error'. So most of the
volume create error is reflected via the volume status instead of REST API return code since it is asynchronous call.

For jumpgate volume driver, it is using the synchronous way to simulate the cinder API. The BadRequest is closest
option, which is similar to the validation code does in the cinder volume api code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, +1, it was the reason I thought. Thanks for the explanation, ajiang38740.

except Exception as e:
return error_handling.volume_fault(resp, str(e))

Expand Down Expand Up @@ -295,25 +297,31 @@ def _match_portable_storage_prices(packageId, size, exact_capacity):
price_matrix = {}
for x in price_list:
price_matrix.update({int(x['capacity']): x['prices']})
# find the closet capacity to the requested size
if exact_capacity:
ret = False
for x in price_matrix:
if size == x:
ret = True
capacity_idx = x
break
if not ret:
raise SoftLayer.SoftLayerAPIError(
HTTP.BAD_REQUEST,
'volume_types: extra_specs: '
'drivers:exact_capacity is set to'
' True and there is no volume with'
' matching capacity')
else:
capacity_idx = min(price_matrix, key=lambda x: abs(x - size))

return price_matrix[capacity_idx]
try:
# find the closest capacity to the requested size
if exact_capacity:
ret = False
for x in price_matrix:
if size == x:
ret = True
capacity_idx = x
break
if not ret:
raise SoftLayer.SoftLayerAPIError(
HTTP.BAD_REQUEST,
'volume_types: extra_specs: '
'drivers:exact_capacity is set to'
' True and there is no volume with'
' matching capacity')
else:
capacity_idx = sorted([cap for cap in price_matrix
if cap >= size])[0]

return price_matrix[capacity_idx]
except IndexError:
Copy link
Member

Choose a reason for hiding this comment

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

Any reason this try/except shouldn't wrap only the code at lines 317-318, rather than the whole if/else block? Looks like that is the only code here that should raise this exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

L317/8 can raise this exception as well.

msg = "Unsupported volume size: %d" % size
LOG.error(msg)
raise IndexError(msg)

def _find_availibility_zone_location(zone):
# make sure there is an availability_zone selected
Expand Down