From 27b84f5b1325ee75340ea55ad0b43b44a3097ba9 Mon Sep 17 00:00:00 2001 From: James Page Date: Fri, 27 Jan 2017 10:34:46 +0000 Subject: [PATCH] Add new subordinate relation for domain backends Support configuration of domains via suboridnate charms that implement the new 'keystone-domain-backend' relation type; these charms will create domain specific configuration files in /etc/keystone/domains, and will notify the keystone charm when configuration is complete, and the domain is ready for creation in the keystone database. Subordinate charms can also request a restart of keystone by setting or changing the value of the 'restart-nonce' key in the relation. Change-Id: Ia2b171e910d7f3a5e6e09ba5b18dddc0a734e57a Partial-Bug: 1645803 --- hooks/domain-backend-relation-changed | 1 + hooks/keystone_hooks.py | 47 +++++++++++ metadata.yaml | 3 + unit_tests/test_keystone_hooks.py | 112 +++++++++++++++++++++++++- 4 files changed, 159 insertions(+), 4 deletions(-) create mode 120000 hooks/domain-backend-relation-changed diff --git a/hooks/domain-backend-relation-changed b/hooks/domain-backend-relation-changed new file mode 120000 index 00000000..dd3b3eff --- /dev/null +++ b/hooks/domain-backend-relation-changed @@ -0,0 +1 @@ +keystone_hooks.py \ No newline at end of file diff --git a/hooks/keystone_hooks.py b/hooks/keystone_hooks.py index 383f2655..a4524713 100755 --- a/hooks/keystone_hooks.py +++ b/hooks/keystone_hooks.py @@ -22,6 +22,7 @@ import sys from subprocess import check_call from charmhelpers.contrib import unison +from charmhelpers.core import unitdata from charmhelpers.core.hookenv import ( Hooks, @@ -42,11 +43,13 @@ from charmhelpers.core.hookenv import ( status_set, network_get_primary_address, open_port, + is_leader, ) from charmhelpers.core.host import ( mkdir, service_pause, + service_restart, ) from charmhelpers.core.strutils import ( @@ -115,6 +118,8 @@ from keystone_utils import ( get_api_version, ADMIN_DOMAIN, ADMIN_PROJECT, + create_or_show_domain, + keystone_service, ) from charmhelpers.contrib.hahelpers.cluster import ( @@ -237,6 +242,7 @@ def config_changed_postupgrade(): initialise_pki() update_all_identity_relation_units() + update_all_domain_backends() # Ensure sync request is sent out (needed for any/all ssl change) send_ssl_sync_request() @@ -344,6 +350,13 @@ def update_all_identity_relation_units_force_sync(): update_all_identity_relation_units() +def update_all_domain_backends(): + """Re-trigger hooks for all domain-backend relations/units""" + for rid in relation_ids('domain-backend'): + for unit in related_units(rid): + domain_backend_changed(relation_id=rid, unit=unit) + + def leader_init_db_if_ready(use_current_context=False): """ Initialise the keystone db if it is ready and mark it as initialised. @@ -370,6 +383,7 @@ def leader_init_db_if_ready(use_current_context=False): # Ensure any existing service entries are updated in the # new database backend. Also avoid duplicate db ready check. update_all_identity_relation_units(check_db_ready=False) + update_all_domain_backends() @hooks.hook('shared-db-relation-changed') @@ -703,6 +717,39 @@ def admin_relation_changed(relation_id=None): relation_set(relation_id=relation_id, **relation_data) +@hooks.hook('domain-backend-relation-changed') +def domain_backend_changed(relation_id=None, unit=None): + if get_api_version() < 3: + log('Domain specific backend identity configuration only supported ' + 'with Keystone v3 API, skipping domain creation and ' + 'restart.') + return + + domain_name = relation_get(attribute='domain-name', + unit=unit, + rid=relation_id) + if domain_name: + # NOTE(jamespage): Only create domain data from lead + # unit when clustered and database + # is configured and created. + if is_leader() and is_db_ready() and is_db_initialised(): + create_or_show_domain(domain_name) + # NOTE(jamespage): Deployment may have multiple domains, + # with different identity backends so + # ensure that a domain specific nonce + # is checked for restarts of keystone + restart_nonce = relation_get(attribute='restart-nonce', + unit=unit, + rid=relation_id) + domain_nonce_key = 'domain-restart-nonce-{}'.format(domain_name) + db = unitdata.kv() + if restart_nonce != db.get(domain_nonce_key): + if not is_unit_paused_set(): + service_restart(keystone_service()) + db.set(domain_nonce_key, restart_nonce) + db.flush() + + @synchronize_ca_if_changed(fatal=True) def configure_https(): ''' diff --git a/metadata.yaml b/metadata.yaml index ff469b0f..d2f3f4fc 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -37,6 +37,9 @@ requires: ha: interface: hacluster scope: container + domain-backend: + interface: keystone-domain-backend + scope: container peers: cluster: interface: keystone-ha diff --git a/unit_tests/test_keystone_hooks.py b/unit_tests/test_keystone_hooks.py index 6cda610f..04653991 100644 --- a/unit_tests/test_keystone_hooks.py +++ b/unit_tests/test_keystone_hooks.py @@ -63,9 +63,11 @@ TO_PATCH = [ 'peer_echo', 'network_get_primary_address', 'open_port', + 'is_leader', # charmhelpers.core.host 'apt_install', 'apt_update', + 'service_restart', # charmhelpers.contrib.openstack.utils 'configure_installation_source', 'git_install_requested', @@ -89,6 +91,9 @@ TO_PATCH = [ 'update_nrpe_config', 'ensure_ssl_dirs', 'is_db_ready', + 'keystone_service', + 'create_or_show_domain', + 'get_api_version', # other 'check_call', 'execd_preinstall', @@ -105,6 +110,8 @@ TO_PATCH = [ 'service_pause', 'disable_package_apache_site', 'run_in_apache', + # unitdata + 'unitdata', ] @@ -341,6 +348,7 @@ class KeystoneRelationTests(CharmTestCase): configs.write.call_args_list) self.assertTrue(leader_init.called) + @patch.object(hooks, 'update_all_domain_backends') @patch.object(hooks, 'update_all_identity_relation_units') @patch.object(hooks, 'run_in_apache') @patch.object(hooks, 'is_db_initialised') @@ -380,7 +388,8 @@ class KeystoneRelationTests(CharmTestCase): mock_log, git_requested, mock_is_db_initialised, mock_run_in_apache, - update): + update, + mock_update_domains): mock_run_in_apache.return_value = False git_requested.return_value = False mock_is_ssl_cert_master.return_value = True @@ -404,7 +413,9 @@ class KeystoneRelationTests(CharmTestCase): self.open_port.assert_called_with(5000) self.assertTrue(update.called) + self.assertTrue(mock_update_domains.called) + @patch.object(hooks, 'update_all_domain_backends') @patch.object(hooks, 'update_all_identity_relation_units') @patch.object(hooks, 'run_in_apache') @patch.object(hooks, 'git_install_requested') @@ -436,7 +447,8 @@ class KeystoneRelationTests(CharmTestCase): ensure_ssl_dirs, mock_ensure_ssl_cert_master, mock_log, git_requested, - mock_run_in_apache, update): + mock_run_in_apache, update, + mock_update_domains): mock_run_in_apache.return_value = False git_requested.return_value = False mock_is_ssl_cert_master.return_value = True @@ -455,7 +467,9 @@ class KeystoneRelationTests(CharmTestCase): self.assertFalse(self.migrate_database.called) self.assertTrue(update.called) + self.assertTrue(mock_update_domains.called) + @patch.object(hooks, 'update_all_domain_backends') @patch.object(hooks, 'update_all_identity_relation_units') @patch.object(hooks, 'run_in_apache') @patch.object(hooks, 'is_db_initialised') @@ -494,7 +508,8 @@ class KeystoneRelationTests(CharmTestCase): mock_log, git_requested, mock_is_db_initialised, mock_run_in_apache, - update): + update, + mock_update_domains): mock_run_in_apache.return_value = False git_requested.return_value = False mock_is_ssl_cert_master.return_value = True @@ -519,7 +534,9 @@ class KeystoneRelationTests(CharmTestCase): self.assertTrue(configs.write_all.called) self.assertTrue(update.called) + self.assertTrue(mock_update_domains.called) + @patch.object(hooks, 'update_all_domain_backends') @patch.object(hooks, 'update_all_identity_relation_units') @patch.object(hooks, 'run_in_apache') @patch.object(hooks, 'initialise_pki') @@ -553,7 +570,8 @@ class KeystoneRelationTests(CharmTestCase): git_requested, mock_initialise_pki, mock_run_in_apache, - update): + update, + mock_update_domains): mock_run_in_apache.return_value = False git_requested.return_value = True mock_ensure_ssl_cert_master.return_value = False @@ -583,6 +601,7 @@ class KeystoneRelationTests(CharmTestCase): self.assertFalse(self.openstack_upgrade_available.called) self.assertFalse(self.do_openstack_upgrade_reexec.called) self.assertTrue(update.called) + self.assertTrue(mock_update_domains.called) @patch.object(hooks, 'run_in_apache') @patch.object(hooks, 'initialise_pki') @@ -1162,3 +1181,88 @@ class KeystoneRelationTests(CharmTestCase): peer_interface='cluster', ensure_local_user=True) self.assertTrue(self.log.called) self.assertFalse(update.called) + + def test_domain_backend_changed_v2(self): + self.get_api_version.return_value = 2 + hooks.domain_backend_changed() + self.assertTrue(self.get_api_version.called) + self.assertFalse(self.relation_get.called) + + def test_domain_backend_changed_incomplete(self): + self.get_api_version.return_value = 3 + self.relation_get.return_value = None + hooks.domain_backend_changed() + self.assertTrue(self.get_api_version.called) + self.relation_get.assert_called_with( + attribute='domain-name', + unit=None, + rid=None + ) + self.assertFalse(self.is_leader.called) + + @patch.object(hooks, 'is_unit_paused_set') + @patch.object(hooks, 'is_db_initialised') + def test_domain_backend_changed_complete(self, + is_db_initialised, + is_unit_paused_set): + self.get_api_version.return_value = 3 + self.relation_get.side_effect = ['mydomain', 'nonce2'] + self.is_leader.return_value = True + self.is_db_ready.return_value = True + is_db_initialised.return_value = True + mock_kv = MagicMock() + mock_kv.get.return_value = None + self.unitdata.kv.return_value = mock_kv + is_unit_paused_set.return_value = False + self.keystone_service.return_value = 'apache2' + + hooks.domain_backend_changed() + + self.assertTrue(self.get_api_version.called) + self.relation_get.assert_has_calls([ + call(attribute='domain-name', + unit=None, + rid=None), + call(attribute='restart-nonce', + unit=None, + rid=None), + ]) + self.create_or_show_domain.assert_called_with('mydomain') + self.service_restart.assert_called_with('apache2') + mock_kv.set.assert_called_with('domain-restart-nonce-mydomain', + 'nonce2') + self.assertTrue(mock_kv.flush.called) + + @patch.object(hooks, 'is_unit_paused_set') + @patch.object(hooks, 'is_db_initialised') + def test_domain_backend_changed_complete_follower(self, + is_db_initialised, + is_unit_paused_set): + self.get_api_version.return_value = 3 + self.relation_get.side_effect = ['mydomain', 'nonce2'] + self.is_leader.return_value = False + self.is_db_ready.return_value = True + is_db_initialised.return_value = True + mock_kv = MagicMock() + mock_kv.get.return_value = None + self.unitdata.kv.return_value = mock_kv + is_unit_paused_set.return_value = False + self.keystone_service.return_value = 'apache2' + + hooks.domain_backend_changed() + + self.assertTrue(self.get_api_version.called) + self.relation_get.assert_has_calls([ + call(attribute='domain-name', + unit=None, + rid=None), + call(attribute='restart-nonce', + unit=None, + rid=None), + ]) + # Only lead unit will create the domain + self.assertFalse(self.create_or_show_domain.called) + self.service_restart.assert_called_with('apache2') + mock_kv.set.assert_called_with('domain-restart-nonce-mydomain', + 'nonce2') + self.assertTrue(mock_kv.flush.called)