diff --git a/manila/api/v2/share_networks.py b/manila/api/v2/share_networks.py index 11f954b150..6d351ff3c0 100644 --- a/manila/api/v2/share_networks.py +++ b/manila/api/v2/share_networks.py @@ -272,10 +272,35 @@ class ShareNetworkController(wsgi.Controller): 'neutron_subnet_id' in update_values): subnet = db_api.share_network_subnet_get_default_subnet( context, id) - if subnet: - db_api.share_network_subnet_update(context, - subnet['id'], - update_values) + if not subnet: + msg = _("The share network %(id)s does not have a " + "'default' subnet that serves all availability " + "zones, so subnet details " + "('neutron_net_id', 'neutron_subnet_id') cannot " + "be updated.") % {'id': id} + raise exc.HTTPBadRequest(explanation=msg) + + # NOTE(silvacarlose): If the default share network subnet have + # the fields neutron_net_id and neutron_subnet_id set as None, + # we need to make sure that in the update request the user is + # passing both parameter since a share network subnet must + # have both fields filled or empty. + subnet_neutron_net_and_subnet_id_are_empty = ( + subnet['neutron_net_id'] is None + and subnet['neutron_subnet_id'] is None) + update_values_without_neutron_net_or_subnet = ( + update_values.get('neutron_net_id') is None or + update_values.get('neutron_subnet_id') is None) + if (subnet_neutron_net_and_subnet_id_are_empty + and update_values_without_neutron_net_or_subnet): + msg = _( + "To update the share network %(id)s you need to " + "specify both 'neutron_net_id' and " + "'neutron_subnet_id'.") % {'id': id} + raise webob.exc.HTTPBadRequest(explanation=msg) + db_api.share_network_subnet_update(context, + subnet['id'], + update_values) share_network = db_api.share_network_update(context, id, update_values) diff --git a/manila/tests/api/v2/test_share_networks.py b/manila/tests/api/v2/test_share_networks.py index d7a068bd12..c1dce8f771 100644 --- a/manila/tests/api/v2/test_share_networks.py +++ b/manila/tests/api/v2/test_share_networks.py @@ -813,6 +813,9 @@ class ShareNetworkAPITest(test.TestCase): self.mock_object( self.controller, '_share_network_subnets_contain_share_servers', mock.Mock(return_value=False)) + self.mock_object(db_api, 'share_network_subnet_get_default_subnet', + mock.Mock(return_value=fake_share_network_subnet)) + self.mock_object(db_api, 'share_network_subnet_update') body = {share_networks.RESOURCE_NAME: {'neutron_subnet_id': 'new subnet'}} @@ -825,6 +828,49 @@ class ShareNetworkAPITest(test.TestCase): self.req, share_nw, body) + db_api.share_network_subnet_get_default_subnet.assert_called_once_with( + self.context, share_nw) + db_api.share_network_subnet_update.assert_called_once_with( + self.context, fake_share_network_subnet['id'], + body['share_network']) + + @ddt.data((webob_exc.HTTPBadRequest, fake_share_network_subnet, None, + 'new subnet'), + (webob_exc.HTTPBadRequest, None, 'neutron net', None)) + @ddt.unpack + def test_update_default_subnet_errors(self, exception_to_raise, + get_default_subnet_return, + neutron_net_id, neutron_subnet_id): + share_nw = 'fake network id' + self.mock_object(db_api, 'share_network_get', + mock.Mock(return_value=fake_share_network)) + self.mock_object( + self.controller, '_share_network_subnets_contain_share_servers', + mock.Mock(return_value=False)) + self.mock_object(db_api, 'share_network_subnet_get_default_subnet', + mock.Mock(return_value=get_default_subnet_return)) + + if get_default_subnet_return: + fake_subnet = copy.deepcopy(fake_share_network_subnet) + fake_subnet['neutron_net_id'] = None + fake_subnet['neutron_subnet_id'] = None + db_api.share_network_subnet_get_default_subnet.return_value = ( + fake_subnet) + body = { + share_networks.RESOURCE_NAME: { + 'neutron_net_id': neutron_net_id, + 'neutron_subnet_id': neutron_subnet_id + } + } + + self.assertRaises(exception_to_raise, + self.controller.update, + self.req, + share_nw, + body) + + db_api.share_network_subnet_get_default_subnet.assert_called_once_with( + self.context, share_nw) @ddt.data(*set(("1.0", "2.25", "2.26", api_version._MAX_API_VERSION))) def test_action_add_security_service(self, microversion): diff --git a/releasenotes/notes/bug-1846836-fix-share-network-update-unexpected-success-eba8f40db392c467.yaml b/releasenotes/notes/bug-1846836-fix-share-network-update-unexpected-success-eba8f40db392c467.yaml new file mode 100644 index 0000000000..cb8a769c89 --- /dev/null +++ b/releasenotes/notes/bug-1846836-fix-share-network-update-unexpected-success-eba8f40db392c467.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixed unexpected behavior when updating a share network's `neutron_net_id` + or `neutron_subnet_id`. Now, Manila does not allow updating a share + network that does not contain a default subnet.