From 6ae96f20783af31dec2460c5e62d7e90fa7cfc5d Mon Sep 17 00:00:00 2001 From: David Festal Date: Wed, 8 Nov 2017 11:13:59 +0100 Subject: [PATCH 01/13] Add the ability to find the workspaceId from the machine token Signed-off-by: David Festal --- .../server/MachineTokenRegistry.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/multiuser/machine-auth/che-multiuser-machine-authentication/src/main/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenRegistry.java b/multiuser/machine-auth/che-multiuser-machine-authentication/src/main/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenRegistry.java index ca5faf7ec4f..3d40e98bbe9 100644 --- a/multiuser/machine-auth/che-multiuser-machine-authentication/src/main/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenRegistry.java +++ b/multiuser/machine-auth/che-multiuser-machine-authentication/src/main/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenRegistry.java @@ -95,6 +95,26 @@ public String getUserId(String token) throws NotFoundException { } } + /** + * Gets workspaceId by machine token + * + * @return workspace identifier + * @throws NotFoundException when no such machine token exists + */ + public String getWorkspaceId(String token) throws NotFoundException { + lock.readLock().lock(); + try { + for (Table.Cell tokenCell : tokens.cellSet()) { + if (tokenCell.getValue().equals(token)) { + return tokenCell.getRowKey(); + } + } + throw new NotFoundException("Workspace not found for token " + token); + } finally { + lock.readLock().unlock(); + } + } + /** * Invalidates machine security tokens for all users of given workspace. * From f651db84da3aa4f5f7a54f7c354dab96ea026fc6 Mon Sep 17 00:00:00 2001 From: David Festal Date: Wed, 8 Nov 2017 11:16:02 +0100 Subject: [PATCH 02/13] Add a log when a synchronous che-server shutdown is requested Signed-off-by: David Festal --- .../java/org/eclipse/che/api/system/server/SystemManager.java | 1 + 1 file changed, 1 insertion(+) diff --git a/wsmaster/che-core-api-system/src/main/java/org/eclipse/che/api/system/server/SystemManager.java b/wsmaster/che-core-api-system/src/main/java/org/eclipse/che/api/system/server/SystemManager.java index bc0918308b5..834a3545a7e 100644 --- a/wsmaster/che-core-api-system/src/main/java/org/eclipse/che/api/system/server/SystemManager.java +++ b/wsmaster/che-core-api-system/src/main/java/org/eclipse/che/api/system/server/SystemManager.java @@ -106,6 +106,7 @@ private void doStopServices() { @PreDestroy @VisibleForTesting void shutdown() throws InterruptedException { + LOG.info("Synchronous shutdown requested"); if (!statusRef.compareAndSet(RUNNING, PREPARING_TO_SHUTDOWN)) { shutdownLatch.await(); } else { From 4dd5c7ba670d72d070258227fbb700d88558950b Mon Sep 17 00:00:00 2001 From: David Festal Date: Wed, 8 Nov 2017 11:20:21 +0100 Subject: [PATCH 03/13] Add a mechanism that allows keeping track of workspace user informations More precisely, the idea is to keep track of the userId that started a workspace and have the corresponding Subject always available and updated with the last connection information (mainly the last Keycloak token). Signed-off-by: David Festal --- .../che-multiuser-keycloak-server/pom.xml | 4 + ...eycloakEnvironmentInitalizationFilter.java | 7 +- .../server/WorkspaceSubjectRegistry.java | 101 ++++++++++++++++++ wsmaster/wsmaster-local/pom.xml | 4 + .../EnvironmentInitializationFilter.java | 5 + 5 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistry.java diff --git a/multiuser/keycloak/che-multiuser-keycloak-server/pom.xml b/multiuser/keycloak/che-multiuser-keycloak-server/pom.xml index e3fb7292d29..5d442009c04 100644 --- a/multiuser/keycloak/che-multiuser-keycloak-server/pom.xml +++ b/multiuser/keycloak/che-multiuser-keycloak-server/pom.xml @@ -66,6 +66,10 @@ org.eclipse.che.core che-core-api-user + + org.eclipse.che.core + che-core-api-workspace + org.eclipse.che.core che-core-commons-annotations diff --git a/multiuser/keycloak/che-multiuser-keycloak-server/src/main/java/org/eclipse/che/multiuser/keycloak/server/KeycloakEnvironmentInitalizationFilter.java b/multiuser/keycloak/che-multiuser-keycloak-server/src/main/java/org/eclipse/che/multiuser/keycloak/server/KeycloakEnvironmentInitalizationFilter.java index afe827a33dd..1323e5b209b 100644 --- a/multiuser/keycloak/che-multiuser-keycloak-server/src/main/java/org/eclipse/che/multiuser/keycloak/server/KeycloakEnvironmentInitalizationFilter.java +++ b/multiuser/keycloak/che-multiuser-keycloak-server/src/main/java/org/eclipse/che/multiuser/keycloak/server/KeycloakEnvironmentInitalizationFilter.java @@ -33,6 +33,7 @@ import org.eclipse.che.api.core.model.user.User; import org.eclipse.che.api.user.server.UserManager; import org.eclipse.che.api.user.server.model.impl.UserImpl; +import org.eclipse.che.api.workspace.server.WorkspaceSubjectRegistry; import org.eclipse.che.commons.auth.token.RequestTokenExtractor; import org.eclipse.che.commons.env.EnvironmentContext; import org.eclipse.che.commons.subject.Subject; @@ -51,15 +52,18 @@ public class KeycloakEnvironmentInitalizationFilter extends AbstractKeycloakFilt private final UserManager userManager; private final RequestTokenExtractor tokenExtractor; private final PermissionChecker permissionChecker; + private final WorkspaceSubjectRegistry workspaceSubjectRegistry; @Inject public KeycloakEnvironmentInitalizationFilter( UserManager userManager, RequestTokenExtractor tokenExtractor, - PermissionChecker permissionChecker) { + PermissionChecker permissionChecker, + WorkspaceSubjectRegistry workspaceSubjectRegistry) { this.userManager = userManager; this.tokenExtractor = tokenExtractor; this.permissionChecker = permissionChecker; + this.workspaceSubjectRegistry = workspaceSubjectRegistry; } @Override @@ -91,6 +95,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha subject = new AuthorizedSubject( new SubjectImpl(user.getName(), user.getId(), token, false), permissionChecker); + workspaceSubjectRegistry.updateSubject(subject); session.setAttribute("che_subject", subject); } catch (ServerException | ConflictException e) { throw new ServletException( diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistry.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistry.java new file mode 100644 index 00000000000..99fb6917115 --- /dev/null +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistry.java @@ -0,0 +1,101 @@ +/* + * Copyright (c) 2012-2017 Red Hat, Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.api.workspace.server; + +import static org.slf4j.LoggerFactory.getLogger; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Multimap; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import javax.annotation.PostConstruct; +import javax.inject.Inject; +import javax.inject.Singleton; +import org.eclipse.che.api.core.notification.EventService; +import org.eclipse.che.api.core.notification.EventSubscriber; +import org.eclipse.che.api.workspace.shared.dto.event.WorkspaceStatusEvent; +import org.eclipse.che.commons.env.EnvironmentContext; +import org.eclipse.che.commons.subject.Subject; +import org.slf4j.Logger; + +@Singleton +public class WorkspaceSubjectRegistry implements EventSubscriber { + + private static final Logger LOG = getLogger(WorkspaceSubjectRegistry.class); + + private final EventService eventService; + private final ReadWriteLock lock = new ReentrantReadWriteLock(); + private final Map workspaceOwners = new HashMap<>(); + private final Multimap userIdToWorkspaces = + ArrayListMultimap.create(); + + @Inject + public WorkspaceSubjectRegistry(EventService eventService) { + this.eventService = eventService; + } + + @Override + public void onEvent(WorkspaceStatusEvent event) { + + String workspaceId = event.getWorkspaceId(); + if (WorkspaceStatusEvent.EventType.STOPPED.equals(event.getEventType())) { + workspaceOwners.remove(workspaceId); + while (userIdToWorkspaces.values().remove(workspaceId)) {} + } + + if (WorkspaceStatusEvent.EventType.STARTING.equals(event.getEventType())) { + Subject subject = EnvironmentContext.getCurrent().getSubject(); + if (subject == Subject.ANONYMOUS) { + LOG.warn("Workspace {} is being started by the 'anonymous' user.", workspaceId); + return; + } + userIdToWorkspaces.put(subject.getUserId(), workspaceId); + updateSubject(subject); + } + } + + @PostConstruct + @VisibleForTesting + void subscribe() { + eventService.subscribe(this); + } + + public Subject getWorkspaceStarter(String workspaceId) { + lock.writeLock().lock(); + try { + return workspaceOwners.get(workspaceId); + } finally { + lock.writeLock().unlock(); + } + } + + public void updateSubject(Subject subject) { + String token = subject != null ? subject.getToken() : null; + if (token != null && token.startsWith("machine")) { + // We are not interested in machine tokens here, but in + // having the up-to-date token used by the user to connect + // to the front-end application and create the workspace + return; + } + lock.readLock().lock(); + try { + String userId = subject.getUserId(); + for (String workspaceId : userIdToWorkspaces.get(userId)) { + workspaceOwners.put(workspaceId, subject); + } + } finally { + lock.readLock().unlock(); + } + } +} diff --git a/wsmaster/wsmaster-local/pom.xml b/wsmaster/wsmaster-local/pom.xml index 3081ef1e95a..03cf46f7043 100644 --- a/wsmaster/wsmaster-local/pom.xml +++ b/wsmaster/wsmaster-local/pom.xml @@ -48,6 +48,10 @@ org.eclipse.che.core che-core-api-user + + org.eclipse.che.core + che-core-api-workspace + org.eclipse.che.core che-core-commons-inject diff --git a/wsmaster/wsmaster-local/src/main/java/org/eclipse/che/api/local/filters/EnvironmentInitializationFilter.java b/wsmaster/wsmaster-local/src/main/java/org/eclipse/che/api/local/filters/EnvironmentInitializationFilter.java index af8176aaf13..ac1784f231a 100644 --- a/wsmaster/wsmaster-local/src/main/java/org/eclipse/che/api/local/filters/EnvironmentInitializationFilter.java +++ b/wsmaster/wsmaster-local/src/main/java/org/eclipse/che/api/local/filters/EnvironmentInitializationFilter.java @@ -12,6 +12,7 @@ import java.io.IOException; import java.security.Principal; +import javax.inject.Inject; import javax.inject.Singleton; import javax.servlet.Filter; import javax.servlet.FilterChain; @@ -22,6 +23,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequestWrapper; import javax.servlet.http.HttpSession; +import org.eclipse.che.api.workspace.server.WorkspaceSubjectRegistry; import org.eclipse.che.commons.env.EnvironmentContext; import org.eclipse.che.commons.subject.Subject; import org.eclipse.che.commons.subject.SubjectImpl; @@ -34,6 +36,8 @@ @Singleton public class EnvironmentInitializationFilter implements Filter { + @Inject private WorkspaceSubjectRegistry workspaceSubjectRegistry; + @Override public void init(FilterConfig filterConfig) throws ServletException {} @@ -45,6 +49,7 @@ public final void doFilter( Subject subject = new SubjectImpl("che", "che", "dummy_token", false); HttpSession session = httpRequest.getSession(); session.setAttribute("codenvy_user", subject); + workspaceSubjectRegistry.updateSubject(subject); final EnvironmentContext environmentContext = EnvironmentContext.getCurrent(); From 070035a721bd5f2a867ca12c23729973e0aa194d Mon Sep 17 00:00:00 2001 From: David Festal Date: Wed, 8 Nov 2017 11:22:16 +0100 Subject: [PATCH 04/13] Make the `OpenshiftConnector` use the new `WorkspaceSubjectRegistry`... ... to allow removing Openshift resources when a workspace is stopped (`removeContainer`, `removeImage`) even if the workspace user is not accessible through the current `EnvironmentContext`. Signed-off-by: David Festal --- .../openshift/client/OpenShiftConnector.java | 191 +++++++++++++----- .../client/OpenShiftDeploymentCleaner.java | 67 ++++-- .../openshift/client/OpenShiftPvcHelper.java | 2 + ...OpenshiftWorkspaceEnvironmentProvider.java | 14 +- .../kubernetes/KubernetesExecHolder.java | 10 + .../client/OpenShiftConnectorTest.java | 3 + 6 files changed, 217 insertions(+), 70 deletions(-) diff --git a/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenShiftConnector.java b/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenShiftConnector.java index 7cc18a20c3e..cd9a02e766c 100644 --- a/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenShiftConnector.java +++ b/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenShiftConnector.java @@ -47,6 +47,7 @@ import io.fabric8.kubernetes.api.model.VolumeMountBuilder; import io.fabric8.kubernetes.api.model.extensions.Deployment; import io.fabric8.kubernetes.api.model.extensions.DeploymentBuilder; +import io.fabric8.kubernetes.client.Config; import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.Watcher; import io.fabric8.kubernetes.client.dsl.ExecWatch; @@ -96,8 +97,11 @@ import okhttp3.OkHttpClient; import org.eclipse.che.api.core.notification.EventService; import org.eclipse.che.api.core.notification.EventSubscriber; +import org.eclipse.che.api.workspace.server.WorkspaceSubjectRegistry; import org.eclipse.che.api.workspace.server.event.ServerIdleEvent; import org.eclipse.che.commons.annotation.Nullable; +import org.eclipse.che.commons.env.EnvironmentContext; +import org.eclipse.che.commons.subject.Subject; import org.eclipse.che.plugin.docker.client.DockerApiVersionPathPrefixProvider; import org.eclipse.che.plugin.docker.client.DockerConnector; import org.eclipse.che.plugin.docker.client.DockerConnectorConfiguration; @@ -235,6 +239,10 @@ public class OpenShiftConnector extends DockerConnector { private final OpenshiftWorkspaceEnvironmentProvider openshiftWorkspaceEnvironmentProvider; private String apiEndpoint; private Boolean apiEndpointRetrieved = false; + private final WorkspaceSubjectRegistry workspaceSubjectRegistry; + + private Map containerIdToWorkspaceId = new HashMap<>(); + private Map imageIdToWorkspaceId = new HashMap<>(); @Inject public OpenShiftConnector( @@ -247,6 +255,7 @@ public OpenShiftConnector( OpenShiftRouteCreator openShiftRouteCreator, OpenShiftDeploymentCleaner openShiftDeploymentCleaner, EventService eventService, + WorkspaceSubjectRegistry workspaceSubjectRegistry, @Nullable @Named("che.docker.ip.external") String cheServerExternalAddress, WorkspacesRoutingSuffixProvider cheWorkspacesRoutingSuffixProvider, @Named("che.openshift.project") String openShiftCheProjectName, @@ -283,6 +292,7 @@ public OpenShiftConnector( this.openShiftPvcHelper = openShiftPvcHelper; this.openShiftRouteCreator = openShiftRouteCreator; this.openShiftDeploymentCleaner = openShiftDeploymentCleaner; + this.workspaceSubjectRegistry = workspaceSubjectRegistry; eventService.subscribe( new EventSubscriber() { @@ -524,6 +534,42 @@ protected Map getPortsToRefName( return portsToRefName; } + private Subject subjectForContainerId(String containerId) { + String workspaceId = containerIdToWorkspaceId.get(containerId); + if (workspaceId != null) { + Subject subject = workspaceSubjectRegistry.getWorkspaceStarter(workspaceId); + if (subject != null) { + return subject; + } + Subject envSubject = EnvironmentContext.getCurrent().getSubject(); + LOG.info( + "Didn't find the user that started workspace '{}' => using environment context subject: '{}'", + workspaceId, + envSubject.getUserName()); + return envSubject; + } + LOG.warn("Didn't find container '{}' in the map of created containers", containerId); + return null; + } + + private Subject subjectForImageId(String imageId) { + String workspaceId = imageIdToWorkspaceId.get(imageId); + if (workspaceId != null) { + Subject subject = workspaceSubjectRegistry.getWorkspaceStarter(workspaceId); + if (subject != null) { + return subject; + } + Subject envSubject = EnvironmentContext.getCurrent().getSubject(); + LOG.info( + "Didn't find the user that started workspace '{}' => using environment context subject: '{}'", + workspaceId, + envSubject.getUserName()); + return envSubject; + } + LOG.warn("Didn't find image '{}' in the map of created images", imageId); + return null; + } + /** * @param createContainerParams * @return @@ -539,6 +585,8 @@ public ContainerCreated createContainer(CreateContainerParams createContainerPar // imageForDocker is the docker version of the image repository. It's needed for other // OpenShiftConnector API methods, but is not acceptable as an OpenShift name String imageForDocker = createContainerParams.getContainerConfig().getImage(); + imageIdToWorkspaceId.put(imageForDocker, getOriginalCheWorkspaceId(createContainerParams)); + // imageStreamTagName is imageForDocker converted into a form that can be used // in OpenShift String imageStreamTagName = KubernetesStringUtils.convertPullSpecToTagName(imageForDocker); @@ -546,7 +594,11 @@ public ContainerCreated createContainer(CreateContainerParams createContainerPar // imageStreamTagName is not enough to fill out a pull spec; it is only the tag, so we // have to get the ImageStreamTag from the tag, and then get the full ImageStreamTag name // from that tag. This works because the tags used in Che are unique. - ImageStreamTag imageStreamTag = getImageStreamTagFromRepo(imageStreamTagName); + ImageStreamTag imageStreamTag = + getImageStreamTagFromRepo( + imageStreamTagName, + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftConfig(), + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace()); String imageStreamTagPullSpec = imageStreamTag.getMetadata().getName(); // Next we need to get the address of the registry where the ImageStreamTag is stored @@ -662,14 +714,15 @@ public ContainerCreated createContainer(CreateContainerParams createContainerPar throw new OpenShiftException( "Failed to get the ID of the container running in the OpenShift pod"); } - } catch (IOException | KubernetesClientException e) { + containerIdToWorkspaceId.put(containerID, getOriginalCheWorkspaceId(createContainerParams)); + } catch (Exception e) { // Make sure we clean up deployment and service in case of an error -- otherwise Che can end // up // in an inconsistent state. LOG.info("Error while creating Pod, removing deployment"); LOG.info(e.getMessage()); - openShiftDeploymentCleaner.cleanDeploymentResources( - deploymentName, openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace()); + imageIdToWorkspaceId.remove(imageForDocker); + openShiftDeploymentCleaner.cleanDeploymentResources(deploymentName); openShiftClient.resource(imageStreamTag).delete(); throw e; } finally { @@ -720,7 +773,7 @@ public void putResource(PutResourceParams params) throws IOException { @Override public ContainerInfo inspectContainer(String containerId) throws IOException { - + Subject subject = subjectForContainerId(containerId); Pod pod = getChePodByContainerId(containerId); if (pod == null) { LOG.warn("No Pod found by container ID {}", containerId); @@ -737,7 +790,7 @@ public ContainerInfo inspectContainer(String containerId) throws IOException { Deployment deployment; try (OpenShiftClient client = new DefaultOpenShiftClient( - openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftConfig())) { + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftConfig(subject))) { deployment = client.extensions().deployments().withName(deploymentName).get(); if (deployment == null) { LOG.warn( @@ -758,10 +811,14 @@ public ContainerInfo inspectContainer(String containerId) throws IOException { String tagName = KubernetesStringUtils.getTagNameFromPullSpec(podPullSpec); - ImageStreamTag tag = getImageStreamTagFromRepo(tagName); + ImageStreamTag tag = + getImageStreamTagFromRepo( + tagName, + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftConfig(subject), + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace(subject)); ImageInfo imageInfo = getImageInfoFromTag(tag); - Service svc = getCheServiceBySelector(OPENSHIFT_DEPLOYMENT_LABEL, deploymentName); + Service svc = getCheServiceBySelector(OPENSHIFT_DEPLOYMENT_LABEL, deploymentName, subject); if (svc == null) { LOG.warn("No Service found by selector {}={}", OPENSHIFT_DEPLOYMENT_LABEL, deploymentName); return null; @@ -772,9 +829,11 @@ public ContainerInfo inspectContainer(String containerId) throws IOException { @Override public void removeContainer(final RemoveContainerParams params) throws IOException { + String containerId = params.getContainer(); + Subject subject = subjectForContainerId(containerId); String deploymentName = getDeploymentName(params); - openShiftDeploymentCleaner.cleanDeploymentResources( - deploymentName, openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace()); + openShiftDeploymentCleaner.cleanDeploymentResources(deploymentName, subject); + containerIdToWorkspaceId.remove(containerId); } @Override @@ -1032,7 +1091,11 @@ public void tag(final TagParams params) throws IOException { // Check if sourceImage matches existing imageStreamTag (e.g. when tagging a snapshot) try { String sourceImageTagName = KubernetesStringUtils.convertPullSpecToTagName(sourceImage); - ImageStreamTag existingTag = getImageStreamTagFromRepo(sourceImageTagName); + ImageStreamTag existingTag = + getImageStreamTagFromRepo( + sourceImageTagName, + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftConfig(), + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace()); sourceImageWithTag = existingTag.getTag().getFrom().getName(); } catch (IOException e) { // Image not found. @@ -1047,24 +1110,39 @@ public void tag(final TagParams params) throws IOException { @Override public ImageInfo inspectImage(InspectImageParams params) throws IOException { + Subject subject = subjectForImageId(params.getImage()); + if (subject == null) { + subject = EnvironmentContext.getCurrent().getSubject(); + } + final Config openshiftConfig = + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftConfig(subject); + final String openshiftNamespace = + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace(subject); String image = KubernetesStringUtils.getImageStreamNameFromPullSpec(params.getImage()); String imageStreamTagName = KubernetesStringUtils.convertPullSpecToTagName(image); - ImageStreamTag imageStreamTag = getImageStreamTagFromRepo(imageStreamTagName); + ImageStreamTag imageStreamTag = + getImageStreamTagFromRepo(imageStreamTagName, openshiftConfig, openshiftNamespace); return getImageInfoFromTag(imageStreamTag); } @Override public void removeImage(final RemoveImageParams params) throws IOException { - try (OpenShiftClient openShiftClient = - new DefaultOpenShiftClient( - openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftConfig())) { + Subject subject = subjectForImageId(params.getImage()); + final Config openshiftConfig = + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftConfig(subject); + final String openshiftNamespace = + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace(subject); + + try (OpenShiftClient openShiftClient = new DefaultOpenShiftClient(openshiftConfig)) { String image = KubernetesStringUtils.getImageStreamNameFromPullSpec(params.getImage()); String imageStreamTagName = KubernetesStringUtils.convertPullSpecToTagName(image); - ImageStreamTag imageStreamTag = getImageStreamTagFromRepo(imageStreamTagName); + ImageStreamTag imageStreamTag = + getImageStreamTagFromRepo(imageStreamTagName, openshiftConfig, openshiftNamespace); openShiftClient.resource(imageStreamTag).delete(); + imageIdToWorkspaceId.remove(params.getImage()); } } @@ -1086,7 +1164,11 @@ public String commit(final CommitParams params) throws IOException { String image = pod.getSpec().getContainers().get(0).getImage(); String imageStreamTagName = KubernetesStringUtils.getTagNameFromPullSpec(image); - ImageStreamTag imageStreamTag = getImageStreamTagFromRepo(imageStreamTagName); + ImageStreamTag imageStreamTag = + getImageStreamTagFromRepo( + imageStreamTagName, + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftConfig(), + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace()); String sourcePullSpec = imageStreamTag.getTag().getFrom().getName(); String trackingRepo = KubernetesStringUtils.stripTagFromPullSpec(sourcePullSpec); String tag = KubernetesStringUtils.getTagNameFromPullSpec(sourcePullSpec); @@ -1138,6 +1220,8 @@ public void getContainerLogs( final GetContainerLogsParams params, MessageProcessor containerLogsProcessor) throws IOException { String container = params.getContainer(); // container ID + Subject subject = subjectForContainerId(container); + Pod pod = getChePodByContainerId(container); if (pod != null) { String podName = pod.getMetadata().getName(); @@ -1145,11 +1229,12 @@ public void getContainerLogs( ret[0] = false; OpenShiftClient openShiftClient = new DefaultOpenShiftClient( - openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftConfig()); + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftConfig(subject)); try (LogWatch watchLog = openShiftClient .pods() - .inNamespace(openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace()) + .inNamespace( + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace(subject)) .withName(podName) .watchLog()) { Watcher watcher = @@ -1169,7 +1254,8 @@ public void onClose(KubernetesClientException cause) { }; openShiftClient .pods() - .inNamespace(openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace()) + .inNamespace( + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace(subject)) .withName(podName) .watch(watcher); Thread.sleep(5000); @@ -1194,6 +1280,7 @@ public void onClose(KubernetesClientException cause) { @Override public ContainerProcesses top(final TopParams params) throws IOException { String containerId = params.getContainer(); + Subject subject = subjectForContainerId(containerId); Pod pod = getChePodByContainerId(containerId); String podName = pod.getMetadata().getName(); String[] command; @@ -1208,11 +1295,14 @@ public ContainerProcesses top(final TopParams params) throws IOException { command[0] = PS_COMMAND; } ContainerProcesses processes = new ContainerProcesses(); - try (OpenShiftClient openShiftClient = new DefaultOpenShiftClient(); + try (OpenShiftClient openShiftClient = + new DefaultOpenShiftClient( + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftConfig(subject)); ExecWatch watch = openShiftClient .pods() - .inNamespace(openShiftCheProjectName) + .inNamespace( + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace(subject)) .withName(podName) .redirectingOutput() .redirectingError() @@ -1261,7 +1351,10 @@ public Exec createExec(final CreateExecParams params) throws IOException { String execId = KubernetesStringUtils.generateWorkspaceID(); KubernetesExecHolder execHolder = - new KubernetesExecHolder().withCommand(command).withPod(podName); + new KubernetesExecHolder() + .withCommand(command) + .withPod(podName) + .withContainerId(containerId); execMap.put(execId, execHolder); return new Exec(command, execId); @@ -1274,6 +1367,7 @@ public void startExec( String execId = params.getExecId(); KubernetesExecHolder exec = execMap.get(execId); + Subject subject = subjectForContainerId(exec.getContainerId()); String podName = exec.getPod(); String[] command = exec.getCommand(); @@ -1284,12 +1378,12 @@ public void startExec( ExecutorService executor = Executors.newFixedThreadPool(2); OpenShiftClient openShiftClient = new DefaultOpenShiftClient( - openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftConfig()); + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftConfig(subject)); try (ExecWatch watch = openShiftClient .pods() .inNamespace( - openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace()) + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace(subject)) .withName(podName) .redirectingOutput() .redirectingError() @@ -1362,7 +1456,9 @@ public SystemInfo getSystemInfo() throws IOException { * @return * @throws IOException if either no matching tag is found, or there are multiple matches. */ - private ImageStreamTag getImageStreamTagFromRepo(String imageStreamTagName) throws IOException { + private ImageStreamTag getImageStreamTagFromRepo( + String imageStreamTagName, Config openshiftConfig, String openshiftNamespace) + throws IOException { // Since repository + tag are limited to 63 chars, it's possible that the entire // tag name did not fit, so we have to match a substring. @@ -1372,15 +1468,9 @@ private ImageStreamTag getImageStreamTagFromRepo(String imageStreamTagName) thro // Note: ideally, ImageStreamTags could be identified with a label, but it seems like // ImageStreamTags do not support labels. List imageStreams; - try (OpenShiftClient openShiftClient = - new DefaultOpenShiftClient( - openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftConfig())) { + try (OpenShiftClient openShiftClient = new DefaultOpenShiftClient(openshiftConfig)) { imageStreams = - openShiftClient - .imageStreamTags() - .inNamespace(openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace()) - .list() - .getItems(); + openShiftClient.imageStreamTags().inNamespace(openshiftNamespace).list().getItems(); } // We only get ImageStreamTag names here, since these ImageStreamTags do not include @@ -1403,30 +1493,31 @@ private ImageStreamTag getImageStreamTagFromRepo(String imageStreamTagName) thro String imageStreamTag = imageStreamTags.get(0); // Finally, get the ImageStreamTag, with Docker metadata. - return getImageStreamTag(imageStreamTag); + return getImageStreamTag(imageStreamTag, openshiftConfig, openshiftNamespace); } - private ImageStreamTag getImageStreamTag(final String imageStreamName) throws OpenShiftException { - try (OpenShiftClient openShiftClient = - new DefaultOpenShiftClient( - openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftConfig())) { + private ImageStreamTag getImageStreamTag( + final String imageStreamName, Config openshiftConfig, String openshiftNamespace) + throws OpenShiftException { + try (OpenShiftClient openShiftClient = new DefaultOpenShiftClient(openshiftConfig)) { return openShiftClient .imageStreamTags() - .inNamespace(openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace()) + .inNamespace(openshiftNamespace) .withName(imageStreamName) .get(); } } - private Service getCheServiceBySelector(String selectorKey, String selectorValue) + private Service getCheServiceBySelector(String selectorKey, String selectorValue, Subject subject) throws OpenShiftException { try (OpenShiftClient openShiftClient = new DefaultOpenShiftClient( - openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftConfig())) { + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftConfig(subject))) { ServiceList svcs = openShiftClient .services() - .inNamespace(openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace()) + .inNamespace( + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace(subject)) .list(); Service svc = @@ -1445,13 +1536,15 @@ private Service getCheServiceBySelector(String selectorKey, String selectorValue } private Pod getChePodByContainerId(String containerId) throws IOException { + Subject subject = subjectForContainerId(containerId); try (OpenShiftClient openShiftClient = new DefaultOpenShiftClient( - openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftConfig())) { + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftConfig(subject))) { PodList pods = openShiftClient .pods() - .inNamespace(openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace()) + .inNamespace( + openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace(subject)) .withLabel( CHE_CONTAINER_IDENTIFIER_LABEL_KEY, KubernetesStringUtils.getLabelFromContainerID(containerId)) @@ -1530,13 +1623,18 @@ private ImageInfo getImageInfoFromTag(ImageStreamTag imageStreamTag) { } protected String getCheWorkspaceId(CreateContainerParams createContainerParams) { + String workspaceID = getOriginalCheWorkspaceId(createContainerParams); + return workspaceID.replaceFirst("workspace", ""); + } + + private String getOriginalCheWorkspaceId(CreateContainerParams createContainerParams) { Stream env = Arrays.stream(createContainerParams.getContainerConfig().getEnv()); String workspaceID = env.filter(v -> v.startsWith(CHE_WORKSPACE_ID_ENV_VAR) && v.contains("=")) .map(v -> v.split("=", 2)[1]) .findFirst() .orElse(""); - return workspaceID.replaceFirst("workspace", ""); + return workspaceID; } private boolean isDevMachine(CreateContainerParams createContainerParams) { @@ -1851,6 +1949,7 @@ private void createWorkspaceDir(String[] volumes) throws OpenShiftException { LOG.info("Making sure directory exists for workspace {}", workspaceSubpath); boolean succeeded = openShiftPvcHelper.createJobPod( + EnvironmentContext.getCurrent().getSubject(), workspacesPersistentVolumeClaim, openshiftWorkspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace(), "create-", diff --git a/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenShiftDeploymentCleaner.java b/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenShiftDeploymentCleaner.java index 44cb79c583f..88c16ea3190 100644 --- a/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenShiftDeploymentCleaner.java +++ b/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenShiftDeploymentCleaner.java @@ -14,6 +14,7 @@ import io.fabric8.kubernetes.api.model.Service; import io.fabric8.kubernetes.api.model.extensions.Deployment; import io.fabric8.kubernetes.api.model.extensions.ReplicaSet; +import io.fabric8.kubernetes.client.Config; import io.fabric8.openshift.api.model.Route; import io.fabric8.openshift.client.DefaultOpenShiftClient; import io.fabric8.openshift.client.OpenShiftClient; @@ -21,6 +22,8 @@ import java.util.List; import javax.inject.Inject; import javax.inject.Singleton; +import org.eclipse.che.commons.env.EnvironmentContext; +import org.eclipse.che.commons.subject.Subject; import org.eclipse.che.plugin.openshift.client.exception.OpenShiftException; import org.eclipse.che.plugin.openshift.client.kubernetes.KubernetesResourceUtil; import org.slf4j.Logger; @@ -34,38 +37,57 @@ public class OpenShiftDeploymentCleaner { @Inject private OpenshiftWorkspaceEnvironmentProvider openshiftUserAccountProvider; - public void cleanDeploymentResources(final String deploymentName, final String namespace) + public void cleanDeploymentResources(final String deploymentName, final Subject subject) throws IOException { - cleanUpWorkspaceResources(deploymentName, namespace); - waitUntilWorkspacePodIsDeleted(deploymentName, namespace); + cleanUpWorkspaceResources(deploymentName, subject); + waitUntilWorkspacePodIsDeleted(deploymentName, subject); } - private void cleanUpWorkspaceResources(final String deploymentName, final String namespace) - throws IOException { - Deployment deployment = - KubernetesResourceUtil.getDeploymentByName( - deploymentName, namespace, openshiftUserAccountProvider.getWorkspacesOpenshiftConfig()); + public void cleanDeploymentResources(final String deploymentName) throws IOException { + Subject subject = EnvironmentContext.getCurrent().getSubject(); + cleanDeploymentResources(deploymentName, subject); + } + + private void cleanUpWorkspaceResources(final String deploymentName, final Subject subject) + throws OpenShiftException { + String openshiftNamespace = + openshiftUserAccountProvider.getWorkspacesOpenshiftNamespace(subject); + Config openshiftConfig = openshiftUserAccountProvider.getWorkspacesOpenshiftConfig(subject); + Deployment deployment = null; + try { + deployment = + KubernetesResourceUtil.getDeploymentByName( + deploymentName, openshiftNamespace, openshiftConfig); + } catch (IOException e) { + LOG.error("Exception while retrieving the deployment to cleanup", e); + } Service service = KubernetesResourceUtil.getServiceBySelector( OpenShiftConnector.OPENSHIFT_DEPLOYMENT_LABEL, deploymentName, - namespace, - openshiftUserAccountProvider.getWorkspacesOpenshiftConfig()); - List routes = - KubernetesResourceUtil.getRoutesByLabel( - OpenShiftConnector.OPENSHIFT_DEPLOYMENT_LABEL, - deploymentName, - namespace, - openshiftUserAccountProvider.getWorkspacesOpenshiftConfig()); + openshiftNamespace, + openshiftConfig); + List routes = null; + try { + routes = + KubernetesResourceUtil.getRoutesByLabel( + OpenShiftConnector.OPENSHIFT_DEPLOYMENT_LABEL, + deploymentName, + openshiftNamespace, + openshiftConfig); + } catch (IOException e) { + LOG.error("Exception while retrieving the routes to cleanup", e); + } List replicaSets = KubernetesResourceUtil.getReplicaSetByLabel( OpenShiftConnector.OPENSHIFT_DEPLOYMENT_LABEL, deploymentName, - namespace, - openshiftUserAccountProvider.getWorkspacesOpenshiftConfig()); + openshiftNamespace, + openshiftConfig); try (OpenShiftClient openShiftClient = - new DefaultOpenShiftClient(openshiftUserAccountProvider.getWorkspacesOpenshiftConfig())) { + new DefaultOpenShiftClient( + openshiftUserAccountProvider.getWorkspacesOpenshiftConfig(subject))) { if (routes != null) { for (Route route : routes) { LOG.info("Removing OpenShift Route {}", route.getMetadata().getName()); @@ -90,15 +112,16 @@ private void cleanUpWorkspaceResources(final String deploymentName, final String } } - private void waitUntilWorkspacePodIsDeleted(final String deploymentName, final String namespace) + private void waitUntilWorkspacePodIsDeleted(final String deploymentName, final Subject subject) throws OpenShiftException { try (OpenShiftClient client = - new DefaultOpenShiftClient(openshiftUserAccountProvider.getWorkspacesOpenshiftConfig())) { + new DefaultOpenShiftClient( + openshiftUserAccountProvider.getWorkspacesOpenshiftConfig(subject))) { for (int waitCount = 0; waitCount < OPENSHIFT_POD_DELETION_TIMEOUT; waitCount++) { List pods = client .pods() - .inNamespace(namespace) + .inNamespace(openshiftUserAccountProvider.getWorkspacesOpenshiftNamespace(subject)) .withLabel(OpenShiftConnector.OPENSHIFT_DEPLOYMENT_LABEL, deploymentName) .list() .getItems(); diff --git a/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenShiftPvcHelper.java b/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenShiftPvcHelper.java index 40b916796b4..b9641bb1e36 100644 --- a/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenShiftPvcHelper.java +++ b/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenShiftPvcHelper.java @@ -36,6 +36,7 @@ import javax.inject.Inject; import javax.inject.Named; import javax.inject.Singleton; +import org.eclipse.che.commons.subject.Subject; import org.eclipse.che.plugin.openshift.client.exception.OpenShiftException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -100,6 +101,7 @@ protected OpenShiftPvcHelper( * @see Command */ protected boolean createJobPod( + Subject subject, String workspacesPvcName, String projectNamespace, String jobNamePrefix, diff --git a/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenshiftWorkspaceEnvironmentProvider.java b/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenshiftWorkspaceEnvironmentProvider.java index 1f69391f834..3b80769eae7 100644 --- a/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenshiftWorkspaceEnvironmentProvider.java +++ b/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenshiftWorkspaceEnvironmentProvider.java @@ -15,6 +15,8 @@ import javax.inject.Inject; import javax.inject.Named; import javax.inject.Singleton; +import org.eclipse.che.commons.env.EnvironmentContext; +import org.eclipse.che.commons.subject.Subject; import org.eclipse.che.plugin.openshift.client.exception.OpenShiftException; @Singleton @@ -28,14 +30,22 @@ public OpenshiftWorkspaceEnvironmentProvider( this.openShiftCheProjectName = openShiftCheProjectName; } - public Config getWorkspacesOpenshiftConfig() throws OpenShiftException { + public Config getWorkspacesOpenshiftConfig(Subject subject) throws OpenShiftException { return new OpenShiftConfigBuilder().build(); } - public String getWorkspacesOpenshiftNamespace() throws OpenShiftException { + public final Config getWorkspacesOpenshiftConfig() throws OpenShiftException { + return getWorkspacesOpenshiftConfig(EnvironmentContext.getCurrent().getSubject()); + } + + public String getWorkspacesOpenshiftNamespace(Subject subject) throws OpenShiftException { return openShiftCheProjectName; } + public final String getWorkspacesOpenshiftNamespace() throws OpenShiftException { + return getWorkspacesOpenshiftNamespace(EnvironmentContext.getCurrent().getSubject()); + } + public Boolean areWorkspacesExternal() { return false; } diff --git a/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/kubernetes/KubernetesExecHolder.java b/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/kubernetes/KubernetesExecHolder.java index 47fb9a2bc44..41fdeb2c92a 100644 --- a/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/kubernetes/KubernetesExecHolder.java +++ b/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/kubernetes/KubernetesExecHolder.java @@ -27,6 +27,7 @@ public class KubernetesExecHolder { private String[] command; private String podName; + private String containerId; public KubernetesExecHolder withCommand(String[] command) { this.command = command; @@ -38,6 +39,11 @@ public KubernetesExecHolder withPod(String podName) { return this; } + public KubernetesExecHolder withContainerId(String containerId) { + this.containerId = containerId; + return this; + } + public String[] getCommand() { return command; } @@ -46,6 +52,10 @@ public String getPod() { return podName; } + public String getContainerId() { + return containerId; + } + public String toString() { return String.format( "KubernetesExecHolder {command=%s, podName=%s}", diff --git a/plugins/plugin-docker/che-plugin-openshift-client/src/test/java/org/eclipse/che/plugin/openshift/client/OpenShiftConnectorTest.java b/plugins/plugin-docker/che-plugin-openshift-client/src/test/java/org/eclipse/che/plugin/openshift/client/OpenShiftConnectorTest.java index 0cb91f00fda..423b31ab15d 100644 --- a/plugins/plugin-docker/che-plugin-openshift-client/src/test/java/org/eclipse/che/plugin/openshift/client/OpenShiftConnectorTest.java +++ b/plugins/plugin-docker/che-plugin-openshift-client/src/test/java/org/eclipse/che/plugin/openshift/client/OpenShiftConnectorTest.java @@ -19,6 +19,7 @@ import java.util.Map; import java.util.Set; import org.eclipse.che.api.core.notification.EventService; +import org.eclipse.che.api.workspace.server.WorkspaceSubjectRegistry; import org.eclipse.che.plugin.docker.client.DockerApiVersionPathPrefixProvider; import org.eclipse.che.plugin.docker.client.DockerConnectorConfiguration; import org.eclipse.che.plugin.docker.client.DockerRegistryAuthResolver; @@ -59,6 +60,7 @@ public class OpenShiftConnectorTest { @Mock private OpenShiftPvcHelper openShiftPvcHelper; @Mock private OpenShiftRouteCreator openShiftRouteCreator; @Mock private OpenShiftDeploymentCleaner openShiftDeploymentCleaner; + @Mock private WorkspaceSubjectRegistry workspaceSubjectRegistry; private OpenShiftConnector openShiftConnector; @@ -77,6 +79,7 @@ private void setup() { openShiftRouteCreator, openShiftDeploymentCleaner, eventService, + workspaceSubjectRegistry, CHE_DEFAULT_SERVER_EXTERNAL_ADDRESS, null, CHE_DEFAULT_OPENSHIFT_PROJECT_NAME, From 56ee2ffc5d9bfae5efcdcfc252569e190d73dde0 Mon Sep 17 00:00:00 2001 From: David Festal Date: Wed, 8 Nov 2017 11:27:18 +0100 Subject: [PATCH 05/13] Adapt the OS workspace file cleaning to the multi-tenant use-case... ... This mainly consists in cleaning the workspace files *synchronously* instead of doing it in batch at che-server idling. The previous behavior remains unchanged if the che server is not multi-tenant. Signed-off-by: David Festal --- .../OpenShiftWorkspaceFilesCleaner.java | 66 +++++++++++++------ .../OpenShiftWorkspaceFilesCleanerTest.java | 24 ++++++- 2 files changed, 69 insertions(+), 21 deletions(-) diff --git a/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenShiftWorkspaceFilesCleaner.java b/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenShiftWorkspaceFilesCleaner.java index 777ce06586a..2c6c0490e46 100644 --- a/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenShiftWorkspaceFilesCleaner.java +++ b/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenShiftWorkspaceFilesCleaner.java @@ -15,6 +15,7 @@ import com.google.common.annotations.VisibleForTesting; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -26,7 +27,10 @@ import org.eclipse.che.api.core.notification.EventService; import org.eclipse.che.api.core.notification.EventSubscriber; import org.eclipse.che.api.workspace.server.WorkspaceFilesCleaner; +import org.eclipse.che.api.workspace.server.WorkspaceSubjectRegistry; import org.eclipse.che.api.workspace.server.event.ServerIdleEvent; +import org.eclipse.che.commons.env.EnvironmentContext; +import org.eclipse.che.commons.subject.Subject; import org.eclipse.che.plugin.openshift.client.exception.OpenShiftException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -48,7 +52,8 @@ public class OpenShiftWorkspaceFilesCleaner implements WorkspaceFilesCleaner { private static final Logger LOG = LoggerFactory.getLogger(OpenShiftConnector.class); private static final Set deleteQueue = ConcurrentHashMap.newKeySet(); - private final String projectNamespace; + private OpenshiftWorkspaceEnvironmentProvider workspaceEnvironmentProvider; + private WorkspaceSubjectRegistry workspaceSubjectRegistry; private final String workspacesPvcName; private final OpenShiftPvcHelper openShiftPvcHelper; @@ -56,22 +61,27 @@ public class OpenShiftWorkspaceFilesCleaner implements WorkspaceFilesCleaner { public OpenShiftWorkspaceFilesCleaner( EventService eventService, OpenShiftPvcHelper openShiftPvcHelper, - @Named("che.openshift.project") String projectNamespace, + OpenshiftWorkspaceEnvironmentProvider workspaceEnvironmentProvider, + WorkspaceSubjectRegistry workspaceSubjectRegistry, @Named("che.openshift.workspaces.pvc.name") String workspacesPvcName) { - this.projectNamespace = projectNamespace; + this.workspaceEnvironmentProvider = workspaceEnvironmentProvider; this.workspacesPvcName = workspacesPvcName; this.openShiftPvcHelper = openShiftPvcHelper; - eventService.subscribe( - new EventSubscriber() { - @Override - public void onEvent(ServerIdleEvent event) { - try { - deleteWorkspacesInQueue(event); - } catch (OpenShiftException e) { - LOG.warn("Error while deleting the workspaces", e); + this.workspaceSubjectRegistry = workspaceSubjectRegistry; + + if (!workspaceEnvironmentProvider.areWorkspacesExternal()) { + eventService.subscribe( + new EventSubscriber() { + @Override + public void onEvent(ServerIdleEvent event) { + try { + deleteWorkspacesInQueue(event); + } catch (OpenShiftException e) { + LOG.warn("Error while deleting the workspaces", e); + } } - } - }); + }); + } } @Override @@ -81,24 +91,40 @@ public void clear(Workspace workspace) throws IOException, ServerException { LOG.error("Could not get workspace name for files removal."); return; } - deleteQueue.add(workspaceName); + if (workspaceEnvironmentProvider.areWorkspacesExternal()) { + String workspaceId = workspace.getId(); + LOG.info("Synchronously deleting workspace {} on PVC {}", workspaceId, workspacesPvcName); + Subject subject = workspaceSubjectRegistry.getWorkspaceStarter(workspaceId); + if (subject == null) { + subject = EnvironmentContext.getCurrent().getSubject(); + } + deleteWorkspacesOnPvc(Arrays.asList(workspaceName), subject); + } else { + deleteQueue.add(workspaceName); + } } private void deleteWorkspacesInQueue(ServerIdleEvent event) throws OpenShiftException { List deleteQueueCopy = new ArrayList<>(deleteQueue); - String[] dirsToDelete = deleteQueueCopy.toArray(new String[deleteQueueCopy.size()]); + if (deleteWorkspacesOnPvc(deleteQueueCopy, null)) { + deleteQueue.removeAll(deleteQueueCopy); + } + } + + private boolean deleteWorkspacesOnPvc(List deleteQueue, Subject subject) + throws OpenShiftException { + String[] dirsToDelete = deleteQueue.toArray(new String[deleteQueue.size()]); - LOG.info("Deleting {} workspaces on PVC {}", deleteQueueCopy.size(), workspacesPvcName); + LOG.info("Deleting {} workspaces on PVC {}", deleteQueue.size(), workspacesPvcName); boolean successful = openShiftPvcHelper.createJobPod( + subject, workspacesPvcName, - projectNamespace, + workspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace(subject), "delete-", OpenShiftPvcHelper.Command.REMOVE, dirsToDelete); - if (successful) { - deleteQueue.removeAll(deleteQueueCopy); - } + return successful; } /** Clears the list of workspace directories to be deleted. Necessary for testing. */ diff --git a/plugins/plugin-docker/che-plugin-openshift-client/src/test/java/org/eclipse/che/plugin/openshift/client/OpenShiftWorkspaceFilesCleanerTest.java b/plugins/plugin-docker/che-plugin-openshift-client/src/test/java/org/eclipse/che/plugin/openshift/client/OpenShiftWorkspaceFilesCleanerTest.java index eab0ffbdf7b..c7dce3659b1 100644 --- a/plugins/plugin-docker/che-plugin-openshift-client/src/test/java/org/eclipse/che/plugin/openshift/client/OpenShiftWorkspaceFilesCleanerTest.java +++ b/plugins/plugin-docker/che-plugin-openshift-client/src/test/java/org/eclipse/che/plugin/openshift/client/OpenShiftWorkspaceFilesCleanerTest.java @@ -11,6 +11,7 @@ package org.eclipse.che.plugin.openshift.client; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.any; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.never; @@ -25,9 +26,11 @@ import org.eclipse.che.api.core.ServerException; import org.eclipse.che.api.core.model.workspace.Workspace; import org.eclipse.che.api.core.notification.EventService; +import org.eclipse.che.api.workspace.server.WorkspaceSubjectRegistry; import org.eclipse.che.api.workspace.server.event.ServerIdleEvent; import org.eclipse.che.api.workspace.server.model.impl.WorkspaceConfigImpl; import org.eclipse.che.api.workspace.server.model.impl.WorkspaceImpl; +import org.eclipse.che.plugin.openshift.client.exception.OpenShiftException; import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -43,6 +46,8 @@ public class OpenShiftWorkspaceFilesCleanerTest { @Mock private OpenShiftPvcHelper pvcHelper; @Mock private ServerIdleEvent serverIdleEvent; + @Mock private OpenshiftWorkspaceEnvironmentProvider workspaceEnvironmentProvider; + @Mock private WorkspaceSubjectRegistry workspaceSubjectRegistry; private EventService eventService; private OpenShiftWorkspaceFilesCleaner cleaner; @@ -50,10 +55,21 @@ public class OpenShiftWorkspaceFilesCleanerTest { public void setup() { OpenShiftWorkspaceFilesCleaner.clearDeleteQueue(); MockitoAnnotations.initMocks(this); + when(workspaceEnvironmentProvider.areWorkspacesExternal()).thenReturn(false); + try { + when(workspaceEnvironmentProvider.getWorkspacesOpenshiftNamespace(null)) + .thenReturn(CHE_OPENSHIFT_PROJECT); + } catch (OpenShiftException e) { + e.printStackTrace(); + } eventService = new EventService(); cleaner = new OpenShiftWorkspaceFilesCleaner( - eventService, pvcHelper, CHE_OPENSHIFT_PROJECT, WORKSPACES_PVC_NAME); + eventService, + pvcHelper, + workspaceEnvironmentProvider, + workspaceSubjectRegistry, + WORKSPACES_PVC_NAME); } @Test @@ -67,6 +83,7 @@ public void shouldDoNothingWithoutIdleEvent() throws ServerException, IOExceptio // Then verify(pvcHelper, never()) .createJobPod( + isNull(), anyString(), anyString(), anyString(), @@ -86,6 +103,7 @@ public void shouldDeleteWorkspaceOnIdleEvent() throws ServerException, IOExcepti // Then verify(pvcHelper, times(1)) .createJobPod( + isNull(), anyString(), anyString(), anyString(), @@ -109,6 +127,7 @@ public void shouldDeleteMultipleQueuedWorkspacesAtOnce() throws ServerException, // Then verify(pvcHelper, times(1)) .createJobPod( + isNull(), anyString(), anyString(), anyString(), @@ -137,6 +156,7 @@ public void shouldRetainQueueIfDeletionFails() throws ServerException, IOExcepti // Then verify(pvcHelper, times(1)) .createJobPod( + isNull(), anyString(), anyString(), anyString(), @@ -149,6 +169,7 @@ public void shouldRetainQueueIfDeletionFails() throws ServerException, IOExcepti // Then verify(pvcHelper, times(2)) .createJobPod( + isNull(), anyString(), anyString(), anyString(), @@ -169,6 +190,7 @@ public void shouldUseProjectNamespaceAndPvcNameAsParameters() // Then verify(pvcHelper, times(1)) .createJobPod( + isNull(), eq(WORKSPACES_PVC_NAME), eq(CHE_OPENSHIFT_PROJECT), anyString(), From f39184e40f737d1eb786aa246b82ee261c06c8af Mon Sep 17 00:00:00 2001 From: David Festal Date: Wed, 8 Nov 2017 11:28:24 +0100 Subject: [PATCH 06/13] First stop the che server before deleting all the resources... ... This allows cleaning the workspaces correctly also in the multi-tenant scenario. Signed-off-by: David Festal --- dockerfiles/init/modules/openshift/files/scripts/deploy_che.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dockerfiles/init/modules/openshift/files/scripts/deploy_che.sh b/dockerfiles/init/modules/openshift/files/scripts/deploy_che.sh index 22f9e728538..ff4f4ed7361 100755 --- a/dockerfiles/init/modules/openshift/files/scripts/deploy_che.sh +++ b/dockerfiles/init/modules/openshift/files/scripts/deploy_che.sh @@ -266,6 +266,8 @@ echo "done!" # If command == clean up then delete all openshift objects # ------------------------------------------------------------- if [ "${COMMAND}" == "cleanup" ]; then + echo "[CHE] Stopping the Che server..." + oc scale --replicas=0 --timeout=3h dc che echo "[CHE] Deleting all OpenShift objects..." oc delete all --all echo "[CHE] Cleanup successfully started. Use \"oc get all\" to verify that all resources have been deleted." From 15642ff2b93258e06534cf71adb2de746e4809e2 Mon Sep 17 00:00:00 2001 From: David Festal Date: Wed, 8 Nov 2017 17:12:46 +0100 Subject: [PATCH 07/13] Change log level to DEBUG Signed-off-by: David Festal --- .../java/org/eclipse/che/api/system/server/SystemManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wsmaster/che-core-api-system/src/main/java/org/eclipse/che/api/system/server/SystemManager.java b/wsmaster/che-core-api-system/src/main/java/org/eclipse/che/api/system/server/SystemManager.java index 834a3545a7e..0aa582132d7 100644 --- a/wsmaster/che-core-api-system/src/main/java/org/eclipse/che/api/system/server/SystemManager.java +++ b/wsmaster/che-core-api-system/src/main/java/org/eclipse/che/api/system/server/SystemManager.java @@ -106,7 +106,7 @@ private void doStopServices() { @PreDestroy @VisibleForTesting void shutdown() throws InterruptedException { - LOG.info("Synchronous shutdown requested"); + LOG.debug("Synchronous shutdown requested"); if (!statusRef.compareAndSet(RUNNING, PREPARING_TO_SHUTDOWN)) { shutdownLatch.await(); } else { From 499a947166878bdf459cca1e7861c33dcfd551f1 Mon Sep 17 00:00:00 2001 From: David Festal Date: Wed, 8 Nov 2017 21:46:46 +0100 Subject: [PATCH 08/13] Add a test for the newly-created `getWorkspaceId()` method. Signed-off-by: David Festal --- .../server/MachineTokenRegistryTest.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/multiuser/machine-auth/che-multiuser-machine-authentication/src/test/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenRegistryTest.java b/multiuser/machine-auth/che-multiuser-machine-authentication/src/test/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenRegistryTest.java index 3e06ff387da..2bca702ea49 100644 --- a/multiuser/machine-auth/che-multiuser-machine-authentication/src/test/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenRegistryTest.java +++ b/multiuser/machine-auth/che-multiuser-machine-authentication/src/test/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenRegistryTest.java @@ -53,4 +53,19 @@ private static boolean exists(MachineTokenRegistry registry, String user, String return false; } } + + @Test + public void shouldreturnWorkspaceId() throws Exception { + final MachineTokenRegistry registry = new MachineTokenRegistry(); + + String token11 = registry.generateToken("user1", "workspace1"); + String token12 = registry.generateToken("user1", "workspace2"); + String token21 = registry.generateToken("user2", "workspace1"); + String token22 = registry.generateToken("user2", "workspace2"); + + assertEquals(registry.getWorkspaceId(token11), "workspace1"); + assertEquals(registry.getWorkspaceId(token12), "workspace2"); + assertEquals(registry.getWorkspaceId(token21), "workspace1"); + assertEquals(registry.getWorkspaceId(token22), "workspace2"); + } } From 3b911356925dabd7c02b5fdd4e96b8c62dae8b3d Mon Sep 17 00:00:00 2001 From: David Festal Date: Wed, 8 Nov 2017 22:03:55 +0100 Subject: [PATCH 09/13] Add JavaDoc to the `WorkspaceSubjectRegistry` class Signed-off-by: David Festal --- .../server/WorkspaceSubjectRegistry.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistry.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistry.java index 99fb6917115..cd218c061a2 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistry.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistry.java @@ -29,6 +29,27 @@ import org.eclipse.che.commons.subject.Subject; import org.slf4j.Logger; +/** + * This class allows maintaining a link between a started workspace and the user who started it. + * This also allows updating, at any time, the user informations (Subject) of users + * that started workspaces. + * + *


+ * + *

In particular, this allows having access to the up-to-date connection information (userName + * and Keycloak token) of the user that was used when creating a workspace. + * + *

This is required for all the use-cases where these user informations would be necessary to + * perform batch-like operations on the user workspaces (such as idling, stop at shutdown, etc ...). + * + *


+ * + *

An example of such use-case is the multi-tenant scenario when deployment is done to Openshift: + * the user connection informations are required to have access to the Openshift cluster / namespace + * where the workspace has been created. + * + * @author David Festal + */ @Singleton public class WorkspaceSubjectRegistry implements EventSubscriber { @@ -80,6 +101,12 @@ public Subject getWorkspaceStarter(String workspaceId) { } } + /* + * If some workspaces have been started by the userId contained + * in this Subject, then the subject (with + * the userName and token) is updated in the workspace-to-subject + * cache for all these workspaces. + */ public void updateSubject(Subject subject) { String token = subject != null ? subject.getToken() : null; if (token != null && token.startsWith("machine")) { From 9d94514a09a4f13fcd2e2d472acddbdda66150f0 Mon Sep 17 00:00:00 2001 From: David Festal Date: Thu, 9 Nov 2017 13:24:40 +0100 Subject: [PATCH 10/13] Throw when a workspace is started by the anonymous user... ... as suggested by @skabashnyuk [here](https://github.com/eclipse/che/pull/7243#discussion_r149763856) Signed-off-by: David Festal --- .../che/api/workspace/server/WorkspaceSubjectRegistry.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistry.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistry.java index cd218c061a2..da413fb6350 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistry.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistry.java @@ -78,8 +78,11 @@ public void onEvent(WorkspaceStatusEvent event) { if (WorkspaceStatusEvent.EventType.STARTING.equals(event.getEventType())) { Subject subject = EnvironmentContext.getCurrent().getSubject(); if (subject == Subject.ANONYMOUS) { - LOG.warn("Workspace {} is being started by the 'anonymous' user.", workspaceId); - return; + throw new IllegalStateException( + "Workspace " + + workspaceId + + " is being started by the 'Anonymous' user.\n" + + "This shouldn't happen, and workspaces should always be created by a real user."); } userIdToWorkspaces.put(subject.getUserId(), workspaceId); updateSubject(subject); From 4a4a4f0495af4446a50bcd59e57896975b0b80bb Mon Sep 17 00:00:00 2001 From: David Festal Date: Thu, 9 Nov 2017 13:25:09 +0100 Subject: [PATCH 11/13] Add unit tests for the `WorkspaceSubjectRegistry` Signed-off-by: David Festal --- .../server/WorkspaceSubjectRegistryTest.java | 150 ++++++++++++++++++ 1 file changed, 150 insertions(+) create mode 100644 wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistryTest.java diff --git a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistryTest.java b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistryTest.java new file mode 100644 index 00000000000..eb71742fd7b --- /dev/null +++ b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistryTest.java @@ -0,0 +1,150 @@ +/* + * Copyright (c) 2012-2017 Red Hat, Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.api.workspace.server; + +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.testng.Assert.*; + +import java.util.Collection; +import java.util.concurrent.Callable; +import java.util.concurrent.Future; +import org.eclipse.che.api.agent.server.AgentRegistry; +import org.eclipse.che.api.agent.server.impl.AgentSorter; +import org.eclipse.che.api.agent.server.launcher.AgentLauncherFactory; +import org.eclipse.che.api.core.notification.EventService; +import org.eclipse.che.api.environment.server.CheEnvironmentEngine; +import org.eclipse.che.api.machine.server.model.impl.SnapshotImpl; +import org.eclipse.che.api.machine.server.spi.SnapshotDao; +import org.eclipse.che.api.workspace.server.model.impl.WorkspaceRuntimeImpl; +import org.eclipse.che.api.workspace.shared.dto.event.WorkspaceStatusEvent; +import org.eclipse.che.commons.env.EnvironmentContext; +import org.eclipse.che.commons.subject.Subject; +import org.eclipse.che.commons.subject.SubjectImpl; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; +import org.mockito.Spy; +import org.mockito.testng.MockitoTestNGListener; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Listeners; +import org.testng.annotations.Test; + +/** @author David Festal */ +@Listeners(MockitoTestNGListener.class) +public class WorkspaceSubjectRegistryTest { + + @Spy private EventService eventService; + @Mock private CheEnvironmentEngine envEngine; + @Mock private AgentSorter agentSorter; + @Mock private AgentLauncherFactory launcherFactory; + @Mock private AgentRegistry agentRegistry; + @Mock private WorkspaceSharedPool sharedPool; + @Mock private SnapshotDao snapshotDao; + @Mock private Future runtimeFuture; + @Mock private WorkspaceRuntimes.StartTask startTask; + @Mock private WorkspaceStatusEvent event; + + @Captor private ArgumentCaptor eventCaptor; + @Captor private ArgumentCaptor> taskCaptor; + @Captor private ArgumentCaptor> snapshotsCaptor; + + private WorkspaceSubjectRegistry workspaceSubjectRegistry; + + @BeforeMethod + public void setUp() throws Exception { + MockitoAnnotations.initMocks(this); + workspaceSubjectRegistry = Mockito.spy(new WorkspaceSubjectRegistry(eventService)); + } + + @Test + public void shouldSubscribeToEventService() throws Exception { + workspaceSubjectRegistry.subscribe(); + verify(eventService).subscribe(workspaceSubjectRegistry); + eventService.publish(event); + verify(workspaceSubjectRegistry).onEvent(event); + } + + public void shouldAddSubjectOnStartingEvent() throws Exception { + final String workspaceId = "myWorkspaceId"; + when(event.getEventType()).thenReturn(WorkspaceStatusEvent.EventType.STARTING); + when(event.getWorkspaceId()).thenReturn(workspaceId); + Subject workspaceStarter = new SubjectImpl("userName", "userId", "userToken", false); + EnvironmentContext.getCurrent().setSubject(workspaceStarter); + workspaceSubjectRegistry.onEvent(event); + assertTrue(workspaceSubjectRegistry.getWorkspaceStarter(workspaceId) == workspaceStarter); + } + + @Test(expectedExceptions = {IllegalStateException.class}) + public void shouldThrowWhenStartingWorkspaceAsAnonymous() throws Exception { + final String workspaceId = "myWorkspaceId"; + when(event.getEventType()).thenReturn(WorkspaceStatusEvent.EventType.STARTING); + when(event.getWorkspaceId()).thenReturn(workspaceId); + EnvironmentContext.getCurrent().setSubject(Subject.ANONYMOUS); + workspaceSubjectRegistry.onEvent(event); + } + + public void shouldUpdateSubjectWithSameId() throws Exception { + final String workspaceId = "myWorkspaceId"; + when(event.getEventType()).thenReturn(WorkspaceStatusEvent.EventType.STARTING); + when(event.getWorkspaceId()).thenReturn(workspaceId); + Subject workspaceStarter = new SubjectImpl("user", "userId", "userToken", false); + EnvironmentContext.getCurrent().setSubject(workspaceStarter); + workspaceSubjectRegistry.onEvent(event); + + Subject updatedSubject = new SubjectImpl("newUserName", "userId", "newUserToken", false); + workspaceSubjectRegistry.updateSubject(updatedSubject); + assertTrue(workspaceSubjectRegistry.getWorkspaceStarter(workspaceId) == updatedSubject); + } + + public void shouldNotUpdateSubjectWithDifferentId() throws Exception { + final String workspaceId = "myWorkspaceId"; + when(event.getEventType()).thenReturn(WorkspaceStatusEvent.EventType.STARTING); + when(event.getWorkspaceId()).thenReturn(workspaceId); + Subject workspaceStarter = new SubjectImpl("user", "userId", "userToken", false); + EnvironmentContext.getCurrent().setSubject(workspaceStarter); + workspaceSubjectRegistry.onEvent(event); + + Subject updatedSubject = new SubjectImpl("newUserName", "newUserId", "newUserToken", false); + workspaceSubjectRegistry.updateSubject(updatedSubject); + assertTrue(workspaceSubjectRegistry.getWorkspaceStarter(workspaceId) == workspaceStarter); + } + + public void shouldRemoveUsersOnWorkspaceStop() throws Exception { + final String workspaceId1 = "myWorkspaceId"; + final String workspaceId2 = "myWorkspaceId2"; + Subject workspaceStarter = new SubjectImpl("user", "userId", "userToken", false); + EnvironmentContext.getCurrent().setSubject(workspaceStarter); + when(event.getEventType()).thenReturn(WorkspaceStatusEvent.EventType.STARTING); + when(event.getWorkspaceId()).thenReturn(workspaceId1); + workspaceSubjectRegistry.onEvent(event); + + when(event.getEventType()).thenReturn(WorkspaceStatusEvent.EventType.STARTING); + when(event.getWorkspaceId()).thenReturn(workspaceId2); + workspaceSubjectRegistry.onEvent(event); + + assertTrue(workspaceSubjectRegistry.getWorkspaceStarter(workspaceId1) == workspaceStarter); + assertTrue(workspaceSubjectRegistry.getWorkspaceStarter(workspaceId2) == workspaceStarter); + + when(event.getEventType()).thenReturn(WorkspaceStatusEvent.EventType.STOPPED); + when(event.getWorkspaceId()).thenReturn(workspaceId2); + workspaceSubjectRegistry.onEvent(event); + + when(event.getEventType()).thenReturn(WorkspaceStatusEvent.EventType.STOPPED); + when(event.getWorkspaceId()).thenReturn(workspaceId1); + workspaceSubjectRegistry.onEvent(event); + + assertNull(workspaceSubjectRegistry.getWorkspaceStarter(workspaceId1) == null); + assertNull(workspaceSubjectRegistry.getWorkspaceStarter(workspaceId2) == null); + } +} From cc969613ece31042561699261281c7813498dc0d Mon Sep 17 00:00:00 2001 From: David Festal Date: Thu, 9 Nov 2017 14:46:04 +0100 Subject: [PATCH 12/13] reduce the stop timeout to 3 minutes Signed-off-by: David Festal --- dockerfiles/init/modules/openshift/files/scripts/deploy_che.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dockerfiles/init/modules/openshift/files/scripts/deploy_che.sh b/dockerfiles/init/modules/openshift/files/scripts/deploy_che.sh index ff4f4ed7361..2714203a625 100755 --- a/dockerfiles/init/modules/openshift/files/scripts/deploy_che.sh +++ b/dockerfiles/init/modules/openshift/files/scripts/deploy_che.sh @@ -267,7 +267,7 @@ echo "done!" # ------------------------------------------------------------- if [ "${COMMAND}" == "cleanup" ]; then echo "[CHE] Stopping the Che server..." - oc scale --replicas=0 --timeout=3h dc che + oc scale --replicas=0 --timeout=3m dc che echo "[CHE] Deleting all OpenShift objects..." oc delete all --all echo "[CHE] Cleanup successfully started. Use \"oc get all\" to verify that all resources have been deleted." From e7050f018d55017b213d19e2523fe02f2fc6b51c Mon Sep 17 00:00:00 2001 From: David Festal Date: Fri, 10 Nov 2017 09:39:16 +0100 Subject: [PATCH 13/13] Also test users are cleaned when all the related workspaces are stopped Signed-off-by: David Festal --- .../api/workspace/server/WorkspaceSubjectRegistry.java | 10 +++++++++- .../workspace/server/WorkspaceSubjectRegistryTest.java | 4 ++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistry.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistry.java index da413fb6350..61c25c8d1dd 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistry.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistry.java @@ -71,8 +71,8 @@ public void onEvent(WorkspaceStatusEvent event) { String workspaceId = event.getWorkspaceId(); if (WorkspaceStatusEvent.EventType.STOPPED.equals(event.getEventType())) { - workspaceOwners.remove(workspaceId); while (userIdToWorkspaces.values().remove(workspaceId)) {} + workspaceOwners.remove(workspaceId); } if (WorkspaceStatusEvent.EventType.STARTING.equals(event.getEventType())) { @@ -128,4 +128,12 @@ public void updateSubject(Subject subject) { lock.readLock().unlock(); } } + + /* + * Only for tests + */ + @VisibleForTesting + boolean isUserKnown(String userId) { + return userIdToWorkspaces.containsKey(userId); + } } diff --git a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistryTest.java b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistryTest.java index eb71742fd7b..f1cc260ad27 100644 --- a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistryTest.java +++ b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/WorkspaceSubjectRegistryTest.java @@ -140,10 +140,14 @@ public void shouldRemoveUsersOnWorkspaceStop() throws Exception { when(event.getWorkspaceId()).thenReturn(workspaceId2); workspaceSubjectRegistry.onEvent(event); + assertTrue(workspaceSubjectRegistry.isUserKnown(workspaceStarter.getUserId())); + when(event.getEventType()).thenReturn(WorkspaceStatusEvent.EventType.STOPPED); when(event.getWorkspaceId()).thenReturn(workspaceId1); workspaceSubjectRegistry.onEvent(event); + assertFalse(workspaceSubjectRegistry.isUserKnown(workspaceStarter.getUserId())); + assertNull(workspaceSubjectRegistry.getWorkspaceStarter(workspaceId1) == null); assertNull(workspaceSubjectRegistry.getWorkspaceStarter(workspaceId2) == null); }