From 5e01dfefcdb8cd43b77977544171b10fac923db6 Mon Sep 17 00:00:00 2001 From: Adam King Date: Fri, 18 Aug 2023 12:32:38 -0400 Subject: [PATCH] cephadm: have attempt_bind raise up OSErrors Before it was always converting the OSError to our self-defined "Error" class. This causes an issue with the port_in_use function that has special handling for OSError when the errno is EADDRNOTAVAIL or EAFNOSUPPORT. Since the error being raised was no longer an OSError it wasn't being caught and checked properly in port_in_use. This has the additional property of being necessary to check port availability for haproxy on its VIP. If we fail deployment when EADDRNOTAVAIL is raised, it becomes difficult to deploy the ingress service. If we deploy haproxy first it fails because the VIP isn't available yet (since keepalive isn't up) and it fails saying the port it wants to bind to is unavailable (specifically EADDRNOTAVAIL). If we try to deploy keepalive first it fails because it needs to know the location of the haproxy daemons in order to build its config file. This has worked in the past by just having the haproxy fail to bind at first and then fix itself once the keepalive daemon is deployed. That no longer works if the haproxy daemon fails to deploy because cephadm is reporting the port it needs is unavailable. Since EADDRNOTAVAIL when deploying haproxy likely means the VIP is not up rather than something else is taking up the port it needs, fixing the handling of this allows ingress deployment to work while also allowing multiple haproxy daemons on the same host to use the same frontend port bound to different VIPs. Signed-off-by: Adam King (cherry picked from commit b2f133fd994cc63f1fa55320aac987e760e43160) --- src/cephadm/cephadm.py | 2 +- src/cephadm/tests/test_cephadm.py | 36 +++++++++++++++++++++++++++---- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/cephadm/cephadm.py b/src/cephadm/cephadm.py index 303408757c4a6..8133a8db81bd4 100755 --- a/src/cephadm/cephadm.py +++ b/src/cephadm/cephadm.py @@ -1586,7 +1586,7 @@ def attempt_bind(ctx, s, address, port): logger.warning(msg) raise PortOccupiedError(msg) else: - raise Error(e) + raise e except Exception as e: raise Error(e) finally: diff --git a/src/cephadm/tests/test_cephadm.py b/src/cephadm/tests/test_cephadm.py index 911ae564656a0..f7d5fab3044ea 100644 --- a/src/cephadm/tests/test_cephadm.py +++ b/src/cephadm/tests/test_cephadm.py @@ -58,8 +58,8 @@ class TestCephAdm(object): for side_effect, expected_exception in ( (os_error(errno.EADDRINUSE), _cephadm.PortOccupiedError), - (os_error(errno.EAFNOSUPPORT), _cephadm.Error), - (os_error(errno.EADDRNOTAVAIL), _cephadm.Error), + (os_error(errno.EAFNOSUPPORT), OSError), + (os_error(errno.EADDRNOTAVAIL), OSError), (None, None), ): _socket = mock.Mock() @@ -92,6 +92,34 @@ class TestCephAdm(object): _attempt_bind.side_effect = os_error assert _cephadm.port_in_use(empty_ctx, _cephadm.EndPoint('0.0.0.0', 9100)) == False + @mock.patch('cephadm.socket.socket.bind') + @mock.patch('cephadm.logger') + def test_port_in_use_special_cases(self, _logger, _bind): + # port_in_use has special handling for + # EAFNOSUPPORT and EADDRNOTAVAIL errno OSErrors. + # If we get those specific errors when attempting + # to bind to the ip:port we should not say the + # port is in use + + def os_error(errno): + _os_error = OSError() + _os_error.errno = errno + return _os_error + + _bind.side_effect = os_error(errno.EADDRNOTAVAIL) + in_use = _cephadm.port_in_use(None, _cephadm.EndPoint('1.2.3.4', 10000)) + assert in_use == False + + _bind.side_effect = os_error(errno.EAFNOSUPPORT) + in_use = _cephadm.port_in_use(None, _cephadm.EndPoint('1.2.3.4', 10000)) + assert in_use == False + + # this time, have it raise the actual port taken error + # so it should report the port is in use + _bind.side_effect = os_error(errno.EADDRINUSE) + in_use = _cephadm.port_in_use(None, _cephadm.EndPoint('1.2.3.4', 10000)) + assert in_use == True + @mock.patch('cephadm.attempt_bind') @mock.patch('cephadm.logger') def test_port_in_use_with_specific_ips(self, _logger, _attempt_bind): @@ -146,8 +174,8 @@ class TestCephAdm(object): ): for side_effect, expected_exception in ( (os_error(errno.EADDRINUSE), _cephadm.PortOccupiedError), - (os_error(errno.EADDRNOTAVAIL), _cephadm.Error), - (os_error(errno.EAFNOSUPPORT), _cephadm.Error), + (os_error(errno.EADDRNOTAVAIL), OSError), + (os_error(errno.EAFNOSUPPORT), OSError), (None, None), ): mock_socket_obj = mock.Mock() -- 2.39.5