From 6e563d97030a960c1f042d2b908b4aa94ef214e1 Mon Sep 17 00:00:00 2001 From: Michael Still Date: Mon, 25 Feb 2019 04:13:33 +0000 Subject: [PATCH] Move dnsmasq restarts to privsep. I don't really love this one, but I don't see a better way to untangle this. Change-Id: Icce18320a7c8fba3cf06bd032fbbe1846804e897 --- nova/network/linux_net.py | 51 ++++++++--------------- nova/privsep/linux_net.py | 51 +++++++++++++++++++++++ nova/tests/unit/network/test_linux_net.py | 17 ++++++-- 3 files changed, 81 insertions(+), 38 deletions(-) diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py index a71e225ee55c..667f94bed2c0 100644 --- a/nova/network/linux_net.py +++ b/nova/network/linux_net.py @@ -999,47 +999,30 @@ def restart_dhcp(context, dev, network_ref, fixedips): else: LOG.debug('Pid %d is stale, relaunching dnsmasq', pid) - cmd = ['env', - 'CONFIG_FILE=%s' % jsonutils.dumps(CONF.dhcpbridge_flagfile), - 'NETWORK_ID=%s' % str(network_ref['id']), - 'dnsmasq', - '--strict-order', - '--bind-interfaces', - '--conf-file=%s' % CONF.dnsmasq_config_file, - '--pid-file=%s' % _dhcp_file(dev, 'pid'), - '--dhcp-optsfile=%s' % _dhcp_file(dev, 'opts'), - '--listen-address=%s' % network_ref['dhcp_server'], - '--except-interface=lo', - '--dhcp-range=set:%s,%s,static,%s,%ss' % - (network_ref['label'], - network_ref['dhcp_start'], - network_ref['netmask'], - CONF.dhcp_lease_time), - '--dhcp-lease-max=%s' % len(netaddr.IPNetwork(network_ref['cidr'])), - '--dhcp-hostsfile=%s' % _dhcp_file(dev, 'conf'), - '--dhcp-script=%s' % CONF.dhcpbridge, - '--no-hosts', - '--leasefile-ro'] - - # dnsmasq currently gives an error for an empty domain, - # rather than ignoring. So only specify it if defined. - if CONF.api.dhcp_domain: - cmd.append('--domain=%s' % CONF.api.dhcp_domain) - dns_servers = CONF.dns_server if CONF.use_network_dns_servers: if network_ref.get('dns1'): dns_servers.append(network_ref.get('dns1')) if network_ref.get('dns2'): dns_servers.append(network_ref.get('dns2')) - if network_ref['multi_host']: - cmd.append('--addn-hosts=%s' % _dhcp_file(dev, 'hosts')) - if dns_servers: - cmd.append('--no-resolv') - for dns_server in dns_servers: - cmd.append('--server=%s' % dns_server) - _execute(*cmd, run_as_root=True) + hosts_path = None + if network_ref['multi_host']: + hosts_path = _dhcp_file(dev, 'hosts') + + nova.privsep.linux_net.restart_dnsmasq( + jsonutils.dumps(CONF.dhcpbridge_flagfile), + network_ref, + CONF.dnsmasq_config_file, + _dhcp_file(dev, 'pid'), + _dhcp_file(dev, 'opts'), + CONF.dhcp_lease_time, + len(netaddr.IPNetwork(network_ref['cidr'])), + _dhcp_file(dev, 'conf'), + CONF.dhcpbridge, + CONF.api.dhcp_domain, + dns_servers, + hosts_path) _add_dnsmasq_accept_rules(dev) diff --git a/nova/privsep/linux_net.py b/nova/privsep/linux_net.py index 90172c1c6a16..06bfa1488cba 100644 --- a/nova/privsep/linux_net.py +++ b/nova/privsep/linux_net.py @@ -289,3 +289,54 @@ def iptables_set_rules(rules, ipv4=True): processutils.execute('%s-restore' % cmd, '-c', process_input=six.b('\n'.join(rules)), attempts=5) + + +@nova.privsep.sys_admin_pctxt.entrypoint +def restart_dnsmasq(flag_file, network_ref, config_file, pid_path, opts_path, + dhcp_lease_time, lease_max, conf_path, dhcp_bridge, + dhcp_domain, dns_servers, hosts_path): + _restart_dnsmasq_inner(flag_file, network_ref, config_file, pid_path, + opts_path, dhcp_lease_time, lease_max, conf_path, + dhcp_bridge, dhcp_domain, dns_servers, hosts_path) + + +# NOTE(mikal): this is done like this to enable unit testing +def _restart_dnsmasq_inner(flag_file, network_ref, config_file, pid_path, + opts_path, dhcp_lease_time, lease_max, conf_path, + dhcp_bridge, dhcp_domain, dns_servers, hosts_path): + cmd = ['env', + 'CONFIG_FILE=%s' % flag_file, + 'NETWORK_ID=%s' % str(network_ref['id']), + 'dnsmasq', + '--strict-order', + '--bind-interfaces', + '--conf-file=%s' % config_file, + '--pid-file=%s' % pid_path, + '--dhcp-optsfile=%s' % opts_path, + '--listen-address=%s' % network_ref['dhcp_server'], + '--except-interface=lo', + '--dhcp-range=set:%s,%s,static,%s,%ss' % + (network_ref['label'], + network_ref['dhcp_start'], + network_ref['netmask'], + dhcp_lease_time), + '--dhcp-lease-max=%s' % lease_max, + '--dhcp-hostsfile=%s' % conf_path, + '--dhcp-script=%s' % dhcp_bridge, + '--no-hosts', + '--leasefile-ro'] + + # dnsmasq currently gives an error for an empty domain, + # rather than ignoring. So only specify it if defined. + if dhcp_domain: + cmd.append('--domain=%s' % dhcp_domain) + + if dns_servers: + cmd.append('--no-resolv') + for dns_server in dns_servers: + cmd.append('--server=%s' % dns_server) + + if network_ref['multi_host']: + cmd.append('--addn-hosts=%s' % hosts_path) + + processutils.execute(*cmd) diff --git a/nova/tests/unit/network/test_linux_net.py b/nova/tests/unit/network/test_linux_net.py index a74405918746..8c24dd418a65 100644 --- a/nova/tests/unit/network/test_linux_net.py +++ b/nova/tests/unit/network/test_linux_net.py @@ -425,7 +425,9 @@ class LinuxNetworkTestCase(test.NoDBTestCase): return_value=('', '')) @mock.patch('nova.privsep.linux_net.iptables_set_rules', return_value=('', '')) - def test_update_dhcp_for_nw00(self, mock_iptables_set_rules, + @mock.patch('nova.privsep.linux_net.restart_dnsmasq') + def test_update_dhcp_for_nw00(self, mock_restart_dnsmasq, + mock_iptables_set_rules, mock_iptables_get_rules, mock_chmod, mock_ensure_tree): with mock.patch.object(self.driver, 'write_to_file') \ @@ -437,6 +439,7 @@ class LinuxNetworkTestCase(test.NoDBTestCase): self.assertEqual(mock_write_to_file.call_count, 2) self.assertEqual(mock_ensure_tree.call_count, 7) self.assertEqual(mock_chmod.call_count, 2) + self.assertEqual(mock_restart_dnsmasq.call_count, 1) @mock.patch.object(fileutils, 'ensure_tree') @mock.patch.object(os, 'chmod') @@ -444,7 +447,9 @@ class LinuxNetworkTestCase(test.NoDBTestCase): return_value=('', '')) @mock.patch('nova.privsep.linux_net.iptables_set_rules', return_value=('', '')) - def test_update_dhcp_for_nw01(self, mock_iptables_set_rules, + @mock.patch('nova.privsep.linux_net.restart_dnsmasq') + def test_update_dhcp_for_nw01(self, mock_restart_dnsmasq, + mock_iptables_set_rules, mock_iptables_get_rules, mock_chmod, mock_ensure_tree): with mock.patch.object(self.driver, 'write_to_file') \ @@ -456,6 +461,7 @@ class LinuxNetworkTestCase(test.NoDBTestCase): self.assertEqual(mock_write_to_file.call_count, 2) self.assertEqual(mock_ensure_tree.call_count, 7) self.assertEqual(mock_chmod.call_count, 2) + self.assertEqual(mock_restart_dnsmasq.call_count, 1) def _get_fixedips(self, network, host=None): return objects.FixedIPList.get_by_network(self.context, @@ -739,12 +745,15 @@ class LinuxNetworkTestCase(test.NoDBTestCase): @mock.patch.object(linux_net, 'write_to_file') @mock.patch('os.chmod') @mock.patch.object(linux_net, '_add_dhcp_mangle_rule') - @mock.patch.object(linux_net, '_execute') + @mock.patch('oslo_concurrency.processutils.execute') @mock.patch('nova.privsep.linux_net.iptables_get_rules', return_value=('', '')) @mock.patch('nova.privsep.linux_net.iptables_set_rules', return_value=('', '')) - def _test_dnsmasq_execute(self, mock_iptables_set_rules, + @mock.patch('nova.privsep.linux_net.restart_dnsmasq', + side_effect=nova.privsep.linux_net._restart_dnsmasq_inner) + def _test_dnsmasq_execute(self, mock_restart_dnsmasq, + mock_iptables_set_rules, mock_iptables_get_rules, mock_execute, mock_add_dhcp_mangle_rule, mock_chmod, mock_write_to_file,