-
Notifications
You must be signed in to change notification settings - Fork 87
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
Conversation
Signed-off-by: Yosiah de Koeyer <[email protected]>
Signed-off-by: Yosiah de Koeyer <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
There was a problem hiding this 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, "-")) |
There was a problem hiding this comment.
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?
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 |
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