diff --git a/java/common/org.eclipse.theia.cloud.common/src/main/java/org/eclipse/theia/cloud/common/util/LabelsUtil.java b/java/common/org.eclipse.theia.cloud.common/src/main/java/org/eclipse/theia/cloud/common/util/LabelsUtil.java index e675c06e..4162c540 100644 --- a/java/common/org.eclipse.theia.cloud.common/src/main/java/org/eclipse/theia/cloud/common/util/LabelsUtil.java +++ b/java/common/org.eclipse.theia.cloud.common/src/main/java/org/eclipse/theia/cloud/common/util/LabelsUtil.java @@ -18,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 createSessionLabels(SessionSpec sessionSpec, AppDefinitionSpec appDefinitionSpec) { @@ -27,15 +28,16 @@ public static Map 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 key. + * 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 getSessionSpecificLabelKeys() { - return Set.of(LABEL_KEY_USER); + return Set.of(LABEL_KEY_SESSION_NAME, LABEL_KEY_USER); } } diff --git a/java/operator/org.eclipse.theia.cloud.operator/src/main/java/org/eclipse/theia/cloud/operator/handler/session/EagerSessionHandler.java b/java/operator/org.eclipse.theia.cloud.operator/src/main/java/org/eclipse/theia/cloud/operator/handler/session/EagerSessionHandler.java index 07534b7b..a627f080 100644 --- a/java/operator/org.eclipse.theia.cloud.operator/src/main/java/org/eclipse/theia/cloud/operator/handler/session/EagerSessionHandler.java +++ b/java/operator/org.eclipse.theia.cloud.operator/src/main/java/org/eclipse/theia/cloud/operator/handler/session/EagerSessionHandler.java @@ -189,6 +189,7 @@ public boolean sessionAdded(Session session, String correlationId) { // This is the case because ConfigMap changes are not propagated to the pod immediately but during a // periodic sync. See // https://kubernetes.io/docs/concepts/configuration/configmap/#mounted-configmaps-are-updated-automatically + // NOTE that this is still not a one hundred percent guarantee that the pod is updated in time. try { LOGGER.info(formatLogMessage(correlationId, "Adding update annotation to pods...")); client.kubernetes().pods().list().getItems().forEach(pod -> { @@ -327,29 +328,32 @@ public boolean sessionDeleted(Session session, String correlationId) { String sessionResourceName = session.getMetadata().getName(); String sessionResourceUID = session.getMetadata().getUid(); Map sessionLabels = LabelsUtil.createSessionLabels(spec, appDefinitionSpec); - String namespace = session.getMetadata().getNamespace(); // Filtering by withLabels(sessionLabels) because the method requires an exact match of the labels. // Additional labels on the service prevent a match and the service has an additional app label. // Thus, filter by each session label separately. + // We rely on the fact that the session labels are unique for each session. + // We cannot rely on owner references because they might have been cleaned up automatically by Kubernetes. + // While this should not happen, it did on Minikube. FilterWatchListDeletable> servicesFilter = client.services(); for (Entry entry : sessionLabels.entrySet()) { servicesFilter = servicesFilter.withLabel(entry.getKey(), entry.getValue()); } List services = servicesFilter.list().getItems(); - Optional ownedService = TheiaCloudServiceUtil.getServiceOwnedBySession(sessionResourceName, - sessionResourceUID, services); - - if (ownedService.isEmpty()) { - LOGGER.error( - formatLogMessage(correlationId, "No Service owned by session " + sessionResourceName + " found.")); + if (services.isEmpty()) { + LOGGER.error(formatLogMessage(correlationId, "No Service owned by session " + spec.getName() + " found.")); + return false; + } else if (services.size() > 1) { + LOGGER.error(formatLogMessage(correlationId, + "Multiple Services owned by session " + spec.getName() + " found. This should never happen.")); return false; } - String serviceName = ownedService.get().getMetadata().getName(); + Service ownedService = services.get(0); + String serviceName = ownedService.getMetadata().getName(); // Remove owner reference and user specific labels from the service Service cleanedService; try { - cleanedService = client.services().inNamespace(namespace).withName(serviceName).edit(service -> { + cleanedService = client.services().withName(serviceName).edit(service -> { TheiaCloudHandlerUtil.removeOwnerReferenceFromItem(correlationId, sessionResourceName, sessionResourceUID, service); service.getMetadata().getLabels().keySet().removeAll(LabelsUtil.getSessionSpecificLabelKeys()); diff --git a/java/operator/org.eclipse.theia.cloud.operator/src/main/java/org/eclipse/theia/cloud/operator/util/TheiaCloudHandlerUtil.java b/java/operator/org.eclipse.theia.cloud.operator/src/main/java/org/eclipse/theia/cloud/operator/util/TheiaCloudHandlerUtil.java index da9c9fcf..36f4e05e 100644 --- a/java/operator/org.eclipse.theia.cloud.operator/src/main/java/org/eclipse/theia/cloud/operator/util/TheiaCloudHandlerUtil.java +++ b/java/operator/org.eclipse.theia.cloud.operator/src/main/java/org/eclipse/theia/cloud/operator/util/TheiaCloudHandlerUtil.java @@ -73,6 +73,9 @@ public static T addOwnerReferenceToItem(String correlati return item; } + /** + * Removes the owner reference from the item if it is present. Does nothing otherwise. + */ public static T removeOwnerReferenceFromItem(String correlationId, String sessionResourceName, String sessionResourceUID, T item) { LOGGER.info(