diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py index 38c497754adc..da06724d0b84 100644 --- a/nova/network/linux_net.py +++ b/nova/network/linux_net.py @@ -781,15 +781,15 @@ def initialize_gateway_device(dev, network_ref): old_routes.append(fields) nova.privsep.linux_net.route_delete(dev, fields[0]) for ip_params in old_ip_params: - _execute(*_ip_bridge_cmd('del', ip_params, dev), - run_as_root=True, check_exit_code=[0, 2, 254]) + nova.privsep.linux_net.address_command_deprecated( + dev, 'del', ip_params) for ip_params in new_ip_params: - _execute(*_ip_bridge_cmd('add', ip_params, dev), - run_as_root=True, check_exit_code=[0, 2, 254]) + nova.privsep.linux_net.address_command_deprecated( + dev, 'add', ip_params) for fields in old_routes: # TODO(mikal): this is horrible and should be re-written - nova.privsep.linux_net.route_add_horrid(fields) + nova.privsep.linux_net.route_add_deprecated(fields) if CONF.send_arp_for_ha and CONF.send_arp_for_ha_count > 0: nova.privsep.linux_net.send_arp_for_ip( network_ref['dhcp_server'], dev, @@ -1182,14 +1182,6 @@ def _ra_pid_for(dev): return int(f.read()) -def _ip_bridge_cmd(action, params, device): - """Build commands to add/del IPs to bridges/devices.""" - cmd = ['ip', 'addr', action] - cmd.extend(params) - cmd.extend(['dev', device]) - return cmd - - def _ovs_vsctl(args): full_args = ['ovs-vsctl', '--timeout=%s' % CONF.ovs_vsctl_timeout] + args try: @@ -1432,7 +1424,7 @@ class LinuxBridgeInterfaceDriver(LinuxNetInterfaceDriver): fields = line.split() if fields and 'via' in fields: old_routes.append(fields) - nova.privsep.linux_net.route_delete_horrid(fields) + nova.privsep.linux_net.route_delete_deprecated(fields) out, err = nova.privsep.linux_net.lookup_ip(interface) for line in out.split('\n'): fields = line.split() @@ -1441,12 +1433,12 @@ class LinuxBridgeInterfaceDriver(LinuxNetInterfaceDriver): params = fields[1:-2] else: params = fields[1:-1] - _execute(*_ip_bridge_cmd('del', params, fields[-1]), - run_as_root=True, check_exit_code=[0, 2, 254]) - _execute(*_ip_bridge_cmd('add', params, bridge), - run_as_root=True, check_exit_code=[0, 2, 254]) + nova.privsep.linux_net.address_command_deprecated( + fields[-1], 'del', params) + nova.privsep.linux_net.address_command_deprecated( + bridge, 'add', params) for fields in old_routes: - nova.privsep.linux_net.route_add_horrid(fields) + nova.privsep.linux_net.route_add_deprecated(fields) if filtering: # Don't forward traffic unless we were told to be a gateway diff --git a/nova/privsep/linux_net.py b/nova/privsep/linux_net.py index 82f3da79c0ab..74bae4dadee1 100644 --- a/nova/privsep/linux_net.py +++ b/nova/privsep/linux_net.py @@ -152,6 +152,18 @@ def change_ip(device, ip): 'dev', device) +# TODO(mikal): this is horrid. The calling code takes arguments from an +# interface list and just regurgitates them here. This isn't good enough, +# but is outside the scope of the privsep transition. Mark it as bonkers and +# hope we clean it up later. +@nova.privsep.sys_admin_pctxt.entrypoint +def address_command_deprecated(device, action, params): + cmd = ['ip', 'addr', action] + cmd.extend(params) + cmd.extend(['dev', device]) + processutils.execute(*cmd, check_exit_code=[0, 2, 254]) + + @nova.privsep.sys_admin_pctxt.entrypoint def dhcp_release(dev, address, mac_address): processutils.execute('dhcp_release', dev, address, mac_address) @@ -168,7 +180,7 @@ def routes_show(dev): # but is outside the scope of the privsep transition. Mark it as bonkers and # hope we clean it up later. @nova.privsep.sys_admin_pctxt.entrypoint -def route_add_horrid(routes): +def route_add_deprecated(routes): processutils.execute('ip', 'route', 'add', *routes) @@ -182,7 +194,7 @@ def route_delete(dev, route): # but is outside the scope of the privsep transition. Mark it as bonkers and # hope we clean it up later. @nova.privsep.sys_admin_pctxt.entrypoint -def route_delete_horrid(dev, routes): +def route_delete_deprecated(dev, routes): processutils.execute('ip', 'route', 'del', *routes) diff --git a/nova/tests/functional/api_sample_tests/api_sample_base.py b/nova/tests/functional/api_sample_tests/api_sample_base.py index d3dd13f82b6c..b2b58b78d6fb 100644 --- a/nova/tests/functional/api_sample_tests/api_sample_base.py +++ b/nova/tests/functional/api_sample_tests/api_sample_base.py @@ -133,6 +133,8 @@ class ApiSampleTestBaseV21(testscenarios.WithScenarios, self.stub_out('nova.privsep.linux_net.routes_show', fake_noop) self.stub_out('nova.privsep.linux_net.lookup_ip', fake_noop) self.stub_out('nova.privsep.linux_net.change_ip', fake_noop) + self.stub_out('nova.privsep.linux_net.address_command_deprecated', + fake_noop) if self.availability_zones: self.useFixture( diff --git a/nova/tests/unit/network/test_linux_net.py b/nova/tests/unit/network/test_linux_net.py index b33f3b78f291..d40598487ad3 100644 --- a/nova/tests/unit/network/test_linux_net.py +++ b/nova/tests/unit/network/test_linux_net.py @@ -878,28 +878,30 @@ class LinuxNetworkTestCase(test.NoDBTestCase): @mock.patch.object(utils, 'execute') @mock.patch('nova.privsep.linux_net.routes_show') @mock.patch('nova.privsep.linux_net.route_delete') - @mock.patch('nova.privsep.linux_net.route_add_horrid') + @mock.patch('nova.privsep.linux_net.route_add_deprecated') @mock.patch('nova.privsep.linux_net.lookup_ip') @mock.patch('nova.privsep.linux_net.change_ip') + @mock.patch('nova.privsep.linux_net.address_command_deprecated') def _test_initialize_gateway(self, existing, expected, - mock_change_ip, mock_lookup_ip, - mock_route_add, mock_route_delete, - mock_routes, mock_execute, routes='', + mock_address_command, mock_change_ip, + mock_lookup_ip, mock_route_add, + mock_route_delete, mock_routes, + mock_execute, routes='', routes_show_called=True, deleted_routes=None, - added_routes=None, changed_interfaces=None): + added_routes=None, changed_interfaces=None, + address_commands=None): self.flags(fake_network=False) mock_lookup_ip.return_value = (existing, '') executes = [] def fake_execute(*args, **kwargs): executes.append(args) - if args[0] == 'ip' and args[1] == 'addr' and args[2] == 'show': - return existing, "" if args[0] == 'sysctl': return '1\n', '' mock_execute.side_effect = fake_execute mock_routes.return_value = (routes, '') + mock_lookup_ip.return_value = (existing, '') network = {'dhcp_server': '192.168.1.1', 'cidr': '192.168.1.0/24', @@ -908,6 +910,7 @@ class LinuxNetworkTestCase(test.NoDBTestCase): self.driver.initialize_gateway_device('eth0', network) self.assertEqual(expected, executes) self.assertTrue(mock_execute.called) + self.assertTrue(mock_lookup_ip.called) if routes_show_called: mock_routes.assert_called_once_with('eth0') @@ -917,6 +920,8 @@ class LinuxNetworkTestCase(test.NoDBTestCase): mock_route_add.assert_has_calls(added_routes) if changed_interfaces: mock_change_ip.assert_has_calls(changed_interfaces) + if address_commands: + mock_address_command.assert_has_calls(address_commands) def test_initialize_gateway_moves_wrong_ip(self): existing = ("2: eth0: " @@ -927,16 +932,20 @@ class LinuxNetworkTestCase(test.NoDBTestCase): " valid_lft forever preferred_lft forever\n") expected = [ ('sysctl', '-n', 'net.ipv4.ip_forward'), - ('ip', 'addr', 'del', '192.168.0.1/24', - 'brd', '192.168.0.255', 'scope', 'global', 'dev', 'eth0'), - ('ip', 'addr', 'add', '192.168.1.1/24', - 'brd', '192.168.1.255', 'dev', 'eth0'), - ('ip', 'addr', 'add', '192.168.0.1/24', - 'brd', '192.168.0.255', 'scope', 'global', 'dev', 'eth0'), ] self._test_initialize_gateway( existing, expected, - changed_interfaces=[mock.call('eth0', '2001:db8::/64')]) + changed_interfaces=[mock.call('eth0', '2001:db8::/64')], + address_commands=[ + mock.call('eth0', 'del', ['192.168.0.1/24', 'brd', + '192.168.0.255', + 'scope', 'global']), + mock.call('eth0', 'add', ['192.168.1.1/24', 'brd', + '192.168.1.255']), + mock.call('eth0', 'add', ['192.168.0.1/24', 'brd', + '192.168.0.255', + 'scope', 'global'])] + ) def test_initialize_gateway_ip_with_dynamic_flag(self): existing = ("2: eth0: " @@ -948,16 +957,20 @@ class LinuxNetworkTestCase(test.NoDBTestCase): " valid_lft forever preferred_lft forever\n") expected = [ ('sysctl', '-n', 'net.ipv4.ip_forward'), - ('ip', 'addr', 'del', '192.168.0.1/24', - 'brd', '192.168.0.255', 'scope', 'global', 'dev', 'eth0'), - ('ip', 'addr', 'add', '192.168.1.1/24', - 'brd', '192.168.1.255', 'dev', 'eth0'), - ('ip', 'addr', 'add', '192.168.0.1/24', - 'brd', '192.168.0.255', 'scope', 'global', 'dev', 'eth0'), ] self._test_initialize_gateway( existing, expected, - changed_interfaces=[mock.call('eth0', '2001:db8::/64')]) + changed_interfaces=[mock.call('eth0', '2001:db8::/64')], + address_commands=[ + mock.call('eth0', 'del', + ['192.168.0.1/24', 'brd', '192.168.0.255', + 'scope', 'global']), + mock.call('eth0', 'add', + ['192.168.1.1/24', 'brd', '192.168.1.255']), + mock.call('eth0', 'add', + ['192.168.0.1/24', 'brd', '192.168.0.255', + 'scope', 'global'])] + ) def test_initialize_gateway_resets_route(self): routes = ("default via 192.168.0.1 dev eth0\n" @@ -970,12 +983,6 @@ class LinuxNetworkTestCase(test.NoDBTestCase): " valid_lft forever preferred_lft forever\n") expected = [ ('sysctl', '-n', 'net.ipv4.ip_forward'), - ('ip', 'addr', 'del', '192.168.0.1/24', - 'brd', '192.168.0.255', 'scope', 'global', 'dev', 'eth0'), - ('ip', 'addr', 'add', '192.168.1.1/24', - 'brd', '192.168.1.255', 'dev', 'eth0'), - ('ip', 'addr', 'add', '192.168.0.1/24', - 'brd', '192.168.0.255', 'scope', 'global', 'dev', 'eth0'), ] self._test_initialize_gateway( existing, expected, routes=routes, @@ -986,7 +993,16 @@ class LinuxNetworkTestCase(test.NoDBTestCase): mock.call(['192.168.100.0/24', 'via', '192.168.0.254', 'dev', 'eth0', 'proto', 'static'])], - changed_interfaces=[mock.call('eth0', '2001:db8::/64')] + changed_interfaces=[mock.call('eth0', '2001:db8::/64')], + address_commands=[ + mock.call('eth0', 'del', + ['192.168.0.1/24', 'brd', '192.168.0.255', + 'scope', 'global']), + mock.call('eth0', 'add', + ['192.168.1.1/24', 'brd', '192.168.1.255']), + mock.call('eth0', 'add', + ['192.168.0.1/24', 'brd', '192.168.0.255', + 'scope', 'global'])] ) def test_initialize_gateway_no_move_right_ip(self): @@ -1013,12 +1029,14 @@ class LinuxNetworkTestCase(test.NoDBTestCase): " valid_lft forever preferred_lft forever\n") expected = [ ('sysctl', '-n', 'net.ipv4.ip_forward'), - ('ip', 'addr', 'add', '192.168.1.1/24', - 'brd', '192.168.1.255', 'dev', 'eth0'), ] self._test_initialize_gateway( existing, expected, - changed_interfaces=[mock.call('eth0', '2001:db8::/64')]) + changed_interfaces=[mock.call('eth0', '2001:db8::/64')], + address_commands=[ + mock.call('eth0', 'add', + ['192.168.1.1/24', 'brd', '192.168.1.255'])] + ) @mock.patch.object(linux_net, 'ensure_ebtables_rules') @mock.patch.object(linux_net.iptables_manager, 'apply') @@ -1170,6 +1188,7 @@ class LinuxNetworkTestCase(test.NoDBTestCase): ifaddresses.assert_called_once_with('eth0') device_enabled.assert_called_once_with('eth0') set_device_macaddr.assert_called_once_with('bridge', fake_mac) + lookup_ip.assert_called_once_with('eth0') def test_ensure_bridge_brclt_addif_exception(self): def fake_execute(*cmd, **kwargs): diff --git a/nova/tests/unit/network/test_manager.py b/nova/tests/unit/network/test_manager.py index 6e59b4abb694..acb20876ed71 100644 --- a/nova/tests/unit/network/test_manager.py +++ b/nova/tests/unit/network/test_manager.py @@ -941,7 +941,9 @@ class VlanNetworkTestCase(test.TestCase): @mock.patch('nova.privsep.linux_net.routes_show', return_value=('', '')) @mock.patch('nova.privsep.linux_net.lookup_ip', return_value=('', '')) @mock.patch('nova.privsep.linux_net.change_ip') - def test_vpn_allocate_fixed_ip(self, mock_change_ip, mock_lookup_ip, + @mock.patch('nova.privsep.linux_net.address_command_deprecated') + def test_vpn_allocate_fixed_ip(self, mock_address_command, + mock_change_ip, mock_lookup_ip, mock_routes_show, mock_enabled, mock_add_bridge): self.mox.StubOutWithMock(db, 'fixed_ip_associate') @@ -981,8 +983,9 @@ class VlanNetworkTestCase(test.TestCase): @mock.patch('nova.privsep.linux_net.routes_show', return_value=('', '')) @mock.patch('nova.privsep.linux_net.lookup_ip', return_value=('', '')) @mock.patch('nova.privsep.linux_net.change_ip') - def test_allocate_fixed_ip(self, mock_change_ip, mock_lookup_ip, - mock_routes_show, mock_enabled, + @mock.patch('nova.privsep.linux_net.address_command_deprecated') + def test_allocate_fixed_ip(self, mock_address_command, mock_change_ip, + mock_lookup_ip, mock_routes_show, mock_enabled, mock_add_bridge): self.stubs.Set(self.network, '_do_trigger_security_group_members_refresh_for_instance', @@ -1706,9 +1709,10 @@ class VlanNetworkTestCase(test.TestCase): return_value=('fake', 0)) @mock.patch('nova.privsep.linux_net.lookup_ip', return_value=('', '')) @mock.patch('nova.privsep.linux_net.change_ip') + @mock.patch('nova.privsep.linux_net.address_command_deprecated') def test_add_fixed_ip_instance_without_vpn_requested_networks( - self, mock_change_ip, mock_lookup_ip, mock_routes_show, - mock_enabled, mock_add_bridge): + self, mock_address_command, mock_change_ip, mock_lookup_ip, + mock_routes_show, mock_enabled, mock_add_bridge): self.stubs.Set(self.network, '_do_trigger_security_group_members_refresh_for_instance', lambda *a, **kw: None) @@ -2865,7 +2869,9 @@ class AllocateTestCase(test.TestCase): @mock.patch('nova.privsep.linux_net.lookup_ip', return_value=('', '')) @mock.patch('nova.privsep.linux_net.change_ip') @mock.patch('nova.privsep.linux_net.clean_conntrack') + @mock.patch('nova.privsep.linux_net.address_command_deprecated') def test_allocate_for_instance(self, mock_clean_conntrack, + mock_address_command, mock_change_ip, mock_lookup_ip, mock_routes_show, mock_unbind, mock_bind, mock_set_macaddr, mock_set_enabled, @@ -2940,7 +2946,9 @@ class AllocateTestCase(test.TestCase): @mock.patch('nova.privsep.linux_net.routes_show', return_value=('', '')) @mock.patch('nova.privsep.linux_net.lookup_ip', return_value=('', '')) @mock.patch('nova.privsep.linux_net.change_ip') - def test_allocate_for_instance_with_mac(self, mock_change_ip, + @mock.patch('nova.privsep.linux_net.address_command_deprecated') + def test_allocate_for_instance_with_mac(self, mock_address_command, + mock_change_ip, mock_lookup_ip, mock_routes_show, mock_set_addr, mock_enabled, diff --git a/nova/tests/unit/virt/xenapi/test_xenapi.py b/nova/tests/unit/virt/xenapi/test_xenapi.py index ca0753577e8f..4c4db2ebe341 100644 --- a/nova/tests/unit/virt/xenapi/test_xenapi.py +++ b/nova/tests/unit/virt/xenapi/test_xenapi.py @@ -1140,7 +1140,9 @@ class XenAPIVMTestCase(stubs.XenAPITestBase, @mock.patch('nova.privsep.linux_net.set_device_enabled') @mock.patch('nova.privsep.linux_net.set_device_macaddr') @mock.patch('nova.privsep.linux_net.change_ip') - def test_spawn_vlanmanager(self, change_ip, mock_set_macaddr, + @mock.patch('nova.privsep.linux_net.address_command_deprecated') + def test_spawn_vlanmanager(self, mock_address_command_deprecated, + mock_change_ip, mock_set_macaddr, mock_set_enabled, mock_set_mtu, mock_add_bridge, mock_create_vifs): self.flags(network_manager='nova.network.manager.VlanManager',