Migrate "dhcp_release" to oslo.privsep

Story: #2007686
Task: #39976
Change-Id: I3414d06b9c6dfe549e79aab5fbe52c8f3ffd63f7

etc/neutron/rootwrap.d/dhcp.filters

@@ -20,8 +20,6 @@ kill_dnsmasq_script: CommandFilter, dnsmasq-kill, root
 
 ovs-vsctl: CommandFilter, ovs-vsctl, root
 mm-ctl: CommandFilter, mm-ctl, root
-dhcp_release: CommandFilter, dhcp_release, root
-dhcp_release6: CommandFilter, dhcp_release6, root
 
 # haproxy
 haproxy: RegExpFilter, haproxy, root, haproxy, -f, .*

neutron/agent/linux/dhcp.py

@@ -27,6 +27,7 @@ from neutron_lib.api.definitions import extra_dhcp_opt as edo_ext
 from neutron_lib import constants
 from neutron_lib import exceptions
 from neutron_lib.utils import file as file_utils
+from oslo_concurrency import processutils
 from oslo_log import log as logging
 from oslo_utils import excutils
 from oslo_utils import fileutils
@@ -42,6 +43,7 @@ from neutron.agent.linux import iptables_manager
 from neutron.cmd import runtime_checks as checks
 from neutron.common import utils as common_utils
 from neutron.ipam import utils as ipam_utils
+from neutron.privileged.agent.linux import dhcp as priv_dhcp
 
 LOG = logging.getLogger(__name__)
 
@@ -515,7 +517,8 @@ class Dnsmasq(DhcpLocalProcess):
 
     def _is_dhcp_release6_supported(self):
         if self._IS_DHCP_RELEASE6_SUPPORTED is None:
-            self._IS_DHCP_RELEASE6_SUPPORTED = checks.dhcp_release6_supported()
+            self._IS_DHCP_RELEASE6_SUPPORTED = (
+                priv_dhcp.dhcp_release6_supported())
             if not self._IS_DHCP_RELEASE6_SUPPORTED:
                 LOG.warning("dhcp_release6 is not present on this system, "
                             "will not call it again.")
@@ -530,24 +533,27 @@ class Dnsmasq(DhcpLocalProcess):
     def _release_lease(self, mac_address, ip, ip_version, client_id=None,
                        server_id=None, iaid=None):
         """Release a DHCP lease."""
-        if ip_version == constants.IP_VERSION_6:
-            if not self._is_dhcp_release6_supported():
-                return
-            cmd = ['dhcp_release6', '--iface', self.interface_name,
-                   '--ip', ip, '--client-id', client_id,
-                   '--server-id', server_id, '--iaid', iaid]
-        else:
-            cmd = ['dhcp_release', self.interface_name, ip, mac_address]
-            if client_id:
-                cmd.append(client_id)
-        ip_wrapper = ip_lib.IPWrapper(namespace=self.network.namespace)
         try:
-            ip_wrapper.netns.execute(cmd, run_as_root=True)
-        except RuntimeError as e:
+            if ip_version == constants.IP_VERSION_6:
+                if not self._is_dhcp_release6_supported():
+                    return
+
+                params = {'interface_name': self.interface_name,
+                          'ip_address': ip, 'client_id': client_id,
+                          'server_id': server_id, 'iaid': iaid,
+                          'namespace': self.network.namespace}
+                priv_dhcp.dhcp_release6(**params)
+            else:
+                params = {'interface_name': self.interface_name,
+                          'ip_address': ip, 'mac_address': mac_address,
+                          'client_id': client_id,
+                          'namespace': self.network.namespace}
+                priv_dhcp.dhcp_release(**params)
+        except (processutils.ProcessExecutionError, OSError) as e:
             # when failed to release single lease there's
             # no need to propagate error further
-            LOG.warning('DHCP release failed for %(cmd)s. '
-                        'Reason: %(e)s', {'cmd': cmd, 'e': e})
+            LOG.warning('DHCP release failed for params %(params)s. '
+                        'Reason: %(e)s', {'params': params, 'e': e})
 
     def _output_config_files(self):
         self._output_hosts_file()

neutron/cmd/runtime_checks.py

@@ -25,19 +25,6 @@ LOG = logging.getLogger(__name__)
 #       which would be run at system setup time. Please consider writing a
 #       sanity check instead.
 
-
-def dhcp_release6_supported():
-    try:
-        cmd = ['dhcp_release6', '--help']
-        env = {'LC_ALL': 'C'}
-        agent_utils.execute(cmd, addl_env=env)
-    except (OSError, RuntimeError, IndexError, ValueError) as e:
-        LOG.debug("Exception while checking dhcp_release6. "
-                  "Exception: %s", e)
-        return False
-    return True
-
-
 def dnsmasq_host_tag_support():
     cmd = ['dnsmasq', '--test', '--dhcp-host=tag:foo']
     env = {'LC_ALL': 'C', 'PATH': '/sbin:/usr/sbin'}

