Merge "Fix share network update erroneously returns success"

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)

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):

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.