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 <adking@redhat.com>
(cherry picked from commit b2f133fd994cc63f1fa55320aac987e760e43160)

diff --git a/src/cephadm/cephadm.py b/src/cephadm/cephadm.py
index 303408757c4a61eddc56689d1e0641ebfe84e39f..8133a8db81bd49a38c415688ca667c3b58403f7d 100755 (executable)
--- 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 911ae564656a0a9501e66cc12b0142a9a4048a76..f7d5fab3044eaaea2c879543df7abcfd5b159f02 100644 (file)
--- 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()