Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,10 @@ Compute
(LIBCLOUD-759, GITHUB-603)
[David Wilson]

- Standardize VolumeSnapshot states into the ``state`` attribute.
(LIBCLOUD-758, GITHUB-602)
[Allard Hoeve]

Storage
~~~~~~~

Expand Down
12 changes: 9 additions & 3 deletions libcloud/compute/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,8 @@ class VolumeSnapshot(object):
"""
A base VolumeSnapshot class to derive from.
"""
def __init__(self, id, driver, size=None, extra=None, created=None):
def __init__(self, id, driver, size=None, extra=None, created=None,
state=None):
"""
VolumeSnapshot constructor.

Expand All @@ -574,12 +575,17 @@ def __init__(self, id, driver, size=None, extra=None, created=None):
:param created: A datetime object that represents when the
snapshot was created
:type created: ``datetime.datetime``

:param state: A string representing the state the snapshot is
in. See `libcloud.compute.types.StorageVolumeState`.
:type state: ``str``
"""
self.id = id
self.driver = driver
self.size = size
self.extra = extra or {}
self.created = created
self.state = state

def destroy(self):
"""
Expand All @@ -590,8 +596,8 @@ def destroy(self):
return self.driver.destroy_volume_snapshot(snapshot=self)

def __repr__(self):
return ('<VolumeSnapshot id=%s size=%s driver=%s>' %
(self.id, self.size, self.driver.name))
return ('<VolumeSnapshot id=%s size=%s driver=%s state=%s>' %
(self.id, self.size, self.driver.name, self.state))


class KeyPair(object):
Expand Down
22 changes: 19 additions & 3 deletions libcloud/compute/drivers/ec2.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
from libcloud.compute.base import NodeImage, StorageVolume, VolumeSnapshot
from libcloud.compute.base import KeyPair
from libcloud.compute.types import NodeState, KeyPairDoesNotExistError, \
StorageVolumeState
StorageVolumeState, VolumeSnapshotState

__all__ = [
'API_VERSION',
Expand Down Expand Up @@ -2107,6 +2107,12 @@ class BaseEC2NodeDriver(NodeDriver):
'error_deleting': StorageVolumeState.ERROR
}

SNAPSHOT_STATE_MAP = {
'pending': VolumeSnapshotState.CREATING,
'completed': VolumeSnapshotState.AVAILABLE,
'error': VolumeSnapshotState.ERROR,
}

def list_nodes(self, ex_node_ids=None, ex_filters=None):
"""
List all nodes
Expand Down Expand Up @@ -4917,8 +4923,18 @@ def _to_snapshot(self, element, name=None):
extra['tags'] = tags
extra['name'] = name

return VolumeSnapshot(snapId, size=int(size),
driver=self, extra=extra, created=created)
# state
state = self.SNAPSHOT_STATE_MAP.get(
extra["state"],
VolumeSnapshotState.UNKNOWN
)

return VolumeSnapshot(snapId,
size=int(size),
driver=self,
extra=extra,
created=created,
state=state)

def _to_key_pairs(self, elems):
key_pairs = [self._to_key_pair(elem=elem) for elem in elems]
Expand Down
20 changes: 18 additions & 2 deletions libcloud/compute/drivers/openstack.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@
from libcloud.compute.base import (NodeDriver, Node, NodeLocation,
StorageVolume, VolumeSnapshot)
from libcloud.compute.base import KeyPair
from libcloud.compute.types import NodeState, StorageVolumeState, Provider
from libcloud.compute.types import NodeState, StorageVolumeState, Provider, \
VolumeSnapshotState
from libcloud.pricing import get_size_price
from libcloud.utils.xml import findall

Expand Down Expand Up @@ -119,6 +120,16 @@ class OpenStackNodeDriver(NodeDriver, OpenStackDriverMixin):
'error_extending': StorageVolumeState.ERROR,
}

# http://developer.openstack.org/api-ref-blockstorage-v2.html#ext-backups-v2
SNAPSHOT_STATE_MAP = {
'creating': VolumeSnapshotState.CREATING,
'available': VolumeSnapshotState.AVAILABLE,
'deleting': VolumeSnapshotState.DELETING,
'error': VolumeSnapshotState.ERROR,
'restoring': VolumeSnapshotState.RESTORING,
'error_restoring': VolumeSnapshotState.ERROR
}

def __new__(cls, key, secret=None, secure=True, host=None, port=None,
api_version=DEFAULT_API_VERSION, **kwargs):
if cls is OpenStackNodeDriver:
Expand Down Expand Up @@ -2165,14 +2176,19 @@ def _to_snapshot(self, data):
'description': description,
'status': status}

state = self.SNAPSHOT_STATE_MAP.get(
status,
VolumeSnapshotState.UNKNOWN
)

try:
created_dt = parse_date(created_at)
except ValueError:
created_dt = None

snapshot = VolumeSnapshot(id=data['id'], driver=self,
size=data['size'], extra=extra,
created=created_dt)
created=created_dt, state=state)
return snapshot

def _to_size(self, api_flavor, price=None, bandwidth=None):
Expand Down
11 changes: 9 additions & 2 deletions libcloud/compute/drivers/rackspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"""
Rackspace driver
"""
from libcloud.compute.types import Provider, LibcloudError
from libcloud.compute.types import Provider, LibcloudError, VolumeSnapshotState
from libcloud.compute.base import NodeLocation, VolumeSnapshot
from libcloud.compute.drivers.openstack import OpenStack_1_0_Connection,\
OpenStack_1_0_NodeDriver, OpenStack_1_0_Response
Expand Down Expand Up @@ -216,14 +216,21 @@ def _to_snapshot(self, api_node):
'description': api_node['displayDescription'],
'status': api_node['status']}

