diff --git a/novaclient/tests/unit/v2/test_volumes.py b/novaclient/tests/unit/v2/test_volumes.py index 60178cb02..932b71d79 100644 --- a/novaclient/tests/unit/v2/test_volumes.py +++ b/novaclient/tests/unit/v2/test_volumes.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +import mock + from novaclient import api_versions from novaclient.tests.unit import utils from novaclient.tests.unit.v2 import fakes @@ -55,6 +57,24 @@ class VolumesTest(utils.TestCase): '/servers/1234/os-volume_attachments/Work') self.assertIsInstance(v, volumes.Volume) + def test_get_server_volume_with_exception(self): + self.assertRaises(TypeError, + self.cs.volumes.get_server_volume, + "1234") + + self.assertRaises(TypeError, + self.cs.volumes.get_server_volume, + "1234", + volume_id="Work", + attachment_id="123") + + @mock.patch('warnings.warn') + def test_get_server_volume_with_warn(self, mock_warn): + self.cs.volumes.get_server_volume(1234, + volume_id=None, + attachment_id="Work") + mock_warn.assert_called_once() + def test_list_server_volumes(self): vl = self.cs.volumes.get_server_volumes(1234) self.assert_request_id(vl, fakes.FAKE_REQUEST_ID_LIST) @@ -88,3 +108,21 @@ class VolumesV249Test(VolumesTest): 'device': '/dev/vdb', 'tag': 'test_tag'}}) self.assertIsInstance(v, volumes.Volume) + + def test_delete_server_volume_with_exception(self): + self.assertRaises(TypeError, + self.cs.volumes.delete_server_volume, + "1234") + + self.assertRaises(TypeError, + self.cs.volumes.delete_server_volume, + "1234", + volume_id="Work", + attachment_id="123") + + @mock.patch('warnings.warn') + def test_delete_server_volume_with_warn(self, mock_warn): + self.cs.volumes.delete_server_volume(1234, + volume_id=None, + attachment_id="Work") + mock_warn.assert_called_once() diff --git a/novaclient/v2/volumes.py b/novaclient/v2/volumes.py index ebccf8a09..d6208cbd1 100644 --- a/novaclient/v2/volumes.py +++ b/novaclient/v2/volumes.py @@ -16,6 +16,7 @@ """ Volume interface """ +import warnings from novaclient import api_versions from novaclient import base @@ -95,17 +96,32 @@ class VolumeManager(base.Manager): (server_id, src_volid,), body, "volumeAttachment") - def get_server_volume(self, server_id, attachment_id): + def get_server_volume(self, server_id, volume_id=None, attachment_id=None): """ - Get the volume identified by the attachment ID, that is attached to + Get the volume identified by the volume ID, that is attached to the given server ID :param server_id: The ID of the server - :param attachment_id: The ID of the attachment + :param volume_id: The ID of the volume to attach :rtype: :class:`Volume` """ + + if attachment_id is not None and volume_id is not None: + raise TypeError("You cannot specify both volume_id " + "and attachment_id arguments.") + + elif attachment_id is not None: + warnings.warn("attachment_id argument " + "of volumes.get_server_volume " + "method is deprecated in favor " + "of volume_id.") + volume_id = attachment_id + + if volume_id is None: + raise TypeError("volume_id is required argument.") + return self._get("/servers/%s/os-volume_attachments/%s" % (server_id, - attachment_id,), "volumeAttachment") + volume_id,), "volumeAttachment") def get_server_volumes(self, server_id): """ @@ -117,13 +133,28 @@ class VolumeManager(base.Manager): return self._list("/servers/%s/os-volume_attachments" % server_id, "volumeAttachments") - def delete_server_volume(self, server_id, attachment_id): + def delete_server_volume(self, server_id, volume_id=None, + attachment_id=None): """ - Detach a volume identified by the attachment ID from the given server + Detach a volume identified by the volume ID from the given server :param server_id: The ID of the server - :param attachment_id: The ID of the attachment + :param volume_id: The ID of the volume to attach :returns: An instance of novaclient.base.TupleWithMeta """ + if attachment_id is not None and volume_id is not None: + raise TypeError("You cannot specify both volume_id " + "and attachment_id arguments.") + + elif attachment_id is not None: + warnings.warn("attachment_id argument " + "of volumes.delete_server_volume " + "method is deprecated in favor " + "of volume_id.") + volume_id = attachment_id + + if volume_id is None: + raise TypeError("volume_id is required argument.") + return self._delete("/servers/%s/os-volume_attachments/%s" % - (server_id, attachment_id,)) + (server_id, volume_id,))