From 71e38aae6401b975683c2b013deb483eadc5c117 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Fri, 10 Jan 2020 10:57:44 +0100 Subject: [PATCH] Use hosts official name for FQDN The current implementations use of a specific interface to build FQDN from has the undesired side effect of the ``nova-compute`` and ``neutron-openvswitch`` charms ending up with using different hostnames in some situations. It may also lead to use of a identifier that is mutable throughout the lifetime of a deployment. Use of a specific interface was chosen due to ``socket.getfqdn()`` not giving reliable results (https://bugs.python.org/issue5004). This patch gets the FQDN by mimicking the behaviour of a call to ``hostname -f`` with fallback to shortname on failure. Add relevant update from c-h. Change-Id: Ic8f8742261b773484687985aa0a366391cd2737a Closes-Bug: #1839300 (cherry picked from commit ee709a5ab30f285ecc1dd3ddb998af970f22e17e) --- .../charmhelpers/contrib/openstack/context.py | 61 ++++++++++++++++++- hooks/neutron_ovs_context.py | 32 ---------- hooks/neutron_ovs_hooks.py | 14 +++-- hooks/neutron_ovs_utils.py | 15 ++++- templates/stein/neutron.conf | 4 +- unit_tests/test_neutron_ovs_context.py | 28 --------- unit_tests/test_neutron_ovs_hooks.py | 30 +++++---- unit_tests/test_neutron_ovs_utils.py | 7 +++ 8 files changed, 108 insertions(+), 83 deletions(-) diff --git a/hooks/charmhelpers/contrib/openstack/context.py b/hooks/charmhelpers/contrib/openstack/context.py index a3d48c41..b171a06d 100644 --- a/hooks/charmhelpers/contrib/openstack/context.py +++ b/hooks/charmhelpers/contrib/openstack/context.py @@ -2177,9 +2177,66 @@ class LogrotateContext(OSContextGenerator): class HostInfoContext(OSContextGenerator): """Context to provide host information.""" + def __init__(self, use_fqdn_hint_cb=None): + """Initialize HostInfoContext + + :param use_fqdn_hint_cb: Callback whose return value used to populate + `use_fqdn_hint` + :type use_fqdn_hint_cb: Callable[[], bool] + """ + # Store callback used to get hint for whether FQDN should be used + + # Depending on the workload a charm manages, the use of FQDN vs. + # shortname may be a deploy-time decision, i.e. behaviour can not + # change on charm upgrade or post-deployment configuration change. + + # The hint is passed on as a flag in the context to allow the decision + # to be made in the Jinja2 configuration template. + self.use_fqdn_hint_cb = use_fqdn_hint_cb + + def _get_canonical_name(self, name=None): + """Get the official FQDN of the host + + The implementation of ``socket.getfqdn()`` in the standard Python + library does not exhaust all methods of getting the official name + of a host ref Python issue https://bugs.python.org/issue5004 + + This function mimics the behaviour of a call to ``hostname -f`` to + get the official FQDN but returns an empty string if it is + unsuccessful. + + :param name: Shortname to get FQDN on + :type name: Optional[str] + :returns: The official FQDN for host or empty string ('') + :rtype: str + """ + name = name or socket.gethostname() + fqdn = '' + + if six.PY2: + exc = socket.error + else: + exc = OSError + + try: + addrs = socket.getaddrinfo( + name, None, 0, socket.SOCK_DGRAM, 0, socket.AI_CANONNAME) + except exc: + pass + else: + for addr in addrs: + if addr[3]: + if '.' in addr[3]: + fqdn = addr[3] + break + return fqdn + def __call__(self): + name = socket.gethostname() ctxt = { - 'host_fqdn': socket.getfqdn(), - 'host': socket.gethostname(), + 'host_fqdn': self._get_canonical_name(name) or name, + 'host': name, + 'use_fqdn_hint': ( + self.use_fqdn_hint_cb() if self.use_fqdn_hint_cb else False) } return ctxt diff --git a/hooks/neutron_ovs_context.py b/hooks/neutron_ovs_context.py index 2546dc9a..ff296ba9 100644 --- a/hooks/neutron_ovs_context.py +++ b/hooks/neutron_ovs_context.py @@ -15,7 +15,6 @@ import collections import glob import os -import socket import uuid from pci import PCINetDevices from charmhelpers.core.hookenv import ( @@ -39,7 +38,6 @@ from charmhelpers.contrib.openstack.utils import ( ) from charmhelpers.contrib.network.ip import ( get_address_in_network, - get_relation_ip, ) from charmhelpers.contrib.openstack.context import ( OSContextGenerator, @@ -50,7 +48,6 @@ from charmhelpers.contrib.openstack.utils import ( os_release, CompareOpenStackReleases, ) -import charmhelpers.contrib.openstack.utils as os_utils from charmhelpers.core.unitdata import kv IPTABLES_HYBRID = 'iptables_hybrid' @@ -624,32 +621,3 @@ class APIIdentityServiceContext(context.IdentityServiceContext): if ctxt['region']: return ctxt return ctxt - - -class HostIPContext(context.OSContextGenerator): - def __call__(self): - ctxt = {} - # Use the address used in the neutron-plugin subordinate relation - host_ip = get_relation_ip('neutron-plugin') - - cmp_release = os_utils.CompareOpenStackReleases( - os_utils.os_release('neutron-common', base='icehouse')) - # the contents of the Neutron ``host`` configuration option is - # referenced throughout a OpenStack deployment, an example being - # Neutron port bindings. It's value should not change after a - # individual units initial deployment. - # - # We do want to migrate to using FQDNs so we enable this for new - # installations. - db = kv() - if (db.get('neutron-ovs-charm-use-fqdn', False) and - cmp_release >= 'stein' and - host_ip): - fqdn = socket.getfqdn(host_ip) - if '.' in fqdn: - # only populate the value if getfqdn() is able to find an - # actual FQDN for this host. If not, we revert back to - # not setting the configuration option and use Neutron's - # default behaviour. - ctxt['host'] = fqdn - return ctxt diff --git a/hooks/neutron_ovs_hooks.py b/hooks/neutron_ovs_hooks.py index 6edc6aaf..9f0d4ea0 100755 --- a/hooks/neutron_ovs_hooks.py +++ b/hooks/neutron_ovs_hooks.py @@ -19,6 +19,8 @@ import uuid from copy import deepcopy +from charmhelpers.contrib.openstack import context as os_context + from charmhelpers.contrib.openstack.utils import ( pausable_restart_on_change as restart_on_change, series_upgrade_prepare, @@ -51,6 +53,7 @@ from neutron_ovs_utils import ( L3HA_PACKAGES, METADATA_PACKAGES, OVS_DEFAULT, + USE_FQDN_KEY, configure_ovs, configure_sriov, get_shared_secret, @@ -70,10 +73,9 @@ from neutron_ovs_utils import ( determine_purge_packages, install_sriov_systemd_files, enable_sriov, + use_fqdn_hint, ) -import neutron_ovs_context - hooks = Hooks() CONFIGS = register_configs() @@ -87,7 +89,7 @@ def install(): release = os_release('neutron-common') if CompareOpenStackReleases(release) >= 'stein': db = kv() - db.set('neutron-ovs-charm-use-fqdn', True) + db.set(USE_FQDN_KEY, True) db.flush() @@ -203,9 +205,9 @@ def neutron_plugin_joined(relation_id=None, request_restart=False): rel_data = { 'metadata-shared-secret': secret, } - host = neutron_ovs_context.HostIPContext()().get('host') - if host: - rel_data.update({'host': host}) + host_info = os_context.HostInfoContext()() + if use_fqdn_hint() and host_info.get('host_fqdn'): + rel_data.update({'host': host_info['host_fqdn']}) if request_restart: rel_data['restart-nonce'] = str(uuid.uuid4()) relation_set(relation_id=relation_id, **rel_data) diff --git a/hooks/neutron_ovs_utils.py b/hooks/neutron_ovs_utils.py index c1281cac..6b91db25 100644 --- a/hooks/neutron_ovs_utils.py +++ b/hooks/neutron_ovs_utils.py @@ -33,6 +33,7 @@ from charmhelpers.contrib.openstack.utils import ( CompareOpenStackReleases, os_release, ) +from charmhelpers.core.unitdata import kv from collections import OrderedDict import neutron_ovs_context from charmhelpers.contrib.network.ovs import ( @@ -153,6 +154,18 @@ NEUTRON_SRIOV_SYSTEMD_UNIT = os.path.join('/lib/systemd/system', NEUTRON_SRIOV_UPSTART_CONF = os.path.join('/etc/init', 'neutron-openvswitch-' 'networking-sriov.conf') +USE_FQDN_KEY = 'neutron-ovs-charm-use-fqdn' + + +def use_fqdn_hint(): + """Hint for whether FQDN should be used for agent registration + + :returns: True or False + :rtype: bool + """ + db = kv() + return db.get(USE_FQDN_KEY, False) + BASE_RESOURCE_MAP = OrderedDict([ (NEUTRON_CONF, { @@ -163,7 +176,7 @@ BASE_RESOURCE_MAP = OrderedDict([ context.AMQPContext(ssl_dir=NEUTRON_CONF_DIR), context.ZeroMQContext(), context.NotificationDriverContext(), - neutron_ovs_context.HostIPContext(), + context.HostInfoContext(use_fqdn_hint_cb=use_fqdn_hint), neutron_ovs_context.ZoneContext(), ], }), diff --git a/templates/stein/neutron.conf b/templates/stein/neutron.conf index b9b31868..9a4571a2 100644 --- a/templates/stein/neutron.conf +++ b/templates/stein/neutron.conf @@ -23,8 +23,8 @@ core_plugin = {{ core_plugin }} transport_url = {{ transport_url }} {% endif %} -{% if host -%} -host = {{ host }} +{% if use_fqdn_hint and host_fqdn -%} +host = {{ host_fqdn }} {% endif -%} api_paste_config = /etc/neutron/api-paste.ini diff --git a/unit_tests/test_neutron_ovs_context.py b/unit_tests/test_neutron_ovs_context.py index 5f994a0f..38de206f 100644 --- a/unit_tests/test_neutron_ovs_context.py +++ b/unit_tests/test_neutron_ovs_context.py @@ -1027,31 +1027,3 @@ class TestFirewallDriver(CharmTestCase): self.lsb_release.return_value = _LSB_RELEASE_XENIAL self.assertEqual(context._get_firewall_driver(ctxt), context.IPTABLES_HYBRID) - - -class TestHostIPContext(CharmTestCase): - - def setUp(self): - super(TestHostIPContext, self).setUp(context, TO_PATCH) - self.config.side_effect = self.test_config.get - - @patch.object(context.os_utils, 'os_release') - @patch.object(context.socket, 'getfqdn') - @patch.object(context, 'kv') - @patch.object(context, 'get_relation_ip') - def test_host_ip_context(self, _get_relation_ip, _kv, _getfqdn, - _os_release): - _os_release.return_value = 'stein' - _kv.return_value = {'install_version': 0} - _getfqdn.return_value = 'some' - ctxt = context.HostIPContext() - self.assertDictEqual({}, ctxt()) - _getfqdn.return_value = 'some.hostname' - ctxt = context.HostIPContext() - self.assertDictEqual({}, ctxt()) - _kv.return_value = {'neutron-ovs-charm-use-fqdn': True} - ctxt = context.HostIPContext() - self.assertDictEqual({'host': 'some.hostname'}, ctxt()) - _os_release.return_value = 'rocky' - ctxt = context.HostIPContext() - self.assertDictEqual({}, ctxt()) diff --git a/unit_tests/test_neutron_ovs_hooks.py b/unit_tests/test_neutron_ovs_hooks.py index 82b003bd..4ecbe1f9 100644 --- a/unit_tests/test_neutron_ovs_hooks.py +++ b/unit_tests/test_neutron_ovs_hooks.py @@ -18,7 +18,8 @@ from test_utils import CharmTestCase with patch('charmhelpers.core.hookenv.config') as config: config.return_value = 'neutron' - import neutron_ovs_utils as utils + with patch('charmhelpers.contrib.openstack.context.HostInfoContext'): + import neutron_ovs_utils as utils _reg = utils.register_configs _map = utils.restart_map @@ -82,8 +83,7 @@ class NeutronOVSHooksTests(CharmTestCase): _os_release.return_value = 'stein' self._call_hook('install') _kv.assert_called_once_with() - fake_dict.set.assert_called_once_with( - 'neutron-ovs-charm-use-fqdn', True) + fake_dict.set.assert_called_once_with(hooks.USE_FQDN_KEY, True) @patch('neutron_ovs_hooks.enable_sriov', MagicMock(return_value=False)) @patch.object(hooks, 'restart_map') @@ -183,13 +183,15 @@ class NeutronOVSHooksTests(CharmTestCase): 'libnetfilter-log1', 'keepalived']) - @patch.object(hooks.neutron_ovs_context, 'HostIPContext') - def test_neutron_plugin_joined_dvr_dhcp(self, _HostIPContext): + @patch.object(hooks, 'use_fqdn_hint') + @patch.object(hooks.os_context, 'HostInfoContext') + def test_neutron_plugin_joined_dvr_dhcp( + self, _HostInfoContext, _use_fqdn_hint): self.enable_nova_metadata.return_value = True self.enable_local_dhcp.return_value = True self.use_dvr.return_value = True self.get_shared_secret.return_value = 'secret' - _HostIPContext()().get.return_value = 'fq.dn' + _HostInfoContext()().__getitem__.return_value = 'fq.dn' self._call_hook('neutron-plugin-relation-joined') rel_data = { 'metadata-shared-secret': 'secret', @@ -201,13 +203,15 @@ class NeutronOVSHooksTests(CharmTestCase): ) self.assertTrue(self.install_packages.called) - @patch.object(hooks.neutron_ovs_context, 'HostIPContext') - def test_neutron_plugin_joined_dvr_nodhcp(self, _HostIPContext): + @patch.object(hooks, 'use_fqdn_hint') + @patch.object(hooks.os_context, 'HostInfoContext') + def test_neutron_plugin_joined_dvr_nodhcp( + self, _HostInfoContext, _use_fqdn_hint): self.enable_nova_metadata.return_value = True self.enable_local_dhcp.return_value = False self.use_dvr.return_value = True self.get_shared_secret.return_value = 'secret' - _HostIPContext()().get.return_value = 'fq.dn' + _HostInfoContext()().__getitem__.return_value = 'fq.dn' self._call_hook('neutron-plugin-relation-joined') rel_data = { 'metadata-shared-secret': 'secret', @@ -220,13 +224,15 @@ class NeutronOVSHooksTests(CharmTestCase): self.purge_packages.assert_called_with(['neutron-dhcp-agent']) self.assertFalse(self.install_packages.called) - @patch.object(hooks.neutron_ovs_context, 'HostIPContext') - def test_neutron_plugin_joined_nodvr_nodhcp(self, _HostIPContext): + @patch.object(hooks, 'use_fqdn_hint') + @patch.object(hooks.os_context, 'HostInfoContext') + def test_neutron_plugin_joined_nodvr_nodhcp( + self, _HostInfoContext, _use_fqdn_hint): self.enable_nova_metadata.return_value = False self.enable_local_dhcp.return_value = False self.use_dvr.return_value = False self.get_shared_secret.return_value = 'secret' - _HostIPContext()().get.return_value = 'fq.dn' + _HostInfoContext()().__getitem__.return_value = 'fq.dn' self._call_hook('neutron-plugin-relation-joined') rel_data = { 'metadata-shared-secret': None, diff --git a/unit_tests/test_neutron_ovs_utils.py b/unit_tests/test_neutron_ovs_utils.py index a0defe67..30b693f4 100644 --- a/unit_tests/test_neutron_ovs_utils.py +++ b/unit_tests/test_neutron_ovs_utils.py @@ -1101,6 +1101,13 @@ class TestNeutronOVSUtils(CharmTestCase): ) self.service_restart.assert_called_with('openvswitch-switch') + @patch.object(nutils, 'kv') + def test_use_fqdn_hint(self, _kv): + _kv().get.return_value = False + self.assertEquals(nutils.use_fqdn_hint(), False) + _kv().get.return_value = True + self.assertEquals(nutils.use_fqdn_hint(), True) + class TestDPDKBridgeBondMap(CharmTestCase):