Browse Source

Allow drivers to ask for additional share_servers

Add new method choose_share_server_compatible_for_share()
to Share Driver interface. This method is executed in
Share Manager on share creation step to filter existing
share servers. Driver should choose one share server from
the list or return None. If this method returns None, then
Share Manager starts the creation of new share server.

Also, this commit adds a possibility to create share
from a snapshot on share server which differs from original
share server where the snapshot is placed.

Implements bp allow-additional-share-servers

Change-Id: Ia85fcf69b953c199808d7e7d3cb0652c2425fd9d
tags/1.0.0.0b1^2
Igor Malinovskiy 3 years ago
parent
commit
5254e52aee

+ 5
- 7
manila/db/api.py View File

@@ -625,14 +625,12 @@ def share_server_get_by_host_and_share_net(context, host, share_net_id,
625 625
                                                        session=session)
626 626
 
627 627
 
628
-def share_server_get_by_host_and_share_net_valid(context, host,
629
-                                                 share_net_id,
630
-                                                 session=None):
628
+def share_server_get_all_by_host_and_share_net_valid(context, host,
629
+                                                     share_net_id,
630
+                                                     session=None):
631 631
     """Get share server DB records by host and share net not error."""
632
-    return IMPL.share_server_get_by_host_and_share_net_valid(context,
633
-                                                             host,
634
-                                                             share_net_id,
635
-                                                             session=session)
632
+    return IMPL.share_server_get_all_by_host_and_share_net_valid(
633
+        context, host, share_net_id, session=session)
636 634
 
637 635
 
638 636
 def share_server_get_all(context):

+ 5
- 4
manila/db/sqlalchemy/api.py View File

@@ -2003,13 +2003,14 @@ def share_server_get(context, server_id, session=None):
2003 2003
 
2004 2004
 
2005 2005
 @require_context
2006
-def share_server_get_by_host_and_share_net_valid(context, host, share_net_id,
2007
-                                                 session=None):
2006
+def share_server_get_all_by_host_and_share_net_valid(context, host,
2007
+                                                     share_net_id,
2008
+                                                     session=None):
2008 2009
     result = _server_get_query(context, session).filter_by(host=host)\
2009 2010
         .filter_by(share_network_id=share_net_id)\
2010 2011
         .filter(models.ShareServer.status.in_(
2011
-            (constants.STATUS_CREATING, constants.STATUS_ACTIVE))).first()
2012
-    if result is None:
2012
+            (constants.STATUS_CREATING, constants.STATUS_ACTIVE))).all()
2013
+    if not result:
2013 2014
         filters_description = ('share_network_id is "%(share_net_id)s",'
2014 2015
                                ' host is "%(host)s" and status in'
2015 2016
                                ' "%(status_cr)s" or "%(status_act)s"') % {

+ 4
- 0
manila/exception.py View File

@@ -194,6 +194,10 @@ class ShareServerInUse(InUse):
194 194
     message = _("Share server %(share_server_id)s is in use.")
195 195
 
196 196
 
197
+class InvalidShareServer(Invalid):
198
+    message = _("Share server %(share_server_id)s is not valid.")
199
+
200
+
197 201
 class ShareServerNotCreated(ManilaException):
198 202
     message = _("Share server %(share_server_id)s failed on creation.")
199 203
 

+ 14
- 0
manila/share/driver.py View File

@@ -327,6 +327,20 @@ class ShareDriver(object):
327 327
         if self.get_network_allocations_number():
328 328
             self.network_api.deallocate_network(context, share_server_id)
329 329
 
330
+    def choose_share_server_compatible_with_share(self, context, share_servers,
331
+                                                  share, snapshot=None):
332
+        """Method that allows driver to choose share server for provided share.
333
+
334
+        If compatible share-server is not found, method should return None.
335
+
336
+        :param context: Current context
337
+        :param share_servers: list with share-server models
338
+        :param share:  share model
339
+        :param snapshot: snapshot model
340
+        :returns: share-server or None
341
+        """
342
+        return share_servers[0] if share_servers else None
343
+
330 344
     def setup_server(self, *args, **kwargs):
331 345
         if self.driver_handles_share_servers:
332 346
             return self._setup_server(*args, **kwargs)

+ 92
- 33
manila/share/manager.py View File

@@ -206,7 +206,7 @@ class ShareManager(manager.SchedulerDependentManager):
206 206
         self.publish_service_capabilities(ctxt)
207 207
 
208 208
     def _provide_share_server_for_share(self, context, share_network_id,
209
-                                        share_id):
209
+                                        share, snapshot=None):
210 210
         """Gets or creates share_server and updates share with its id.
211 211
 
212 212
         Active share_server can be deleted if there are no dependent shares
@@ -218,21 +218,87 @@ class ShareManager(manager.SchedulerDependentManager):
218 218
         For this purpose used shared lock between this method and the one
219 219
         with deletion of share_server.
220 220
 
221
+        :param context: Current context
222
+        :param share_network_id: Share network where existing share server
223
+                                 should be found or created. If
224
+                                 share_network_id is None method use
225
+                                 share_network_id from provided snapshot.
226
+        :param share: Share model
227
+        :param snapshot: Optional -- Snapshot model
228
+
221 229
         :returns: dict, dict -- first value is share_server, that
222 230
                   has been chosen for share schedule. Second value is
223 231
                   share updated with share_server_id.
224 232
         """
