OpenStack discoverer: Improve handling of LB and Listener names

[alexbrand]

We use the LBaaS listener name as the port name. When the name is long enough, it can result in an invalid port name.

time="2018-04-26T14:54:23Z" level=error msg="error handling update endpoints 'vioadmin/1-61fb2abd-f007-4dd2-8784-9db807b27e4d-openstack': Endpoints "1-61fb2abd-f007-4dd2-8784-9db807b27e4d-openstack" is invalid: subsets[1].ports[0].name: Invalid value: "k8s-registry-listener-a2fd7571-5091-4a1f-8a65-610250ce8877-11000": must be no more than 63 characters"

[alexbrand]

We must also ensure that we do not produce kubernetes identifier that have uppercase letters, as they are invalid according to the Kubernetes DNS_LABEL specification. We might have to lowercase any incoming name from OpenStack.

Additionally, Kubernetes label values are also subject to length constraints.

[alexbrand]

Working through this issue raised a couple of things in the OpenStack discoverer that we should improve.

Currently, the name of a discovered service is derived from the OpenStack load balancer ID and name as follows:

a) When the LB is unnamed, use the load balancer’s ID (UUID) as the service name and append the cluster name.
b) When the LB has a name, use the name and append the ID to produce a unique name. Append the cluster name.

Additionally, if the final name is longer than 63 characters, we attempt to shorten the name using a hash of the final name. If the source names are long enough, and we cannot shorten them to 63 chars, we simply use the hash as the final name.

EndpointPort names are derived in a similar manner, but we use the Load Balancer’s Listener’s name and port number.

Both service names and EndpointPort names must adhere to the Kubernetes DNS_LABEL spec, which allows a max of 63 chars and requires names to start with a letter, contain lowercase, alphanumeric characters (a-z and 0-9) and allows the - character anywhere except first and last position.

Following is a table that lists scenarios we don't handle well, the current outcome and proposed change:

Scenario	Current Outcome	Proposed Change
LB/Listener name starts with a number or contains invalid characters	Failure. The resulting service name is invalid per Kubernetes DNS_LABEL spec	Discoverer does not attempt to sync this load balancer. It will log an error and increase error metric. Add documentation around this requirement.
LB/Listener name contains uppercase letters	Failure. The resulting service name starts with a number (prohibited by DNS_LABEL spec)	Lowercase all names that come from OpenStack. When using names, we append IDs (or ports in Listener case), which avoids collisions
LB/Listener name is long enough that the hash must be used to produce service name, and the resulting hash starts with a number.	Failure. The name starts with a number (prohibited by DNS_LABEL spec)	I am not sure about this one. One possibility is to prepend the resulting hash with a known character whenever we have to use the hash as the name.

Label values are also subject to a similar set of requirements. Given that they are a superset of the DNS_LABEL reqs, we should be able to use the same approach we end up using for service names.

Thoughts? cc @stevesloka @davecheney @rosskukulinski

[stevesloka]

• I think we should document this in the user docs so it's clear of the issue.
• I'm not a fan of logging error and not sync-ing the LB. I think we should come up with a way to make the name fit a k8s service. We already add the full name to the label, so there's a good cross reference there.
  • Maybe prefix them all with ostk or something? Then add the other bits + truncate.

[alexbrand]

Definitely agree. Whatever solution we come up with, we should document this in the user documentation so that it is very clear what is going on.

I usually err on the side of predictability instead of "magic". With that said, prefixing the load balancer and listener names with a known value to make it fit into a k8s DNS_LABEL could work, and doesn't seem like that much magic (as long as we document it). Additionally, going down this route means that users don't have to rename pre-existing load balancers to fit our needs.

Happy to explore this option!

[alexbrand]

Prefixing the name works for cases where there are no invalid characters in the names (e.g. foo!). In that case, it seems like we would still have to consider this an invalid name and skip syncing the LB.

I am starting to wonder if we should always just use IDs regardless of whether the OpenStack resource has a name. With that said, we would still have somewhat of a similar issue when adding the load balancer name as a label, given that Kubernetes label values also have character and length restrictions.

[rosskukulinski]

Chatted with some openstack users:

1. UUID as primary name identifier is not friendly
2. no magic replacing invalid characters
3. prefix with discoverer-specific name (k8s, os, ec2) is a good thing
4. If invalid name, logs via stdout AND tenant-specific labeled prometheus metric would be sufficient

[alexbrand]

@rosskukulinski Thanks!

In yesterday's conversation we arrived at always using UUIDs.

Given point 1, should we reverse that decision and continue using name+UUID when the name is valid?