From 66f8666e4e85d1b36216e70646e8febc4c44abc1 Mon Sep 17 00:00:00 2001 From: Patrick Seidensal Date: Thu, 11 Feb 2021 16:22:06 +0100 Subject: [PATCH] cephadm: fix port_in_use when IPv6 is disabled Do not return "port is in use" when the protocol family tested is not supported (due to being deactivated). Fixes: https://tracker.ceph.com/issues/49273 Conflicts: src/cephadm/cephadm src/cephadm/tests/test_cephadm.py Signed-off-by: Patrick Seidensal (cherry picked from commit a0ffcec4af7a6f464439023d3dd0ccb62c8a856c) --- src/cephadm/cephadm | 46 +++++++++------ src/cephadm/tests/test_cephadm.py | 97 ++++++++++++++++++++++++++++++- 2 files changed, 122 insertions(+), 21 deletions(-) diff --git a/src/cephadm/cephadm b/src/cephadm/cephadm index 6eb2c2b5b1699..8e99357557d93 100755 --- a/src/cephadm/cephadm +++ b/src/cephadm/cephadm @@ -712,6 +712,9 @@ def get_supported_daemons(): ################################## +class PortOccupiedError(Error): + pass + def attempt_bind(s, address, port): # type: (socket.socket, str, int) -> None @@ -719,12 +722,12 @@ def attempt_bind(s, address, port): s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) s.bind((address, port)) except (socket.error, OSError) as e: # py2 and py3 - msg = 'Cannot bind to IP %s port %d: %s' % (address, port, e) - logger.warning(msg) if e.errno == errno.EADDRINUSE: - raise OSError(msg) - elif e.errno == errno.EADDRNOTAVAIL: - pass + msg = 'Cannot bind to IP %s port %d: %s' % (address, port, e) + logger.warning(msg) + raise PortOccupiedError(msg) + else: + raise e finally: s.close() @@ -733,16 +736,26 @@ def port_in_use(port_num): # type: (int) -> bool """Detect whether a port is in use on the local machine - IPv4 and IPv6""" logger.info('Verifying port %d ...' % port_num) - try: - s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - attempt_bind(s, '0.0.0.0', port_num) - - s = socket.socket(socket.AF_INET6, socket.SOCK_STREAM) - attempt_bind(s, '::', port_num) - except OSError: - return True - else: + def _port_in_use(af, address): + # type: (socket.AddressFamily, str) -> bool + try: + s = socket.socket(af, socket.SOCK_STREAM) + attempt_bind(s, address, port_num) + except PortOccupiedError: + return True + except OSError as e: + if e.errno in (errno.EAFNOSUPPORT, errno.EADDRNOTAVAIL): + # Ignore EAFNOSUPPORT and EADDRNOTAVAIL as two interfaces are + # being tested here and one might be intentionally be disabled. + # In that case no error should be raised. + return False + else: + raise e return False + return any(_port_in_use(af, address) for af, address in ( + (socket.AF_INET, '0.0.0.0'), + (socket.AF_INET6, '::') + )) def check_ip_port(ip, port): @@ -754,10 +767,7 @@ def check_ip_port(ip, port): ip = unwrap_ipv6(ip) else: s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - try: - attempt_bind(s, ip, port) - except OSError as e: - raise Error(e) + attempt_bind(s, ip, port) ################################## diff --git a/src/cephadm/tests/test_cephadm.py b/src/cephadm/tests/test_cephadm.py index 2df7d559aebb7..e0d51f2c45e6d 100644 --- a/src/cephadm/tests/test_cephadm.py +++ b/src/cephadm/tests/test_cephadm.py @@ -1,9 +1,9 @@ # type: ignore import mock -from mock import patch -import os -import sys +from mock import patch, call import unittest +import errno +import socket import pytest @@ -22,6 +22,97 @@ class TestCephAdm(object): r = cd.get_unit_file('9b9d7609-f4d5-4aba-94c8-effa764d96c9') assert 'Requires=docker.service' not in r + def test_attempt_bind(self): + cd.logger = mock.Mock() + address = None + port = 0 + + def os_error(errno): + _os_error = OSError() + _os_error.errno = errno + return _os_error + + for side_effect, expected_exception in ( + (os_error(errno.EADDRINUSE), cd.PortOccupiedError), + (os_error(errno.EAFNOSUPPORT), OSError), + (os_error(errno.EADDRNOTAVAIL), OSError), + (None, None), + ): + _socket = mock.Mock() + _socket.bind.side_effect = side_effect + try: + cd.attempt_bind(_socket, address, port) + except Exception as e: + assert isinstance(e, expected_exception) + else: + if expected_exception is not None: + assert False, '{} should not be None'.format(expected_exception) + + @mock.patch('cephadm.attempt_bind') + def test_port_in_use(self, attempt_bind): + + assert cd.port_in_use(9100) == False + + attempt_bind.side_effect = cd.PortOccupiedError('msg') + assert cd.port_in_use(9100) == True + + os_error = OSError() + os_error.errno = errno.EADDRNOTAVAIL + attempt_bind.side_effect = os_error + assert cd.port_in_use(9100) == False + + os_error = OSError() + os_error.errno = errno.EAFNOSUPPORT + attempt_bind.side_effect = os_error + assert cd.port_in_use(9100) == False + + @mock.patch('socket.socket') + @mock.patch('cephadm.args') + def test_check_ip_port_success(self, args, _socket): + args.skip_ping_check = False + + for address, address_family in ( + ('0.0.0.0', socket.AF_INET), + ('::', socket.AF_INET6), + ): + try: + cd.check_ip_port(address, 9100) + except: + assert False + else: + assert _socket.call_args == call(address_family, socket.SOCK_STREAM) + + @mock.patch('socket.socket') + @mock.patch('cephadm.args') + def test_check_ip_port_failure(self, args, _socket): + args.skip_ping_check = False + + def os_error(errno): + _os_error = OSError() + _os_error.errno = errno + return _os_error + + for address, address_family in ( + ('0.0.0.0', socket.AF_INET), + ('::', socket.AF_INET6), + ): + for side_effect, expected_exception in ( + (os_error(errno.EADDRINUSE), cd.PortOccupiedError), + (os_error(errno.EADDRNOTAVAIL), OSError), + (os_error(errno.EAFNOSUPPORT), OSError), + (None, None), + ): + mock_socket_obj = mock.Mock() + mock_socket_obj.bind.side_effect = side_effect + _socket.return_value = mock_socket_obj + try: + cd.check_ip_port(address, 9100) + except Exception as e: + assert isinstance(e, expected_exception) + else: + assert side_effect is None + + def test_is_not_fsid(self): assert not cd.is_fsid('no-uuid') -- 2.39.5