From 10f19870c4bc503f414d5beef92c3939d91764d9 Mon Sep 17 00:00:00 2001 From: Michael Chapman Date: Wed, 4 Nov 2020 15:24:43 +1100 Subject: [PATCH] Add timeout to rndc commands In the event of a backend BIND server being unreachable for any reason, rndc commands will persist for a very long time and can consume significant resources. This can be seen when running devstack with a pool configured to point at a bind server that doesn't exist - the rndc process count can climb into the thousands. An optional timeout has been added to rndc to alleviate this. Change-Id: Idd61e79715b21fdd3249136cf68a7b9d3069c3f9 Related-Bug: 1896783 --- designate/backend/impl_bind9.py | 13 +++++++-- designate/conf/bind9.py | 1 + designate/tests/unit/backend/test_bind9.py | 33 +++++++++++++++++++++- lower-constraints.txt | 2 +- requirements.txt | 2 +- 5 files changed, 45 insertions(+), 6 deletions(-) diff --git a/designate/backend/impl_bind9.py b/designate/backend/impl_bind9.py index b1b7d65e3..ab8d06b09 100644 --- a/designate/backend/impl_bind9.py +++ b/designate/backend/impl_bind9.py @@ -21,6 +21,8 @@ Bind 9 backend. Create and delete zones by executing rndc import random import six +import subprocess + from oslo_log import log as logging from oslo_utils import strutils @@ -51,6 +53,9 @@ class Bind9Backend(base.Backend): self.options.get('clean_zonefile', 'false')) self._rndc_call_base = self._generate_rndc_base_call() + self._rndc_timeout = self.options.get('rndc_timeout', None) + if self._rndc_timeout == 0: + self._rndc_timeout = None def _generate_rndc_base_call(self): """Generate argument list to execute rndc""" @@ -174,7 +179,9 @@ class Bind9Backend(base.Backend): """ try: rndc_call = self._rndc_call_base + rndc_op - LOG.debug('Executing RNDC call: %r', rndc_call) - utils.execute(*rndc_call) - except utils.processutils.ProcessExecutionError as e: + LOG.debug('Executing RNDC call: %r with timeout %s', + rndc_call, self._rndc_timeout) + utils.execute(*rndc_call, timeout=self._rndc_timeout) + except (utils.processutils.ProcessExecutionError, + subprocess.TimeoutExpired) as e: raise exceptions.Backend(e) diff --git a/designate/conf/bind9.py b/designate/conf/bind9.py index 34086671e..beee8696a 100644 --- a/designate/conf/bind9.py +++ b/designate/conf/bind9.py @@ -26,6 +26,7 @@ BINS9_OPTS = [ cfg.StrOpt('rndc_config_file', help='RNDC Config File'), cfg.StrOpt('rndc_key_file', help='RNDC Key File'), + cfg.IntOpt('rndc_timeout', default=0, min=0, help='RNDC command timeout'), cfg.StrOpt('zone_file_path', default='$state_path/zones', help='Path where zone files are stored'), cfg.StrOpt('query_destination', default='127.0.0.1', diff --git a/designate/tests/unit/backend/test_bind9.py b/designate/tests/unit/backend/test_bind9.py index 8e4abfa63..c1189e84a 100644 --- a/designate/tests/unit/backend/test_bind9.py +++ b/designate/tests/unit/backend/test_bind9.py @@ -20,6 +20,8 @@ from designate import utils from designate.backend import impl_bind9 from designate.tests import fixtures +import subprocess + class Bind9BackendTestCase(designate.tests.TestCase): def setUp(self): @@ -223,7 +225,36 @@ class Bind9BackendTestCase(designate.tests.TestCase): mock_execute.assert_called_with( '/usr/sbin/rndc', '-s', '192.168.2.4', '-p', '953', '-c', '/etc/rndc.conf', '-k', '/etc/rndc.key', - 'delzone', 'example.com ' + 'delzone', 'example.com ', timeout=None + ) + + @mock.patch('designate.utils.execute') + def test_execute_rndc_timeout(self, mock_execute): + rndc_op = ['delzone', 'example.com '] + + self.backend._rndc_timeout = 10 + self.backend._execute_rndc(rndc_op) + + mock_execute.assert_called_with( + '/usr/sbin/rndc', '-s', '192.168.2.4', '-p', '953', + '-c', '/etc/rndc.conf', '-k', '/etc/rndc.key', + 'delzone', 'example.com ', timeout=10 + ) + + @mock.patch('designate.utils.execute') + def test_execute_rndc_timeout_exception(self, mock_execute): + rndc_op = ['delzone', 'example.com '] + + self.backend._rndc_timeout = 10 + + mock_execute.side_effect = subprocess.TimeoutExpired([ + '/usr/sbin/rndc', '-s', '192.168.2.4', '-p', '953', + '-c', '/etc/rndc.conf', '-k', '/etc/rndc.key', + 'delzone', 'example.com '], 10) + + self.assertRaises( + exceptions.Backend, + self.backend._execute_rndc, rndc_op ) @mock.patch('designate.utils.execute') diff --git a/lower-constraints.txt b/lower-constraints.txt index 998c5d841..ac48bfd3b 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -69,7 +69,7 @@ os-service-types==1.2.0 os-win==3.0.0 osc-lib==1.10.0 oslo.cache==1.29.0 -oslo.concurrency==3.26.0 +oslo.concurrency==4.2.0 oslo.config==5.2.0 oslo.context==2.19.2 oslo.db==8.3.0 diff --git a/requirements.txt b/requirements.txt index b4285b03b..b9ea2b80f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,7 +11,7 @@ keystoneauth1>=3.4.0 # Apache-2.0 keystonemiddleware>=4.17.0 # Apache-2.0 netaddr>=0.7.18 # BSD oslo.config>=5.2.0 # Apache-2.0 -oslo.concurrency>=3.26.0 # Apache-2.0 +oslo.concurrency>=4.2.0 # Apache-2.0 oslo.messaging>=12.4.0 # Apache-2.0 oslo.middleware>=3.31.0 # Apache-2.0 oslo.log>=3.36.0 # Apache-2.0