From 672175728fac7357195af880d91f30824de7d34d Mon Sep 17 00:00:00 2001
From: nidhimittalhada <nidhimittal19@gmail.com>
Date: Mon, 11 Jan 2016 14:52:53 +0530
Subject: [PATCH] Fix 'extend' API for 2.7+ microversions

Extend/Shrink share is not working in microversion 2.7+.
When trying to extend/shrink a share, manila raises an
error telling that the new share size must be an integer.
Bug was due to improper use of get method on dictionary.
Corrected by using get method properly.

Change-Id: I700581d815da0fdd6addedc42f3e2ba528680e60
Closes-Bug: #1531536
---
 manila/api/v1/shares.py                       |  3 +-
 manila/tests/api/v2/test_shares.py            | 28 ++++++++++++-------
 .../tests/api/test_shares_actions.py          | 26 +++++++++++++----
 3 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/manila/api/v1/shares.py b/manila/api/v1/shares.py
index d9b3d56f41..fb73585ac4 100644
--- a/manila/api/v1/shares.py
+++ b/manila/api/v1/shares.py
@@ -462,7 +462,8 @@ class ShareMixin(object):
             raise webob.exc.HTTPNotFound(explanation=six.text_type(e))
 
         try:
-            size = int(body.get(action, action.split('os-')[-1])['new_size'])
+            size = int(body.get(action,
+                                body.get(action.split('os-')[-1]))['new_size'])
         except (KeyError, ValueError, TypeError):
             msg = _("New share size must be specified as an integer.")
             raise webob.exc.HTTPBadRequest(explanation=msg)
diff --git a/manila/tests/api/v2/test_shares.py b/manila/tests/api/v2/test_shares.py
index 5265bc2c8c..d795a1d357 100644
--- a/manila/tests/api/v2/test_shares.py
+++ b/manila/tests/api/v2/test_shares.py
@@ -946,16 +946,20 @@ class ShareActionsTest(test.TestCase):
         expected = _fake_access_get_all()
         self.assertEqual(expected, res_dict['access_list'])
 
-    def test_extend(self):
+    @ddt.unpack
+    @ddt.data(
+        {'body': {'os-extend': {'new_size': 2}}, 'version': '2.6'},
+        {'body': {'extend': {'new_size': 2}}, 'version': '2.7'},
+    )
+    def test_extend(self, body, version):
         id = 'fake_share_id'
         share = stubs.stub_share_get(None, None, id)
         self.mock_object(share_api.API, 'get', mock.Mock(return_value=share))
         self.mock_object(share_api.API, "extend")
 
-        size = '123'
-        body = {"os-extend": {'new_size': size}}
-        req = fakes.HTTPRequest.blank('/v1/shares/%s/action' % id)
-
+        size = '2'
+        req = fakes.HTTPRequest.blank(
+            '/v2/shares/%s/action' % id, version=version)
         actual_response = self.controller._extend(req, id, body)
 
         share_api.API.get.assert_called_once_with(mock.ANY, id)
@@ -989,16 +993,20 @@ class ShareActionsTest(test.TestCase):
 
         self.assertRaises(target, self.controller._extend, req, id, body)
 
-    def test_shrink(self):
+    @ddt.unpack
+    @ddt.data(
+        {'body': {'os-shrink': {'new_size': 1}}, 'version': '2.6'},
+        {'body': {'shrink': {'new_size': 1}}, 'version': '2.7'},
+    )
+    def test_shrink(self, body, version):
         id = 'fake_share_id'
         share = stubs.stub_share_get(None, None, id)
         self.mock_object(share_api.API, 'get', mock.Mock(return_value=share))
         self.mock_object(share_api.API, "shrink")
 
-        size = '123'
-        body = {"os-shrink": {'new_size': size}}
-        req = fakes.HTTPRequest.blank('/v1/shares/%s/action' % id)
-
+        size = '1'
+        req = fakes.HTTPRequest.blank(
+            '/v2/shares/%s/action' % id, version=version)
         actual_response = self.controller._shrink(req, id, body)
 
         share_api.API.get.assert_called_once_with(mock.ANY, id)
diff --git a/manila_tempest_tests/tests/api/test_shares_actions.py b/manila_tempest_tests/tests/api/test_shares_actions.py
index 36fd6e53e7..42a758962b 100644
--- a/manila_tempest_tests/tests/api/test_shares_actions.py
+++ b/manila_tempest_tests/tests/api/test_shares_actions.py
@@ -473,12 +473,19 @@ class SharesActionsTest(base.BaseSharesTest):
         new_size = 2
 
         # extend share and wait for active status
-        self.shares_client.extend_share(share['id'], new_size)
+        self.shares_v2_client.extend_share(share['id'], new_size)
         self.shares_client.wait_for_share_status(share['id'], 'available')
 
         # check state and new size
-        share = self.shares_client.get_share(share['id'])
-        self.assertEqual(new_size, share['size'])
+        share_get = self.shares_v2_client.get_share(share['id'])
+        msg = (
+            "Share could not be extended. "
+            "Expected %(expected)s, got %(actual)s." % {
+                "expected": new_size,
+                "actual": share_get['size'],
+            }
+        )
+        self.assertEqual(new_size, share_get['size'], msg)
 
     @test.attr(type=["gate", ])
     @testtools.skipUnless(
@@ -489,12 +496,19 @@ class SharesActionsTest(base.BaseSharesTest):
         new_size = 1
 
         # shrink share and wait for active status
-        self.shares_client.shrink_share(share['id'], new_size)
+        self.shares_v2_client.shrink_share(share['id'], new_size)
         self.shares_client.wait_for_share_status(share['id'], 'available')
 
         # check state and new size
-        share = self.shares_client.get_share(share['id'])
-        self.assertEqual(new_size, share['size'])
+        share_get = self.shares_v2_client.get_share(share['id'])
+        msg = (
+            "Share could not be shrunk. "
+            "Expected %(expected)s, got %(actual)s." % {
+                "expected": new_size,
+                "actual": share_get['size'],
+            }
+        )
+        self.assertEqual(new_size, share_get['size'], msg)
 
 
 class SharesRenameTest(base.BaseSharesTest):