Skip to content

How can I change the name of namespaces created by LocallyRunOperatorExtension? #2163

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

Closed
machenity opened this issue Dec 15, 2023 · 5 comments · Fixed by #2171
Closed

How can I change the name of namespaces created by LocallyRunOperatorExtension? #2163

machenity opened this issue Dec 15, 2023 · 5 comments · Fixed by #2171
Assignees
Milestone

Comments

@machenity
Copy link
Contributor

machenity commented Dec 15, 2023

Hi. I wrote some tests using LocallyRunOperatorExtension, but I encountered the following error:

io.fabric8.kubernetes.client.KubernetesClientException: Failure executing: POST at: 
https://xxxxxx.com/api/v1/namespaces. Message: namespaces "xxxxxxxxxoperatorsecuritye2e-xxxxxxxxxxxxxxxxxxxx" 
already exists. Received status: Status(apiVersion=v1, code=409, details=StatusDetails(causes=[], group=null, kind=namespaces, 
name=xxxxxxxxxoperatorsecuritye2e-xxxxxxxxxxxxxxxxxxxx, retryAfterSeconds=null, uid=null, additionalProperties={}), 
kind=Status, message=namespaces "xxxxxxxxxoperatorsecuritye2e-xxxxxxxxxxxxxxxxxxxx" already exists, 
metadata=ListMeta(_continue=null, remainingItemCount=null, resourceVersion=null, selfLink=null, additionalProperties={}), 
reason=AlreadyExists, status=Failure, additionalProperties={}).

(I have obscured some data that reveals personal information).

I suspected it was because the class name + test method name I use as the namespace name was longer than 63 characters, and the UUID part that ensures uniqueness, was truncated, causing two tests running in parallel to create a namespace with the same name.

Looking at the java-operator-sdk code, it appears that there is no configuration to provide a custom namespace name, or at least modify the prefix.

Maybe I missed that configuration?

Besides, if there really is no such configuration, I'd like to add such a feature to the JOSDK, if you don't mind.

@csviri csviri added this to the 4.7 milestone Dec 15, 2023
@csviri
Copy link
Collaborator

csviri commented Dec 15, 2023

Hi @machenity , yes this would definetelly would make sense to improve.
What I can also think of is to always reserve space for UUID at the end of the name for the namespace. Would that fix you problem?
We could also make this configurable, if really necessary, but rather just fix like that, it seems to fix the problem nicely and generically. WDYT?

@csviri csviri self-assigned this Dec 15, 2023
@csviri
Copy link
Collaborator

csviri commented Dec 15, 2023

@machenity if you would volunteer to do this would be nice :)

@machenity
Copy link
Contributor Author

Thanks for your answer :)

Actually first idea was adding an option accepts custom prefix string or string supplier for namespace.

It would be like this:

LocallyRunOperatorExtension.builder()
	.oneNamespacePerClass(false)
	.namespaceNamePrefix("NAMESPACE_NAME_PREFIX") // pass static string for prefix
	.withNamespaceNameSupplier(() -> "TEST" + new Random().nextInt().toString()) // pass Supplier<String> to call each time a namespace name is created
	.build()

But while reading your comments, your idea that truncating name earlier before postfix UUID seems nice.
Restrictive but quite simpler.

To be honest, I think having a 36-character UUID string in the name is overmuch for a 63-character limit. 🤔

Just wanna hear your opinion.

@csviri
Copy link
Collaborator

csviri commented Dec 18, 2023

To be honest, I think having a 36-character UUID string in the name is overmuch for a 63-character limit.

Yes, definetelly, this can be a very short sequence of random chars.

I can also imagine something like this (very similar to your sample):

LocallyRunOperatorExtension.builder()
   .withNamespaceNameSupplier(ExtensionContext context -> context.getTestMethod() + "-"+randomString() )

The point is that extension context is passed to access information about the test case (method name). Maybe method name would be just enough in this case, but for sake of generality, this might be a way to go

@csviri
Copy link
Collaborator

csviri commented Dec 18, 2023

So would be nice to do both, like able to specify namespace name supplier, but also fix this issue with to long names. (The late I would give priority, or in other words a better default supplier)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants