Browse Source

QNAP: Fix inconsistent cases while create/manage from snapshot

There are two situation may cause the size of share/snapshot
managed by manila is inconsistent with the NAS backend.
One is to create a share from snapshot. While the other one
is to manage an existing snapshot.

Change-Id: Iaef8d8cb4be0d8872a2796c0fc69279c14f15a80
Closes-Bug: #1810476
tags/8.0.0.0rc1
Chris Yang 7 months ago
parent
commit
ad62e9dde3

+ 2
- 1
manila/share/drivers/qnap/api.py View File

@@ -467,13 +467,14 @@ class QnapAPIExecutor(object):
467 467
             raise exception.ShareBackendException(msg=msg)
468 468
 
469 469
     @_connection_checker
470
-    def clone_snapshot(self, snapshot_id, new_sharename):
470
+    def clone_snapshot(self, snapshot_id, new_sharename, clone_size):
471 471
         """Execute CGI to clone snapshot as share."""
472 472
         params = {
473 473
             'func': 'clone_qsnapshot',
474 474
             'by_vol': '1',
475 475
             'snapshotID': snapshot_id,
476 476
             'new_name': new_sharename,
477
+            'clone_size': '{}g'.format(clone_size),
477 478
             'sid': self.sid,
478 479
         }
479 480
         sanitized_params = self._sanitize_params(params)

+ 9
- 18
manila/share/drivers/qnap/qnap.py View File

@@ -77,6 +77,8 @@ class QnapShareDriver(driver.ShareDriver):
77 77
         1.0.7 - Add support for QES fw on TDS series NAS model.
78 78
         1.0.8 - Fix bug, driver should not manage snapshot which does not
79 79
                 exist in NAS.
80
+                Fix bug, driver should create share from snapshot with
81
+                specified size.
80 82
     """
81 83
 
82 84
     DRIVER_VERSION = '1.0.8'
@@ -540,7 +542,8 @@ class QnapShareDriver(driver.ShareDriver):
540 542
             msg = _("Failed to create an unused share name.")
541 543
             raise exception.ShareBackendException(msg=msg)
542 544
 
543
-        self.api_executor.clone_snapshot(snapshot_id, create_share_name)
545
+        self.api_executor.clone_snapshot(snapshot_id,
546
+                                         create_share_name, share['size'])
544 547
 
545 548
         create_volID = ""
546 549
         created_share = self.api_executor.get_share_info(
@@ -553,10 +556,6 @@ class QnapShareDriver(driver.ShareDriver):
553 556
             msg = _("Failed to clone a snapshot in time.")
554 557
             raise exception.ShareBackendException(msg=msg)
555 558
 
556
-        snap_share = self.share_api.get(
557
-            context, snapshot['share_instance']['share_id'])
558
-        LOG.debug('snap_share[size]: %s', snap_share['size'])
559
-
560 559
         thin_provision = self.private_storage.get(
561 560
             snapshot['share_instance_id'], 'thin_provision')
562 561
         compression = self.private_storage.get(
@@ -574,19 +573,6 @@ class QnapShareDriver(driver.ShareDriver):
574 573
                    'deduplication': deduplication,
575 574
                    'ssd_cache': ssd_cache})
576 575
 
577
-        if (share['size'] > snap_share['size']):
578
-            share_dict = {
579
-                'sharename': create_share_name,
580
-                'old_sharename': create_share_name,
581
-                'new_size': share['size'],
582
-                'thin_provision': thin_provision == 'True',
583
-                'compression': compression == 'True',
584
-                'deduplication': deduplication == 'True',
585
-                'ssd_cache': ssd_cache == 'True',
586
-                'share_proto': share['share_proto']
587
-            }
588
-            self.api_executor.edit_share(share_dict)
589
-
590 576
         # Use private_storage to record volume ID and Name created in the NAS.
591 577
         _metadata = {
592 578
             'volID': create_volID,
@@ -930,6 +916,11 @@ class QnapShareDriver(driver.ShareDriver):
930 916
             'snapshot_id': snapshot_id,
931 917
         }
932 918
         self.private_storage.update(snapshot['id'], _metadata)
919
+        parent_size = check_snapshot.find('parent_size')
920
+        snap_size_gb = None
921
+        if parent_size is not None:
922
+            snap_size_gb = math.ceil(float(parent_size.text) / units.Gi)
923
+        return {'size': snap_size_gb}
933 924
 
934 925
     def unmanage_snapshot(self, snapshot):
935 926
         """Remove the specified snapshot from Manila management."""

+ 1
- 106
manila/tests/share/drivers/qnap/fakes.py View File

@@ -173,62 +173,6 @@ FAKE_RES_DETAIL_DATA_GETBASIC_INFO_TES_ES_2_2_0 = """
173 173
         </firmware>
