Fix and Improve Eager Start Handling #393
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Fix endless loop and a NPE in the NamingUtil's methods for eager start name creation (based on an app definition) - Fix id extraction from generated names for the adapted name generation in an earlier PR.
Add an annotation to an eagerly started instance pod after the email config map was updated. This is necessary to trigger a sync with the Kubelet. Otherwise, the pod might not be updated with the new email list for the OAuth proxy in time. This is the case because ConfigMap changes are not propagated to the pod immediately but during a periodic sync.
- Rename EagerSessionStartHandler to EagerSessionHandler - Implement `sessionDeleted` method to clean up and make the instance available for another user. This includes restarting the pod to get a new temporary workspace.
Check whether max instances are reached by counting sessions relevant to the app definition that are not in error state. This makes it unnecessary to analyze all pods belonging to the session. Thus, performance for large amounts of pods is increased. K8sUtil#getPodForSession now finds a pod belonging to a session via owner references instead of ENV variables. With this, the pod can also be resolved in eager mode where the session name cannot be injected as an env variable. Also adds some logging to K8sUtil#reportPerformance and increases the log level for exceptions in the performance REST handler method.
jfaltermeier
requested changes
Jan 13, 2025
...ator/src/main/java/org/eclipse/theia/cloud/operator/handler/session/EagerSessionHandler.java
Show resolved
Hide resolved
- Extend LabelsUtil to add the session name to session labels - Find service from session via labels instead of owner references in eager session delete handler With this, the handling also works if the owner reference has been removed automatically - Add minor clarifying comments
|
Hi @jfaltermeier , I added one commit that should fix session delete handling even if dangling owner references are cleaned up automatically. Please have a look again :) |
jfaltermeier
approved these changes
Jan 14, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requires:
Summary
This pull request addresses critical issues in the eager start handling, which was previously broken, and introduces improvements to naming, session management, pod synchronization, and performance in Kubernetes environments.
Details
Common
Operator - Eager Session Handling
Service
Testing
Notes on Testing
minInstancesand currently lazy starting up until maxInstances is not supported (and maybe never will be). Use the CDT cloud app.