Skip to content

Commit

Permalink
Openshift connector improvements (#5052)
Browse files Browse the repository at this point in the history
* CHE-4141 - Use Persistent Volumes Claims when creating workspaces

Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>

* Implement getContainerLogs method in OpenShiftConnector

Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>

* Implement createExec() and startExec() in OpenShiftConnector

Add implementations of createExec() and startExec(). Since OpenShift
does not separate the create and start steps, a holder class
KubernetesExecHolder is necessary, to pass information between
the call to createExec() (which just saves relevant information)
and startExec().

Additionally, adds KubernetesOutputAdapter, which parses the output
from OpenShift into LogMessages that can be handled by Che's
MessageProcessor<LogMessage> class.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>

* Add implementation of getEvents() to avoid busy wait

Signed-off-by: Angel Misevski <amisevsk@redhat.com>

* Update Dockerfile to avoid permissions issues

Signed-off-by: Mario Loriedo <mloriedo@redhat.com>

* Che server and workpaces exposed on the same single TCP port (#4351)

Signed-off-by: Mario Loriedo <mloriedo@redhat.com>

* Disabling usage of user account service in openshift-connector

Signed-off-by: Sun Seng David Tan <sutan@redhat.com>

* Update Docker Compose tests to fix test failure

Updating to Jackson 2.7.7 causes tests in the docker compose
plugin to fail. This is due to the fact that the tests expect
empty values in dictionaries to be parsed as the empty string,
whereas jackson 2.7.7 parses them as null (as specified by the
yaml spec).

Modifies the affected tests to explicitly use an empty string
(i.e. "") instead of an empty value.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>

* Find an alternative to subPath in volumeMount

Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>

* Setting rwx permissions for all on /data/ in case it's not mounted

Signed-off-by: Mario Loriedo <mloriedo@redhat.com>

* Add support for resource limits when running on Openshift

Add resource limits to workspace Pods when running on OpenShift.
The memory limit is normally obtained from the API request to
create the workspace, however it can be overridden via the property
`che.openshift.workspace.memory.override`. The cpu limit used is
determined by the property `che.openshift.workspace.cpu.limit`.

In both cases, the value of the property is passed directly to
OpenShift, so any valid quantity is acceptable (e.g. 150Mi,
1Gi, 1024, etc).

Signed-off-by: Angel Misevski <amisevsk@redhat.com>

* Fix dockerImageConfig is null (since v1.5 of OpenShift API)

Signed-off-by: Sun Seng David Tan <sutan@redhat.com>

* Add Nullable annotation to che.docker.ip.external

The property che.docker.ip.external can be null, but
OpenShiftConnector does not include the annotation. This
prevents Che from initialising if e.g. running on docker
without the property set.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>

* CHE-158 Adding TLS support for Workspace routes

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>

* Adding property to set requests for RAM

Signed-off-by: Mario Loriedo <mloriedo@redhat.com>

* CHE-158 Using '-' instead of '.' for generating OpenShift route Urls

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>

* Fixing tests after changing Url generation logic

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>

* Redirect insecure HTTP requests to TLS endpoint

Signed-off-by: Mario Loriedo <mloriedo@redhat.com>

* CHE-180: Creating and closing OpenShiftClient in every method of OpenshiftConnector

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>

* Update route naming to make it work on OSO

Signed-off-by: Mario Loriedo <mloriedo@redhat.com>

* Rework PVC management on OpenShift

- Change how subdirectories are created in pods to
  use a short, terminating job instead of a full deployment
- Add OpenShiftWorkspaceFilesCleaner class to properly
  notice workspace deleted events
- Add helper class to manage job pods. For creation, some
  effort is made to avoid attempting to create workspaces
  unnecessarily, but only exists in-memory
- Workspace deletions are batched together so that removing
  workspaces directories can be done when server is idled,
  avoiding unnecessary PVC mounts
- Add two new properties: che.openshift.jobs.image and
  che.openshift.jobs.memorylimit, which are used by
  OpenShiftPvcHelper to set up pods

Current issues:
- Since workspace directories are not deleted immediately,
  attempting to re-create a workspace with the same name
  will result in the previous instance's project to already
  be there. This should have a minor impact.
- Memory for which workspace dirs have been created is not
  persisted, resulting in potentially unnecessary jobs
- Openshift workspace files cleaner is included by overwriting
  binding in WsMasterModule instead of using a provider. This
  could be better, but OpenShift integration may be reaching a
  point where a custom module is a better solution.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Signed-off-by: Sun Seng David Tan <sutan@redhat.com>

* Delete ReplicaSets explicitly when shutting down a workspace

Signed-off-by: Angel Misevski <amisevsk@redhat.com>

* Fix OpenShiftConnectorTest

Signed-off-by: Mario Loriedo <mloriedo@redhat.com>

* Fix route server names if unknown should start with server-.

https://issues.jboss.org/browse/CHE-230

Signed-off-by: Sun Seng David Tan <sutan@redhat.com>

* Add property to control manual workspace dir creation in OpenShift

Add property 'che.openshift.precreate.workspace.dirs'. If property is
true, OpenShiftConnector will run a pod before launching workspaces
to create a subpath in the workspace's persistent volume with correct
permissions. If the property is false, this step is skipped.

This is necessary as in older versions of OpenShift/Kubernetes, subpaths
created as part of a volume mount are created with root permissions, and
so cannot be modified by workspace pods. More recent versions fix this,
creating subpath volumes with correct permissions, making the step above
unnecessary.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>

* CHE-102 - Idle detection of che-server and workspaces

Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>

* Add and modify tests for OpenShift helper classes

Add tests for the untested classes in openshift.client.kuberentes,
and update existing tests where necessary.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>

* Recent changes required access to `/` which is impossible under OS 

Signed-off-by: David Festal <dfestal@redhat.com>

* adapt che-server entrypoint.sh to environments without write permissions in '/' (#5344)

* adapt che-server entrypoint.sh to environments without write permissions in '/'

* CHE-280: Adding container's state info to the 'inspectContainer' API

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>

* Factorize code of `ServerEvaluationStrategy` classes, to use the Custom strategy as the basis of other strategies (#5366)

* Pull-up the local docker port management (use exposed ports)

Signed-off-by: David Festal <dfestal@redhat.com>

* Make all the strategies extend `CustomEvaluationStrategy`

Signed-off-by: David Festal <dfestal@redhat.com>

* Add a `workspaceIdWithoutPrefix` macro and use it for `single-port`

This macro is based on the `workspaceId` macro, but without the
`workspace` prefix.

Signed-off-by: David Festal <dfestal@redhat.com>

* Add the `isDevMachine` to allow conditions in the ST template.

This is required to allow the `single-port` strategy to have a different
url according to the type of machine. (see the work done for CHE-175 :
Support multi-container workspaces on OpenShift)

Signed-off-by: David Festal <dfestal@redhat.com>

* Small fixes after comments from @fbenoit

Signed-off-by: David Festal <dfestal@redhat.com>

* Fix unnecessary space pointed out by @sunix

Signed-off-by: David Festal <dfestal@redhat.com>

* Remove unnecessary `else` as suggested by @sunix

Signed-off-by: David Festal <dfestal@redhat.com>

* Keep the method signatures compatible with the `condenvy` strategy

Signed-off-by: David Festal <dfestal@redhat.com>

* Align names of parameters of constructors (requested by @garagatyi)

Signed-off-by: David Festal <dfestal@redhat.com>

* Add a default implementation to avoid breaking the Codenvy build

Signed-off-by: David Festal <dfestal@redhat.com>

* Also rename the attributes

Signed-off-by: David Festal <dfestal@redhat.com>

* Use a constant for the `workspace` prefix string

Signed-off-by: David Festal <dfestal@redhat.com>

* Fix formatting as requested by @sunix

Signed-off-by: David Festal <dfestal@redhat.com>

* Use a constant for the `isDevMachine` macro name

Signed-off-by: David Festal <dfestal@redhat.com>

* Add unit tests for `workspaceIdWithoutPrefixè and `isDevMachine` macros

Signed-off-by: David Festal <dfestal@redhat.com>

* Another requested formatting fix

Signed-off-by: David Festal <dfestal@redhat.com>

* Make new tests clearer

Signed-off-by: David Festal <dfestal@redhat.com>

* yet another formatting request

Signed-off-by: David Festal <dfestal@redhat.com>

* Respect the original order of imports

Signed-off-by: David Festal <dfestal@redhat.com>

* remove unnecessary `toString()`

Signed-off-by: David Festal <dfestal@redhat.com>

* use a lowercase `S` in the `server-` prefix

Signed-off-by: David Festal <dfestal@redhat.com>

* Multi-container workspace Support (#5110)

* Fix 2 NPE that prevented using *non-dev* additional machines

In the context of https://issues.jboss.org/browse/CHE-175

Signed-off-by: David Festal <dfestal@redhat.com>

* Name openshift resources based on the machine name for non-dev machines

This fixes https://issues.jboss.org/browse/CHE-259
and https://issues.jboss.org/browse/CHE-258

Signed-off-by: David Festal <dfestal@redhat.com>

* Fix failing Traeffik tests...

... by:
- adding the new `CHE_IS_DEV_MACHINE` env variable in tests
- pulling up all the `CustomServerEvaluationStrategy` features in an
abstract `BaseServerEvaluationStrategy` (which all other Che strategies
extend) and have the `CustomServerEvaluationStrategy` class simply
extend this `BaseServerEvaluationStrategy`.

Signed-off-by: David Festal <dfestal@redhat.com>

* Fix tests in the LocalDockerEvaluationStrategy...

... by correctly using the boolean attribute to manage the new use-case
introduced by @fbenoit in master.

Signed-off-by: David Festal <dfestal@redhat.com>

* Replace OSIO-specific `single-port` strategy by `docker-local-custom` 

This fixes redhat-developer/rh-che#113

Signed-off-by: David Festal <dfestal@redhat.com>
  • Loading branch information
sunix authored Jul 4, 2017
1 parent ea372fb commit 8049320
Show file tree
Hide file tree
Showing 41 changed files with 2,959 additions and 759 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,14 @@ che.docker.ip.external=NULL
# - 'docker-local': internal address is address of container within docker network, and exposed ports
# are used.
# - 'custom': The evaluation strategy may be customized through a template property.
# - 'docker-local-custom': internal address is set as in docker-local strategy, external address is composed
# as in the custom strategy with the 'template' and the 'external.protocol' properties.

# The 'docker-local' strategy may be useful if a firewall prevents communication between che-server and
# workspace containers, but will prevent communication when che-server and workspace containers are not
# on the same Docker network.
# The 'docker-local-custom' strategy may be useful when Che and the workspace servers need to be exposed on the
# same single TCP port.
che.docker.server_evaluation_strategy=default


Expand Down Expand Up @@ -298,12 +303,52 @@ che.openshift.project=eclipse-che
che.openshift.serviceaccountname=cheserviceaccount
che.openshift.liveness.probe.delay=300
che.openshift.liveness.probe.timeout=1
che.openshift.workspaces.pvc.name=claim-che-workspace
che.openshift.workspaces.pvc.quantity=10Gi
# Create secure route against HTTPS
# NOTE: In order to create routes against HTTPS
# Property 'strategy.che.docker.server_evaluation_strategy.secure.external.urls' should be also set to true
che.openshift.secure.routes=false
# Pod that is launched when performing persistent volume claim maintenance jobs on OpenShift
che.openshift.jobs.image=centos:centos7
che.openshift.jobs.memorylimit=250Mi

# Run job to create workspace subpath directories in persistent volume before launching workspace.
# Necessary in some versions of OpenShift/Kubernetes as workspace subpath volumemounts are created
# with root permissions, and thus cannot be modified by workspaces running as user (presents as error
# importing projects into workspace in Che). Default is "true", but should be set to false if version
# of Openshift/Kubernetes creates subdirectories with user permissions.
# Relevant issue: https://github.com/kubernetes/kubernetes/issues/41638
che.openshift.precreate.workspace.dirs=true

# Specifications of compute resources that can be consumed
# by the workspace container:
#
# - Amount of memory required for a workspace container to run e.g. 512Mi
che.openshift.workspace.memory.request=NULL
#
# - Maximum amount of memory a workspace container can use e.g. 1.3Gi
che.openshift.workspace.memory.override=NULL

# The Openshift will idle the server if no workspace is run for
# this length of time.
che.openshift.server.inactive.stop.timeout.ms=1800000

#
#
# Be aware that setting che.openshift.workspace.memory.override
# will override Che memory limits
#
# More information about setting Compute Resources in OpenShift can be
# found here: https://docs.openshift.org/latest/dev_guide/compute_resources.html#dev-compute-resources

# Which implementation of DockerConnector to use in managing containers. In general,
# the base implementation of DockerConnector is appropriate, but OpenShiftConnector
# is necessary for deploying Che on OpenShift. Options:
# - 'default' : Use DockerConnector
# - 'openshift' : use OpenShiftConnector
# Note that if 'openshift' connector is used, the property che.docker.ip.external
# MUST be set.
che.docker.connector=default

# Defines whether stacks loaded once or each time server starts.
Expand Down
11 changes: 11 additions & 0 deletions core/che-core-api-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,17 @@
</execution>
</executions>
</plugin>
<plugin>
<groupId>com.mycila</groupId>
<artifactId>license-maven-plugin</artifactId>
<configuration>
<excludes>
<!-- Exclude files until #3281 is resolved -->
<exclude>**/ServerIdleEvent.java</exclude>
<!-- End excluded files -->
</excludes>
</configuration>
</plugin>
</plugins>
</build>
<profiles>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*******************************************************************************
* Copyright (c) 2012-2017 Red Hat, Inc.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v10.html
*
* Contributors:
* Red Hat, Inc. - initial API and implementation
*******************************************************************************/
package org.eclipse.che.api.core.event;
/**
* Event informing about idling the che server.
*/
public class ServerIdleEvent {
private long timeout;

/**
* Implements the handler to handle idling.
*/
public ServerIdleEvent(long timeout) {
super();
this.timeout = timeout;
}


public long getTimeout() {
return timeout;
}

public void setTimeout(long timeout) {
this.timeout = timeout;
}
}
3 changes: 3 additions & 0 deletions dockerfiles/che/Dockerfile.centos
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,6 @@ EXPOSE 8000 8080
COPY entrypoint.sh /entrypoint.sh
ENTRYPOINT ["/entrypoint.sh"]
ADD eclipse-che.tar.gz /home/user/
RUN mkdir /logs && chmod 0777 /logs
RUN chmod -R 0777 /home/user/
RUN mkdir /data && chmod 0777 /data
42 changes: 32 additions & 10 deletions dockerfiles/che/entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ Variables:
export CHE_REGISTRY_HOST=${CHE_REGISTRY_HOST:-${DEFAULT_CHE_REGISTRY_HOST}}

DEFAULT_CHE_PORT=8080
CHE_PORT=${CHE_PORT:-${DEFAULT_CHE_PORT}}
export CHE_PORT=${CHE_PORT:-${DEFAULT_CHE_PORT}}

DEFAULT_CHE_IP=
CHE_IP=${CHE_IP:-${DEFAULT_CHE_IP}}
Expand Down Expand Up @@ -246,18 +246,40 @@ init() {
fi
### Are we going to use the embedded che.properties or one provided by user?`
### CHE_LOCAL_CONF_DIR is internal Che variable that sets where to load
export CHE_LOCAL_CONF_DIR="/conf"
if [ -f "/conf/che.properties" ]; then
echo "Found custom che.properties..."
if [ "$CHE_USER" != "root" ]; then
sudo chown -R ${CHE_USER} ${CHE_LOCAL_CONF_DIR}
# check if we have permissions to create /conf folder.
if [ -w / ]; then
export CHE_LOCAL_CONF_DIR="/conf"
if [ -f "/conf/che.properties" ]; then
echo "Found custom che.properties..."
if [ "$CHE_USER" != "root" ]; then
sudo chown -R ${CHE_USER} ${CHE_LOCAL_CONF_DIR}
fi
else
if [ ! -d ${CHE_LOCAL_CONF_DIR} ]; then
mkdir -p ${CHE_LOCAL_CONF_DIR}
fi
if [ -w ${CHE_LOCAL_CONF_DIR} ];then
echo "ERROR: user ${CHE_USER} does OK have write permissions to ${CHE_LOCAL_CONF_DIR}"
echo "Using embedded che.properties... Copying template to ${CHE_LOCAL_CONF_DIR}/che.properties"
cp -rf "${CHE_HOME}/conf/che.properties" ${CHE_LOCAL_CONF_DIR}/che.properties
else
echo "ERROR: user ${CHE_USER} does not have write permissions to ${CHE_LOCAL_CONF_DIR}"
exit 1
fi
fi
else
if [ ! -d /conf ]; then
mkdir -p /conf
echo "WARN: parent dir is not writeable, CHE_LOCAL_CONF_DIR will be set to ${CHE_DATA}/conf"
export CHE_LOCAL_CONF_DIR="${CHE_DATA}/conf"
if [ ! -d ${CHE_LOCAL_CONF_DIR} ]; then
mkdir -p ${CHE_LOCAL_CONF_DIR}
fi
if [ -w ${CHE_LOCAL_CONF_DIR} ];then
echo "Using embedded che.properties... Copying template to ${CHE_LOCAL_CONF_DIR}/che.properties"
cp -rf "${CHE_HOME}/conf/che.properties" ${CHE_LOCAL_CONF_DIR}/che.properties
else
echo "ERROR: user ${CHE_USER} does not have write permissions to ${CHE_LOCAL_CONF_DIR}"
exit 1
fi
echo "Using embedded che.properties... Copying template to ${CHE_LOCAL_CONF_DIR}/che.properties"
cp -rf "${CHE_HOME}/conf/che.properties" ${CHE_LOCAL_CONF_DIR}/che.properties
fi

# Update the provided che.properties with the location of the /data mounts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class Exec {
private final String[] command;
private final String id;

Exec(String[] command, String id) {
public Exec(String[] command, String id) {
this.command = command;
this.id = id;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ public void composeServiceCommandShouldBeParsedSuccessfully(String command,
assertEquals(environment.get("MYSQL_PASSWORD"), "password");
assertTrue(service.getExpose().containsAll(asList("4403", "5502")));

assertEquals(service.getCommand(), commandWords);
assertTrue(service.getCommand().containsAll(commandWords));
assertEquals(service.getCommand().size(), commandNumberOfWords);
}

@DataProvider(name = "validCommand")
Expand Down Expand Up @@ -137,7 +138,7 @@ private Object[][] validCommand() {
{"\"echo ${PWD}\"", asList("echo", "${PWD}"), 2},
{"\"(Test)\"", singletonList("(Test)"), 1},

{"", null, 1},
{"\"\"", singletonList(""), 1},
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ public Object[][] correctContentTestData() {
+ " dev-machine: \n"
+ " image: codenvy/ubuntu_jdk8\n"
+ " environment:\n"
+ " MYSQL_ROOT_PASSWORD: ",
ImmutableMap.of("MYSQL_ROOT_PASSWORD", null)
+ " MYSQL_ROOT_PASSWORD: \"\"",
ImmutableMap.of("MYSQL_ROOT_PASSWORD", "")
},

// dictionary format, value of variable contains colon sign
Expand Down
3 changes: 3 additions & 0 deletions plugins/plugin-docker/che-plugin-docker-machine/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,11 @@
<exclude>**/DefaultServerEvaluationStrategyTest.java</exclude>
<exclude>**/LocalDockerServerEvaluationStrategy.java</exclude>
<exclude>**/LocalDockerServerEvaluationStrategyTest.java</exclude>
<exclude>**/LocalDockerCustomServerEvaluationStrategy.java</exclude>
<exclude>**/LocalDockerCustomServerEvaluationStrategyTest.java</exclude>
<exclude>**/DockerInstanceRuntimeInfo.java</exclude>
<exclude>**/DockerInstanceRuntimeInfoTest.java</exclude>
<exclude>**/ServerIdleDetector.java</exclude>
<!-- End excluded files -->
</excludes>
</configuration>
Expand Down
Loading

0 comments on commit 8049320

Please sign in to comment.