From aa7cacf9c99a275796ba02ec14efac5390037695 Mon Sep 17 00:00:00 2001 From: Vitalii Parfonov Date: Mon, 20 Jul 2020 18:19:11 +0300 Subject: [PATCH 1/4] Various fixes after meging PR #16972 Signed-off-by: Vitalii Parfonov --- .../openshift/AsyncStorageModeValidator.java | 19 ++++++++++++------- .../provision/AsyncStorageProvisioner.java | 14 +++++++++----- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/AsyncStorageModeValidator.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/AsyncStorageModeValidator.java index a0cff2c0401..f7461ec1b57 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/AsyncStorageModeValidator.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/AsyncStorageModeValidator.java @@ -11,13 +11,13 @@ */ package org.eclipse.che.workspace.infrastructure.openshift; +import static com.google.common.base.Strings.isNullOrEmpty; import static java.lang.Boolean.parseBoolean; import static java.lang.String.format; import static org.eclipse.che.api.workspace.shared.Constants.ASYNC_PERSIST_ATTRIBUTE; import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.CommonPVCStrategy.COMMON_STRATEGY; import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.EphemeralWorkspaceUtility.isEphemeral; -import com.google.common.base.Strings; import java.util.Map; import javax.inject.Inject; import javax.inject.Named; @@ -57,8 +57,10 @@ public class AsyncStorageModeValidator implements WorkspaceAttributeValidator { private final String pvcStrategy; private final boolean allowUserDefinedNamespaces; - private final String defaultNamespaceName; private final int runtimesPerUser; + private final boolean isNamespaceStrategyNotValid; + private final boolean isPvcStrategyNotValid; + private final boolean runtimesPerUserLimited; @Inject public AsyncStorageModeValidator( @@ -70,8 +72,12 @@ public AsyncStorageModeValidator( this.pvcStrategy = pvcStrategy; this.allowUserDefinedNamespaces = allowUserDefinedNamespaces; - this.defaultNamespaceName = defaultNamespaceName; this.runtimesPerUser = runtimesPerUser; + + this.isPvcStrategyNotValid = !COMMON_STRATEGY.equals(pvcStrategy); + this.runtimesPerUserLimited = runtimesPerUser > 1; + this.isNamespaceStrategyNotValid = + isNullOrEmpty(defaultNamespaceName) || !defaultNamespaceName.contains(""); } @Override @@ -114,7 +120,7 @@ private void isEphemeralAttributeValidation(Map attributes) } private void runtimesPerUserValidation() throws ValidationException { - if (runtimesPerUser > 1) { + if (runtimesPerUserLimited) { String message = format( "Workspace configuration not valid: Asynchronous storage available only if 'che.limits.user.workspaces.run.count' set to 1, but got %s", @@ -125,8 +131,7 @@ private void runtimesPerUserValidation() throws ValidationException { } private void nameSpaceStrategyValidation() throws ValidationException { - if (Strings.isNullOrEmpty(defaultNamespaceName) - || !defaultNamespaceName.contains("")) { + if (isNamespaceStrategyNotValid) { String message = "Workspace configuration not valid: Asynchronous storage available only for 'per-user' namespace strategy"; LOG.warn(message); @@ -146,7 +151,7 @@ private void alowUserDefinedNamespaceValidation() throws ValidationException { } private void pvcStrategyValidation() throws ValidationException { - if (!COMMON_STRATEGY.equals(pvcStrategy)) { + if (isPvcStrategyNotValid) { String message = format( "Workspace configuration not valid: Asynchronous storage available only for 'common' PVC strategy, but got %s", diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/provision/AsyncStorageProvisioner.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/provision/AsyncStorageProvisioner.java index 6af22f963d9..e1801f57b40 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/provision/AsyncStorageProvisioner.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/provision/AsyncStorageProvisioner.java @@ -69,6 +69,7 @@ import org.eclipse.che.api.workspace.server.spi.InfrastructureException; import org.eclipse.che.workspace.infrastructure.kubernetes.Names; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesObjectUtil; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.ServerServiceBuilder; import org.eclipse.che.workspace.infrastructure.openshift.OpenShiftClientFactory; import org.eclipse.che.workspace.infrastructure.openshift.environment.OpenShiftEnvironment; import org.slf4j.Logger; @@ -369,15 +370,18 @@ private void createStorageServiceIfNotExist( .withPort(SERVICE_PORT) .withTargetPort(targetPort) .build(); + ServiceSpec spec = new ServiceSpec(); spec.setPorts(singletonList(port)); spec.setSelector(of("app", ASYNC_STORAGE)); - Service service = new Service(); - service.setApiVersion("v1"); - service.setKind("Service"); - service.setMetadata(meta); - service.setSpec(spec); + ServerServiceBuilder serviceBuilder = new ServerServiceBuilder(); + Service service = + serviceBuilder + .withPorts(singletonList(port)) + .withSelectorEntry("app", ASYNC_STORAGE) + .withName(ASYNC_STORAGE) + .build(); oc.services().inNamespace(namespace).create(service); } From d23914d93d6b11f12072397ba16dd870185cea93 Mon Sep 17 00:00:00 2001 From: Vitalii Parfonov Date: Mon, 20 Jul 2020 22:41:44 +0300 Subject: [PATCH 2/4] Add some comments to che.properties about limitation for async storage Signed-off-by: Vitalii Parfonov --- .../src/main/webapp/WEB-INF/classes/che/che.properties | 6 ++++++ 1 file changed, 6 insertions(+) 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 125c9ce68e2..185b9dd1d81 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 @@ -533,6 +533,12 @@ che.workspace.devfile_registry_url=https://che-devfile-registry.prod-preview.ope # - 'async': Experimental feature: Asynchronous storage is combination of Ephemeral # and Persistent storage. Allows for faster I/O and keep your changes, will backup on stop # and restore on start workspace. +# Will work only if: +# - che.infra.kubernetes.pvc.strategy='common' +# - che.limits.user.workspaces.run.count=1 +# - che.infra.kubernetes.namespace.allow_user_defined=false +# - che.infra.kubernetes.namespace.default contains +# in other way remove 'async'. che.workspace.storage.available_types=persistent,ephemeral,async # The configuration property that defines a default value for storage type that clients like From e0ff8fb0b583963a0d51b78af3021b323b05b39a Mon Sep 17 00:00:00 2001 From: Vitalii Parfonov Date: Tue, 21 Jul 2020 10:25:47 +0300 Subject: [PATCH 3/4] rephrase --- .../src/main/webapp/WEB-INF/classes/che/che.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 185b9dd1d81..1c5b2bbe99b 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 @@ -538,7 +538,7 @@ che.workspace.devfile_registry_url=https://che-devfile-registry.prod-preview.ope # - che.limits.user.workspaces.run.count=1 # - che.infra.kubernetes.namespace.allow_user_defined=false # - che.infra.kubernetes.namespace.default contains -# in other way remove 'async'. +# in other cases remove 'async' from the list. che.workspace.storage.available_types=persistent,ephemeral,async # The configuration property that defines a default value for storage type that clients like From e3d058262907015698f476b9b3b4eef28dd97b26 Mon Sep 17 00:00:00 2001 From: Vitalii Parfonov Date: Tue, 21 Jul 2020 11:05:55 +0300 Subject: [PATCH 4/4] Improving condition for che.limits.user.workspaces.run.count checking Signed-off-by: Vitalii Parfonov --- .../infrastructure/openshift/AsyncStorageModeValidator.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/AsyncStorageModeValidator.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/AsyncStorageModeValidator.java index f7461ec1b57..03699a5b53a 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/AsyncStorageModeValidator.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/AsyncStorageModeValidator.java @@ -60,7 +60,7 @@ public class AsyncStorageModeValidator implements WorkspaceAttributeValidator { private final int runtimesPerUser; private final boolean isNamespaceStrategyNotValid; private final boolean isPvcStrategyNotValid; - private final boolean runtimesPerUserLimited; + private final boolean singleRuntimeAllowed; @Inject public AsyncStorageModeValidator( @@ -75,7 +75,7 @@ public AsyncStorageModeValidator( this.runtimesPerUser = runtimesPerUser; this.isPvcStrategyNotValid = !COMMON_STRATEGY.equals(pvcStrategy); - this.runtimesPerUserLimited = runtimesPerUser > 1; + this.singleRuntimeAllowed = runtimesPerUser == 1; this.isNamespaceStrategyNotValid = isNullOrEmpty(defaultNamespaceName) || !defaultNamespaceName.contains(""); } @@ -120,7 +120,7 @@ private void isEphemeralAttributeValidation(Map attributes) } private void runtimesPerUserValidation() throws ValidationException { - if (runtimesPerUserLimited) { + if (!singleRuntimeAllowed) { String message = format( "Workspace configuration not valid: Asynchronous storage available only if 'che.limits.user.workspaces.run.count' set to 1, but got %s",