Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make namespace auto-generated name more unique #501

Closed
wants to merge 2 commits into from
Closed

Make namespace auto-generated name more unique #501

wants to merge 2 commits into from

Conversation

Y0sh1dk
Copy link

@Y0sh1dk Y0sh1dk commented Dec 5, 2023

What this PR does / why we need it:
Bump the auto-generated namespace name words from 2 -> 4 to reduce the chance of a clash.

Fixes #500

Signed-off-by: Yosiah de Koeyer <[email protected]>
Signed-off-by: Yosiah de Koeyer <[email protected]>
@Y0sh1dk Y0sh1dk changed the title Feature/unique ns gen Make namespage auto-generated name more unique Dec 5, 2023
@Y0sh1dk Y0sh1dk changed the title Make namespage auto-generated name more unique Make namespace auto-generated name more unique Dec 5, 2023
Copy link

@ajith-anz ajith-anz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

Copy link
Member

@porridge porridge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution, but I have a concern inline. Even though I despise adding yet another knob, I'm afraid depending on the answer to the question there, we might have to make this configurable, and keep the current 2 as default. 🤔

@@ -385,7 +385,7 @@ func (t *Case) determineNamespace() *namespace {
}
// no preferred ns, means we auto-create with petnames
if t.PreferredNamespace == "" {
ns.Name = fmt.Sprintf("kuttl-test-%s", petname.Generate(2, "-"))
ns.Name = fmt.Sprintf("kuttl-test-%s", petname.Generate(4, "-"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm more than a little afraid we might blow through the limit of max namespace length because of this. 😟
Or, more likely, when the code under test takes the namespace name as input to generating names of cluster-scoped resources (by slapping some additional prefix on it). Does petname provide any guarantees on the max generated name length?

@porridge
Copy link
Member

An alternative would be to use a different string of similar length as the current one, but containing more entropy. I.e. use a string of random alphanumeric characters rather than petname. The k8s code which generates unique pod or replicaset names for deployments might serve as an example...

@Y0sh1dk Y0sh1dk closed this by deleting the head repository Apr 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto-generated namespace name clashes
3 participants