state = self.SNAPSHOT_STATE_MAP.get(
api_node['status'],
VolumeSnapshotState.UNKNOWN
)

try:
created_td = parse_date(api_node['createdAt'])
except ValueError:
created_td = None

snapshot = VolumeSnapshot(id=api_node['id'], driver=self,
size=api_node['size'],
extra=extra, created=created_td)
extra=extra,
created=created_td,
state=state)
return snapshot

def _ex_connection_class_kwargs(self):
Expand Down
26 changes: 19 additions & 7 deletions libcloud/compute/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,15 +236,27 @@ class StorageVolumeState(object):
"""
Standard states of a StorageVolume
"""
AVAILABLE = "available"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry, for a late comment, I missed this in the previous pass.

For consistency, it would be great if you can change "INUSE" to "IN_USE" and "BACKUP" to "SNAPSHOTING" (this way it's a verb and it's consistent with other states).

Thanks!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

True, but I personally find "backup" state a bit confusing (backuping is also better, imo, but snapshotting is more provider agnostic) :)

ERROR = "error"
INUSE = "in_use"
CREATING = "creating"
DELETING = "deleting"
DELETED = "deleted"
BACKUP = "backup"
ATTACHING = "attaching"
UNKNOWN = "unknown"


class VolumeSnapshotState(object):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In newer parts of the code we decided to replace numeric enum values with string values (e.g. RecordType enum).

Strings makes it easier to see what's going on since you don't need to check the enum type definition to see to which enum name a particular value maps to.

I would appreciate if you can update the code to use string values (e.g. AVAILABLE = 'available', ERROR = ' error', etc.).

In the future I also plan to clean and improve the legacy code and update NodeState to also use string values.

"""
Standard states of VolumeSnapshots
"""
AVAILABLE = 0
ERROR = 1
INUSE = 2
CREATING = 3
DELETING = 4
DELETED = 5
BACKUP = 6
ATTACHING = 7
UNKNOWN = 8
CREATING = 2
DELETING = 3
RESTORING = 4
UNKNOWN = 5


class Architecture(object):
Expand Down
2 changes: 1 addition & 1 deletion libcloud/test/compute/fixtures/ec2/describe_snapshots.xml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<item>
<snapshotId>snap-18349159</snapshotId>
<volumeId>vol-b5a2c1v9</volumeId>
<status>pending</status>
<status>completed</status>
<startTime>2013-09-15T16:00:30.000Z</startTime>
<progress>30%</progress>
<ownerId>1938218230</ownerId>
Expand Down
6 changes: 5 additions & 1 deletion libcloud/test/compute/test_ec2.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@
from libcloud.compute.drivers.ec2 import EC2NetworkSubnet
from libcloud.compute.base import Node, NodeImage, NodeSize, NodeLocation
from libcloud.compute.base import StorageVolume, VolumeSnapshot
from libcloud.compute.types import KeyPairDoesNotExistError, StorageVolumeState
from libcloud.compute.types import KeyPairDoesNotExistError, StorageVolumeState, \
VolumeSnapshotState

from libcloud.test import MockHttpTestCase, LibcloudTestCase
from libcloud.test.compute import TestCaseMixin
Expand Down Expand Up @@ -866,6 +867,7 @@ def test_create_volume_snapshot(self):
self.assertEqual('Test snapshot', snap.extra['name'])
self.assertEqual(vol.id, snap.extra['volume_id'])
self.assertEqual('pending', snap.extra['state'])
self.assertEqual(VolumeSnapshotState.CREATING, snap.state)
# 2013-08-15T16:22:30.000Z
self.assertEqual(datetime(2013, 8, 15, 16, 22, 30, tzinfo=UTC), snap.created)

Expand All @@ -875,11 +877,13 @@ def test_list_snapshots(self):
self.assertEqual(len(snaps), 3)

self.assertEqual('snap-428abd35', snaps[0].id)
self.assertEqual(VolumeSnapshotState.CREATING, snaps[0].state)
self.assertEqual('vol-e020df80', snaps[0].extra['volume_id'])
self.assertEqual(30, snaps[0].size)
self.assertEqual('Daily Backup', snaps[0].extra['description'])

self.assertEqual('snap-18349159', snaps[1].id)
self.assertEqual(VolumeSnapshotState.AVAILABLE, snaps[1].state)
self.assertEqual('vol-b5a2c1v9', snaps[1].extra['volume_id'])
self.assertEqual(15, snaps[1].size)
self.assertEqual('Weekly backup', snaps[1].extra['description'])
Expand Down
4 changes: 3 additions & 1 deletion libcloud/test/compute/test_openstack.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@

from libcloud.common.types import InvalidCredsError, MalformedResponseError, \
LibcloudError
from libcloud.compute.types import Provider, KeyPairDoesNotExistError, StorageVolumeState
from libcloud.compute.types import Provider, KeyPairDoesNotExistError, StorageVolumeState, \
VolumeSnapshotState
from libcloud.compute.providers import get_driver
from libcloud.compute.drivers.openstack import (
OpenStack_1_0_NodeDriver, OpenStack_1_0_Response,
Expand Down Expand Up @@ -1431,6 +1432,7 @@ def test_ex_list_snapshots(self):
self.assertEqual(snapshots[0].created, datetime.datetime(2012, 2, 29, 3, 50, 7, tzinfo=UTC))
self.assertEqual(snapshots[0].extra['created'], "2012-02-29T03:50:07Z")
self.assertEqual(snapshots[0].extra['name'], 'snap-001')
self.assertEqual(snapshots[0].state, VolumeSnapshotState.AVAILABLE)

# invalid date is parsed as None
assert snapshots[2].created is None
Expand Down