Revert "Add ipset element and hashsize tunables"

This reverts commit b5b919a7a3569ccb93c3d7d523c1edfaeddb7cb9.

The current ipset manager code isn't robust enough to handle
ipsets that already exist with different parameters. This reverts
the ability to change the parameters so we don't break upgrades
to Kilo.

Conflicts:
	neutron/agent/linux/ipset_manager.py
	neutron/tests/unit/agent/linux/test_ipset_manager.py

Change-Id: I538714df52424f0502cb75daea310517d1142c42
Closes-Bug: #1444201
(cherry picked from commit 03be14a569d240865dabff8b4c30385abf1dbe62)

diff --git a/etc/neutron.conf b/etc/neutron.conf
index 5d8640f90ea275b14008e1604e5bd73b3edcb9ba..2983cc6c0ff84768ef035a8c15ae45fc65c8157e 100644 (file)
--- a/etc/neutron.conf
+++ b/etc/neutron.conf
@@ -660,15 +660,6 @@
 # each rule's purpose. (System must support the iptables comments module.)
 # comment_iptables_rules = True
 
-# Maximum number of elements which can be stored in an IPset.
-# If None is specified, the system default will be used.
-# ipset_maxelem = 131072
-
-# Initial hash size for an IPset. Must be a power of 2,
-# else the kernel will round it up automatically.
-# If None is specified, the system default will be used.
-# ipset_hashsize = 2048
-
 # Root helper daemon application to use when possible.
 # root_helper_daemon =
 

diff --git a/neutron/agent/common/config.py b/neutron/agent/common/config.py
index efc1ca47602f4a1cd453809e7a95e349cee16d73..7e63ea3878982d5d5e1d006d8a6d68ccd47e50fd 100644 (file)
--- a/neutron/agent/common/config.py
+++ b/neutron/agent/common/config.py
@@ -63,17 +63,6 @@ IPTABLES_OPTS = [
                 help=_("Add comments to iptables rules.")),
 ]
 