233
+        if not (share_network_id or snapshot):
234
+            msg = _("'share_network_id' parameter or 'snapshot'"
235
+                    " should be provided. ")
236
+            raise ValueError(msg)
237
+
238
+        parent_share_server = None
239
+
240
+        def error(msg, *args):
241
+            LOG.error(msg, *args)
242
+            self.db.share_update(context, share['id'],
243
+                                 {'status': constants.STATUS_ERROR})
244
+
245
+        if snapshot:
246
+            parent_share_server_id = snapshot['share']['share_server_id']
247
+            try:
248
+                parent_share_server = self.db.share_server_get(
249
+                    context, parent_share_server_id)
250
+            except exception.ShareServerNotFound:
251
+                with excutils.save_and_reraise_exception():
252
+                    error(_LE("Parent share server %s does not exist."),
253
+                          parent_share_server_id)
254
+
255
+            if parent_share_server['status'] != constants.STATUS_ACTIVE:
256
+                error_params = {
257
+                    'id': parent_share_server_id,
258
+                    'status': parent_share_server['status'],
259
+                }
260
+                error(_LE("Parent share server %(id)s has invalid status "
261
+                          "'%(status)s'."), error_params)
262
+                raise exception.InvalidShareServer(
263
+                    share_server_id=parent_share_server
264
+                )
265
+
266
+        if parent_share_server and not share_network_id:
267
+            share_network_id = parent_share_server['share_network_id']
268
+
269
+        def get_available_share_servers():
270
+            if parent_share_server:
271
+                return [parent_share_server]
272
+            else:
273
+                return (
274
+                    self.db.share_server_get_all_by_host_and_share_net_valid(
275
+                        context, self.host, share_network_id)
276
+                )
225 277
 
226 278
         @utils.synchronized("share_manager_%s" % share_network_id)
227 279
         def _provide_share_server_for_share():
228
-            exist = False
229 280
             try:
230
-                share_server = \
231
-                    self.db.share_server_get_by_host_and_share_net_valid(
232
-                        context, self.host, share_network_id)
233
-                exist = True
281
+                available_share_servers = get_available_share_servers()
234 282
             except exception.ShareServerNotFound:
