From 8a10b84875f80fab400cffa22290ec2f3601e224 Mon Sep 17 00:00:00 2001 From: Michal Vala Date: Mon, 5 Oct 2020 14:22:15 +0200 Subject: [PATCH] fixes #17999 cleanup che namespace only when needed (#18021) --- .../kubernetes/KubernetesInternalRuntime.java | 16 ++-- .../kubernetes/RuntimeCleaner.java | 61 ++++++++++++++++ .../KubernetesInternalRuntimeTest.java | 35 ++++----- .../kubernetes/RuntimeCleanerTest.java | 73 +++++++++++++++++++ .../openshift/OpenShiftInternalRuntime.java | 3 + .../OpenShiftInternalRuntimeTest.java | 3 + 6 files changed, 161 insertions(+), 30 deletions(-) create mode 100644 infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/RuntimeCleaner.java create mode 100644 infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/RuntimeCleanerTest.java diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntime.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntime.java index 87900aa506..85e70bdd82 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntime.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntime.java @@ -133,6 +133,7 @@ public class KubernetesInternalRuntime private final PreviewUrlCommandProvisioner previewUrlCommandProvisioner; private final SecretAsContainerResourceProvisioner secretAsContainerResourceProvisioner; private final KubernetesServerResolverFactory serverResolverFactory; + private final RuntimeCleaner runtimeCleaner; protected final CheNamespace cheNamespace; protected final Tracer tracer; @@ -158,6 +159,7 @@ public class KubernetesInternalRuntime PreviewUrlCommandProvisioner previewUrlCommandProvisioner, SecretAsContainerResourceProvisioner secretAsContainerResourceProvisioner, KubernetesServerResolverFactory kubernetesServerResolverFactory, + RuntimeCleaner runtimeCleaner, CheNamespace cheNamespace, Tracer tracer, @Assisted KubernetesRuntimeContext context, @@ -184,6 +186,7 @@ public class KubernetesInternalRuntime this.previewUrlCommandProvisioner = previewUrlCommandProvisioner; this.secretAsContainerResourceProvisioner = secretAsContainerResourceProvisioner; this.serverResolverFactory = kubernetesServerResolverFactory; + this.runtimeCleaner = runtimeCleaner; this.tracer = tracer; } @@ -195,7 +198,7 @@ public class KubernetesInternalRuntime startSynchronizer.setStartThread(); startSynchronizer.start(); - cleanUp(workspaceId); + runtimeCleaner.cleanUp(namespace, workspaceId); provisionWorkspace(startOptions, context, workspaceId); volumesStrategy.prepare( @@ -260,7 +263,7 @@ public class KubernetesInternalRuntime // stop watching before namespace cleaning up namespace.deployments().stopWatch(true); try { - cleanUp(workspaceId); + runtimeCleaner.cleanUp(namespace, workspaceId); } catch (InfrastructureException cleanUppingEx) { LOG.warn( "Failed to clean up namespace after workspace '{}' start failing. Cause: {}", @@ -277,11 +280,6 @@ public class KubernetesInternalRuntime } } - private void cleanUp(String workspaceId) throws InfrastructureException { - namespace.cleanUp(); - cheNamespace.cleanUp(workspaceId); - } - protected void provisionWorkspace( Map startOptions, KubernetesRuntimeContext context, String workspaceId) throws InfrastructureException { @@ -594,7 +592,7 @@ public class KubernetesInternalRuntime // Che Server that is crashed so start is hung up in STOPPING phase. // Need to clean up runtime resources probeScheduler.cancel(identity.getWorkspaceId()); - cleanUp(identity.getWorkspaceId()); + runtimeCleaner.cleanUp(namespace, identity.getWorkspaceId()); } } catch (InterruptedException e) { throw new InfrastructureException( @@ -604,7 +602,7 @@ public class KubernetesInternalRuntime // runtime is RUNNING. Clean up used resources // Cancels workspace servers probes if any probeScheduler.cancel(identity.getWorkspaceId()); - cleanUp(identity.getWorkspaceId()); + runtimeCleaner.cleanUp(namespace, identity.getWorkspaceId()); } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/RuntimeCleaner.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/RuntimeCleaner.java new file mode 100644 index 0000000000..2f167ceae2 --- /dev/null +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/RuntimeCleaner.java @@ -0,0 +1,61 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.workspace.infrastructure.kubernetes; + +import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.SingleHostExternalServiceExposureStrategy.SINGLE_HOST_STRATEGY; + +import javax.inject.Inject; +import javax.inject.Named; +import javax.inject.Singleton; +import org.eclipse.che.api.workspace.server.spi.InfrastructureException; +import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.CheNamespace; +import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespace; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.WorkspaceExposureType; + +/** + * Little helper bean that decides what is needed to cleanup for given workspace. It does not delete + * anything on it's own, but delegates the cleaning operations to provided {@link + * KubernetesNamespace} and {@link CheNamespace}. + */ +@Singleton +public class RuntimeCleaner { + private final boolean cleanupCheNamespace; + private final CheNamespace cheNamespace; + + @Inject + public RuntimeCleaner( + @Named("che.infra.kubernetes.server_strategy") String exposureStrategy, + @Named("che.infra.kubernetes.singlehost.workspace.exposure") String singleHostStrategy, + CheNamespace cheNamespace) { + this.cheNamespace = cheNamespace; + + this.cleanupCheNamespace = + SINGLE_HOST_STRATEGY.equals(exposureStrategy) + && WorkspaceExposureType.GATEWAY.getConfigValue().equals(singleHostStrategy); + } + + /** + * Remove all workspace related k8s objects in both workspace's namespace and in Che namespace if + * needed. + * + * @param namespace to cleanup + * @param workspaceId to cleanup + * @throws InfrastructureException when exception during cleaning occurs. + */ + public void cleanUp(KubernetesNamespace namespace, String workspaceId) + throws InfrastructureException { + namespace.cleanUp(); + if (cleanupCheNamespace) { + cheNamespace.cleanUp(workspaceId); + } + } +} diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java index a6e7b4f966..0a932323e4 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java @@ -216,6 +216,7 @@ public class KubernetesInternalRuntimeTest { @Mock private RuntimeHangingDetector runtimeHangingDetector; @Mock private KubernetesPreviewUrlCommandProvisioner previewUrlCommandProvisioner; @Mock private SecretAsContainerResourceProvisioner secretAsContainerResourceProvisioner; + @Mock private RuntimeCleaner runtimeCleaner; private KubernetesServerResolverFactory serverResolverFactory; @Mock @@ -290,6 +291,7 @@ public class KubernetesInternalRuntimeTest { previewUrlCommandProvisioner, secretAsContainerResourceProvisioner, serverResolverFactory, + runtimeCleaner, cheNamespace, tracer, context, @@ -431,8 +433,7 @@ public class KubernetesInternalRuntimeTest { verify(services).create(any()); verify(secrets).create(any()); verify(configMaps).create(any()); - verify(namespace).cleanUp(); - verify(cheNamespace).cleanUp(WORKSPACE_ID); + verify(runtimeCleaner).cleanUp(namespace, WORKSPACE_ID); verify(namespace.deployments(), times(1)).watchEvents(any()); verify(eventService, times(4)).publish(any()); verifyOrderedEventsChains( @@ -449,9 +450,8 @@ public class KubernetesInternalRuntimeTest { internalRuntime.start(emptyMap()); InOrder cleanupInOrderExecutionVerification = - Mockito.inOrder(namespace, cheNamespace, deployments, toolingProvisioner); - cleanupInOrderExecutionVerification.verify(namespace).cleanUp(); - cleanupInOrderExecutionVerification.verify(cheNamespace).cleanUp(WORKSPACE_ID); + Mockito.inOrder(runtimeCleaner, deployments, toolingProvisioner); + cleanupInOrderExecutionVerification.verify(runtimeCleaner).cleanUp(namespace, WORKSPACE_ID); cleanupInOrderExecutionVerification .verify(toolingProvisioner) .provision(any(), any(), any(), any()); @@ -654,8 +654,7 @@ public class KubernetesInternalRuntimeTest { try { internalRuntime.start(emptyMap()); } catch (Exception rethrow) { - verify(namespace, times(2)).cleanUp(); - verify(cheNamespace, times(2)).cleanUp(WORKSPACE_ID); + verify(runtimeCleaner, times(2)).cleanUp(namespace, WORKSPACE_ID); verify(namespace, never()).services(); verify(namespace, never()).ingresses(); throw rethrow; @@ -693,7 +692,10 @@ public class KubernetesInternalRuntimeTest { @Test(expectedExceptions = InfrastructureException.class) public void throwsInfrastructureExceptionWhenErrorOccursAndCleanupFailed() throws Exception { - doNothing().doThrow(InfrastructureException.class).when(namespace).cleanUp(); + doNothing() + .doThrow(InfrastructureException.class) + .when(runtimeCleaner) + .cleanUp(namespace, WORKSPACE_ID); when(k8sEnv.getServices()).thenReturn(singletonMap("testService", mock(Service.class))); when(services.create(any())).thenThrow(new InfrastructureException("service creation failed")); doThrow(IllegalStateException.class).when(namespace).services(); @@ -701,7 +703,7 @@ public class KubernetesInternalRuntimeTest { try { internalRuntime.start(emptyMap()); } catch (Exception rethrow) { - verify(namespace, times(2)).cleanUp(); + verify(runtimeCleaner, times(2)).cleanUp(namespace, WORKSPACE_ID); verify(namespace).services(); verify(namespace, never()).ingresses(); throw rethrow; @@ -734,21 +736,12 @@ public class KubernetesInternalRuntimeTest { internalRuntime.internalStop(emptyMap()); verify(runtimeHangingDetector).stopTracking(IDENTITY); - verify(namespace).cleanUp(); - verify(cheNamespace).cleanUp(WORKSPACE_ID); + verify(runtimeCleaner).cleanUp(namespace, WORKSPACE_ID); } @Test(expectedExceptions = InfrastructureException.class) public void throwsInfrastructureExceptionWhenKubernetesNamespaceCleanupFailed() throws Exception { - doThrow(InfrastructureException.class).when(namespace).cleanUp(); - - internalRuntime.internalStop(emptyMap()); - } - - @Test(expectedExceptions = InfrastructureException.class) - public void throwsInfrastructureExceptionWhenKubernetesCheNamespaceCleanupFailed() - throws Exception { - doThrow(InfrastructureException.class).when(cheNamespace).cleanUp(WORKSPACE_ID); + doThrow(InfrastructureException.class).when(runtimeCleaner).cleanUp(namespace, WORKSPACE_ID); internalRuntime.internalStop(emptyMap()); } @@ -1161,7 +1154,7 @@ public class KubernetesInternalRuntimeTest { verify(cheNamespace) .createConfigMaps( expectedCreatedCheNamespaceConfigmaps, internalRuntime.getContext().getIdentity()); - verify(cheNamespace).cleanUp(WORKSPACE_ID); + verify(runtimeCleaner).cleanUp(namespace, WORKSPACE_ID); } private static MachineStatusEvent newEvent(String machineName, MachineStatus status) { diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/RuntimeCleanerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/RuntimeCleanerTest.java new file mode 100644 index 0000000000..61dd7f67cf --- /dev/null +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/RuntimeCleanerTest.java @@ -0,0 +1,73 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.workspace.infrastructure.kubernetes; + +import static org.eclipse.che.workspace.infrastructure.kubernetes.server.WorkspaceExposureType.GATEWAY; +import static org.eclipse.che.workspace.infrastructure.kubernetes.server.WorkspaceExposureType.NATIVE; +import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.DefaultHostExternalServiceExposureStrategy.DEFAULT_HOST_STRATEGY; +import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.MultiHostExternalServiceExposureStrategy.MULTI_HOST_STRATEGY; +import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.SingleHostExternalServiceExposureStrategy.SINGLE_HOST_STRATEGY; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.testng.Assert.*; + +import org.eclipse.che.api.workspace.server.spi.InfrastructureException; +import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.CheNamespace; +import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespace; +import org.mockito.Mock; +import org.mockito.testng.MockitoTestNGListener; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Listeners; +import org.testng.annotations.Test; + +@Listeners(MockitoTestNGListener.class) +public class RuntimeCleanerTest { + private final String WS_ID = "123"; + + @Mock private CheNamespace cheNamespace; + @Mock private KubernetesNamespace kubeNamespace; + + @Test + public void shouldCleanupEverythingIfSinglehostGateway() throws InfrastructureException { + RuntimeCleaner runtimeCleaner = + new RuntimeCleaner(SINGLE_HOST_STRATEGY, GATEWAY.getConfigValue(), cheNamespace); + + runtimeCleaner.cleanUp(kubeNamespace, WS_ID); + + verify(cheNamespace).cleanUp(WS_ID); + verify(kubeNamespace).cleanUp(); + } + + @Test(dataProvider = "notSinglehostGateway") + public void shouldNotCleanupCheNamespaceIfNotSinglehostGateway( + String serverStrategy, String singleHostStrategy) throws InfrastructureException { + RuntimeCleaner runtimeCleaner = + new RuntimeCleaner(serverStrategy, singleHostStrategy, cheNamespace); + + runtimeCleaner.cleanUp(kubeNamespace, WS_ID); + + verify(kubeNamespace).cleanUp(); + verify(cheNamespace, never()).cleanUp(any()); + } + + @DataProvider + public static Object[][] notSinglehostGateway() { + return new Object[][] { + {SINGLE_HOST_STRATEGY, NATIVE.getConfigValue()}, + {MULTI_HOST_STRATEGY, GATEWAY.getConfigValue()}, + {MULTI_HOST_STRATEGY, NATIVE.getConfigValue()}, + {DEFAULT_HOST_STRATEGY, GATEWAY.getConfigValue()}, + {DEFAULT_HOST_STRATEGY, NATIVE.getConfigValue()}, + }; + } +} diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntime.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntime.java index 883b7ac145..23967d0165 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntime.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntime.java @@ -34,6 +34,7 @@ import org.eclipse.che.commons.annotation.Traced; import org.eclipse.che.commons.tracing.TracingTags; import org.eclipse.che.workspace.infrastructure.kubernetes.KubernetesInternalRuntime; import org.eclipse.che.workspace.infrastructure.kubernetes.KubernetesRuntimeContext; +import org.eclipse.che.workspace.infrastructure.kubernetes.RuntimeCleaner; import org.eclipse.che.workspace.infrastructure.kubernetes.RuntimeHangingDetector; import org.eclipse.che.workspace.infrastructure.kubernetes.StartSynchronizerFactory; import org.eclipse.che.workspace.infrastructure.kubernetes.cache.KubernetesMachineCache; @@ -84,6 +85,7 @@ public class OpenShiftInternalRuntime extends KubernetesInternalRuntime secretAsContainerResourceProvisioner, OpenShiftServerResolverFactory serverResolverFactory, + RuntimeCleaner runtimeCleaner, CheNamespace cheNamespace, Tracer tracer, Openshift4TrustedCAProvisioner trustedCAProvisioner, @@ -110,6 +112,7 @@ public class OpenShiftInternalRuntime extends KubernetesInternalRuntime