174 174
     </QDocRoot>"""
175 175
 
176
-FAKE_RES_DETAIL_DATA_GETBASIC_INFO_TDS_TS_4_0_0 = """
177
-    <QDocRoot version="1.0">
178
-        <model>
179
-            <displayModelName><![CDATA[TDS-16489U R2]]></displayModelName>
180
-            <internalModelName><![CDATA[TS-X89]]></internalModelName>
181
-        </model>
182
-        <firmware>
183
-            <version><![CDATA[4.0.0]]></version>
184
-        </firmware>
185
-    </QDocRoot>"""
186
-
187
-FAKE_RES_DETAIL_DATA_GETBASIC_INFO_TDS_TS_4_3_0 = """
188
-    <QDocRoot version="1.0">
189
-        <model>
190
-            <displayModelName><![CDATA[TDS-16489U R2]]></displayModelName>
191
-            <internalModelName><![CDATA[TS-X89]]></internalModelName>
192
-        </model>
193
-        <firmware>
194
-            <version><![CDATA[4.3.0]]></version>
195
-        </firmware>
196
-    </QDocRoot>"""
197
-
198
-FAKE_RES_DETAIL_DATA_GETBASIC_INFO_TDS_ES_1_1_1 = """
199
-    <QDocRoot version="1.0">
200
-        <model>
201
-            <displayModelName><![CDATA[TDS-16489U R2]]></displayModelName>
202
-            <internalModelName><![CDATA[ES-X85U]]></internalModelName>
203
-        </model>
204
-        <firmware>
205
-            <version><![CDATA[1.1.1]]></version>
206
-        </firmware>
207
-    </QDocRoot>"""
208
-
209
-FAKE_RES_DETAIL_DATA_GETBASIC_INFO_TDS_ES_2_1_0 = """
210
-    <QDocRoot version="1.0">
211
-        <model>
212
-            <displayModelName><![CDATA[TDS-16489U R2]]></displayModelName>
213
-            <internalModelName><![CDATA[ES-X85U]]></internalModelName>
214
-        </model>
215
-        <firmware>
216
-            <version><![CDATA[2.1.0]]></version>
217
-        </firmware>
218
-    </QDocRoot>"""
219
-
220
-FAKE_RES_DETAIL_DATA_GETBASIC_INFO_TDS_ES_2_2_0 = """
221
-    <QDocRoot version="1.0">
222
-        <model>
223
-            <displayModelName><![CDATA[TDS-16489U R2]]></displayModelName>
224
-            <internalModelName><![CDATA[ES-X85U]]></internalModelName>
225
-        </model>
226
-        <firmware>
227
-            <version><![CDATA[2.2.0]]></version>
228
-        </firmware>
229
-    </QDocRoot>"""
230
-
231
-
232 176
 FAKE_RES_DETAIL_DATA_GETBASIC_INFO_ERROR = """
233 177
     <QDocRoot version="1.0">
234 178
         <model>
