Basic resource.prop for ID attributes (block store)

This patch set updates all block store objects to use basic
properties for ID attributes. In particular, the following changes
were made:
  - Use basic *_id resource.prop for ID attributes
  - Clarify documentation for ID attributes
  - Revert 8c7ca1e62e

In addition, functional tests were added for the snapshot,
type and volume proxy interfaces.

Change-Id: Ica597e65a59fc054a8544cd6e5888528bf1d2a70
Partial-Bug: #1461200

openstack/block_store/v2/_proxy.py

@@ -18,23 +18,6 @@ from openstack import proxy
 
 class Proxy(proxy.BaseProxy):
 
-    def _convert_id(self, attrs, value, resource_type):
-        """Convert potential Resource values into IDs
-
-        The structure of Snapshot and Volume resources is such that their
-        Resource subclasses contain properties of each other's types, because
-        a Snapshot can be created of a Volume, and a Volume can be created from
-        a Snapshot. Additionally, a Volume can be created from another
-        Volume, yet the source_volume prop can't refer to the current class.
-        We work around this by simply looking for those Resource types
-        before sending them on.
-        """
-        val = attrs.pop(value, None)
-        if val is not None:
-            if isinstance(val, resource_type):
-                val = val.id
-            attrs[value] = val
-
     def get_snapshot(self, snapshot):
         """Get a single snapshot
 
@@ -58,7 +41,6 @@ class Proxy(proxy.BaseProxy):
         :returns: The results of snapshot creation
         :rtype: :class:`~openstack.volume.v2.snapshot.Snapshot`
         """
-        self._convert_id(attrs, "volume", _volume.Volume)
         return self._create(_snapshot.Snapshot, **attrs)
 
     def delete_snapshot(self, snapshot, ignore_missing=True):
