Always register the VIP with keystone catalog
Resolve_address is used to register an API charm's keystone catalog entry. Resolve_address was checking if the unit was clustered and if not falling back to its private-address. This is incorrect behavior. If a VIP is set it should always register the VIP with the keystone catalog. The fallout from this is manifested in two bugs: Bug#1750915 and Bug#1749280. In first, keystone is creating and passing SSL certificates for the local unit's private-ip and only subsequently for the VIP cause apache to fail to start. In the second, a race condition exists where clients of the API charm relying on catalog entries get disrupted when the catalog entry changes. This fix removes the is_clustered check from resolve_address. The gating on is_clustered should happen elsewhere in the charm not at registration time with keystone. Change-Id: Ieaee4fbcd1b45b95ddca4ec3a9a09d4522b09b03 Partial-Bug: #1750915 Partial-Bug: #1749280
This commit is contained in:
parent
d0d53c6163
commit
d8e6722c45
|
@ -18,7 +18,6 @@ import netaddr
|
|||
|
||||
import charmhelpers.core.hookenv as hookenv
|
||||
import charmhelpers.contrib.network.ip as net_ip
|
||||
import charmhelpers.contrib.hahelpers.cluster as cluster
|
||||
import charms.reactive.bus
|
||||
|
||||
PUBLIC = 'public'
|
||||
|
@ -113,11 +112,13 @@ def _resolve_network_cidr(ip_address):
|
|||
def resolve_address(endpoint_type=PUBLIC, override=True):
|
||||
"""Return unit address depending on net config.
|
||||
|
||||
If unit is clustered with vip(s) and has net splits defined, return vip on
|
||||
correct network. If clustered with no nets defined, return primary vip.
|
||||
If unit is has vip(s) configured and has net splits defined, return vip on
|
||||
correct network. If vip configured with no nets defined, return primary
|
||||
vip.
|
||||
|
||||
If not clustered, return unit address ensuring address is on configured net
|
||||
split if one is configured, or a Juju 2.0 extra-binding has been used.
|
||||
If no vip is configured, return unit address ensuring address is on
|
||||
configured net split if one is configured, or a Juju 2.0 extra-binding has
|
||||
been used.
|
||||
|
||||
:param endpoint_type: Network endpoing type
|
||||
:param override: Accept hostname overrides or not
|
||||
|
@ -136,9 +137,8 @@ def resolve_address(endpoint_type=PUBLIC, override=True):
|
|||
net_addr = hookenv.config(net_type)
|
||||
net_fallback = ADDRESS_MAP[endpoint_type]['fallback']
|
||||
binding = ADDRESS_MAP[endpoint_type]['binding']
|
||||
clustered = cluster.is_clustered()
|
||||
|
||||
if clustered and vips:
|
||||
if vips:
|
||||
if net_addr:
|
||||
for vip in vips:
|
||||
if net_ip.is_address_in_network(net_addr, vip):
|
||||
|
@ -179,6 +179,6 @@ def resolve_address(endpoint_type=PUBLIC, override=True):
|
|||
if resolved_address is None:
|
||||
raise ValueError("Unable to resolve a suitable IP address based on "
|
||||
"charm state and configuration. (net_type=%s, "
|
||||
"clustered=%s)" % (net_type, clustered))
|
||||
% (net_type))
|
||||
|
||||
return resolved_address
|
||||
|
|
|
@ -14,6 +14,7 @@
|
|||
|
||||
import sys
|
||||
import mock
|
||||
import os
|
||||
|
||||
# mock out some charmhelpers libraries as they have apt install side effects
|
||||
apt_pkg = mock.MagicMock()
|
||||
|
@ -64,6 +65,9 @@ charmhelpers.contrib.openstack.utils.OPENSTACK_RELEASES = (
|
|||
'pike',
|
||||
)
|
||||
|
||||
# charms.reactive uses hookenv.charm_dir which must return a directory
|
||||
charmhelpers.core.hookenv.charm_dir.return_value = os.path.curdir
|
||||
|
||||
|
||||
def _fake_retry(num_retries, base_delay=0, exc_type=Exception):
|
||||
def _retry_on_exception_inner_1(f):
|
||||
|
|
|
@ -51,7 +51,6 @@ class TestCharmOpenStackIp(utils.BaseTestCase):
|
|||
self.resolve_address.assert_called_once_with(ip.INTERNAL)
|
||||
|
||||
def test_resolve_address(self):
|
||||
self.patch_object(ip.cluster, 'is_clustered')
|
||||
self.patch_object(ip.hookenv, 'config')
|
||||
self.patch_object(ip.hookenv, 'network_get_primary_address')
|
||||
self.patch_object(ip.net_ip, 'is_address_in_network')
|
||||
|
@ -63,7 +62,7 @@ class TestCharmOpenStackIp(utils.BaseTestCase):
|
|||
# what it was called with.
|
||||
calls_list = []
|
||||
_config = {
|
||||
'vip': 'vip-address',
|
||||
'vip': None,
|
||||
'prefer-ipv6': False,
|
||||
'os-public-network': 'the-public-network',
|
||||
'os-public-hostname': None,
|
||||
|
@ -80,10 +79,9 @@ class TestCharmOpenStackIp(utils.BaseTestCase):
|
|||
# Juju pre 2.0 behaviour where network-get is not implemented
|
||||
self.network_get_primary_address.side_effect = NotImplementedError
|
||||
|
||||
# first test, if not clustered, that the function uses unit_get() and
|
||||
# first test, if no VIP, that the function uses unit_get() and
|
||||
# get_address_in_network to get a real address.
|
||||
# for the default PUBLIC endpoint
|
||||
self.is_clustered.return_value = False
|
||||
self.get_address_in_network.return_value = 'got-address'
|
||||
self.unit_get.return_value = 'unit-get-address'
|
||||
addr = ip.resolve_address()
|
||||
|
@ -97,18 +95,18 @@ class TestCharmOpenStackIp(utils.BaseTestCase):
|
|||
self.get_address_in_network.assert_called_once_with(
|
||||
'the-public-network', 'unit-get-address')
|
||||
|
||||
# second test: not clustered, prefer-ipv6 is True
|
||||
# second test: no vip, prefer-ipv6 is True
|
||||
_config['prefer-ipv6'] = True
|
||||
calls_list = []
|
||||
self.get_ipv6_addr.return_value = ['ipv6-addr']
|
||||
self.get_address_in_network.reset_mock()
|
||||
addr = ip.resolve_address()
|
||||
self.get_ipv6_addr.assert_called_once_with(exc_list=['vip-address'])
|
||||
self.get_ipv6_addr.assert_called_once_with(exc_list=None)
|
||||
self.get_address_in_network.assert_called_once_with(
|
||||
'the-public-network', 'ipv6-addr')
|
||||
|
||||
# Third test: clustered, and config(...) returns None
|
||||
self.is_clustered.return_value = True
|
||||
# Third test: vip, and config(...) returns None
|
||||
_config['vip'] = 'vip-address'
|
||||
_config['os-public-network'] = None
|
||||
calls_list = []
|
||||
addr = ip.resolve_address()
|
||||
|
@ -141,7 +139,6 @@ class TestCharmOpenStackIp(utils.BaseTestCase):
|
|||
addr = ip.resolve_address()
|
||||
|
||||
def test_resolve_address_network_binding(self):
|
||||
self.patch_object(ip.cluster, 'is_clustered')
|
||||
self.patch_object(ip.hookenv, 'config')
|
||||
self.patch_object(ip.hookenv, 'network_get_primary_address')
|
||||
self.patch_object(ip.net_ip, 'is_address_in_network')
|
||||
|
@ -154,7 +151,7 @@ class TestCharmOpenStackIp(utils.BaseTestCase):
|
|||
# what it was called with.
|
||||
calls_list = []
|
||||
_config = {
|
||||
'vip': 'vip1 vip2',
|
||||
'vip': None,
|
||||
'prefer-ipv6': False,
|
||||
'os-public-network': None,
|
||||
'os-public-hostname': None,
|
||||
|
@ -168,10 +165,9 @@ class TestCharmOpenStackIp(utils.BaseTestCase):
|
|||
|
||||
self.config.side_effect = fake_config
|
||||
|
||||
# first test, if not clustered, that the function uses unit_get
|
||||
# first test, if no vip, that the function uses unit_get
|
||||
# network_get_primary_address to get a real address.
|
||||
# for the default PUBLIC endpoint
|
||||
self.is_clustered.return_value = False
|
||||
self.network_get_primary_address.return_value = 'got-address'
|
||||
self._resolve_network_cidr.return_value = 'cidr'
|
||||
self.unit_get.return_value = 'unit-get-address'
|
||||
|
@ -187,7 +183,7 @@ class TestCharmOpenStackIp(utils.BaseTestCase):
|
|||
'public'
|
||||
)
|
||||
|
||||
# second test: not clustered, prefer-ipv6 is True, ensure
|
||||
# second test: no vip, prefer-ipv6 is True, ensure
|
||||
# that ipv6 address is fallback and network-get is still
|
||||
# used to determine the public endpoint binding
|
||||
_config['prefer-ipv6'] = True
|
||||
|
@ -195,7 +191,7 @@ class TestCharmOpenStackIp(utils.BaseTestCase):
|
|||
self.get_ipv6_addr.return_value = ['ipv6-addr']
|
||||
self.get_address_in_network.reset_mock()
|
||||
addr = ip.resolve_address()
|
||||
self.get_ipv6_addr.assert_called_once_with(exc_list=['vip1', 'vip2'])
|
||||
self.get_ipv6_addr.assert_called_once_with(exc_list=None)
|
||||
self.network_get_primary_address.assert_called_with(
|
||||
'public'
|
||||
)
|
||||
|
@ -206,7 +202,7 @@ class TestCharmOpenStackIp(utils.BaseTestCase):
|
|||
self.is_address_in_network.side_effect = _fake_addr_in_net
|
||||
|
||||
# Third test: clustered
|
||||
self.is_clustered.return_value = True
|
||||
_config['vip'] = 'vip1 vip2'
|
||||
calls_list = []
|
||||
addr = ip.resolve_address()
|
||||
self.assertEqual(calls_list, [('os-public-hostname',),
|
||||
|
|
Loading…
Reference in New Issue