openstack
/
kuryr-kubernetes
Code Issues Proposed changes

Add error handling and logging to CNI daemon 

This adds error handling and improves logging in CNI daemon. Besides
that unit tests for new codepaths are added.

Implements: blueprint cni-split-exec-daemon

Change-Id: Ieff94a5c0b85e8d958421fc4db2f09a411f4e667
This commit is contained in:
Michał Dulko 2017-10-12 12:38:51 +02:00
parent d688e6f6ec
commit 7b4b68e16e
4 changed files with 107 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
kuryr_kubernetes/cni/api.py
View File

@ -147,12 +147,20 @@ class CNIDaemonizedRunner(CNIRunner):
def _add(self, params):
resp = self._make_request('addNetwork', params)
if resp.status != 201:
LOG.error('CNI daemon returned error "%(status)d %(reason)s".',
{'status': resp.status, 'reason': resp.reason})
raise k_exc.CNIError('Got invalid status code from CNI daemon.')
vif_dict = jsonutils.loads(resp.read())
vif = base.VersionedObject.obj_from_primitive(vif_dict)
return self._vif_data(vif)
def _delete(self, params):
self._make_request('delNetwork', params)
resp = self._make_request('delNetwork', params)
if resp.status != 204:
LOG.error('CNI daemon returned error "%(status)d %(reason)s".',
{'status': resp.status, 'reason': resp.reason})
raise k_exc.CNIError('Got invalid status code from CNI daemon.')
def prepare_env(self, env, stdin):
cni_envs = {}
@ -162,13 +170,6 @@ class CNIDaemonizedRunner(CNIRunner):
return cni_envs
def _make_request(self, base, cni_envs):
"""Make request
:param method
:param path
:param kwargs (body, headers)
:return response
"""
method = 'POST'
body = jsonutils.dumps(cni_envs)
headers = {
@ -179,7 +180,18 @@ class CNIDaemonizedRunner(CNIRunner):
# (janonymous) make base configurable
socket_path = config.CONF.kubernetes.sock_file
conn = UnixDomainHttpConnection(socket_path)
conn.request(method, path, body=body, headers=headers)
resp = conn.getresponse()
try:
conn = UnixDomainHttpConnection(socket_path)
LOG.debug('Making request to CNI Daemon. Method: %(method)s,'
'path: %(path)s, headers: %(headers)s, body %(body)s',
{'method': method, 'path': path, 'body': body,
'headers': headers})
conn.request(method, path, body=body, headers=headers)
resp = conn.getresponse()
except (socket.error, OSError):
LOG.exception('Looks like the socket file is not present in %s. Is'
'kuryr-daemon running?', socket_path)
raise
LOG.debug('CNI Daemon returned "%(status)d %(reason)s".',
{'status': resp.status, 'reason': resp.reason})
return resp

57
kuryr_kubernetes/cni/cni_daemon.py
View File

@ -90,20 +90,41 @@ class RequestHandler(BaseHTTPRequestHandler):
def do_POST(self):
# Delegate to add/del network based on path
# and write on wfile
content_length = int(self.headers.get('Content-Length', 0))
body = self.rfile.read(content_length)
self._params = utils.CNIParameters(dict(jsonutils.loads(body)))
try:
content_length = int(self.headers.get('Content-Length', 0))
body = self.rfile.read(content_length)
self._params = utils.CNIParameters(dict(jsonutils.loads(body)))
except Exception:
LOG.warning('Malformed request.')
self.send_error(400, 'Bad Request')
return
if self.path.endswith('/addNetwork'):
resp = self.addNetwork()
try:
resp = self.addNetwork()
except Exception:
LOG.exception('Error when processing addNetwork request. '
'CNI Params: %s', self._params)
self.send_error(500, 'Internal Server Error')
return
self.send_response(201, "Created")
self.send_header("Content-type", 'application/json')
self.end_headers()
self.wfile.write(resp.encode())
elif self.path.endswith('/delNetwork'):
self.delNetwork()
try:
self.delNetwork()
except Exception:
LOG.exception('Error when processing delNetwork request. '
'CNI Params: %s', self._params)
self.send_error(500, 'Internal Server Error')
return
self.send_response(204, "No Content")
else:
self.send_error(405, "Method Not Allowed")
LOG.warning('Unexpected path %s in the call.' % self.path)
self.send_error(404, "Not Found")
def addNetwork(self):
vif = self._plugin.add(self._params)
@ -114,20 +135,16 @@ class RequestHandler(BaseHTTPRequestHandler):
self._plugin.delete(self._params)
def log_message(self, format, *args):
"""Log an arbitrary message.
The client ip address and current date/time are prefixed to every
message.
"""
# Default log_message assumes client_address to log which is not
# passed in unix sockets.
sys.stderr.write("%s - - [%s] %s\n" %
(socket.gethostname(),
self.log_date_time_string(),
format % args))
LOG.debug("%(hostname)s - - [%(time)s] %(msg)s\n" %
{'hostname': socket.gethostname(),
'time': self.log_date_time_string(),
'msg': format % args})
def run_daemon():
config.init(sys.argv[1:])
config.setup_logging()
# TODO(vikasc): Should be done using dynamically loadable OVO types plugin.
objects.register_locally_defined_vifs()
@ -138,11 +155,15 @@ def run_daemon():
except OSError:
if os.path.exists(server_address):
raise
httpd = None
try:
LOG.info('Starting daemon HTTP server on socket %s.', server_address)
httpd = UnixDomainHttpServer(server_address, RequestHandler)
httpd.serve_forever()
except KeyboardInterrupt:
pass
except Exception:
LOG.exception('Failed to start kuryr-daemon.')
httpd.socket.close()
finally:
if httpd:
httpd.socket.close()

21
kuryr_kubernetes/tests/unit/cni/test_api.py
View File

@ -15,6 +15,7 @@
import mock
from six import StringIO
import socket
from oslo_serialization import jsonutils
@ -117,8 +118,28 @@ class TestCNIDaemonizedRunner(test_base.TestCase, TestCNIRunnerMixin):
m_socket)
self.assertEqual(0, result)
def test_run_add_invalid(self, m_getresponse, m_request, m_socket):
m_response = mock.Mock(status=400)
m_response.read = mock.Mock()
result = self._test_run('ADD', 'addNetwork', m_getresponse, m_request,
m_socket)
self.assertEqual(1, result)
m_response.read.assert_not_called()
def test_run_del(self, m_getresponse, m_request, m_socket):
m_getresponse.return_value = mock.Mock(status=204)
result = self._test_run('DEL', 'delNetwork', m_getresponse, m_request,
m_socket)
self.assertEqual(0, result)
def test_run_del_invalid(self, m_getresponse, m_request, m_socket):
m_getresponse.return_value = mock.Mock(status=400)
result = self._test_run('DEL', 'delNetwork', m_getresponse, m_request,
m_socket)
self.assertEqual(1, result)
def test_run_socket_error(self, m_getresponse, m_request, m_socket):
m_request.side_effect = socket.error
result = self._test_run('DEL', 'delNetwork', m_getresponse, m_request,
m_socket)
self.assertEqual(1, result)

