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

Rework Kubernetes resource naming and set up common unit tests #326

Merged
merged 4 commits into from
Jul 22, 2024

Conversation

lucas-koehler
Copy link
Contributor

Fixes #324

Note: This will need a rebase if the formatting PR #325 is merged first. Otherwise, the touched Java files need to be formatted in the PR, too.

Rework the naming of Kubernetes resources for workspaces and sessions in the common package used by operator and service.

The goal is to make resource names more readable and contain useful information. Resource names now contain a prefix, optional identifier, user, app definition and the last segment of the source CRs (session, workspace, app definition) UID.

Also configure unit tests using JUnit 5 for the common maven module and add some tests for the new name generation.


Notes:

  • Most suffixes and prefixes containing the resource type were removed because this is already given by the type.
  • The new name calculation limits names to length 57 to leave 6 characters for Kubernetes to append a hash to deployment pods. This is at least 5 characters plus a dash. If there is not enough space in the resource name, the end of the pod name is replaced with this. Thus, the limit of 57 characters to avoid this.
  • Changed the identifiers for the email and proxy config maps to email resp. proxy to be as short and concise as possible.

See an example of created resources for a user bar with app definition theia-cloud-demo:
image

Rework the naming of Kubernetes resources for workspaces and sessions
in the common package used by operator and service.

The goal is to make resource names more readable and contain useful information.
Resource names now contain a prefix, optional identifier, user, app definition and
the last segment of the source CRs (session, workspace, app definition) UID.

Also configure unit tests using JUnit 5 for the common maven module
and add some tests for the new name generation.
The setting from the java-formatter.xml does not seem to be respected.
Set the tab width explicitly via `editor.tabSize` to ensure this works independently of the VSCode settings regarding tab size.
@lucas-koehler
Copy link
Contributor Author

lucas-koehler commented Jul 19, 2024

@sgraband I rebased the changes and shortened the workspace prefix to ws. I did not improve the algorithm to use the space fully, yet because I'm out of time. I suggest doing this in a follow up so we can merge this already.
EDIT: Created the follow up here #327

Copy link
Contributor

@sgraband sgraband left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM 👍

@lucas-koehler lucas-koehler merged commit 76dd607 into main Jul 22, 2024
13 checks passed
@lucas-koehler lucas-koehler deleted the lk/k8s-resource-names branch July 22, 2024 07:28
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.

Improve Kubernetes resource naming
2 participants