Merge lp:~niedbalski/charm-helpers/ip-host-translation into lp:charm-helpers

[Jorge Niedbalski]

- Moved out all the host<->ip translation logic from contrib.openstack to contrib.network module.
- Re-factored the get_host_ip method to also check for the socket.gethostbyname and fallback to the defined 'fallback' parameter if no host is found.
- Added tests for covering the changes.

[lp:~niedbalski/charm-helpers/ip-host-translation]

325. By Jorge Niedbalski on 2015-02-26

Minor lints

326. By Jorge Niedbalski on 2015-02-26

Minor lints

[Billy Olsen (billy-olsen)]

LGTM approve

review: Approve

=== modified file 'charmhelpers/contrib/network/ip.py'
--- charmhelpers/contrib/network/ip.py	2015-01-22 06:06:03 +0000
+++ charmhelpers/contrib/network/ip.py	2015-02-26 23:24:37 +0000
@@ -17,13 +17,16 @@
 import glob
 import re
 import subprocess
+import six
+import socket
 
 from functools import partial
 
 from charmhelpers.core.hookenv import unit_get
 from charmhelpers.fetch import apt_install
 from charmhelpers.core.hookenv import (
-    log
+    log,
+    WARNING,
 )
 
 try:
@@ -365,3 +368,83 @@
             return True
 
     return False
+
+
+def is_ip(address):
+    """
+    Returns True if address is a valid IP address.
+    """
+    try:
+        # Test to see if already an IPv4 address
+        socket.inet_aton(address)
+        return True
+    except socket.error:
+        return False
+
+
+def ns_query(address):
+    try:
+        import dns.resolver
+    except ImportError:
+        apt_install('python-dnspython')
+        import dns.resolver
+
+    if isinstance(address, dns.name.Name):
+        rtype = 'PTR'
+    elif isinstance(address, six.string_types):
+        rtype = 'A'
+    else:
+        return None
+
+    answers = dns.resolver.query(address, rtype)
+    if answers:
+        return str(answers[0])
+    return None
+
+
+def get_host_ip(hostname, fallback=None):
+    """
+    Resolves the IP for a given hostname, or returns
+    the input if it is already an IP.
+    """
+    if is_ip(hostname):
+        return hostname
+
+    ip_addr = ns_query(hostname)
+    if not ip_addr:
+        try:
+            ip_addr = socket.gethostbyname(hostname)
+        except:
+            log("Failed to resolve hostname '%s'" % (hostname),
+                level=WARNING)
+            return fallback
+    return ip_addr
+
+
+def get_hostname(address, fqdn=True):
+    """
+    Resolves hostname for given IP, or returns the input
+    if it is already a hostname.
+    """
+    if is_ip(address):
+        try:
+            import dns.reversename
+        except ImportError:
+            apt_install("python-dnspython")
+            import dns.reversename
+
+        rev = dns.reversename.from_address(address)
+        result = ns_query(rev)
+        if not result:
+            return None
+    else:
+        result = address
+
+    if fqdn:
+        # strip trailing .
+        if result.endswith('.'):
+            return result[:-1]
+        else:
+            return result
+    else:
+        return result.split('.')[0]

=== modified file 'charmhelpers/contrib/openstack/utils.py'
--- charmhelpers/contrib/openstack/utils.py	2015-02-16 21:55:34 +0000
+++ charmhelpers/contrib/openstack/utils.py	2015-02-26 23:24:37 +0000
@@ -23,12 +23,13 @@
 import subprocess
 import json
 import os
-import socket
 import sys
 
 import six
 import yaml
 