@@ -139,8 +121,6 @@ class Proxy(proxy.BaseProxy):
         :returns: The results of volume creation
         :rtype: :class:`~openstack.volume.v2.volume.Volume`
         """
-        self._convert_id(attrs, "source_volume", _volume.Volume)
-        self._convert_id(attrs, "snapshot", _snapshot.Snapshot)
         return self._create(_volume.Volume, **attrs)
 
     def delete_volume(self, volume, ignore_missing=True):

openstack/block_store/v2/snapshot.py

@@ -27,7 +27,7 @@ class Snapshot(resource.Resource):
     allow_update = True
 
     # Properties
-    #: A UUID representing this snapshot.
+    #: A ID representing this snapshot.
     id = resource.prop("id")
     #: Name of the snapshot. Default is None.
     name = resource.prop("name")
@@ -42,7 +42,7 @@ class Snapshot(resource.Resource):
     #: Metadata associated with this snapshot.
     metadata = resource.prop("metadata", type=dict)
     #: The ID of the volume this snapshot was taken of.
-    volume = resource.prop("volume_id")
+    volume_id = resource.prop("volume_id")
     #: The size of the volume, in GBs.
     size = resource.prop("size", type=int)
     #: Indicate whether to snapshot, even if the volume is attached.
@@ -56,5 +56,5 @@ class SnapshotDetail(Snapshot):
 
     #: The percentage of completeness the snapshot is currently at.
     progress = resource.prop("os-extended-snapshot-attributes:progress")
-    #: The tenant ID this snapshot is associatd with.
+    #: The project ID this snapshot is associated with.
     project_id = resource.prop("os-extended-snapshot-attributes:project_id")

openstack/block_store/v2/type.py

@@ -26,7 +26,7 @@ class Type(resource.Resource):
     allow_delete = True
 
     # Properties
-    #: A UUID representing this volume.
+    #: A ID representing this type.
     id = resource.prop("id")
     #: Name of the type.
     name = resource.prop("name")

openstack/block_store/v2/volume.py

@@ -11,8 +11,6 @@
 # under the License.
 
 from openstack.block_store import block_store_service
-from openstack.block_store.v2 import type as _type
-from openstack.image.v2 import image as _image
 from openstack import resource
 
 
@@ -29,7 +27,7 @@ class Volume(resource.Resource):
     allow_update = True
 
     # Properties
-    #: A UUID representing this volume.
+    #: A ID representing this volume.
     id = resource.prop("id")
     #: The name of this volume.
     name = resource.prop("name")
@@ -41,20 +39,20 @@ class Volume(resource.Resource):
     #: To create a volume from an existing volume, specify the ID of
     #: the existing volume. If specified, the volume is created with
     #: same size of the source volume.
-    source_volume = resource.prop("source_volid")
+    source_volume_id = resource.prop("source_volid")
     #: The volume description.
     description = resource.prop("description")
     #: To create a volume from an existing snapshot, specify the ID of
     #: the existing volume snapshot. If specified, the volume is created
     #: in same availability zone and with same size of the snapshot.
-    snapshot = resource.prop("snapshot_id")
+    snapshot_id = resource.prop("snapshot_id")
     #: The size of the volume, in GBs. *Type: int*
     size = resource.prop("size", type=int)
     #: The ID of the image from which you want to create the volume.
     #: Required to create a bootable volume.
-    image = resource.prop("imageRef", type=_image.Image)
+    image_id = resource.prop("imageRef")
-    #: The associated volume type.
+    #: The name of the associated volume type.
-    type = resource.prop("volume_type", type=_type.Type)
+    volume_type = resource.prop("volume_type")
     #: Enables or disables the bootable attribute. You can boot an
     #: instance from a bootable volume. *Type: bool*
     bootable = resource.prop("bootable", type=bool)
@@ -91,7 +89,7 @@ class VolumeDetail(Volume):
     extended_replication_status = resource.prop(
         "os-volume-replication:extended_status")
     #: ID of the consistency group.
-    consistency_group = resource.prop("consistencygroup_id")
+    consistency_group_id = resource.prop("consistencygroup_id")
     #: Data set by the replication driver
     replication_driver_data = resource.prop(
         "os-volume-replication:driver_data")

openstack/tests/functional/block_store/v2/test_snapshot.py

@@ -0,0 +1,68 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import uuid
+
+from openstack.block_store.v2 import snapshot as _snapshot
+from openstack.block_store.v2 import volume as _volume
+from openstack.tests.functional import base
+
+
+class TestSnapshot(base.BaseFunctionalTest):
+
+    SNAPSHOT_NAME = uuid.uuid4().hex
+    SNAPSHOT_ID = None
+    VOLUME_NAME = uuid.uuid4().hex
+    VOLUME_ID = None
+
+    @classmethod
+    def setUpClass(cls):
+        super(TestSnapshot, cls).setUpClass()
+        volume = cls.conn.block_store.create_volume(
+            name=cls.VOLUME_NAME,
+            size=1)
+        cls.conn.block_store.wait_for_status(volume,
+                                             status='available',
+                                             failures=['error'],
+                                             interval=2,
+                                             wait=120)
+        assert isinstance(volume, _volume.Volume)
+        cls.assertIs(cls.VOLUME_NAME, volume.name)
+        cls.VOLUME_ID = volume.id
+        snapshot = cls.conn.block_store.create_snapshot(
+            name=cls.SNAPSHOT_NAME,
+            volume_id=cls.VOLUME_ID)
+        cls.conn.block_store.wait_for_status(snapshot,
+                                             status='available',
+                                             failures=['error'],
+                                             interval=2,
+                                             wait=120)
+        assert isinstance(snapshot, _snapshot.Snapshot)
+        cls.assertIs(cls.SNAPSHOT_NAME, snapshot.name)
+        cls.SNAPSHOT_ID = snapshot.id
+
+    @classmethod
+    def tearDownClass(cls):
+        snapshot = cls.conn.block_store.get_snapshot(cls.SNAPSHOT_ID)
+        sot = cls.conn.block_store.delete_snapshot(snapshot,
+                                                   ignore_missing=False)
+        cls.conn.block_store.wait_for_delete(snapshot,
+                                             interval=2,
+                                             wait=120)
+        cls.assertIs(None, sot)
+        sot = cls.conn.block_store.delete_volume(cls.VOLUME_ID,
+                                                 ignore_missing=False)
+        cls.assertIs(None, sot)
+
+    def test_get(self):
+        sot = self.conn.block_store.get_snapshot(self.SNAPSHOT_ID)
+        self.assertEqual(self.SNAPSHOT_NAME, sot.name)

openstack/tests/functional/block_store/v2/test_type.py

@@ -0,0 +1,40 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import uuid
+
+from openstack.block_store.v2 import type as _type
+from openstack.tests.functional import base
+
+
+class TestType(base.BaseFunctionalTest):
+
+    TYPE_NAME = uuid.uuid4().hex
+    TYPE_ID = None
+
+    @classmethod
+    def setUpClass(cls):
+        super(TestType, cls).setUpClass()
+        sot = cls.conn.block_store.create_type(name=cls.TYPE_NAME)
+        assert isinstance(sot, _type.Type)
+        cls.assertIs(cls.TYPE_NAME, sot.name)
+        cls.TYPE_ID = sot.id
+
+    @classmethod
+    def tearDownClass(cls):
+        sot = cls.conn.block_store.delete_type(cls.TYPE_ID,
+                                               ignore_missing=False)
+        cls.assertIs(None, sot)
+
+    def test_get(self):
+        sot = self.conn.block_store.get_type(self.TYPE_ID)
+        self.assertEqual(self.TYPE_NAME, sot.name)

openstack/tests/functional/block_store/v2/test_volume.py

@@ -0,0 +1,47 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import uuid
+
+from openstack.block_store.v2 import volume as _volume
+from openstack.tests.functional import base
+
+
+class TestVolume(base.BaseFunctionalTest):
+
+    VOLUME_NAME = uuid.uuid4().hex
+    VOLUME_ID = None
+
+    @classmethod
+    def setUpClass(cls):
+        super(TestVolume, cls).setUpClass()
+        volume = cls.conn.block_store.create_volume(
+            name=cls.VOLUME_NAME,
+            size=1)
+        cls.conn.block_store.wait_for_status(volume,
+                                             status='available',
+                                             failures=['error'],
+                                             interval=2,
+                                             wait=120)
+        assert isinstance(volume, _volume.Volume)
+        cls.assertIs(cls.VOLUME_NAME, volume.name)
+        cls.VOLUME_ID = volume.id
+
+    @classmethod
+    def tearDownClass(cls):
+        sot = cls.conn.block_store.delete_volume(cls.VOLUME_ID,
+                                                 ignore_missing=False)
+        cls.assertIs(None, sot)
+
+    def test_get(self):
+        sot = self.conn.block_store.get_volume(self.VOLUME_ID)
+        self.assertEqual(self.VOLUME_NAME, sot.name)

openstack/tests/unit/block_store/v2/test_proxy.py

@@ -10,8 +10,6 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-import mock
-
 from openstack.block_store.v2 import _proxy
 from openstack.block_store.v2 import snapshot
 from openstack.block_store.v2 import type
@@ -30,16 +28,6 @@ class TestVolumeProxy(test_proxy_base.TestProxyBase):
     def test_snapshot_create_attrs(self):
         self.verify_create(self.proxy.create_snapshot, snapshot.Snapshot)
 
-    def test_snapshot_create_volume_prop(self):
-        self.proxy._create = mock.Mock()
-
-        id = "12345"
-        vol = volume.Volume({"id": id})
-        self.proxy.create_snapshot(volume=vol)
-
-        kwargs = self.proxy._create.call_args[1]
-        self.assertEqual({"volume": id}, kwargs)
-
     def test_snapshot_delete(self):
         self.verify_delete(self.proxy.delete_snapshot,
                            snapshot.Snapshot, False)
@@ -66,18 +54,6 @@ class TestVolumeProxy(test_proxy_base.TestProxyBase):
     def test_volume_create_attrs(self):
         self.verify_create(self.proxy.create_volume, volume.Volume)
 
-    def test_volume_create_snapshot_prop(self):
-        self.proxy._create = mock.Mock()
-
-        id1 = "12345"
-        id2 = "67890"
-        snap = snapshot.Snapshot({"id": id1})
-        source = volume.Volume({"id": id2})
-        self.proxy.create_volume(snapshot=snap, source_volume=source)
-
-        kwargs = self.proxy._create.call_args[1]
-        self.assertEqual({"snapshot": id1, "source_volume": id2}, kwargs)
-
     def test_volume_delete(self):
         self.verify_delete(self.proxy.delete_volume, volume.Volume, False)
 

openstack/tests/unit/block_store/v2/test_snapshot.py

@@ -58,7 +58,7 @@ class TestSnapshot(testtools.TestCase):
         self.assertEqual(SNAPSHOT["status"], sot.status)
         self.assertEqual(SNAPSHOT["created_at"], sot.created_at)
         self.assertEqual(SNAPSHOT["metadata"], sot.metadata)
-        self.assertEqual(SNAPSHOT["volume_id"], sot.volume)
+        self.assertEqual(SNAPSHOT["volume_id"], sot.volume_id)
         self.assertEqual(SNAPSHOT["size"], sot.size)
         self.assertEqual(SNAPSHOT["name"], sot.name)
 

openstack/tests/unit/block_store/v2/test_volume.py

@@ -14,9 +14,7 @@ import copy
 
 import testtools
 
-from openstack.block_store.v2 import type
 from openstack.block_store.v2 import volume
-from openstack.image.v2 import image
 
 FAKE_ID = "6685584b-1eac-4da6-b5c3-555430cf68ff"
 
@@ -31,7 +29,7 @@ VOLUME = {
     "volume_type": "some_type",
     "snapshot_id": "93c2e2aa-7744-4fd6-a31a-80c4726b08d7",
     "source_volid": None,
-    "image": "some_image",
+    "imageRef": "some_image",
     "metadata": {},
     "id": FAKE_ID,
     "size": 10
@@ -77,12 +75,12 @@ class TestVolume(testtools.TestCase):
         self.assertEqual(VOLUME["bootable"], sot.bootable)
         self.assertEqual(VOLUME["created_at"], sot.created)
         self.assertEqual(VOLUME["description"], sot.description)
-        self.assertEqual(type.Type({"id": VOLUME["volume_type"]}), sot.type)
+        self.assertEqual(VOLUME["volume_type"], sot.volume_type)
-        self.assertEqual(VOLUME["snapshot_id"], sot.snapshot)
+        self.assertEqual(VOLUME["snapshot_id"], sot.snapshot_id)
-        self.assertEqual(VOLUME["source_volid"], sot.source_volume)
+        self.assertEqual(VOLUME["source_volid"], sot.source_volume_id)
         self.assertEqual(VOLUME["metadata"], sot.metadata)
         self.assertEqual(VOLUME["size"], sot.size)
-        self.assertEqual(image.Image({"id": VOLUME["image"]}), sot.image)
+        self.assertEqual(VOLUME["imageRef"], sot.image_id)
 
 
 class TestVolumeDetail(testtools.TestCase):
@@ -107,7 +105,7 @@ class TestVolumeDetail(testtools.TestCase):
             VOLUME_DETAIL["os-volume-replication:extended_status"],
             sot.extended_replication_status)
         self.assertEqual(VOLUME_DETAIL["consistencygroup_id"],
-                         sot.consistency_group)
+                         sot.consistency_group_id)
         self.assertEqual(VOLUME_DETAIL["os-volume-replication:driver_data"],
                          sot.replication_driver_data)
         self.assertEqual(VOLUME_DETAIL["encrypted"], sot.encrypted)