Fix performance issue when updating endpoints
This patch eliminates almost all the manager.py calls when updating/checking the endpoints from the relation(s) with other charms. Change-Id: Ibb7999239ec9927e76052b7e45c4545127b5919a Closes-Bug: #1890602
This commit is contained in:
Alex Kavanagh
parent
9e2d5cf2b7
commit
13f5ce49fe
|
|
@ -979,10 +979,13 @@ def delete_service_entry(service_name, service_type):
|
|||
log("Deleted service entry '{}'".format(service_name), level=DEBUG)
|
||||
|
||||
|
||||
def create_service_entry(service_name, service_type, service_desc, owner=None):
|
||||
def create_service_entry(service_name, service_type, service_desc, owner=None,
|
||||
list_services=None):
|
||||
""" Add a new service entry to keystone if one does not already exist """
|
||||
manager = get_manager()
|
||||
for service in manager.list_services():
|
||||
if list_services is None:
|
||||
list_services = manager.list_services()
|
||||
for service in list_services:
|
||||
if service['name'] == service_name:
|
||||
log("Service entry for '{}' already exists.".format(service_name),
|
||||
level=DEBUG)
|
||||
|
|
@ -995,24 +998,28 @@ def create_service_entry(service_name, service_type, service_desc, owner=None):
|
|||
|
||||
|
||||
def create_endpoint_template(region, service, publicurl, adminurl,
|
||||
internalurl):
|
||||
internalurl, list_endpoints=None):
|
||||
manager = get_manager()
|
||||
# this needs to be a round-trip to the manager.py script to discover what
|
||||
# the "current" api_version might be, as it can't just be asserted.
|
||||
if manager.resolved_api_version() == 2:
|
||||
create_endpoint_template_v2(manager, region, service, publicurl,
|
||||
adminurl, internalurl)
|
||||
adminurl, internalurl,
|
||||
list_endpoints=list_endpoints)
|
||||
else:
|
||||
create_endpoint_template_v3(manager, region, service, publicurl,
|
||||
adminurl, internalurl)
|
||||
adminurl, internalurl,
|
||||
list_endpoints=list_endpoints)
|
||||
|
||||
|
||||
def create_endpoint_template_v2(manager, region, service, publicurl, adminurl,
|
||||
internalurl):
|
||||
internalurl, list_endpoints=None):
|
||||
""" Create a new endpoint template for service if one does not already
|
||||
exist matching name *and* region """
|
||||
service_id = manager.resolve_service_id(service)
|
||||
for ep in manager.list_endpoints():
|
||||
if list_endpoints is None:
|
||||
list_endpoints = manager.list_endpoints()
|
||||
for ep in list_endpoints:
|
||||
if ep['service_id'] == service_id and ep['region'] == region:
|
||||
log("Endpoint template already exists for '%s' in '%s'"
|
||||
% (service, region))
|
||||
|
|
@ -1046,27 +1053,33 @@ def create_endpoint_template_v2(manager, region, service, publicurl, adminurl,
|
|||
|
||||
|
||||
def create_endpoint_template_v3(manager, region, service, publicurl, adminurl,
|
||||
internalurl):
|
||||
internalurl, list_endpoints=None):
|
||||
# //HERE
|
||||
service_id = manager.resolve_service_id(service)
|
||||
endpoints = {
|
||||
'public': publicurl,
|
||||
'admin': adminurl,
|
||||
'internal': internalurl,
|
||||
}
|
||||
if list_endpoints is None:
|
||||
list_endpoints = manager.list_endpoints()
|
||||
# Optimisation: we want to check the list of endpoints with the details and
|
||||
# ONLY create an endpoint if it doesn't exist and only update an existing
|
||||
# one if the details don't match. `list_endpoints` is a list of
|
||||
# dictionaries of the associated from manager.list_endpoints() as
|
||||
# serialised by to_dict()
|
||||
# NOTE: ep_type is 'interface' in python_keystoneclient 'speak'.
|
||||
for ep_type in endpoints.keys():
|
||||
# Delete endpoint if its has changed
|
||||
ep_deleted = manager.delete_old_endpoint_v3(
|
||||
ep_type,
|
||||
service_id,
|
||||
region,
|
||||
endpoints[ep_type]
|
||||
)
|
||||
ep_exists = manager.find_endpoint_v3(
|
||||
ep_type,
|
||||
service_id,
|
||||
region
|
||||
)
|
||||
if ep_deleted or not ep_exists:
|
||||
# see if the endpoint exists, but the URL doesn't match
|
||||
for ep in list_endpoints:
|
||||
if (ep['interface'] == ep_type and
|
||||
ep['service_id'] == service_id and ep['region'] == region):
|
||||
# if the url doesn't match then it needs to be modified
|
||||
if ep.get('url', None) != endpoints[ep_type]:
|
||||
manager.update_endpoint(ep['id'], url=endpoints[ep_type])
|
||||
break
|
||||
else:
|
||||
# Not found so create the endpoint
|
||||
manager.create_endpoint_by_type(
|
||||
region=region,
|
||||
service_id=service_id,
|
||||
|
|
@ -1856,11 +1869,13 @@ def add_service_to_keystone(relation_id=None, remote_unit=None):
|
|||
urllib.parse.urlparse(settings['admin_url']).hostname)
|
||||
else:
|
||||
endpoints = assemble_endpoints(settings)
|
||||
_list_services_result = manager.list_services()
|
||||
_list_endpoints_result = manager.list_endpoints()
|
||||
|
||||
services = []
|
||||
for ep in endpoints:
|
||||
# weed out any unrelated relation stuff Juju might have added
|
||||
# by ensuring each possible endpiont has appropriate fields
|
||||
# by ensuring each possible endpoint has appropriate fields
|
||||
# ['service', 'region', 'public_url', 'admin_url', 'internal_url']
|
||||
if single.issubset(endpoints[ep]):
|
||||
ep = endpoints[ep]
|
||||
|
|
@ -1868,7 +1883,9 @@ def add_service_to_keystone(relation_id=None, remote_unit=None):
|
|||
add_endpoint(region=ep['region'], service=ep['service'],
|
||||
publicurl=ep['public_url'],
|
||||
adminurl=ep['admin_url'],
|
||||
internalurl=ep['internal_url'])
|
||||
internalurl=ep['internal_url'],
|
||||
list_services=_list_services_result,
|
||||
list_endpoints=_list_endpoints_result)
|
||||
services.append(ep['service'])
|
||||
# NOTE(jamespage) internal IP for backwards compat for
|
||||
# SSL certs
|
||||
|
|
@ -2037,17 +2054,21 @@ def ensure_valid_service(service):
|
|||
return
|
||||
|
||||
|
||||
def add_endpoint(region, service, publicurl, adminurl, internalurl):
|
||||
def add_endpoint(region, service, publicurl, adminurl, internalurl,
|
||||
list_services=None,
|
||||
list_endpoints=None):
|
||||
status_message = 'Updating endpoint for {}'.format(service)
|
||||
log(status_message)
|
||||
status_set('maintenance', status_message)
|
||||
desc = valid_services[service]["desc"]
|
||||
service_type = valid_services[service]["type"]
|
||||
create_service_entry(service, service_type, desc)
|
||||
create_service_entry(service, service_type, desc,
|
||||
list_services=list_services)
|
||||
create_endpoint_template(region=region, service=service,
|
||||
publicurl=publicurl,
|
||||
adminurl=adminurl,
|
||||
internalurl=internalurl)
|
||||
internalurl=internalurl,
|
||||
list_endpoints=list_endpoints)
|
||||
|
||||
|
||||
def get_requested_roles(settings):
|
||||
|
|
|
|||
|
|
@ -403,6 +403,32 @@ class KeystoneManager3(KeystoneManager):
|
|||
self.api.endpoints.create(
|
||||
service_id, endpoint, interface=interface, region=region)
|
||||
|
||||
def update_endpoint(self, endpoint_id, service_id=None, url=None,
|
||||
interface=None, region=None, enabled=None, **kwargs):
|
||||
"""Update the endpoint by the endpoint_id.
|
||||
|
||||
Any param that is None is not changed.
|
||||
"""
|
||||
res = self.api.endpoints.update(
|
||||
endpoint_id, service_id, url, interface, region, enabled, **kwargs)
|
||||
return res.to_dict()
|
||||
|
||||
def find_endpoint_v3(self, interface, service_id, region):
|
||||
found_eps = []
|
||||
for ep in self.api.endpoints.list():
|
||||
if ep.service_id == service_id and ep.region == region and \
|
||||
ep.interface == interface:
|
||||
found_eps.append(ep)
|
||||
return [e.to_dict() for e in found_eps]
|
||||
|
||||
def delete_old_endpoint_v3(self, interface, service_id, region, url):
|
||||
eps = self.find_endpoint_v3(interface, service_id, region)
|
||||
for ep in eps:
|
||||
if ep.get('url', None) != url:
|
||||
self.api.endpoints.delete(ep['id'])
|
||||
return True
|
||||
return False
|
||||
|
||||
def tenants_list(self):
|
||||
return self.api.projects.list()
|
||||
|
||||
|
|
@ -555,24 +581,6 @@ class KeystoneManager3(KeystoneManager):
|
|||
if tenant:
|
||||
self.api.roles.grant(role, user=user, project=tenant)
|
||||
|
||||
def find_endpoint_v3(self, interface, service_id, region):
|
||||
found_eps = []
|
||||
for ep in self.api.endpoints.list():
|
||||
if ep.service_id == service_id and ep.region == region and \
|
||||
ep.interface == interface:
|
||||
found_eps.append(ep)
|
||||
return [e.to_dict() for e in found_eps]
|
||||
|
||||
def delete_old_endpoint_v3(self, interface, service_id, region, url):
|
||||
eps = self.find_endpoint_v3(interface, service_id, region)
|
||||
for ep in eps:
|
||||
# if getattr(ep, 'url') != url:
|
||||
if ep.get('url', None) != url:
|
||||
# self.api.endpoints.delete(ep.id)
|
||||
self.api.endpoints.delete(ep['id'])
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
# the following functions are proxied from keystone_utils, so that a Python3
|
||||
# charm can work with a Python2 keystone_client (i.e. in the case of a snap
|
||||
|
|
|
|||
|
|
@ -502,7 +502,9 @@ class TestKeystoneUtils(CharmTestCase):
|
|||
add_endpoint.assert_called_with(region='RegionOne', service='nova',
|
||||
publicurl='10.0.0.1',
|
||||
adminurl='10.0.0.2',
|
||||
internalurl='192.168.1.2')
|
||||
internalurl='192.168.1.2',
|
||||
list_services=ANY,
|
||||
list_endpoints=ANY)
|
||||
|
||||
@patch.object(utils, 'get_requested_roles')
|
||||
@patch.object(utils, 'create_service_credentials')
|
||||
|
|
@ -813,7 +815,8 @@ class TestKeystoneUtils(CharmTestCase):
|
|||
self.create_service_entry.assert_called_with(
|
||||
'nova',
|
||||
'compute',
|
||||
'Nova Compute Service')
|
||||
'Nova Compute Service',
|
||||
list_services=None)
|
||||
self.create_endpoint_template.asssert_called_with(
|
||||
region='RegionOne', service='nova',
|
||||
publicurl=publicurl, adminurl=adminurl,
|
||||
|
|
|
|||
Loading…
Reference in New Issue