neutron/cmd/sanity/checks.py

@@ -33,10 +33,10 @@ from neutron.agent.linux import external_process
 from neutron.agent.linux import ip_lib
 from neutron.agent.linux import keepalived
 from neutron.agent.linux import utils as agent_utils
-from neutron.cmd import runtime_checks
 from neutron.common import utils as common_utils
 from neutron.plugins.ml2.drivers.openvswitch.agent.common \
     import constants as ovs_const
+from neutron.privileged.agent.linux import dhcp as priv_dhcp
 
 LOG = logging.getLogger(__name__)
 
@@ -239,7 +239,7 @@ def ovs_qos_direct_port_supported():
 
 
 def dhcp_release6_supported():
-    return runtime_checks.dhcp_release6_supported()
+    return priv_dhcp.dhcp_release6_supported()
 
 
 def bridge_firewalling_enabled():

neutron/privileged/__init__.py

@@ -28,3 +28,12 @@ default = priv_context.PrivContext(
                   caps.CAP_DAC_READ_SEARCH,
                   caps.CAP_SYS_PTRACE],
 )
+
+
+dhcp_release_cmd = priv_context.PrivContext(
+    __name__,
+    cfg_section='privsep_dhcp_release',
+    pypath=__name__ + '.dhcp_release_cmd',
+    capabilities=[caps.CAP_SYS_ADMIN,
+                  caps.CAP_NET_ADMIN]
+)

neutron/privileged/agent/linux/dhcp.py

@@ -0,0 +1,48 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from oslo_concurrency import processutils
+
+from neutron import privileged
+
+
+@privileged.dhcp_release_cmd.entrypoint
+def dhcp_release(interface_name, ip_address, mac_address, client_id,
+                 namespace=None):
+    cmd = []
+    if namespace:
+        cmd += ['ip', 'netns', 'exec', namespace]
+    cmd += ['dhcp_release', interface_name, ip_address, mac_address]
+    if client_id:
+        cmd += client_id
+    log_errors = processutils.LOG_FINAL_ERROR
+    return processutils.execute(*cmd, log_errors=log_errors)
+
+
+@privileged.dhcp_release_cmd.entrypoint
+def dhcp_release6(interface_name, ip_address, client_id, server_id, iaid,
+                  namespace=None):
+    cmd = []
+    if namespace:
+        cmd += ['ip', 'netns', 'exec', namespace]
+    cmd += ['dhcp_release6', '--iface', interface_name, '--ip', ip_address,
+            '--client-id', client_id, '--server-id', server_id, '--iaid', iaid]
+    log_errors = processutils.LOG_FINAL_ERROR
+    return processutils.execute(*cmd, log_errors=log_errors)
+
+
+@privileged.dhcp_release_cmd.entrypoint
+def dhcp_release6_supported():
+    cmd = ['dhcp_release6', '--help']
+    result = processutils.execute(*cmd, check_exit_code=False,
+                                  env_variables={'LC_ALL': 'C'})
+    return not bool(result[1])

neutron/tests/unit/agent/dhcp/test_agent.py

@@ -88,11 +88,11 @@ fake_ipv6_subnet = dhcp.DictModel(id='bbbbbbbb-1111-2222-bbbbbbbbbbbb',
                                   ipv6_ra_mode='slaac', ipv6_address_mode=None)
 
 fake_meta_subnet = dhcp.DictModel(dict(id='bbbbbbbb-1111-2222-bbbbbbbbbbbb',
-                                  network_id=FAKE_NETWORK_UUID,
-                                  cidr='169.254.169.252/30',
-                                  gateway_ip='169.254.169.253',
-                                  enable_dhcp=True,
-                                  ip_version=const.IP_VERSION_4))
+                                       network_id=FAKE_NETWORK_UUID,
+                                       cidr='169.254.169.252/30',
+                                       gateway_ip='169.254.169.253',
+                                       enable_dhcp=True,
+                                       ip_version=const.IP_VERSION_4))
 
 fake_fixed_ip1 = dhcp.DictModel(id='', subnet_id=fake_subnet1.id,
                                 ip_address='172.9.9.9')
@@ -1040,10 +1040,10 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
 
     def test_refresh_dhcp_helper_no_dhcp_enabled_networks(self):
         network = dhcp.NetModel(dict(id='net-id',
-                                tenant_id=FAKE_TENANT_ID,
-                                admin_state_up=True,
-                                subnets=[],
-                                ports=[]))
+                                     tenant_id=FAKE_TENANT_ID,
+                                     admin_state_up=True,
+                                     subnets=[],
+                                     ports=[]))
 
         self.cache.get_network_by_id.return_value = network
         self.plugin.get_network_info.return_value = network
