feat: Deleting the common PVC if there are no workspaces left for a given user (#16)

* che #18369 Deleting the common PVC if there are no workspaces left for a given user

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>

* che #18369 Handling a case when the common PVC is used across multiple users ('workspaceNamespaceDefault' without placeholders is defined)

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>

* che #18369 Improving the case when the subject is anonymous

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>

* Use DeletionPropagation.BACKGROUND

* Remove @Nullable

Signed-off-by: David Kwon <dakwon@redhat.com>

* Throw RuntimeException

* Remove 'che.infra.kubernetes.namespace.default' and placeholder check

Signed-off-by: David Kwon <dakwon@redhat.com>

* Remove lenient() from tests

Signed-off-by: David Kwon <dakwon@redhat.com>

* Remove anonymous subject check

Signed-off-by: David Kwon <dakwon@redhat.com>

* Use InfrastructureException

Signed-off-by: David Kwon <dakwon@redhat.com>

* Remove test

Signed-off-by: David Kwon <dakwon@redhat.com>

* Remove unnecessary variable, get workspaces from AccountImpl

Signed-off-by: David Kwon <dakwon@redhat.com>

* Create CommonPVCStrategy#isPersonalAccount()

Signed-off-by: David Kwon <dakwon@redhat.com>

Co-authored-by: Ilya Buziuk <ibuziuk@redhat.com>
pull/24/head
David Kwon 2021-06-07 08:48:31 -04:00 committed by GitHub
parent a4f454b518
commit 30a8e4832e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 218 additions and 31 deletions

View File

@ -129,6 +129,10 @@
<groupId>javax.ws.rs</groupId>
<artifactId>javax.ws.rs-api</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.che.core</groupId>
<artifactId>che-core-api-account</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.che.core</groupId>
<artifactId>che-core-api-core</artifactId>
@ -197,6 +201,10 @@
<groupId>org.eclipse.che.multiuser</groupId>
<artifactId>che-multiuser-machine-authentication</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.che.multiuser</groupId>
<artifactId>che-multiuser-personal-account</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.persistence</groupId>
<artifactId>javax.persistence</artifactId>
@ -230,11 +238,6 @@
<artifactId>kubernetes-server-mock</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.che.core</groupId>
<artifactId>che-core-api-account</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.che.core</groupId>
<artifactId>che-core-commons-test</artifactId>

View File

@ -159,6 +159,26 @@ public class KubernetesPersistentVolumeClaims {
}
}
/**
* Removes PVC by name.
*
* @param name the name of the PVC
* @throws InfrastructureException when any error occurs while removing
*/
public void delete(final String name) throws InfrastructureException {
try {
clientFactory
.create(workspaceId)
.persistentVolumeClaims()
.inNamespace(namespace)
.withName(name)
.withPropagationPolicy(BACKGROUND)
.delete();
} catch (KubernetesClientException e) {
throw new KubernetesInfrastructureException(e);
}
}
/**
* Waits until persistent volume claim state is bound. If used k8s Storage Class has
* 'volumeBindingMode: WaitForFirstConsumer', we don't wait to avoid deadlock.

View File

@ -15,6 +15,7 @@ import static com.google.common.base.Strings.isNullOrEmpty;
import static java.lang.String.format;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toSet;
import static org.eclipse.che.api.user.server.UserManager.PERSONAL_ACCOUNT;
import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesObjectUtil.newPVC;
import com.google.inject.Inject;
@ -24,8 +25,13 @@ import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import javax.inject.Named;
import org.eclipse.che.account.spi.AccountImpl;
import org.eclipse.che.api.core.Page;
import org.eclipse.che.api.core.ServerException;
import org.eclipse.che.api.core.model.workspace.Workspace;
import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity;
import org.eclipse.che.api.workspace.server.WorkspaceManager;
import org.eclipse.che.api.workspace.server.model.impl.WorkspaceImpl;
import org.eclipse.che.api.workspace.server.spi.InfrastructureException;
import org.eclipse.che.commons.annotation.Traced;
import org.eclipse.che.commons.tracing.TracingTags;
@ -70,7 +76,6 @@ import org.slf4j.LoggerFactory;
* @author Alexander Garagatyi
*/
public class CommonPVCStrategy implements WorkspaceVolumesStrategy {
// use non-static variable to reuse child class logger
private final Logger log = LoggerFactory.getLogger(getClass());
@ -96,6 +101,7 @@ public class CommonPVCStrategy implements WorkspaceVolumesStrategy {
private final PodsVolumes podsVolumes;
private final SubPathPrefixes subpathPrefixes;
private final boolean waitBound;
private final WorkspaceManager workspaceManager;
@Inject
public CommonPVCStrategy(
@ -110,7 +116,8 @@ public class CommonPVCStrategy implements WorkspaceVolumesStrategy {
EphemeralWorkspaceAdapter ephemeralWorkspaceAdapter,
PVCProvisioner pvcProvisioner,
PodsVolumes podsVolumes,
SubPathPrefixes subpathPrefixes) {
SubPathPrefixes subpathPrefixes,
WorkspaceManager workspaceManager) {
this.configuredPVCName = configuredPVCName;
this.pvcQuantity = pvcQuantity;
this.pvcAccessMode = pvcAccessMode;
@ -123,6 +130,7 @@ public class CommonPVCStrategy implements WorkspaceVolumesStrategy {
this.pvcProvisioner = pvcProvisioner;
this.podsVolumes = podsVolumes;
this.subpathPrefixes = subpathPrefixes;
this.workspaceManager = workspaceManager;
}
/**
@ -230,6 +238,14 @@ public class CommonPVCStrategy implements WorkspaceVolumesStrategy {
if (EphemeralWorkspaceUtility.isEphemeral(workspace)) {
return;
}
AccountImpl account = ((WorkspaceImpl) workspace).getAccount();
if (isPersonalAccount(account) && accountHasNoWorkspaces(account)) {
log.debug("Deleting the common PVC: '{}',", configuredPVCName);
deleteCommonPVC(workspace);
return;
}
String workspaceId = workspace.getId();
PersistentVolumeClaim pvc = createCommonPVC(workspaceId);
pvcSubPathHelper.removeDirsAsync(
@ -258,4 +274,35 @@ public class CommonPVCStrategy implements WorkspaceVolumesStrategy {
.filter(subpath -> !isNullOrEmpty(subpath))
.collect(Collectors.toSet());
}
private void deleteCommonPVC(Workspace workspace) throws InfrastructureException {
factory.get(workspace).persistentVolumeClaims().delete(configuredPVCName);
}
/**
* @param account the account of interest
* @return true, if the given account is a personal account, false otherwise
*/
private boolean isPersonalAccount(AccountImpl account) {
return PERSONAL_ACCOUNT.equals(account.getType());
}
/**
* @param account the account of interest
* @return true, if the given account has no workspaces, false otherwise
* @throws InfrastructureException
*/
private boolean accountHasNoWorkspaces(AccountImpl account) throws InfrastructureException {
try {
Page<WorkspaceImpl> workspaces = workspaceManager.getWorkspaces(account.getId(), false, 1, 0);
if (workspaces.isEmpty()) {
log.debug("User '{}' has no more workspaces left", account.getId());
return true;
}
} catch (ServerException e) {
// should never happen
throw new InfrastructureException(e.getLocalizedMessage(), e);
}
return false;
}
}

View File

@ -20,6 +20,7 @@ import io.fabric8.kubernetes.api.model.PersistentVolumeClaim;
import javax.inject.Inject;
import javax.inject.Named;
import org.eclipse.che.api.core.model.workspace.Workspace;
import org.eclipse.che.api.workspace.server.WorkspaceManager;
import org.eclipse.che.api.workspace.server.spi.InfrastructureException;
import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespaceFactory;
@ -62,7 +63,8 @@ public class PerWorkspacePVCStrategy extends CommonPVCStrategy {
EphemeralWorkspaceAdapter ephemeralWorkspaceAdapter,
PVCProvisioner pvcProvisioner,
PodsVolumes podsVolumes,
SubPathPrefixes subpathPrefixes) {
SubPathPrefixes subpathPrefixes,
WorkspaceManager workspaceManager) {
super(
pvcName,
pvcQuantity,
@ -75,7 +77,8 @@ public class PerWorkspacePVCStrategy extends CommonPVCStrategy {
ephemeralWorkspaceAdapter,
pvcProvisioner,
podsVolumes,
subpathPrefixes);
subpathPrefixes,
workspaceManager);
this.pvcNamePrefix = pvcName;
this.factory = factory;
this.pvcAccessMode = pvcAccessMode;

View File

@ -14,9 +14,11 @@ package org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc;
import static java.lang.String.format;
import static java.util.Collections.emptyList;
import static java.util.Collections.emptyMap;
import static org.eclipse.che.api.user.server.UserManager.PERSONAL_ACCOUNT;
import static org.eclipse.che.api.workspace.shared.Constants.PERSIST_VOLUMES_ATTRIBUTE;
import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.CommonPVCStrategy.SUBPATHS_PROPERTY_FMT;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
@ -41,10 +43,13 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import org.eclipse.che.api.core.model.workspace.Workspace;
import org.eclipse.che.api.core.model.workspace.WorkspaceConfig;
import org.eclipse.che.account.spi.AccountImpl;
import org.eclipse.che.api.core.Page;
import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity;
import org.eclipse.che.api.workspace.server.WorkspaceManager;
import org.eclipse.che.api.workspace.server.model.impl.RuntimeIdentityImpl;
import org.eclipse.che.api.workspace.server.model.impl.WorkspaceConfigImpl;
import org.eclipse.che.api.workspace.server.model.impl.WorkspaceImpl;
import org.eclipse.che.api.workspace.server.spi.InfrastructureException;
import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment;
import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespace;
@ -91,6 +96,7 @@ public class CommonPVCStrategyTest {
@Mock private PVCProvisioner volumeConverter;
@Mock private PodsVolumes podsVolumes;
@Mock private SubPathPrefixes subpathPrefixes;
@Mock private WorkspaceManager workspaceManager;
private InOrder provisionOrder;
@ -111,7 +117,8 @@ public class CommonPVCStrategyTest {
ephemeralWorkspaceAdapter,
volumeConverter,
podsVolumes,
subpathPrefixes);
subpathPrefixes,
workspaceManager);
k8sEnv = KubernetesEnvironment.builder().build();
@ -177,7 +184,8 @@ public class CommonPVCStrategyTest {
ephemeralWorkspaceAdapter,
volumeConverter,
podsVolumes,
subpathPrefixes);
subpathPrefixes,
workspaceManager);
commonPVCStrategy.provision(k8sEnv, IDENTITY);
@ -225,7 +233,8 @@ public class CommonPVCStrategyTest {
ephemeralWorkspaceAdapter,
volumeConverter,
podsVolumes,
subpathPrefixes);
subpathPrefixes,
workspaceManager);
final PersistentVolumeClaim pvc = newPVC(PVC_NAME);
pvc.getAdditionalProperties()
.put(format(SUBPATHS_PROPERTY_FMT, WORKSPACE_ID), WORKSPACE_SUBPATHS);
@ -278,14 +287,25 @@ public class CommonPVCStrategyTest {
public void shouldDeletePVCsIfThereIsNoPersistAttributeInWorkspaceConfigWhenCleanupCalled()
throws Exception {
// given
Workspace workspace = mock(Workspace.class);
lenient().when(workspace.getId()).thenReturn(WORKSPACE_ID);
WorkspaceImpl workspace = mock(WorkspaceImpl.class);
Page workspaces = mock(Page.class);
WorkspaceConfig workspaceConfig = mock(WorkspaceConfig.class);
lenient().when(workspace.getConfig()).thenReturn(workspaceConfig);
when(workspace.getId()).thenReturn(WORKSPACE_ID);
when(workspaceManager.getWorkspaces(anyString(), eq(false), anyInt(), anyLong()))
.thenReturn((workspaces));
when(workspaces.isEmpty()).thenReturn(false);
WorkspaceConfigImpl workspaceConfig = mock(WorkspaceConfigImpl.class);
when(workspace.getConfig()).thenReturn(workspaceConfig);
AccountImpl account = mock(AccountImpl.class);
when(account.getType()).thenReturn(PERSONAL_ACCOUNT);
when(account.getId()).thenReturn("id123");
when(workspace.getAccount()).thenReturn(account);
Map<String, String> workspaceConfigAttributes = new HashMap<>();
lenient().when(workspaceConfig.getAttributes()).thenReturn(workspaceConfigAttributes);
when(workspaceConfig.getAttributes()).thenReturn(workspaceConfigAttributes);
KubernetesNamespace ns = mock(KubernetesNamespace.class);
when(factory.get(eq(workspace))).thenReturn(ns);
@ -302,15 +322,24 @@ public class CommonPVCStrategyTest {
public void shouldDeletePVCsIfPersistAttributeIsSetToTrueInWorkspaceConfigWhenCleanupCalled()
throws Exception {
// given
Workspace workspace = mock(Workspace.class);
WorkspaceImpl workspace = mock(WorkspaceImpl.class);
Page workspaces = mock(Page.class);
lenient().when(workspace.getId()).thenReturn(WORKSPACE_ID);
when(workspaceManager.getWorkspaces(anyString(), eq(false), anyInt(), anyLong()))
.thenReturn((workspaces));
when(workspaces.isEmpty()).thenReturn(false);
when(workspace.getId()).thenReturn(WORKSPACE_ID);
WorkspaceConfig workspaceConfig = mock(WorkspaceConfig.class);
lenient().when(workspace.getConfig()).thenReturn(workspaceConfig);
WorkspaceConfigImpl workspaceConfig = mock(WorkspaceConfigImpl.class);
when(workspace.getConfig()).thenReturn(workspaceConfig);
AccountImpl account = mock(AccountImpl.class);
when(account.getType()).thenReturn(PERSONAL_ACCOUNT);
when(account.getId()).thenReturn("id123");
when(workspace.getAccount()).thenReturn(account);
Map<String, String> workspaceConfigAttributes = new HashMap<>();
lenient().when(workspaceConfig.getAttributes()).thenReturn(workspaceConfigAttributes);
when(workspaceConfig.getAttributes()).thenReturn(workspaceConfigAttributes);
workspaceConfigAttributes.put(PERSIST_VOLUMES_ATTRIBUTE, "true");
KubernetesNamespace ns = mock(KubernetesNamespace.class);
@ -324,16 +353,96 @@ public class CommonPVCStrategyTest {
verify(pvcSubPathHelper).removeDirsAsync(WORKSPACE_ID, "ns", PVC_NAME, WORKSPACE_ID);
}
@Test
public void shouldDeleteCommonPVCIfUserHasNoWorkspaces() throws Exception {
// given
WorkspaceImpl workspace = mock(WorkspaceImpl.class);
Page workspaces = mock(Page.class);
KubernetesPersistentVolumeClaims persistentVolumeClaims =
mock(KubernetesPersistentVolumeClaims.class);
when(workspaceManager.getWorkspaces(anyString(), eq(false), anyInt(), anyLong()))
.thenReturn((workspaces));
when(workspaces.isEmpty()).thenReturn(true);
WorkspaceConfigImpl workspaceConfig = mock(WorkspaceConfigImpl.class);
when(workspace.getConfig()).thenReturn(workspaceConfig);
AccountImpl account = mock(AccountImpl.class);
when(account.getType()).thenReturn(PERSONAL_ACCOUNT);
when(account.getId()).thenReturn("id123");
when(workspace.getAccount()).thenReturn(account);
Map<String, String> workspaceConfigAttributes = new HashMap<>();
when(workspaceConfig.getAttributes()).thenReturn(workspaceConfigAttributes);
workspaceConfigAttributes.put(PERSIST_VOLUMES_ATTRIBUTE, "true");
KubernetesNamespace ns = mock(KubernetesNamespace.class);
when(factory.get(eq(workspace))).thenReturn(ns);
when(ns.persistentVolumeClaims()).thenReturn(persistentVolumeClaims);
// when
commonPVCStrategy.cleanup(workspace);
// then
verify(ns).persistentVolumeClaims();
verify(persistentVolumeClaims).delete(PVC_NAME);
verify(pvcSubPathHelper, never()).removeDirsAsync(WORKSPACE_ID, "ns", PVC_NAME, WORKSPACE_ID);
}
@Test
public void shouldNotDeleteCommonPVCIfUserHasWorkspaces() throws Exception {
// given
WorkspaceImpl workspace = mock(WorkspaceImpl.class);
Page workspaces = mock(Page.class);
KubernetesPersistentVolumeClaims persistentVolumeClaims =
mock(KubernetesPersistentVolumeClaims.class);
when(workspaceManager.getWorkspaces(anyString(), eq(false), anyInt(), anyLong()))
.thenReturn((workspaces));
when(workspaces.isEmpty()).thenReturn(false);
when(workspace.getId()).thenReturn(WORKSPACE_ID);
WorkspaceConfigImpl workspaceConfig = mock(WorkspaceConfigImpl.class);
when(workspace.getConfig()).thenReturn(workspaceConfig);
AccountImpl account = mock(AccountImpl.class);
when(account.getType()).thenReturn(PERSONAL_ACCOUNT);
when(account.getId()).thenReturn("id123");
when(workspace.getAccount()).thenReturn(account);
Map<String, String> workspaceConfigAttributes = new HashMap<>();
when(workspaceConfig.getAttributes()).thenReturn(workspaceConfigAttributes);
workspaceConfigAttributes.put(PERSIST_VOLUMES_ATTRIBUTE, "true");
KubernetesNamespace ns = mock(KubernetesNamespace.class);
when(factory.get(eq(workspace))).thenReturn(ns);
when(ns.getName()).thenReturn("ns");
// when
commonPVCStrategy.cleanup(workspace);
// then
verify(ns, never()).persistentVolumeClaims();
verify(persistentVolumeClaims, never()).delete(PVC_NAME);
verify(pvcSubPathHelper).removeDirsAsync(WORKSPACE_ID, "ns", PVC_NAME, WORKSPACE_ID);
}
@Test
public void shouldDoNothingIfPersistAttributeIsSetToFalseInWorkspaceConfigWhenCleanupCalled()
throws Exception {
// given
Workspace workspace = mock(Workspace.class);
WorkspaceImpl workspace = mock(WorkspaceImpl.class);
lenient().when(workspace.getId()).thenReturn(WORKSPACE_ID);
WorkspaceConfig workspaceConfig = mock(WorkspaceConfig.class);
WorkspaceConfigImpl workspaceConfig = mock(WorkspaceConfigImpl.class);
lenient().when(workspace.getConfig()).thenReturn(workspaceConfig);
AccountImpl account = mock(AccountImpl.class);
when(account.getType()).thenReturn(PERSONAL_ACCOUNT);
when(account.getId()).thenReturn("id123");
when(workspace.getAccount()).thenReturn(account);
Map<String, String> workspaceConfigAttributes = new HashMap<>();
lenient().when(workspaceConfig.getAttributes()).thenReturn(workspaceConfigAttributes);
workspaceConfigAttributes.put(PERSIST_VOLUMES_ATTRIBUTE, "false");

View File

@ -34,6 +34,7 @@ import java.util.Map;
import org.eclipse.che.api.core.model.workspace.Workspace;
import org.eclipse.che.api.core.model.workspace.WorkspaceConfig;
import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity;
import org.eclipse.che.api.workspace.server.WorkspaceManager;
import org.eclipse.che.api.workspace.server.model.impl.RuntimeIdentityImpl;
import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment;
import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespace;
@ -73,6 +74,7 @@ public class PerWorkspacePVCStrategyTest {
@Mock private PVCProvisioner volumeConverter;
@Mock private PodsVolumes podsVolumes;
@Mock private SubPathPrefixes subpathPrefixes;
@Mock private WorkspaceManager workspaceManager;
private PerWorkspacePVCStrategy strategy;
@ -91,7 +93,8 @@ public class PerWorkspacePVCStrategyTest {
ephemeralWorkspaceAdapter,
volumeConverter,
podsVolumes,
subpathPrefixes);
subpathPrefixes,
workspaceManager);
lenient().when(factory.getOrCreate(IDENTITY)).thenReturn(k8sNamespace);
lenient().when(factory.get(any(Workspace.class))).thenReturn(k8sNamespace);
@ -137,7 +140,8 @@ public class PerWorkspacePVCStrategyTest {
ephemeralWorkspaceAdapter,
volumeConverter,
podsVolumes,
subpathPrefixes);
subpathPrefixes,
workspaceManager);
final PersistentVolumeClaim pvc = newPVC(PVC_NAME_PREFIX + "-" + WORKSPACE_ID);
String perWorkspacePVCName = pvc.getMetadata().getName();
@ -187,7 +191,8 @@ public class PerWorkspacePVCStrategyTest {
ephemeralWorkspaceAdapter,
volumeConverter,
podsVolumes,
subpathPrefixes);
subpathPrefixes,
workspaceManager);
final PersistentVolumeClaim commonPVC = strategy.createCommonPVC(WORKSPACE_ID);

View File

@ -230,7 +230,7 @@ public class UniqueWorkspacePVCStrategyTest {
strategy.cleanup(workspace);
// then
verify(pvcs).delete(any());
verify(pvcs).delete(ImmutableMap.of(CHE_WORKSPACE_ID_LABEL, WORKSPACE_ID));
}
@Test
@ -251,7 +251,7 @@ public class UniqueWorkspacePVCStrategyTest {
strategy.cleanup(workspace);
// then
verify(pvcs, never()).delete(any());
verify(pvcs, never()).delete(ImmutableMap.of(CHE_WORKSPACE_ID_LABEL, WORKSPACE_ID));
}
static PersistentVolumeClaim newPVC(String name) {