]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Allowed Addresspairs: Removing check for overlap with fixed ips
authorPraveen Yalagandula <ypraveen@gmail.com>
Tue, 20 May 2014 21:10:12 +0000 (21:10 +0000)
committerPraveen Yalagandula <ypraveen@gmail.com>
Wed, 21 May 2014 18:41:55 +0000 (18:41 +0000)
Current code does not allow assigning a fixed ip to a port when that ip
overlaps with one of the addresses in the allowed-addresspairs list.
This is an unnecessary check as the overlap does not have any negative
effect. Further, such a check actually makes it hard to use this
API. For example, if a fixed IP 10.10.1.1 exists on a port and we
want to allow addresses in 10.10.1.0/24 cidr on that port, then one
has to configure a list of 8 cidrs ([10.10.1.0/32, 10.10.1.2/31,
10.10.1.4/30, ..., 10.10.1.128/25]) on the allowed-addresspairs.
In addition to the above reasons, the current code also does not
check for the overlaps in all cases.

This patch summarily removes this overlap check.

Closes-Bug: #1321864
Change-Id: I5498c4a72b31267644da10a54a9860c1fc3bb250

neutron/db/allowedaddresspairs_db.py
neutron/plugins/bigswitch/plugin.py
neutron/plugins/ml2/plugin.py
neutron/plugins/nec/nec_plugin.py
neutron/plugins/openvswitch/ovs_neutron_plugin.py
neutron/plugins/vmware/plugins/base.py
neutron/tests/unit/test_extension_allowedaddresspairs.py

index 861139a9fff4f6630e83d625dccf262e4962f9a4..7fdde0e529311dc5d4ec822e1075073f801caa99 100644 (file)
@@ -61,14 +61,6 @@ class AllowedAddressPairsMixin(object):
 
         return allowed_address_pairs
 
-    def _check_fixed_ips_and_address_pairs_no_overlap(self, context, port):
-        address_pairs = self.get_allowed_address_pairs(context, port['id'])
-        for fixed_ip in port['fixed_ips']:
-            for address_pair in address_pairs:
-                if (fixed_ip['ip_address'] == address_pair['ip_address']
-                    and port['mac_address'] == address_pair['mac_address']):
-                    raise addr_pair.AddressPairMatchesPortFixedIPAndMac()
-
     def get_allowed_address_pairs(self, context, port_id):
         pairs = (context.session.query(AllowedAddressPair).
                  filter_by(port_id=port_id))
index 13362b5154ad848489993e8d6bb6476203331464..1e9621846d6a7a421f19a549c19f56ef38d572ff 100644 (file)
@@ -754,9 +754,6 @@ class NeutronRestProxyV2(NeutronRestProxyV2Base,
                 ctrl_update_required |= (
                     self.update_address_pairs_on_port(context, port_id, port,
                                                       orig_port, new_port))
-            if 'fixed_ips' in port['port']:
-                self._check_fixed_ips_and_address_pairs_no_overlap(
-                    context, new_port)
             self._update_extra_dhcp_opts_on_port(context, port_id, port,
                                                  new_port)
             old_host_id = porttracker_db.get_port_hostid(context,
index 8820c76181c7535a70235caeb64898ea1bfa4324..b501bc195801ac23f46f8b32fbdcb9aa66c4efb4 100644 (file)
@@ -669,7 +669,6 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
         need_port_update_notify = False
 
         session = context.session
-        changed_fixed_ips = 'fixed_ips' in port['port']
         with session.begin(subtransactions=True):
             try:
                 port_db = (session.query(models_v2.Port).
@@ -685,9 +684,6 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                     self.update_address_pairs_on_port(context, id, port,
                                                       original_port,
                                                       updated_port))
-            elif changed_fixed_ips:
-                self._check_fixed_ips_and_address_pairs_no_overlap(
-                    context, updated_port)
             need_port_update_notify |= self.update_security_group_on_port(
                 context, id, port, original_port, updated_port)
             network = self.get_network(context, original_port['network_id'])
index 753b8932842421bfc04d48a6d4f1c6b69123f349..78e15da6e1b5e10af96d1702723b059ed2401325 100644 (file)
@@ -611,7 +611,6 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
                     "id=%(id)s port=%(port)s ."),
                   {'id': id, 'port': port})
         need_port_update_notify = False
-        changed_fixed_ips = 'fixed_ips' in port['port']
         with context.session.begin(subtransactions=True):
             old_port = super(NECPluginV2, self).get_port(context, id)
             new_port = super(NECPluginV2, self).update_port(context, id, port)
@@ -622,9 +621,6 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
                     self.update_address_pairs_on_port(context, id, port,
                                                       old_port,
                                                       new_port))
-            elif changed_fixed_ips:
-                self._check_fixed_ips_and_address_pairs_no_overlap(
-                    context, new_port)
             need_port_update_notify |= self.update_security_group_on_port(
                 context, id, port, old_port, new_port)
 
index 5987e0d198cd2a69ad38c092edaea9f8d1dd8a02..1ea769c7846b34475d0116578dab8582fd528333 100644 (file)
@@ -583,7 +583,6 @@ class OVSNeutronPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
     def update_port(self, context, id, port):
         session = context.session
         need_port_update_notify = False
-        changed_fixed_ips = 'fixed_ips' in port['port']
         with session.begin(subtransactions=True):
             original_port = super(OVSNeutronPluginV2, self).get_port(
                 context, id)
