From 7d56354a5efefe4ff7e9d961ee1ca5688acea478 Mon Sep 17 00:00:00 2001
From: Lukas Krejci <metlos@users.noreply.github.com>
Date: Wed, 23 Oct 2019 13:22:51 +0200
Subject: [PATCH] Use che.infrastructure.kubernetes.namespace.default during
workspace namespace resolution (#14828)
Load the workspace during then namespace resolution so that we can check
for the attribute stored in the workspace attributes.
Switch the default namespace to `<username>-che`.
Update the property descriptions in che.properties and use the new
CHE_INFRA_KUBERNETES_NAMESPACE_DEFAULT in the helm chart.
Move the delete to the KubernetesNamespace/OpenShiftProject to
concentrate the handling in one place.
Split the isNamespaceStatic() (previously called isPredefined()) into
isCreatingNamespaces() and isManagingNamespaces() to separately capture
the two usecases in which that single method was previously incorrectly
called. Also make these methods workspace-specific to be able to
consistently work with the legacy logic across all methods.
Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
---
.../webapp/WEB-INF/classes/che/che.properties | 22 +-
.../helm/che/templates/configmap.yaml | 2 +-
.../helm/che/templates/deployment.yaml | 4 +-
.../helm/che/templates/exec-role.yaml | 2 +-
.../workspace-exec-role-binding.yaml | 2 +-
.../templates/workspace-service-account.yaml | 2 +-
.../workspace-view-role-binding.yaml | 2 +-
.../che/templates/workspace-view-role.yaml | 2 +-
deploy/kubernetes/helm/che/values.yaml | 6 +-
.../namespace/KubernetesNamespace.java | 32 ++
.../namespace/KubernetesNamespaceFactory.java | 275 ++++++++++++++---
.../RemoveNamespaceOnWorkspaceRemove.java | 41 +--
.../namespace/pvc/WorkspacePVCCleaner.java | 10 +-
.../KubernetesNamespaceFactoryTest.java | 289 +++++++++++++++---
.../namespace/KubernetesNamespaceTest.java | 49 +++
.../RemoveNamespaceOnWorkspaceRemoveTest.java | 25 +-
.../pvc/WorkspacePVCCleanerTest.java | 62 ++--
.../openshift/project/OpenShiftProject.java | 33 ++
.../project/OpenShiftProjectFactory.java | 65 +++-
.../RemoveProjectOnWorkspaceRemove.java | 41 +--
.../project/OpenShiftProjectFactoryTest.java | 192 +++++++++++-
.../project/OpenShiftProjectTest.java | 44 +++
.../RemoveProjectOnWorkspaceRemoveTest.java | 26 +-
.../che/api/workspace/shared/Constants.java | 7 +
.../workspace/server/WorkspaceManager.java | 51 ++--
.../server/WorkspaceManagerTest.java | 7 +-
26 files changed, 1040 insertions(+), 253 deletions(-)
diff --git a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties
index 4102405b77..e2ed48c89c 100644
--- a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties
+++ b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties
@@ -377,6 +377,9 @@ che.infra.kubernetes.server_strategy=default-host
# Used to generate domain for a server in a workspace in case property `che.infra.kubernetes.server_strategy` is set to `multi-host`
che.infra.kubernetes.ingress.domain=
+# DEPRECATED - please do not change the value of this property otherwise the existing workspaces will loose data. Do not
+# set it on new installations.
+#
# Defines Kubernetes namespace in which all workspaces will be created.
# If not set, every workspace will be created in a new namespace, where namespace = workspace id
# It's possible to use <username> and <userid> placeholders (e.g.: che-workspace-<username>).
@@ -384,24 +387,23 @@ che.infra.kubernetes.ingress.domain=
# to create new namespace must be used.
#
# Ignored for OpenShift infra. Use `che.infra.openshift.project` instead
+#
+# If the namespace pointed to by this property exists, it will be used for all workspaces. If it does not exist,
+# the namespace specified by the che.infra.kubernetes.namespace.default will be created and used.
che.infra.kubernetes.namespace=
# Defines Kubernetes default namespace in which user's workspaces are created
# if user does not override it.
-# It's possible to use <username> and <userid> placeholders (e.g.: che-workspace-<username>).
-# In that case, new namespace will be created for each user.
+# It's possible to use <username>, <userid> and <workspaceid> placeholders (e.g.: che-workspace-<username>).
+# In that case, new namespace will be created for each user (or workspace).
# Is used by OpenShift infra as well to specify Project
-#
-# BETA It's not fully supported by infra.
-# Use che.infra.kubernetes.namespace to configure workspaces' namespace
che.infra.kubernetes.namespace.default=<username>-che
# Defines if a user is able to specify Kubernetes namespace different from default.
# It's NOT RECOMMENDED to configured true without OAuth configured.
# Is used by OpenShift infra as well to allows users choose Project
#
-# BETA It's not fully supported by infra.
-# Use che.infra.kubernetes.namespace to configure workspaces' namespace
+# BETA This is not currently supported and setting it to true doesn't have any effect.
che.infra.kubernetes.namespace.allow_user_defined=false
# Defines Kubernetes Service Account name which should be specified to be bound to all workspaces pods.
@@ -606,11 +608,17 @@ che.infra.kubernetes.runtimes_consistency_check_period_min=-1
# OpenShift infrastructure reuse most of the Kubernetes configuration attributes.
#
+# DEPRECATED - please do not change the value of this property otherwise the existing workspaces will loose data. Do not
+# set it on new installations.
+#
# Defines OpenShift namespace in which all workspaces will be created.
# If not set, every workspace will be created in a new project, where project name = workspace id
# It's possible to use <username> and <userid> placeholders (e.g.: che-workspace-<username>).
# In that case, new project will be created for each user. OpenShift oauth or service account with
# permission to create new projects must be used.
+#
+# If the project pointed to by this property exists, it will be used for all workspaces. If it does not exist,
+# the namespace specified by the che.infra.kubernetes.namespace.default will be created and used.
che.infra.openshift.project=
# Single port mode wildcard domain host & port. nip.io is used by default
diff --git a/deploy/kubernetes/helm/che/templates/configmap.yaml b/deploy/kubernetes/helm/che/templates/configmap.yaml
index 233bbe6a28..8dcbc1d3ee 100644
--- a/deploy/kubernetes/helm/che/templates/configmap.yaml
+++ b/deploy/kubernetes/helm/che/templates/configmap.yaml
@@ -50,7 +50,7 @@ data:
{{- if and .Values.global.multiuser .Values.customOidcUsernameClaim }}
CHE_KEYCLOAK_USERNAME__CLAIM: {{ .Values.customOidcUsernameClaim }}
{{- end }}
- CHE_INFRA_KUBERNETES_NAMESPACE: {{ .Values.global.cheWorkspacesNamespace | quote}}
+ CHE_INFRA_KUBERNETES_NAMESPACE_DEFAULT: {{ .Values.global.cheWorkspacesNamespace | quote}}
CHE_INFRA_KUBERNETES_SERVICE__ACCOUNT__NAME: {{ .Values.global.cheWorkspaceServiceAccount }}
CHE_INFRA_KUBERNETES_TRUST__CERTS: "false"
CHE_INFRA_KUBERNETES_PVC_STRATEGY: "common"
diff --git a/deploy/kubernetes/helm/che/templates/deployment.yaml b/deploy/kubernetes/helm/che/templates/deployment.yaml
index 0fa452f5be..339fb24e5d 100644
--- a/deploy/kubernetes/helm/che/templates/deployment.yaml
+++ b/deploy/kubernetes/helm/che/templates/deployment.yaml
@@ -85,9 +85,9 @@ spec:
optional: false
{{- end }}
- # If workspaces are created in different namespace than Che Server's one
+ # If workspaces are created in a separate precreated namespace
# then configure Che Server to propagate TLS secret to workspaces' namespaces
- {{- if ne .Release.Namespace .Values.global.cheWorkspacesNamespace }}
+ {{- if not (contains "<" .Values.global.cheWorkspacesNamespace) }}
- name: "CHE_INFRA_KUBERNETES_TLS__CERT"
valueFrom:
secretKeyRef:
diff --git a/deploy/kubernetes/helm/che/templates/exec-role.yaml b/deploy/kubernetes/helm/che/templates/exec-role.yaml
index 9801d644bf..e5b8f05ccf 100644
--- a/deploy/kubernetes/helm/che/templates/exec-role.yaml
+++ b/deploy/kubernetes/helm/che/templates/exec-role.yaml
@@ -7,7 +7,7 @@
# SPDX-License-Identifier: EPL-2.0
#
-{{- if (.Values.global.cheWorkspacesNamespace) }}
+{{- if not (contains "<" .Values.global.cheWorkspacesNamespace) }}
kind: Role
apiVersion: rbac.authorization.k8s.io/v1beta1
metadata:
diff --git a/deploy/kubernetes/helm/che/templates/workspace-exec-role-binding.yaml b/deploy/kubernetes/helm/che/templates/workspace-exec-role-binding.yaml
index 0c35fefe0f..53df54a700 100644
--- a/deploy/kubernetes/helm/che/templates/workspace-exec-role-binding.yaml
+++ b/deploy/kubernetes/helm/che/templates/workspace-exec-role-binding.yaml
@@ -7,7 +7,7 @@
# SPDX-License-Identifier: EPL-2.0
#
-{{- if (.Values.global.cheWorkspacesNamespace) }}
+{{- if not (contains "<" .Values.global.cheWorkspacesNamespace) }}
kind: RoleBinding
apiVersion: rbac.authorization.k8s.io/v1beta1
metadata:
diff --git a/deploy/kubernetes/helm/che/templates/workspace-service-account.yaml b/deploy/kubernetes/helm/che/templates/workspace-service-account.yaml
index 310667740a..52bd1d46f2 100644
--- a/deploy/kubernetes/helm/che/templates/workspace-service-account.yaml
+++ b/deploy/kubernetes/helm/che/templates/workspace-service-account.yaml
@@ -7,7 +7,7 @@
# SPDX-License-Identifier: EPL-2.0
#
-{{- if (.Values.global.cheWorkspacesNamespace) }}
+{{- if not (contains "<" .Values.global.cheWorkspacesNamespace) }}
kind: ServiceAccount
apiVersion: v1
metadata:
diff --git a/deploy/kubernetes/helm/che/templates/workspace-view-role-binding.yaml b/deploy/kubernetes/helm/che/templates/workspace-view-role-binding.yaml
index 0bcd17c4e2..5faa5f6b08 100644
--- a/deploy/kubernetes/helm/che/templates/workspace-view-role-binding.yaml
+++ b/deploy/kubernetes/helm/che/templates/workspace-view-role-binding.yaml
@@ -7,7 +7,7 @@
# SPDX-License-Identifier: EPL-2.0
#
-{{- if (.Values.global.cheWorkspacesNamespace) }}
+{{- if not (contains "<" .Values.global.cheWorkspacesNamespace) }}
kind: RoleBinding
apiVersion: rbac.authorization.k8s.io/v1beta1
metadata:
diff --git a/deploy/kubernetes/helm/che/templates/workspace-view-role.yaml b/deploy/kubernetes/helm/che/templates/workspace-view-role.yaml
index 59e3732c30..4cf8e2bb36 100644
--- a/deploy/kubernetes/helm/che/templates/workspace-view-role.yaml
+++ b/deploy/kubernetes/helm/che/templates/workspace-view-role.yaml
@@ -7,7 +7,7 @@
# SPDX-License-Identifier: EPL-2.0
#
-{{- if (.Values.global.cheWorkspacesNamespace) }}
+{{- if not (contains "<" .Values.global.cheWorkspacesNamespace) }}
kind: Role
apiVersion: rbac.authorization.k8s.io/v1beta1
metadata:
diff --git a/deploy/kubernetes/helm/che/values.yaml b/deploy/kubernetes/helm/che/values.yaml
index b297031488..24ab39322c 100644
--- a/deploy/kubernetes/helm/che/values.yaml
+++ b/deploy/kubernetes/helm/che/values.yaml
@@ -53,11 +53,11 @@ global:
gitHubClientID: ""
gitHubClientSecret: ""
pvcClaim: "1Gi"
- cheWorkspacesNamespace: ""
+ cheWorkspacesNamespace: "<username>-che"
# Service account name that will be mounted to workspaces pods
# Note that:
- # if `cheWorkspacesNamespace` is configured then service account with configured name will be created by helm chart during deploying Che
- # if `cheWorkspacesNamespace` is empty then Che Server creates new namespace for each workspace and ensures that configured SA exists there
+ # if `cheWorkspacesNamespace` doesn't contain placeholders then service account with configured name will be created by helm chart during deploying Che
+ # if `cheWorkspacesNamespace` contains placeholders then Che Server creates new namespaces accordingly and ensures that configured SA exists there
cheWorkspaceServiceAccount: "che-workspace"
# If set, Che will bind the specified cluster role to the workspace service account when creating a workspace.
cheWorkspaceClusterRole: ""
diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespace.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespace.java
index 16cb46375e..cc4c9373a7 100644
--- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespace.java
+++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespace.java
@@ -11,6 +11,8 @@
*/
package org.eclipse.che.workspace.infrastructure.kubernetes.namespace;
+import static java.lang.String.format;
+
import com.google.common.annotations.VisibleForTesting;
import io.fabric8.kubernetes.api.model.ConfigMap;
import io.fabric8.kubernetes.api.model.DoneableServiceAccount;
@@ -117,6 +119,17 @@ public class KubernetesNamespace {
}
}
+ /**
+ * Deletes the namespace. Deleting a non-existent namespace is not an error as is not an attempt
+ * to delete a namespace that is already being deleted.
+ *
+ * @throws InfrastructureException if any unexpected exception occurs during namespace deletion
+ */
+ void delete() throws InfrastructureException {
+ KubernetesClient client = clientFactory.create(workspaceId);
+ delete(name, client);
+ }
+
/** Returns namespace name */
public String getName() {
return name;
@@ -214,6 +227,25 @@ public class KubernetesNamespace {
}
}
+ private void delete(String namespaceName, KubernetesClient client)
+ throws InfrastructureException {
+ try {
+ client.namespaces().withName(namespaceName).delete();
+ } catch (KubernetesClientException e) {
+ if (e.getCode() == 404) {
+ LOG.warn(
+ format(
+ "Tried to delete namespace '%s' but it doesn't exist in the cluster.",
+ namespaceName),
+ e);
+ } else if (e.getCode() == 409) {
+ LOG.info(format("The namespace '%s' is currently being deleted.", namespaceName), e);
+ } else {
+ throw new KubernetesInfrastructureException(e);
+ }
+ }
+ }
+
/**
* Waits few seconds until 'default' service account become available otherwise an infrastructure
* exception will be thrown.
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 243d454649..dd2718a0cf 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
@@ -13,6 +13,7 @@ package org.eclipse.che.workspace.infrastructure.kubernetes.namespace;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Strings.isNullOrEmpty;
+import static java.lang.String.format;
import static java.util.Collections.singletonList;
import static org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta.DEFAULT_ATTRIBUTE;
import static org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta.PHASE_ATTRIBUTE;
@@ -30,7 +31,14 @@ import java.util.Optional;
import java.util.function.Function;
import java.util.stream.Collectors;
import javax.inject.Named;
+import org.eclipse.che.api.core.ConflictException;
+import org.eclipse.che.api.core.NotFoundException;
+import org.eclipse.che.api.core.ServerException;
+import org.eclipse.che.api.core.ValidationException;
+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.api.workspace.shared.Constants;
import org.eclipse.che.commons.annotation.Nullable;
import org.eclipse.che.commons.env.EnvironmentContext;
import org.eclipse.che.commons.subject.Subject;
@@ -38,6 +46,8 @@ import org.eclipse.che.inject.ConfigurationException;
import org.eclipse.che.workspace.infrastructure.kubernetes.KubernetesClientFactory;
import org.eclipse.che.workspace.infrastructure.kubernetes.api.server.impls.KubernetesNamespaceMetaImpl;
import org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* Helps to create {@link KubernetesNamespace} instances.
@@ -46,41 +56,57 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.Kubernetes
*/
@Singleton
public class KubernetesNamespaceFactory {
+ private static final Logger LOG = LoggerFactory.getLogger(KubernetesNamespaceFactory.class);
- private static final Map<String, Function<Subject, String>> NAMESPACE_NAME_PLACEHOLDERS =
- new HashMap<>();
+ private static final Map<String, Function<PlaceholderResolutionContext, String>>
+ NAMESPACE_NAME_PLACEHOLDERS = new HashMap<>();
+
+ private static final String USERNAME_PLACEHOLDER = "<username>";
+ private static final String USERID_PLACEHOLDER = "<userid>";
+ private static final String WORKSPACEID_PLACEHOLDER = "<workspaceid>";
static {
- NAMESPACE_NAME_PLACEHOLDERS.put("<username>", Subject::getUserName);
- NAMESPACE_NAME_PLACEHOLDERS.put("<userid>", Subject::getUserId);
+ NAMESPACE_NAME_PLACEHOLDERS.put(USERNAME_PLACEHOLDER, ctx -> ctx.user.getUserName());
+ NAMESPACE_NAME_PLACEHOLDERS.put(USERID_PLACEHOLDER, ctx -> ctx.user.getUserId());
+ NAMESPACE_NAME_PLACEHOLDERS.put(WORKSPACEID_PLACEHOLDER, ctx -> ctx.workspaceId);
}
private final String defaultNamespaceName;
private final boolean allowUserDefinedNamespaces;
- private final String namespaceName;
- private final boolean isPredefined;
+ private final String legacyNamespaceName;
private final String serviceAccountName;
private final String clusterRoleName;
private final KubernetesClientFactory clientFactory;
+ private final WorkspaceManager workspaceManager;
@Inject
public KubernetesNamespaceFactory(
- @Nullable @Named("che.infra.kubernetes.namespace") String namespaceName,
+ @Nullable @Named("che.infra.kubernetes.namespace") String legacyNamespaceName,
@Nullable @Named("che.infra.kubernetes.service_account_name") String serviceAccountName,
@Nullable @Named("che.infra.kubernetes.cluster_role_name") String clusterRoleName,
@Nullable @Named("che.infra.kubernetes.namespace.default") String defaultNamespaceName,
@Named("che.infra.kubernetes.namespace.allow_user_defined")
boolean allowUserDefinedNamespaces,
- KubernetesClientFactory clientFactory)
+ KubernetesClientFactory clientFactory,
+ WorkspaceManager workspaceManager)
throws ConfigurationException {
- this.namespaceName = namespaceName;
- this.isPredefined = !isNullOrEmpty(namespaceName) && hasNoPlaceholders(this.namespaceName);
+ this.legacyNamespaceName = legacyNamespaceName;
this.serviceAccountName = serviceAccountName;
this.clusterRoleName = clusterRoleName;
this.clientFactory = clientFactory;
this.defaultNamespaceName = defaultNamespaceName;
this.allowUserDefinedNamespaces = allowUserDefinedNamespaces;
+ this.workspaceManager = workspaceManager;
+
+ // This will disappear once we support user selection of workspaces...
+ if (allowUserDefinedNamespaces) {
+ LOG.warn(
+ "'che.infra.kubernetes.namespace.allow_user_defined' is not supported yet. It currently has no"
+ + " effect.");
+ }
+
+ // right now allowUserDefinedNamespaces can't be true, but eventually we will implement it.
if (isNullOrEmpty(defaultNamespaceName) && !allowUserDefinedNamespaces) {
throw new ConfigurationException(
"che.infra.kubernetes.namespace.default or "
@@ -88,16 +114,82 @@ public class KubernetesNamespaceFactory {
}
}
- private boolean hasNoPlaceholders(String namespaceName) {
- return NAMESPACE_NAME_PLACEHOLDERS.keySet().stream().noneMatch(namespaceName::contains);
+ private boolean hasPlaceholders(String namespaceName) {
+ return namespaceName != null
+ && NAMESPACE_NAME_PLACEHOLDERS.keySet().stream().anyMatch(namespaceName::contains);
}
/**
- * True if namespace is predefined for all workspaces. False if each workspace will be provided
- * with a new namespace or provided for each user when using placeholders.
+ * True if namespace is potentially created for the workspace, false otherwise.
+ *
+ * <p>The logic is a little bit non-trivial and best expressed by just fully evaluting the truth
+ * table as below ({@code ...namespace} stands for the legacy namespace property, {@code
+ * ...namespace.default} stands for the namespace default property):
+ *
+ * <pre>{@code
+ * ...namespace | ...namespace exists | ...namespace.default | creating?
+ * no-placeholders | no | null | error
+ * no-placeholders | no | no-placeholders | no
+ * no-placeholders | no | placeholders | yes
+ * no-placeholders | yes | null | no
+ * no-placeholders | yes | no-placeholders | no
+ * no-placeholders | yes | placeholders | no
+ * placeholders | no | null | error
+ * placeholders | no | no-placeholders | no
+ * placeholders | no | placeholders | yes
+ * placeholders | yes | null | yes
+ * placeholders | yes | no-placeholders | yes
+ * placeholders | yes | placeholders | yes
+ * }</pre>
*/
- public boolean isPredefined() {
- return isPredefined;
+ protected boolean isCreatingNamespace(String workspaceId) throws InfrastructureException {
+ boolean legacyExists =
+ checkNamespaceExists(
+ resolveLegacyNamespaceName(EnvironmentContext.getCurrent().getSubject(), workspaceId));
+
+ // legacy namespace exists and should be used
+ if (legacyExists) {
+ // if it contains any placeholder("" is <workspaceid>) - it indicates that Che created
+ // namespace by itself
+ return isNullOrEmpty(legacyNamespaceName) || hasPlaceholders(legacyNamespaceName);
+ }
+
+ if (isNullOrEmpty(defaultNamespaceName)) {
+ throw new InfrastructureException(
+ "Cannot determine whether a new namespace and service account should be"
+ + " created for workspace %s. There is no pre-existing workspace namespace to be"
+ + " found using the legacy `che.infra.kubernetes.namespace` property yet the"
+ + " `che.infra.kubernetes.namespace.default` property is undefined.");
+ }
+
+ return hasPlaceholders(defaultNamespaceName);
+ }
+
+ /**
+ * Returns true if the namespace is fully managed by Che (e.g. created, used and deleted), false
+ * otherwise.
+ */
+ public boolean isManagingNamespace(String workspaceId) throws InfrastructureException {
+ // the new logic is quite simple.
+ boolean newLogic =
+ defaultNamespaceName != null && defaultNamespaceName.contains(WORKSPACEID_PLACEHOLDER);
+
+ // but we must follow the same logic as #evalNamespaceName - we need to make sure that the old
+ // logic can't be used first...
+ // empty legacy namespace name ~ <workspaceid>
+ if (isNullOrEmpty(legacyNamespaceName)
+ || legacyNamespaceName.contains(WORKSPACEID_PLACEHOLDER)) {
+
+ // there's a chance of using the old logic - if the namespace exists, we're managing it.
+ // if it doesn't, we're using the new logic.
+ return checkNamespaceExists(
+ resolveLegacyNamespaceName(EnvironmentContext.getCurrent().getSubject(), workspaceId))
+ || newLogic;
+ } else {
+ // there's no way the namespace of the workspace is managed using the old logic. Let's just
+ // use the result of the new logic.
+ return newLogic;
+ }
}
/**
@@ -141,8 +233,7 @@ public class KubernetesNamespaceFactory {
// the default namespace must be configured if user defined are not allowed
// so return only it
String evaluatedName =
- evalDefaultNamespaceName(
- defaultNamespaceName, EnvironmentContext.getCurrent().getSubject());
+ evalPlaceholders(defaultNamespaceName, EnvironmentContext.getCurrent().getSubject(), null);
Optional<KubernetesNamespaceMeta> defaultNamespaceOpt = fetchNamespace(evaluatedName);
@@ -164,8 +255,7 @@ public class KubernetesNamespaceFactory {
*/
private void provisionDefaultNamespace(List<KubernetesNamespaceMeta> namespaces) {
String evaluatedName =
- evalDefaultNamespaceName(
- defaultNamespaceName, EnvironmentContext.getCurrent().getSubject());
+ evalPlaceholders(defaultNamespaceName, EnvironmentContext.getCurrent().getSubject(), null);
Optional<KubernetesNamespaceMeta> defaultNamespaceOpt =
namespaces.stream().filter(n -> evaluatedName.equals(n.getName())).findAny();
@@ -244,12 +334,22 @@ public class KubernetesNamespaceFactory {
* @throws InfrastructureException if any exception occurs during namespace preparing
*/
public KubernetesNamespace create(String workspaceId) throws InfrastructureException {
- final String namespaceName =
- evalNamespaceName(workspaceId, EnvironmentContext.getCurrent().getSubject());
+ final String namespaceName;
+ try {
+ namespaceName = evalNamespaceName(workspaceId, EnvironmentContext.getCurrent().getSubject());
+ } catch (NotFoundException | ServerException | ConflictException | ValidationException e) {
+ throw new InfrastructureException(
+ format(
+ "Failed to determine the namespace to put the workspace %s to."
+ + " The error message was: %s",
+ workspaceId, e.getMessage()),
+ e);
+ }
+
KubernetesNamespace namespace = doCreateNamespace(workspaceId, namespaceName);
namespace.prepare();
- if (!isPredefined() && !isNullOrEmpty(serviceAccountName)) {
+ if (isCreatingNamespace(workspaceId) && !isNullOrEmpty(serviceAccountName)) {
// prepare service account for workspace only if account name is configured
// and project is not predefined
// since predefined project should be prepared during Che deployment
@@ -261,29 +361,114 @@ public class KubernetesNamespaceFactory {
return namespace;
}
- protected String evalNamespaceName(String workspaceId, Subject currentUser) {
- if (isPredefined) {
- return this.namespaceName;
- } else if (isNullOrEmpty(this.namespaceName)) {
- return workspaceId;
+ public void delete(String workspaceId) throws InfrastructureException {
+ final String namespaceName;
+ try {
+ namespaceName = evalNamespaceName(workspaceId, EnvironmentContext.getCurrent().getSubject());
+ } catch (NotFoundException | ServerException | ConflictException | ValidationException e) {
+ throw new InfrastructureException(
+ format("Failed to determine the namespace of the workspace %s", workspaceId), e);
+ }
+
+ KubernetesNamespace namespace = doCreateNamespace(workspaceId, namespaceName);
+ namespace.delete();
+ }
+
+ protected String evalNamespaceName(String workspaceId, Subject currentUser)
+ throws NotFoundException, ServerException, InfrastructureException, ConflictException,
+ ValidationException {
+ // This, my friend, is a hack of magnificent proportions put forth as a result of dire time
+ // constraints imposed on the tears shedding developer writing these unfortunate lines.
+ // The effort required to propagate the full workspace, including the attributes (or some
+ // alternative thereof) from the callers (all of which happen to already possess the
+ // information) down to this sad place is too effing much to do with confidence in the
+ // couple of days left until the release. Let's pretend we will have time to fix this later
+ // in the better times...
+ Workspace wkspc = workspaceManager.getWorkspace(workspaceId);
+ String ns = wkspc.getAttributes().get(Constants.WORKSPACE_INFRASTRUCTURE_NAMESPACE_ATTRIBUTE);
+
+ if (ns != null) {
+ LOG.debug(
+ "Found target namespace in workspace attributes. Using namespace {} for workspace {}",
+ ns,
+ workspaceId);
+ return ns;
} else {
- String tmpNamespaceName = this.namespaceName;
- for (String placeholder : NAMESPACE_NAME_PLACEHOLDERS.keySet()) {
- tmpNamespaceName =
- tmpNamespaceName.replaceAll(
- placeholder, NAMESPACE_NAME_PLACEHOLDERS.get(placeholder).apply(currentUser));
+ String namespace = resolveLegacyNamespaceName(currentUser, workspaceId);
+
+ if (checkNamespaceExists(namespace)) {
+ LOG.debug(
+ "The namespace specified using the legacy config exists: {}. Using it for workspace {}.",
+ namespace,
+ workspaceId);
+ } else {
+ // ok, the namespace pointed to by the legacy config doesn't exist.. that means there can be
+ // no damage done by storing the workspace in the namespace designated by the new way of
+ // doing things...
+
+ if (isNullOrEmpty(defaultNamespaceName)) {
+ throw new InfrastructureException(
+ format(
+ "'che.infra.kubernetes.namespace.default' is not"
+ + " defined and no explicit namespace configured for workspace %s",
+ workspaceId));
+ }
+
+ namespace = evalPlaceholders(defaultNamespaceName, currentUser, workspaceId);
+
+ LOG.debug(
+ "Evaluated the namespace for workspace {} using the namespace default to {}",
+ workspaceId,
+ namespace);
}
- return tmpNamespaceName;
+
+ // Now, believe it or not, the horror continues - since the callers are as of now unaware
+ // of the namespaces being stored within the workspace, we need to do it all here. Hopefully,
+ // one day, when the callers (and workspace manager in particular) support workspace namespace
+ // selection, things will make more sense again because this logic will have to move up a
+ // layer or two and become infrastructure independent.
+ wkspc.getAttributes().put(Constants.WORKSPACE_INFRASTRUCTURE_NAMESPACE_ATTRIBUTE, namespace);
+ workspaceManager.updateWorkspace(workspaceId, wkspc);
+
+ return namespace;
}
}
- protected String evalDefaultNamespaceName(String defaultNamespace, Subject currentUser) {
- checkArgument(!isNullOrEmpty(defaultNamespace));
- String evaluated = defaultNamespace;
- for (Entry<String, Function<Subject, String>> placeHolder :
+ private String resolveLegacyNamespaceName(Subject user, String workspaceId) {
+ String effectiveOldLogicNamespace =
+ isNullOrEmpty(legacyNamespaceName) ? WORKSPACEID_PLACEHOLDER : legacyNamespaceName;
+
+ return evalPlaceholders(effectiveOldLogicNamespace, user, workspaceId);
+ }
+
+ protected boolean checkNamespaceExists(String namespaceName) throws InfrastructureException {
+ try {
+ return clientFactory.create().namespaces().withName(namespaceName).get() != null;
+ } catch (KubernetesClientException e) {
+ if (e.getCode() == 403) {
+ // 403 means that the project does not exist
+ // or a user really is not permitted to access it which is Che Server misconfiguration
+ return false;
+ } else {
+ throw new InfrastructureException(
+ "Error occurred when tried to fetch default project. Cause: " + e.getMessage(), e);
+ }
+ }
+ }
+
+ protected String evalPlaceholders(String namespace, Subject currentUser, String workspaceId) {
+ checkArgument(!isNullOrEmpty(namespace));
+ String evaluated = namespace;
+ PlaceholderResolutionContext ctx = new PlaceholderResolutionContext(currentUser, workspaceId);
+ for (Entry<String, Function<PlaceholderResolutionContext, String>> placeHolder :
NAMESPACE_NAME_PLACEHOLDERS.entrySet()) {
- evaluated =
- evaluated.replaceAll(placeHolder.getKey(), placeHolder.getValue().apply(currentUser));
+
+ String key = placeHolder.getKey();
+ String value = placeHolder.getValue().apply(ctx);
+
+ if (value != null) {
+ evaluated = evaluated.replaceAll(key, value);
+ }
}
return evaluated;
}
@@ -302,4 +487,14 @@ public class KubernetesNamespaceFactory {
protected String getClusterRoleName() {
return clusterRoleName;
}
+
+ private static final class PlaceholderResolutionContext {
+ final Subject user;
+ final String workspaceId;
+
+ private PlaceholderResolutionContext(Subject user, String workspaceId) {
+ this.user = user;
+ this.workspaceId = workspaceId;
+ }
+ }
}
diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/RemoveNamespaceOnWorkspaceRemove.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/RemoveNamespaceOnWorkspaceRemove.java
index 14815a4fdc..058ead96f7 100644
--- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/RemoveNamespaceOnWorkspaceRemove.java
+++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/RemoveNamespaceOnWorkspaceRemove.java
@@ -11,20 +11,12 @@
*/
package org.eclipse.che.workspace.infrastructure.kubernetes.namespace;
-import static com.google.common.base.Strings.isNullOrEmpty;
-
-import com.google.common.annotations.VisibleForTesting;
import com.google.inject.Inject;
import com.google.inject.Singleton;
-import io.fabric8.kubernetes.client.KubernetesClientException;
-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.KubernetesClientFactory;
-import org.eclipse.che.workspace.infrastructure.kubernetes.KubernetesInfrastructureException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -37,45 +29,30 @@ import org.slf4j.LoggerFactory;
public class RemoveNamespaceOnWorkspaceRemove implements EventSubscriber<WorkspaceRemovedEvent> {
private static final Logger LOG = LoggerFactory.getLogger(RemoveNamespaceOnWorkspaceRemove.class);
- private final KubernetesClientFactory clientFactory;
- private final String namespaceName;
+ private final KubernetesNamespaceFactory namespaceFactory;
@Inject
- public RemoveNamespaceOnWorkspaceRemove(
- @Nullable @Named("che.infra.kubernetes.namespace") String namespaceName,
- KubernetesClientFactory clientFactory) {
- this.namespaceName = namespaceName;
- this.clientFactory = clientFactory;
+ public RemoveNamespaceOnWorkspaceRemove(KubernetesNamespaceFactory namespaceFactory) {
+ this.namespaceFactory = namespaceFactory;
}
@Inject
public void subscribe(EventService eventService) {
- if (isNullOrEmpty(namespaceName)) {
- eventService.subscribe(this);
- }
+ eventService.subscribe(this);
}
@Override
public void onEvent(WorkspaceRemovedEvent event) {
+ String workspaceId = event.getWorkspace().getId();
try {
- doRemoveNamespace(event.getWorkspace().getId());
+ if (namespaceFactory.isManagingNamespace(workspaceId)) {
+ namespaceFactory.delete(workspaceId);
+ }
} catch (InfrastructureException e) {
LOG.warn(
"Fail to remove Kubernetes namespace for workspace with id {}. Cause: {}",
- event.getWorkspace().getId(),
+ workspaceId,
e.getMessage());
}
}
-
- @VisibleForTesting
- void doRemoveNamespace(String namespaceName) throws InfrastructureException {
- try {
- clientFactory.create(namespaceName).namespaces().withName(namespaceName).delete();
- } catch (KubernetesClientException e) {
- if (!(e.getCode() == 403)) {
- throw new KubernetesInfrastructureException(e);
- }
- // namespace doesn't exist
- }
- }
}
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 cdc365f428..89ce7d4247 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
@@ -52,11 +52,18 @@ public class WorkspacePVCCleaner {
@Inject
public void subscribe(EventService eventService) {
- if (pvcEnabled && namespaceFactory.isPredefined())
+ if (pvcEnabled) {
eventService.subscribe(
event -> {
final Workspace workspace = event.getWorkspace();
try {
+ if (namespaceFactory.isManagingNamespace(workspace.getId())) {
+ // the namespaces of managed workspaces are deleted, so no need to do the cleanup
+ LOG.debug(
+ "Not cleaning up the PVCs of workspace %s, because its namespace is"
+ + " going to be deleted.");
+ return;
+ }
strategy.cleanup(workspace);
} catch (InfrastructureException ex) {
LOG.error(
@@ -66,5 +73,6 @@ public class WorkspacePVCCleaner {
}
},
WorkspaceRemovedEvent.class);
+ }
}
}
diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactoryTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactoryTest.java
index b1e1b1b46b..8065ba3e57 100644
--- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactoryTest.java
+++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactoryTest.java
@@ -11,10 +11,14 @@
*/
package org.eclipse.che.workspace.infrastructure.kubernetes.namespace;
+import static java.lang.String.format;
+import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonList;
+import static java.util.Collections.singletonMap;
import static org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta.DEFAULT_ATTRIBUTE;
import static org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta.PHASE_ATTRIBUTE;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
@@ -25,7 +29,7 @@ import static org.mockito.Mockito.when;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertNull;
-import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
import io.fabric8.kubernetes.api.model.DoneableNamespace;
import io.fabric8.kubernetes.api.model.Namespace;
@@ -37,7 +41,10 @@ import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation;
import io.fabric8.kubernetes.client.dsl.Resource;
import java.util.Arrays;
import java.util.List;
+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.api.workspace.shared.Constants;
import org.eclipse.che.commons.subject.SubjectImpl;
import org.eclipse.che.inject.ConfigurationException;
import org.eclipse.che.workspace.infrastructure.kubernetes.KubernetesClientFactory;
@@ -45,6 +52,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.Kubernetes
import org.mockito.Mock;
import org.mockito.testng.MockitoTestNGListener;
import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.DataProvider;
import org.testng.annotations.Listeners;
import org.testng.annotations.Test;
@@ -58,18 +66,27 @@ public class KubernetesNamespaceFactoryTest {
@Mock private KubernetesClientFactory clientFactory;
@Mock private KubernetesClient k8sClient;
+ @Mock private WorkspaceManager workspaceManager;
@Mock
private NonNamespaceOperation<
Namespace, NamespaceList, DoneableNamespace, Resource<Namespace, DoneableNamespace>>
namespaceOperation;
+ @Mock private Resource<Namespace, DoneableNamespace> namespaceResource;
+
private KubernetesNamespaceFactory namespaceFactory;
@BeforeMethod
public void setUp() throws Exception {
lenient().when(clientFactory.create()).thenReturn(k8sClient);
lenient().when(k8sClient.namespaces()).thenReturn(namespaceOperation);
+ lenient()
+ .when(workspaceManager.getWorkspace(any()))
+ .thenReturn(WorkspaceImpl.builder().setId("1").setAttributes(emptyMap()).build());
+
+ lenient().when(namespaceOperation.withName(any())).thenReturn(namespaceResource);
+ lenient().when(namespaceResource.get()).thenReturn(mock(Namespace.class));
}
@Test(
@@ -81,7 +98,8 @@ public class KubernetesNamespaceFactoryTest {
shouldThrowExceptionIfNoDefaultNamespaceIsConfiguredAndUserDefinedNamespacesAreNotAllowed()
throws Exception {
namespaceFactory =
- new KubernetesNamespaceFactory("predefined", "", "", null, false, clientFactory);
+ new KubernetesNamespaceFactory(
+ "predefined", "", "", null, false, clientFactory, workspaceManager);
}
@Test
@@ -96,7 +114,8 @@ public class KubernetesNamespaceFactoryTest {
.withNewStatus("Active")
.build());
namespaceFactory =
- new KubernetesNamespaceFactory("predefined", "", "", "che-default", false, clientFactory);
+ new KubernetesNamespaceFactory(
+ "predefined", "", "", "che-default", false, clientFactory, workspaceManager);
List<KubernetesNamespaceMeta> availableNamespaces = namespaceFactory.list();
assertEquals(availableNamespaces.size(), 1);
@@ -112,7 +131,8 @@ public class KubernetesNamespaceFactoryTest {
prepareNamespaceToBeFoundByName("che-default", null);
namespaceFactory =
- new KubernetesNamespaceFactory("predefined", "", "", "che-default", false, clientFactory);
+ new KubernetesNamespaceFactory(
+ "predefined", "", "", "che-default", false, clientFactory, workspaceManager);
List<KubernetesNamespaceMeta> availableNamespaces = namespaceFactory.list();
assertEquals(availableNamespaces.size(), 1);
@@ -131,7 +151,8 @@ public class KubernetesNamespaceFactoryTest {
"Error occurred when tried to fetch default namespace. Cause: connection refused")
public void shouldThrownExceptionWhenFailedToGetInfoAboutDefaultNamespace() throws Exception {
namespaceFactory =
- new KubernetesNamespaceFactory("predefined", "", "", "che", false, clientFactory);
+ new KubernetesNamespaceFactory(
+ "predefined", "", "", "che", false, clientFactory, workspaceManager);
throwOnTryToGetNamespaceByName("che", new KubernetesClientException("connection refused"));
namespaceFactory.list();
@@ -145,7 +166,8 @@ public class KubernetesNamespaceFactoryTest {
createNamespace("experimental", "Terminating")));
namespaceFactory =
- new KubernetesNamespaceFactory("predefined", "", "", null, true, clientFactory);
+ new KubernetesNamespaceFactory(
+ "predefined", "", "", null, true, clientFactory, workspaceManager);
List<KubernetesNamespaceMeta> availableNamespaces = namespaceFactory.list();
assertEquals(availableNamespaces.size(), 2);
@@ -167,7 +189,8 @@ public class KubernetesNamespaceFactoryTest {
createNamespace("my-for-ws", "Active"), createNamespace("default", "Active")));
namespaceFactory =
- new KubernetesNamespaceFactory("predefined", "", "", "default", true, clientFactory);
+ new KubernetesNamespaceFactory(
+ "predefined", "", "", "default", true, clientFactory, workspaceManager);
List<KubernetesNamespaceMeta> availableNamespaces = namespaceFactory.list();
@@ -190,7 +213,8 @@ public class KubernetesNamespaceFactoryTest {
prepareListedNamespaces(singletonList(createNamespace("my-for-ws", "Active")));
namespaceFactory =
- new KubernetesNamespaceFactory("predefined", "", "", "default", true, clientFactory);
+ new KubernetesNamespaceFactory(
+ "predefined", "", "", "default", true, clientFactory, workspaceManager);
List<KubernetesNamespaceMeta> availableNamespaces = namespaceFactory.list();
assertEquals(availableNamespaces.size(), 2);
@@ -214,54 +238,141 @@ public class KubernetesNamespaceFactoryTest {
"Error occurred when tried to list all available namespaces. Cause: connection refused")
public void shouldThrownExceptionWhenFailedToGetNamespaces() throws Exception {
namespaceFactory =
- new KubernetesNamespaceFactory("predefined", "", "", null, true, clientFactory);
+ new KubernetesNamespaceFactory(
+ "predefined", "", "", null, true, clientFactory, workspaceManager);
throwOnTryToGetNamespacesList(new KubernetesClientException("connection refused"));
namespaceFactory.list();
}
- @Test
- public void shouldReturnTrueIfNamespaceIsNotEmptyOnCheckingIfNamespaceIsPredefined() {
+ @DataProvider
+ public static Object[][] creatingNamespaceConditions() {
+ // Whether or not the factory potentially creates a namespace depends on the 3 conditions:
+ // 1) the value of the legacy property 'che.infra.kubernetes.namespace'
+ // 2) legacy property pointing to the existing namespace
+ // 3) value of the new property 'che.infra.kubernetes.namespace.default'
+ // the output is either true, false, or error (represented as a null here)
+
+ // this is the truth table we need to follow
+ // ...namespace | ...namespace exists | ...namespace.default | creating?
+ // no-placeholders | no | null | error
+ // no-placeholders | no | no-placeholders | no
+ // no-placeholders | no | placeholders | yes
+ // no-placeholders | yes | null | no
+ // no-placeholders | yes | no-placeholders | no
+ // no-placeholders | yes | placeholders | no
+ // placeholders | no | null | error
+ // placeholders | no | no-placeholders | no
+ // placeholders | no | placeholders | yes
+ // placeholders | yes | null | yes
+ // placeholders | yes | no-placeholders | yes
+ // placeholders | yes | placeholders | yes
+
+ // additionally, we want to test that if the legacy property is null, it behaves exactly
+ // the same as having a placeholder, because it should default to <workspaceid>
+ return new Object[][] {
+ new Object[] {"some", false, null, null},
+ new Object[] {"some", false, "some", false},
+ new Object[] {"some", false, "<userid>", true},
+ new Object[] {"some", true, null, false},
+ new Object[] {"some", true, "some", false},
+ new Object[] {"some", true, "<userid>", false},
+ new Object[] {"<workspaceid>", false, null, null},
+ new Object[] {"<workspaceid>", false, "some", false},
+ new Object[] {"<workspaceid>", false, "<userid>", true},
+ new Object[] {"<workspaceid>", true, null, true},
+ new Object[] {"<workspaceid>", true, "some", true},
+ new Object[] {"<workspaceid>", true, "<userid>", true},
+ new Object[] {null, false, null, null},
+ new Object[] {null, false, "some", false},
+ new Object[] {null, false, "<userid>", true},
+ new Object[] {null, true, null, true},
+ new Object[] {null, true, "some", true},
+ new Object[] {null, true, "<userid>", true},
+ };
+ }
+
+ @Test(dataProvider = "creatingNamespaceConditions")
+ public void shouldDetermineWhenNamespaceCanBeCreated(
+ String legacyProperty,
+ boolean legacyNamespaceExists,
+ String namespaceProperty,
+ Boolean expectedOutcome) {
+
// given
namespaceFactory =
- new KubernetesNamespaceFactory("predefined", "", "", "che", false, clientFactory);
+ new KubernetesNamespaceFactory(
+ legacyProperty, "", "", namespaceProperty, true, clientFactory, workspaceManager);
+
+ Namespace existingLegacyNamespace = legacyNamespaceExists ? mock(Namespace.class) : null;
+ when(namespaceResource.get()).thenReturn(existingLegacyNamespace);
// when
- boolean isPredefined = namespaceFactory.isPredefined();
+ Boolean result;
+ try {
+ result = namespaceFactory.isCreatingNamespace("123");
+ } catch (InfrastructureException e) {
+ // this can happen and we test for it below...
+ result = null;
+ }
// then
- assertTrue(isPredefined);
+ assertEquals(result, expectedOutcome);
}
- @Test
- public void shouldReturnTrueIfNamespaceIsEmptyOnCheckingIfNamespaceIsPredefined() {
- // given
- namespaceFactory = new KubernetesNamespaceFactory("", "", "", "che", false, clientFactory);
-
- // when
- boolean isPredefined = namespaceFactory.isPredefined();
+ @Test(dataProvider = "creatingNamespaceConditions")
+ public void testNotManagingNamespacesWheneverNotCreatingThem(
+ String legacyProperty,
+ boolean legacyNamespaceExists,
+ String namespaceProperty,
+ Boolean expectedCreating)
+ throws InfrastructureException {
- // then
- assertFalse(isPredefined);
- }
+ // it is possible that we are creating namespaces that we are not fully managing, e.g. <user*>
+ // namespaces are created but not fully deleted afterwards. We just clean them.
+ // However, whenever a namespace is NOT being created, we should never claim we're managing the
+ // namespace.
+ // This is what this test asserts.
- @Test
- public void shouldReturnTrueIfNamespaceIsNullOnCheckingIfNamespaceIsPredefined() {
// given
- namespaceFactory = new KubernetesNamespaceFactory(null, "", "", "che", false, clientFactory);
+ namespaceFactory =
+ new KubernetesNamespaceFactory(
+ legacyProperty, "", "", namespaceProperty, true, clientFactory, workspaceManager);
+
+ Namespace existingLegacyNamespace = legacyNamespaceExists ? mock(Namespace.class) : null;
+ when(namespaceResource.get()).thenReturn(existingLegacyNamespace);
// when
- boolean isPredefined = namespaceFactory.isPredefined();
+ boolean creating;
+ try {
+ creating = namespaceFactory.isCreatingNamespace("123");
+ } catch (InfrastructureException e) {
+ // if we can't determine whether we're potentially creating a namespace, we shouldn't claim
+ // we're managing it
+ if (expectedCreating != null) {
+ fail("Shouldn't have failed.");
+ }
+ creating = false;
+ }
+ boolean managing = namespaceFactory.isManagingNamespace("123");
// then
- assertFalse(isPredefined);
+ if (!creating) {
+ assertFalse(
+ managing,
+ format(
+ "legacyProp=%s, legacyExists=%s, namespaceProp=%s, expectedCreating=%s",
+ legacyProperty, legacyNamespaceExists, namespaceProperty, expectedCreating));
+ }
}
@Test
public void shouldCreateAndPrepareNamespaceWithPredefinedValueIfItIsNotEmpty() throws Exception {
// given
namespaceFactory =
- spy(new KubernetesNamespaceFactory("predefined", "", "", "che", false, clientFactory));
+ spy(
+ new KubernetesNamespaceFactory(
+ "predefined", "", "", "che", false, clientFactory, workspaceManager));
KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class);
doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespace(any(), any());
@@ -278,7 +389,10 @@ public class KubernetesNamespaceFactoryTest {
public void shouldCreateAndPrepareNamespaceWithWorkspaceIdAsNameIfConfiguredNameIsNotPredefined()
throws Exception {
// given
- namespaceFactory = spy(new KubernetesNamespaceFactory("", "", "", "che", false, clientFactory));
+ namespaceFactory =
+ spy(
+ new KubernetesNamespaceFactory(
+ "", "", "", "che", false, clientFactory, workspaceManager));
KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class);
doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespace(any(), any());
@@ -296,7 +410,10 @@ public class KubernetesNamespaceFactoryTest {
shouldCreateNamespaceAndDoNotPrepareNamespaceOnCreatingNamespaceWithWorkspaceIdAndNameSpecified()
throws Exception {
// given
- namespaceFactory = spy(new KubernetesNamespaceFactory("", "", "", "che", false, clientFactory));
+ namespaceFactory =
+ spy(
+ new KubernetesNamespaceFactory(
+ "", "", "", "che", false, clientFactory, workspaceManager));
KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class);
doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespace(any(), any());
@@ -314,7 +431,9 @@ public class KubernetesNamespaceFactoryTest {
throws Exception {
// given
namespaceFactory =
- spy(new KubernetesNamespaceFactory("", "serviceAccount", "", "che", false, clientFactory));
+ spy(
+ new KubernetesNamespaceFactory(
+ "", "serviceAccount", "", "<workspaceid>", false, clientFactory, workspaceManager));
KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class);
doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespace(any(), any());
@@ -337,7 +456,13 @@ public class KubernetesNamespaceFactoryTest {
namespaceFactory =
spy(
new KubernetesNamespaceFactory(
- "namespace", "serviceAccount", "clusterRole", "che", false, clientFactory));
+ "namespace",
+ "serviceAccount",
+ "clusterRole",
+ "che",
+ false,
+ clientFactory,
+ workspaceManager));
KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class);
doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespace(any(), any());
@@ -356,7 +481,10 @@ public class KubernetesNamespaceFactoryTest {
public void shouldNotPrepareWorkspaceServiceAccountIfItIsNotConfiguredAndProjectIsNotPredefined()
throws Exception {
// given
- namespaceFactory = spy(new KubernetesNamespaceFactory("", "", "", "che", false, clientFactory));
+ namespaceFactory =
+ spy(
+ new KubernetesNamespaceFactory(
+ "", "", "", "che", false, clientFactory, workspaceManager));
KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class);
doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespace(any(), any());
@@ -372,21 +500,106 @@ public class KubernetesNamespaceFactoryTest {
}
@Test
- public void testPlaceholder() {
+ public void
+ testEvalNamespaceUsesNamespaceDefaultIfWorkspaceDoesntRecordNamespaceAndLegacyNamespaceDoesntExist()
+ throws Exception {
+ namespaceFactory =
+ new KubernetesNamespaceFactory(
+ "blabol-<userid>-<username>-<userid>-<username>--",
+ "",
+ "",
+ "che-<userid>",
+ false,
+ clientFactory,
+ workspaceManager);
+
+ when(namespaceResource.get()).thenReturn(null);
+
+ String namespace =
+ namespaceFactory.evalNamespaceName(null, new SubjectImpl("JonDoe", "123", null, false));
+
+ assertEquals(namespace, "che-123");
+ }
+
+ @Test
+ public void
+ testEvalNamespaceUsesLegacyNamespaceIfWorkspaceDoesntRecordNamespaceAndLegacyNamespaceExists()
+ throws Exception {
+
namespaceFactory =
new KubernetesNamespaceFactory(
"blabol-<userid>-<username>-<userid>-<username>--",
"",
"",
- "che",
+ "che-<userid>",
false,
- clientFactory);
+ clientFactory,
+ workspaceManager);
+
String namespace =
namespaceFactory.evalNamespaceName(null, new SubjectImpl("JonDoe", "123", null, false));
assertEquals(namespace, "blabol-123-JonDoe-123-JonDoe--");
}
+ @Test
+ public void testEvalNamespaceUsesWorkspaceRecordedNamespaceIfWorkspaceRecordsIt()
+ throws Exception {
+
+ namespaceFactory =
+ new KubernetesNamespaceFactory(
+ "blabol-<userid>-<username>-<userid>-<username>--",
+ "",
+ "",
+ "che-<userid>",
+ false,
+ clientFactory,
+ workspaceManager);
+
+ when(workspaceManager.getWorkspace(eq("42")))
+ .thenReturn(
+ WorkspaceImpl.builder()
+ .setId("42")
+ .setAttributes(
+ singletonMap(
+ Constants.WORKSPACE_INFRASTRUCTURE_NAMESPACE_ATTRIBUTE, "wkspcnmspc"))
+ .build());
+
+ String namespace =
+ namespaceFactory.evalNamespaceName("42", new SubjectImpl("JonDoe", "123", null, false));
+
+ assertEquals(namespace, "wkspcnmspc");
+ }
+
+ @Test
+ public void testEvalNamespaceTreatsWorkspaceRecordedNamespaceLiterally() throws Exception {
+
+ namespaceFactory =
+ new KubernetesNamespaceFactory(
+ "blabol-<userid>-<username>-<userid>-<username>--",
+ "",
+ "",
+ "che-<userid>",
+ false,
+ clientFactory,
+ workspaceManager);
+
+ when(workspaceManager.getWorkspace(eq("42")))
+ .thenReturn(
+ WorkspaceImpl.builder()
+ .setId("42")
+ .setAttributes(
+ singletonMap(
+ Constants.WORKSPACE_INFRASTRUCTURE_NAMESPACE_ATTRIBUTE, "<userid>"))
+ .build());
+
+ String namespace =
+ namespaceFactory.evalNamespaceName("42", new SubjectImpl("JonDoe", "123", null, false));
+
+ // this is an invalid name, but that is not a purpose of this test.
+ assertEquals(namespace, "<userid>");
+ }
+
private void prepareNamespaceToBeFoundByName(String name, Namespace namespace) throws Exception {
@SuppressWarnings("unchecked")
Resource<Namespace, DoneableNamespace> getNamespaceByNameOperation = mock(Resource.class);
diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceTest.java
index 6cfa7eeb90..a57e1ba13a 100644
--- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceTest.java
+++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceTest.java
@@ -18,6 +18,7 @@ import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.testng.Assert.assertEquals;
@@ -98,11 +99,16 @@ public class KubernetesNamespaceTest {
@Test
public void testKubernetesNamespacePreparingWhenNamespaceExists() throws Exception {
// given
+ MetadataNested namespaceMeta = prepareCreateNamespaceRequest();
+
prepareNamespace(NAMESPACE);
KubernetesNamespace namespace = new KubernetesNamespace(clientFactory, NAMESPACE, WORKSPACE_ID);
// when
namespace.prepare();
+
+ // then
+ verify(namespaceMeta, never()).withName(NAMESPACE);
}
@Test
@@ -216,6 +222,49 @@ public class KubernetesNamespaceTest {
verify(serviceAccountResource).watch(any());
}
+ @Test
+ public void testDeletesExistingNamespace() throws Exception {
+ // given
+ KubernetesNamespace namespace = new KubernetesNamespace(clientFactory, NAMESPACE, WORKSPACE_ID);
+ Resource resource = prepareNamespaceResource(NAMESPACE);
+
+ // when
+ namespace.delete();
+
+ // then
+ verify(resource).delete();
+ }
+
+ @Test
+ public void testDoesntFailIfDeletedNamespaceDoesntExist() throws Exception {
+ // given
+ KubernetesNamespace namespace = new KubernetesNamespace(clientFactory, NAMESPACE, WORKSPACE_ID);
+ Resource resource = prepareNamespaceResource(NAMESPACE);
+ when(resource.delete()).thenThrow(new KubernetesClientException("err", 404, null));
+
+ // when
+ namespace.delete();
+
+ // then
+ verify(resource).delete();
+ // and no exception is thrown
+ }
+
+ @Test
+ public void testDoesntFailIfDeletedNamespaceIsBeingDeleted() throws Exception {
+ // given
+ KubernetesNamespace namespace = new KubernetesNamespace(clientFactory, NAMESPACE, WORKSPACE_ID);
+ Resource resource = prepareNamespaceResource(NAMESPACE);
+ when(resource.delete()).thenThrow(new KubernetesClientException("err", 409, null));
+
+ // when
+ namespace.delete();
+
+ // then
+ verify(resource).delete();
+ // and no exception is thrown
+ }
+
private MetadataNested prepareCreateNamespaceRequest() {
DoneableNamespace namespace = mock(DoneableNamespace.class);
MetadataNested metadataNested = mock(MetadataNested.class);
diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/RemoveNamespaceOnWorkspaceRemoveTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/RemoveNamespaceOnWorkspaceRemoveTest.java
index fbe81882a7..94f808bb07 100644
--- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/RemoveNamespaceOnWorkspaceRemoveTest.java
+++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/RemoveNamespaceOnWorkspaceRemoveTest.java
@@ -11,9 +11,11 @@
*/
package org.eclipse.che.workspace.infrastructure.kubernetes.namespace;
+import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
-import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -38,14 +40,15 @@ public class RemoveNamespaceOnWorkspaceRemoveTest {
private static final String WORKSPACE_ID = "workspace123";
@Mock private Workspace workspace;
+ @Mock private KubernetesNamespaceFactory namespaceFactory;
private RemoveNamespaceOnWorkspaceRemove removeNamespaceOnWorkspaceRemove;
@BeforeMethod
public void setUp() throws Exception {
- removeNamespaceOnWorkspaceRemove = spy(new RemoveNamespaceOnWorkspaceRemove(null, null));
+ removeNamespaceOnWorkspaceRemove = spy(new RemoveNamespaceOnWorkspaceRemove(namespaceFactory));
- doNothing().when(removeNamespaceOnWorkspaceRemove).doRemoveNamespace(anyString());
+ lenient().doNothing().when(namespaceFactory).delete(anyString());
when(workspace.getId()).thenReturn(WORKSPACE_ID);
}
@@ -60,9 +63,21 @@ public class RemoveNamespaceOnWorkspaceRemoveTest {
}
@Test
- public void shouldRemoveNamespaceOnWorkspaceRemovedEvent() throws Exception {
+ public void shouldRemoveNamespaceOnWorkspaceRemovedEventIfNamespaceIsManaged() throws Exception {
+ when(namespaceFactory.isManagingNamespace(any())).thenReturn(true);
+
+ removeNamespaceOnWorkspaceRemove.onEvent(new WorkspaceRemovedEvent(workspace));
+
+ verify(namespaceFactory).delete(WORKSPACE_ID);
+ }
+
+ @Test
+ public void shouldNotRemoveNamespaceOnWorkspaceRemovedEventIfNamespaceIsNotManaged()
+ throws Exception {
+ when(namespaceFactory.isManagingNamespace(any())).thenReturn(false);
+
removeNamespaceOnWorkspaceRemove.onEvent(new WorkspaceRemovedEvent(workspace));
- verify(removeNamespaceOnWorkspaceRemove).doRemoveNamespace(WORKSPACE_ID);
+ verify(namespaceFactory, never()).delete(any());
}
}
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 c54b9a16c1..48cda98184 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
@@ -13,9 +13,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;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
@@ -23,7 +21,6 @@ import static org.mockito.Mockito.when;
import org.eclipse.che.api.core.model.workspace.Workspace;
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.workspace.infrastructure.kubernetes.namespace.KubernetesNamespaceFactory;
@@ -42,20 +39,25 @@ import org.testng.annotations.Test;
public class WorkspacePVCCleanerTest {
@Mock private WorkspaceVolumesStrategy pvcStrategy;
- @Mock private EventService eventService;
+ private EventService eventService;
@Mock private KubernetesNamespaceFactory namespaceFactory;
@Mock private Workspace workspace;
+ @Mock WorkspaceRemovedEvent event;
private WorkspacePVCCleaner workspacePVCCleaner;
@BeforeMethod
- public void setUp() {
- when(namespaceFactory.isPredefined()).thenReturn(true);
+ public void setUp() throws Exception {
+ when(namespaceFactory.isManagingNamespace(any())).thenReturn(false);
workspacePVCCleaner = new WorkspacePVCCleaner(true, namespaceFactory, pvcStrategy);
+ when(workspace.getId()).thenReturn("123");
+ when(event.getWorkspace()).thenReturn(workspace);
+
+ eventService = spy(new EventService());
}
@Test
- public void testDoNotSubscribesCleanerWhenPVCDisabled() {
+ public void testDoNotSubscribesCleanerWhenPVCDisabled() throws Exception {
workspacePVCCleaner = spy(new WorkspacePVCCleaner(false, namespaceFactory, pvcStrategy));
workspacePVCCleaner.subscribe(eventService);
@@ -64,58 +66,42 @@ public class WorkspacePVCCleanerTest {
}
@Test
- public void testDoNotSubscribesCleanerWhenPVCEnabledAndNamespaceIsNotPredefined() {
- when(namespaceFactory.isPredefined()).thenReturn(false);
- workspacePVCCleaner = spy(new WorkspacePVCCleaner(false, namespaceFactory, pvcStrategy));
-
+ public void testSubscribesDeleteKubernetesOnWorkspaceRemove() throws Exception {
workspacePVCCleaner.subscribe(eventService);
- verify(eventService, never()).subscribe(any(), eq(WorkspaceRemovedEvent.class));
+ verify(eventService).subscribe(any(), eq(WorkspaceRemovedEvent.class));
}
@Test
- public void testSubscribesDeleteKubernetesOnWorkspaceRemove() throws Exception {
+ public void testInvokeCleanupWhenWorkspaceRemovedEventPublishedAndNamespaceIsNotManaged()
+ throws Exception {
workspacePVCCleaner.subscribe(eventService);
- verify(eventService).subscribe(any(), eq(WorkspaceRemovedEvent.class));
+ eventService.publish(event);
+
+ verify(pvcStrategy).cleanup(workspace);
}
@Test
- public void testInvokeCleanupWhenWorkspaceRemovedEventPublished() throws Exception {
- doAnswer(
- invocationOnMock -> {
- final EventSubscriber<WorkspaceRemovedEvent> argument =
- invocationOnMock.getArgument(0);
- final WorkspaceRemovedEvent event = mock(WorkspaceRemovedEvent.class);
- when(event.getWorkspace()).thenReturn(workspace);
- argument.onEvent(event);
- return invocationOnMock;
- })
- .when(eventService)
- .subscribe(any(), eq(WorkspaceRemovedEvent.class));
+ public void testNotInvokeCleanupWhenWorkspaceRemovedEventPublishedAndNamespaceIsManaged()
+ throws Exception {
+ when(namespaceFactory.isManagingNamespace(any())).thenReturn(true);
workspacePVCCleaner.subscribe(eventService);
- verify(pvcStrategy).cleanup(workspace);
+ eventService.publish(event);
+
+ verify(pvcStrategy, never()).cleanup(workspace);
}
@Test
public void testDoNotRethrowExceptionWhenErrorOnCleanupOccurs() throws Exception {
- doAnswer(
- invocationOnMock -> {
- final EventSubscriber<WorkspaceRemovedEvent> argument =
- invocationOnMock.getArgument(0);
- final WorkspaceRemovedEvent event = mock(WorkspaceRemovedEvent.class);
- when(event.getWorkspace()).thenReturn(workspace);
- argument.onEvent(event);
- return invocationOnMock;
- })
- .when(eventService)
- .subscribe(any(), eq(WorkspaceRemovedEvent.class));
doThrow(InfrastructureException.class).when(pvcStrategy).cleanup(workspace);
workspacePVCCleaner.subscribe(eventService);
+ eventService.publish(event);
+
verify(pvcStrategy).cleanup(workspace);
}
}
diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProject.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProject.java
index c6b148fbd7..bd5ec9c8e1 100644
--- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProject.java
+++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProject.java
@@ -11,6 +11,8 @@
*/
package org.eclipse.che.workspace.infrastructure.openshift.project;
+import static java.lang.String.format;
+
import com.google.common.annotations.VisibleForTesting;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.KubernetesClientException;
@@ -94,6 +96,20 @@ public class OpenShiftProject extends KubernetesNamespace {
}
}
+ /**
+ * Deletes the project. Deleting a non-existent projects is not an error as is not an attempt to
+ * delete a project that is already being deleted.
+ *
+ * @throws InfrastructureException if any unexpected exception occurs during project deletion
+ */
+ void delete() throws InfrastructureException {
+ String workspaceId = getWorkspaceId();
+ String projectName = getName();
+
+ OpenShiftClient osClient = clientFactory.createOC(workspaceId);
+ delete(projectName, osClient);
+ }
+
/** Returns object for managing {@link Route} instances inside project. */
public OpenShiftRoutes routes() {
return routes;
@@ -128,6 +144,23 @@ public class OpenShiftProject extends KubernetesNamespace {
}
}
+ private void delete(String projectName, OpenShiftClient osClient) throws InfrastructureException {
+ try {
+ osClient.projects().withName(projectName).delete();
+ } catch (KubernetesClientException e) {
+ if (e.getCode() == 404) {
+ LOG.warn(
+ format(
+ "Tried to delete project '%s' but it doesn't exist in the cluster.", projectName),
+ e);
+ } else if (e.getCode() == 409) {
+ LOG.info(format("The project '%s' is currently being deleted.", projectName), e);
+ } else {
+ throw new KubernetesInfrastructureException(e);
+ }
+ }
+ }
+
private Project get(String projectName, OpenShiftClient client) throws InfrastructureException {
try {
return client.projects().withName(projectName).get();
diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectFactory.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectFactory.java
index fb4f2bb9e6..f998faffbf 100644
--- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectFactory.java
+++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectFactory.java
@@ -12,6 +12,7 @@
package org.eclipse.che.workspace.infrastructure.openshift.project;
import static com.google.common.base.Strings.isNullOrEmpty;
+import static java.lang.String.format;
import static org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta.PHASE_ATTRIBUTE;
import com.google.common.annotations.VisibleForTesting;
@@ -26,9 +27,15 @@ import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;
import javax.inject.Named;
+import org.eclipse.che.api.core.ConflictException;
+import org.eclipse.che.api.core.NotFoundException;
+import org.eclipse.che.api.core.ServerException;
+import org.eclipse.che.api.core.ValidationException;
+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.commons.env.EnvironmentContext;
+import org.eclipse.che.commons.subject.Subject;
import org.eclipse.che.workspace.infrastructure.kubernetes.api.server.impls.KubernetesNamespaceMetaImpl;
import org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta;
import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespaceFactory;
@@ -58,14 +65,16 @@ public class OpenShiftProjectFactory extends KubernetesNamespaceFactory {
@Named("che.infra.kubernetes.namespace.allow_user_defined")
boolean allowUserDefinedNamespaces,
OpenShiftClientFactory clientFactory,
- OpenShiftClientConfigFactory clientConfigFactory) {
+ OpenShiftClientConfigFactory clientConfigFactory,
+ WorkspaceManager workspaceManager) {
super(
projectName,
serviceAccountName,
clusterRoleName,
defaultNamespaceName,
allowUserDefinedNamespaces,
- clientFactory);
+ clientFactory,
+ workspaceManager);
if (allowUserDefinedNamespaces && !clientConfigFactory.isPersonalized()) {
LOG.warn(
"Users are allowed to list projects but Che server is configured with a service account. "
@@ -86,12 +95,21 @@ public class OpenShiftProjectFactory extends KubernetesNamespaceFactory {
* @throws InfrastructureException if any exception occurs during project preparing
*/
public OpenShiftProject create(String workspaceId) throws InfrastructureException {
- final String projectName =
- evalNamespaceName(workspaceId, EnvironmentContext.getCurrent().getSubject());
+ final String projectName;
+ try {
+ projectName = evalNamespaceName(workspaceId, EnvironmentContext.getCurrent().getSubject());
+ } catch (NotFoundException | ServerException | ConflictException | ValidationException e) {
+ throw new InfrastructureException(
+ format(
+ "Failed to evaluate the project name to use for workspace %s. The error message"
+ + " was: %s",
+ workspaceId, e.getMessage()),
+ e);
+ }
OpenShiftProject osProject = doCreateProject(workspaceId, projectName);
osProject.prepare();
- if (!isPredefined() && !isNullOrEmpty(getServiceAccountName())) {
+ if (isCreatingNamespace(workspaceId) && !isNullOrEmpty(getServiceAccountName())) {
// prepare service account for workspace only if account name is configured
// and project is not predefined
// since predefined project should be prepared during Che deployment
@@ -103,6 +121,37 @@ public class OpenShiftProjectFactory extends KubernetesNamespaceFactory {
return osProject;
}
+ @Override
+ public void delete(String workspaceId) throws InfrastructureException {
+ String projectName;
+ try {
+ projectName = evalNamespaceName(workspaceId, EnvironmentContext.getCurrent().getSubject());
+ } catch (NotFoundException | ServerException | ConflictException | ValidationException e) {
+ throw new InfrastructureException(
+ format(
+ "Failed to evaluate the project name to use for workspace %s."
+ + " The error message was: %s",
+ workspaceId, e.getMessage()),
+ e);
+ }
+
+ OpenShiftProject osProject = doCreateProject(workspaceId, projectName);
+ osProject.delete();
+ }
+
+ @VisibleForTesting
+ @Override
+ protected String evalNamespaceName(String workspaceId, Subject currentUser)
+ throws NotFoundException, ServerException, InfrastructureException, ConflictException,
+ ValidationException {
+ return super.evalNamespaceName(workspaceId, currentUser);
+ }
+
+ @Override
+ protected boolean checkNamespaceExists(String namespaceName) throws InfrastructureException {
+ return fetchNamespaceObject(namespaceName).isPresent();
+ }
+
/**
* Creates a kubernetes namespace for the specified workspace.
*
@@ -129,9 +178,13 @@ public class OpenShiftProjectFactory extends KubernetesNamespaceFactory {
@Override
protected Optional<KubernetesNamespaceMeta> fetchNamespace(String name)
throws InfrastructureException {
+ return fetchNamespaceObject(name).map(this::asNamespaceMeta);
+ }
+
+ private Optional<Project> fetchNamespaceObject(String name) throws InfrastructureException {
try {
Project project = clientFactory.createOC().projects().withName(name).get();
- return Optional.of(asNamespaceMeta(project));
+ return Optional.ofNullable(project);
} catch (KubernetesClientException e) {
if (e.getCode() == 403) {
// 403 means that the project does not exist
diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/RemoveProjectOnWorkspaceRemove.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/RemoveProjectOnWorkspaceRemove.java
index 8af34840e7..fd6c7fe1fd 100644
--- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/RemoveProjectOnWorkspaceRemove.java
+++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/RemoveProjectOnWorkspaceRemove.java
@@ -11,20 +11,12 @@
*/
package org.eclipse.che.workspace.infrastructure.openshift.project;
-import static com.google.common.base.Strings.isNullOrEmpty;
-
-import com.google.common.annotations.VisibleForTesting;
import com.google.inject.Inject;
import com.google.inject.Singleton;
-import io.fabric8.kubernetes.client.KubernetesClientException;
-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.KubernetesInfrastructureException;
-import org.eclipse.che.workspace.infrastructure.openshift.OpenShiftClientFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -38,45 +30,30 @@ public class RemoveProjectOnWorkspaceRemove implements EventSubscriber<Workspace
private static final Logger LOG = LoggerFactory.getLogger(RemoveProjectOnWorkspaceRemove.class);
- private final OpenShiftClientFactory clientFactory;
- private final String projectName;
+ private final OpenShiftProjectFactory projectFactory;
@Inject
- public RemoveProjectOnWorkspaceRemove(
- @Nullable @Named("che.infra.kubernetes.namespace") String projectName,
- OpenShiftClientFactory clientFactory) {
- this.projectName = projectName;
- this.clientFactory = clientFactory;
+ public RemoveProjectOnWorkspaceRemove(OpenShiftProjectFactory projectFactory) {
+ this.projectFactory = projectFactory;
}
@Inject
public void subscribe(EventService eventService) {
- if (isNullOrEmpty(projectName)) {
- eventService.subscribe(this);
- }
+ eventService.subscribe(this);
}
@Override
public void onEvent(WorkspaceRemovedEvent event) {
+ String workspaceId = event.getWorkspace().getId();
try {
- doRemoveProject(event.getWorkspace().getId());
+ if (projectFactory.isManagingNamespace(workspaceId)) {
+ projectFactory.delete(workspaceId);
+ }
} catch (InfrastructureException e) {
LOG.warn(
"Fail to remove OpenShift project for workspace with id {}. Cause: {}",
- event.getWorkspace().getId(),
+ workspaceId,
e.getMessage());
}
}
-
- @VisibleForTesting
- void doRemoveProject(String projectName) throws InfrastructureException {
- try {
- clientFactory.createOC(projectName).projects().withName(projectName).delete();
- } catch (KubernetesClientException e) {
- if (!(e.getCode() == 403)) {
- throw new KubernetesInfrastructureException(e);
- }
- // project doesn't exist
- }
- }
}
diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectFactoryTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectFactoryTest.java
index 5963d1fda0..24da0326e3 100644
--- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectFactoryTest.java
+++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectFactoryTest.java
@@ -11,7 +11,9 @@
*/
package org.eclipse.che.workspace.infrastructure.openshift.project;
+import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonList;
+import static java.util.Collections.singletonMap;
import static org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta.DEFAULT_ATTRIBUTE;
import static org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta.PHASE_ATTRIBUTE;
import static org.eclipse.che.workspace.infrastructure.openshift.Constants.PROJECT_DESCRIPTION_ANNOTATION;
@@ -19,6 +21,7 @@ import static org.eclipse.che.workspace.infrastructure.openshift.Constants.PROJE
import static org.eclipse.che.workspace.infrastructure.openshift.Constants.PROJECT_DISPLAY_NAME_ANNOTATION;
import static org.eclipse.che.workspace.infrastructure.openshift.Constants.PROJECT_DISPLAY_NAME_ATTRIBUTE;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
@@ -42,7 +45,11 @@ import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+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.api.workspace.shared.Constants;
+import org.eclipse.che.commons.subject.SubjectImpl;
import org.eclipse.che.inject.ConfigurationException;
import org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta;
import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespace;
@@ -64,12 +71,15 @@ public class OpenShiftProjectFactoryTest {
@Mock private OpenShiftClientConfigFactory configFactory;
@Mock private OpenShiftClientFactory clientFactory;
+ @Mock private WorkspaceManager workspaceManager;
@Mock
private NonNamespaceOperation<
Project, ProjectList, DoneableProject, Resource<Project, DoneableProject>>
projectOperation;
+ @Mock private Resource<Project, DoneableProject> projectResource;
+
@Mock private OpenShiftClient osClient;
private OpenShiftProjectFactory projectFactory;
@@ -78,6 +88,12 @@ public class OpenShiftProjectFactoryTest {
public void setUp() throws Exception {
lenient().when(clientFactory.createOC()).thenReturn(osClient);
lenient().when(osClient.projects()).thenReturn(projectOperation);
+
+ when(workspaceManager.getWorkspace(any()))
+ .thenReturn(WorkspaceImpl.builder().setId("1").setAttributes(emptyMap()).build());
+
+ when(projectOperation.withName(any())).thenReturn(projectResource);
+ when(projectResource.get()).thenReturn(mock(Project.class));
}
@Test(
@@ -90,7 +106,7 @@ public class OpenShiftProjectFactoryTest {
throws Exception {
projectFactory =
new OpenShiftProjectFactory(
- "projectName", "", "", null, false, clientFactory, configFactory);
+ "projectName", "", "", null, false, clientFactory, configFactory, workspaceManager);
}
@Test
@@ -112,7 +128,14 @@ public class OpenShiftProjectFactoryTest {
projectFactory =
new OpenShiftProjectFactory(
- "predefined", "", "", "che-default", false, clientFactory, configFactory);
+ "predefined",
+ "",
+ "",
+ "che-default",
+ false,
+ clientFactory,
+ configFactory,
+ workspaceManager);
List<KubernetesNamespaceMeta> availableNamespaces = projectFactory.list();
assertEquals(availableNamespaces.size(), 1);
@@ -135,7 +158,14 @@ public class OpenShiftProjectFactoryTest {
projectFactory =
new OpenShiftProjectFactory(
- "predefined", "", "", "che-default", false, clientFactory, configFactory);
+ "predefined",
+ "",
+ "",
+ "che-default",
+ false,
+ clientFactory,
+ configFactory,
+ workspaceManager);
List<KubernetesNamespaceMeta> availableNamespaces = projectFactory.list();
assertEquals(availableNamespaces.size(), 1);
@@ -158,7 +188,14 @@ public class OpenShiftProjectFactoryTest {
projectFactory =
new OpenShiftProjectFactory(
- "predefined", "", "", "che-default", false, clientFactory, configFactory);
+ "predefined",
+ "",
+ "",
+ "che-default",
+ false,
+ clientFactory,
+ configFactory,
+ workspaceManager);
projectFactory.list();
}
@@ -171,7 +208,8 @@ public class OpenShiftProjectFactoryTest {
createProject("experimental", null, null, "Terminating")));
projectFactory =
- new OpenShiftProjectFactory("predefined", "", "", null, true, clientFactory, configFactory);
+ new OpenShiftProjectFactory(
+ "predefined", "", "", null, true, clientFactory, configFactory, workspaceManager);
List<KubernetesNamespaceMeta> availableNamespaces = projectFactory.list();
assertEquals(availableNamespaces.size(), 2);
@@ -197,7 +235,7 @@ public class OpenShiftProjectFactoryTest {
projectFactory =
new OpenShiftProjectFactory(
- "predefined", "", "", "default", true, clientFactory, configFactory);
+ "predefined", "", "", "default", true, clientFactory, configFactory, workspaceManager);
List<KubernetesNamespaceMeta> availableNamespaces = projectFactory.list();
@@ -228,7 +266,7 @@ public class OpenShiftProjectFactoryTest {
projectFactory =
new OpenShiftProjectFactory(
- "predefined", "", "", "default", true, clientFactory, configFactory);
+ "predefined", "", "", "default", true, clientFactory, configFactory, workspaceManager);
List<KubernetesNamespaceMeta> availableNamespaces = projectFactory.list();
assertEquals(availableNamespaces.size(), 2);
@@ -253,7 +291,8 @@ public class OpenShiftProjectFactoryTest {
public void shouldThrownExceptionWhenFailedToGetNamespaces() throws Exception {
throwOnTryToGetProjectsList(new KubernetesClientException("connection refused"));
projectFactory =
- new OpenShiftProjectFactory("predefined", "", "", "", true, clientFactory, configFactory);
+ new OpenShiftProjectFactory(
+ "predefined", "", "", "", true, clientFactory, configFactory, workspaceManager);
projectFactory.list();
}
@@ -264,7 +303,14 @@ public class OpenShiftProjectFactoryTest {
projectFactory =
spy(
new OpenShiftProjectFactory(
- "projectName", "", "", "che", false, clientFactory, configFactory));
+ "projectName",
+ "",
+ "",
+ "che",
+ false,
+ clientFactory,
+ configFactory,
+ workspaceManager));
OpenShiftProject toReturnProject = mock(OpenShiftProject.class);
doReturn(toReturnProject).when(projectFactory).doCreateProject(any(), any());
@@ -282,7 +328,9 @@ public class OpenShiftProjectFactoryTest {
throws Exception {
// given
projectFactory =
- spy(new OpenShiftProjectFactory("", "", "", "che", false, clientFactory, configFactory));
+ spy(
+ new OpenShiftProjectFactory(
+ "", "", "", "che", false, clientFactory, configFactory, workspaceManager));
OpenShiftProject toReturnProject = mock(OpenShiftProject.class);
doReturn(toReturnProject).when(projectFactory).doCreateProject(any(), any());
@@ -302,7 +350,14 @@ public class OpenShiftProjectFactoryTest {
projectFactory =
spy(
new OpenShiftProjectFactory(
- "", "serviceAccount", "", "che", false, clientFactory, configFactory));
+ "",
+ "serviceAccount",
+ "",
+ "<workspaceid>",
+ false,
+ clientFactory,
+ configFactory,
+ workspaceManager));
OpenShiftProject toReturnProject = mock(OpenShiftProject.class);
doReturn(toReturnProject).when(projectFactory).doCreateProject(any(), any());
@@ -330,7 +385,8 @@ public class OpenShiftProjectFactoryTest {
"che",
false,
clientFactory,
- configFactory));
+ configFactory,
+ workspaceManager));
OpenShiftProject toReturnProject = mock(OpenShiftProject.class);
doReturn(toReturnProject).when(projectFactory).doCreateProject(any(), any());
@@ -346,7 +402,9 @@ public class OpenShiftProjectFactoryTest {
throws Exception {
// given
projectFactory =
- spy(new OpenShiftProjectFactory("", "", "", "che", false, clientFactory, configFactory));
+ spy(
+ new OpenShiftProjectFactory(
+ "", "", "", "che", false, clientFactory, configFactory, workspaceManager));
OpenShiftProject toReturnProject = mock(OpenShiftProject.class);
doReturn(toReturnProject).when(projectFactory).doCreateProject(any(), any());
@@ -371,7 +429,8 @@ public class OpenShiftProjectFactoryTest {
"che",
false,
clientFactory,
- configFactory));
+ configFactory,
+ workspaceManager));
OpenShiftProject toReturnProject = mock(OpenShiftProject.class);
doReturn(toReturnProject).when(projectFactory).doCreateProject(any(), any());
@@ -384,6 +443,111 @@ public class OpenShiftProjectFactoryTest {
verify(toReturnProject, never()).prepare();
}
+ @Test
+ public void
+ testEvalNamespaceUsesNamespaceDefaultIfWorkspaceDoesntRecordNamespaceAndLegacyNamespaceDoesntExist()
+ throws Exception {
+ projectFactory =
+ new OpenShiftProjectFactory(
+ "blabol-<userid>-<username>-<userid>-<username>--",
+ "",
+ "",
+ "che-<userid>",
+ false,
+ clientFactory,
+ configFactory,
+ workspaceManager);
+
+ when(projectResource.get()).thenThrow(new KubernetesClientException("", 403, null));
+
+ String namespace =
+ projectFactory.evalNamespaceName(null, new SubjectImpl("JonDoe", "123", null, false));
+
+ assertEquals(namespace, "che-123");
+ }
+
+ @Test
+ public void
+ testEvalNamespaceUsesLegacyNamespaceIfWorkspaceDoesntRecordNamespaceAndLegacyNamespaceExists()
+ throws Exception {
+
+ projectFactory =
+ new OpenShiftProjectFactory(
+ "blabol-<userid>-<username>-<userid>-<username>--",
+ "",
+ "",
+ "che-<userid>",
+ false,
+ clientFactory,
+ configFactory,
+ workspaceManager);
+
+ String namespace =
+ projectFactory.evalNamespaceName(null, new SubjectImpl("JonDoe", "123", null, false));
+
+ assertEquals(namespace, "blabol-123-JonDoe-123-JonDoe--");
+ }
+
+ @Test
+ public void testEvalNamespaceUsesWorkspaceRecordedNamespaceIfWorkspaceRecordsIt()
+ throws Exception {
+
+ projectFactory =
+ new OpenShiftProjectFactory(
+ "blabol-<userid>-<username>-<userid>-<username>--",
+ "",
+ "",
+ "che-<userid>",
+ false,
+ clientFactory,
+ configFactory,
+ workspaceManager);
+
+ when(workspaceManager.getWorkspace(eq("42")))
+ .thenReturn(
+ WorkspaceImpl.builder()
+ .setId("42")
+ .setAttributes(
+ singletonMap(
+ Constants.WORKSPACE_INFRASTRUCTURE_NAMESPACE_ATTRIBUTE, "wkspcprj"))
+ .build());
+
+ String namespace =
+ projectFactory.evalNamespaceName("42", new SubjectImpl("JonDoe", "123", null, false));
+
+ assertEquals(namespace, "wkspcprj");
+ }
+
+ @Test
+ public void testEvalNamespaceTreatsWorkspaceRecordedNamespaceLiterally() throws Exception {
+
+ projectFactory =
+ new OpenShiftProjectFactory(
+ "blabol-<userid>-<username>-<userid>-<username>--",
+ "",
+ "",
+ "che-<userid>",
+ false,
+ clientFactory,
+ configFactory,
+ workspaceManager);
+
+ when(workspaceManager.getWorkspace(eq("42")))
+ .thenReturn(
+ WorkspaceImpl.builder()
+ .setId("42")
+ .setAttributes(
+ singletonMap(
+ Constants.WORKSPACE_INFRASTRUCTURE_NAMESPACE_ATTRIBUTE, "<userid>"))
+ .build());
+
+ String namespace =
+ projectFactory.evalNamespaceName("42", new SubjectImpl("JonDoe", "123", null, false));
+
+ // this is an invalid name, but that is not a purpose of this test.
+ assertEquals(namespace, "<userid>");
+ }
+
private void prepareNamespaceToBeFoundByName(String name, Project project) throws Exception {
@SuppressWarnings("unchecked")
Resource<Project, DoneableProject> getProjectByNameOperation = mock(Resource.class);
diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectTest.java
index 37c43b7491..74fca56e61 100644
--- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectTest.java
+++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectTest.java
@@ -17,6 +17,7 @@ import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotNull;
@@ -160,6 +161,49 @@ public class OpenShiftProjectTest {
verify(routes).delete();
}
+ @Test
+ public void testDeletesExistingProject() throws Exception {
+ // given
+ OpenShiftProject project = new OpenShiftProject(clientFactory, PROJECT_NAME, WORKSPACE_ID);
+ Resource resource = prepareProjectResource(PROJECT_NAME);
+
+ // when
+ project.delete();
+
+ // then
+ verify(resource).delete();
+ }
+
+ @Test
+ public void testDoesntFailIfDeletedProjectDoesntExist() throws Exception {
+ // given
+ OpenShiftProject project = new OpenShiftProject(clientFactory, PROJECT_NAME, WORKSPACE_ID);
+ Resource resource = prepareProjectResource(PROJECT_NAME);
+ when(resource.delete()).thenThrow(new KubernetesClientException("err", 404, null));
+
+ // when
+ project.delete();
+
+ // then
+ verify(resource).delete();
+ // and no exception is thrown
+ }
+
+ @Test
+ public void testDoesntFailIfDeletedProjectIsBeingDeleted() throws Exception {
+ // given
+ OpenShiftProject project = new OpenShiftProject(clientFactory, PROJECT_NAME, WORKSPACE_ID);
+ Resource resource = prepareProjectResource(PROJECT_NAME);
+ when(resource.delete()).thenThrow(new KubernetesClientException("err", 409, null));
+
+ // when
+ project.delete();
+
+ // then
+ verify(resource).delete();
+ // and no exception is thrown
+ }
+
private MetadataNested prepareProjectRequest() {
ProjectRequestOperation projectRequestOperation = mock(ProjectRequestOperation.class);
DoneableProjectRequest projectRequest = mock(DoneableProjectRequest.class);
diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/project/RemoveProjectOnWorkspaceRemoveTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/project/RemoveProjectOnWorkspaceRemoveTest.java
index be72c96ce2..92ab109e18 100644
--- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/project/RemoveProjectOnWorkspaceRemoveTest.java
+++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/project/RemoveProjectOnWorkspaceRemoveTest.java
@@ -11,9 +11,11 @@
*/
package org.eclipse.che.workspace.infrastructure.openshift.project;
+import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
-import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -38,14 +40,15 @@ public class RemoveProjectOnWorkspaceRemoveTest {
private static final String WORKSPACE_ID = "workspace123";
@Mock private Workspace workspace;
+ @Mock private OpenShiftProjectFactory projectFactory;
private RemoveProjectOnWorkspaceRemove removeProjectOnWorkspaceRemove;
@BeforeMethod
public void setUp() throws Exception {
- removeProjectOnWorkspaceRemove = spy(new RemoveProjectOnWorkspaceRemove(null, null));
+ removeProjectOnWorkspaceRemove = spy(new RemoveProjectOnWorkspaceRemove(projectFactory));
- doNothing().when(removeProjectOnWorkspaceRemove).doRemoveProject(anyString());
+ lenient().doNothing().when(projectFactory).delete(anyString());
when(workspace.getId()).thenReturn(WORKSPACE_ID);
}
@@ -60,9 +63,22 @@ public class RemoveProjectOnWorkspaceRemoveTest {
}
@Test
- public void shouldRemoveProjectOnWorkspaceRemovedEvent() throws Exception {
+ public void shouldRemoveProjectOnWorkspaceRemovedEventIfFactoryIsManagingNamespaces()
+ throws Exception {
+ when(projectFactory.isManagingNamespace(any())).thenReturn(true);
+
+ removeProjectOnWorkspaceRemove.onEvent(new WorkspaceRemovedEvent(workspace));
+
+ verify(projectFactory).delete(WORKSPACE_ID);
+ }
+
+ @Test
+ public void shouldNotRemoveProjectOnWorkspaceRemovedEventIfFactoryIsNotManagingNamespaces()
+ throws Exception {
+ when(projectFactory.isManagingNamespace(any())).thenReturn(false);
+
removeProjectOnWorkspaceRemove.onEvent(new WorkspaceRemovedEvent(workspace));
- verify(removeProjectOnWorkspaceRemove).doRemoveProject(WORKSPACE_ID);
+ verify(projectFactory, never()).delete(any());
}
}
diff --git a/wsmaster/che-core-api-workspace-shared/src/main/java/org/eclipse/che/api/workspace/shared/Constants.java b/wsmaster/che-core-api-workspace-shared/src/main/java/org/eclipse/che/api/workspace/shared/Constants.java
index a7b34a72d7..406e186807 100644
--- a/wsmaster/che-core-api-workspace-shared/src/main/java/org/eclipse/che/api/workspace/shared/Constants.java
+++ b/wsmaster/che-core-api-workspace-shared/src/main/java/org/eclipse/che/api/workspace/shared/Constants.java
@@ -185,5 +185,12 @@ public final class Constants {
/** When generating workspace name from generateName, append this many characters. */
public static final int WORKSPACE_GENERATE_NAME_CHARS_APPEND = 5;
+ /**
+ * The attribute of the workspace where we store the infrastructure namespace the workspace is
+ * deployed to
+ */
+ public static final String WORKSPACE_INFRASTRUCTURE_NAMESPACE_ATTRIBUTE =
+ "infrastructureNamespace";
+
private Constants() {}
}
diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceManager.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceManager.java
index 554e3ea3ae..9a0a667b73 100644
--- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceManager.java
+++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceManager.java
@@ -415,7 +415,7 @@ public class WorkspaceManager {
.whenComplete(
(aVoid, throwable) -> {
if (workspace.isTemporary()) {
- removeWorkspaceQuietly(workspace);
+ removeWorkspaceQuietly(workspace.getId());
}
});
}
@@ -441,13 +441,13 @@ public class WorkspaceManager {
runtimes
.startAsync(workspace, envName, firstNonNull(options, Collections.emptyMap()))
- .thenAccept(aVoid -> handleStartupSuccess(workspace))
+ .thenAccept(aVoid -> handleStartupSuccess(workspace.getId()))
.exceptionally(
ex -> {
if (workspace.isTemporary()) {
- removeWorkspaceQuietly(workspace);
+ removeWorkspaceQuietly(workspace.getId());
} else {
- handleStartupError(workspace, ex.getCause());
+ handleStartupError(workspace.getId(), ex.getCause());
}
return null;
});
@@ -467,11 +467,14 @@ public class WorkspaceManager {
}
}
- private void removeWorkspaceQuietly(Workspace workspace) {
+ private void removeWorkspaceQuietly(String workspaceId) {
try {
- workspaceDao.remove(workspace.getId());
+ workspaceDao.remove(workspaceId);
} catch (ServerException x) {
- LOG.error("Unable to remove temporary workspace '{}'", workspace.getId());
+ LOG.error(
+ "Unable to remove temporary workspace '{}'. Error message was: {}",
+ workspaceId,
+ x.getMessage());
}
}
@@ -493,35 +496,41 @@ public class WorkspaceManager {
return workspace;
}
- private void handleStartupError(Workspace workspace, Throwable t) {
- workspace
- .getAttributes()
- .put(
- ERROR_MESSAGE_ATTRIBUTE_NAME,
- t instanceof RuntimeException ? t.getCause().getMessage() : t.getMessage());
- workspace.getAttributes().put(STOPPED_ATTRIBUTE_NAME, Long.toString(currentTimeMillis()));
- workspace.getAttributes().put(STOPPED_ABNORMALLY_ATTRIBUTE_NAME, Boolean.toString(true));
+ private void handleStartupError(String workspaceId, Throwable t) {
try {
+ // we need to reload the workspace because the runtimes might have updated it
+ Workspace workspace = getWorkspace(workspaceId);
+ workspace
+ .getAttributes()
+ .put(
+ ERROR_MESSAGE_ATTRIBUTE_NAME,
+ t instanceof RuntimeException ? t.getCause().getMessage() : t.getMessage());
+ workspace.getAttributes().put(STOPPED_ATTRIBUTE_NAME, Long.toString(currentTimeMillis()));
+ workspace.getAttributes().put(STOPPED_ABNORMALLY_ATTRIBUTE_NAME, Boolean.toString(true));
updateWorkspace(workspace.getId(), workspace);
} catch (NotFoundException | ServerException | ValidationException | ConflictException e) {
LOG.warn(
String.format(
"Cannot set error status of the workspace %s. Error is: %s",
- workspace.getId(), e.getMessage()));
+ workspaceId, e.getMessage()));
}
}
- private void handleStartupSuccess(Workspace workspace) {
- workspace.getAttributes().remove(STOPPED_ATTRIBUTE_NAME);
- workspace.getAttributes().remove(STOPPED_ABNORMALLY_ATTRIBUTE_NAME);
- workspace.getAttributes().remove(ERROR_MESSAGE_ATTRIBUTE_NAME);
+ private void handleStartupSuccess(String workspaceId) {
try {
+ // we need to reload the workspace because the runtimes might have updated it
+ Workspace workspace = getWorkspace(workspaceId);
+
+ workspace.getAttributes().remove(STOPPED_ATTRIBUTE_NAME);
+ workspace.getAttributes().remove(STOPPED_ABNORMALLY_ATTRIBUTE_NAME);
+ workspace.getAttributes().remove(ERROR_MESSAGE_ATTRIBUTE_NAME);
+
updateWorkspace(workspace.getId(), workspace);
} catch (NotFoundException | ServerException | ValidationException | ConflictException e) {
LOG.warn(
String.format(
"Cannot clear error status status of the workspace %s. Error is: %s",
- workspace.getId(), e.getMessage()));
+ workspaceId, e.getMessage()));
}
}
diff --git a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/WorkspaceManagerTest.java b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/WorkspaceManagerTest.java
index e46956041b..bc8a7788b1 100644
--- a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/WorkspaceManagerTest.java
+++ b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/WorkspaceManagerTest.java
@@ -589,9 +589,10 @@ public class WorkspaceManagerTest {
final WorkspaceImpl workspace = createAndMockWorkspace(workspaceConfig, NAMESPACE_1);
mockAnyWorkspaceStartFailed(new ServerException("start failed"));
- workspaceManager.startWorkspace(workspaceConfig, workspace.getNamespace(), false, emptyMap());
- verify(workspaceDao).update(workspaceCaptor.capture());
- Workspace ws = workspaceCaptor.getAllValues().get(workspaceCaptor.getAllValues().size() - 1);
+ workspaceManager.startWorkspace(workspace.getId(), null, null);
+ // the first update is capturing the start time, the second update is capturing the error
+ verify(workspaceDao, times(2)).update(workspaceCaptor.capture());
+ Workspace ws = workspaceCaptor.getAllValues().get(1);
assertNotNull(ws.getAttributes().get(STOPPED_ATTRIBUTE_NAME));
assertTrue(Boolean.valueOf(ws.getAttributes().get(STOPPED_ABNORMALLY_ATTRIBUTE_NAME)));
assertEquals(ws.getAttributes().get(ERROR_MESSAGE_ATTRIBUTE_NAME), "start failed");
--
GitLab