From b49605945a8e58ea33abfd559d9b87a6507519ab Mon Sep 17 00:00:00 2001
From: kpdev <kinpaa@gmail.com>
Date: Sat, 14 Aug 2021 19:38:55 +0200
Subject: [PATCH] Add "share-network" option for replica create API.
Share replica create API does not allow to specify share network and
forces to use parent share's share network. This is problem for some
use-cases, e.g. migration from one share network to another share
network via replication is not possible. Fixed by allowing to pass
'share-network' option for share replica create API and make sure both
parent share-network and user provided share-network will have same
security service association.
Partial-Bug: #1925486
Change-Id: I9049dcd418fbb16d663ab8ed27b90c765fafc5d3
---
api-ref/source/share-replicas.inc | 11 +++--
manila/api/openstack/api_version_request.py | 3 +-
.../openstack/rest_api_version_history.rst | 4 ++
manila/api/v2/share_replicas.py | 21 ++++++--
manila/db/api.py | 6 +++
manila/db/sqlalchemy/api.py | 11 +++++
manila/share/api.py | 15 ++++++
manila/tests/api/v2/test_share_replicas.py | 48 +++++++++++++++++--
manila/tests/db/sqlalchemy/test_api.py | 15 ++++++
...o-replica-create-api-7d2ff3628e93fc77.yaml | 7 +++
10 files changed, 127 insertions(+), 14 deletions(-)
create mode 100644 releasenotes/notes/bug-1925486-add-share-network-option-to-replica-create-api-7d2ff3628e93fc77.yaml
diff --git a/api-ref/source/share-replicas.inc b/api-ref/source/share-replicas.inc
index 2fede8b473..b9ad1eaea5 100644
--- a/api-ref/source/share-replicas.inc
+++ b/api-ref/source/share-replicas.inc
@@ -92,11 +92,12 @@ Request example
:language: javascript
.. note::
- Since API version 2.51, the parameter ``share_network_id``
- is deprecated. It will be inherited from its parent share, and the
- Shared File Systems service will automatically choose which share network
- subnet your share replica will be placed, according to the specified
- availability zone.
+ Since API version 2.72, the parameter ``share_network_id`` is added which
+ was earlier supported but later deprecated from version 2.51. In case, the
+ parameter is not specified, it will be inherited from its parent share,
+ and the Shared File Systems service will automatically choose which share
+ network subnet your share replica will be placed, according to the
+ specified availability zone.
Response parameters
diff --git a/manila/api/openstack/api_version_request.py b/manila/api/openstack/api_version_request.py
index 44a11edf2a..adcf081f4f 100644
--- a/manila/api/openstack/api_version_request.py
+++ b/manila/api/openstack/api_version_request.py
@@ -186,6 +186,7 @@ REST_API_VERSION_HISTORY = """
availability zone. Also, users can add subnets for an in-use share
network.
* 2.71 - Added 'updated_at' field in share instance show API output.
+ * 2.72 - Added new option ``share-network`` to share replica creare API.
"""
@@ -193,7 +194,7 @@ REST_API_VERSION_HISTORY = """
# The default api version request is defined to be the
# minimum version of the API supported.
_MIN_API_VERSION = "2.0"
-_MAX_API_VERSION = "2.71"
+_MAX_API_VERSION = "2.72"
DEFAULT_API_VERSION = _MIN_API_VERSION
diff --git a/manila/api/openstack/rest_api_version_history.rst b/manila/api/openstack/rest_api_version_history.rst
index 5aef3c5c2e..b617f2b456 100644
--- a/manila/api/openstack/rest_api_version_history.rst
+++ b/manila/api/openstack/rest_api_version_history.rst
@@ -396,3 +396,7 @@ ____
2.71
----
Added 'updated_at' field in share instance show API output.
+
+2.72
+----
+ Added 'share_network' option to share replica create API.
diff --git a/manila/api/v2/share_replicas.py b/manila/api/v2/share_replicas.py
index d9ee628282..638c422462 100644
--- a/manila/api/v2/share_replicas.py
+++ b/manila/api/v2/share_replicas.py
@@ -21,6 +21,7 @@ import webob
from webob import exc
from manila.api import common
+from manila.api.openstack import api_version_request as api_version
from manila.api.openstack import wsgi
from manila.api.views import share_replicas as replication_view
from manila.common import constants
@@ -174,11 +175,23 @@ class ShareReplicationController(wsgi.Controller, wsgi.AdminActionsMixin):
"since it has been soft deleted.") % share_id
raise exc.HTTPForbidden(explanation=msg)
- share_network_id = share_ref.get('share_network_id', None)
-
+ share_network_id = body.get('share_replica').get('share_network_id')
if share_network_id:
- share_network = db.share_network_get(context, share_network_id)
- common.check_share_network_is_active(share_network)
+ if req.api_version_request < api_version.APIVersionRequest("2.72"):
+ msg = _("'share_network_id' option is not supported by this "
+ "microversion. Use 2.72 or greater microversion to "
+ "be able to use 'share_network_id'.")
+ raise exc.HTTPBadRequest(explanation=msg)
+ else:
+ share_network_id = share_ref.get('share_network_id', None)
+
+ try:
+ if share_network_id:
+ share_network = db.share_network_get(context, share_network_id)
+ common.check_share_network_is_active(share_network)
+ except exception.ShareNetworkNotFound:
+ msg = _("No share network exists with ID %s.")
+ raise exc.HTTPNotFound(explanation=msg % share_network_id)
try:
new_replica = self.share_api.create_share_replica(
diff --git a/manila/db/api.py b/manila/db/api.py
index 441431176a..dccdac7851 100644
--- a/manila/db/api.py
+++ b/manila/db/api.py
@@ -792,6 +792,12 @@ def security_service_get_all_by_project(context, project_id):
return IMPL.security_service_get_all_by_project(context, project_id)
+def security_service_get_all_by_share_network(context, share_network_id):
+ """Get all security service DB records for the given share network."""
+ return IMPL.security_service_get_all_by_share_network(context,
+ share_network_id)
+
+
####################
def share_metadata_get(context, share_id):
"""Get all metadata for a share."""
diff --git a/manila/db/sqlalchemy/api.py b/manila/db/sqlalchemy/api.py
index 46458948bd..04ac98f28e 100644
--- a/manila/db/sqlalchemy/api.py
+++ b/manila/db/sqlalchemy/api.py
@@ -3977,6 +3977,17 @@ def _security_service_get_query(context, session=None, project_only=False):
project_only=project_only)
+@require_context
+def security_service_get_all_by_share_network(context, share_network_id):
+ session = get_session()
+ return (model_query(context, models.SecurityService, session=session).
+ join(models.ShareNetworkSecurityServiceAssociation,
+ models.SecurityService.id ==
+ models.ShareNetworkSecurityServiceAssociation.security_service_id).
+ filter_by(share_network_id=share_network_id, deleted=0)
+ .all())
+
+
###################
diff --git a/manila/share/api.py b/manila/share/api.py
index fc2251090d..ac9a7d451d 100644
--- a/manila/share/api.py
+++ b/manila/share/api.py
@@ -641,6 +641,21 @@ class API(base.Base):
def create_share_replica(self, context, share, availability_zone=None,
share_network_id=None, scheduler_hints=None):
+ parent_share_network_id = share.get('share_network_id')
+ if (parent_share_network_id and share_network_id and
+ parent_share_network_id != share_network_id):
+ parent_share_services = (
+ self.db.security_service_get_all_by_share_network(
+ context, parent_share_network_id))
+ share_services = (
+ self.db.security_service_get_all_by_share_network(
+ context, share_network_id))
+ for service in parent_share_services:
+ if service not in share_services:
+ msg = _("Share and its replica can't be in"
+ "different authentication domains.")
+ raise exception.InvalidInput(reason=msg)
+
if not share.get('replication_type'):
msg = _("Replication not supported for share %s.")
raise exception.InvalidShare(message=msg % share['id'])
diff --git a/manila/tests/api/v2/test_share_replicas.py b/manila/tests/api/v2/test_share_replicas.py
index 03f40ff84a..80613a5a48 100644
--- a/manila/tests/api/v2/test_share_replicas.py
+++ b/manila/tests/api/v2/test_share_replicas.py
@@ -396,6 +396,36 @@ class ShareReplicasApiTest(test.TestCase):
self.mock_policy_check.assert_called_once_with(
self.member_context, self.resource_name, 'create')
+ @ddt.data('2.72')
+ def test_create_invalid_network_id(self, microversion):
+ fake_replica, _ = self._get_fake_replica(
+ replication_type='writable')
+ req = self._get_request(microversion, False)
+ req_context = req.environ['manila.context']
+
+ body = {
+ 'share_replica': {
+ 'share_id': 'FAKE_SHAREID',
+ 'availability_zone': 'FAKE_AZ',
+ 'share_network_id': 'FAKE_NETID'
+ }
+ }
+ mock__view_builder_call = self.mock_object(
+ share_replicas.replication_view.ReplicationViewBuilder,
+ 'detail_list')
+ self.mock_object(share_replicas.db, 'share_get',
+ mock.Mock(return_value=fake_replica))
+ self.mock_object(share_replicas.db, 'share_network_get',
+ mock.Mock(side_effect=exception.ShareNetworkNotFound(
+ share_network_id='FAKE_NETID')))
+
+ self.assertRaises(exc.HTTPNotFound,
+ self.controller.create,
+ req, body)
+ self.assertFalse(mock__view_builder_call.called)
+ self.mock_policy_check.assert_called_once_with(
+ req_context, self.resource_name, 'create')
+
@ddt.data(exception.AvailabilityZoneNotFound,
exception.ReplicationException, exception.ShareBusyException)
def test_create_exception_path(self, exception_type):
@@ -432,19 +462,25 @@ class ShareReplicasApiTest(test.TestCase):
common.check_share_network_is_active.assert_called_once_with(
share_network)
- @ddt.data((True, PRE_GRADUATION_VERSION), (False, GRADUATION_VERSION))
+ @ddt.data((True, PRE_GRADUATION_VERSION), (False, GRADUATION_VERSION),
+ (False, "2.72"))
@ddt.unpack
def test_create(self, is_admin, microversion):
fake_replica, expected_replica = self._get_fake_replica(
replication_type='writable', admin=is_admin,
microversion=microversion)
- share_network = db_utils.create_share_network()
body = {
'share_replica': {
'share_id': 'FAKE_SHAREID',
'availability_zone': 'FAKE_AZ'
}
}
+ if self.is_microversion_ge(microversion, '2.72'):
+ body["share_replica"].update({"share_network_id": 'FAKE_NETID'})
+ share_network = {'id': 'FAKE_NETID'}
+ else:
+ share_network = db_utils.create_share_network()
+
self.mock_object(share_replicas.db, 'share_get',
mock.Mock(return_value=fake_replica))
self.mock_object(share.API, 'create_share_replica',
@@ -465,8 +501,12 @@ class ShareReplicasApiTest(test.TestCase):
self.assertEqual(expected_replica, res_dict['share_replica'])
self.mock_policy_check.assert_called_once_with(
req_context, self.resource_name, 'create')
- share_replicas.db.share_network_get.assert_called_once_with(
- req_context, fake_replica['share_network_id'])
+ if self.is_microversion_ge(microversion, '2.72'):
+ share_replicas.db.share_network_get.assert_called_once_with(
+ req_context, 'FAKE_NETID')
+ else:
+ share_replicas.db.share_network_get.assert_called_once_with(
+ req_context, fake_replica['share_network_id'])
common.check_share_network_is_active.assert_called_once_with(
share_network)
diff --git a/manila/tests/db/sqlalchemy/test_api.py b/manila/tests/db/sqlalchemy/test_api.py
index e2207c0b09..568b87a8a7 100644
--- a/manila/tests/db/sqlalchemy/test_api.py
+++ b/manila/tests/db/sqlalchemy/test_api.py
@@ -3121,6 +3121,21 @@ class SecurityServiceDatabaseAPITestCase(BaseDatabaseAPITestCase):
self.fake_context,
'wrong id')
+ def test_get_all_by_share_network(self):
+ db_api.security_service_create(self.fake_context,
+ security_service_dict)
+ share_nw_dict = {'id': 'fake network id',
+ 'project_id': 'fake project',
+ 'user_id': 'fake_user_id'}
+ db_api.share_network_create(self.fake_context, share_nw_dict)
+ db_api.share_network_add_security_service(
+ self.fake_context,
+ share_nw_dict['id'], security_service_dict['id'])
+
+ result = db_api.security_service_get_all_by_share_network(
+ self.fake_context, share_nw_dict['id'])
+ self._check_expected_fields(result[0], security_service_dict)
+
def test_delete(self):
db_api.security_service_create(self.fake_context,
security_service_dict)
diff --git a/releasenotes/notes/bug-1925486-add-share-network-option-to-replica-create-api-7d2ff3628e93fc77.yaml b/releasenotes/notes/bug-1925486-add-share-network-option-to-replica-create-api-7d2ff3628e93fc77.yaml
new file mode 100644
index 0000000000..a4124dcd55
--- /dev/null
+++ b/releasenotes/notes/bug-1925486-add-share-network-option-to-replica-create-api-7d2ff3628e93fc77.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+ - |
+ `Bug #1925486 <https://bugs.launchpad.net/manila/+bug/1925486>`_
+ Share replica create API does not support share network option and uses
+ parent share's share network. Fixed it to allow any share network by providing
+ option ``share-network``. Added in API microversion starting with '2.72'.