Generate broker container names more sensibly #16289
Conversation
Fixes an issue where container names for brokers could be longer than 63 characters: - If a broker image is referenced by tag, trim the digest to 10 chars and use that instead of the full thing - If the generated name is longer than 63 chars for whatever reason, trim it to allow the workspace to start. Signed-off-by: Angel Misevski <amisevsk@redhat.com>
amisevsk
added
the
kind/bug
amisevsk
requested review from
l0rd,
metlos,
rhopp and
sleshchenko
as code owners
che-bot
added
the
status/code-review
| @@ -247,7 +247,25 @@ private String generateUniqueName(String suffix) { | |||
| */ | |||
| @VisibleForTesting | |||
| protected String generateContainerNameFromImageRef(String image) { | |||
| return image.toLowerCase().replaceAll("[^/]*/", "").replaceAll("[^\\d\\w-]", "-"); | |||
| String containerName; | |||
There was a problem hiding this comment.
While this is certainly improving the situation, it is still not taking care of the possible clashes. What do you think can be done there?
There was a problem hiding this comment.
A container name can be up to 30 chars, if you want to use an image with a really long tag there's not much we can do that's intelligent. Apart from that we could choose a completely different tack for naming these containers, but that's potentially more risky and I would prefer that be a separate issue + PR to minimize risk in 7.9.x.
Are there any big cases I've missed in the PR? I admit I wrote it hastily.
There was a problem hiding this comment.
One solution would be to just name the containers "metadata-broker" and "artifacts-broker", though that obscures the version being used in the default use case (where it's currently something like che-plugin-metadata-broker-v3-1-0.
|
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1) ℹ️ |
|
crw-ci-test |
|
ci-test |
|
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1) ℹ️ |
che-bot
removed
the
status/code-review
What does this PR do?
Fixes an issue where container names for brokers could be longer than 63 characters:
What issues does this PR fix or reference?
Fixes #16288