From: Wilson Liu <liuxinguo@huawei.com>
Date: Thu, 25 Feb 2016 03:27:19 +0000 (+0800)
Subject: Huawei: delete_snapshot need not return any value
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=66a77e4112c3a3c4a800b504f27895994b58c4f5;p=openstack-build%2Fcinder-build.git

Huawei: delete_snapshot need not return any value

Currently delete_snapshot return True or False to indicate
whether the snapshot is really deleted on the array. This
is useless and just write for unit tests. Now we will remove
the return value and add another unit test for delete_snapshot.

Closes-Bug: #1549584
Change-Id: I17fea61e3c0f5565ab04bc82f172e638480f45dc
---

diff --git a/cinder/tests/unit/test_huawei_drivers.py b/cinder/tests/unit/test_huawei_drivers.py
index 6ea717e6b..433198758 100644
--- a/cinder/tests/unit/test_huawei_drivers.py
+++ b/cinder/tests/unit/test_huawei_drivers.py
@@ -444,7 +444,8 @@ FAKE_LUN_COUNT_RESPONSE = """
 FAKE_SNAPSHOT_LIST_INFO_RESPONSE = """
 {
     "error": {
-        "code": 0
+        "code": 0,
+        "description": "0"
     },
     "data": [{
         "ID": 11,
@@ -478,7 +479,8 @@ FAKE_CREATE_SNAPSHOT_INFO_RESPONSE = """
 FAKE_GET_SNAPSHOT_INFO_RESPONSE = """
 {
     "error": {
-        "code": 0
+        "code": 0,
+        "description": "0"
     },
     "data": {
         "ID": 11,
@@ -2049,8 +2051,7 @@ class HuaweiISCSIDriverTestCase(test.TestCase):
         self.assertEqual(11, lun_info['provider_location'])
 
     def test_delete_snapshot_success(self):
-        delete_flag = self.driver.delete_snapshot(test_snap)
-        self.assertTrue(delete_flag)
+        self.driver.delete_snapshot(test_snap)
 
     def test_create_volume_from_snapsuccess(self):
         self.mock_object(
@@ -2111,6 +2112,21 @@ class HuaweiISCSIDriverTestCase(test.TestCase):
         self.driver.client.test_fail = True
         self.driver.delete_volume(test_snap)
 
+    def test_delete_snapshot_with_snapshot_nonexistent(self):
+        fake_snap = {
+            'name': 'volume-21ec7341-9256-497b-97d9-ef48edcf0635',
+            'size': 1,
+            'volume_name': 'vol1',
+            'id': '21ec7341-9256-497b-97d9-ef48edcf0635',
+            'volume_id': '21ec7341-9256-497b-97d9-ef48edcf0635',
+            'provider_auth': None,
+            'project_id': 'project',
+            'display_name': 'vol1',
+            'display_description': 'test volume',
+            'volume_type_id': None,
+            'provider_location': None, }
+        self.driver.delete_snapshot(fake_snap)
+
     def test_initialize_connection_fail(self):
 
         self.driver.client.test_fail = True
@@ -3085,8 +3101,7 @@ class HuaweiFCDriverTestCase(test.TestCase):
         self.assertEqual(11, lun_info['provider_location'])
 
     def test_delete_snapshot_success(self):
-        delete_flag = self.driver.delete_snapshot(test_snap)
-        self.assertTrue(delete_flag)
+        self.driver.delete_snapshot(test_snap)
 
     def test_create_volume_from_snapsuccess(self):
         lun_info = self.driver.create_volume_from_snapshot(test_volume,
diff --git a/cinder/volume/drivers/huawei/huawei_driver.py b/cinder/volume/drivers/huawei/huawei_driver.py
index 80e02b600..d475ccc00 100644
--- a/cinder/volume/drivers/huawei/huawei_driver.py
+++ b/cinder/volume/drivers/huawei/huawei_driver.py
@@ -741,17 +741,11 @@ class HuaweiBaseDriver(driver.VolumeDriver):
         if snapshot_id is None:
             snapshot_id = self.client.get_snapshot_id_by_name(snapshotname)
 
-        if snapshot_id is not None:
-            if self.client.check_snapshot_exist(snapshot_id):
-                self.client.stop_snapshot(snapshot_id)
-                self.client.delete_snapshot(snapshot_id)
-            else:
-                LOG.warning(_LW("Can't find snapshot on the array."))
+        if snapshot_id and self.client.check_snapshot_exist(snapshot_id):
+            self.client.stop_snapshot(snapshot_id)
+            self.client.delete_snapshot(snapshot_id)
         else:
             LOG.warning(_LW("Can't find snapshot on the array."))
-            return False
-
-        return True
 
     def retype(self, ctxt, volume, new_type, diff, host):
         """Convert the volume to be of the new type."""