Fix support for network-spaces

Fix misc issues with use of Juju 2.0 network spaces:

 - Ensure that wsrep address for local unit is correctly
   set using the binding for the cluster peer relation.

 - Correctly set the DB access hostname for db and db-admin
   relation types.

This includes a little refactoring to support reuse within
the charm.

Closes-Bug: 1657305

Change-Id: Id1a800e2ada6fd196422b003fd8e251cab5ad724

hooks/percona_hooks.py

@@ -77,7 +77,7 @@ from charmhelpers.contrib.openstack.ha.utils import (
 from percona_utils import (
     determine_packages,
     setup_percona_repo,
-    get_host_ip,
+    resolve_hostname_to_ip,
     get_cluster_hosts,
     configure_sstuser,
     configure_mysql_root_password,
@@ -96,6 +96,7 @@ from percona_utils import (
     resolve_cnf_file,
     create_binlogs_directory,
     bootstrap_pxc,
+    get_cluster_host_ip,
 )
 
 
@@ -138,7 +139,7 @@ def render_config(clustered=False, hosts=None):
 
     context = {
         'cluster_name': 'juju_cluster',
-        'private_address': get_host_ip(),
+        'private_address': get_cluster_host_ip(),
         'clustered': clustered,
         'cluster_hosts': ",".join(hosts),
         'sst_method': config('sst-method'),
@@ -328,17 +329,7 @@ def cluster_joined():
         relation_settings = {'private-address': addr,
                              'hostname': socket.gethostname()}
 
-    cluster_network = config('cluster-network')
+    relation_settings['cluster-address'] = get_cluster_host_ip()
-    if cluster_network:
-        cluster_addr = get_address_in_network(cluster_network, fatal=True)
-        relation_settings['cluster-address'] = cluster_addr
-    else:
-        try:
-            cluster_addr = network_get_primary_address('cluster')
-            relation_settings['cluster-address'] = cluster_addr
-        except NotImplementedError:
-            # NOTE(jamespage): skip - fallback to previous behaviour
-            pass
 
     log("Setting cluster relation: '%s'" % (relation_settings),
         level=INFO)
@@ -383,22 +374,17 @@ def db_changed(relation_id=None, unit=None, admin=None):
         relation_clear(relation_id)
         return
 
-    if is_clustered():
-        db_host = config('vip')
-    else:
-        if config('prefer-ipv6'):
-            db_host = get_ipv6_addr(exc_list=[config('vip')])[0]
-        else:
-            db_host = unit_get('private-address')
-
     if admin not in [True, False]:
         admin = relation_type() == 'db-admin'
+
     db_name, _ = remote_unit().split("/")
     username = db_name
     db_helper = get_db_helper()
     addr = relation_get('private-address', unit=unit, rid=relation_id)
     password = db_helper.configure_db(addr, db_name, username, admin=admin)
 
+    db_host = get_db_host(addr, interface=relation_type())
+
     relation_set(relation_id=relation_id,
                  relation_settings={
                      'user': username,
@@ -409,7 +395,7 @@ def db_changed(relation_id=None, unit=None, admin=None):
 
 
 def get_db_host(client_hostname, interface='shared-db'):
-    """Get address of local database host.
+    """Get address of local database host for use by db clients
 
     If an access-network has been configured, expect selected address to be
     on that network. If none can be found, revert to primary address.
@@ -417,16 +403,24 @@ def get_db_host(client_hostname, interface='shared-db'):
     If network spaces are supported (Juju >= 2.0), use network-get to
     retrieve the network binding for the interface.
 
+    If DNSHA is set pass os-access-hostname
+
     If vip(s) are configured, chooses first available.
+
+    @param client_hostname: hostname of client side relation setting hostname.
+                            Only used if access-network is configured
+    @param interface: Network space binding to check.
+                      Usually the relationship name.
+    @returns IP for use with db clients
     """
     vips = config('vip').split() if config('vip') else []
     dns_ha = config('dns-ha')
     access_network = config('access-network')
-    client_ip = get_host_ip(client_hostname)
     if is_clustered() and dns_ha:
         log("Using DNS HA hostname: {}".format(config('os-access-hostname')))
         return config('os-access-hostname')
     elif access_network:
+        client_ip = resolve_hostname_to_ip(client_hostname)
         if is_address_in_network(access_network, client_ip):
             if is_clustered():
                 for vip in vips:
@@ -463,6 +457,7 @@ def get_db_host(client_hostname, interface='shared-db'):
     if config('prefer-ipv6'):
         return get_ipv6_addr(exc_list=vips)[0]
 
+    # Last resort
     return unit_get('private-address')
 
 
@@ -523,7 +518,7 @@ def shared_db_changed(relation_id=None, unit=None):
         database = settings['database']
         username = settings['username']
 
-        normalized_address = get_host_ip(hostname)
+        normalized_address = resolve_hostname_to_ip(hostname)
         if access_network and not is_address_in_network(access_network,
                                                         normalized_address):
             # NOTE: for configurations using access-network, only setup
@@ -583,7 +578,7 @@ def shared_db_changed(relation_id=None, unit=None):
                 hostname = databases[db]['hostname']
                 username = databases[db]['username']
 
-                normalized_address = get_host_ip(hostname)
+                normalized_address = resolve_hostname_to_ip(hostname)
                 if (access_network and
                         not is_address_in_network(access_network,
                                                   normalized_address)):

hooks/percona_utils.py

@@ -102,7 +102,12 @@ def setup_percona_repo():
     subprocess.check_call(['apt-key', 'add', KEY])
 
 
-def get_host_ip(hostname=None):
+def resolve_hostname_to_ip(hostname):
+    """Resolve hostname to IP
+
+    @param hostname: hostname to be resolved
+    @returns IP address
+    """
     try:
         import dns.resolver
     except ImportError:
@@ -110,12 +115,6 @@ def get_host_ip(hostname=None):
                     fatal=True)
         import dns.resolver
 
-    if config('prefer-ipv6'):
-        # Ensure we have a valid ipv6 address configured
-        get_ipv6_addr(exc_list=[config('vip')], fatal=True)[0]
-        return socket.gethostname()
-
-    hostname = hostname or unit_get('private-address')
     try:
         # Test to see if already an IPv4 address
         socket.inet_aton(hostname)
@@ -154,23 +153,15 @@ def is_sufficient_peers():
 def get_cluster_hosts():
     hosts_map = {}
 
-    if config('cluster-network'):
+    local_cluster_address = get_cluster_host_ip()
-        hostname = get_address_in_network(config('cluster-network'),
-                                          fatal=True)
-    else:
-        try:
-            hostname = network_get_primary_address('cluster')
-        except NotImplementedError:
-            # NOTE(jamespage): skip - fallback to previous behaviour
-            hostname = get_host_ip()
 
     # We need to add this localhost dns name to /etc/hosts along with peer
     # hosts to ensure percona gets consistently resolved addresses.
     if config('prefer-ipv6'):
         addr = get_ipv6_addr(exc_list=[config('vip')], fatal=True)[0]
-        hosts_map = {addr: hostname}
+        hosts_map = {addr: socket.gethostname()}
 
-    hosts = [hostname]
+    hosts = [local_cluster_address]
     for relid in relation_ids('cluster'):
         for unit in related_units(relid):
             rdata = relation_get(unit=unit, rid=relid)
@@ -192,7 +183,7 @@ def get_cluster_hosts():
                 hosts_map[cluster_address] = hostname
                 hosts.append(hostname)
             else:
-                hosts.append(get_host_ip(cluster_address))
+                hosts.append(resolve_hostname_to_ip(cluster_address))
 
     if hosts_map:
         update_hosts_file(hosts_map)
@@ -570,3 +561,24 @@ def create_binlogs_directory():
 
     if not os.path.isdir(binlogs_directory):
         mkdir(binlogs_directory, 'mysql', 'mysql', 0o750)
+
+
+def get_cluster_host_ip():
+    """Get the this host's IP address for use with percona cluster peers
+
+    @returns IP to pass to cluster peers
+    """
+
+    cluster_network = config('cluster-network')
+    if cluster_network:
+        cluster_addr = get_address_in_network(cluster_network, fatal=True)
+    else:
+        try:
+            cluster_addr = network_get_primary_address('cluster')
+        except NotImplementedError:
+            # NOTE(jamespage): fallback to previous behaviour
+            cluster_addr = resolve_hostname_to_ip(
+                unit_get('private-address')
+            )
+
+    return cluster_addr

unit_tests/test_percona_hooks.py

@@ -26,7 +26,7 @@ TO_PATCH = ['log', 'config',
             'network_get_primary_address',
             'resolve_network_cidr',
             'unit_get',
-            'get_host_ip',
+            'resolve_hostname_to_ip',
             'is_clustered',
             'get_ipv6_addr',
             'get_hacluster_config',
@@ -165,7 +165,7 @@ class TestHostResolution(CharmTestCase):
         Ensure that with nothing other than defaults private-address is used
         '''
         self.unit_get.return_value = 'mydbhost'
-        self.get_host_ip.return_value = '10.0.0.2'
+        self.resolve_hostname_to_ip.return_value = '10.0.0.2'
         self.assertEqual(hooks.get_db_host('myclient'), 'mydbhost')
 
     def test_get_db_host_network_spaces(self):
@@ -173,7 +173,7 @@ class TestHostResolution(CharmTestCase):
         Ensure that if the shared-db relation is bound, its bound address
         is used
         '''
-        self.get_host_ip.return_value = '10.0.0.2'
+        self.resolve_hostname_to_ip.return_value = '10.0.0.2'
         self.network_get_primary_address.side_effect = None
         self.network_get_primary_address.return_value = '192.168.20.2'
         self.assertEqual(hooks.get_db_host('myclient'), '192.168.20.2')
@@ -184,7 +184,7 @@ class TestHostResolution(CharmTestCase):
         Ensure that if the shared-db relation is bound and the unit is
         clustered, that the correct VIP is chosen
         '''
-        self.get_host_ip.return_value = '10.0.0.2'
+        self.resolve_hostname_to_ip.return_value = '10.0.0.2'
         self.is_clustered.return_value = True
         self.test_config.set('vip', '10.0.0.100 192.168.20.200')
         self.network_get_primary_address.side_effect = None

unit_tests/test_percona_utils.py

@@ -73,23 +73,24 @@ class UtilsTests(unittest.TestCase):
         self.assertEqual(lines[1], "%s %s\n" % (map.items()[0]))
         self.assertEqual(lines[4], "%s %s\n" % (map.items()[3]))
 
+    @mock.patch("percona_utils.get_cluster_host_ip")
     @mock.patch("percona_utils.log")
     @mock.patch("percona_utils.config")
     @mock.patch("percona_utils.update_hosts_file")
-    @mock.patch("percona_utils.get_host_ip")
     @mock.patch("percona_utils.relation_get")
     @mock.patch("percona_utils.related_units")
     @mock.patch("percona_utils.relation_ids")
     def test_get_cluster_hosts(self, mock_rel_ids, mock_rel_units,
-                               mock_rel_get, mock_get_host_ip,
+                               mock_rel_get,
                                mock_update_hosts_file, mock_config,
-                               mock_log):
+                               mock_log,
+                               mock_get_cluster_host_ip):
         mock_rel_ids.return_value = [1]
         mock_rel_units.return_value = [2]
-        mock_get_host_ip.return_value = 'hostA'
+        mock_get_cluster_host_ip.return_value = '10.2.0.1'
 
         def _mock_rel_get(*args, **kwargs):
-            return {'private-address': '0.0.0.0'}
+            return {'private-address': '10.2.0.2'}
 
         mock_rel_get.side_effect = _mock_rel_get
         mock_config.side_effect = lambda k: False
@@ -98,25 +99,29 @@ class UtilsTests(unittest.TestCase):
 
         self.assertFalse(mock_update_hosts_file.called)
         mock_rel_get.assert_called_with(rid=1, unit=2)
-        self.assertEqual(hosts, ['hostA', 'hostA'])
+        self.assertEqual(hosts, ['10.2.0.1', '10.2.0.2'])
 
+    @mock.patch.object(percona_utils, 'socket')
+    @mock.patch("percona_utils.get_cluster_host_ip")
     @mock.patch.object(percona_utils, 'get_ipv6_addr')
     @mock.patch.object(percona_utils, 'log')
     @mock.patch.object(percona_utils, 'config')
     @mock.patch.object(percona_utils, 'update_hosts_file')
-    @mock.patch.object(percona_utils, 'get_host_ip')
     @mock.patch.object(percona_utils, 'relation_get')
     @mock.patch.object(percona_utils, 'related_units')
     @mock.patch.object(percona_utils, 'relation_ids')
     def test_get_cluster_hosts_ipv6(self, mock_rel_ids, mock_rel_units,
-                                    mock_rel_get, mock_get_host_ip,
+                                    mock_rel_get,
                                     mock_update_hosts_file, mock_config,
-                                    mock_log, mock_get_ipv6_addr):
+                                    mock_log, mock_get_ipv6_addr,
+                                    mock_get_cluster_host_ip,
+                                    mock_socket):
         ipv6addr = '2001:db8:1:0:f816:3eff:fe79:cd'
         mock_get_ipv6_addr.return_value = [ipv6addr]
         mock_rel_ids.return_value = [88]
         mock_rel_units.return_value = [1, 2]
-        mock_get_host_ip.return_value = 'hostA'
+        mock_get_cluster_host_ip.return_value = 'hostA'
+        mock_socket.gethostname.return_value = 'hostA'
 
         def _mock_rel_get(*args, **kwargs):
             host_suffix = 'BC'