summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorIgor Malinovskiy <imalinovskiy@mirantis.com>2015-06-22 11:11:31 +0300
committerIgor Malinovskiy <imalinovskiy@mirantis.com>2015-06-22 11:11:31 +0300
commit5254e52aeecb21f72a2580fa7412abaf319257dd (patch)
tree40ecb265d720800d6bcece25098ce10f1c4493cc
parentb7ab5553faf0eef79ba7324e782e9420e3d5bc4f (diff)
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
Notes
Notes (review): Verified+2: Jenkins Code-Review+2: Ben Swartzlander <ben@swartzlander.org> Code-Review+2: xing-yang <xing.yang@emc.com> Code-Review+2: Mark Sturdevant <mark.sturdevant@hp.com> Workflow+1: Mark Sturdevant <mark.sturdevant@hp.com> Submitted-by: Jenkins Submitted-at: Tue, 23 Jun 2015 23:58:37 +0000 Reviewed-on: https://review.openstack.org/177834 Project: openstack/manila Branch: refs/heads/master
-rw-r--r--manila/db/api.py12
-rw-r--r--manila/db/sqlalchemy/api.py9
-rw-r--r--manila/exception.py4
-rw-r--r--manila/share/driver.py14
-rw-r--r--manila/share/manager.py125
-rw-r--r--manila/tests/share/drivers/test_generic.py15
-rw-r--r--manila/tests/share/test_manager.py84
-rw-r--r--manila/tests/test_db.py10
8 files changed, 218 insertions, 55 deletions
diff --git a/manila/db/api.py b/manila/db/api.py
index a9d384a..f9de026 100644
--- a/manila/db/api.py
+++ b/manila/db/api.py
@@ -625,14 +625,12 @@ def share_server_get_by_host_and_share_net(context, host, share_net_id,
625 session=session) 625 session=session)
626 626
627 627
628def share_server_get_by_host_and_share_net_valid(context, host, 628def share_server_get_all_by_host_and_share_net_valid(context, host,
629 share_net_id, 629 share_net_id,
630 session=None): 630 session=None):
631 """Get share server DB records by host and share net not error.""" 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, 632 return IMPL.share_server_get_all_by_host_and_share_net_valid(
633 host, 633 context, host, share_net_id, session=session)
634 share_net_id,
635 session=session)
636 634
637 635
638def share_server_get_all(context): 636def share_server_get_all(context):
diff --git a/manila/db/sqlalchemy/api.py b/manila/db/sqlalchemy/api.py
index b6cfae7..f025c0e 100644
--- a/manila/db/sqlalchemy/api.py
+++ b/manila/db/sqlalchemy/api.py
@@ -2003,13 +2003,14 @@ def share_server_get(context, server_id, session=None):
2003 2003
2004 2004
2005@require_context 2005@require_context
2006def share_server_get_by_host_and_share_net_valid(context, host, share_net_id, 2006def share_server_get_all_by_host_and_share_net_valid(context, host,
2007 session=None): 2007 share_net_id,
2008 session=None):
2008 result = _server_get_query(context, session).filter_by(host=host)\ 2009 result = _server_get_query(context, session).filter_by(host=host)\
2009 .filter_by(share_network_id=share_net_id)\ 2010 .filter_by(share_network_id=share_net_id)\
2010 .filter(models.ShareServer.status.in_( 2011 .filter(models.ShareServer.status.in_(
2011 (constants.STATUS_CREATING, constants.STATUS_ACTIVE))).first() 2012 (constants.STATUS_CREATING, constants.STATUS_ACTIVE))).all()
2012 if result is None: 2013 if not result:
2013 filters_description = ('share_network_id is "%(share_net_id)s",' 2014 filters_description = ('share_network_id is "%(share_net_id)s",'
2014 ' host is "%(host)s" and status in' 2015 ' host is "%(host)s" and status in'
2015 ' "%(status_cr)s" or "%(status_act)s"') % { 2016 ' "%(status_cr)s" or "%(status_act)s"') % {
diff --git a/manila/exception.py b/manila/exception.py
index 6ec78c4..b3d024e 100644
--- a/manila/exception.py
+++ b/manila/exception.py
@@ -194,6 +194,10 @@ class ShareServerInUse(InUse):
194 message = _("Share server %(share_server_id)s is in use.") 194 message = _("Share server %(share_server_id)s is in use.")
195 195
196 196
197class InvalidShareServer(Invalid):
198 message = _("Share server %(share_server_id)s is not valid.")
199
200
197class ShareServerNotCreated(ManilaException): 201class ShareServerNotCreated(ManilaException):
198 message = _("Share server %(share_server_id)s failed on creation.") 202 message = _("Share server %(share_server_id)s failed on creation.")
199 203
diff --git a/manila/share/driver.py b/manila/share/driver.py
index e3345d6..d1a7ae3 100644
--- a/manila/share/driver.py
+++ b/manila/share/driver.py
@@ -327,6 +327,20 @@ class ShareDriver(object):
327 if self.get_network_allocations_number(): 327 if self.get_network_allocations_number():
328 self.network_api.deallocate_network(context, share_server_id) 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 def setup_server(self, *args, **kwargs): 344 def setup_server(self, *args, **kwargs):
331 if self.driver_handles_share_servers: 345 if self.driver_handles_share_servers:
332 return self._setup_server(*args, **kwargs) 346 return self._setup_server(*args, **kwargs)
diff --git a/manila/share/manager.py b/manila/share/manager.py
index 115ec2f..19a0e2f 100644
--- a/manila/share/manager.py
+++ b/manila/share/manager.py
@@ -206,7 +206,7 @@ class ShareManager(manager.SchedulerDependentManager):
206 self.publish_service_capabilities(ctxt) 206 self.publish_service_capabilities(ctxt)
207 207
208 def _provide_share_server_for_share(self, context, share_network_id, 208 def _provide_share_server_for_share(self, context, share_network_id,
209 share_id): 209 share, snapshot=None):
210 """Gets or creates share_server and updates share with its id. 210 """Gets or creates share_server and updates share with its id.
211 211
212 Active share_server can be deleted if there are no dependent shares 212 Active share_server can be deleted if there are no dependent shares
@@ -218,21 +218,87 @@ class ShareManager(manager.SchedulerDependentManager):
218 For this purpose used shared lock between this method and the one 218 For this purpose used shared lock between this method and the one
219 with deletion of share_server. 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 :returns: dict, dict -- first value is share_server, that 229 :returns: dict, dict -- first value is share_server, that
222 has been chosen for share schedule. Second value is 230 has been chosen for share schedule. Second value is
223 share updated with share_server_id. 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 @utils.synchronized("share_manager_%s" % share_network_id) 278 @utils.synchronized("share_manager_%s" % share_network_id)
227 def _provide_share_server_for_share(): 279 def _provide_share_server_for_share():
228 exist = False
229 try: 280 try:
230 share_server = \ 281 available_share_servers = get_available_share_servers()
231 self.db.share_server_get_by_host_and_share_net_valid(
232 context, self.host, share_network_id)
233 exist = True
234 except exception.ShareServerNotFound: 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 context, 302 context,
237 { 303 {
238 'host': self.host, 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" % ( 310 msg = "Using share_server %(share_server)s for share %(share_id)s"
245 share_server['id'], share_id)) 311 LOG.debug(msg, {
312 'share_server': compatible_share_server['id'],
313 'share_id': share['id']
314 })
315
246 share_ref = self.db.share_update( 316 share_ref = self.db.share_update(
247 context, 317 context,
248 share_id, 318 share['id'],
249 {'share_server_id': share_server['id']}, 319 {'share_server_id': compatible_share_server['id']},
250 ) 320 )
251 321
252 if not exist: 322 if compatible_share_server['status'] == constants.STATUS_CREATING:
253 # Create share server on backend with data from db 323 # Create share server on backend with data from db.
254 share_server = self._setup_server(context, share_server) 324 compatible_share_server = self._setup_server(
325 context, compatible_share_server)
255 LOG.info(_LI("Share server created successfully.")) 326 LOG.info(_LI("Share server created successfully."))
256 else: 327 else:
257 LOG.info(_LI("Used already existed share server " 328 LOG.info(_LI("Used preexisting share server "
258 "'%(share_server_id)s'"), 329 "'%(share_server_id)s'"),
259 {'share_server_id': share_server['id']}) 330 {'share_server_id': compatible_share_server['id']})
260 return share_server, share_ref 331 return compatible_share_server, share_ref
261 332
262 return _provide_share_server_for_share() 333 return _provide_share_server_for_share()
263 334
@@ -292,24 +363,12 @@ class ShareManager(manager.SchedulerDependentManager):
292 snapshot_ref = None 363 snapshot_ref = None
293 parent_share_server_id = None 364 parent_share_server_id = None
294 365
295 if parent_share_server_id: 366 if share_network_id or 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:
310 try: 367 try:
311 share_server, share_ref = self._provide_share_server_for_share( 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 except Exception: 372 except Exception:
314 with excutils.save_and_reraise_exception(): 373 with excutils.save_and_reraise_exception():
315 LOG.error(_LE("Failed to get share server" 374 LOG.error(_LE("Failed to get share server"
diff --git a/manila/tests/share/drivers/test_generic.py b/manila/tests/share/drivers/test_generic.py
index 2cb7523..7161e08 100644
--- a/manila/tests/share/drivers/test_generic.py
+++ b/manila/tests/share/drivers/test_generic.py
@@ -1360,6 +1360,21 @@ class GenericShareDriverTestCase(test.TestCase):
1360 fake_server_details, ['sudo', 'resize2fs', '/dev/fake']) 1360 fake_server_details, ['sudo', 'resize2fs', '/dev/fake'])
1361 self.assertEqual(2, self._driver._ssh_exec.call_count) 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@generic.ensure_server 1379@generic.ensure_server
1365def fake(driver_instance, context, share_server=None): 1380def fake(driver_instance, context, share_server=None):
diff --git a/manila/tests/share/test_manager.py b/manila/tests/share/test_manager.py
index ea71e1e..4103fa3 100644
--- a/manila/tests/share/test_manager.py
+++ b/manila/tests/share/test_manager.py
@@ -570,7 +570,10 @@ class ShareManagerTestCase(test.TestCase):
570 570
571 def test_create_share_with_share_network_server_creation_failed(self): 571 def test_create_share_with_share_network_server_creation_failed(self):
572 fake_share = {'id': 'fake_share_id', 'share_network_id': 'fake_sn_id'} 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 self.mock_object(db, 'share_server_create', 577 self.mock_object(db, 'share_server_create',
575 mock.Mock(return_value=fake_server)) 578 mock.Mock(return_value=fake_server))
576 self.mock_object(db, 'share_update', 579 self.mock_object(db, 'share_update',
@@ -585,7 +588,8 @@ class ShareManagerTestCase(test.TestCase):
585 def raise_manila_exception(*args, **kwargs): 588 def raise_manila_exception(*args, **kwargs):
586 raise exception.ManilaException() 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 mock.Mock(side_effect=raise_share_server_not_found)) 593 mock.Mock(side_effect=raise_share_server_not_found))
590 self.mock_object(self.share_manager, '_setup_server', 594 self.mock_object(self.share_manager, '_setup_server',
591 mock.Mock(side_effect=raise_manila_exception)) 595 mock.Mock(side_effect=raise_manila_exception))
@@ -596,7 +600,7 @@ class ShareManagerTestCase(test.TestCase):
596 self.context, 600 self.context,
597 fake_share['id'], 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 assert_called_once_with( 604 assert_called_once_with(
601 utils.IsAMatcher(context.RequestContext), 605 utils.IsAMatcher(context.RequestContext),
602 self.share_manager.host, 606 self.share_manager.host,
@@ -642,8 +646,12 @@ class ShareManagerTestCase(test.TestCase):
642 646
643 share_id = share['id'] 647 share_id = share['id']
644 648
645 self.share_manager.driver = mock.Mock() 649 driver_mock = mock.Mock()
646 self.share_manager.driver.create_share.return_value = "fake_location" 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 self.share_manager.create_share(self.context, share_id) 655 self.share_manager.create_share(self.context, share_id)
648 self.assertFalse(self.share_manager.driver.setup_network.called) 656 self.assertFalse(self.share_manager.driver.setup_network.called)
649 self.assertEqual(share_id, db.share_get(context.get_admin_context(), 657 self.assertEqual(share_id, db.share_get(context.get_admin_context(),
@@ -682,7 +690,10 @@ class ShareManagerTestCase(test.TestCase):
682 share_network_id=share_net['id'], host=self.share_manager.host, 690 share_network_id=share_net['id'], host=self.share_manager.host,
683 state=constants.STATUS_ERROR) 691 state=constants.STATUS_ERROR)
684 share_id = share['id'] 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 self.mock_object(db, 'share_server_create', 697 self.mock_object(db, 'share_server_create',
687 mock.Mock(return_value=fake_server)) 698 mock.Mock(return_value=fake_server))
688 self.mock_object(self.share_manager, '_setup_server', 699 self.mock_object(self.share_manager, '_setup_server',
@@ -736,6 +747,67 @@ class ShareManagerTestCase(test.TestCase):
736 utils.IsAMatcher(models.Share), 747 utils.IsAMatcher(models.Share),
737 share_server=None) 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 def test_manage_share_invalid_driver(self): 811 def test_manage_share_invalid_driver(self):
740 self.mock_object(self.share_manager, 'driver', mock.Mock()) 812 self.mock_object(self.share_manager, 'driver', mock.Mock())
741 self.share_manager.driver.driver_handles_share_servers = True 813 self.share_manager.driver.driver_handles_share_servers = True
diff --git a/manila/tests/test_db.py b/manila/tests/test_db.py
index dbed630..0fd9e0b 100644
--- a/manila/tests/test_db.py
+++ b/manila/tests/test_db.py
@@ -105,7 +105,7 @@ class ShareServerTableTestCase(test.TestCase):
105 db.share_server_update, 105 db.share_server_update,
106 self.ctxt, fake_id, {}) 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 valid = { 109 valid = {
110 'share_network_id': '1', 110 'share_network_id': '1',
111 'host': 'host1', 111 'host': 'host1',
@@ -125,15 +125,15 @@ class ShareServerTableTestCase(test.TestCase):
125 self._create_share_server(invalid) 125 self._create_share_server(invalid)
126 self._create_share_server(other) 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 self.ctxt, 129 self.ctxt,
130 host='host1', 130 host='host1',
131 share_net_id='1') 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 self.assertRaises(exception.ShareServerNotFound, 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 self.ctxt, host='fake', share_net_id='fake') 137 self.ctxt, host='fake', share_net_id='fake')
138 138
139 def test_share_server_get_all(self): 139 def test_share_server_get_all(self):