@@ -594,9 +593,6 @@ class OVSNeutronPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
                     self.update_address_pairs_on_port(context, id, port,
                                                       original_port,
                                                       updated_port))
-            elif changed_fixed_ips:
-                self._check_fixed_ips_and_address_pairs_no_overlap(
-                    context, updated_port)
             need_port_update_notify |= self.update_security_group_on_port(
                 context, id, port, original_port, updated_port)
             self._process_portbindings_create_and_update(context,
index a5f57e72522949a81bd3f1ab7752492618eab494..0d1ba9db43b512ca8ddedaadbef1766777d631ed 100644 (file)
@@ -1208,7 +1208,6 @@ class NsxPluginV2(addr_pair_db.AllowedAddressPairsMixin,
         return port_data
 
     def update_port(self, context, id, port):
-        changed_fixed_ips = 'fixed_ips' in port['port']
         delete_security_groups = self._check_update_deletes_security_groups(
             port)
         has_security_groups = self._check_update_has_security_groups(port)
@@ -1250,9 +1249,6 @@ class NsxPluginV2(addr_pair_db.AllowedAddressPairsMixin,
                 self._delete_allowed_address_pairs(context, id)
                 self._process_create_allowed_address_pairs(
                     context, ret_port, ret_port[addr_pair.ADDRESS_PAIRS])
-            elif changed_fixed_ips:
-                self._check_fixed_ips_and_address_pairs_no_overlap(context,
-                                                                   ret_port)
             # checks if security groups were updated adding/modifying
             # security groups, port security is set and port has ip
             if not (has_ip and ret_port[psec.PORTSECURITY]):
index 25de95995c7854f6845061dcbf472795f3876ed4..850ebc43a5256edb88f8df4f065bafe829391825 100644 (file)
@@ -63,7 +63,6 @@ class AllowedAddressPairTestPlugin(portsecurity_db.PortSecurityDbMixin,
         return port['port']
 
     def update_port(self, context, id, port):
-        changed_fixed_ips = 'fixed_ips' in port['port']
         delete_addr_pairs = self._check_update_deletes_allowed_address_pairs(
             port)
         has_addr_pairs = self._check_update_has_allowed_address_pairs(port)
@@ -81,9 +80,6 @@ class AllowedAddressPairTestPlugin(portsecurity_db.PortSecurityDbMixin,
                 self._process_create_allowed_address_pairs(
                     context, ret_port,
                     ret_port[addr_pair.ADDRESS_PAIRS])
-            elif changed_fixed_ips:
-                self._check_fixed_ips_and_address_pairs_no_overlap(context,
-                                                                   ret_port)
 
         return ret_port
 
@@ -192,34 +188,6 @@ class TestAllowedAddressPairs(AllowedAddressPairDBTestCase):
                              address_pairs)
             self._delete('ports', port['port']['id'])
 
-    def test_update_fixed_ip_to_address_pair_ip_fail(self):
-        with self.network() as net:
-            with self.subnet(network=net):
-                address_pairs = [{'ip_address': '10.0.0.65'}]
-                res = self._create_port(self.fmt, net['network']['id'],
-                                        arg_list=(addr_pair.ADDRESS_PAIRS,),
-                                        allowed_address_pairs=address_pairs)
-                port = self.deserialize(self.fmt, res)['port']
-                data = {'port': {'fixed_ips': [{'ip_address': '10.0.0.65'}]}}
-                req = self.new_update_request('ports', data, port['id'])
-                res = req.get_response(self.api)
-                self.assertEqual(res.status_int, 400)
-                self._delete('ports', port['id'])
-
-    def test_update_fixed_ip_to_address_pair_with_mac_fail(self):
-        with self.network() as net:
-            with self.subnet(network=net):
-                res = self._create_port(self.fmt, net['network']['id'])
-                port = self.deserialize(self.fmt, res)['port']
-                address_pairs = [
-                    {'mac_address': port['mac_address'],
-                     'ip_address': port['fixed_ips'][0]['ip_address']}]
-                data = {'port': {addr_pair.ADDRESS_PAIRS: address_pairs}}
-                req = self.new_update_request('ports', data, port['id'])
-                res = req.get_response(self.api)
-                self.assertEqual(res.status_int, 400)
-                self._delete('ports', port['id'])
-
     def test_create_address_gets_port_mac(self):
         with self.network() as net:
             address_pairs = [{'ip_address': '23.23.23.23'}]
@@ -233,23 +201,6 @@ class TestAllowedAddressPairs(AllowedAddressPairDBTestCase):
                              port['mac_address'])
             self._delete('ports', port['id'])
 
-    def test_update_address_pair_to_match_fixed_ip_and_mac(self):
-        with self.network() as net:
-            with self.subnet(network=net):
-                res = self._create_port(self.fmt, net['network']['id'])
-                port = self.deserialize(self.fmt, res)['port']
-                address_pairs = [{'mac_address': port['mac_address'],
-                                  'ip_address':
-                                  port['fixed_ips'][0]['ip_address']}]
-
-                update_port = {'port': {addr_pair.ADDRESS_PAIRS:
-                                        address_pairs}}
-                req = self.new_update_request('ports', update_port,
-                                              port['id'])
-                res = req.get_response(self.api)
-                self.assertEqual(res.status_int, 400)
-                self._delete('ports', port['id'])
-
     def test_update_port_security_off_address_pairs(self):
         if self._skip_port_security:
             self.skipTest("Plugin does not implement port-security extension")