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

Fix and Improve Eager Start Handling #393

Merged
merged 7 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,47 @@
"vmArgs": "-Dlog4j2.configurationFile=log4j2.xml",
"preLaunchTask": "Build and Install Operator library"
},
{
// This works with terraform config 2-04_try-now_paths_eager_start
"type": "java",
"name": "Debug Operator (eager start try-now-paths)",
"request": "launch",
"cwd": "${workspaceFolder}/java/operator/org.eclipse.theia.cloud.defaultoperator",
"mainClass": "org.eclipse.theia.cloud.defaultoperator.DefaultTheiaCloudOperatorLauncher",
"args": [
"--keycloak",
"--keycloakURL",
"https://${input:minikubeIP}.nip.io/keycloak/",
"--keycloakRealm",
"TheiaCloud",
"--keycloakClientId",
"theia-cloud",
"--usePaths",
"--instancesPath",
"instances",
"--instancesHost",
"${input:minikubeIP}.nip.io",
"--serviceUrl",
"https://${input:minikubeIP}.nip.io/service",
"--cloudProvider",
"MINIKUBE",
"--sessionsPerUser",
"3",
"--appId",
"asdfghjkl",
"--storageClassName",
"default",
"--requestedStorage",
"250Mi",
"--bandwidthLimiter",
"WONDERSHAPER",
"--oAuth2ProxyVersion",
"v7.5.1",
"--eagerStart"
],
"vmArgs": "-Dlog4j2.configurationFile=log4j2.xml",
"preLaunchTask": "Build and Install Operator library"
},
{
// Attach to the service running (Task: Run Service)
"type": "java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.util.HashMap;
import java.util.Map;
import java.util.Set;

import org.eclipse.theia.cloud.common.k8s.resource.appdefinition.AppDefinitionSpec;
import org.eclipse.theia.cloud.common.k8s.resource.session.SessionSpec;
Expand All @@ -17,6 +18,7 @@ public class LabelsUtil {

public static final String LABEL_KEY_USER = LABEL_CUSTOM_PREFIX + "/user";
public static final String LABEL_KEY_APPDEF = LABEL_CUSTOM_PREFIX + "/app-definition";
public static final String LABEL_KEY_SESSION_NAME = LABEL_CUSTOM_PREFIX + "/session";

public static Map<String, String> createSessionLabels(SessionSpec sessionSpec,
AppDefinitionSpec appDefinitionSpec) {
Expand All @@ -26,6 +28,16 @@ public static Map<String, String> createSessionLabels(SessionSpec sessionSpec,
String sanitizedUser = sessionSpec.getUser().replaceAll("@", "_at_").replaceAll("[^a-zA-Z0-9]", "_");
labels.put(LABEL_KEY_USER, sanitizedUser);
labels.put(LABEL_KEY_APPDEF, appDefinitionSpec.getName());
labels.put(LABEL_KEY_SESSION_NAME, sessionSpec.getName());
return labels;
}

/**
* Returns the set of label keys that are specific to a specific session, i.e. the user and session name keys.
*
* @return The session specific label keys.
*/
public static Set<String> getSessionSpecificLabelKeys() {
return Set.of(LABEL_KEY_SESSION_NAME, LABEL_KEY_USER);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ public final class NamingUtil {

public static final char VALID_NAME_SUFFIX = 'z';

/**
* Prefix for names generated based on an app definition and an index. These names are typically used for objects
* related to eagerly started instance pods (e.g. services or deployments).
*/
public static final String APP_DEFINITION_INSTANCE_PREFIX = "instance-";

private static final Locale US_LOCALE = new Locale("en", "US");

private NamingUtil() {
Expand All @@ -41,7 +47,7 @@ private NamingUtil() {
* @see NamingUtil#createName(AppDefinition, int, String)
*/
public static String createName(AppDefinition appDefinition, int instance) {
return createName(appDefinition, instance);
return createName(appDefinition, instance, null);
}

/**
Expand All @@ -56,10 +62,10 @@ public static String createName(AppDefinition appDefinition, int instance) {
* same type for an AppDefinition.
* </p>
* <p>
* The created name contains a "session" prefix, the session's user and app definition, the identifier (if given),
* and the last segment of the Session's UID. User, app definition and identifier are shortened to keep the name
* within Kubernetes' character limit (63) minus 6 characters. The latter allows Kubernetes to add 6 characters at
* the end of deployment pod names while the pod names pod names will still contain the whole name of the deployment
* The created name contains a "instance-" prefix, instance number, the identifier (if given), and the last segment
* of the App Definition's UID. User, app definition and identifier are shortened to keep the name within
* Kubernetes' character limit (63) minus 6 characters. The latter allows Kubernetes to add 6 characters at the end
* of deployment pod names while the pod names pod names will still contain the whole name of the deployment
* </p>
*
* @param appDefinition the {@link AppDefinition}
Expand All @@ -69,7 +75,7 @@ public static String createName(AppDefinition appDefinition, int instance) {
* @return the name
*/
public static String createName(AppDefinition appDefinition, int instance, String identifier) {
String prefix = "instance-" + instance;
String prefix = APP_DEFINITION_INSTANCE_PREFIX + instance;
return createName(prefix, identifier, null, appDefinition.getSpec().getName(),
appDefinition.getMetadata().getUid());
}
Expand Down Expand Up @@ -174,7 +180,7 @@ private static String createName(String prefix, String identifier, String user,

// If the user is an email address, only take the part before the @ sign because
// this is usually sufficient to identify the user.
String userName = user.split("@")[0];
String userName = user != null ? user.split("@")[0] : null;

int infoSegmentLength;
String shortenedIdentifier = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

import static org.junit.jupiter.api.Assertions.assertEquals;

import org.eclipse.theia.cloud.common.k8s.resource.appdefinition.AppDefinition;
import org.eclipse.theia.cloud.common.k8s.resource.appdefinition.AppDefinitionSpec;
import org.eclipse.theia.cloud.common.k8s.resource.session.Session;
import org.eclipse.theia.cloud.common.k8s.resource.session.SessionSpec;
import org.eclipse.theia.cloud.common.k8s.resource.workspace.Workspace;
Expand All @@ -30,6 +32,22 @@
*/
class NamingUtilTests {

@Test
void createName_AppDefinitionAndInstace() {
AppDefinition appDefinition = createAppDefinition();

String result = NamingUtil.createName(appDefinition, 1);
assertEquals("instance-1-some-app-definiti-381261d79c23", result);
}

@Test
void createName_AppDefinitionAndInstaceAndIdentifier() {
AppDefinition appDefinition = createAppDefinition();

String result = NamingUtil.createName(appDefinition, 1, "longidentifier");
assertEquals("instance-1-longidentif-some-app-de-381261d79c23", result);
}

@Test
void createName_SessionAndNullIdentifier() {
Session session = createSession();
Expand Down Expand Up @@ -94,6 +112,21 @@ void createName_WorkspaceAndIdentifier() {
assertEquals("ws-longidentif-some-userna-test-app-de-381261d79c23", result);
}

private AppDefinition createAppDefinition() {
AppDefinition appDefinition = new AppDefinition();
ObjectMeta objectMeta = new ObjectMeta();
objectMeta.setUid("6f1a8966-4d5a-41dc-82ba-381261d79c23");
appDefinition.setMetadata(objectMeta);
AppDefinitionSpec spec = new AppDefinitionSpec() {
@Override
public String getName() {
return "some-app-definition";
}
};
appDefinition.setSpec(spec);
return appDefinition;
}

private Session createSession() {
Session session = new Session();
ObjectMeta objectMeta = new ObjectMeta();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import org.eclipse.theia.cloud.operator.handler.appdef.AppDefinitionHandler;
import org.eclipse.theia.cloud.operator.handler.appdef.EagerStartAppDefinitionAddedHandler;
import org.eclipse.theia.cloud.operator.handler.appdef.LazyStartAppDefinitionHandler;
import org.eclipse.theia.cloud.operator.handler.session.EagerStartSessionHandler;
import org.eclipse.theia.cloud.operator.handler.session.EagerSessionHandler;
import org.eclipse.theia.cloud.operator.handler.session.LazySessionHandler;
import org.eclipse.theia.cloud.operator.handler.session.SessionHandler;
import org.eclipse.theia.cloud.operator.handler.ws.LazyWorkspaceHandler;
Expand Down Expand Up @@ -143,7 +143,7 @@ protected Class<? extends WorkspaceHandler> bindWorkspaceHandler() {

protected Class<? extends SessionHandler> bindSessionHandler() {
if (arguments.isEagerStart()) {
return EagerStartSessionHandler.class;
return EagerSessionHandler.class;
} else {
return LazySessionHandler.class;
}
Expand Down
Loading
Loading