Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 13 commits into from
Jun 7, 2021
Merged
13 changes: 8 additions & 5 deletions infrastructures/kubernetes/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,26 @@ public void delete(Map<String, String> labels) throws InfrastructureException {
}
}

/**
* 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
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;
Expand All @@ -24,8 +25,13 @@
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;
Expand Down Expand Up @@ -70,7 +76,6 @@
* @author Alexander Garagatyi
*/
public class CommonPVCStrategy implements WorkspaceVolumesStrategy {

// use non-static variable to reuse child class logger
private final Logger log = LoggerFactory.getLogger(getClass());

Expand All @@ -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(
Expand All @@ -110,7 +116,8 @@ public CommonPVCStrategy(
EphemeralWorkspaceAdapter ephemeralWorkspaceAdapter,
PVCProvisioner pvcProvisioner,
PodsVolumes podsVolumes,
SubPathPrefixes subpathPrefixes) {
SubPathPrefixes subpathPrefixes,
WorkspaceManager workspaceManager) {
this.configuredPVCName = configuredPVCName;
this.pvcQuantity = pvcQuantity;
this.pvcAccessMode = pvcAccessMode;
Expand All @@ -123,6 +130,7 @@ public CommonPVCStrategy(
this.pvcProvisioner = pvcProvisioner;
this.podsVolumes = podsVolumes;
this.subpathPrefixes = subpathPrefixes;
this.workspaceManager = workspaceManager;
}

/**
Expand Down Expand Up @@ -230,6 +238,14 @@ public void cleanup(Workspace workspace) throws InfrastructureException {
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(
Expand Down Expand Up @@ -258,4 +274,35 @@ private Set<String> combineVolumeMountsSubpaths(KubernetesEnvironment k8sEnv) {
.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
*/
skabashnyuk marked this conversation as resolved.
Show resolved Hide resolved
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
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;

Expand Down Expand Up @@ -62,7 +63,8 @@ public PerWorkspacePVCStrategy(
EphemeralWorkspaceAdapter ephemeralWorkspaceAdapter,
PVCProvisioner pvcProvisioner,
PodsVolumes podsVolumes,
SubPathPrefixes subpathPrefixes) {
SubPathPrefixes subpathPrefixes,
WorkspaceManager workspaceManager) {
super(
pvcName,
pvcQuantity,
Expand All @@ -75,7 +77,8 @@ public PerWorkspacePVCStrategy(
ephemeralWorkspaceAdapter,
pvcProvisioner,
podsVolumes,
subpathPrefixes);
subpathPrefixes,
workspaceManager);
this.pvcNamePrefix = pvcName;
this.factory = factory;
this.pvcAccessMode = pvcAccessMode;
Expand Down
Loading