Merge lp:~james-page/charm-helpers/network-spaces-api-endpoints into lp:charm-helpers

[James Page]

[lp:~james-page/charm-helpers/network-spaces-api-endpoints]

559. By James Page on 2016-03-30

Drop network_get until juju actually implements it

[David Ames (thedac)]

Looks good. Merging.

review: Approve

=== modified file 'charmhelpers/contrib/network/ip.py'
--- charmhelpers/contrib/network/ip.py	2016-01-27 12:47:55 +0000
+++ charmhelpers/contrib/network/ip.py	2016-03-30 21:04:29 +0000
@@ -191,6 +191,15 @@
 get_netmask_for_address = partial(_get_for_address, key='netmask')
 
 
+def resolve_network_cidr(ip_address):
+    '''
+    Resolves the full address cidr of an ip_address based on
+    configured network interfaces
+    '''
+    netmask = get_netmask_for_address(ip_address)
+    return str(netaddr.IPNetwork("%s/%s" % (ip_address, netmask)).cidr)
+
+
 def format_ipv6_addr(address):
     """If address is IPv6, wrap it in '[]' otherwise return None.
 

=== modified file 'charmhelpers/contrib/openstack/ip.py'
--- charmhelpers/contrib/openstack/ip.py	2015-06-04 22:16:21 +0000
+++ charmhelpers/contrib/openstack/ip.py	2016-03-30 21:04:29 +0000
@@ -14,16 +14,19 @@
 # You should have received a copy of the GNU Lesser General Public License
 # along with charm-helpers.  If not, see <http://www.gnu.org/licenses/>.
 
+
 from charmhelpers.core.hookenv import (
     config,
     unit_get,
     service_name,
+    network_get_primary_address,
 )
 from charmhelpers.contrib.network.ip import (
     get_address_in_network,
     is_address_in_network,
     is_ipv6,
     get_ipv6_addr,
+    resolve_network_cidr,
 )
 from charmhelpers.contrib.hahelpers.cluster import is_clustered
 
@@ -33,16 +36,19 @@
 
 ADDRESS_MAP = {
     PUBLIC: {
+        'binding': 'public',
         'config': 'os-public-network',
         'fallback': 'public-address',
         'override': 'os-public-hostname',
     },
     INTERNAL: {
+        'binding': 'internal',
         'config': 'os-internal-network',
         'fallback': 'private-address',
         'override': 'os-internal-hostname',
     },
     ADMIN: {
+        'binding': 'admin',
         'config': 'os-admin-network',
         'fallback': 'private-address',
         'override': 'os-admin-hostname',
@@ -110,7 +116,7 @@
     correct network. If clustered with no nets defined, return primary vip.
 
     If not clustered, return unit address ensuring address is on configured net
-    split if one is configured.
+    split if one is configured, or a Juju 2.0 extra-binding has been used.
 
     :param endpoint_type: Network endpoing type
     """
@@ -125,23 +131,45 @@
     net_type = ADDRESS_MAP[endpoint_type]['config']
     net_addr = config(net_type)
     net_fallback = ADDRESS_MAP[endpoint_type]['fallback']
+    binding = ADDRESS_MAP[endpoint_type]['binding']
     clustered = is_clustered()
-    if clustered:
-        if not net_addr:
-            # If no net-splits defined, we expect a single vip
-            resolved_address = vips[0]
-        else:
+
+    if clustered and vips:
+        if net_addr:
             for vip in vips:
                 if is_address_in_network(net_addr, vip):
                     resolved_address = vip
                     break
+        else:
+            # NOTE: endeavour to check vips against network space
+            #       bindings
+            try:
+                bound_cidr = resolve_network_cidr(
+                    network_get_primary_address(binding)
+                )
+                for vip in vips:
+                    if is_address_in_network(bound_cidr, vip):
+                        resolved_address = vip
+                        break
+            except NotImplementedError:
+                # If no net-splits configured and no support for extra
+                # bindings/network spaces so we expect a single vip
+                resolved_address = vips[0]
     else:
         if config('prefer-ipv6'):
             fallback_addr = get_ipv6_addr(exc_list=vips)[0]
         else:
             fallback_addr = unit_get(net_fallback)
 
