From 8d81e415765d82ae8ee7640a5a89572d92b72b47 Mon Sep 17 00:00:00 2001 From: Billy Olsen Date: Thu, 24 Aug 2017 20:53:21 -0700 Subject: [PATCH] Change MAAS DNS OCF to read IP from file The MAAS DNS ocf resource is created specifying the IP address from the leader node. When the resource is moved to another node the IP address is not updated because the DNS record already points to the leader's IP address. This means that the DNS record will never be updated with any other unit's IP address when the resource is moved around the cluster. The IP address to use for the DNS record should be stored in the pacemaker resource as the configuration is the same across the entire cluster. This change makes it so that the IP address that should be bound is written to a file in /etc/maas_dns/$resource_name and used by the ocf:maas:dns resource when managing the DNS resource records. Migration is handled by checking the current version of the MAAS OCF resource file and determining if the OCF_RESOURCE_INSTANCE (the name of the resource) is present in the file. Change-Id: If4e07079dd66dac51cd77c2600106b9b562c2483 Closes-Bug: #1711476 --- hooks/hooks.py | 32 ++++++++++++++- hooks/utils.py | 40 ++++++++++++++++++ ocf/maas/dns | 66 +++++++++++++++++++++++++++--- unit_tests/test_hacluster_hooks.py | 49 ++++++++++++++++++++-- unit_tests/test_hacluster_utils.py | 27 ++++++++++++ 5 files changed, 204 insertions(+), 10 deletions(-) diff --git a/hooks/hooks.py b/hooks/hooks.py index 83e3916..7f9b5ea 100755 --- a/hooks/hooks.py +++ b/hooks/hooks.py @@ -69,6 +69,7 @@ from utils import ( disable_lsb_services, disable_upstart_services, get_ipv6_addr, + get_ip_addr_from_resource_params, validate_dns_ha, setup_maas_api, setup_ocf_files, @@ -77,6 +78,8 @@ from utils import ( kill_legacy_ocf_daemon_process, try_pcmk_wait, maintenance_mode, + needs_maas_dns_migration, + write_maas_dns_address, ) from charmhelpers.contrib.charmsupport import nrpe @@ -174,10 +177,34 @@ def config_changed(): maintenance_mode(cfg['maintenance-mode']) +def migrate_maas_dns(): + """ + Migrates the MAAS DNS HA configuration to write local IP address + information to files. + """ + if not needs_maas_dns_migration(): + log("MAAS DNS migration is not necessary.", INFO) + return + + for relid in relation_ids('ha'): + for unit in related_units(relid): + resources = parse_data(relid, unit, 'resources') + resource_params = parse_data(relid, unit, 'resource_params') + + if True in [ra.startswith('ocf:maas') + for ra in resources.values()]: + for resource in resource_params.keys(): + if resource.endswith("_hostname"): + res_ipaddr = get_ip_addr_from_resource_params( + resource_params[resource]) + log("Migrating MAAS DNS resource %s" % resource, INFO) + write_maas_dns_address(resource, res_ipaddr) + + @hooks.hook() def upgrade_charm(): install() - + migrate_maas_dns() update_nrpe_config() @@ -258,10 +285,13 @@ def ha_relation_changed(): # credentials for resource in resource_params.keys(): if resource.endswith("_hostname"): + res_ipaddr = get_ip_addr_from_resource_params( + resource_params[resource]) resource_params[resource] += ( ' maas_url="{}" maas_credentials="{}"' ''.format(config('maas_url'), config('maas_credentials'))) + write_maas_dns_address(resource, res_ipaddr) else: msg = ("DNS HA is requested but maas_url " "or maas_credentials are not set") diff --git a/hooks/utils.py b/hooks/utils.py index 80dffbb..ed57d08 100644 --- a/hooks/utils.py +++ b/hooks/utils.py @@ -110,6 +110,9 @@ SYSTEMD_OVERRIDES_DIR = '/etc/systemd/system/{}.service.d' SYSTEMD_OVERRIDES_FILE = '{}/overrides.conf' +MAAS_DNS_CONF_DIR = '/etc/maas_dns' + + class MAASConfigIncomplete(Exception): pass @@ -561,6 +564,16 @@ def configure_cluster_global(): pcmk.commit(cmd) +def get_ip_addr_from_resource_params(params): + """Returns the IP address in the resource params provided + + :return: the IP address in the params or None if not found + """ + reg_ex = r'.* ip_address="([a-fA-F\d\:\.]+)".*' + res = re.search(reg_ex, params) + return res.group(1) if res else None + + def restart_corosync_on_change(): """Simple decorator to restart corosync if any of its config changes""" def wrap(f): @@ -710,6 +723,33 @@ def setup_ocf_files(): rsync('ocf/maas/maasclient/', '/usr/lib/heartbeat/maasclient/') +def write_maas_dns_address(resource_name, resource_addr): + """Writes the specified IP address to the resource file for MAAS dns. + + :param resource_name: the name of the resource the address belongs to. + This is the name of the file that will be written in /etc/maas_dns. + :param resource_addr: the IP address for the resource. This will be + written to the resource_name file. + """ + mkdir(MAAS_DNS_CONF_DIR) + write_file(os.path.join(MAAS_DNS_CONF_DIR, resource_name), + content=resource_addr) + + +def needs_maas_dns_migration(): + """Determines if the MAAS DNS ocf resources need migration. + + :return: True if migration is necessary, False otherwise. + """ + try: + subprocess.check_call(['grep', 'OCF_RESOURCE_INSTANCE', + '/usr/lib/ocf/resource.d/maas/dns']) + return True + except subprocess.CalledProcessError: + # check_call will raise an exception if grep doesn't find the string + return False + + def is_in_standby_mode(node_name): """Check if node is in standby mode in pacemaker diff --git a/ocf/maas/dns b/ocf/maas/dns index 842c668..c55b961 100755 --- a/ocf/maas/dns +++ b/ocf/maas/dns @@ -42,6 +42,7 @@ # OCF_RESKEY_ttl # OCF_RESKEY_maas_url # OCF_RESKEY_maas_credentials +# OCF_RESKEY_cfg_dir # Defaults @@ -53,6 +54,28 @@ Expects to have a fully populated OCF RA-compliant environment set. END } +# Retrieve the local IP for the current resource +# +# returns: +# ip address contained in $OCF_RESKEY_cfg_dir/$OCF_RESOURCE_INSTANCE +# no = nothing or no file +my_ip() { + if [ ! -r $ipaddrfile ] + then + echo "no" + return 0 + fi + + ip_addr=`cat $OCF_RESKEY_cfg_dir/$OCF_RESOURCE_INSTANCE` + if [ "x$ip_addr" != "x" ] + then + echo $ip_addr + return 0 + else + echo "no" + return 0 + fi +} # Do we already serve this IP address on the given $NIC? # @@ -66,7 +89,17 @@ dns_served() { target=`dig +short $OCF_RESKEY_fqdn` if [ "x$target" != "x" ] then - if test "$OCF_RESKEY_ip_address" = "$target" + ip_address=`my_ip` + # The $ip_address should be set as it is ensured in the validate + # function, but this is a sanity check. The maas_dns.log file should + # contain the error that the $ip_address file is not found. + if test "$ip_address" = "no" + then + echo "no" + return 0 + fi + + if test "$ip_address" = "$target" then echo "ok" return 0 @@ -95,8 +128,13 @@ maas_dns_start() { if [ "$dns_status" = "ok" ]; then exit $OCF_SUCCESS fi + local myipaddr=`my_ip` + if [ "$myipaddr" = "no" ]; then + ocf_log err "No ip address found in $ipaddrfile" + exit $OCF_ERR_GENERIC + fi - cmd="python3 $binfile --fqdn=$OCF_RESKEY_fqdn --ip_address=$OCF_RESKEY_ip_address --maas_server=$OCF_RESKEY_maas_url --maas_credentials=$OCF_RESKEY_maas_credentials " + cmd="python3 $binfile --fqdn=$OCF_RESKEY_fqdn --ip_address=$myipaddr --maas_server=$OCF_RESKEY_maas_url --maas_credentials=$OCF_RESKEY_maas_credentials " if [ -n "$OCF_RESKEY_ttl" ]; then cmd="$cmd --ttl=$OCF_RESKEY_ttl" fi @@ -133,7 +171,6 @@ maas_dns_monitor() { return $OCF_SUCCESS ;; no) - exit 7 exit $OCF_NOT_RUNNING ;; *) @@ -148,6 +185,7 @@ binfile="$HA_BIN/maas_dns.py" logfile="$OCF_RESKEY_logfile" errlogfile="$OCF_RESKEY_errlogfile" user="$OCF_RESKEY_user" +ipaddrfile="$OCF_RESKEY_cfg_dir/$OCF_RESOURCE_INSTANCE" [ -z "$user" ] && user=root maas_dns_validate() { @@ -171,6 +209,16 @@ maas_dns_validate() { } fi done + if [ ! -r $ipaddrfile ] + then + ocf_log err "$ipaddrfile does not exist or cannot be read" + exit $OCF_ERR_INSTALLED + fi + if [ `my_ip` = "no" ] + then + ocf_log err "IP address is not found in $ipaddrfile" + exit $OCF_ERR_INSTALLED + fi return $OCF_SUCCESS } @@ -192,9 +240,9 @@ The fully qualified domain name for the DNS entry. Fully qualified domain name - + -The IP address for the DNS entry +The IP address for the DNS entry. Deprecated option, do not use. IP Address @@ -227,6 +275,14 @@ File to write STDERR to File to write STDERR to + + +Directory containing resource config files containing IP address information +for the resource running on the local server. + +IP address config file directory + + diff --git a/unit_tests/test_hacluster_hooks.py b/unit_tests/test_hacluster_hooks.py index 609401b..f9d9eb0 100644 --- a/unit_tests/test_hacluster_hooks.py +++ b/unit_tests/test_hacluster_hooks.py @@ -32,6 +32,7 @@ class TestCorosyncConf(unittest.TestCase): def setUp(self): self.tmpdir = tempfile.mkdtemp() + @mock.patch.object(hooks, 'write_maas_dns_address') @mock.patch('pcmk.wait_for_pcmk') @mock.patch.object(hooks, 'peer_units') @mock.patch('pcmk.crm_opt_exists') @@ -54,7 +55,7 @@ class TestCorosyncConf(unittest.TestCase): configure_stonith, configure_monitor_host, configure_cluster_global, configure_corosync, oldest_peer, crm_opt_exists, peer_units, - wait_for_pcmk): + wait_for_pcmk, write_maas_dns_address): crm_opt_exists.return_value = False oldest_peer.return_value = True related_units.return_value = ['ha/0', 'ha/1', 'ha/2'] @@ -90,6 +91,7 @@ class TestCorosyncConf(unittest.TestCase): configure_monitor_host.assert_called_with() configure_cluster_global.assert_called_with() configure_corosync.assert_called_with() + write_maas_dns_address.assert_not_called() for kw, key in [('location', 'locations'), ('clone', 'clones'), @@ -108,6 +110,7 @@ class TestCorosyncConf(unittest.TestCase): commit.assert_any_call( 'crm -w -F configure %s %s %s' % (kw, name, params)) + @mock.patch.object(hooks, 'write_maas_dns_address') @mock.patch.object(hooks, 'setup_maas_api') @mock.patch.object(hooks, 'validate_dns_ha') @mock.patch('pcmk.wait_for_pcmk') @@ -135,7 +138,7 @@ class TestCorosyncConf(unittest.TestCase): configure_corosync, oldest_peer, crm_opt_exists, peer_units, wait_for_pcmk, validate_dns_ha, - setup_maas_api): + setup_maas_api, write_maas_dns_addr): validate_dns_ha.return_value = True crm_opt_exists.return_value = False oldest_peer.return_value = True @@ -158,7 +161,9 @@ class TestCorosyncConf(unittest.TestCase): 'groups': {'grp_foo': 'res_foo'}, 'colocations': {'co_foo': 'inf: grp_foo cl_foo'}, 'resources': {'res_foo_hostname': 'ocf:maas:dns'}, - 'resource_params': {'res_foo_hostname': 'params bar'}, + 'resource_params': { + 'res_foo_hostname': 'params bar ' + 'ip_address="172.16.0.1"'}, 'ms': {'ms_foo': 'res_foo meta notify=true'}, 'orders': {'foo_after': 'inf: res_foo ms_foo'}} @@ -170,10 +175,12 @@ class TestCorosyncConf(unittest.TestCase): hooks.ha_relation_changed() self.assertTrue(validate_dns_ha.called) self.assertTrue(setup_maas_api.called) + write_maas_dns_addr.assert_called_with('res_foo_hostname', + '172.16.0.1') # Validate maas_credentials and maas_url are added to params commit.assert_any_call( 'crm -w -F configure primitive res_foo_hostname ocf:maas:dns ' - 'params bar maas_url="http://maas/MAAAS/" ' + 'params bar ip_address="172.16.0.1" maas_url="http://maas/MAAAS/" ' 'maas_credentials="secret"') @mock.patch.object(hooks, 'setup_maas_api') @@ -279,3 +286,37 @@ class TestHooks(test_utils.CharmTestCase): self.test_config.set('maintenance-mode', False) hooks.config_changed() mock_maintenance_mode.assert_called_with(False) + + @mock.patch.object(hooks, 'needs_maas_dns_migration') + @mock.patch.object(hooks, 'relation_ids') + def test_migrate_maas_dns_no_migration(self, relation_ids, + needs_maas_dns_migration): + needs_maas_dns_migration.return_value = False + hooks.migrate_maas_dns() + relation_ids.assert_not_called() + + @mock.patch.object(hooks, 'needs_maas_dns_migration') + @mock.patch.object(hooks, 'write_maas_dns_address') + @mock.patch.object(hooks, 'relation_ids') + @mock.patch.object(hooks, 'related_units') + @mock.patch.object(hooks, 'parse_data') + def test_migrate_maas_dns_(self, parse_data, related_units, relation_ids, + write_maas_dns_address, + needs_maas_dns_migration): + needs_maas_dns_migration.return_value = True + related_units.return_value = 'keystone/0' + relation_ids.return_value = 'ha:4' + + def mock_parse_data(relid, unit, key): + if key == 'resources': + return {'res_keystone_public_hostname': 'ocf:maas:dns'} + elif key == 'resource_params': + return {'res_keystone_public_hostname': + 'params fqdn="keystone.maas" ip_address="172.16.0.1"'} + else: + raise KeyError("unexpected key {}".format(key)) + + parse_data.side_effect = mock_parse_data + hooks.migrate_maas_dns() + write_maas_dns_address.assert_called_with( + "res_keystone_public_hostname", "172.16.0.1") diff --git a/unit_tests/test_hacluster_utils.py b/unit_tests/test_hacluster_utils.py index 4cbfb79..701a810 100644 --- a/unit_tests/test_hacluster_utils.py +++ b/unit_tests/test_hacluster_utils.py @@ -381,3 +381,30 @@ class UtilsTestCase(unittest.TestCase): utils.maintenance_mode(False) mock_get_property.assert_called_with('maintenance-mode') mock_set_property.assert_not_called() + + @mock.patch('subprocess.check_call') + def test_needs_maas_dns_migration(self, check_call): + ret = utils.needs_maas_dns_migration() + self.assertEqual(True, ret) + + check_call.side_effect = subprocess.CalledProcessError(1, '') + ret = utils.needs_maas_dns_migration() + self.assertEqual(False, ret) + + def test_get_ip_addr_from_resource_params(self): + param_str = 'params fqdn="keystone.maas" ip_address="{}" ' + for addr in ("172.16.0.4", "2001:0db8:85a3:0000:0000:8a2e:0370:7334"): + ip = utils.get_ip_addr_from_resource_params(param_str.format(addr)) + self.assertEqual(addr, ip) + + ip = utils.get_ip_addr_from_resource_params("no_ip_addr") + self.assertEqual(None, ip) + + @mock.patch.object(utils, 'write_file') + @mock.patch.object(utils, 'mkdir') + def test_write_maas_dns_address(self, mkdir, write_file): + utils.write_maas_dns_address("res_keystone_public_hostname", + "172.16.0.1") + mkdir.assert_called_once_with("/etc/maas_dns") + write_file.assert_called_once_with( + "/etc/maas_dns/res_keystone_public_hostname", content="172.16.0.1")