diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactory.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactory.java index a0c41805d5b3..e4dc2f63dcf2 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactory.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactory.java @@ -38,6 +38,14 @@ public KubernetesNamespaceFactory( this.clientFactory = clientFactory; } + /** + * Returns true if namespace is predefined for all workspaces or false if each workspace will + * be provided with a new namespace. + */ + public boolean isPredefined() { + return isNullOrEmpty(namespaceName); + } + /** * Creates a Kubernetes namespace for the specified workspace. * diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesPersistentVolumeClaims.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesPersistentVolumeClaims.java index b85cadca4025..77d0a19a23e7 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesPersistentVolumeClaims.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesPersistentVolumeClaims.java @@ -16,6 +16,7 @@ import io.fabric8.kubernetes.client.KubernetesClientException; import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.Set; import org.eclipse.che.api.workspace.server.spi.InfrastructureException; import org.eclipse.che.workspace.infrastructure.kubernetes.KubernetesClientFactory; @@ -28,6 +29,7 @@ * @author Sergii Leshchenko */ public class KubernetesPersistentVolumeClaims { + private final String namespace; private final String workspaceId; private final KubernetesClientFactory clientFactory; @@ -115,4 +117,23 @@ public void createIfNotExist(Collection toCreate) } } } + + /** + * Removes all PVCs which have the specified labels. + * + * @param labels labels to filter PVCs + * @throws InfrastructureException when any error occurs while removing + */ + public void delete(Map labels) throws InfrastructureException { + try { + clientFactory + .create(workspaceId) + .persistentVolumeClaims() + .inNamespace(namespace) + .withLabels(labels) + .delete(); + } catch (KubernetesClientException e) { + throw new KubernetesInfrastructureException(e); + } + } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/pvc/UniqueWorkspacePVCStrategy.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/pvc/UniqueWorkspacePVCStrategy.java index 1f2ad36ad465..35b730d60a88 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/pvc/UniqueWorkspacePVCStrategy.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/pvc/UniqueWorkspacePVCStrategy.java @@ -18,6 +18,7 @@ import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesObjectUtil.putLabel; import static org.eclipse.che.workspace.infrastructure.kubernetes.provision.LogsVolumeMachineProvisioner.LOGS_VOLUME_NAME; +import com.google.common.collect.ImmutableMap; import io.fabric8.kubernetes.api.model.Container; import io.fabric8.kubernetes.api.model.ObjectMeta; import io.fabric8.kubernetes.api.model.PersistentVolumeClaim; @@ -32,8 +33,6 @@ import org.eclipse.che.api.core.model.workspace.config.Volume; import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity; import org.eclipse.che.api.workspace.server.spi.InfrastructureException; -import org.eclipse.che.commons.annotation.Nullable; -import org.eclipse.che.workspace.infrastructure.kubernetes.KubernetesClientFactory; import org.eclipse.che.workspace.infrastructure.kubernetes.Names; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespaceFactory; @@ -65,25 +64,19 @@ public class UniqueWorkspacePVCStrategy implements WorkspaceVolumesStrategy { public static final String UNIQUE_STRATEGY = "unique"; private final String pvcNamePrefix; - private final String namespaceName; private final String pvcQuantity; private final String pvcAccessMode; - private final KubernetesClientFactory clientFactory; private final KubernetesNamespaceFactory factory; @Inject public UniqueWorkspacePVCStrategy( - @Nullable @Named("che.infra.kubernetes.namespace") String namespaceName, @Named("che.infra.kubernetes.pvc.name") String pvcNamePrefix, @Named("che.infra.kubernetes.pvc.quantity") String pvcQuantity, @Named("che.infra.kubernetes.pvc.access_mode") String pvcAccessMode, - KubernetesNamespaceFactory factory, - KubernetesClientFactory clientFactory) { + KubernetesNamespaceFactory factory) { this.pvcNamePrefix = pvcNamePrefix; this.pvcQuantity = pvcQuantity; - this.namespaceName = namespaceName; this.pvcAccessMode = pvcAccessMode; - this.clientFactory = clientFactory; this.factory = factory; } @@ -173,12 +166,10 @@ else if (provisionedVolumeName2PVC.containsKey(volumeName)) { @Override public void cleanup(String workspaceId) throws InfrastructureException { - clientFactory + factory .create(workspaceId) .persistentVolumeClaims() - .inNamespace(namespaceName) - .withLabel(CHE_WORKSPACE_ID_LABEL, workspaceId) - .delete(); + .delete(ImmutableMap.of(CHE_WORKSPACE_ID_LABEL, workspaceId)); } private void addVolumeIfAbsent(PodSpec podSpec, String pvcUniqueName) { diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/pvc/WorkspacePVCCleaner.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/pvc/WorkspacePVCCleaner.java index 4a21b7e6d68d..281870f77c33 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/pvc/WorkspacePVCCleaner.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/pvc/WorkspacePVCCleaner.java @@ -10,16 +10,13 @@ */ package org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc; -import static com.google.common.base.Strings.isNullOrEmpty; - import com.google.inject.Inject; import com.google.inject.Singleton; import javax.inject.Named; import org.eclipse.che.api.core.notification.EventService; -import org.eclipse.che.api.core.notification.EventSubscriber; import org.eclipse.che.api.workspace.server.spi.InfrastructureException; import org.eclipse.che.api.workspace.shared.event.WorkspaceRemovedEvent; -import org.eclipse.che.commons.annotation.Nullable; +import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespaceFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -38,36 +35,32 @@ public class WorkspacePVCCleaner { private static final Logger LOG = LoggerFactory.getLogger(WorkspacePVCCleaner.class); private final boolean pvcEnabled; - private final String namespaceName; private final WorkspaceVolumesStrategy strategy; + private final KubernetesNamespaceFactory namespaceFactory; @Inject public WorkspacePVCCleaner( @Named("che.infra.kubernetes.pvc.enabled") boolean pvcEnabled, - @Nullable @Named("che.infra.kubernetes.namespace") String namespaceName, + KubernetesNamespaceFactory namespaceFactory, WorkspaceVolumesStrategy pvcStrategy) { this.pvcEnabled = pvcEnabled; - this.namespaceName = namespaceName; + this.namespaceFactory = namespaceFactory; this.strategy = pvcStrategy; } @Inject public void subscribe(EventService eventService) { - if (pvcEnabled && !isNullOrEmpty(namespaceName)) + if (pvcEnabled && !namespaceFactory.isPredefined()) eventService.subscribe( - new EventSubscriber() { - @Override - public void onEvent(WorkspaceRemovedEvent event) { - final String workspaceId = event.getWorkspace().getId(); - try { - strategy.cleanup(workspaceId); - } catch (InfrastructureException ex) { - LOG.error( - "Failed to cleanup workspace '{}' data. Cause: {}", - workspaceId, - ex.getMessage()); - } + event -> { + final String workspaceId = event.getWorkspace().getId(); + try { + strategy.cleanup(workspaceId); + } catch (InfrastructureException ex) { + LOG.error( + "Failed to cleanup workspace '{}' data. Cause: {}", workspaceId, ex.getMessage()); } - }); + }, + WorkspaceRemovedEvent.class); } } diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/pvc/UniqueWorkspacePVCStrategyTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/pvc/UniqueWorkspacePVCStrategyTest.java index edf1bc64b4f4..e4218cd47867 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/pvc/UniqueWorkspacePVCStrategyTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/pvc/UniqueWorkspacePVCStrategyTest.java @@ -17,7 +17,6 @@ import static org.eclipse.che.workspace.infrastructure.kubernetes.Constants.CHE_WORKSPACE_ID_LABEL; import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.CommonPVCStrategyTest.mockName; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; @@ -34,10 +33,6 @@ import io.fabric8.kubernetes.api.model.PersistentVolumeClaim; import io.fabric8.kubernetes.api.model.Pod; import io.fabric8.kubernetes.api.model.PodSpec; -import io.fabric8.kubernetes.client.KubernetesClient; -import io.fabric8.kubernetes.client.dsl.FilterWatchListDeletable; -import io.fabric8.kubernetes.client.dsl.MixedOperation; -import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation; import java.util.ArrayList; import java.util.HashMap; import java.util.Map; @@ -47,7 +42,6 @@ import org.eclipse.che.api.workspace.server.model.impl.VolumeImpl; import org.eclipse.che.api.workspace.server.spi.InfrastructureException; import org.eclipse.che.api.workspace.server.spi.environment.InternalMachineConfig; -import org.eclipse.che.workspace.infrastructure.kubernetes.KubernetesClientFactory; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespace; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespaceFactory; @@ -67,7 +61,6 @@ public class UniqueWorkspacePVCStrategyTest { private static final String WORKSPACE_ID = "workspace123"; - private static final String NAMESPACE_NAME = "che"; private static final String PVC_NAME_PREFIX = "che-claim"; private static final String POD_NAME = "main"; private static final String POD_NAME_2 = "second"; @@ -86,8 +79,6 @@ public class UniqueWorkspacePVCStrategyTest { new RuntimeIdentityImpl(WORKSPACE_ID, "env1", "id1"); @Mock private KubernetesEnvironment k8sEnv; - @Mock private KubernetesClientFactory clientFactory; - @Mock private KubernetesClient client; @Mock private KubernetesNamespaceFactory factory; @Mock private KubernetesNamespace k8sNamespace; @Mock private KubernetesPersistentVolumeClaims pvcs; @@ -104,10 +95,7 @@ public class UniqueWorkspacePVCStrategyTest { @BeforeMethod public void setup() throws Exception { strategy = - new UniqueWorkspacePVCStrategy( - NAMESPACE_NAME, PVC_NAME_PREFIX, PVC_QUANTITY, PVC_ACCESS_MODE, factory, clientFactory); - when(clientFactory.create()).thenReturn(client); - when(clientFactory.create(anyString())).thenReturn(client); + new UniqueWorkspacePVCStrategy(PVC_NAME_PREFIX, PVC_QUANTITY, PVC_ACCESS_MODE, factory); Map machines = new HashMap<>(); InternalMachineConfig machine1 = mock(InternalMachineConfig.class); @@ -239,32 +227,9 @@ public void throwsInfrastructureExceptionWhenFailedToCreatePVCs() throws Excepti @Test public void testRemovesPVCWhenCleanupCalled() throws Exception { - final MixedOperation mixedOperation = mock(MixedOperation.class); - final NonNamespaceOperation namespace = mock(NonNamespaceOperation.class); - final FilterWatchListDeletable filterList = mock(FilterWatchListDeletable.class); - doReturn(mixedOperation).when(client).persistentVolumeClaims(); - doReturn(namespace).when(mixedOperation).inNamespace(NAMESPACE_NAME); - doReturn(filterList).when(namespace).withLabel(CHE_WORKSPACE_ID_LABEL, WORKSPACE_ID); - when(filterList.delete()).thenReturn(true); - - strategy.cleanup(WORKSPACE_ID); - - verify(filterList).delete(); - } - - @Test - public void testDoNothingWhenNoPVCFoundInNamespaceOnCleanup() throws Exception { - final MixedOperation mixedOperation = mock(MixedOperation.class); - final NonNamespaceOperation namespace = mock(NonNamespaceOperation.class); - final FilterWatchListDeletable filterList = mock(FilterWatchListDeletable.class); - doReturn(mixedOperation).when(client).persistentVolumeClaims(); - doReturn(namespace).when(mixedOperation).inNamespace(NAMESPACE_NAME); - doReturn(filterList).when(namespace).withLabel(CHE_WORKSPACE_ID_LABEL, WORKSPACE_ID); - when(filterList.delete()).thenReturn(false); - strategy.cleanup(WORKSPACE_ID); - verify(filterList).delete(); + verify(pvcs).delete(ImmutableMap.of(CHE_WORKSPACE_ID_LABEL, WORKSPACE_ID)); } private static PersistentVolumeClaim mockPVC(Map labels, String name) { diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/pvc/WorkspacePVCCleanerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/pvc/WorkspacePVCCleanerTest.java index 1ad774f3fdab..2306f43661a8 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/pvc/WorkspacePVCCleanerTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/pvc/WorkspacePVCCleanerTest.java @@ -11,6 +11,7 @@ package org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; @@ -24,6 +25,7 @@ import org.eclipse.che.api.core.notification.EventSubscriber; import org.eclipse.che.api.workspace.server.spi.InfrastructureException; import org.eclipse.che.api.workspace.shared.event.WorkspaceRemovedEvent; +import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespaceFactory; import org.mockito.Mock; import org.mockito.testng.MockitoTestNGListener; import org.testng.annotations.BeforeMethod; @@ -39,32 +41,43 @@ public class WorkspacePVCCleanerTest { private static final String WORKSPACE_ID = "workspace132"; - private static final String NAMESPACE_NAME = "che"; @Mock private WorkspaceVolumesStrategy pvcStrategy; @Mock private EventService eventService; + @Mock private KubernetesNamespaceFactory namespaceFactory; private WorkspacePVCCleaner workspacePVCCleaner; @BeforeMethod public void setUp() { - workspacePVCCleaner = new WorkspacePVCCleaner(true, NAMESPACE_NAME, pvcStrategy); + when(namespaceFactory.isPredefined()).thenReturn(false); + workspacePVCCleaner = new WorkspacePVCCleaner(true, namespaceFactory, pvcStrategy); } @Test public void testDoNotSubscribesCleanerWhenPVCDisabled() { - workspacePVCCleaner = spy(new WorkspacePVCCleaner(false, NAMESPACE_NAME, pvcStrategy)); + workspacePVCCleaner = spy(new WorkspacePVCCleaner(false, namespaceFactory, pvcStrategy)); workspacePVCCleaner.subscribe(eventService); - verify(eventService, never()).subscribe(any()); + verify(eventService, never()).subscribe(any(), eq(WorkspaceRemovedEvent.class)); + } + + @Test + public void testDoNotSubscribesCleanerWhenPVCEnabledAndNamespaceIsPredefined() { + when(namespaceFactory.isPredefined()).thenReturn(true); + workspacePVCCleaner = spy(new WorkspacePVCCleaner(false, namespaceFactory, pvcStrategy)); + + workspacePVCCleaner.subscribe(eventService); + + verify(eventService, never()).subscribe(any(), eq(WorkspaceRemovedEvent.class)); } @Test public void testSubscribesDeleteKubernetesOnWorkspaceRemove() throws Exception { workspacePVCCleaner.subscribe(eventService); - verify(eventService).subscribe(any(EventSubscriber.class)); + verify(eventService).subscribe(any(), eq(WorkspaceRemovedEvent.class)); } @Test @@ -81,7 +94,7 @@ public void testInvokeCleanupWhenWorkspaceRemovedEventPublished() throws Excepti return invocationOnMock; }) .when(eventService) - .subscribe(any()); + .subscribe(any(), eq(WorkspaceRemovedEvent.class)); workspacePVCCleaner.subscribe(eventService); @@ -102,7 +115,7 @@ public void testDoNotRethrowExceptionWhenErrorOnCleanupOccurs() throws Exception return invocationOnMock; }) .when(eventService) - .subscribe(any()); + .subscribe(any(), eq(WorkspaceRemovedEvent.class)); doThrow(InfrastructureException.class).when(pvcStrategy).cleanup(WORKSPACE_ID); workspacePVCCleaner.subscribe(eventService);