@@ -276,6 +220,7 @@ FAKE_RES_DETAIL_DATA_SNAPSHOT = """
276 220
             <row>
277 221
                 <snapshot_id><![CDATA[fakeSnapshotId]]></snapshot_id>
278 222
                 <snapshot_name><![CDATA[fakeSnapshotName]]></snapshot_name>
223
+                <parent_size>10</parent_size>
279 224
             </row>
280 225
         </SnapshotList>
281 226
         <result><![CDATA[0]]></result>
@@ -681,56 +626,6 @@ class FakeGetBasicInfoResponseTesEs_2_2_0(object):
681 626
         return FAKE_RES_DETAIL_DATA_GETBASIC_INFO_TES_ES_2_2_0
682 627
 
683 628
 
684
-class FakeGetBasicInfoResponseTdsTs_4_0_0(object):
685
-    """Fake GetBasicInfoTS response from TS nas."""
686
-
687
-    status = 'fackStatus'
688
-
689
-    def read(self):
690
-        """Mock response.read."""
691
-        return FAKE_RES_DETAIL_DATA_GETBASIC_INFO_TDS_TS_4_0_0
692
-
693
-
694
-class FakeGetBasicInfoResponseTdsTs_4_3_0(object):
695
-    """Fake GetBasicInfoTS response from TS nas."""
696
-
697
-    status = 'fackStatus'
698
-
699
-    def read(self):
700
-        """Mock response.read."""
701
-        return FAKE_RES_DETAIL_DATA_GETBASIC_INFO_TDS_TS_4_3_0
702
-
703
-
704
-class FakeGetBasicInfoResponseTdsEs_1_1_1(object):
705
-    """Fake GetBasicInfoTS response from ES nas."""
706
-
707
-    status = 'fackStatus'
708
-
709
-    def read(self):
710
-        """Mock response.read."""
711
-        return FAKE_RES_DETAIL_DATA_GETBASIC_INFO_TDS_ES_1_1_1
712
-
713
-
714
-class FakeGetBasicInfoResponseTdsEs_2_1_0(object):
715
-    """Fake GetBasicInfoTS response from ES nas."""
716
-
717
-    status = 'fackStatus'
718
-
719
-    def read(self):
720
-        """Mock response.read."""
721
-        return FAKE_RES_DETAIL_DATA_GETBASIC_INFO_TDS_ES_2_1_0
722
-
723
-
724
-class FakeGetBasicInfoResponseTdsEs_2_2_0(object):
725
-    """Fake GetBasicInfoTS response from ES nas."""
726
-
727
-    status = 'fackStatus'
728
-
729
-    def read(self):
730
-        """Mock response.read."""
731
-        return FAKE_RES_DETAIL_DATA_GETBASIC_INFO_TDS_ES_2_2_0
732
-
733
-
734 629
 class FakeGetBasicInfoResponseError(object):
735 630
     """Fake GetBasicInfoTS response from TS nas."""
736 631
 

+ 7
- 3
manila/tests/share/drivers/qnap/test_api.py View File

@@ -424,13 +424,15 @@ class QnapAPITestCase(QnapShareDriverBaseTestCase):
424 424
                        'qnapadmin', 'Storage Pool 1')
425 425
         self.driver.api_executor.clone_snapshot(
426 426
             'fakeSnapshotId',
427
-            'fakeNewShareName')
427
+            'fakeNewShareName',
428
+            'fakeCloneSize')
428 429
 
429 430
         fake_params = {
430 431
             'func': 'clone_qsnapshot',
431 432
             'by_vol': '1',
432 433
             'snapshotID': 'fakeSnapshotId',
433 434
             'new_name': 'fakeNewShareName',
435
+            'clone_size': '{}g'.format('fakeCloneSize'),
434 436
             'sid': 'fakeSid',
435 437
         }
436 438
         sanitized_params = self._sanitize_params(fake_params)
@@ -804,12 +806,14 @@ class QnapAPITestCase(QnapShareDriverBaseTestCase):
804 806
               fakes.FakeGetBasicInfoResponseEs_1_1_3()],
805 807
               ['self.driver.api_executor.clone_snapshot',
806 808
               {'snapshot_id': 'fakeSnapshotId',
807
-               'new_sharename': 'fakeNewShareName'},
809
+               'new_sharename': 'fakeNewShareName',
810
+               'clone_size': 'fakeCloneSize'},
808 811
               fakes.FakeResultNegativeResponse(),
809 812
               fakes.FakeGetBasicInfoResponseEs_1_1_3()],
810 813
               ['self.driver.api_executor.clone_snapshot',
811 814
               {'snapshot_id': 'fakeSnapshotId',
812
-               'new_sharename': 'fakeNewShareName'},
815
+               'new_sharename': 'fakeNewShareName',
816
+               'clone_size': 'fakeCloneSize'},
813 817
               fakes.FakeAuthPassFailResponse(),
814 818
               fakes.FakeGetBasicInfoResponseEs_1_1_3()],
815 819
               ['self.driver.api_executor.edit_share',

+ 4
- 63
manila/tests/share/drivers/qnap/test_qnap.py View File

@@ -159,12 +159,6 @@ class QnapShareDriverLoginTestCase(QnapShareDriverBaseTestCase):
159 159
     }, {
160 160
         'fake_basic_info': fakes.FakeGetBasicInfoResponseTesEs_2_1_0(),
161 161
         'expect_result': api.QnapAPIExecutor
162
-    }, {
163
-        'fake_basic_info': fakes.FakeGetBasicInfoResponseTdsTs_4_3_0(),
164
-        'expect_result': api.QnapAPIExecutorTS
165
-    }, {
166
-        'fake_basic_info': fakes.FakeGetBasicInfoResponseTdsEs_2_1_0(),
167
-        'expect_result': api.QnapAPIExecutor
168 162
     }, {
169 163
         'fake_basic_info': fakes.FakeGetBasicInfoResponseEs_1_1_3(),
170 164
         'expect_result': api.QnapAPIExecutor
@@ -200,15 +194,6 @@ class QnapShareDriverLoginTestCase(QnapShareDriverBaseTestCase):
200 194
     }, {
201 195
         'fake_basic_info': fakes.FakeGetBasicInfoResponseTesEs_2_2_0(),
202 196
         'expect_result': exception.ShareBackendException
203
-    }, {
204
-        'fake_basic_info': fakes.FakeGetBasicInfoResponseTdsTs_4_0_0(),
205
-        'expect_result': exception.ShareBackendException
206
-    }, {
207
-        'fake_basic_info': fakes.FakeGetBasicInfoResponseTdsEs_1_1_1(),
208
-        'expect_result': exception.ShareBackendException
209
-    }, {
210
-        'fake_basic_info': fakes.FakeGetBasicInfoResponseTdsEs_2_2_0(),
211
-        'expect_result': exception.ShareBackendException
212 197
     }, {
213 198
         'fake_basic_info': fakes.FakeGetBasicInfoResponseEs_1_1_1(),
214 199
         'expect_result': exception.ShareBackendException
@@ -730,15 +715,13 @@ class QnapShareDriverTestCase(QnapShareDriverBaseTestCase):
730 715
             expected_call_list,
731 716
             mock_api_return.get_share_info.call_args_list)
732 717
         mock_api_return.clone_snapshot.assert_called_once_with(
733
-            'fakeShareName@fakeSnapshotName', 'fakeShareName')
718
+            'fakeShareName@fakeSnapshotName', 'fakeShareName', 10)
734 719
 
735 720
     @mock.patch.object(qnap.QnapShareDriver, '_get_location_path')
736
-    @mock.patch('manila.share.API')
737 721
     @mock.patch.object(qnap.QnapShareDriver, '_gen_random_name')
738 722
     def test_create_share_from_snapshot_diff_size(
739 723
             self,
740 724
             mock_gen_random_name,
741
-            mock_share_api,
742 725
             mock_get_location_path):
743 726
         """Test create share from snapshot."""
744 727
         fake_snapshot = fakes.SnapshotClass(
@@ -755,9 +738,6 @@ class QnapShareDriverTestCase(QnapShareDriverBaseTestCase):
755 738
             'False',
756 739
             'False',
757 740
             'fakeVolName']
758
-        mock_share_api.return_value.get.return_value = {'size': 5}
759
-        mock_api_executor.return_value.edit_share.return_value = (
760
-            None)
761 741
 
762 742
         self._do_setup('http://1.2.3.4:8080', '1.2.3.4', 'admin',
763 743
                        'qnapadmin', 'Storage Pool 1',
@@ -775,19 +755,7 @@ class QnapShareDriverTestCase(QnapShareDriverBaseTestCase):
775 755
             expected_call_list,
776 756
             mock_api_return.get_share_info.call_args_list)
777 757
         mock_api_return.clone_snapshot.assert_called_once_with(
778
-            'fakeShareName@fakeSnapshotName', 'fakeShareName')
779
-        expect_share_dict = {
780
-            'sharename': 'fakeShareName',
781
-            'old_sharename': 'fakeShareName',
782
-            'new_size': 10,
783
-            'thin_provision': True,
784
-            'compression': True,
785
-            'deduplication': False,
786
-            'ssd_cache': False,
787
-            'share_proto': 'NFS'
788
-        }
789
-        mock_api_return.edit_share.assert_called_once_with(
790
-            expect_share_dict)
758
+            'fakeShareName@fakeSnapshotName', 'fakeShareName', 10)
791 759
 
792 760
     def test_create_share_from_snapshot_without_snapshot_id(self):
793 761
         """Test create share from snapshot."""
@@ -1129,6 +1097,8 @@ class QnapShareDriverTestCase(QnapShareDriverBaseTestCase):
1129 1097
         mock_api_executor = qnap.QnapShareDriver._create_api_executor
1130 1098
         mock_api_executor.return_value.get_share_info.return_value = (
1131 1099
             self.get_share_info_return_value())
1100
+        mock_api_executor.return_value.get_snapshot_info.return_value = (
1101
+            self.get_snapshot_info_return_value())
1132 1102
         mock_private_storage = mock.Mock()
1133 1103
         mock_private_storage.update.return_value = None
1134 1104
         mock_private_storage.get.side_effect = [
@@ -1147,35 +1117,6 @@ class QnapShareDriverTestCase(QnapShareDriverBaseTestCase):
1147 1117
         mock_private_storage.update.assert_called_once_with(
1148 1118
             'fakeSnapshotId', fake_metadata)
1149 1119
 
1150
-    @mock.patch.object(qnap.QnapShareDriver, '_get_location_path')
1151
-    def test_manage_existing_snapshot_not_exist(
1152
-            self,
1153
-            mock_get_location_path):
1154
-        """Test manage existing snapshot with snapshot which does not exist."""
1155
-        fake_snapshot = fakes.SnapshotClass(
1156
-            10, 'fakeShareName@fakeSnapshotName')
1157
-
1158
-        mock_api_executor = qnap.QnapShareDriver._create_api_executor
1159
-        mock_api_executor.return_value.get_share_info.return_value = (
1160
-            self.get_share_info_return_value())
1161
-        mock_api_executor.return_value.get_snapshot_info.return_value = None
1162
-        mock_private_storage = mock.Mock()
1163
-        mock_private_storage.get.side_effect = [
1164
-            'fakeVolId', 'fakeVolName']
1165
-
1166
-        self._do_setup('http://1.2.3.4:8080', '1.2.3.4', 'admin',
1167
-                       'qnapadmin', 'Storage Pool 1',
1168
-                       private_storage=mock_private_storage)
1169
-
1170
-        mock_api_return = mock_api_executor.return_value
1171
-        self.assertRaises(
1172
-            exception.InvalidParameterValue,
1173
-            self.driver.manage_existing_snapshot,
1174
-            snapshot=fake_snapshot,
1175
-            driver_options='driver_options')
1176
-        mock_api_return.get_share_info.assert_called_once_with(
1177
-            'Storage Pool 1', vol_no='fakeVolId')
1178
-
1179 1120
     def test_unmanage_snapshot(self):
1180 1121
         """Test unmanage snapshot."""
1181 1122
         fake_snapshot = fakes.SnapshotClass(

+ 5
- 0
releasenotes/notes/qnap-fix-share-and-snapshot-inconsistant-bd628c6e14eeab14.yaml View File

@@ -0,0 +1,5 @@
1
+---
2
+fixes:
3
+  - |
4
+    Fixed the QNAP driver so that the managed snapshot and the share which
5
+    created from snapshot will not be inconsistent in some cases.

Loading…
Cancel
Save