235
-                share_server = self.db.share_server_create(
283
+                available_share_servers = None
284
+
285
+            compatible_share_server = None
286
+
287
+            if available_share_servers:
288
+                try:
289
+                    compatible_share_server = (
290
+                        self.driver.choose_share_server_compatible_with_share(
291
+                            context, available_share_servers, share,
292
+                            snapshot=snapshot
293
+                        )
294
+                    )
295
+                except Exception as e:
296
+                    with excutils.save_and_reraise_exception():
297
+                        error(_LE("Cannot choose compatible share-server: %s"),
298
+                              e)
299
+
300
+            if not compatible_share_server:
301
+                compatible_share_server = self.db.share_server_create(
236 302
                     context,
237 303
                     {
238 304
                         'host': self.host,
@@ -241,23 +307,28 @@ class ShareManager(manager.SchedulerDependentManager):
241 307
                     }
242 308
                 )
243 309
 
244
-            LOG.debug("Using share_server %s for share %s" % (
245
-                share_server['id'], share_id))
310
+            msg = "Using share_server %(share_server)s for share %(share_id)s"
311
+            LOG.debug(msg, {
312
+                'share_server': compatible_share_server['id'],
313
+                'share_id': share['id']
314
+            })
315
+
246 316
             share_ref = self.db.share_update(
247 317
                 context,
248
-                share_id,
249
-                {'share_server_id': share_server['id']},
318
+                share['id'],
319
+                {'share_server_id': compatible_share_server['id']},
250 320
             )
251 321
 
252
-            if not exist:
253
-                # Create share server on backend with data from db
254
-                share_server = self._setup_server(context, share_server)
322
+            if compatible_share_server['status'] == constants.STATUS_CREATING:
323
+                # Create share server on backend with data from db.
324
+                compatible_share_server = self._setup_server(
325
+                    context, compatible_share_server)
255 326
                 LOG.info(_LI("Share server created successfully."))
256 327
             else:
257
-                LOG.info(_LI("Used already existed share server "
328
+                LOG.info(_LI("Used preexisting share server "
258 329
                              "'%(share_server_id)s'"),
259
-                         {'share_server_id': share_server['id']})
260
-            return share_server, share_ref
330
+                         {'share_server_id': compatible_share_server['id']})
331
+            return compatible_share_server, share_ref
261 332
 
262 333
         return _provide_share_server_for_share()
263 334
 
@@ -292,24 +363,12 @@ class ShareManager(manager.SchedulerDependentManager):
292 363
             snapshot_ref = None
293 364
             parent_share_server_id = None
294 365
 
295
-        if parent_share_server_id:
296
-            try:
297
-                share_server = self.db.share_server_get(context,
298
-                                                        parent_share_server_id)
299
-                LOG.debug("Using share_server "
300
-                          "%s for share %s" % (share_server['id'], share_id))
301
-                share_ref = self.db.share_update(
302
-                    context, share_id, {'share_server_id': share_server['id']})
303
-            except exception.ShareServerNotFound:
304
-                with excutils.save_and_reraise_exception():
305
-                    LOG.error(_LE("Share server %s does not exist."),
306
-                              parent_share_server_id)
307
-                    self.db.share_update(context, share_id,
308
-                                         {'status': constants.STATUS_ERROR})
309
-        elif share_network_id:
366
+        if share_network_id or parent_share_server_id:
310 367
             try:
311 368
                 share_server, share_ref = self._provide_share_server_for_share(
312
-                    context, share_network_id, share_id)
369
+                    context, share_network_id, share_ref,
370
+                    snapshot=snapshot_ref
371
+                )
313 372
             except Exception:
314 373
                 with excutils.save_and_reraise_exception():
315 374
                     LOG.error(_LE("Failed to get share server"

+ 15
- 0
manila/tests/share/drivers/test_generic.py View File

@@ -1360,6 +1360,21 @@ class GenericShareDriverTestCase(test.TestCase):
1360 1360
             fake_server_details, ['sudo', 'resize2fs', '/dev/fake'])
1361 1361
         self.assertEqual(2, self._driver._ssh_exec.call_count)
1362 1362
 
1363
+    @ddt.data({'share_servers': [], 'result': None},
1364
+              {'share_servers': None, 'result': None},
1365
+              {'share_servers': ['fake'], 'result': 'fake'},
1366
+              {'share_servers': ['fake', 'test'], 'result': 'fake'})
1367
+    @ddt.unpack
1368
+    def tests_choose_share_server_compatible_with_share(self, share_servers,
1369
+                                                        result):
1370
+        fake_share = "fake"
1371
+
1372
+        actual_result = self._driver.choose_share_server_compatible_with_share(
1373
+            self._context, share_servers, fake_share
1374
+        )
1375
+
1376
+        self.assertEqual(result, actual_result)
1377
+
1363 1378
 
1364 1379
 @generic.ensure_server
1365 1380
 def fake(driver_instance, context, share_server=None):

+ 78
- 6
manila/tests/share/test_manager.py View File

@@ -570,7 +570,10 @@ class ShareManagerTestCase(test.TestCase):
570 570
 
571 571
     def test_create_share_with_share_network_server_creation_failed(self):
572 572
         fake_share = {'id': 'fake_share_id', 'share_network_id': 'fake_sn_id'}
573
-        fake_server = {'id': 'fake_srv_id'}
573
+        fake_server = {
574
+            'id': 'fake_srv_id',
575
+            'status': constants.STATUS_CREATING,
576
+        }
574 577
         self.mock_object(db, 'share_server_create',
575 578
                          mock.Mock(return_value=fake_server))
576 579
         self.mock_object(db, 'share_update',
@@ -585,7 +588,8 @@ class ShareManagerTestCase(test.TestCase):
585 588
         def raise_manila_exception(*args, **kwargs):
586 589
             raise exception.ManilaException()
587 590
 
588
-        self.mock_object(db, 'share_server_get_by_host_and_share_net_valid',
591
+        self.mock_object(db,
592
+                         'share_server_get_all_by_host_and_share_net_valid',
589 593
                          mock.Mock(side_effect=raise_share_server_not_found))
590 594
         self.mock_object(self.share_manager, '_setup_server',
591 595
                          mock.Mock(side_effect=raise_manila_exception))
@@ -596,7 +600,7 @@ class ShareManagerTestCase(test.TestCase):
596 600
             self.context,
597 601
             fake_share['id'],
598 602
         )
599
-        db.share_server_get_by_host_and_share_net_valid.\
603
+        db.share_server_get_all_by_host_and_share_net_valid.\
600 604
             assert_called_once_with(
601 605
                 utils.IsAMatcher(context.RequestContext),
602 606
                 self.share_manager.host,
@@ -642,8 +646,12 @@ class ShareManagerTestCase(test.TestCase):
642 646
 
643 647
         share_id = share['id']
644 648
 
645
-        self.share_manager.driver = mock.Mock()
646
-        self.share_manager.driver.create_share.return_value = "fake_location"
649
+        driver_mock = mock.Mock()
650
+        driver_mock.create_share.return_value = "fake_location"
651
+        driver_mock.choose_share_server_compatible_with_share.return_value = (
652
+            share_srv
653
+        )
654
+        self.share_manager.driver = driver_mock
647 655
         self.share_manager.create_share(self.context, share_id)
648 656
         self.assertFalse(self.share_manager.driver.setup_network.called)
649 657
         self.assertEqual(share_id, db.share_get(context.get_admin_context(),
@@ -682,7 +690,10 @@ class ShareManagerTestCase(test.TestCase):
682 690
             share_network_id=share_net['id'], host=self.share_manager.host,
683 691
             state=constants.STATUS_ERROR)
684 692
         share_id = share['id']
685
-        fake_server = {'id': 'fake_srv_id'}
693
+        fake_server = {
694
+            'id': 'fake_srv_id',
695
+            'status': constants.STATUS_CREATING,
696
+        }
686 697
         self.mock_object(db, 'share_server_create',
687 698
                          mock.Mock(return_value=fake_server))
688 699
         self.mock_object(self.share_manager, '_setup_server',
@@ -736,6 +747,67 @@ class ShareManagerTestCase(test.TestCase):
736 747
             utils.IsAMatcher(models.Share),
737 748
             share_server=None)
738 749
 
750
+    def test_provide_share_server_for_share_incompatible_servers(self):
751
+        fake_exception = exception.ManilaException("fake")
752
+        fake_share_server = {'id': 'fake'}
753
+        share = self._create_share()
754
+
755
+        self.mock_object(db,
756
+                         'share_server_get_all_by_host_and_share_net_valid',
757
+                         mock.Mock(return_value=[fake_share_server]))
758
+        self.mock_object(
759
+            self.share_manager.driver,
760
+            "choose_share_server_compatible_with_share",
761
+            mock.Mock(side_effect=fake_exception)
762
+        )
763
+
764
+        self.assertRaises(exception.ManilaException,
765
+                          self.share_manager._provide_share_server_for_share,
766
+                          self.context, "fake_id", share)
767
+        driver_mock = self.share_manager.driver
768
+        driver_method_mock = (
769
+            driver_mock.choose_share_server_compatible_with_share
770
+        )
771
+        driver_method_mock.assert_called_once_with(
772
+            self.context, [fake_share_server], share, snapshot=None)
773
+
774
+    def test_provide_share_server_for_share_invalid_arguments(self):
775
+        self.assertRaises(ValueError,
776
+                          self.share_manager._provide_share_server_for_share,
777
+                          self.context, None, None)
778
+
779
+    def test_provide_share_server_for_share_parent_ss_not_found(self):
780
+        fake_parent_id = "fake_server_id"
781
+        fake_exception = exception.ShareServerNotFound("fake")
782
+        share = self._create_share()
783
+        fake_snapshot = {'share': {'share_server_id': fake_parent_id}}
784
+        self.mock_object(db, 'share_server_get',
785
+                         mock.Mock(side_effect=fake_exception))
786
+
787
+        self.assertRaises(exception.ShareServerNotFound,
788
+                          self.share_manager._provide_share_server_for_share,
789
+                          self.context, "fake_id", share,
790
+                          snapshot=fake_snapshot)
791
+
792
+        db.share_server_get.assert_called_once_with(
793
+            self.context, fake_parent_id)
794
+
795
+    def test_provide_share_server_for_share_parent_ss_invalid(self):
796
+        fake_parent_id = "fake_server_id"
797
+        share = self._create_share()
798
+        fake_snapshot = {'share': {'share_server_id': fake_parent_id}}
799
+        fake_parent_share_server = {'status': 'fake'}
800
+        self.mock_object(db, 'share_server_get',
801
+                         mock.Mock(return_value=fake_parent_share_server))
802
+
803
+        self.assertRaises(exception.InvalidShareServer,
804
+                          self.share_manager._provide_share_server_for_share,
805
+                          self.context, "fake_id", share,
806
+                          snapshot=fake_snapshot)
807
+
808
+        db.share_server_get.assert_called_once_with(
809
+            self.context, fake_parent_id)
810
+
739 811
     def test_manage_share_invalid_driver(self):
740 812
         self.mock_object(self.share_manager, 'driver', mock.Mock())
741 813
         self.share_manager.driver.driver_handles_share_servers = True

+ 5
- 5
manila/tests/test_db.py View File

@@ -105,7 +105,7 @@ class ShareServerTableTestCase(test.TestCase):
105 105
                           db.share_server_update,
106 106
                           self.ctxt, fake_id, {})
107 107
 
108
-    def test_share_server_get_by_host_and_share_net_valid(self):
108
+    def test_share_server_get_all_by_host_and_share_net_valid(self):
109 109
         valid = {
110 110
             'share_network_id': '1',
111 111
             'host': 'host1',
@@ -125,15 +125,15 @@ class ShareServerTableTestCase(test.TestCase):
125 125
         self._create_share_server(invalid)
126 126
         self._create_share_server(other)
127 127
 
128
-        servers = db.share_server_get_by_host_and_share_net_valid(
128
+        servers = db.share_server_get_all_by_host_and_share_net_valid(
129 129
             self.ctxt,
130 130
             host='host1',
131 131
             share_net_id='1')
132
-        self.assertEqual(servers['id'], valid['id'])
132
+        self.assertEqual(servers[0]['id'], valid['id'])
133 133
 
134
-    def test_share_server_get_by_host_and_share_net_not_found(self):
134
+    def test_share_server_get_all_by_host_and_share_net_not_found(self):
135 135
         self.assertRaises(exception.ShareServerNotFound,
136
-                          db.share_server_get_by_host_and_share_net_valid,
136
+                          db.share_server_get_all_by_host_and_share_net_valid,
137 137
                           self.ctxt, host='fake', share_net_id='fake')
138 138
 
139 139
     def test_share_server_get_all(self):

Loading…
Cancel
Save