Avoid shared-db change when using access-network

When the percona-cluster charm sets an access-network but the default
unit-get address is not on that network extra shared-db relations get
executed. This is specifically a problem when running upgrades and
trying to avoid API downtime.

The root cause is that the access-network is not checked until the
SharedDBContext is consulted. But then db_joined function will
change it back to the wrong ip on subsequent runs.

This change adds a check for access-network on the relation during
the db_joined function and pushes IP selection off to get_relation_ip.

Charm helpers sync to pull in changes to get_relation_ip.

Partial-bug: #1677647

Change-Id: I20f35dd7a12315ef61939feb5199680db128bc0b
This commit is contained in:
David Ames 2017-04-25 13:43:44 -07:00
parent 90669a3b21
commit 43e4f2124f
13 changed files with 129 additions and 63 deletions

View File

@ -2,6 +2,7 @@ branch: lp:charm-helpers
destination: tests/charmhelpers destination: tests/charmhelpers
include: include:
- contrib.amulet - contrib.amulet
- osplatform
- contrib.openstack.amulet - contrib.openstack.amulet
- core - core
- osplatform - osplatform

View File

@ -373,7 +373,7 @@ def add_init_service_checks(nrpe, services, unit_name, immediate_check=True):
checkpath = '%s/service-check-%s.txt' % (nrpe.homedir, svc) checkpath = '%s/service-check-%s.txt' % (nrpe.homedir, svc)
croncmd = ( croncmd = (
'/usr/local/lib/nagios/plugins/check_exit_status.pl ' '/usr/local/lib/nagios/plugins/check_exit_status.pl '
'-s /etc/init.d/%s status' % svc '-e -s /etc/init.d/%s status' % svc
) )
cron_file = '*/5 * * * * root %s > %s\n' % (croncmd, checkpath) cron_file = '*/5 * * * * root %s > %s\n' % (croncmd, checkpath)
f = open(cronpath, 'w') f = open(cronpath, 'w')

View File

@ -111,11 +111,11 @@ def get_address_in_network(network, fallback=None, fatal=False):
for iface in netifaces.interfaces(): for iface in netifaces.interfaces():
addresses = netifaces.ifaddresses(iface) addresses = netifaces.ifaddresses(iface)
if network.version == 4 and netifaces.AF_INET in addresses: if network.version == 4 and netifaces.AF_INET in addresses:
addr = addresses[netifaces.AF_INET][0]['addr'] for addr in addresses[netifaces.AF_INET]:
netmask = addresses[netifaces.AF_INET][0]['netmask'] cidr = netaddr.IPNetwork("%s/%s" % (addr['addr'],
cidr = netaddr.IPNetwork("%s/%s" % (addr, netmask)) addr['netmask']))
if cidr in network: if cidr in network:
return str(cidr.ip) return str(cidr.ip)
if network.version == 6 and netifaces.AF_INET6 in addresses: if network.version == 6 and netifaces.AF_INET6 in addresses:
for addr in addresses[netifaces.AF_INET6]: for addr in addresses[netifaces.AF_INET6]:
@ -239,6 +239,16 @@ def format_ipv6_addr(address):
return None return None
def is_ipv6_disabled():
try:
result = subprocess.check_output(
['sysctl', 'net.ipv6.conf.all.disable_ipv6'],
stderr=subprocess.STDOUT)
return "net.ipv6.conf.all.disable_ipv6 = 1" in result
except subprocess.CalledProcessError:
return True
def get_iface_addr(iface='eth0', inet_type='AF_INET', inc_aliases=False, def get_iface_addr(iface='eth0', inet_type='AF_INET', inc_aliases=False,
fatal=True, exc_list=None): fatal=True, exc_list=None):
"""Return the assigned IP address for a given interface, if any. """Return the assigned IP address for a given interface, if any.
@ -544,31 +554,38 @@ def assert_charm_supports_ipv6():
"versions less than Trusty 14.04") "versions less than Trusty 14.04")
def get_relation_ip(interface, config_override=None): def get_relation_ip(interface, cidr_network=None):
"""Return this unit's IP for the given relation. """Return this unit's IP for the given interface.
Allow for an arbitrary interface to use with network-get to select an IP. Allow for an arbitrary interface to use with network-get to select an IP.
Handle all address selection options including configuration parameter Handle all address selection options including passed cidr network and
override and IPv6. IPv6.
Usage: get_relation_ip('amqp', config_override='access-network') Usage: get_relation_ip('amqp', cidr_network='10.0.0.0/8')
@param interface: string name of the relation. @param interface: string name of the relation.
@param config_override: string name of the config option for network @param cidr_network: string CIDR Network to select an address from.
override. Supports legacy network override configuration parameters.
@raises Exception if prefer-ipv6 is configured but IPv6 unsupported. @raises Exception if prefer-ipv6 is configured but IPv6 unsupported.
@returns IPv6 or IPv4 address @returns IPv6 or IPv4 address
""" """
# Select the interface address first
# For possible use as a fallback bellow with get_address_in_network
try:
# Get the interface specific IP
address = network_get_primary_address(interface)
except NotImplementedError:
# If network-get is not available
address = get_host_ip(unit_get('private-address'))
fallback = get_host_ip(unit_get('private-address'))
if config('prefer-ipv6'): if config('prefer-ipv6'):
# Currently IPv6 has priority, eventually we want IPv6 to just be
# another network space.
assert_charm_supports_ipv6() assert_charm_supports_ipv6()
return get_ipv6_addr()[0] return get_ipv6_addr()[0]
elif config_override and config(config_override): elif cidr_network:
return get_address_in_network(config(config_override), # If a specific CIDR network is passed get the address from that
fallback) # network.
else: return get_address_in_network(cidr_network, address)
try:
return network_get_primary_address(interface) # Return the interface address
except NotImplementedError: return address
return fallback

