*: use a trimmed, unique cluster name for ELB names #1277
Conversation
AWS ELB names are limited to 32 characters. The cluster name is used in the ELB name and can result in an ELB name that is too long. These changes generate a unique cluster name that is trimmed to fit the limits of ELB names. https://jira.coreos.com/browse/CORS-1004
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: staebler The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
openshift-ci-robot
added
the
size/L
|
/hold This is just a POC while the details of cloud tagging are finalized. |
openshift-ci-robot
added
the
do-not-merge/hold
|
@staebler: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
Also related is #1089. Do we want to pull that in too? |
| } | ||
| } | ||
|
|
||
| // Generate generates a random, unique cluster name |
There was a problem hiding this comment.
nit: missing a trailing period here and in some other godocs.
|
|
||
| variable "aws_unique_cluster_name" { | ||
| type = "string" | ||
| description = "The cluster name trimmed down and made unique" |
There was a problem hiding this comment.
This should probably include "The value will be at most 28 characters".
|
I'm fine with this, although it looks like you're missing a vendor for k8s.io/apimachinery/pkg/util/rand? I'm also fine recycling part of the cluster UUID here, since that's already a pool of unique bits associated with the cluster, and that would avoid the new dependency. Personally, I'd rather slant towards the UUID/random side over the name, and in this case that means I'd rather just use the first 28 characters of the UUID after removing hyphens. But I understand that other folks are more bothered by having to poke around in the AWS web UI to find human-readable tags, and are willing to take the slightly increased risk of collision in the smaller 5-character random identifier-space. |
|
@staebler |
| @@ -1,5 +1,5 @@ | |||
| resource "aws_lb" "api_internal" { | |||
| name = "${var.cluster_name}-int" | |||
| name = "${var.unique_cluster_name}-int" | |||
There was a problem hiding this comment.
I am not concerned with the risk of collision with only 5 random characters. The potential is only between clusters that have the same first 22 characters of their cluster name. And even when there is a collision, the result is a failed installation rather than a faulty cluster. 5 random characters is sufficient for kube proper as the default name generator. |
Yes, we should address hyphens as well. Thanks. I can add that after the plan for tagging is finalized. This PR was only meant as a POC. |
|
Superseded by #1280. |
4 participants
AWS ELB names are limited to 32 characters. The cluster name is used in the ELB name and can result in an ELB name that is too long. These changes generate a unique cluster name that is trimmed to fit
the limits of ELB names.
https://jira.coreos.com/browse/CORS-1004