From 9310a3b76a8f1415e3cbcc876d1cc54a24053b2a Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Tue, 10 Feb 2015 01:58:26 -0600 Subject: [PATCH] Remove root_helper arg from DHCP agent Partially-Implements: blueprint rootwrap-daemon-mode Change-Id: I394fc2bf8b9579203fc0a878047a57cf6e09927e --- neutron/agent/dhcp/agent.py | 5 +-- neutron/agent/dhcp_agent.py | 1 - neutron/agent/linux/dhcp.py | 42 +++++++------------ neutron/cmd/netns_cleanup.py | 2 - neutron/tests/unit/test_dhcp_agent.py | 53 ++++++++++-------------- neutron/tests/unit/test_linux_dhcp.py | 8 ++-- neutron/tests/unit/test_netns_cleanup.py | 2 - 7 files changed, 42 insertions(+), 71 deletions(-) diff --git a/neutron/agent/dhcp/agent.py b/neutron/agent/dhcp/agent.py index cdbd755ab..6e467a66d 100644 --- a/neutron/agent/dhcp/agent.py +++ b/neutron/agent/dhcp/agent.py @@ -56,7 +56,6 @@ class DhcpAgent(manager.Manager): self.needs_resync_reasons = collections.defaultdict(list) self.conf = cfg.CONF self.cache = NetworkCache() - self.root_helper = config.get_root_helper(self.conf) self.dhcp_driver_cls = importutils.import_class(self.conf.dhcp_driver) ctx = context.get_admin_context_without_session() self.plugin_rpc = DhcpPluginApi(topics.PLUGIN, @@ -75,8 +74,7 @@ class DhcpAgent(manager.Manager): """Populate the networks cache when the DHCP-agent starts.""" try: existing_networks = self.dhcp_driver_cls.existing_dhcp_networks( - self.conf, - self.root_helper + self.conf ) for net_id in existing_networks: net = dhcp.NetModel(self.conf.use_namespaces, @@ -109,7 +107,6 @@ class DhcpAgent(manager.Manager): driver = self.dhcp_driver_cls(self.conf, network, self._process_monitor, - self.root_helper, self.dhcp_version, self.plugin_rpc) getattr(driver, action)(**action_kwargs) diff --git a/neutron/agent/dhcp_agent.py b/neutron/agent/dhcp_agent.py index 0e7564511..ff01bd900 100644 --- a/neutron/agent/dhcp_agent.py +++ b/neutron/agent/dhcp_agent.py @@ -34,7 +34,6 @@ def register_options(): config.register_interface_driver_opts_helper(cfg.CONF) config.register_use_namespaces_opts_helper(cfg.CONF) config.register_agent_state_opts_helper(cfg.CONF) - config.register_root_helper(cfg.CONF) cfg.CONF.register_opts(dhcp_config.DHCP_AGENT_OPTS) cfg.CONF.register_opts(dhcp_config.DHCP_OPTS) cfg.CONF.register_opts(dhcp_config.DNSMASQ_OPTS) diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index 3b8da5fd4..2d97f3bc7 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -110,14 +110,12 @@ class NetModel(DictModel): @six.add_metaclass(abc.ABCMeta) class DhcpBase(object): - def __init__(self, conf, network, process_monitor, root_helper='sudo', + def __init__(self, conf, network, process_monitor, version=None, plugin=None): self.conf = conf self.network = network - self.root_helper = root_helper self.process_monitor = process_monitor - self.device_manager = DeviceManager(self.conf, - self.root_helper, plugin) + self.device_manager = DeviceManager(self.conf, plugin) self.version = version @abc.abstractmethod @@ -142,7 +140,7 @@ class DhcpBase(object): """Force the DHCP server to reload the assignment database.""" @classmethod - def existing_dhcp_networks(cls, conf, root_helper): + def existing_dhcp_networks(cls, conf): """Return a list of existing networks ids that we have configs for.""" raise NotImplementedError() @@ -167,10 +165,10 @@ class DhcpBase(object): class DhcpLocalProcess(DhcpBase): PORTS = [] - def __init__(self, conf, network, process_monitor, root_helper='sudo', - version=None, plugin=None): + def __init__(self, conf, network, process_monitor, version=None, + plugin=None): super(DhcpLocalProcess, self).__init__(conf, network, process_monitor, - root_helper, version, plugin) + version, plugin) self.confs_dir = self.get_confs_dir(conf) self.network_conf_dir = os.path.join(self.confs_dir, network.id) self._ensure_network_conf_dir() @@ -227,8 +225,7 @@ class DhcpLocalProcess(DhcpBase): if not retain_port: if self.conf.dhcp_delete_namespaces and self.network.namespace: - ns_ip = ip_lib.IPWrapper(self.root_helper, - self.network.namespace) + ns_ip = ip_lib.IPWrapper(namespace=self.network.namespace) try: ns_ip.netns.delete(self.network.namespace) except RuntimeError: @@ -288,7 +285,7 @@ class Dnsmasq(DhcpLocalProcess): pass @classmethod - def existing_dhcp_networks(cls, conf, root_helper): + def existing_dhcp_networks(cls, conf): """Return a list of existing networks ids that we have configs for.""" confs_dir = cls.get_confs_dir(conf) try: @@ -399,8 +396,7 @@ class Dnsmasq(DhcpLocalProcess): def _release_lease(self, mac_address, ip): """Release a DHCP lease.""" cmd = ['dhcp_release', self.interface_name, ip, mac_address] - ip_wrapper = ip_lib.IPWrapper(self.root_helper, - self.network.namespace) + ip_wrapper = ip_lib.IPWrapper(namespace=self.network.namespace) ip_wrapper.netns.execute(cmd) def _output_config_files(self): @@ -676,11 +672,8 @@ class Dnsmasq(DhcpLocalProcess): return options def _make_subnet_interface_ip_map(self): - ip_dev = ip_lib.IPDevice( - self.interface_name, - self.root_helper, - self.network.namespace - ) + ip_dev = ip_lib.IPDevice(self.interface_name, + namespace=self.network.namespace) subnet_lookup = dict( (netaddr.IPNetwork(subnet.cidr), subnet.id) @@ -775,9 +768,8 @@ class Dnsmasq(DhcpLocalProcess): class DeviceManager(object): - def __init__(self, conf, root_helper, plugin): + def __init__(self, conf, plugin): self.conf = conf - self.root_helper = root_helper self.plugin = plugin if not conf.interface_driver: LOG.error(_LE('An interface driver must be specified')) @@ -809,9 +801,7 @@ class DeviceManager(object): it would change it from what it already is. This makes it safe to call and avoids unnecessary perturbation of the system. """ - device = ip_lib.IPDevice(device_name, - self.root_helper, - network.namespace) + device = ip_lib.IPDevice(device_name, namespace=network.namespace) gateway = device.route.get_gateway() if gateway: gateway = gateway['gateway'] @@ -927,8 +917,7 @@ class DeviceManager(object): interface_name = self.get_interface_name(network, port) if ip_lib.ensure_device_is_ready(interface_name, - self.root_helper, - network.namespace): + namespace=network.namespace): LOG.debug('Reusing existing device: %s.', interface_name) else: self.driver.plug(network.id, @@ -953,8 +942,7 @@ class DeviceManager(object): # ensure that the dhcp interface is first in the list if network.namespace is None: - device = ip_lib.IPDevice(interface_name, - self.root_helper) + device = ip_lib.IPDevice(interface_name) device.route.pullup_route(interface_name) if self.conf.use_namespaces: diff --git a/neutron/cmd/netns_cleanup.py b/neutron/cmd/netns_cleanup.py index c1e7dd2ff..c6c471de5 100644 --- a/neutron/cmd/netns_cleanup.py +++ b/neutron/cmd/netns_cleanup.py @@ -81,7 +81,6 @@ def _get_dhcp_process_monitor(config): def kill_dhcp(conf, namespace): """Disable DHCP for a network if DHCP is still active.""" - root_helper = agent_config.get_root_helper(conf) network_id = namespace.replace(dhcp.NS_PREFIX, '') dhcp_driver = importutils.import_object( @@ -89,7 +88,6 @@ def kill_dhcp(conf, namespace): conf=conf, process_monitor=_get_dhcp_process_monitor(conf), network=dhcp.NetModel(conf.use_namespaces, {'id': network_id}), - root_helper=root_helper, plugin=FakeDhcpPlugin()) if dhcp_driver.active: diff --git a/neutron/tests/unit/test_dhcp_agent.py b/neutron/tests/unit/test_dhcp_agent.py index 5255de955..5e100e188 100644 --- a/neutron/tests/unit/test_dhcp_agent.py +++ b/neutron/tests/unit/test_dhcp_agent.py @@ -232,7 +232,6 @@ class TestDhcpAgent(base.BaseTestCase): cfg.CONF.register_opts(dhcp_config.DHCP_AGENT_OPTS) config.register_interface_driver_opts_helper(cfg.CONF) config.register_agent_state_opts_helper(cfg.CONF) - config.register_root_helper(cfg.CONF) cfg.CONF.register_opts(interface.OPTS) common_config.init(sys.argv[1:]) agent_mgr = dhcp_agent.DhcpAgentWithStateReport( @@ -278,7 +277,6 @@ class TestDhcpAgent(base.BaseTestCase): self.driver.assert_called_once_with(cfg.CONF, mock.ANY, mock.ANY, - 'sudo', mock.ANY, mock.ANY) @@ -295,7 +293,6 @@ class TestDhcpAgent(base.BaseTestCase): self.driver.assert_called_once_with(cfg.CONF, mock.ANY, mock.ANY, - 'sudo', mock.ANY, mock.ANY) self.assertEqual(log.call_count, 1) @@ -413,7 +410,6 @@ class TestDhcpAgent(base.BaseTestCase): self.driver.existing_dhcp_networks.assert_called_once_with( dhcp.conf, - cfg.CONF.AGENT.root_helper ) self.assertFalse(dhcp.cache.get_network_ids()) @@ -427,7 +423,6 @@ class TestDhcpAgent(base.BaseTestCase): self.driver.existing_dhcp_networks.assert_called_once_with( dhcp.conf, - cfg.CONF.AGENT.root_helper ) self.assertEqual(set(networks), set(dhcp.cache.get_network_ids())) @@ -436,7 +431,7 @@ class TestDhcpAgent(base.BaseTestCase): cfg.CONF.set_override('interface_driver', None) with mock.patch.object(dhcp, 'LOG') as log: self.assertRaises(SystemExit, dhcp.DeviceManager, - cfg.CONF, 'sudo', None) + cfg.CONF, None) msg = 'An interface driver must be specified' log.error.assert_called_once_with(msg) @@ -448,7 +443,7 @@ class TestDhcpAgent(base.BaseTestCase): cfg.CONF.set_override('interface_driver', 'foo') with mock.patch.object(dhcp, 'LOG') as log: self.assertRaises(SystemExit, dhcp.DeviceManager, - cfg.CONF, 'sudo', None) + cfg.CONF, None) self.assertEqual(log.error.call_count, 1) @@ -533,7 +528,6 @@ class TestDhcpAgentEventHandler(base.BaseTestCase): config.register_interface_driver_opts_helper(cfg.CONF) cfg.CONF.set_override('interface_driver', 'neutron.agent.linux.interface.NullDriver') - config.register_root_helper(cfg.CONF) cfg.CONF.register_opts(dhcp_config.DHCP_AGENT_OPTS) self.plugin_p = mock.patch(DHCP_PLUGIN) @@ -1160,7 +1154,6 @@ class TestDeviceManager(base.BaseTestCase): cfg.CONF.register_opts(dhcp_config.DHCP_AGENT_OPTS) cfg.CONF.set_override('interface_driver', 'neutron.agent.linux.interface.NullDriver') - config.register_root_helper(cfg.CONF) cfg.CONF.set_override('use_namespaces', True) cfg.CONF.set_override('enable_isolated_metadata', True) @@ -1189,7 +1182,7 @@ class TestDeviceManager(base.BaseTestCase): self.ensure_device_is_ready.return_value = device_is_ready self.mock_driver.get_device_name.return_value = 'tap12345678-12' - dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.AGENT.root_helper, plugin) + dh = dhcp.DeviceManager(cfg.CONF, plugin) dh._set_default_route = mock.Mock() interface_name = dh.setup(net) @@ -1240,7 +1233,7 @@ class TestDeviceManager(base.BaseTestCase): def test_create_dhcp_port_raise_conflict(self): plugin = mock.Mock() - dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.AGENT.root_helper, plugin) + dh = dhcp.DeviceManager(cfg.CONF, plugin) plugin.create_dhcp_port.return_value = None self.assertRaises(exceptions.Conflict, dh.setup_dhcp_port, @@ -1248,7 +1241,7 @@ class TestDeviceManager(base.BaseTestCase): def test_create_dhcp_port_create_new(self): plugin = mock.Mock() - dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.AGENT.root_helper, plugin) + dh = dhcp.DeviceManager(cfg.CONF, plugin) plugin.create_dhcp_port.return_value = fake_network.ports[0] dh.setup_dhcp_port(fake_network) plugin.assert_has_calls([ @@ -1262,7 +1255,7 @@ class TestDeviceManager(base.BaseTestCase): def test_create_dhcp_port_update_add_subnet(self): plugin = mock.Mock() - dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.AGENT.root_helper, plugin) + dh = dhcp.DeviceManager(cfg.CONF, plugin) fake_network_copy = copy.deepcopy(fake_network) fake_network_copy.ports[0].device_id = dh.get_device_id(fake_network) fake_network_copy.subnets[1].enable_dhcp = True @@ -1280,7 +1273,7 @@ class TestDeviceManager(base.BaseTestCase): def test_update_dhcp_port_raises_conflict(self): plugin = mock.Mock() - dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.AGENT.root_helper, plugin) + dh = dhcp.DeviceManager(cfg.CONF, plugin) fake_network_copy = copy.deepcopy(fake_network) fake_network_copy.ports[0].device_id = dh.get_device_id(fake_network) fake_network_copy.subnets[1].enable_dhcp = True @@ -1291,7 +1284,7 @@ class TestDeviceManager(base.BaseTestCase): def test_create_dhcp_port_no_update_or_create(self): plugin = mock.Mock() - dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.AGENT.root_helper, plugin) + dh = dhcp.DeviceManager(cfg.CONF, plugin) fake_network_copy = copy.deepcopy(fake_network) fake_network_copy.ports[0].device_id = dh.get_device_id(fake_network) dh.setup_dhcp_port(fake_network_copy) @@ -1315,8 +1308,7 @@ class TestDeviceManager(base.BaseTestCase): plugin = mock.Mock() plugin.get_dhcp_port.return_value = fake_port - dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.AGENT.root_helper, - plugin) + dh = dhcp.DeviceManager(cfg.CONF, plugin) dh.destroy(fake_net, 'tap12345678-12') dvr_cls.assert_called_once_with(cfg.CONF) @@ -1343,8 +1335,7 @@ class TestDeviceManager(base.BaseTestCase): plugin = mock.Mock() plugin.get_dhcp_port.return_value = fake_port - dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.AGENT.root_helper, - plugin) + dh = dhcp.DeviceManager(cfg.CONF, plugin) dh.get_interface_name(fake_net, fake_port) dvr_cls.assert_called_once_with(cfg.CONF) @@ -1363,7 +1354,7 @@ class TestDeviceManager(base.BaseTestCase): with mock.patch('uuid.uuid5') as uuid5: uuid5.return_value = '1ae5f96c-c527-5079-82ea-371a01645457' - dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.AGENT.root_helper, None) + dh = dhcp.DeviceManager(cfg.CONF, None) uuid5.called_once_with(uuid.NAMESPACE_DNS, cfg.CONF.host) self.assertEqual(dh.get_device_id(fake_net), expected) @@ -1371,7 +1362,7 @@ class TestDeviceManager(base.BaseTestCase): # Try with namespaces and no metadata network cfg.CONF.set_override('use_namespaces', True) cfg.CONF.set_override('enable_metadata_network', False) - dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.AGENT.root_helper, None) + dh = dhcp.DeviceManager(cfg.CONF, None) dh._set_default_route = mock.Mock() network = mock.Mock() @@ -1383,7 +1374,7 @@ class TestDeviceManager(base.BaseTestCase): # No namespaces, shouldn't set default route. cfg.CONF.set_override('use_namespaces', False) cfg.CONF.set_override('enable_metadata_network', False) - dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.AGENT.root_helper, None) + dh = dhcp.DeviceManager(cfg.CONF, None) dh._set_default_route = mock.Mock() dh.update(FakeV4Network(), 'tap12345678-12') @@ -1393,7 +1384,7 @@ class TestDeviceManager(base.BaseTestCase): # Meta data network enabled, don't interfere with its gateway. cfg.CONF.set_override('use_namespaces', True) cfg.CONF.set_override('enable_metadata_network', True) - dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.AGENT.root_helper, None) + dh = dhcp.DeviceManager(cfg.CONF, None) dh._set_default_route = mock.Mock() dh.update(FakeV4Network(), 'ns-12345678-12') @@ -1403,7 +1394,7 @@ class TestDeviceManager(base.BaseTestCase): # For completeness cfg.CONF.set_override('use_namespaces', False) cfg.CONF.set_override('enable_metadata_network', True) - dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.AGENT.root_helper, None) + dh = dhcp.DeviceManager(cfg.CONF, None) dh._set_default_route = mock.Mock() dh.update(FakeV4Network(), 'ns-12345678-12') @@ -1411,7 +1402,7 @@ class TestDeviceManager(base.BaseTestCase): self.assertFalse(dh._set_default_route.called) def test_set_default_route(self): - dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.AGENT.root_helper, None) + dh = dhcp.DeviceManager(cfg.CONF, None) with mock.patch.object(dhcp.ip_lib, 'IPDevice') as mock_IPDevice: device = mock.Mock() mock_IPDevice.return_value = device @@ -1425,7 +1416,7 @@ class TestDeviceManager(base.BaseTestCase): device.route.add_gateway.assert_called_once_with('192.168.0.1') def test_set_default_route_no_subnet(self): - dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.AGENT.root_helper, None) + dh = dhcp.DeviceManager(cfg.CONF, None) with mock.patch.object(dhcp.ip_lib, 'IPDevice') as mock_IPDevice: device = mock.Mock() mock_IPDevice.return_value = device @@ -1439,7 +1430,7 @@ class TestDeviceManager(base.BaseTestCase): self.assertFalse(device.route.add_gateway.called) def test_set_default_route_no_subnet_delete_gateway(self): - dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.AGENT.root_helper, None) + dh = dhcp.DeviceManager(cfg.CONF, None) with mock.patch.object(dhcp.ip_lib, 'IPDevice') as mock_IPDevice: device = mock.Mock() mock_IPDevice.return_value = device @@ -1453,7 +1444,7 @@ class TestDeviceManager(base.BaseTestCase): self.assertFalse(device.route.add_gateway.called) def test_set_default_route_no_gateway(self): - dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.AGENT.root_helper, None) + dh = dhcp.DeviceManager(cfg.CONF, None) with mock.patch.object(dhcp.ip_lib, 'IPDevice') as mock_IPDevice: device = mock.Mock() mock_IPDevice.return_value = device @@ -1467,7 +1458,7 @@ class TestDeviceManager(base.BaseTestCase): self.assertFalse(device.route.add_gateway.called) def test_set_default_route_do_nothing(self): - dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.AGENT.root_helper, None) + dh = dhcp.DeviceManager(cfg.CONF, None) with mock.patch.object(dhcp.ip_lib, 'IPDevice') as mock_IPDevice: device = mock.Mock() mock_IPDevice.return_value = device @@ -1480,7 +1471,7 @@ class TestDeviceManager(base.BaseTestCase): self.assertFalse(device.route.add_gateway.called) def test_set_default_route_change_gateway(self): - dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.AGENT.root_helper, None) + dh = dhcp.DeviceManager(cfg.CONF, None) with mock.patch.object(dhcp.ip_lib, 'IPDevice') as mock_IPDevice: device = mock.Mock() mock_IPDevice.return_value = device @@ -1494,7 +1485,7 @@ class TestDeviceManager(base.BaseTestCase): def test_set_default_route_two_subnets(self): # Try two subnets. Should set gateway from the first. - dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.AGENT.root_helper, None) + dh = dhcp.DeviceManager(cfg.CONF, None) with mock.patch.object(dhcp.ip_lib, 'IPDevice') as mock_IPDevice: device = mock.Mock() mock_IPDevice.return_value = device diff --git a/neutron/tests/unit/test_linux_dhcp.py b/neutron/tests/unit/test_linux_dhcp.py index 95cd83896..e20a2c3c4 100644 --- a/neutron/tests/unit/test_linux_dhcp.py +++ b/neutron/tests/unit/test_linux_dhcp.py @@ -600,7 +600,7 @@ class TestDhcpBase(TestBase): def test_existing_dhcp_networks_abstract_error(self): self.assertRaises(NotImplementedError, dhcp.DhcpBase.existing_dhcp_networks, - None, None) + None) def test_check_version_abstract_error(self): self.assertRaises(NotImplementedError, @@ -676,7 +676,7 @@ class TestDhcpLocalProcess(TestBase): lp.enable() self.mock_mgr.assert_has_calls( - [mock.call(self.conf, 'sudo', None), + [mock.call(self.conf, None), mock.call().setup(mock.ANY)]) self.assertEqual(lp.called, ['spawn']) self.assertTrue(mocks['interface_name'].__set__.called) @@ -719,7 +719,7 @@ class TestDhcpLocalProcess(TestBase): lp.process_monitor.pid.return_value = 5 lp.disable() - self.mock_mgr.assert_has_calls([mock.call(self.conf, 'sudo', None), + self.mock_mgr.assert_has_calls([mock.call(self.conf, None), mock.call().destroy(network, 'tap0')]) self.assertEqual(ip.return_value.netns.delete.call_count, 0) @@ -1330,7 +1330,7 @@ class TestDnsmasq(TestBase): mock_active.__get__ = active_fake mock_listdir.return_value = cases.keys() - result = dhcp.Dnsmasq.existing_dhcp_networks(self.conf, 'sudo') + result = dhcp.Dnsmasq.existing_dhcp_networks(self.conf) mock_listdir.assert_called_once_with(path) self.assertEqual(['aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa', diff --git a/neutron/tests/unit/test_netns_cleanup.py b/neutron/tests/unit/test_netns_cleanup.py index 9ca96c3ce..82c3f5793 100644 --- a/neutron/tests/unit/test_netns_cleanup.py +++ b/neutron/tests/unit/test_netns_cleanup.py @@ -33,7 +33,6 @@ class TestNetnsCleanup(base.BaseTestCase): def test_kill_dhcp(self, dhcp_active=True): conf = mock.Mock() - conf.AGENT.root_helper = 'sudo', conf.dhcp_driver = 'driver' method_to_patch = 'oslo_utils.importutils.import_object' @@ -46,7 +45,6 @@ class TestNetnsCleanup(base.BaseTestCase): util.kill_dhcp(conf, 'ns') expected_params = {'conf': conf, 'network': mock.ANY, - 'root_helper': conf.AGENT.root_helper, 'plugin': mock.ANY, 'process_monitor': mock.ANY} import_object.assert_called_once_with('driver', **expected_params) -- 2.45.2