diff --git a/hooks/keystone_utils.py b/hooks/keystone_utils.py index 2db8becc..8cfdd145 100644 --- a/hooks/keystone_utils.py +++ b/hooks/keystone_utils.py @@ -2054,7 +2054,35 @@ def assess_status(configs): os_application_version_set(VERSION_PACKAGE) -def assess_status_func(configs): +def get_managed_services_and_ports(): + """Get the services and ports managed by this charm. + + Return only the services and corresponding ports that are managed by this + charm. This excludes haproxy when there is a relation with hacluster. This + is because this charm passes responsability for stopping and starting + haproxy to hacluster. + + Similarly, if a relation with hacluster exists then the ports returned by + this method correspond to those managed by the apache server rather than + haproxy. + + :returns: A tuple containing a list of services first followed by a list of + ports. + :rtype: Tuple[List[str], List[int]] + """ + _services = services() + _ports = determine_ports() + if relation_ids('ha'): + try: + _services.remove('haproxy') + except ValueError: + pass + _ports = [determine_api_port(api_port(space), singlenode_mode=True) + for space in ('keystone-admin', 'keystone-public')] + return _services, _ports + + +def assess_status_func(configs, exclude_ha_resource=False): """Helper function to create the function that will assess_status() for the unit. Uses charmhelpers.contrib.openstack.utils.make_assess_status_func() to @@ -2071,11 +2099,12 @@ def assess_status_func(configs): """ required_interfaces = REQUIRED_INTERFACES.copy() required_interfaces.update(get_optional_interfaces()) + _services, _ports = get_managed_services_and_ports() return make_assess_status_func( configs, required_interfaces, charm_func=check_optional_relations, - services=services(), - ports=determine_ports()) + services=_services, + ports=_ports) def get_file_stored_domain_id(backing_file): @@ -2118,9 +2147,10 @@ def _pause_resume_helper(f, configs): @param f: the function to be used with the assess_status(...) function @returns None - this function is executed for its side-effect """ + _services, _ports = get_managed_services_and_ports() f(assess_status_func(configs), - services=services(), - ports=determine_ports()) + services=_services, + ports=_ports) def post_snap_install(): diff --git a/unit_tests/test_keystone_utils.py b/unit_tests/test_keystone_utils.py index 83792b2f..e24ab7da 100644 --- a/unit_tests/test_keystone_utils.py +++ b/unit_tests/test_keystone_utils.py @@ -874,25 +874,22 @@ class TestKeystoneUtils(CharmTestCase): @patch.object(utils, 'get_optional_interfaces') @patch.object(utils, 'REQUIRED_INTERFACES') @patch.object(utils, 'check_optional_relations') - @patch.object(utils, 'services') - @patch.object(utils, 'determine_ports') + @patch.object(utils, 'get_managed_services_and_ports') @patch.object(utils, 'make_assess_status_func') def test_assess_status_func(self, make_assess_status_func, - determine_ports, - services, + get_managed_services_and_ports, check_optional_relations, REQUIRED_INTERFACES, get_optional_interfaces): - services.return_value = 's1' - determine_ports.return_value = 'p1' + get_managed_services_and_ports.return_value = (['s1'], [200]) REQUIRED_INTERFACES.copy.return_value = {'int': ['test 1']} get_optional_interfaces.return_value = {'opt': ['test 2']} utils.assess_status_func('test-config') make_assess_status_func.assert_called_once_with( 'test-config', {'int': ['test 1'], 'opt': ['test 2']}, - charm_func=check_optional_relations, services='s1', ports='p1') + charm_func=check_optional_relations, services=['s1'], ports=[200]) def test_pause_unit_helper(self): with patch.object(utils, '_pause_resume_helper') as prh: @@ -902,17 +899,50 @@ class TestKeystoneUtils(CharmTestCase): utils.resume_unit_helper('random-config') prh.assert_called_once_with(utils.resume_unit, 'random-config') - @patch.object(utils, 'services') - @patch.object(utils, 'determine_ports') - def test_pause_resume_helper(self, determine_ports, services): + @patch.object(utils, 'get_managed_services_and_ports') + def test_pause_resume_helper(self, get_managed_services_and_ports): f = MagicMock() - services.return_value = 's1' - determine_ports.return_value = 'p1' + get_managed_services_and_ports.return_value = (['s1'], [200]) with patch.object(utils, 'assess_status_func') as asf: asf.return_value = 'assessor' utils._pause_resume_helper(f, 'some-config') asf.assert_called_once_with('some-config') - f.assert_called_once_with('assessor', services='s1', ports='p1') + f.assert_called_once_with('assessor', services=['s1'], ports=[200]) + + @patch.object(utils, 'services') + @patch.object(utils, 'determine_ports') + @patch.object(utils, 'relation_ids') + @patch.object(utils, 'determine_api_port') + def test_get_managed_services_and_ports_no_ha(self, determine_api_port, + relation_ids, + determine_ports, + services): + services.return_value = ['apache2', 'haproxy'] + determine_ports.return_value = [80, 8080] + relation_ids.return_value = None + self.assertEqual( + utils.get_managed_services_and_ports(), + (['apache2', 'haproxy'], [80, 8080])) + + @patch.object(utils, 'services') + @patch.object(utils, 'determine_ports') + @patch.object(utils, 'relation_ids') + @patch.object(utils, 'determine_api_port') + def test_get_managed_services_and_ports(self, determine_api_port, + relation_ids, determine_ports, + services): + ports = { + 'keystone-admin': 5000, + 'keystone-public': 3535} + services.return_value = ['apache2', 'haproxy'] + determine_ports.return_value = [80, 8080] + self.api_port.return_value = 100 + determine_api_port.side_effect = lambda x, singlenode_mode: x-10 + self.api_port.side_effect = lambda x: ports.get(x) + relation_ids.return_value = ['rel:1'] + self.assertEqual( + utils.get_managed_services_and_ports(), + (['apache2'], [4990, 3525])) @patch.object(utils, 'run_in_apache') @patch.object(utils, 'restart_keystone')