-        resolved_address = get_address_in_network(net_addr, fallback_addr)
+        if net_addr:
+            resolved_address = get_address_in_network(net_addr, fallback_addr)
+        else:
+            # NOTE: only try to use extra bindings if legacy network
+            #       configuration is not in use
+            try:
+                resolved_address = network_get_primary_address(binding)
+            except NotImplementedError:
+                resolved_address = fallback_addr
 
     if resolved_address is None:
         raise ValueError("Unable to resolve a suitable IP address based on "

=== modified file 'tests/contrib/openstack/test_ip.py'
--- tests/contrib/openstack/test_ip.py	2015-06-04 22:16:21 +0000
+++ tests/contrib/openstack/test_ip.py	2016-03-30 21:04:29 +0000
@@ -9,6 +9,8 @@
     'get_address_in_network',
     'is_clustered',
     'service_name',
+    'network_get_primary_address',
+    'resolve_network_cidr',
 ]
 
 
@@ -32,6 +34,7 @@
             setattr(self, m, self._patch(m))
         self.test_config = TestConfig()
         self.config.side_effect = self.test_config.get
+        self.network_get_primary_address.side_effect = NotImplementedError
 
     def _patch(self, method):
         _m = patch('charmhelpers.contrib.openstack.ip.' + method)
@@ -48,7 +51,6 @@
         calls = [call('os-public-network'),
                  call('prefer-ipv6')]
         self.config.assert_has_calls(calls)
-        self.get_address_in_network.assert_called_with(None, 'unit1')
 
     def test_resolve_address_default_internal(self):
         self.is_clustered.return_value = False
@@ -59,7 +61,6 @@
         calls = [call('os-internal-network'),
                  call('prefer-ipv6')]
         self.config.assert_has_calls(calls)
-        self.get_address_in_network.assert_called_with(None, 'unit1')
 
     def test_resolve_address_public_not_clustered(self):
         self.is_clustered.return_value = False
@@ -111,8 +112,7 @@
     def test_resolve_address_ipv6_fallback(self):
         self.test_config.set('prefer-ipv6', True)
         self.is_clustered.return_value = False
-        ip.resolve_address()
-        self.get_address_in_network.assert_called_with(None, '::1')
+        self.assertEqual(ip.resolve_address(), '::1')
 
     @patch.object(ip, 'resolve_address')
     def test_canonical_url_http(self, resolve_address):
@@ -135,3 +135,28 @@
     def test_canonical_url_ipv6(self, resolve_address):
         resolve_address.return_value = 'unit1'
         self.assertTrue(ip.canonical_url(None), 'http://[unit1]')
+
+    def test_resolve_address_network_get(self):
+        self.is_clustered.return_value = False
+        self.unit_get.return_value = 'unit1'
+        self.network_get_primary_address.side_effect = None
+        self.network_get_primary_address.return_value = '10.5.60.1'
+        self.assertEqual(ip.resolve_address(), '10.5.60.1')
+        self.unit_get.assert_called_with('public-address')
+        calls = [call('os-public-network'),
+                 call('prefer-ipv6')]
+        self.config.assert_has_calls(calls)
+        self.network_get_primary_address.assert_called_with('public')
+
+    def test_resolve_address_network_get_clustered(self):
+        self.is_clustered.return_value = True
+        self.test_config.set('vip', '10.5.60.20 192.168.1.20')
+        self.network_get_primary_address.side_effect = None
+        self.network_get_primary_address.return_value = '10.5.60.1'
+        self.resolve_network_cidr.return_value = '10.5.60.1/24'
+        self.assertEqual(ip.resolve_address(), '10.5.60.20')
+        calls = [call('os-public-hostname'),
+                 call('vip'),
+                 call('os-public-network')]
+        self.config.assert_has_calls(calls)
+        self.network_get_primary_address.assert_called_with('public')