openstack
/
neutron
Code Issues Proposed changes

Execute ipset command using check_exit_code 

When l2 agent execute ipset command, we should pass
parameter 'check_exit_code' to self.execute acording to
action.
The intention is to safely fail if we try to destroy a
non existing set, or delete a non existing member.
Otherwise the agent gets stuck in a loop if such situation
happens. Such kind of event would be logged as errors.

Change-Id: If67330523d114d6da13d0280851e7138a51d08f7
Closes-bug: #1497074
This commit is contained in:
shihanzhang 2015-09-18 10:38:43 +08:00
parent 693db7cf7c
commit e82733f5d9
3 changed files with 19 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
neutron/agent/linux/ipset_manager.py
View File

@ -124,7 +124,7 @@ class IpsetManager(object):
def _del_member_from_set(self, set_name, member_ip):
cmd = ['ipset', 'del', set_name, member_ip]
self._apply(cmd)
self._apply(cmd, fail_on_errors=False)
self.ipset_sets[set_name].remove(member_ip)
def _create_set(self, set_name, ethertype):
@ -133,13 +133,14 @@ class IpsetManager(object):
self._apply(cmd)
self.ipset_sets[set_name] = []
def _apply(self, cmd, input=None):
def _apply(self, cmd, input=None, fail_on_errors=True):
input = '\n'.join(input) if input else None
cmd_ns = []
if self.namespace:
cmd_ns.extend(['ip', 'netns', 'exec', self.namespace])
cmd_ns.extend(cmd)
self.execute(cmd_ns, run_as_root=True, process_input=input)
self.execute(cmd_ns, run_as_root=True, process_input=input,
check_exit_code=fail_on_errors)
def _get_new_set_ips(self, set_name, expected_ips):
new_member_ips = (set(expected_ips) -
@ -175,5 +176,5 @@ class IpsetManager(object):
def _destroy(self, set_name, forced=False):
if set_name in self.ipset_sets or forced:
cmd = ['ipset', 'destroy', set_name]
self._apply(cmd)
self._apply(cmd, fail_on_errors=False)
self.ipset_sets.pop(set_name, None)

1
neutron/tests/functional/agent/linux/test_ipset.py
View File

@ -97,6 +97,5 @@ class IpsetManagerTestCase(IpsetBase):
self.source.assert_ping(self.destination.ip)
def test_destroy_ipset_set(self):
self.assertRaises(RuntimeError, self.ipset._destroy, self.ipset_name)
self._remove_iptables_ipset_rules()
self.ipset._destroy(self.ipset_name)

21
neutron/tests/unit/agent/linux/test_ipset_manager.py
View File

@ -67,19 +67,23 @@ class BaseIpsetManagerTest(base.BaseTestCase):
self.expected_calls.extend([
mock.call(['ipset', 'restore', '-exist'],
process_input=input,
run_as_root=True),
run_as_root=True,
check_exit_code=True),
mock.call(['ipset', 'swap', TEST_SET_NAME_NEW, TEST_SET_NAME],
process_input=None,
run_as_root=True),
run_as_root=True,
check_exit_code=True),
mock.call(['ipset', 'destroy', TEST_SET_NAME_NEW],
process_input=None,
run_as_root=True)])
run_as_root=True,
check_exit_code=False)])
def expect_add(self, addresses):
self.expected_calls.extend(
mock.call(['ipset', 'add', '-exist', TEST_SET_NAME, ip],
process_input=None,
run_as_root=True)
run_as_root=True,
check_exit_code=True)
for ip in self.ipset._sanitize_addresses(addresses))
def expect_del(self, addresses):
@ -87,7 +91,8 @@ class BaseIpsetManagerTest(base.BaseTestCase):
self.expected_calls.extend(
mock.call(['ipset', 'del', TEST_SET_NAME, ip],
process_input=None,
run_as_root=True)
run_as_root=True,
check_exit_code=False)
for ip in self.ipset._sanitize_addresses(addresses))
def expect_create(self):
@ -95,13 +100,15 @@ class BaseIpsetManagerTest(base.BaseTestCase):
mock.call(['ipset', 'create', '-exist', TEST_SET_NAME,
'hash:net', 'family', 'inet'],
process_input=None,
run_as_root=True))
run_as_root=True,
check_exit_code=True))
def expect_destroy(self):
self.expected_calls.append(
mock.call(['ipset', 'destroy', TEST_SET_NAME],
process_input=None,
run_as_root=True))
run_as_root=True,
check_exit_code=False))
def add_first_ip(self):
self.expect_set([FAKE_IPS[0]])