From 544d05f24bba3ae620cf8cdb376e76a53e569608 Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Tue, 5 Feb 2019 18:36:48 -0800 Subject: [PATCH] Fix some driver library bugs This patch corrects some python3 byte string issues in the driver library callbacks. It also corrects an issue where multiple update calls may cause a bad file descriptor error. Change-Id: I3a03f2d8e65d48fe3791611486cb5da4961335b6 --- octavia_lib/api/drivers/driver_lib.py | 31 ++++++++++--------- .../tests/unit/api/drivers/test_driver_lib.py | 31 ++++++++++--------- setup.cfg | 1 + 3 files changed, 33 insertions(+), 30 deletions(-) diff --git a/octavia_lib/api/drivers/driver_lib.py b/octavia_lib/api/drivers/driver_lib.py index 3339263..117db38 100644 --- a/octavia_lib/api/drivers/driver_lib.py +++ b/octavia_lib/api/drivers/driver_lib.py @@ -28,37 +28,38 @@ class DriverLibrary(object): def __init__(self, status_socket=DEFAULT_STATUS_SOCKET, stats_socket=DEFAULT_STATS_SOCKET, **kwargs): - self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) - self.sock.settimeout(SOCKET_TIMEOUT) self.status_socket = status_socket self.stats_socket = stats_socket super(DriverLibrary, self).__init__(**kwargs) - def _recv(self): - size_str = '' - char = self.sock.recv(1) - while char != '\n': + def _recv(self, sock): + size_str = b'' + char = sock.recv(1) + while char != b'\n': size_str += char - char = self.sock.recv(1) + char = sock.recv(1) payload_size = int(size_str) mv_buffer = memoryview(bytearray(payload_size)) next_offset = 0 while payload_size - next_offset > 0: - recv_size = self.sock.recv_into(mv_buffer[next_offset:], - payload_size - next_offset) + recv_size = sock.recv_into(mv_buffer[next_offset:], + payload_size - next_offset) next_offset += recv_size return jsonutils.loads(mv_buffer.tobytes()) def _send(self, socket_path, data): - self.sock.connect(socket_path) + sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + sock.settimeout(SOCKET_TIMEOUT) + sock.connect(socket_path) try: - json_data = jsonutils.dumps(data) - self.sock.send('%d\n' % len(json_data)) - self.sock.sendall(json_data) - response = self._recv() + json_data = jsonutils.dump_as_bytes(data) + len_str = '{}\n'.format(len(json_data)).encode('utf-8') + sock.send(len_str) + sock.sendall(json_data) + response = self._recv(sock) finally: - self.sock.close() + sock.close() return response def update_loadbalancer_status(self, status): diff --git a/octavia_lib/tests/unit/api/drivers/test_driver_lib.py b/octavia_lib/tests/unit/api/drivers/test_driver_lib.py index 709ea57..2e8d4a2 100644 --- a/octavia_lib/tests/unit/api/drivers/test_driver_lib.py +++ b/octavia_lib/tests/unit/api/drivers/test_driver_lib.py @@ -21,40 +21,41 @@ from octavia_lib.tests.unit import base class TestDriverLib(base.TestCase): def setUp(self): - self.mock_socket = mock.MagicMock() - with mock.patch('socket.socket') as socket_mock: - socket_mock.return_value = self.mock_socket - self.driver_lib = driver_lib.DriverLibrary() + self.driver_lib = driver_lib.DriverLibrary() super(TestDriverLib, self).setUp() @mock.patch('six.moves.builtins.memoryview') def test_recv(self, mock_memoryview): - self.mock_socket.recv.side_effect = ['1', '\n'] - self.mock_socket.recv_into.return_value = 1 + mock_socket = mock.MagicMock() + mock_socket.recv.side_effect = [b'1', b'\n'] + mock_socket.recv_into.return_value = 1 mv_mock = mock.MagicMock() mock_memoryview.return_value = mv_mock - mv_mock.tobytes.return_value = '"test data"' + mv_mock.tobytes.return_value = b'"test data"' - response = self.driver_lib._recv() + response = self.driver_lib._recv(mock_socket) calls = [mock.call(1), mock.call(1)] - self.mock_socket.recv.assert_has_calls(calls) - self.mock_socket.recv_into.assert_called_once_with( + mock_socket.recv.assert_has_calls(calls) + mock_socket.recv_into.assert_called_once_with( mv_mock.__getitem__(), 1) self.assertEqual('test data', response) @mock.patch('octavia_lib.api.drivers.driver_lib.DriverLibrary._recv') def test_send(self, mock_recv): + mock_socket = mock.MagicMock() mock_recv.return_value = 'fake_response' - response = self.driver_lib._send('fake_path', 'test data') + with mock.patch('socket.socket') as socket_mock: + socket_mock.return_value = mock_socket + response = self.driver_lib._send('fake_path', 'test data') - self.mock_socket.connect.assert_called_once_with('fake_path') - self.mock_socket.send.assert_called_once_with('11\n') - self.mock_socket.sendall.assert_called_once_with('"test data"') - self.mock_socket.close.assert_called_once() + mock_socket.connect.assert_called_once_with('fake_path') + mock_socket.send.assert_called_once_with(b'11\n') + mock_socket.sendall.assert_called_once_with(b'"test data"') + mock_socket.close.assert_called_once() self.assertEqual(mock_recv.return_value, response) @mock.patch('octavia_lib.api.drivers.driver_lib.DriverLibrary._send') diff --git a/setup.cfg b/setup.cfg index d22236b..e3192b3 100644 --- a/setup.cfg +++ b/setup.cfg @@ -6,6 +6,7 @@ description-file = author = OpenStack author-email = openstack-discuss@lists.openstack.org home-page = https://docs.openstack.org/octavia-lib/latest/ +license = Apache License, Version 2.0 classifier = Development Status :: 5 - Production/Stable Environment :: OpenStack