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 4102405b77494020f74816bbc1b5f4dfe4e79f8d..e2ed48c89cbc3d027c518ee90ed8a9ec36adce64 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 233bbe6a28d9d777198e901ad485ef21b8fdfebc..8dcbc1d3eefc998dd7c2f51c3f3d96bc94be53f2 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 0fa452f5be3896716f9eb9451f0a6f437f702754..339fb24e5db099c963a569efb5b4640293b046dd 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 9801d644bf93573f8a1ffee702159c39b5cdcf73..e5b8f05ccfcebf067837128855e11ccf81e638af 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 0c35fefe0f77fde628b8c2ce5da61bc081f3b1b1..53df54a700a9587aef1659552faab18e1ed7da4d 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 310667740a972aba74b5dce0a84db8455dfc334a..52bd1d46f22546f18a54a1592ad76ca26ef542b3 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 0bcd17c4e2bf303058a8c1118b82dbb4e97293e5..5faa5f6b08493a38dc252d33ea8bd97de851ec1f 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 59e3732c30304d8d9bcb43f09ea677fe7da0fc4d..4cf8e2bb36c0207fcf56aa171d068489dc9d3584 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 b2970314887ea6ea15a25b117ff16b0fd55e0fd6..24ab39322c6785a9ee4f4ecd1757e1cb280f19b9 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 16cb46375ed05ff7c2f4285dcc82012dc9e7676f..cc4c9373a7d877f89db5078cdb486009009bdbc6 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 243d454649782e3f53c16d84ca7f05dfb588a0a9..dd2718a0cfefd4fe57901160162b1d08b6eb47db 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 14815a4fdcc50c1b49a36bab23b34c1e7ce29913..058ead96f7db310dc9b14b0627cd2f7563bf3024 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 cdc365f42803fcadd94c8e6a54218795939d3c89..89ce7d4247bfb6d2fe713d613d5623c548448f50 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 b1e1b1b46b7e460bd1235691222f7035bf89ddf6..8065ba3e571ef7aa2f04381bec5b62ab41f7ffce 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 6cfa7eeb902ebe7825ef16ee9195758d4e94b95a..a57e1ba13aac88f26cf47bfcfd600d001a3dc145 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 fbe81882a75e7a80ad4663111a2b0f65a3bf961f..94f808bb0785e13b046cdf5782a897be7922f12e 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 c54b9a16c13469967440b35ad8cf205a05b2f5a5..48cda981845e878049bee4ed7bc9f88eca0b81b6 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 c6b148fbd7675f8e0aede49b4a04fdf4e109f266..bd5ec9c8e1cb5d5139b999de55e2b4dd18039b7d 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 fb4f2bb9e6c500c7d1ecfb2b38884cbae1be5bef..f998faffbfe9fef48fc21bbfc567f5fbcc3098d4 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 8af34840e7bb00caa54866a7b95b4dd4125b7b44..fd6c7fe1fd1aa799206fa49a095a2e275059311d 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 5963d1fda0d5ca5435034fc36bdffecf7a7a70b9..24da0326e3908e5d328b5ef360a43505011d76bc 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 37c43b749116ed53935b1bad6dfc6641e02fb54b..74fca56e61a741fae22407c8ea4b03329a4f8649 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 be72c96ce2c14c67b8ac0bf1c21c4b43b0e1081e..92ab109e18a0e07db0a0f4b3eb68dcea857a8fd7 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 a7b34a72d752fc097b30b33c71039bdb6a138992..406e18680754650bdc71a4fff1feaa2b605c82c6 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 554e3ea3ae441b17d7a54670c444a9b3a22fbe4f..9a0a667b73c6c454a27e68a43a38397368e43589 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 e46956041b781d2d353e581b090f03bba8b31c04..bc8a7788b1c6fdc0b969622ad1b53e39fb7a9114 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");