Use AF_UNSPEC for name resolution (#389)

* Use AF_UNSPEC for name resolution

This reverts most of 1ad97fb14c0c3c57395ca525932f95a830e51a88, but
keeps tests which still have general applicability.

The reason the original change was made was to try and work around a
bug[1] in the eventlet library.  Eventlet monkey-patches the
socket.getaddrinfo function and replaces it with its own async,
eventlet-aware implementation.  The reason name resolution was broken
in the first place is because eventlet was consulting DNS first, and
then if that failed, falling back to /etc/hosts, which is just flat
out incorrect behavior.

It's important to note that this was *only* when running py-amqp under
eventlet, and *only* for specific versions of eventlet that have long
been fixed.  So this workaround is not even needed anymore.

With "normal" (non-eventlet) use, socket.getaddrinfo instead calls
into the glibc getaddrinfo implementation, which ultimately uses
libnss to resolve hostnames.

However, there is an issue with the original workaround when using the
default (glibc) getaddrinfo.  The workaround (current) implementation
explicitly forces resolution to use AF_INET (IPv4) and then only if
that does not succeed, it in turn will try with AF_INET6 (IPv6).  This
generally works well for IPv4-only hosts, but can be unnecessarily
slow for dual-stack IPv4/IPv6 hosts.

Consider the following:

- We want to connect to example.org

- The /etc/hosts file contains an IPv6 entry:

    example.org f00d::1

- The /etc/nsswitch.conf file contains typical (simplified) hosts
  config:

    hosts: files dns

In this case, the current code will involve nss iterating through the
modules:

- files (with AF_INET): fails, because there is no IPv4 address in
  /etc/hosts

- dns (with AF_INET): may or may not succeed per-site, depending on
  how DNS is configured.  If DNS is slow/misconfigured, this may incur
  a delay and block for a significant amount of time.

- files (with AF_INET6): succeeds, and getaddrinfo returns f00d::1.

Now in the same scenario as before, with this fix which reverts back
to using AF_UNSPEC instead:

- files (with AF_UNSPEC) succeeds, and getaddrinfo returns f00d::1.

There is no need to involve DNS at all.  Even a well-configured,
quick-to-respond DNS server is going to be many orders of magnitude
slower than consulting with /etc/hosts which libnss keeps cached in
memory.

[1] https://bugs.launchpad.net/neutron/+bug/1696094/comments/22

* tests: ensure getaddrinfo is called with AF_UNSPEC

1 files changed, 3 insertions, 37 deletions

diff --git a/t/unit/test_transport.py b/t/unit/test_transport.py
index 348b6c2..e9c7114 100644
--- a/t/unit/test_transport.py
+++ b/t/unit/test_transport.py
@@ -520,54 +520,20 @@ class test_AbstractTransport_connect:
                               side_effect=(socket.error, None)):
                 self.t.connect()
 
-    def test_connect_short_curcuit_on_INET_succeed(self):
+    def test_connect_calls_getaddrinfo_with_af_unspec(self):
         with patch('socket.socket', return_value=MockSocket()), \
-            patch('socket.getaddrinfo',
-                  side_effect=[
-                      [(socket.AF_INET, 1, socket.IPPROTO_TCP,
-                        '', ('127.0.0.1', 5672))],
-                      [(socket.AF_INET6, 1, socket.IPPROTO_TCP,
-                        '', ('::1', 5672))]
-                  ]) as getaddrinfo:
+            patch('socket.getaddrinfo') as getaddrinfo:
             self.t.sock = Mock()
             self.t.close()
             self.t.connect()
             getaddrinfo.assert_called_with(
-                'localhost', 5672, socket.AF_INET, ANY, ANY)
-
-    def test_connect_short_curcuit_on_INET_fails(self):
-        with patch('socket.socket', return_value=MockSocket()) as sock_mock, \
-            patch('socket.getaddrinfo',
-                  side_effect=[
-                      [(socket.AF_INET, 1, socket.IPPROTO_TCP,
-                        '', ('127.0.0.1', 5672))],
-                      [(socket.AF_INET6, 1, socket.IPPROTO_TCP,
-                        '', ('::1', 5672))]
-                  ]) as getaddrinfo:
-            self.t.sock = Mock()
-            self.t.close()
-            with patch.object(sock_mock.return_value, 'connect',
-                              side_effect=(socket.error, None)):
-                self.t.connect()
-            getaddrinfo.assert_has_calls(
-                [call('localhost', 5672, addr_type, ANY, ANY)
-                 for addr_type in (socket.AF_INET, socket.AF_INET6)])
+                'localhost', 5672, socket.AF_UNSPEC, ANY, ANY)
 
     def test_connect_getaddrinfo_raises_gaierror(self):
         with patch('socket.getaddrinfo', side_effect=socket.gaierror):
             with pytest.raises(socket.error):
                 self.t.connect()
 
-    def test_connect_getaddrinfo_raises_gaierror_once_recovers(self):
-        with patch('socket.socket', return_value=MockSocket()), \
-            patch('socket.getaddrinfo',
-                  side_effect=[
-                      socket.gaierror,
-                      [(socket.AF_INET6, 1, socket.IPPROTO_TCP,
-                        '', ('::1', 5672))]
-                  ]):
-            self.t.connect()
-
     def test_connect_survives_not_implemented_set_cloexec(self):
         with patch('socket.socket', return_value=MockSocket()), \
             patch('socket.getaddrinfo',