Refactor Keystone Endpoint Functions and Bootstrap Jobs For services #142
Conversation
common/templates/_endpoints.tpl
Outdated
| {{- $endpointType | quote -}} | ||
| {{- $endpointMap := index $context.Values.endpoints $type }} | ||
| {{- $endpointName := index $endpointMap "name" }} | ||
| {{- $endpointName | quote -}} |
There was a problem hiding this comment.
the | quote here needs to be removed as it causes issues when used in an environment variable.
| # this function returns the endpoint uri for a service, it takes an tuple | ||
| # input in the form: service-name, endpoint-class, port-name. eg: | ||
| # { tuple "heat" "public" "api" . | include "endpoint_addr_lookup" } | ||
| # input in the form: service-type, endpoint-class, port-name. eg: |
There was a problem hiding this comment.
We should include an example endpoint: definition and how the "default" host works and how it can be overridden with internal, public, and admin. We should also note the context - as its confusing for first-time helmers. This pulls from the calling cookbooks context, so these values.yaml definitions need to be there, not in common.
| # will return the appropriate URI. Once merged this should phase out the above. | ||
|
|
||
| {{- define "endpoint_addr_lookup" -}} | ||
| {{- $name := index . 0 -}} | ||
| {{- define "endpoint_type_lookup_addr" -}} |
There was a problem hiding this comment.
I'm thinking this really needs to be renamed -- I am thinking common.endpoint_lookup as it doesn't actually return an address, it returns a URL, and potentially a hostname or hostname:ip (below).
At first glance, this should support MySQL endpoints and with hostname only lookups (below) added, both rabbit, and memcache can move under endpoints (that should be our goal) -- meaning all lookups for all services done in one place.
We should also account for a CSV list of ips/hostnames/host/ip:ports supported - this would support k8s services, and a list in endpoints: of IPs or names.
I would also like to see a type of host and host_port supported, to use the same code path to simply return the host or the host:port unless we want two different functions. Seems like we don't need that as the lookup logic is exactly the same, only the printf differs. This completely removes the need for _hosts.tpl which I will be glad to rid of.
| {{- $fqdn := $context.Release.Namespace -}} | ||
| {{- if $context.Values.endpoints.fqdn -}} | ||
| {{- $fqdn := $context.Values.endpoints.fqdn -}} | ||
| {{- end -}} | ||
| {{- with $endpointMap -}} | ||
| {{- $endpointScheme := .scheme }} | ||
| {{- $endpointHost := index .hosts $endpoint | default .hosts.default}} | ||
| {{- $endpointHost := index .hosts $endpoint | default .hosts.default }} | ||
| {{- $endpointPort := index .port $port }} |
There was a problem hiding this comment.
We should be able to default here, shouldn't we? The port is in endpoints:
| {{- $endpointPath := .path }} | ||
| {{- printf "%s://%s.%s:%1.f%s" $endpointScheme $endpointHost $fqdn $endpointPort $endpointPath | quote -}} | ||
| {{- $endpointPath := .path | default "" }} | ||
| {{- printf "%s://%s.%s:%1.f%s" $endpointScheme $endpointHost $fqdn $endpointPort $endpointPath -}} | ||
| {{- end -}} | ||
| {{- end -}} | ||
|
|
||
|
|
||
| #------------------------------- |
There was a problem hiding this comment.
Couldn't the map live in common so it doesn't have to be unnecessarily duplicated in every charts endpoints: - it won't change much once its defined.
| @@ -0,0 +1,20 @@ | |||
| apiVersion: v1 | |||
There was a problem hiding this comment.
This whole thing seems generic - couldn't we pass .Values.keystone to a common template for this type of secret?
| @@ -130,7 +130,7 @@ SESSION_ENGINE = 'django.contrib.sessions.backends.cache' | |||
| CACHES = { | |||
| 'default': { | |||
| 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', | |||
| 'LOCATION': '{{ include "memcached_host" . }}' | |||
| 'LOCATION': '{{ .Values.memcached.host }}:{{ .Values.memcached.port }}' | |||
There was a problem hiding this comment.
I think memcache should be an endpoint like everything else.
| @@ -80,6 +79,8 @@ service: | |||
| proto: "http" | |||
|
|
|||
| database: | |||
There was a problem hiding this comment.
I think the database should be an endpoint like everything else.
| @@ -26,6 +26,10 @@ network: | |||
| node_port: 30000 | |||
| enable_node_port: false | |||
|
|
|||
| memcached: | |||
| @@ -4,7 +4,8 @@ use_syslog = False | |||
| use_stderr = True | |||
|
|
|||
| [database] | |||
| connection = mysql+pymysql://{{ .Values.database.keystone_user }}:{{ .Values.database.keystone_password }}@{{ include "keystone_db_host" . }}/{{ .Values.database.keystone_database_name }} | |||
| connection = mysql+pymysql://{{ .Values.database.keystone_user }}:{{ .Values.database.keystone_password }}@{{ .Values.database.address }}:{{ .Values.database.port }}/{{ .Values.database.keystone_database_name }} | |||
There was a problem hiding this comment.
Same. And if it was, you can use your new routine to generate this. Endpoint for dbs just gets expanded to have database username and passwords (path is already akin to dbname)
| @@ -0,0 +1,9 @@ | |||
| {{- $dbRootConnection := printf "mysql+pymysql://%s:%s@%s:%1.f" .Values.database.root_user .Values.database.root_password .Values.database.address .Values.database.port }} | |||
|
@intlabs Pete, I'd love to see this taken to completion as its huge for consistency. If you need to chunk any of it up, I'm happy to help. If you want, ignore the request to rope in databases and memcache and I'll take that on. |
|
@gardlt once some changes have been made and the conflicts are resolved...the plan is to pull this in (and even possibly break this up to get it in a big faster). |
…agement. Removes the dependency on Ansible for the creation and management of databases for OpenStack Services. It adds a small python script that utilizes PyMySQL via SQLAlchemy, two libraries that are present in every OpenStack Service Container regardless of origin.
Update the common lib to find endpoints by service type not name.
|
This is undergoing refactoring, and should not be merged until complete. |
|
This PR is being closed in favor of a series of smaller PR's beginning with #208, with smaller scopes that will be much more manageable to review. |
Removes the dependency on Ansible for the creation and management of Users, Services and Endpoints for OpenStack Services.
As part of this PR the Endpoint functions are updated from being based around service name to service type. This brings OpenStack Helm inline with other OpenStack Deployment tools and makes the management of versioned endpoints possible.
Builds on the following which should be reviewed/merged first:
The commits specific to this PR start at: "Update Common Lib Endpoint Functions" (currently 5000b66) If reviewed individually they are a lot less scary than would appear from the overall size of this PR.
This change is