View File

@ -547,7 +547,7 @@ class OpenStackAmuletUtils(AmuletUtils):
"""Create the specified instance.""" """Create the specified instance."""
self.log.debug('Creating instance ' self.log.debug('Creating instance '
'({}|{}|{})'.format(instance_name, image_name, flavor)) '({}|{}|{})'.format(instance_name, image_name, flavor))
image = nova.images.find(name=image_name) image = nova.glance.find_image(image_name)
flavor = nova.flavors.find(name=flavor) flavor = nova.flavors.find(name=flavor)
instance = nova.servers.create(name=instance_name, image=image, instance = nova.servers.create(name=instance_name, image=image,
flavor=flavor) flavor=flavor)

View File

@ -60,6 +60,7 @@ from charmhelpers.core.host import (
pwgen, pwgen,
lsb_release, lsb_release,
CompareHostReleases, CompareHostReleases,
is_container,
) )
from charmhelpers.contrib.hahelpers.cluster import ( from charmhelpers.contrib.hahelpers.cluster import (
determine_apache_port, determine_apache_port,
@ -88,6 +89,7 @@ from charmhelpers.contrib.network.ip import (
format_ipv6_addr, format_ipv6_addr,
is_address_in_network, is_address_in_network,
is_bridge_member, is_bridge_member,
is_ipv6_disabled,
) )
from charmhelpers.contrib.openstack.utils import ( from charmhelpers.contrib.openstack.utils import (
config_flags_parser, config_flags_parser,
@ -109,6 +111,7 @@ except ImportError:
CA_CERT_PATH = '/usr/local/share/ca-certificates/keystone_juju_ca_cert.crt' CA_CERT_PATH = '/usr/local/share/ca-certificates/keystone_juju_ca_cert.crt'
ADDRESS_TYPES = ['admin', 'internal', 'public'] ADDRESS_TYPES = ['admin', 'internal', 'public']
HAPROXY_RUN_DIR = '/var/run/haproxy/'
def ensure_packages(packages): def ensure_packages(packages):
@ -534,6 +537,8 @@ class HAProxyContext(OSContextGenerator):
"""Provides half a context for the haproxy template, which describes """Provides half a context for the haproxy template, which describes
all peers to be included in the cluster. Each charm needs to include all peers to be included in the cluster. Each charm needs to include
its own context generator that describes the port mapping. its own context generator that describes the port mapping.
:side effect: mkdir is called on HAPROXY_RUN_DIR
""" """
interfaces = ['cluster'] interfaces = ['cluster']
@ -541,6 +546,8 @@ class HAProxyContext(OSContextGenerator):
self.singlenode_mode = singlenode_mode self.singlenode_mode = singlenode_mode
def __call__(self): def __call__(self):
if not os.path.isdir(HAPROXY_RUN_DIR):
mkdir(path=HAPROXY_RUN_DIR)
if not relation_ids('cluster') and not self.singlenode_mode: if not relation_ids('cluster') and not self.singlenode_mode:
return {} return {}
@ -1221,6 +1228,10 @@ class BindHostContext(OSContextGenerator):
return {'bind_host': '0.0.0.0'} return {'bind_host': '0.0.0.0'}
MAX_DEFAULT_WORKERS = 4
DEFAULT_MULTIPLIER = 2
class WorkerConfigContext(OSContextGenerator): class WorkerConfigContext(OSContextGenerator):
@property @property
@ -1232,10 +1243,19 @@ class WorkerConfigContext(OSContextGenerator):
return psutil.NUM_CPUS return psutil.NUM_CPUS
def __call__(self): def __call__(self):
multiplier = config('worker-multiplier') or 0 multiplier = config('worker-multiplier') or DEFAULT_MULTIPLIER
count = int(self.num_cpus * multiplier) count = int(self.num_cpus * multiplier)
if multiplier > 0 and count == 0: if multiplier > 0 and count == 0:
count = 1 count = 1
if config('worker-multiplier') is None and is_container():
# NOTE(jamespage): Limit unconfigured worker-multiplier
# to MAX_DEFAULT_WORKERS to avoid insane
# worker configuration in LXD containers
# on large servers
# Reference: https://pad.lv/1665270
count = min(count, MAX_DEFAULT_WORKERS)
ctxt = {"workers": count} ctxt = {"workers": count}
return ctxt return ctxt
@ -1588,7 +1608,7 @@ class MemcacheContext(OSContextGenerator):
"""Memcache context """Memcache context
This context provides options for configuring a local memcache client and This context provides options for configuring a local memcache client and
server server for both IPv4 and IPv6
""" """
def __init__(self, package=None): def __init__(self, package=None):
@ -1606,13 +1626,24 @@ class MemcacheContext(OSContextGenerator):
# Trusty version of memcached does not support ::1 as a listen # Trusty version of memcached does not support ::1 as a listen
# address so use host file entry instead # address so use host file entry instead
release = lsb_release()['DISTRIB_CODENAME'].lower() release = lsb_release()['DISTRIB_CODENAME'].lower()
if CompareHostReleases(release) > 'trusty': if is_ipv6_disabled():
ctxt['memcache_server'] = '::1' if CompareHostReleases(release) > 'trusty':
ctxt['memcache_server'] = '127.0.0.1'
else:
ctxt['memcache_server'] = 'localhost'
ctxt['memcache_server_formatted'] = '127.0.0.1'
ctxt['memcache_port'] = '11211'
ctxt['memcache_url'] = '{}:{}'.format(
ctxt['memcache_server_formatted'],
ctxt['memcache_port'])
else: else:
ctxt['memcache_server'] = 'ip6-localhost' if CompareHostReleases(release) > 'trusty':
ctxt['memcache_server_formatted'] = '[::1]' ctxt['memcache_server'] = '::1'
ctxt['memcache_port'] = '11211' else:
ctxt['memcache_url'] = 'inet6:{}:{}'.format( ctxt['memcache_server'] = 'ip6-localhost'
ctxt['memcache_server_formatted'], ctxt['memcache_server_formatted'] = '[::1]'
ctxt['memcache_port']) ctxt['memcache_port'] = '11211'
ctxt['memcache_url'] = 'inet6:{}:{}'.format(
ctxt['memcache_server_formatted'],
ctxt['memcache_port'])
return ctxt return ctxt

View File

@ -5,6 +5,8 @@ global
user haproxy user haproxy
group haproxy group haproxy
spread-checks 0 spread-checks 0
stats socket /var/run/haproxy/admin.sock mode 600 level admin
stats timeout 2m
defaults defaults
log global log global
@ -58,6 +60,15 @@ frontend tcp-in_{{ service }}
{% for frontend in frontends -%} {% for frontend in frontends -%}
backend {{ service }}_{{ frontend }} backend {{ service }}_{{ frontend }}
balance leastconn balance leastconn
{% if backend_options -%}
{% if backend_options[service] -%}
{% for option in backend_options[service] -%}
{% for key, value in option.items() -%}
{{ key }} {{ value }}
{% endfor -%}
{% endfor -%}
{% endif -%}
{% endif -%}
{% for unit, address in frontends[frontend]['backends'].items() -%} {% for unit, address in frontends[frontend]['backends'].items() -%}
server {{ unit }} {{ address }}:{{ ports[1] }} check server {{ unit }} {{ address }}:{{ ports[1] }} check
{% endfor %} {% endfor %}

View File

@ -987,18 +987,20 @@ def ensure_ceph_storage(service, pool, rbd_img, sizemb, mount_point,
service_start(svc) service_start(svc)
def ensure_ceph_keyring(service, user=None, group=None, relation='ceph'): def ensure_ceph_keyring(service, user=None, group=None,
relation='ceph', key=None):
"""Ensures a ceph keyring is created for a named service and optionally """Ensures a ceph keyring is created for a named service and optionally
ensures user and group ownership. ensures user and group ownership.
Returns False if no ceph key is available in relation state. @returns boolean: Flag to indicate whether a key was successfully written
to disk based on either relation data or a supplied key
""" """
key = None if not key:
for rid in relation_ids(relation): for rid in relation_ids(relation):
for unit in related_units(rid): for unit in related_units(rid):
key = relation_get('key', rid=rid, unit=unit) key = relation_get('key', rid=rid, unit=unit)
if key: if key:
break break
if not key: if not key:
return False return False

View File

@ -191,7 +191,7 @@ def service_pause(service_name, init_dir="/etc/init", initd_dir="/etc/init.d",
upstart_file = os.path.join(init_dir, "{}.conf".format(service_name)) upstart_file = os.path.join(init_dir, "{}.conf".format(service_name))
sysv_file = os.path.join(initd_dir, service_name) sysv_file = os.path.join(initd_dir, service_name)
if init_is_systemd(): if init_is_systemd():
service('disable', service_name) service('mask', service_name)
elif os.path.exists(upstart_file): elif os.path.exists(upstart_file):
override_path = os.path.join( override_path = os.path.join(
init_dir, '{}.override'.format(service_name)) init_dir, '{}.override'.format(service_name))
@ -224,7 +224,7 @@ def service_resume(service_name, init_dir="/etc/init",
upstart_file = os.path.join(init_dir, "{}.conf".format(service_name)) upstart_file = os.path.join(init_dir, "{}.conf".format(service_name))
sysv_file = os.path.join(initd_dir, service_name) sysv_file = os.path.join(initd_dir, service_name)
if init_is_systemd(): if init_is_systemd():
service('enable', service_name) service('unmask', service_name)
elif os.path.exists(upstart_file): elif os.path.exists(upstart_file):
override_path = os.path.join( override_path = os.path.join(
init_dir, '{}.override'.format(service_name)) init_dir, '{}.override'.format(service_name))

View File

@ -34,7 +34,7 @@ from charmhelpers.core.hookenv import (
status_set, status_set,
open_port, open_port,
unit_get, unit_get,
network_get_primary_address, related_units,
) )
from charmhelpers.core.host import ( from charmhelpers.core.host import (
@ -120,7 +120,8 @@ from charmhelpers.contrib.network.ip import (
get_netmask_for_address, get_netmask_for_address,
get_address_in_network, get_address_in_network,
get_ipv6_addr, get_ipv6_addr,
is_ipv6 is_ipv6,
get_relation_ip,
) )
from charmhelpers.contrib.openstack.context import ADDRESS_TYPES from charmhelpers.contrib.openstack.context import ADDRESS_TYPES
@ -328,13 +329,14 @@ def db_joined():
sync_db_with_multi_ipv6_addresses(config('database'), sync_db_with_multi_ipv6_addresses(config('database'),
config('database-user')) config('database-user'))
else: else:
host = None # Avoid churn check for access-network early
try: access_network = None
# NOTE: try to use network spaces for unit in related_units():
host = network_get_primary_address('shared-db') access_network = relation_get(unit=unit,
except NotImplementedError: attribute='access-network')
# NOTE: fallback to private-address if access_network:
host = unit_get('private-address') break
host = get_relation_ip('shared-db', cidr_network=access_network)
relation_set(database=config('database'), relation_set(database=config('database'),
username=config('database-user'), username=config('database-user'),

View File

@ -547,7 +547,7 @@ class OpenStackAmuletUtils(AmuletUtils):
"""Create the specified instance.""" """Create the specified instance."""
self.log.debug('Creating instance ' self.log.debug('Creating instance '
'({}|{}|{})'.format(instance_name, image_name, flavor)) '({}|{}|{})'.format(instance_name, image_name, flavor))
image = nova.images.find(name=image_name) image = nova.glance.find_image(image_name)
flavor = nova.flavors.find(name=flavor) flavor = nova.flavors.find(name=flavor)
instance = nova.servers.create(name=instance_name, image=image, instance = nova.servers.create(name=instance_name, image=image,
flavor=flavor) flavor=flavor)

View File

@ -191,7 +191,7 @@ def service_pause(service_name, init_dir="/etc/init", initd_dir="/etc/init.d",
upstart_file = os.path.join(init_dir, "{}.conf".format(service_name)) upstart_file = os.path.join(init_dir, "{}.conf".format(service_name))
sysv_file = os.path.join(initd_dir, service_name) sysv_file = os.path.join(initd_dir, service_name)
if init_is_systemd(): if init_is_systemd():
service('disable', service_name) service('mask', service_name)
elif os.path.exists(upstart_file): elif os.path.exists(upstart_file):
override_path = os.path.join( override_path = os.path.join(
init_dir, '{}.override'.format(service_name)) init_dir, '{}.override'.format(service_name))
@ -224,7 +224,7 @@ def service_resume(service_name, init_dir="/etc/init",
upstart_file = os.path.join(init_dir, "{}.conf".format(service_name)) upstart_file = os.path.join(init_dir, "{}.conf".format(service_name))
sysv_file = os.path.join(initd_dir, service_name) sysv_file = os.path.join(initd_dir, service_name)
if init_is_systemd(): if init_is_systemd():
service('enable', service_name) service('unmask', service_name)
elif os.path.exists(upstart_file): elif os.path.exists(upstart_file):
override_path = os.path.join( override_path = os.path.join(
init_dir, '{}.override'.format(service_name)) init_dir, '{}.override'.format(service_name))

View File

@ -256,14 +256,16 @@ class HAProxyContextTest(CharmTestCase):
def tearDown(self): def tearDown(self):
super(HAProxyContextTest, self).tearDown() super(HAProxyContextTest, self).tearDown()
@patch.object(charmhelpers.contrib.openstack.context, 'mkdir')
@patch.object(charmhelpers.contrib.openstack.context, 'relation_ids') @patch.object(charmhelpers.contrib.openstack.context, 'relation_ids')
@patch.object(charmhelpers.contrib.openstack.context, 'log') @patch.object(charmhelpers.contrib.openstack.context, 'log')
def test_context_No_peers(self, _log, _rids): def test_context_No_peers(self, _log, _rids, _mkdir):
_rids.return_value = [] _rids.return_value = []
hap_ctxt = context.HAProxyContext() hap_ctxt = context.HAProxyContext()
with patch('__builtin__.__import__'): with patch('__builtin__.__import__'):
self.assertTrue('units' not in hap_ctxt()) self.assertTrue('units' not in hap_ctxt())
@patch.object(charmhelpers.contrib.openstack.context, 'mkdir')
@patch.object( @patch.object(
charmhelpers.contrib.openstack.context, 'get_netmask_for_address') charmhelpers.contrib.openstack.context, 'get_netmask_for_address')
@patch.object( @patch.object(
@ -280,7 +282,8 @@ class HAProxyContextTest(CharmTestCase):
@patch('__builtin__.open') @patch('__builtin__.open')
def test_context_peers(self, _open, _import, _kv, _log, _rids, _runits, def test_context_peers(self, _open, _import, _kv, _log, _rids, _runits,
_rget, _uget, _lunit, _config, _rget, _uget, _lunit, _config,
_get_address_in_network, _get_netmask_for_address): _get_address_in_network, _get_netmask_for_address,
_mkdir):
unit_addresses = { unit_addresses = {
'neutron-api-0': '10.10.10.10', 'neutron-api-0': '10.10.10.10',
'neutron-api-1': '10.10.10.11', 'neutron-api-1': '10.10.10.11',

View File

@ -77,6 +77,7 @@ TO_PATCH = [
'relation_get', 'relation_get',
'relation_ids', 'relation_ids',
'relation_set', 'relation_set',
'related_units',
'service_restart', 'service_restart',
'unit_get', 'unit_get',
'get_iface_for_address', 'get_iface_for_address',
@ -88,7 +89,7 @@ TO_PATCH = [
'IdentityServiceContext', 'IdentityServiceContext',
'force_etcd_restart', 'force_etcd_restart',
'status_set', 'status_set',
'network_get_primary_address', 'get_relation_ip',
'update_dns_ha_resource_params', 'update_dns_ha_resource_params',
] ]
NEUTRON_CONF_DIR = "/etc/neutron" NEUTRON_CONF_DIR = "/etc/neutron"
@ -133,7 +134,6 @@ class NeutronAPIHooksTests(CharmTestCase):
self.test_config.set('openstack-origin', 'distro') self.test_config.set('openstack-origin', 'distro')
self.test_config.set('neutron-plugin', 'ovs') self.test_config.set('neutron-plugin', 'ovs')
self.neutron_plugin_attribute.side_effect = _mock_nuage_npa self.neutron_plugin_attribute.side_effect = _mock_nuage_npa
self.network_get_primary_address.side_effect = NotImplementedError
def _fake_relids(self, rel_name): def _fake_relids(self, rel_name):
return [randrange(100) for _count in range(2)] return [randrange(100) for _count in range(2)]
@ -350,17 +350,16 @@ class NeutronAPIHooksTests(CharmTestCase):
def test_db_joined(self): def test_db_joined(self):
self.is_relation_made.return_value = False self.is_relation_made.return_value = False
self.unit_get.return_value = 'myhostname' self.get_relation_ip.return_value = '10.0.0.1'
self._call_hook('shared-db-relation-joined') self._call_hook('shared-db-relation-joined')
self.relation_set.assert_called_with( self.relation_set.assert_called_with(
username='neutron', username='neutron',
database='neutron', database='neutron',
hostname='myhostname', hostname='10.0.0.1',
) )
def test_db_joined_spaces(self): def test_db_joined_spaces(self):
self.network_get_primary_address.side_effect = None self.get_relation_ip.return_value = '192.168.20.1'
self.network_get_primary_address.return_value = '192.168.20.1'
self.is_relation_made.return_value = False self.is_relation_made.return_value = False
self.unit_get.return_value = 'myhostname' self.unit_get.return_value = 'myhostname'
self._call_hook('shared-db-relation-joined') self._call_hook('shared-db-relation-joined')