Merge lp:~hopem/charm-helpers/fix-lp1425999 into lp:charm-helpers

[Edward Hope-Morley]

[Jorge Niedbalski (niedbalski)]

Hello Edward.

I found out that there is a few other places on which similar host<->ip logic
is being used. I just re-factored/moved this to the contrib.network.ip module, which
in my opinion is a better place for having this.

The proposal: https://code.launchpad.net/~niedbalski/charm-helpers/ip-host-translation/+merge/251189

I would suggest to review that and integrate your code according to the inline comment.

review: Needs Fixing

[lp:~hopem/charm-helpers/fix-lp1425999]

325. By Edward Hope-Morley on 2015-02-27

sync trunk

326. By Edward Hope-Morley on 2015-02-27

switch to ip.get_host_ip()

[Edward Hope-Morley (hopem)]

@jorge Fixed.

[Jorge Niedbalski (niedbalski)]

Hello Ed,

I fixed a few lint errors:

charmhelpers/contrib/database/mysql.py:3:1: F401 'socket' imported but unused
charmhelpers/contrib/database/mysql.py:17:1: F401 'WARNING' imported but unused
tests/contrib/database/test_mysql.py:72:66: W291 trailing whitespace

Merged.

[Jorge Niedbalski (niedbalski)]

review: Approve

=== modified file 'charmhelpers/contrib/database/mysql.py'
--- charmhelpers/contrib/database/mysql.py	2015-02-26 16:05:44 +0000
+++ charmhelpers/contrib/database/mysql.py	2015-02-27 10:03:09 +0000
@@ -15,14 +15,15 @@
     write_file
 )
 from charmhelpers.core.hookenv import (
+    config as config_get,
     relation_get,
     related_units,
     unit_get,
     log,
     DEBUG,
     INFO,
+    WARNING,
 )
-from charmhelpers.core.hookenv import config as config_get
 from charmhelpers.fetch import (
     apt_install,
     apt_update,
@@ -32,6 +33,7 @@
     peer_store,
     peer_retrieve,
 )
+from charmhelpers.contrib.network.ip import get_host_ip
 
 try:
     import MySQLdb
@@ -220,6 +222,18 @@
         """Retrieve or generate mysql root password for service units."""
         return self.get_mysql_password(username=None, password=password)
 
