Improve DNS Service test coverage
- Cleaned up error handling logic to make testing easier. Change-Id: Idddf63a4decc31895e3c5a3b9a11f51cddffedaa
This commit is contained in:
Erik Olof Gunnar Andersson
parent
61b408287f
commit
991e0afe44
@ -169,10 +169,11 @@ class DNSService(object):
|
||||
|
||||
def _start(self, host, port):
|
||||
sock_tcp = utils.bind_tcp(
|
||||
host, port, self.tcp_backlog)
|
||||
|
||||
host, port, self.tcp_backlog
|
||||
)
|
||||
sock_udp = utils.bind_udp(
|
||||
host, port)
|
||||
host, port
|
||||
)
|
||||
|
||||
self._dns_socks_tcp.append(sock_tcp)
|
||||
self._dns_socks_udp.append(sock_udp)
|
||||
@ -194,6 +195,7 @@ class DNSService(object):
|
||||
|
||||
client = None
|
||||
while self._running.is_set():
|
||||
addr = None
|
||||
try:
|
||||
# handle a new TCP connection
|
||||
client, addr = sock_tcp.accept()
|
||||
@ -201,11 +203,21 @@ class DNSService(object):
|
||||
if self.tcp_recv_timeout:
|
||||
client.settimeout(self.tcp_recv_timeout)
|
||||
|
||||
LOG.debug('Handling TCP Request from: %(host)s:%(port)d',
|
||||
{'host': addr[0], 'port': addr[1]})
|
||||
LOG.debug(
|
||||
'Handling TCP Request from: %(host)s:%(port)d',
|
||||
{
|
||||
'host': addr[0],
|
||||
'port': addr[1]
|
||||
}
|
||||
)
|
||||
if len(addr) == 4:
|
||||
LOG.debug('Flow info: %(host)s scope: %(port)d',
|
||||
{'host': addr[2], 'port': addr[3]})
|
||||
LOG.debug(
|
||||
'Flow info: %(host)s scope: %(port)d',
|
||||
{
|
||||
'host': addr[2],
|
||||
'port': addr[3]
|
||||
}
|
||||
)
|
||||
|
||||
# Dispatch a thread to handle the connection
|
||||
self.tg.add_thread(self._dns_handle_tcp_conn, addr, client)
|
||||
@ -219,14 +231,27 @@ class DNSService(object):
|
||||
if client:
|
||||
client.close()
|
||||
errname = errno.errorcode[e.args[0]]
|
||||
LOG.warning('Socket error %(err)s from: %(host)s:%(port)d',
|
||||
{'host': addr[0], 'port': addr[1], 'err': errname})
|
||||
addr = addr or (None, 0)
|
||||
LOG.warning(
|
||||
'Socket error %(err)s from: %(host)s:%(port)d',
|
||||
{
|
||||
'host': addr[0],
|
||||
'port': addr[1],
|
||||
'err': errname
|
||||
}
|
||||
)
|
||||
except Exception:
|
||||
if client:
|
||||
client.close()
|
||||
LOG.exception('Unknown exception handling TCP request from: '
|
||||
'%(host)s:%(port)d',
|
||||
{'host': addr[0], 'port': addr[1]})
|
||||
addr = addr or (None, 0)
|
||||
LOG.exception(
|
||||
'Unknown exception handling TCP request from: '
|
||||
'%(host)s:%(port)d',
|
||||
{
|
||||
'host': addr[0],
|
||||
'port': addr[1]
|
||||
}
|
||||
)
|
||||
|
||||
def _dns_handle_tcp_conn(self, addr, client):
|
||||
"""
|
||||
@ -240,7 +265,7 @@ class DNSService(object):
|
||||
(IPv6 addr, Port, Flow info, Scope ID)
|
||||
:type addr: tuple
|
||||
:param client: Client socket
|
||||
:type client: socket
|
||||
:type client: socket.socket
|
||||
:raises: None
|
||||
"""
|
||||
host, port = addr[:2]
|
||||
@ -282,20 +307,40 @@ class DNSService(object):
|
||||
client.sendall(tcp_response)
|
||||
|
||||
except socket.timeout:
|
||||
LOG.info('TCP Timeout from: %(host)s:%(port)d',
|
||||
{'host': host, 'port': port})
|
||||
LOG.info(
|
||||
'TCP Timeout from: %(host)s:%(port)d',
|
||||
{
|
||||
'host': host,
|
||||
'port': port
|
||||
}
|
||||
)
|
||||
except socket.error as e:
|
||||
errname = errno.errorcode[e.args[0]]
|
||||
LOG.warning('Socket error %(err)s from: %(host)s:%(port)d',
|
||||
{'host': host, 'port': port, 'err': errname})
|
||||
|
||||
LOG.warning(
|
||||
'Socket error %(err)s from: %(host)s:%(port)d',
|
||||
{
|
||||
'host': host,
|
||||
'port': port,
|
||||
'err': errname
|
||||
}
|
||||
)
|
||||
except struct.error:
|
||||
LOG.warning('Invalid packet from: %(host)s:%(port)d',
|
||||
{'host': host, 'port': port})
|
||||
|
||||
LOG.warning(
|
||||
'Invalid packet from: %(host)s:%(port)d',
|
||||
{
|
||||
'host': host,
|
||||
'port': port
|
||||
}
|
||||
)
|
||||
except Exception:
|
||||
LOG.exception('Unknown exception handling TCP request from: '
|
||||
"%(host)s:%(port)d", {'host': host, 'port': port})
|
||||
LOG.exception(
|
||||
'Unknown exception handling TCP request from: '
|
||||
'%(host)s:%(port)d',
|
||||
{
|
||||
'host': host,
|
||||
'port': port
|
||||
}
|
||||
)
|
||||
finally:
|
||||
if client:
|
||||
client.close()
|
||||
@ -304,19 +349,25 @@ class DNSService(object):
|
||||
"""Handle a DNS Query over UDP in a dedicated thread
|
||||
|
||||
:param sock_udp: UDP socket
|
||||
:type sock_udp: socket
|
||||
:type sock_udp: socket.socket
|
||||
:raises: None
|
||||
"""
|
||||
LOG.info('_handle_udp thread started')
|
||||
|
||||
while self._running.is_set():
|
||||
addr = None
|
||||
try:
|
||||
# TODO(kiall): Determine the appropriate default value for
|
||||
# UDP recvfrom.
|
||||
payload, addr = sock_udp.recvfrom(8192)
|
||||
|
||||
LOG.debug('Handling UDP Request from: %(host)s:%(port)d',
|
||||
{'host': addr[0], 'port': addr[1]})
|
||||
LOG.debug(
|
||||
'Handling UDP Request from: %(host)s:%(port)d',
|
||||
{
|
||||
'host': addr[0],
|
||||
'port': addr[1]
|
||||
}
|
||||
)
|
||||
|
||||
# Dispatch a thread to handle the query
|
||||
self.tg.add_thread(self._dns_handle_udp_query, sock_udp, addr,
|
||||
@ -325,19 +376,32 @@ class DNSService(object):
|
||||
pass
|
||||
except socket.error as e:
|
||||
errname = errno.errorcode[e.args[0]]
|
||||
LOG.warning('Socket error %(err)s from: %(host)s:%(port)d',
|
||||
{'host': addr[0], 'port': addr[1], 'err': errname})
|
||||
addr = addr or (None, 0)
|
||||
LOG.warning(
|
||||
'Socket error %(err)s from: %(host)s:%(port)d',
|
||||
{
|
||||
'host': addr[0],
|
||||
'port': addr[1],
|
||||
'err': errname
|
||||
}
|
||||
)
|
||||
except Exception:
|
||||
LOG.exception('Unknown exception handling UDP request from: '
|
||||
'%(host)s:%(port)d',
|
||||
{'host': addr[0], 'port': addr[1]})
|
||||
addr = addr or (None, 0)
|
||||
LOG.exception(
|
||||
'Unknown exception handling UDP request from: '
|
||||
'%(host)s:%(port)d',
|
||||
{
|
||||
'host': addr[0],
|
||||
'port': addr[1]
|
||||
}
|
||||
)
|
||||
|
||||
def _dns_handle_udp_query(self, sock, addr, payload):
|
||||
"""
|
||||
Handle a DNS Query over UDP
|
||||
|
||||
:param sock: UDP socket
|
||||
:type sock: socket
|
||||
:type sock: socket.socket
|
||||
:param addr: Tuple of the client's (IP, Port)
|
||||
:type addr: tuple
|
||||
:param payload: Raw DNS query payload
|
||||
@ -346,17 +410,18 @@ class DNSService(object):
|
||||
"""
|
||||
try:
|
||||
# Call into the DNS Application itself with the payload and addr
|
||||
for response in self.app(
|
||||
{'payload': payload, 'addr': addr}):
|
||||
|
||||
# Send back a response only if present
|
||||
for response in self.app({'payload': payload, 'addr': addr}):
|
||||
if response is not None:
|
||||
sock.sendto(response, addr)
|
||||
|
||||
except Exception:
|
||||
LOG.exception('Unhandled exception while processing request from '
|
||||
"%(host)s:%(port)d",
|
||||
{'host': addr[0], 'port': addr[1]})
|
||||
LOG.exception(
|
||||
'Unhandled exception while processing request from '
|
||||
'%(host)s:%(port)d',
|
||||
{
|
||||
'host': addr[0],
|
||||
'port': addr[1]
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
_launcher = None
|
||||
|
||||
@ -9,20 +9,81 @@
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
import errno
|
||||
import socket
|
||||
from unittest import mock
|
||||
|
||||
from oslo_config import cfg
|
||||
from oslo_config import fixture as cfg_fixture
|
||||
from oslo_service import service
|
||||
import oslotest.base
|
||||
|
||||
from designate.common import profiler
|
||||
from designate.mdns import handler
|
||||
from designate import policy
|
||||
from designate import rpc
|
||||
from designate import service as designate_service
|
||||
from designate import utils
|
||||
|
||||
CONF = cfg.CONF
|
||||
|
||||
|
||||
class TestBaseService(oslotest.base.BaseTestCase):
|
||||
def tearDown(self):
|
||||
designate_service._launcher = None
|
||||
super(TestBaseService, self).tearDown()
|
||||
|
||||
@mock.patch.object(service, 'launch')
|
||||
def test_serve(self, mock_service_launch):
|
||||
server = mock.Mock()
|
||||
designate_service.serve(server)
|
||||
|
||||
mock_service_launch.assert_called_with(
|
||||
mock.ANY, server, workers=None, restart_method='mutate'
|
||||
)
|
||||
|
||||
@mock.patch.object(service, 'launch')
|
||||
def test_serve_twice(self, _):
|
||||
server = mock.Mock()
|
||||
designate_service.serve(server)
|
||||
self.assertRaisesRegex(
|
||||
RuntimeError,
|
||||
r'serve\(\) can only be called once',
|
||||
designate_service.serve, server
|
||||
)
|
||||
|
||||
@mock.patch.object(rpc, 'cleanup')
|
||||
@mock.patch.object(service, 'launch')
|
||||
def test_wait(self, mock_service_launch, mock_rpc_cleanup):
|
||||
server = mock.Mock()
|
||||
|
||||
designate_service.serve(server)
|
||||
designate_service.wait()
|
||||
|
||||
mock_service_launch.assert_called_with(
|
||||
mock.ANY, server, workers=None, restart_method='mutate'
|
||||
)
|
||||
mock_rpc_cleanup.assert_called()
|
||||
|
||||
@mock.patch.object(rpc, 'cleanup')
|
||||
@mock.patch.object(service, 'launch')
|
||||
def test_wait_keyboard_interrupt(self, mock_service_launch,
|
||||
mock_rpc_cleanup):
|
||||
server = mock.Mock()
|
||||
mock_launcher = mock.Mock()
|
||||
mock_service_launch.return_value = mock_launcher
|
||||
mock_launcher.wait.side_effect = [KeyboardInterrupt]
|
||||
|
||||
designate_service.serve(server)
|
||||
designate_service.wait()
|
||||
|
||||
mock_rpc_cleanup.assert_called()
|
||||
|
||||
|
||||
@mock.patch.object(policy, 'init')
|
||||
@mock.patch.object(rpc, 'init')
|
||||
@mock.patch.object(profiler, 'setup_profiler')
|
||||
class TestDesignateServiceInit(oslotest.base.BaseTestCase):
|
||||
class TestServiceInit(oslotest.base.BaseTestCase):
|
||||
def test_service_init(self, mock_setup_profiler, mock_rpc_init,
|
||||
mock_policy_init):
|
||||
service = designate_service.Service('test-service')
|
||||
@ -48,13 +109,13 @@ class TestDesignateServiceInit(oslotest.base.BaseTestCase):
|
||||
self.assertEqual('test-rpc-service', service.name)
|
||||
|
||||
|
||||
class TestDesignateRpcService(oslotest.base.BaseTestCase):
|
||||
class TestRpcService(oslotest.base.BaseTestCase):
|
||||
@mock.patch.object(policy, 'init')
|
||||
@mock.patch.object(rpc, 'init')
|
||||
@mock.patch.object(profiler, 'setup_profiler')
|
||||
def setUp(self, mock_setup_profiler, mock_rpc_init,
|
||||
mock_policy_init):
|
||||
super(TestDesignateRpcService, self).setUp()
|
||||
super(TestRpcService, self).setUp()
|
||||
self.service = designate_service.RPCService(
|
||||
'test-rpc-service', 'test-topic'
|
||||
)
|
||||
@ -89,3 +150,132 @@ class TestDesignateRpcService(oslotest.base.BaseTestCase):
|
||||
|
||||
def test_rpc_service_wait(self):
|
||||
self.assertIsNone(self.service.wait())
|
||||
|
||||
|
||||
class TestDNSService(oslotest.base.BaseTestCase):
|
||||
def setUp(self):
|
||||
super(TestDNSService, self).setUp()
|
||||
self.useFixture(cfg_fixture.Config(CONF))
|
||||
|
||||
self.tg = mock.Mock()
|
||||
self.storage = mock.Mock()
|
||||
self.application = handler.RequestHandler(self.storage, self.tg)
|
||||
|
||||
self.service = designate_service.DNSService(
|
||||
self.application, self.tg,
|
||||
CONF['service:mdns'].listen,
|
||||
CONF['service:mdns'].tcp_backlog,
|
||||
CONF['service:mdns'].tcp_recv_timeout,
|
||||
)
|
||||
self.service._running = mock.Mock()
|
||||
|
||||
def test_service_init(self):
|
||||
self.assertEqual(CONF['service:mdns'].listen, self.service.listen)
|
||||
self.assertEqual(
|
||||
CONF['service:mdns'].tcp_backlog, self.service.tcp_backlog
|
||||
)
|
||||
self.assertEqual(
|
||||
CONF['service:mdns'].tcp_recv_timeout,
|
||||
self.service.tcp_recv_timeout
|
||||
)
|
||||
|
||||
@mock.patch.object(utils, 'bind_tcp')
|
||||
@mock.patch.object(utils, 'bind_udp')
|
||||
def test_service_start(self, mock_bind_udp, mock_bind_tcp):
|
||||
self.service.start()
|
||||
|
||||
mock_bind_udp.assert_called_with('0.0.0.0', 5354)
|
||||
mock_bind_tcp.assert_called_with(
|
||||
'0.0.0.0', 5354, CONF['service:mdns'].tcp_backlog
|
||||
)
|
||||
|
||||
def test_service_stop(self):
|
||||
mock_sock_tcp = mock.Mock()
|
||||
mock_sock_udp = mock.Mock()
|
||||
|
||||
self.service._dns_socks_tcp = [mock_sock_tcp]
|
||||
self.service._dns_socks_udp = [mock_sock_udp]
|
||||
|
||||
self.service.stop()
|
||||
|
||||
mock_sock_tcp.close.assert_called_once()
|
||||
mock_sock_udp.close.assert_called_once()
|
||||
|
||||
def test_handle_tcp(self):
|
||||
self.service._running.is_set.side_effect = [True, True, False]
|
||||
|
||||
mock_client = mock.Mock()
|
||||
addr = ('192.0.2.1', 5353, '127.0.0.1', 5353)
|
||||
|
||||
mock_sock_tcp = mock.Mock()
|
||||
mock_sock_tcp.accept.return_value = (mock_client, addr)
|
||||
|
||||
self.assertIsNone(self.service._dns_handle_tcp(mock_sock_tcp))
|
||||
|
||||
mock_sock_tcp.accept.assert_called()
|
||||
mock_client.settimeout.assert_called()
|
||||
|
||||
def test_handle_tcp_handle_errors(self):
|
||||
self.service._running.is_set.side_effect = [True, True, True, False]
|
||||
|
||||
mock_sock_tcp = mock.Mock()
|
||||
mock_sock_tcp.accept.side_effect = [
|
||||
socket.timeout(), socket.error(errno.EACCES), Exception()
|
||||
]
|
||||
|
||||
self.assertIsNone(self.service._dns_handle_tcp(mock_sock_tcp))
|
||||
|
||||
mock_sock_tcp.accept.assert_called()
|
||||
|
||||
def test_handle_tcp_without_timeout(self):
|
||||
CONF.set_override('tcp_recv_timeout', 0, 'service:mdns')
|
||||
|
||||
self.service = designate_service.DNSService(
|
||||
self.application, self.tg,
|
||||
CONF['service:mdns'].listen,
|
||||
CONF['service:mdns'].tcp_backlog,
|
||||
CONF['service:mdns'].tcp_recv_timeout,
|
||||
)
|
||||
self.service._running = mock.Mock()
|
||||
self.service._running.is_set.side_effect = [True, True, False]
|
||||
|
||||
mock_client = mock.Mock()
|
||||
addr = ('192.0.2.1', 5353)
|
||||
|
||||
mock_sock_tcp = mock.Mock()
|
||||
mock_sock_tcp.accept.return_value = (mock_client, addr)
|
||||
|
||||
self.assertIsNone(self.service._dns_handle_tcp(mock_sock_tcp))
|
||||
|
||||
mock_sock_tcp.accept.assert_called()
|
||||
mock_client.settimeout.assert_not_called()
|
||||
|
||||
def test_handle_tcp_running_not_set(self):
|
||||
mock_sock_tcp = mock.Mock()
|
||||
self.service._running.is_set.side_effect = [False]
|
||||
self.assertIsNone(self.service._dns_handle_tcp(mock_sock_tcp))
|
||||
|
||||
def test_handle_udp(self):
|
||||
self.service._running.is_set.side_effect = [True, True, False]
|
||||
|
||||
mock_client = mock.Mock()
|
||||
addr = ('192.0.2.1', 5353)
|
||||
|
||||
mock_sock_udp = mock.Mock()
|
||||
mock_sock_udp.recvfrom.return_value = (mock_client, addr)
|
||||
|
||||
self.assertIsNone(self.service._dns_handle_udp(mock_sock_udp))
|
||||
|
||||
mock_sock_udp.recvfrom.assert_called()
|
||||
|
||||
def test_handle_udp_handle_errors(self):
|
||||
self.service._running.is_set.side_effect = [True, True, True, False]
|
||||
|
||||
mock_sock_tcp = mock.Mock()
|
||||
mock_sock_tcp.recvfrom.side_effect = [
|
||||
socket.timeout(), socket.error(errno.EACCES), Exception()
|
||||
]
|
||||
|
||||
self.assertIsNone(self.service._dns_handle_udp(mock_sock_tcp))
|
||||
|
||||
mock_sock_tcp.recvfrom.assert_called()
|
||||
|
||||
Loading…
Reference in New Issue
Block a user