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. Change-Id: I538714df52424f0502cb75daea310517d1142c42 Closes-Bug: #1444201
This commit is contained in:
parent
354ae7c1d3
commit
03be14a569
@ -660,15 +660,6 @@
|
|||||||
# each rule's purpose. (System must support the iptables comments module.)
|
# each rule's purpose. (System must support the iptables comments module.)
|
||||||
# comment_iptables_rules = True
|
# 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 application to use when possible.
|
||||||
# root_helper_daemon =
|
# root_helper_daemon =
|
||||||
|
|
||||||
|
@ -63,17 +63,6 @@ IPTABLES_OPTS = [
|
|||||||
help=_("Add comments to iptables rules.")),
|
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 = [
|
PROCESS_MONITOR_OPTS = [
|
||||||
cfg.StrOpt('check_child_processes_action', default='respawn',
|
cfg.StrOpt('check_child_processes_action', default='respawn',
|
||||||
choices=['respawn', 'exit'],
|
choices=['respawn', 'exit'],
|
||||||
@ -133,10 +122,6 @@ def register_iptables_opts(conf):
|
|||||||
conf.register_opts(IPTABLES_OPTS, 'AGENT')
|
conf.register_opts(IPTABLES_OPTS, 'AGENT')
|
||||||
|
|
||||||
|
|
||||||
def register_ipset_opts(conf):
|
|
||||||
conf.register_opts(IPSET_OPTS, 'AGENT')
|
|
||||||
|
|
||||||
|
|
||||||
def register_process_monitor_opts(conf):
|
def register_process_monitor_opts(conf):
|
||||||
conf.register_opts(PROCESS_MONITOR_OPTS, 'AGENT')
|
conf.register_opts(PROCESS_MONITOR_OPTS, 'AGENT')
|
||||||
|
|
||||||
|
@ -13,9 +13,6 @@
|
|||||||
|
|
||||||
import copy
|
import copy
|
||||||
|
|
||||||
from oslo_config import cfg
|
|
||||||
|
|
||||||
from neutron.agent.common import config
|
|
||||||
from neutron.agent.linux import utils as linux_utils
|
from neutron.agent.linux import utils as linux_utils
|
||||||
from neutron.common import utils
|
from neutron.common import utils
|
||||||
|
|
||||||
@ -23,8 +20,6 @@ IPSET_ADD_BULK_THRESHOLD = 5
|
|||||||
SWAP_SUFFIX = '-new'
|
SWAP_SUFFIX = '-new'
|
||||||
IPSET_NAME_MAX_LENGTH = 31 - len(SWAP_SUFFIX)
|
IPSET_NAME_MAX_LENGTH = 31 - len(SWAP_SUFFIX)
|
||||||
|
|
||||||
config.register_ipset_opts(cfg.CONF)
|
|
||||||
|
|
||||||
|
|
||||||
class IpsetManager(object):
|
class IpsetManager(object):
|
||||||
"""Smart wrapper for ipset.
|
"""Smart wrapper for ipset.
|
||||||
@ -46,15 +41,6 @@ class IpsetManager(object):
|
|||||||
name = ethertype + id
|
name = ethertype + id
|
||||||
return name[:IPSET_NAME_MAX_LENGTH]
|
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):
|
def set_exists(self, id, ethertype):
|
||||||
"""Returns true if the id+ethertype pair is known to the manager."""
|
"""Returns true if the id+ethertype pair is known to the manager."""
|
||||||
set_name = self.get_name(id, ethertype)
|
set_name = self.get_name(id, ethertype)
|
||||||
@ -101,10 +87,8 @@ class IpsetManager(object):
|
|||||||
def _refresh_set(self, set_name, member_ips, ethertype):
|
def _refresh_set(self, set_name, member_ips, ethertype):
|
||||||
new_set_name = set_name + SWAP_SUFFIX
|
new_set_name = set_name + SWAP_SUFFIX
|
||||||
set_type = self._get_ipset_set_type(ethertype)
|
set_type = self._get_ipset_set_type(ethertype)
|
||||||
hash_args = ' '.join(self.get_hashargs())
|
process_input = ["create %s hash:ip family %s" % (new_set_name,
|
||||||
process_input = ["create %s hash:ip family %s %s" % (new_set_name,
|
set_type)]
|
||||||
set_type,
|
|
||||||
hash_args)]
|
|
||||||
for ip in member_ips:
|
for ip in member_ips:
|
||||||
process_input.append("add %s %s" % (new_set_name, ip))
|
process_input.append("add %s %s" % (new_set_name, ip))
|
||||||
|
|
||||||
@ -121,7 +105,6 @@ class IpsetManager(object):
|
|||||||
def _create_set(self, set_name, ethertype):
|
def _create_set(self, set_name, ethertype):
|
||||||
cmd = ['ipset', 'create', '-exist', set_name, 'hash:ip', 'family',
|
cmd = ['ipset', 'create', '-exist', set_name, 'hash:ip', 'family',
|
||||||
self._get_ipset_set_type(ethertype)]
|
self._get_ipset_set_type(ethertype)]
|
||||||
cmd.extend(self.get_hashargs())
|
|
||||||
self._apply(cmd)
|
self._apply(cmd)
|
||||||
self.ipset_sets[set_name] = []
|
self.ipset_sets[set_name] = []
|
||||||
|
|
||||||
|
@ -12,7 +12,6 @@
|
|||||||
# limitations under the License.
|
# limitations under the License.
|
||||||
|
|
||||||
import mock
|
import mock
|
||||||
from oslo_config import cfg
|
|
||||||
|
|
||||||
from neutron.agent.linux import ipset_manager
|
from neutron.agent.linux import ipset_manager
|
||||||
from neutron.tests import base
|
from neutron.tests import base
|
||||||
@ -26,12 +25,8 @@ FAKE_IPS = ['10.0.0.1', '10.0.0.2', '10.0.0.3', '10.0.0.4',
|
|||||||
|
|
||||||
|
|
||||||
class BaseIpsetManagerTest(base.BaseTestCase):
|
class BaseIpsetManagerTest(base.BaseTestCase):
|
||||||
def setUp(self, maxelem, hashsize):
|
def setUp(self):
|
||||||
super(BaseIpsetManagerTest, self).setUp()
|
super(BaseIpsetManagerTest, self).setUp()
|
||||||
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.ipset = ipset_manager.IpsetManager()
|
||||||
self.execute = mock.patch.object(self.ipset, "execute").start()
|
self.execute = mock.patch.object(self.ipset, "execute").start()
|
||||||
self.expected_calls = []
|
self.expected_calls = []
|
||||||
@ -41,13 +36,7 @@ class BaseIpsetManagerTest(base.BaseTestCase):
|
|||||||
self.execute.assert_has_calls(self.expected_calls, any_order=False)
|
self.execute.assert_has_calls(self.expected_calls, any_order=False)
|
||||||
|
|
||||||
def expect_set(self, addresses):
|
def expect_set(self, addresses):
|
||||||
hash_args = []
|
temp_input = ['create IPv4fake_sgid-new hash:ip family inet']
|
||||||
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.extend('add IPv4fake_sgid-new %s' % ip for ip in addresses)
|
temp_input.extend('add IPv4fake_sgid-new %s' % ip for ip in addresses)
|
||||||
input = '\n'.join(temp_input)
|
input = '\n'.join(temp_input)
|
||||||
self.expected_calls.extend([
|
self.expected_calls.extend([
|
||||||
@ -74,14 +63,9 @@ class BaseIpsetManagerTest(base.BaseTestCase):
|
|||||||
run_as_root=True) for ip in addresses)
|
run_as_root=True) for ip in addresses)
|
||||||
|
|
||||||
def expect_create(self):
|
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(
|
self.expected_calls.append(
|
||||||
mock.call(ipset_call,
|
mock.call(['ipset', 'create', '-exist', TEST_SET_NAME,
|
||||||
|
'hash:ip', 'family', 'inet'],
|
||||||
process_input=None,
|
process_input=None,
|
||||||
run_as_root=True))
|
run_as_root=True))
|
||||||
|
|
||||||
@ -101,11 +85,6 @@ class BaseIpsetManagerTest(base.BaseTestCase):
|
|||||||
|
|
||||||
|
|
||||||
class IpsetManagerTestCase(BaseIpsetManagerTest):
|
class IpsetManagerTestCase(BaseIpsetManagerTest):
|
||||||
"""Run all tests, but with maxelem/hashsize values not configured
|
|
||||||
"""
|
|
||||||
def setUp(self, maxelem=None, hashsize=None):
|
|
||||||
super(IpsetManagerTestCase, self).setUp(maxelem=maxelem,
|
|
||||||
hashsize=hashsize)
|
|
||||||
|
|
||||||
def test_set_exists(self):
|
def test_set_exists(self):
|
||||||
self.add_first_ip()
|
self.add_first_ip()
|
||||||
@ -138,11 +117,3 @@ class IpsetManagerTestCase(BaseIpsetManagerTest):
|
|||||||
self.expect_destroy()
|
self.expect_destroy()
|
||||||
self.ipset.destroy(TEST_SET_ID, ETHERTYPE)
|
self.ipset.destroy(TEST_SET_ID, ETHERTYPE)
|
||||||
self.verify_mock_calls()
|
self.verify_mock_calls()
|
||||||
|
|
||||||
|
|
||||||
class IpsetManagerTestCaseHashArgs(IpsetManagerTestCase):
|
|
||||||
"""Run all the above tests, but with maxelem/hashsize values configured
|
|
||||||
"""
|
|
||||||
def setUp(self):
|
|
||||||
super(IpsetManagerTestCaseHashArgs, self).setUp(maxelem=131072,
|
|
||||||
hashsize=2048)
|
|
||||||
|
Loading…
x
Reference in New Issue
Block a user