-IPSET_OPTS = [
-    cfg.IntOpt('ipset_maxelem', default=131072,
-               help=_("Maximum number of elements which can be stored in "
-                      "an IPset. If None is specified, the system default "
-                      "will be used.")),
-    cfg.IntOpt('ipset_hashsize', default=2048,
-               help=_("Initial hash size for an IPset. Must be a power of 2, "
-                      "else the kernel will round it up automatically. If "
-                      "None is specified, the system default will be used.")),
-]
-
 PROCESS_MONITOR_OPTS = [
     cfg.StrOpt('check_child_processes_action', default='respawn',
                choices=['respawn', 'exit'],
@@ -133,10 +122,6 @@ def register_iptables_opts(conf):
     conf.register_opts(IPTABLES_OPTS, 'AGENT')
 
 
-def register_ipset_opts(conf):
-    conf.register_opts(IPSET_OPTS, 'AGENT')
-
-
 def register_process_monitor_opts(conf):
     conf.register_opts(PROCESS_MONITOR_OPTS, 'AGENT')
 

diff --git a/neutron/agent/linux/ipset_manager.py b/neutron/agent/linux/ipset_manager.py
index 33b6379b586f2cc68fd94f3df67aa959b56ff5c5..e5ab7a01e9c5813e4e1035b43158d434968c3a2f 100644 (file)
--- a/neutron/agent/linux/ipset_manager.py
+++ b/neutron/agent/linux/ipset_manager.py
@@ -11,9 +11,6 @@
 #    See the License for the specific language governing permissions and
 #    limitations under the License.
 
-from oslo_config import cfg
-
-from neutron.agent.common import config
 from neutron.agent.linux import utils as linux_utils
 from neutron.common import utils
 
@@ -32,7 +29,6 @@ class IpsetManager(object):
     def __init__(self, execute=None, namespace=None):
         self.execute = execute or linux_utils.execute
         self.namespace = namespace
-        config.register_ipset_opts(cfg.CONF)
         self.ipset_sets = {}
 
     @staticmethod
@@ -43,15 +39,6 @@ class IpsetManager(object):
         name = ethertype + id
         return name[:IPSET_NAME_MAX_LENGTH]
 
-    @staticmethod
-    def get_hashargs():
-        args = []
-        if cfg.CONF.AGENT.ipset_hashsize:
-            args.extend(['hashsize', str(cfg.CONF.AGENT.ipset_hashsize)])
-        if cfg.CONF.AGENT.ipset_maxelem:
-            args.extend(['maxelem', str(cfg.CONF.AGENT.ipset_maxelem)])
-        return args
-
     def set_exists(self, id, ethertype):
         """Returns true if the id+ethertype pair is known to the manager."""
         set_name = self.get_name(id, ethertype)
@@ -98,10 +85,8 @@ class IpsetManager(object):
     def _refresh_set(self, set_name, member_ips, ethertype):
         new_set_name = set_name + SWAP_SUFFIX
         set_type = self._get_ipset_set_type(ethertype)
-        hash_args = ' '.join(self.get_hashargs())
-        process_input = ["create %s hash:ip family %s %s" % (new_set_name,
-                                                             set_type,
-                                                             hash_args)]
+        process_input = ["create %s hash:ip family %s" % (new_set_name,
+                                                          set_type)]
         for ip in member_ips:
             process_input.append("add %s %s" % (new_set_name, ip))
 
@@ -118,7 +103,6 @@ class IpsetManager(object):
     def _create_set(self, set_name, ethertype):
         cmd = ['ipset', 'create', '-exist', set_name, 'hash:ip', 'family',
                self._get_ipset_set_type(ethertype)]
-        cmd.extend(self.get_hashargs())
         self._apply(cmd)
         self.ipset_sets[set_name] = []
 

diff --git a/neutron/tests/unit/agent/linux/test_ipset_manager.py b/neutron/tests/unit/agent/linux/test_ipset_manager.py
index 19fbb7e20e656dbf92f38b1fb400656b0245f54e..cbd156218ffe2b7a5deb7929a1aaa5c5aef6a7d9 100644 (file)
--- a/neutron/tests/unit/agent/linux/test_ipset_manager.py
+++ b/neutron/tests/unit/agent/linux/test_ipset_manager.py
@@ -12,9 +12,7 @@
 #    limitations under the License.
 
 import mock
-from oslo_config import cfg
 
-from neutron.agent.common import config as a_cfg
 from neutron.agent.linux import ipset_manager
 from neutron.tests import base
 
@@ -27,13 +25,8 @@ FAKE_IPS = ['10.0.0.1', '10.0.0.2', '10.0.0.3', '10.0.0.4',
 
 
 class BaseIpsetManagerTest(base.BaseTestCase):
-    def setUp(self, maxelem=None, hashsize=None):
+    def setUp(self):
         super(BaseIpsetManagerTest, self).setUp()
-        cfg.CONF.register_opts(a_cfg.IPSET_OPTS, 'AGENT')
-        cfg.CONF.set_override('ipset_maxelem', maxelem, 'AGENT')
-        cfg.CONF.set_override('ipset_hashsize', hashsize, 'AGENT')
-        self.maxelem = maxelem
-        self.hashsize = hashsize
         self.ipset = ipset_manager.IpsetManager()
         self.execute = mock.patch.object(self.ipset, "execute").start()
         self.expected_calls = []
@@ -43,13 +36,7 @@ class BaseIpsetManagerTest(base.BaseTestCase):
         self.execute.assert_has_calls(self.expected_calls, any_order=False)
 
     def expect_set(self, addresses):
-        hash_args = []
-        if self.hashsize:
-            hash_args.extend(['hashsize', str(self.hashsize)])
-        if self.maxelem:
-            hash_args.extend(['maxelem', str(self.maxelem)])
-        temp_input = ['create IPv4fake_sgid-new hash:ip family inet %s' %
-                      ' '.join(hash_args)]
+        temp_input = ['create IPv4fake_sgid-new hash:ip family inet']
         temp_input.extend('add IPv4fake_sgid-new %s' % ip for ip in addresses)
         input = '\n'.join(temp_input)
         self.expected_calls.extend([
@@ -76,14 +63,9 @@ class BaseIpsetManagerTest(base.BaseTestCase):
                       run_as_root=True) for ip in addresses)
 
     def expect_create(self):
-        ipset_call = ['ipset', 'create', '-exist', TEST_SET_NAME,
-                      'hash:ip', 'family', 'inet']
-        if self.hashsize:
-            ipset_call.extend(['hashsize', str(self.hashsize)])
-        if self.maxelem:
-            ipset_call.extend(['maxelem', str(self.maxelem)])
         self.expected_calls.append(
-            mock.call(ipset_call,
+            mock.call(['ipset', 'create', '-exist', TEST_SET_NAME,
+                       'hash:ip', 'family', 'inet'],
                       process_input=None,
                       run_as_root=True))
 
@@ -103,10 +85,6 @@ class BaseIpsetManagerTest(base.BaseTestCase):
 
 
 class IpsetManagerTestCase(BaseIpsetManagerTest):
-    """Run all tests, but with maxelem/hashsize values not configured
-    """
-    def setUp(self):
-        super(IpsetManagerTestCase, self).setUp()
 
     def test_set_exists(self):
         self.add_first_ip()
@@ -139,10 +117,3 @@ class IpsetManagerTestCase(BaseIpsetManagerTest):
         self.expect_destroy()
         self.ipset.destroy(TEST_SET_ID, ETHERTYPE)
         self.verify_mock_calls()
-
-
-class IpsetManagerTestCaseHashArgs(IpsetManagerTestCase):
-    """Run all the above tests, but with maxelem/hashsize values configured
-    """
-    def setUp(self):
-        super(IpsetManagerTestCase, self).setUp(maxelem=131072, hashsize=2048)