From 0655b58eaf9b186b20391cd8cf68b8f97ea0358a Mon Sep 17 00:00:00 2001 From: Valeriy Ponomaryov Date: Fri, 26 Sep 2014 13:41:57 +0300 Subject: [PATCH] Fix creation of share from snapshot Current behaviour does not take into accout source host of snapshot for share creation from snapshot. It leads to impossibility to create share if improper host was scheduled using multibackend setup. Solution: Implement config opt that allows us call required host directly without scheduling in case we have host restriction and allow scheduling in case we don't. We do not have such restricion, for example, using multibackend installation with generic drivers onboard. Change-Id: I0743c92b55e123409133ea623e0f8ec03d8e9aaa Closes-Bug: #1373882 --- manila/share/api.py | 56 +++++++++++++----- manila/tests/share/test_api.py | 103 ++++++++++++++++++++++++++------- 2 files changed, 124 insertions(+), 35 deletions(-) diff --git a/manila/share/api.py b/manila/share/api.py index 56809d5b5b..a6322e95f0 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -31,7 +31,17 @@ from manila import quota from manila.scheduler import rpcapi as scheduler_rpcapi from manila.share import rpcapi as share_rpcapi +share_api_opts = [ + cfg.BoolOpt('use_scheduler_creating_share_from_snapshot', + default=False, + help='If set to False, then share creation from snapshot will ' + 'be performed on the same host. ' + 'If set to True, then scheduling step will be used.') +] + CONF = cfg.CONF +CONF.register_opts(share_api_opts) + LOG = logging.getLogger(__name__) GB = 1048576 * 1024 QUOTAS = quota.QUOTAS @@ -152,22 +162,40 @@ class API(base.Base): finally: QUOTAS.rollback(context, reservations) - request_spec = {'share_properties': options, - 'share_proto': share_proto, - 'share_id': share['id'], - 'snapshot_id': share['snapshot_id'], - 'volume_type': volume_type - } - + request_spec = { + 'share_properties': options, + 'share_proto': share_proto, + 'share_id': share['id'], + 'snapshot_id': snapshot_id, + 'volume_type': volume_type, + } filter_properties = {} - self.scheduler_rpcapi.create_share( - context, - CONF.share_topic, - share['id'], - snapshot_id, - request_spec=request_spec, - filter_properties=filter_properties) + if (snapshot and not CONF.use_scheduler_creating_share_from_snapshot): + # Shares from snapshots with restriction - source host only. + # It is common situation for different types of backends. + host = snapshot['share']['host'] + share = self.db.share_update(context, share['id'], {'host': host}) + self.share_rpcapi.create_share( + context, + share, + host, + request_spec=request_spec, + filter_properties=filter_properties, + snapshot_id=snapshot_id, + ) + else: + # Shares from scratch and from snapshots when source host is not + # the only allowed, it is possible, for example, in multibackend + # installation with Generic drivers only. + self.scheduler_rpcapi.create_share( + context, + CONF.share_topic, + share['id'], + snapshot_id, + request_spec=request_spec, + filter_properties=filter_properties, + ) return share diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py index 2fe88160e5..4980bd5db2 100644 --- a/manila/tests/share/test_api.py +++ b/manila/tests/share/test_api.py @@ -18,6 +18,7 @@ import datetime import uuid import mock +from oslo.config import cfg from manila import context from manila import db as db_driver @@ -30,6 +31,8 @@ from manila import test from manila.tests.db import fakes as db_fakes from manila import utils +CONF = cfg.CONF + def fake_share(id, **kwargs): share = { @@ -73,7 +76,8 @@ def fake_snapshot(id, **kwargs): 'progress': 'fakeprogress99%', 'scheduled_at': datetime.datetime(1, 1, 1, 1, 1, 1), 'launched_at': datetime.datetime(1, 1, 1, 1, 1, 1), - 'terminated_at': datetime.datetime(1, 1, 1, 1, 1, 1) + 'terminated_at': datetime.datetime(1, 1, 1, 1, 1, 1), + 'share': {'host': 'fake_source_host'}, } snapshot.update(kwargs) return snapshot @@ -439,6 +443,7 @@ class ShareAPITestCase(test.TestCase): mock.Mock(return_value='reservation')) @mock.patch.object(quota.QUOTAS, 'commit', mock.Mock()) def test_create_from_snapshot_available(self): + CONF.set_default("use_scheduler_creating_share_from_snapshot", False) date = datetime.datetime(1, 1, 1, 1, 1, 1) self.mock_utcnow.return_value = date snapshot = fake_snapshot('fakesnapshotid', @@ -460,28 +465,80 @@ class ShareAPITestCase(test.TestCase): 'volume_type': None, 'snapshot_id': share['snapshot_id'], } - with mock.patch.object(db_driver, 'share_create', - mock.Mock(return_value=share)): - self.api.create(self.context, 'nfs', '1', 'fakename', 'fakedesc', - snapshot=snapshot, availability_zone='fakeaz') - self.scheduler_rpcapi.create_share.assert_called_once_with( - self.context, 'manila-share', share['id'], - share['snapshot_id'], request_spec=request_spec, - filter_properties={}) - db_driver.share_create.assert_called_once_with( - self.context, options) - share_api.policy.check_policy.assert_called_once_with( - self.context, 'share', 'create') - quota.QUOTAS.reserve.assert_called_once_with( - self.context, gigabytes=1, shares=1) - quota.QUOTAS.commit.assert_called_once_with( - self.context, 'reservation') + self.stubs.Set(db_driver, 'share_create', + mock.Mock(return_value=share)) + self.stubs.Set(db_driver, 'share_update', + mock.Mock(return_value=share)) + + self.api.create(self.context, 'nfs', '1', 'fakename', 'fakedesc', + snapshot=snapshot, availability_zone='fakeaz') + + self.share_rpcapi.create_share.assert_called_once_with( + self.context, share, snapshot['share']['host'], + request_spec=request_spec, filter_properties={}, + snapshot_id=snapshot['id']) + db_driver.share_create.assert_called_once_with( + self.context, options) + share_api.policy.check_policy.assert_called_once_with( + self.context, 'share', 'create') + quota.QUOTAS.reserve.assert_called_once_with( + self.context, gigabytes=1, shares=1) + quota.QUOTAS.commit.assert_called_once_with( + self.context, 'reservation') + + @mock.patch.object(quota.QUOTAS, 'reserve', + mock.Mock(return_value='reservation')) + @mock.patch.object(quota.QUOTAS, 'commit', mock.Mock()) + def test_create_from_snapshot_without_host_restriction(self): + CONF.set_default("use_scheduler_creating_share_from_snapshot", True) + date = datetime.datetime(1, 1, 1, 1, 1, 1) + self.mock_utcnow.return_value = date + snapshot = fake_snapshot('fakesnapshotid', + share_id='fakeshare_id', + status='available') + share = fake_share('fakeid', + user_id=self.context.user_id, + project_id=self.context.project_id, + snapshot_id=snapshot['id'], + status='creating') + options = share.copy() + for name in ('id', 'export_location', 'host', 'launched_at', + 'terminated_at'): + options.pop(name, None) + request_spec = { + 'share_properties': options, + 'share_proto': share['share_proto'], + 'share_id': share['id'], + 'volume_type': None, + 'snapshot_id': share['snapshot_id'], + } + self.stubs.Set(db_driver, 'share_create', + mock.Mock(return_value=share)) + self.stubs.Set(db_driver, 'share_update', + mock.Mock(return_value=share)) + + self.api.create(self.context, 'nfs', '1', 'fakename', 'fakedesc', + snapshot=snapshot, availability_zone='fakeaz') + + self.scheduler_rpcapi.create_share.assert_called_once_with( + self.context, 'manila-share', share['id'], + share['snapshot_id'], request_spec=request_spec, + filter_properties={}) + db_driver.share_create.assert_called_once_with( + self.context, options) + share_api.policy.check_policy.assert_called_once_with( + self.context, 'share', 'create') + quota.QUOTAS.reserve.assert_called_once_with( + self.context, gigabytes=1, shares=1) + quota.QUOTAS.commit.assert_called_once_with( + self.context, 'reservation') @mock.patch.object(quota.QUOTAS, 'reserve', mock.Mock(return_value='reservation')) @mock.patch.object(quota.QUOTAS, 'commit', mock.Mock()) def test_create_from_snapshot_with_volume_type_same(self): # Prepare data for test + CONF.set_default("use_scheduler_creating_share_from_snapshot", False) date = datetime.datetime(1, 1, 1, 1, 1, 1) self.mock_utcnow.return_value = date share_id = 'fake_share_id' @@ -506,6 +563,8 @@ class ShareAPITestCase(test.TestCase): } self.stubs.Set(db_driver, 'share_create', mock.Mock(return_value=share)) + self.stubs.Set(db_driver, 'share_update', + mock.Mock(return_value=share)) self.stubs.Set(db_driver, 'share_get', mock.Mock(return_value=share)) # Call tested method @@ -514,10 +573,10 @@ class ShareAPITestCase(test.TestCase): volume_type=volume_type) # Verify results - self.scheduler_rpcapi.create_share.assert_called_once_with( - self.context, 'manila-share', share_id, - share['snapshot_id'], request_spec=request_spec, - filter_properties={}) + self.share_rpcapi.create_share.assert_called_once_with( + self.context, share, snapshot['share']['host'], + request_spec=request_spec, filter_properties={}, + snapshot_id=snapshot['id']) db_driver.share_create.assert_called_once_with( self.context, options) share_api.policy.check_policy.assert_called_once_with( @@ -528,6 +587,8 @@ class ShareAPITestCase(test.TestCase): self.context, 'reservation') db_driver.share_get.assert_called_once_with( self.context, share_id) + db_driver.share_update.assert_called_once_with( + self.context, share_id, {'host': snapshot['share']['host']}) @mock.patch.object(quota.QUOTAS, 'reserve', mock.Mock(return_value='reservation'))