+    def normalize_address(self, hostname):
+        """Ensure that address returned is an IP address (i.e. not fqdn)"""
+        if config_get('prefer-ipv6'):
+            # TODO: add support for ipv6 dns
+            return hostname
+
+        if hostname != unit_get('private-address'):
+            return get_host_ip(hostname, fallback=hostname)
+
+        # Otherwise assume localhost
+        return '127.0.0.1'
+
     def get_allowed_units(self, database, username, relation_id=None):
         """Get list of units with access grants for database with username.
 
@@ -247,6 +261,7 @@
 
             if hosts:
                 for host in hosts:
+                    host = self.normalize_address(host)
                     if self.grant_exists(database, username, host):
                         log("Grant exists for host '%s' on db '%s'" %
                             (host, database), level=DEBUG)
@@ -262,21 +277,11 @@
 
     def configure_db(self, hostname, database, username, admin=False):
         """Configure access to database for username from hostname."""
-        if config_get('prefer-ipv6'):
-            remote_ip = hostname
-        elif hostname != unit_get('private-address'):
-            try:
-                remote_ip = socket.gethostbyname(hostname)
-            except Exception:
-                # socket.gethostbyname doesn't support ipv6
-                remote_ip = hostname
-        else:
-            remote_ip = '127.0.0.1'
-
         self.connect(password=self.get_mysql_root_password())
         if not self.database_exists(database):
             self.create_database(database)
 
+        remote_ip = self.normalize_address(hostname)
         password = self.get_mysql_password(username)
         if not self.grant_exists(database, username, remote_ip):
             if not admin:

=== modified file 'tests/contrib/database/test_mysql.py'
--- tests/contrib/database/test_mysql.py	2015-02-25 19:15:53 +0000
+++ tests/contrib/database/test_mysql.py	2015-02-27 10:03:09 +0000
@@ -11,6 +11,7 @@
     def setUp(self):
         super(MysqlTests, self).setUp()
 
+    @mock.patch.object(mysql.MySQLHelper, 'normalize_address')
     @mock.patch.object(mysql.MySQLHelper, 'get_mysql_password')
     @mock.patch.object(mysql.MySQLHelper, 'grant_exists')
     @mock.patch.object(mysql, 'relation_get')
@@ -19,7 +20,11 @@
     def test_get_allowed_units(self, mock_log, mock_related_units,
                                mock_relation_get,
                                mock_grant_exists,
-                               mock_get_password):
+                               mock_get_password,
+                               mock_normalize_address):
+
+        # echo
+        mock_normalize_address.side_effect = lambda addr: addr
 
         def mock_rel_get(unit, rid):
             if unit == 'unit/0':
@@ -54,6 +59,44 @@
         helper.grant_exists.assert_has_calls(calls)
         self.assertEqual(units, set(['unit/0', 'unit/1', 'unit/2']))
 
+    @mock.patch('charmhelpers.contrib.network.ip.socket')
+    @mock.patch.object(mysql, 'unit_get')
+    @mock.patch.object(mysql, 'config_get')
+    @mock.patch.object(mysql, 'log')
+    def test_normalize_address(self, mock_log, mock_config_get, mock_unit_get,
+                               mock_socket):
+        helper = mysql.MySQLHelper('foo', 'bar', host='hostA')
+        # prefer-ipv6
+        mock_config_get.return_value = False
+        # echo
+        mock_socket.gethostbyname.side_effect = lambda addr: addr 
+
+        mock_unit_get.return_value = '10.0.0.1'
+        out = helper.normalize_address('10.0.0.1')
+        self.assertEqual('127.0.0.1', out)
+        mock_config_get.assert_called_with('prefer-ipv6')
+
+        mock_unit_get.return_value = '10.0.0.1'
+        out = helper.normalize_address('10.0.0.2')
+        self.assertEqual('10.0.0.2', out)
+        mock_config_get.assert_called_with('prefer-ipv6')
+
+        out = helper.normalize_address('2001:db8:1::1')
+        self.assertEqual('2001:db8:1::1', out)
+        mock_config_get.assert_called_with('prefer-ipv6')
+
+        mock_socket.gethostbyname.side_effect = Exception
+        out = helper.normalize_address('unresolvable')
+        self.assertEqual('unresolvable', out)
+        mock_config_get.assert_called_with('prefer-ipv6')
+
+        # prefer-ipv6
+        mock_config_get.return_value = True
+        mock_socket.gethostbyname.side_effect = 'other'
+        out = helper.normalize_address('unresolvable')
+        self.assertEqual('unresolvable', out)
+        mock_config_get.assert_called_with('prefer-ipv6')
+
 
 class PerconaTests(unittest.TestCase):
 
@@ -62,7 +105,8 @@
 
     @mock.patch.object(mysql.PerconaClusterHelper, 'get_mem_total')
     @mock.patch.object(mysql, 'config_get')
-    def test_parse_config_innodb_pool_fixed(self, config, mem):
+    @mock.patch.object(mysql, 'log')
+    def test_parse_config_innodb_pool_fixed(self, log, config, mem):
         mem.return_value = "100G"
         config.return_value = {
             'innodb-buffer-pool-size': "50%",
@@ -76,7 +120,8 @@
 
     @mock.patch.object(mysql.PerconaClusterHelper, 'get_mem_total')
     @mock.patch.object(mysql, 'config_get')
-    def test_parse_config_innodb_pool_not_set(self, config, mem):
+    @mock.patch.object(mysql, 'log')
+    def test_parse_config_innodb_pool_not_set(self, mog, config, mem):
         mem.return_value = "100G"
         config.return_value = {
             'innodb-buffer-pool-size': '',