Skip to content

Commit

Permalink
che #18369 Handling a case when the common PVC is used across multipl…
Browse files Browse the repository at this point in the history 
…e users ('workspaceNamespaceDefault' without placeholders is defined)

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
  • Loading branch information
@ibuziuk
ibuziuk committed Dec 31, 2020
1 parent b56a4c2 commit 8e02f78
Show file tree
Hide file tree
Showing 4 changed files with 201 additions and 1 deletion.
44 changes: 43 additions & 1 deletion .../org/eclipse/che/workspace/infrastructure/kubernetes/namespace/pvc/CommonPVCStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
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.Nullable;
import org.eclipse.che.commons.annotation.Traced;
import org.eclipse.che.commons.env.EnvironmentContext;
import org.eclipse.che.commons.subject.Subject;
Expand Down Expand Up @@ -101,6 +102,7 @@ public class CommonPVCStrategy implements WorkspaceVolumesStrategy {
private final PodsVolumes podsVolumes;
private final SubPathPrefixes subpathPrefixes;
private final boolean waitBound;
private final String defaultNamespaceName;
private final WorkspaceManager workspaceManager;

@Inject
Expand All @@ -111,6 +113,7 @@ public CommonPVCStrategy(
@Named("che.infra.kubernetes.pvc.precreate_subpaths") boolean preCreateDirs,
@Named("che.infra.kubernetes.pvc.storage_class_name") String pvcStorageClassName,
@Named("che.infra.kubernetes.pvc.wait_bound") boolean waitBound,
@Nullable @Named("che.infra.kubernetes.namespace.default") String defaultNamespaceName,
PVCSubPathHelper pvcSubPathHelper,
KubernetesNamespaceFactory factory,
EphemeralWorkspaceAdapter ephemeralWorkspaceAdapter,
Expand All @@ -130,6 +133,7 @@ public CommonPVCStrategy(
this.pvcProvisioner = pvcProvisioner;
this.podsVolumes = podsVolumes;
this.subpathPrefixes = subpathPrefixes;
this.defaultNamespaceName = defaultNamespaceName;
this.workspaceManager = workspaceManager;
}

Expand Down Expand Up @@ -237,7 +241,7 @@ public void prepare(
public void cleanup(Workspace workspace) throws InfrastructureException {
if (EphemeralWorkspaceUtility.isEphemeral(workspace)) {
return;
} else if (userHasNoWorkspaces()) {
} else if (noWorkspacesLeft()) {
log.debug("Deleting the common PVC: '{}',", configuredPVCName);
deleteCommonPVC(workspace);
return;
Expand Down Expand Up @@ -275,6 +279,44 @@ private void deleteCommonPVC(Workspace workspace) throws InfrastructureException
factory.get(workspace).persistentVolumeClaims().delete(configuredPVCName);
}

/**
* @return true, if the common PVC is expected to be deleted, false otherwise. Depending on the
* configuration it could be either the common PVC of a particular user, or the common PVC
* that is shared across all the users (the last setup is considered to be uncommon and
* not-recommended)
*/
private boolean noWorkspacesLeft() {
return defaultNamespaceWithoutPlaceholderIsDefined()
? totalNumberOfWorkspacesIsZero()
: userHasNoWorkspaces();
}

/**
* The method is expected to be used in order to identify if the common PVC is used across all the
* users. The common PVC for all the users will be used if
* 'che.infra.kubernetes.namespace.default' points to a particular namespace e.g. 'che',
* 'workspaces' etc.
*
* @return true, if 'che.infra.kubernetes.namespace.default' is defined and does NOT contain the
* placeholder e.g.: che-workspace-<username>), false otherwise
*/
private boolean defaultNamespaceWithoutPlaceholderIsDefined() {
return defaultNamespaceName != null
&& !defaultNamespaceName.contains("<")
&& !defaultNamespaceName.contains(">");
}

/** @return true, if there are no workspaces left across all the users, false otherwise */
private boolean totalNumberOfWorkspacesIsZero() {
try {
return workspaceManager.getWorkspacesTotalCount() == 0;
} catch (ServerException e) {
log.error("Unable to get the total number of workspaces. Cause: {}", e.getMessage(), e);
}
return false;
}

/** @return true, if a given user has no workspaces, false otherwise */
private boolean userHasNoWorkspaces() {
String userId = getSessionUserIdOrUndefined();
if (!"undefined".equals(userId)) {
Expand Down
3 changes: 3 additions & 0 deletions ...clipse/che/workspace/infrastructure/kubernetes/namespace/pvc/PerWorkspacePVCStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
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.commons.annotation.Nullable;
import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespaceFactory;

/**
Expand Down Expand Up @@ -58,6 +59,7 @@ public PerWorkspacePVCStrategy(
@Named("che.infra.kubernetes.pvc.precreate_subpaths") boolean preCreateDirs,
@Named("che.infra.kubernetes.pvc.storage_class_name") String pvcStorageClassName,
@Named("che.infra.kubernetes.pvc.wait_bound") boolean waitBound,
@Nullable @Named("che.infra.kubernetes.namespace.default") String defaultNamespaceName,
PVCSubPathHelper pvcSubPathHelper,
KubernetesNamespaceFactory factory,
EphemeralWorkspaceAdapter ephemeralWorkspaceAdapter,
Expand All @@ -72,6 +74,7 @@ public PerWorkspacePVCStrategy(
preCreateDirs,
pvcStorageClassName,
waitBound,
defaultNamespaceName,
pvcSubPathHelper,
factory,
ephemeralWorkspaceAdapter,
Expand Down
151 changes: 151 additions & 0 deletions .../eclipse/che/workspace/infrastructure/kubernetes/namespace/pvc/CommonPVCStrategyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ public class CommonPVCStrategyTest {
private static final String WORKSPACE_ID = "workspace123";
private static final String NAMESPACE = "infraNamespace";
private static final String PVC_NAME = "che-claim";
private static final String DEFAULT_NAMESPACE_WITH_PLACEHOLDER = "<username>-che";
private static final String DEFAULT_NAMESPACE_WITHOUT_PLACEHOLDER = "che";

private static final String PVC_QUANTITY = "10Gi";
private static final String PVC_ACCESS_MODE = "RWO";
Expand Down Expand Up @@ -112,6 +114,7 @@ public void setup() throws Exception {
true,
PVC_STORAGE_CLASS_NAME,
true,
DEFAULT_NAMESPACE_WITH_PLACEHOLDER,
pvcSubPathHelper,
factory,
ephemeralWorkspaceAdapter,
Expand Down Expand Up @@ -179,6 +182,7 @@ public void testDoNotAddsSubpathsPropertyWhenPreCreationIsNotNeeded() throws Exc
false,
PVC_STORAGE_CLASS_NAME,
true,
DEFAULT_NAMESPACE_WITH_PLACEHOLDER,
pvcSubPathHelper,
factory,
ephemeralWorkspaceAdapter,
Expand Down Expand Up @@ -228,6 +232,7 @@ public void testCreatesPVCsWithSubpathsOnPrepareIfWaitIsDisabled() throws Except
true,
PVC_STORAGE_CLASS_NAME,
false, // wait bound PVCs
DEFAULT_NAMESPACE_WITH_PLACEHOLDER,
pvcSubPathHelper,
factory,
ephemeralWorkspaceAdapter,
Expand Down Expand Up @@ -461,6 +466,152 @@ public void shoulNotDeleteCommonPVCIfUserIdIsUndefined() throws Exception {
verify(pvcSubPathHelper).removeDirsAsync(WORKSPACE_ID, "ns", PVC_NAME, WORKSPACE_ID);
}

@Test
public void shoulDeleteCommonPVCIfUsedAcrossAllUsersAndNoWorspacesLeft() throws Exception {
// given
commonPVCStrategy =
new CommonPVCStrategy(
PVC_NAME,
PVC_QUANTITY,
PVC_ACCESS_MODE,
true,
PVC_STORAGE_CLASS_NAME,
false, // wait bound PVCs
DEFAULT_NAMESPACE_WITHOUT_PLACEHOLDER,
pvcSubPathHelper,
factory,
ephemeralWorkspaceAdapter,
volumeConverter,
podsVolumes,
subpathPrefixes,
workspaceManager);
Workspace workspace = mock(Workspace.class);
KubernetesPersistentVolumeClaims persistentVolumeClaims =
mock(KubernetesPersistentVolumeClaims.class);

when(workspaceManager.getWorkspacesTotalCount()).thenReturn((long) 0);

WorkspaceConfig workspaceConfig = mock(WorkspaceConfig.class);
lenient().when(workspace.getConfig()).thenReturn(workspaceConfig);

Map<String, String> workspaceConfigAttributes = new HashMap<>();
lenient().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(workspaceManager).getWorkspacesTotalCount();
verify(workspaceManager, never()).getWorkspaces(anyString(), eq(false), anyInt(), anyLong());
verify(ns).persistentVolumeClaims();
verify(persistentVolumeClaims).delete(PVC_NAME);
verify(pvcSubPathHelper, never()).removeDirsAsync(WORKSPACE_ID, "ns", PVC_NAME, WORKSPACE_ID);
}

@Test
public void shoulNotDeleteCommonPVCIfUsedAcrossAllUsersAndThereAreWorkspaces() throws Exception {
// given
commonPVCStrategy =
new CommonPVCStrategy(
PVC_NAME,
PVC_QUANTITY,
PVC_ACCESS_MODE,
true,
PVC_STORAGE_CLASS_NAME,
false, // wait bound PVCs
DEFAULT_NAMESPACE_WITHOUT_PLACEHOLDER,
pvcSubPathHelper,
factory,
ephemeralWorkspaceAdapter,
volumeConverter,
podsVolumes,
subpathPrefixes,
workspaceManager);
Workspace workspace = mock(Workspace.class);
Page workspaces = mock(Page.class);
KubernetesPersistentVolumeClaims persistentVolumeClaims =
mock(KubernetesPersistentVolumeClaims.class);

lenient()
.when(workspaceManager.getWorkspaces(anyString(), eq(false), anyInt(), anyLong()))
.thenReturn((workspaces));
lenient().when(workspaces.isEmpty()).thenReturn(false);

when(workspaceManager.getWorkspacesTotalCount()).thenReturn((long) 10);

WorkspaceConfig workspaceConfig = mock(WorkspaceConfig.class);
lenient().when(workspace.getConfig()).thenReturn(workspaceConfig);

Map<String, String> workspaceConfigAttributes = new HashMap<>();
lenient().when(workspaceConfig.getAttributes()).thenReturn(workspaceConfigAttributes);
workspaceConfigAttributes.put(PERSIST_VOLUMES_ATTRIBUTE, "true");

KubernetesNamespace ns = mock(KubernetesNamespace.class);
when(factory.get(eq(workspace))).thenReturn(ns);

// when
commonPVCStrategy.cleanup(workspace);

// then
verify(workspaceManager).getWorkspacesTotalCount();
verify(workspaceManager, never()).getWorkspaces(anyString(), eq(false), anyInt(), anyLong());
verify(ns, never()).persistentVolumeClaims();
verify(persistentVolumeClaims, never()).delete(PVC_NAME);
verify(pvcSubPathHelper, never()).removeDirsAsync(WORKSPACE_ID, "ns", PVC_NAME, WORKSPACE_ID);
}

@Test
public void shouldNotCheckTotalNumberOfWorkspacesIfDefaultNamespaceContainsPlaceholder()
throws Exception {
// given
commonPVCStrategy =
new CommonPVCStrategy(
PVC_NAME,
PVC_QUANTITY,
PVC_ACCESS_MODE,
true,
PVC_STORAGE_CLASS_NAME,
false, // wait bound PVCs
DEFAULT_NAMESPACE_WITH_PLACEHOLDER,
pvcSubPathHelper,
factory,
ephemeralWorkspaceAdapter,
volumeConverter,
podsVolumes,
subpathPrefixes,
workspaceManager);
Workspace workspace = mock(Workspace.class);
Page workspaces = mock(Page.class);
KubernetesPersistentVolumeClaims persistentVolumeClaims =
mock(KubernetesPersistentVolumeClaims.class);

lenient()
.when(workspaceManager.getWorkspaces(anyString(), eq(false), anyInt(), anyLong()))
.thenReturn((workspaces));
lenient().when(workspaces.isEmpty()).thenReturn(false);

WorkspaceConfig workspaceConfig = mock(WorkspaceConfig.class);
lenient().when(workspace.getConfig()).thenReturn(workspaceConfig);

Map<String, String> workspaceConfigAttributes = new HashMap<>();
lenient().when(workspaceConfig.getAttributes()).thenReturn(workspaceConfigAttributes);
workspaceConfigAttributes.put(PERSIST_VOLUMES_ATTRIBUTE, "true");

KubernetesNamespace ns = mock(KubernetesNamespace.class);
when(factory.get(eq(workspace))).thenReturn(ns);

// when
commonPVCStrategy.cleanup(workspace);

// then
verify(workspaceManager, never()).getWorkspacesTotalCount();
}

@Test
public void shouldDoNothingIfPersistAttributeIsSetToFalseInWorkspaceConfigWhenCleanupCalled()
throws Exception {
Expand Down
4 changes: 4 additions & 0 deletions ...se/che/workspace/infrastructure/kubernetes/namespace/pvc/PerWorkspacePVCStrategyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public class PerWorkspacePVCStrategyTest {
private static final String WORKSPACE_ID = "workspace123";
private static final String INFRA_NAMESPACE = "infraNamespace";
private static final String PVC_NAME_PREFIX = "che-claim";
private static final String DEFAULT_NAMESPACE_WITH_PLACEHOLDER = "<username>-che";

private static final String PVC_QUANTITY = "10Gi";
private static final String PVC_ACCESS_MODE = "RWO";
Expand Down Expand Up @@ -88,6 +89,7 @@ public void setup() throws Exception {
true,
PVC_STORAGE_CLASS_NAME,
true,
DEFAULT_NAMESPACE_WITH_PLACEHOLDER,
pvcSubPathHelper,
factory,
ephemeralWorkspaceAdapter,
Expand Down Expand Up @@ -135,6 +137,7 @@ public void shouldPreparePerWorkspacePVCWithSubPathsWhenWaitBoundIsDisabled() th
true,
PVC_STORAGE_CLASS_NAME,
false,
DEFAULT_NAMESPACE_WITH_PLACEHOLDER,
pvcSubPathHelper,
factory,
ephemeralWorkspaceAdapter,
Expand Down Expand Up @@ -186,6 +189,7 @@ public void shouldReturnNullStorageClassNameWhenStorageClassNameIsEmptyOrNull()
true,
storageClassName,
true,
DEFAULT_NAMESPACE_WITH_PLACEHOLDER,
pvcSubPathHelper,
factory,
ephemeralWorkspaceAdapter,
Expand Down

0 comments on commit 8e02f78

Please sign in to comment.