@@ -1057,10 +1057,10 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
 
     def test_refresh_dhcp_helper_exception_during_rpc(self):
         network = dhcp.NetModel(dict(id='net-id',
-                                tenant_id=FAKE_TENANT_ID,
-                                admin_state_up=True,
-                                subnets=[],
-                                ports=[]))
+                                     tenant_id=FAKE_TENANT_ID,
+                                     admin_state_up=True,
+                                     subnets=[],
+                                     ports=[]))
 
         self.cache.get_network_by_id.return_value = network
         self.plugin.get_network_info.side_effect = Exception
@@ -1148,10 +1148,10 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
 
     def test_subnet_update_end_restart(self):
         new_state = dhcp.NetModel(dict(id=fake_network.id,
-                                  tenant_id=fake_network.tenant_id,
-                                  admin_state_up=True,
-                                  subnets=[fake_subnet1, fake_subnet3],
-                                  ports=[fake_port1]))
+                                       tenant_id=fake_network.tenant_id,
+                                       admin_state_up=True,
+                                       subnets=[fake_subnet1, fake_subnet3],
+                                       ports=[fake_port1]))
 
         payload = dict(subnet=dict(network_id=fake_network.id),
                        priority=FAKE_PRIORITY)
@@ -1167,10 +1167,10 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
 
     def test_subnet_delete_end_no_network_id(self):
         prev_state = dhcp.NetModel(dict(id=fake_network.id,
-                                   tenant_id=fake_network.tenant_id,
-                                   admin_state_up=True,
-                                   subnets=[fake_subnet1, fake_subnet3],
-                                   ports=[fake_port1]))
+                                        tenant_id=fake_network.tenant_id,
+                                        admin_state_up=True,
+                                        subnets=[fake_subnet1, fake_subnet3],
+                                        ports=[fake_port1]))
 
         payload = dict(subnet_id=fake_subnet1.id, priority=FAKE_PRIORITY)
         self.cache.get_network_by_subnet_id.return_value = prev_state
@@ -1192,10 +1192,10 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
 
     def test_subnet_update_end_delete_payload(self):
         prev_state = dhcp.NetModel(dict(id=fake_network.id,
-                                   tenant_id=fake_network.tenant_id,
-                                   admin_state_up=True,
-                                   subnets=[fake_subnet1, fake_subnet3],
-                                   ports=[fake_port1]))
+                                        tenant_id=fake_network.tenant_id,
+                                        admin_state_up=True,
+                                        subnets=[fake_subnet1, fake_subnet3],
+                                        ports=[fake_port1]))
 
         payload = dict(subnet_id=fake_subnet1.id, network_id=fake_network.id,
                        priority=FAKE_PRIORITY)

neutron/tests/unit/agent/linux/test_dhcp.py

@@ -34,6 +34,7 @@ from neutron.cmd import runtime_checks as checks
 from neutron.conf.agent import common as config
 from neutron.conf.agent import dhcp as dhcp_config
 from neutron.conf import common as base_config
+from neutron.privileged.agent.linux import dhcp as priv_dhcp
 from neutron.tests import base
 
 
@@ -2244,19 +2245,24 @@ class TestDnsmasq(TestBase):
                                 'server_id': 'server_id'}
                           },
                          {}])
-        ipw = mock.patch(
-            'neutron.agent.linux.ip_lib.IpNetnsCommand.execute').start()
+        mock_dhcp_release = mock.patch.object(priv_dhcp,
+                                              'dhcp_release').start()
+        mock_dhcp_release6 = mock.patch.object(priv_dhcp,
+                                               'dhcp_release6').start()
+        mock_dhcp_release6_supported = mock.patch.object(
+            priv_dhcp, 'dhcp_release6_supported').start()
         dnsmasq._release_unused_leases()
         # Verify that dhcp_release is called both for ipv4 and ipv6 addresses.
-        self.assertEqual(2, ipw.call_count)
-        ipw.assert_has_calls([mock.call(['dhcp_release6',
-                                         '--iface', None, '--ip', ip1,
-                                         '--client-id', 'client_id',
-                                         '--server-id', 'server_id',
-                                         '--iaid', 0xff],
-                                        run_as_root=True)])
-        ipw.assert_has_calls([mock.call(['dhcp_release', None, ip2, mac2],
-                             run_as_root=True), ])
+        self.assertEqual(1, mock_dhcp_release.call_count)
+        self.assertEqual(1, mock_dhcp_release6.call_count)
+        mock_dhcp_release.assert_called_once_with(
+            interface_name=None, ip_address=ip2, mac_address=mac2,
+            client_id=None, namespace=dnsmasq.network.namespace)
+        mock_dhcp_release6.assert_called_once_with(
+            interface_name=None, ip_address=ip1, client_id='client_id',
+            server_id='server_id', iaid=0xff,
+            namespace=dnsmasq.network.namespace)
+        mock_dhcp_release6_supported.assert_called_once_with()
 
     def test_release_for_ipv6_lease_no_dhcp_release6(self):
         dnsmasq = self._get_dnsmasq(FakeDualNetwork())