28
kuryr_kubernetes/tests/unit/cni/test_cni_daemon.py
View File

@ -95,21 +95,41 @@ class TestRequestHandler(test_base.TestCase):
mock_request.sendall = mock.MagicMock()
handler_class(mock_request, ip_port, self)
def _test_path(self, path, mock_method):
def _test_path(self, path, mock_method, expect_run=True):
mock_method.return_value = fake._fake_vif()
TestRequestHandler.path = path
TestRequestHandler.MockServer(('0.0.0.0', 8888),
cni_daemon.RequestHandler)
self.assertTrue(mock_method.called)
self.assertEqual(TestRequestHandler.params['CNI_ARGS'],
mock_method.call_args[0][0].CNI_ARGS)
self.assertEqual(expect_run, mock_method.called)
if expect_run:
self.assertEqual(TestRequestHandler.params['CNI_ARGS'],
mock_method.call_args[0][0].CNI_ARGS)
@mock.patch('kuryr_kubernetes.cni.cni_daemon.K8sCNIPlugin.add')
def test_addNetwork(self, m_add):
self._test_path('addNetwork', m_add)
@mock.patch('kuryr_kubernetes.cni.cni_daemon.RequestHandler.send_error')
@mock.patch('kuryr_kubernetes.cni.cni_daemon.K8sCNIPlugin.add')
def test_addNetwork_exception(self, m_add, m_send_error):
m_add.side_effect = Exception
self._test_path('addNetwork', m_add)
m_send_error.assert_called_with(500, 'Internal Server Error')
@mock.patch('kuryr_kubernetes.cni.cni_daemon.K8sCNIPlugin.delete')
def test_delNetwork(self, m_delete):
self._test_path('delNetwork', m_delete)
@mock.patch('kuryr_kubernetes.cni.cni_daemon.RequestHandler.send_error')
@mock.patch('kuryr_kubernetes.cni.cni_daemon.K8sCNIPlugin.delete')
def test_delNetwork_exception(self, m_delete, m_send_error):
m_delete.side_effect = Exception
self._test_path('delNetwork', m_delete)
m_send_error.assert_called_with(500, 'Internal Server Error')
@mock.patch('kuryr_kubernetes.cni.cni_daemon.RequestHandler.send_error')
def test_invalid_path(self, m_send_error):
self._test_path('invalid', mock.Mock(), False)
m_send_error.assert_called_with(404, 'Not Found')