openstack
/
charms.openstack
Code Issues Proposed changes

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:
David Ames 2018-02-27 10:06:21 -08:00
parent d0d53c6163
commit d8e6722c45
3 changed files with 23 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
charms_openstack/ip.py
View File

@ -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

4
unit_tests/__init__.py
View File

@ -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):

26
unit_tests/test_charms_openstack_ip.py
View File

@ -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',),