+from charmhelpers.contrib.network import ip
+
 from charmhelpers.core.hookenv import (
     config,
     log as juju_log,
@@ -421,77 +422,10 @@
     else:
         zap_disk(block_device)
 
-
-def is_ip(address):
-    """
-    Returns True if address is a valid IP address.
-    """
-    try:
-        # Test to see if already an IPv4 address
-        socket.inet_aton(address)
-        return True
-    except socket.error:
-        return False
-
-
-def ns_query(address):
-    try:
-        import dns.resolver
-    except ImportError:
-        apt_install('python-dnspython')
-        import dns.resolver
-
-    if isinstance(address, dns.name.Name):
-        rtype = 'PTR'
-    elif isinstance(address, six.string_types):
-        rtype = 'A'
-    else:
-        return None
-
-    answers = dns.resolver.query(address, rtype)
-    if answers:
-        return str(answers[0])
-    return None
-
-
-def get_host_ip(hostname):
-    """
-    Resolves the IP for a given hostname, or returns
-    the input if it is already an IP.
-    """
-    if is_ip(hostname):
-        return hostname
-
-    return ns_query(hostname)
-
-
-def get_hostname(address, fqdn=True):
-    """
-    Resolves hostname for given IP, or returns the input
-    if it is already a hostname.
-    """
-    if is_ip(address):
-        try:
-            import dns.reversename
-        except ImportError:
-            apt_install('python-dnspython')
-            import dns.reversename
-
-        rev = dns.reversename.from_address(address)
-        result = ns_query(rev)
-        if not result:
-            return None
-    else:
-        result = address
-
-    if fqdn:
-        # strip trailing .
-        if result.endswith('.'):
-            return result[:-1]
-        else:
-            return result
-    else:
-        return result.split('.')[0]
+is_ip = ip.is_ip
+ns_query = ip.ns_query
+get_host_ip = ip.get_host_ip
+get_hostname = ip.get_hostname
 
 
 def get_matchmaker_map(mm_file='/etc/oslo/matchmaker_ring.json'):

=== modified file 'tests/contrib/network/test_ip.py'
--- tests/contrib/network/test_ip.py	2014-11-25 15:07:02 +0000
+++ tests/contrib/network/test_ip.py	2015-02-26 23:24:37 +0000
@@ -5,8 +5,17 @@
 import netifaces
 
 import charmhelpers.contrib.network.ip as net_ip
-from mock import patch
+from mock import patch, MagicMock
+
 import nose.tools
+import six
+
+if not six.PY3:
+    builtin_open = '__builtin__.open'
+    builtin_import = '__builtin__.__import__'
+else:
+    builtin_open = 'builtins.open'
+    builtin_import = 'builtins.__import__'
 
 DUMMY_ADDRESSES = {
     'lo': {
@@ -82,6 +91,43 @@
 """
 
 
+class FakeAnswer(object):
+    def __init__(self, ip):
+        self.ip = ip
+
+    def __str__(self):
+        return self.ip
+
+
+class FakeResolver(object):
+    def __init__(self, ip):
+        self.ip = ip
+
+    def query(self, hostname, query_type):
+        if self.ip == '':
+            return []
+        else:
+            return [FakeAnswer(self.ip)]
+
+
+class FakeReverse(object):
+    def from_address(self, address):
+        return '156.94.189.91.in-addr.arpa'
+
+
+class FakeDNSName(object):
+    def __init__(self, dnsname):
+        pass
+
+
+class FakeDNS(object):
+    def __init__(self, ip):
+        self.resolver = FakeResolver(ip)
+        self.reversename = FakeReverse()
+        self.name = MagicMock()
+        self.name.Name = FakeDNSName
+
+
 class IPTest(unittest.TestCase):
 
     def mock_ifaddresses(self, iface):
@@ -501,3 +547,134 @@
 
         with nose.tools.assert_raises(Exception):
             net_ip.get_iface_from_addr('1.2.3.4')
+
+    def test_is_ip(self):
+        self.assertTrue(net_ip.is_ip('10.0.0.1'))
+        self.assertFalse(net_ip.is_ip('www.ubuntu.com'))
+
+    @patch('charmhelpers.contrib.network.ip.apt_install')
+    def test_get_host_ip_with_hostname(self, apt_install):
+        fake_dns = FakeDNS('10.0.0.1')
+        with patch(builtin_import, side_effect=[fake_dns]):
+            ip = net_ip.get_host_ip('www.ubuntu.com')
+        self.assertEquals(ip, '10.0.0.1')
+
+    @patch('charmhelpers.contrib.network.ip.ns_query')
+    @patch('charmhelpers.contrib.network.ip.socket.gethostbyname')
+    @patch('charmhelpers.contrib.network.ip.apt_install')
+    def test_get_host_ip_with_hostname_no_dns(self, apt_install, socket,
+                                              ns_query):
+        ns_query.return_value = []
+        fake_dns = FakeDNS(None)
+        socket.return_value = '10.0.0.1'
+        with patch(builtin_import, side_effect=[fake_dns]):
+            ip = net_ip.get_host_ip('www.ubuntu.com')
+        self.assertEquals(ip, '10.0.0.1')
+
+    @patch('charmhelpers.contrib.network.ip.log')
+    @patch('charmhelpers.contrib.network.ip.ns_query')
+    @patch('charmhelpers.contrib.network.ip.socket.gethostbyname')
+    @patch('charmhelpers.contrib.network.ip.apt_install')
+    def test_get_host_ip_with_hostname_fallback(self, apt_install, socket,
+                                                ns_query, *args):
+        ns_query.return_value = []
+        fake_dns = FakeDNS(None)
+
+        def r():
+            raise Exception()
+
+        socket.side_effect = r
+        with patch(builtin_import, side_effect=[fake_dns]):
+            ip = net_ip.get_host_ip('www.ubuntu.com', fallback='127.0.0.1')
+        self.assertEquals(ip, '127.0.0.1')
+
+    @patch('charmhelpers.contrib.network.ip.apt_install')
+    def test_get_host_ip_with_ip(self, apt_install):
+        fake_dns = FakeDNS('5.5.5.5')
+        with patch(builtin_import, side_effect=[fake_dns]):
+            ip = net_ip.get_host_ip('4.2.2.1')
+        self.assertEquals(ip, '4.2.2.1')
+
+    @patch('charmhelpers.contrib.network.ip.apt_install')
+    def test_ns_query_trigger_apt_install(self, apt_install):
+        fake_dns = FakeDNS('5.5.5.5')
+        with patch(builtin_import, side_effect=[ImportError, fake_dns]):
+            nsq = net_ip.ns_query('5.5.5.5')
+            apt_install.assert_called_with('python-dnspython')
+        self.assertEquals(nsq, '5.5.5.5')
+
+    @patch('charmhelpers.contrib.network.ip.apt_install')
+    def test_ns_query_ptr_record(self, apt_install):
+        fake_dns = FakeDNS('127.0.0.1')
+        with patch(builtin_import, side_effect=[fake_dns]):
+            nsq = net_ip.ns_query('127.0.0.1')
+        self.assertEquals(nsq, '127.0.0.1')
+
+    @patch('charmhelpers.contrib.network.ip.apt_install')
+    def test_ns_query_a_record(self, apt_install):
+        fake_dns = FakeDNS('127.0.0.1')
+        fake_dns_name = FakeDNSName('www.somedomain.tld')
+        with patch(builtin_import, side_effect=[fake_dns]):
+            nsq = net_ip.ns_query(fake_dns_name)
+        self.assertEquals(nsq, '127.0.0.1')
+
+    @patch('charmhelpers.contrib.network.ip.apt_install')
+    def test_ns_query_blank_record(self, apt_install):
+        fake_dns = FakeDNS(None)
+        with patch(builtin_import, side_effect=[fake_dns, fake_dns]):
+            nsq = net_ip.ns_query(None)
+        self.assertEquals(nsq, None)
+
+    @patch('charmhelpers.contrib.network.ip.apt_install')
+    def test_ns_query_lookup_fail(self, apt_install):
+        fake_dns = FakeDNS('')
+        with patch(builtin_import, side_effect=[fake_dns, fake_dns]):
+            nsq = net_ip.ns_query('nonexistant')
+        self.assertEquals(nsq, None)
+
+    @patch('charmhelpers.contrib.network.ip.apt_install')
+    def test_get_hostname_with_ip(self, apt_install):
+        fake_dns = FakeDNS('www.ubuntu.com')
+        with patch(builtin_import, side_effect=[fake_dns, fake_dns]):
+            hn = net_ip.get_hostname('4.2.2.1')
+        self.assertEquals(hn, 'www.ubuntu.com')
+
+    @patch('charmhelpers.contrib.network.ip.apt_install')
+    def test_get_hostname_with_ip_not_fqdn(self, apt_install):
+        fake_dns = FakeDNS('packages.ubuntu.com')
+        with patch(builtin_import, side_effect=[fake_dns, fake_dns]):
+            hn = net_ip.get_hostname('4.2.2.1', fqdn=False)
+        self.assertEquals(hn, 'packages')
+
+    @patch('charmhelpers.contrib.network.ip.apt_install')
+    def test_get_hostname_with_hostname(self, apt_install):
+        hn = net_ip.get_hostname('www.ubuntu.com')
+        self.assertEquals(hn, 'www.ubuntu.com')
+
+    @patch('charmhelpers.contrib.network.ip.apt_install')
+    def test_get_hostname_with_hostname_trailingdot(self, apt_install):
+        hn = net_ip.get_hostname('www.ubuntu.com.')
+        self.assertEquals(hn, 'www.ubuntu.com')
+
+    @patch('charmhelpers.contrib.network.ip.apt_install')
+    def test_get_hostname_with_hostname_not_fqdn(self, apt_install):
+        hn = net_ip.get_hostname('packages.ubuntu.com', fqdn=False)
+        self.assertEquals(hn, 'packages')
+
+    @patch('charmhelpers.contrib.network.ip.apt_install')
+    def test_get_hostname_trigger_apt_install(self, apt_install):
+        fake_dns = FakeDNS('www.ubuntu.com')
+        with patch(builtin_import, side_effect=[ImportError, fake_dns,
+                                                fake_dns]):
+            hn = net_ip.get_hostname('4.2.2.1')
+            apt_install.assert_called_with('python-dnspython')
+        self.assertEquals(hn, 'www.ubuntu.com')
+
+    @patch('charmhelpers.contrib.network.ip.ns_query')
+    @patch('charmhelpers.contrib.network.ip.apt_install')
+    def test_get_hostname_lookup_fail(self, apt_install, ns_query):
+        fake_dns = FakeDNS('www.ubuntu.com')
+        ns_query.return_value = []
+        with patch(builtin_import, side_effect=[fake_dns, fake_dns]):
+            hn = net_ip.get_hostname('4.2.2.1')
+        self.assertEquals(hn, None)

=== modified file 'tests/contrib/openstack/test_openstack_utils.py'
--- tests/contrib/openstack/test_openstack_utils.py	2014-12-03 13:27:17 +0000
+++ tests/contrib/openstack/test_openstack_utils.py	2015-02-26 23:24:37 +0000
@@ -595,107 +595,6 @@
         openstack.clean_storage('/dev/vdb')
         zap_disk.assert_called_with('/dev/vdb')
 
-    def test_is_ip(self):
-        self.assertTrue(openstack.is_ip('10.0.0.1'))
-        self.assertFalse(openstack.is_ip('www.ubuntu.com'))
-
-    @patch.object(openstack, 'apt_install')
-    def test_get_host_ip_with_hostname(self, apt_install):
-        fake_dns = FakeDNS('10.0.0.1')
-        with patch(builtin_import, side_effect=[fake_dns]):
-            ip = openstack.get_host_ip('www.ubuntu.com')
-        self.assertEquals(ip, '10.0.0.1')
-
-    @patch.object(openstack, 'apt_install')
-    def test_get_host_ip_with_ip(self, apt_install):
-        fake_dns = FakeDNS('5.5.5.5')
-        with patch(builtin_import, side_effect=[fake_dns]):
-            ip = openstack.get_host_ip('4.2.2.1')
-        self.assertEquals(ip, '4.2.2.1')
-
-    @patch.object(openstack, 'apt_install')
-    def test_ns_query_trigger_apt_install(self, apt_install):
-        fake_dns = FakeDNS('5.5.5.5')
-        with patch(builtin_import, side_effect=[ImportError, fake_dns]):
-            nsq = openstack.ns_query('5.5.5.5')
-            apt_install.assert_called_with('python-dnspython')
-        self.assertEquals(nsq, '5.5.5.5')
-
-    @patch.object(openstack, 'apt_install')
-    def test_ns_query_ptr_record(self, apt_install):
-        fake_dns = FakeDNS('127.0.0.1')
-        with patch(builtin_import, side_effect=[fake_dns]):
-            nsq = openstack.ns_query('127.0.0.1')
-        self.assertEquals(nsq, '127.0.0.1')
-
-    @patch.object(openstack, 'apt_install')
-    def test_ns_query_a_record(self, apt_install):
-        fake_dns = FakeDNS('127.0.0.1')
-        fake_dns_name = FakeDNSName('www.somedomain.tld')
-        with patch(builtin_import, side_effect=[fake_dns]):
-            nsq = openstack.ns_query(fake_dns_name)
-        self.assertEquals(nsq, '127.0.0.1')
-
-    @patch.object(openstack, 'apt_install')
-    def test_ns_query_blank_record(self, apt_install):
-        fake_dns = FakeDNS(None)
-        with patch(builtin_import, side_effect=[fake_dns, fake_dns]):
-            nsq = openstack.ns_query(None)
-        self.assertEquals(nsq, None)
-
-    @patch.object(openstack, 'apt_install')
-    def test_ns_query_lookup_fail(self, apt_install):
-        fake_dns = FakeDNS('')
-        with patch(builtin_import, side_effect=[fake_dns, fake_dns]):
-            nsq = openstack.ns_query('nonexistant')
-        self.assertEquals(nsq, None)
-
-    @patch.object(openstack, 'apt_install')
-    def test_get_hostname_with_ip(self, apt_install):
-        fake_dns = FakeDNS('www.ubuntu.com')
-        with patch(builtin_import, side_effect=[fake_dns, fake_dns]):
-            hn = openstack.get_hostname('4.2.2.1')
-        self.assertEquals(hn, 'www.ubuntu.com')
-
-    @patch.object(openstack, 'apt_install')
-    def test_get_hostname_with_ip_not_fqdn(self, apt_install):
-        fake_dns = FakeDNS('packages.ubuntu.com')
-        with patch(builtin_import, side_effect=[fake_dns, fake_dns]):
-            hn = openstack.get_hostname('4.2.2.1', fqdn=False)
-        self.assertEquals(hn, 'packages')
-
-    @patch.object(openstack, 'apt_install')
-    def test_get_hostname_with_hostname(self, apt_install):
-        hn = openstack.get_hostname('www.ubuntu.com')
-        self.assertEquals(hn, 'www.ubuntu.com')
-
-    @patch.object(openstack, 'apt_install')
-    def test_get_hostname_with_hostname_trailingdot(self, apt_install):
-        hn = openstack.get_hostname('www.ubuntu.com.')
-        self.assertEquals(hn, 'www.ubuntu.com')
-
-    @patch.object(openstack, 'apt_install')
-    def test_get_hostname_with_hostname_not_fqdn(self, apt_install):
-        hn = openstack.get_hostname('packages.ubuntu.com', fqdn=False)
-        self.assertEquals(hn, 'packages')
-
-    @patch.object(openstack, 'apt_install')
-    def test_get_hostname_trigger_apt_install(self, apt_install):
-        fake_dns = FakeDNS('www.ubuntu.com')
-        with patch(builtin_import, side_effect=[ImportError, fake_dns, fake_dns]):
-            hn = openstack.get_hostname('4.2.2.1')
-            apt_install.assert_called_with('python-dnspython')
-        self.assertEquals(hn, 'www.ubuntu.com')
-
-    @patch.object(openstack, 'ns_query')
-    @patch.object(openstack, 'apt_install')
-    def test_get_hostname_lookup_fail(self, apt_install, ns_query):
-        fake_dns = FakeDNS('www.ubuntu.com')
-        ns_query.return_value = []
-        with patch(builtin_import, side_effect=[fake_dns, fake_dns]):
-            hn = openstack.get_hostname('4.2.2.1')
-        self.assertEquals(hn, None)
-
     @patch('os.path.isfile')
     @patch(builtin_open)
     def test_get_matchmaker_map(self, _open, _isfile):
@@ -846,7 +745,8 @@
 
         openstack._git_clone_and_install_single(repo, branch)
         mkdir.assert_called_with(dest_parent_dir)
-        install_remote.assert_called_with(repo, dest=dest_parent_dir, branch=branch)
+        install_remote.assert_called_with(repo, dest=dest_parent_dir,
+                                          branch=branch)
         assert not _git_update_reqs.called
         pip_install.assert_called_with(dest_dir)
 
@@ -871,7 +771,8 @@
 
         openstack._git_clone_and_install_single(repo, branch, True)
         mkdir.assert_called_with(dest_parent_dir)
-        install_remote.assert_called_with(repo, dest=dest_parent_dir, branch=branch)
+        install_remote.assert_called_with(repo, dest=dest_parent_dir,
+                                          branch=branch)
         _git_update_reqs.assert_called_with(dest_dir, reqs_dir)
         pip_install.assert_called_with(dest_dir)