Simplify Tomcat che.sh bootsrap script #2786

TylerJewell · 2016-10-14T16:32:50Z

What does this PR do?

Contains a rewrite for the native Tomcat scripts for starting and stopping Che to incorporate a number of improvements requested by users, and to remove a number of Docker-launch capabilities that are no longer needed as we provide che-server and che-launcher containers, which are more commonly used.

The improvements targeted for this pull request:

Get rid of all --<command> command line options and replace them with environment variables.
Move any bootstrap code that is specific to a container into che-server container.
Improve the che-server container to handle container-specific logic, such as che-lib.
Simplify the che-server container to only require a single /data mount folder.
Make improvements to che-launcher container to adapt to these improvements. Located in che-dockerfiles.
Support SIGTERM and SIGINT signals, so that docker stop che handled proper shutdown.

In order to deliver on this capability, we now have the Dockerfile entrypoint be start.sh which is then responsible for calling /home/user/che/bin/che.sh with the right command. We launch the command in the background which is necessary for JVM programs to receive a SIGTERM. This will also allow us to intercept the CHE_ASSEMBLY and handle the logic there along with some nice checking of the path to see if the path is valid for usage.

As part of this change, we are now only supporting a single volume mount of /data. Everything that is user configurable and / or needs to be saved in between multiple runs is now saved in this directory and volume mounted. In the basic tests - the following worked. We do an inspection in the che-server container to find the host volume that is mounted to /data. We then take that value and update the default che.properties to replace five fields with this location.

docker run -p 8080:8080 --name che --rm -v /var/run/docker.sock:/var/run/docker.sock -v /C/tmp:/data codenvy/che-server:nightly

# Fields modified in che.properties by the startup script to write in the proper /data location:
che.workspace.storage=${CHE_DATA_HOST}/workspaces
che.conf.storage=/data/storage
machine.server.ext.archive=${CHE_DATA_HOST}/lib/ws-agent.tar.gz
machine.server.terminal.path_to_archive.linux_amd64=${CHE_DATA_HOST}/lib/linux_amd64/terminal
machine.server.terminal.path_to_archive.linux_arm7=${CHE_DATA_HOST}/lib/linux_arm7/terminal
# where CHE_DATA_HOST is derived as the volume mount of /data inside the container

Shadow documentation:
https://eclipse-che.readme.io/v5.0/docs/usage-docker-server-1

Build

This image is not yet hosted on Docker Hub. You need to build it locally.

git clone http://github.com/eclipse/che
cd che
git checkout che-startup-tcomat
cd assembly/assembly-main
mvn clean install
cd ../..
docker build -t codenvy/che-server:nightly .

What issues does this PR fix or reference?

#2754 (open) - che.sh not checking for /var/run/docker.sock, & do DOCKER_HOST checks.
#2755 (open) - Usability improvements to che-server execution
#2771 - mapping

Previous behavior

The existing native script has a mix of environment variables and command line options which are not consistently applied. Also, the current bootstrap script will attempt to launch Docker using docker-machine if it is not present. This is no longer required as the only users of the Tomcat bootstrap script are those that understand how to configure Docker properly.

New behavior

*1. Have the trap shut down che
*2. Remove -v, -m, -a options.
*3. Remove launch_docker_vm from che.sh?
*4. Remove print_client_connect method
*5. Convert all parameters to inherit from env variables & CHE_ values
*6. Add check for any parameters with nice output for codenvy/che-server run
*7. -p:port ==> CHE_PORT
*8. -l:level ==> CHE_LOG_LEVEL (check CLI version)
*9. Show help if no parameters
*9. -r:ip ==> CHE_IP environment variable
*10. Check che-launcher for how debug is handled
*11. Add CHE_BLOCKING_ENTROPY
*12. Add CHE_LAUNCH_DOCKER_REGISTRY
*13. Add CHE_SKIP_JAVA_VERSION_CHECK
*14. Add CHE_SKIP_DOCKER_UID_ENFORCEMENT
*15. Remove USE_DOCKER, add check into early compatibility check
*16. Remove CHE_DOCKER_TAG
*17. Move help to better section - it's in the core body now
*18. Formalize CHE_SERVER_ACTION
*19. Add CHE_IN_CONTAINER=true to replace COPY_LIB
*20. Remove unnecessary Windows command output
*22. Auto detect if the container is in a VM and set necessary properties including
CHE_DOCKER_MACHINE_HOST_EXTERNAL=${HOSTNAME}
CHE_WORKSPACE_STORAGE="${CHE_DATA_HOST}/workspaces"
CHE_WORKSPACE_STORAGE_CREATE_FOLDERS=false

1. Rename CHE_LOCAL_BINARY to just simply CHE_ASSEMBLY
1. Refactor certain che-launcher commands to make use of CHE_HOST value instead of --remote property.

codenvy-ci · 2016-10-14T17:16:53Z

Build # 704 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/704/ to view the results.

codenvy-ci · 2016-10-14T18:19:36Z

Build # 705 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/705/ to view the results.

codenvy-ci · 2016-10-14T20:05:13Z

Build # 706 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/706/ to view the results.

codenvy-ci · 2016-10-14T22:34:39Z

Build # 707 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/707/ to view the results.

codenvy-ci · 2016-10-15T01:05:18Z

Build # 708 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/708/ to view the results.

codenvy-ci · 2016-10-15T18:06:39Z

Build # 709 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/709/ to view the results.

codenvy-ci · 2016-10-15T20:20:15Z

Build # 710 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/710/ to view the results.

codenvy-ci · 2016-10-15T21:25:23Z

Build # 711 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/711/ to view the results.

benoitf · 2016-10-16T13:52:31Z

Dockerfile

-ENTRYPOINT [ "/home/user/che/bin/che.sh", "-c" ]
-
-CMD [ "run" ]
+ENTRYPOINT ["/home/user/che/bin/start.sh"]


missing EOL

benoitf · 2016-10-16T14:11:02Z

assembly/assembly-main/src/assembly/bin/che.sh

-  # Short circuit
-  JUMP_TO_END=false
+determine_os () {
+  # Set OS.  Mac & Windows require VirtualBox and docker-machine.


Mac And windows could use as well docker 4 mac, docker 4 windows ?

It was a bad comment - leftover. Removed it.

benoitf · 2016-10-16T14:28:47Z

assembly/assembly-main/pom.xml

@@ -83,6 +83,17 @@
    </dependencies>
    <build>
        <plugins>
+              <plugin>


We can get rid of this plugin definition by adding a newline in start.sh script after the header

# <--insert newline there pid=0

Yes, good catch - that explains the issue.

benoitf · 2016-10-16T14:30:51Z

assembly/assembly-main/src/assembly/bin/che.sh

-        error_exit "This Linux user is not in 'docker' group. `
-                   `See https://docs.docker.com/engine/installation/ubuntulinux/#create-a-docker-group"
+        error "This Linux user is not in 'docker' group. `
+              `See https://docs.docker.com/engine/installation/ubuntulinux/#create-a-docker-group"


working link seems to be https://docs.docker.com/engine/installation/linux/ubuntulinux/#/create-a-docker-group but it's kind specific to ubuntu
for example for debian we have https://docs.docker.com/engine/installation/linux/debian/#/giving-non-root-access

This entire check was removed - as we no longer need to perform these checks.

benoitf · 2016-10-16T14:42:43Z

assembly/assembly-main/src/assembly/bin/che.sh

+#  if [ "${CHE_IN_CONTAINER}" == "true" ]; then
+
+    # Make sure the user named "user" is the owner of the CHE_HOME directory.
+    # Make sure the user named "user" is the owner of the CHE_DATA directory.


maybe say CHE_HOME and CHE_DATA directories instead of 2 lines

This is removed in che.sh and it's been added into docker.sh with an improved message.

benoitf · 2016-10-16T14:47:05Z

assembly/assembly-main/src/assembly/bin/start.sh

@@ -0,0 +1,235 @@
+#!/bin/bash


do we really need bash ? or sh should be enough ?

Yes, correct - downgraded it to sh

benoitf

I've tested on ubuntu box and it worked fine

On Mac I wasn't able to run native che with docker4mac but it is already the case now, so :-)

codenvy-ci · 2016-10-16T17:15:23Z

Build # 715 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/715/ to view the results.

l0rd · 2016-10-19T14:18:57Z

assembly/assembly-main/src/assembly/bin/che.sh

-    launch_docker_registry
+execute_che () { 
+  check_docker
+  determine_os


check_doker and determine_os won't work when running in a non privileged container on Kubernetes or OpenShift

We will have to manage that in another PR, I believe. The scope of the current scirpt is related to the assumption that you are running the container on a host without privileged mode and /var/run/docker.sock activated.

We have two scenarios that we have to address with the che-server container, and the underlying che.sh Tomcat script:
1: - if it is a non-privileged container, then we need to rewrite certain functions - or pass in environment values from the container orchestrator.
2: - we have customers that are going to require Che in localhost mode - without the use of any Docker capabilities. This will need to come a bit later this year. But we make a large number of assumptions about Docker in the current scripts which would not be necessary and must be removed as well.

Ok I see your point: some customers still need to run che-server in localhost mode, without Docker, and they deserve a "launcher" too. And I completely agree. But why not expanding the launcher itself to support this scenario? Or a new no-docker-launcher script that would reuse launcher_funcs.sh.

Current PR has a couple of drawbacks:

Code duplication (launcher already does a lot of these checks)

Incompatibility with kubernetes/openshift

And I'm for a complete rewrite of che.sh to remove all the checks that are already handled by the launcher(s).

TylerJewell · 2016-10-24T16:44:27Z

I agree with some of these points but not all of them. I will get time this week to articulate better the separation of concerns.

I don't think the goal is necessarily avoidance of duplication. However I think the separation of concerns has to be driven by the desired customer use case and we should better articulate the purpose of each entry point.

bmicklea · 2016-10-25T15:00:42Z

@skabashnyuk please plan to have this merged early in M7

Simplify Tomcat che.sh bootsrap script

TylerJewell added kind/enhancement A feature request - must adhere to the feature request template. status/in-progress This issue has been taken by an engineer and is under active development. team/pm labels Oct 14, 2016

TylerJewell self-assigned this Oct 14, 2016

TylerJewell mentioned this pull request Oct 14, 2016

Usability improvements to che-server container model #2755

Closed

4 tasks

TylerJewell mentioned this pull request Oct 14, 2016

Che launcher: not clear error message #2777

Closed

TylerJewell mentioned this pull request Oct 16, 2016

Question: Running Che on Mesos #2536

Closed

benoitf reviewed Oct 16, 2016

View reviewed changes

TylerJewell mentioned this pull request Oct 16, 2016

Handle DOCKER_HOST better #2754

Closed

benoitf approved these changes Oct 16, 2016

View reviewed changes

This was referenced Oct 17, 2016

Persisting workspaces on Dockerized Che #2741

Closed

Remove redundant CHE_HOME env #2817

Closed

TylerJewell changed the title ~~WIP: :Simplify Tomcat che.sh bootsrap script~~ Simplify Tomcat che.sh bootsrap script Oct 19, 2016

TylerJewell mentioned this pull request Oct 19, 2016

CHE-2015 Rename configuration properties #2707

Merged

l0rd mentioned this pull request Oct 19, 2016

Build che server Docker image with CentOS base image #2643

Merged

11 tasks

l0rd reviewed Oct 19, 2016

View reviewed changes

bmicklea added this to the 5.0.0-M7 milestone Oct 25, 2016

skabashnyuk approved these changes Oct 27, 2016

View reviewed changes

Simplify Tomcat che.sh bootsrap script

cc8ba3b

mkuznyetsov force-pushed the che-startup-tcomat branch from 82b9a21 to cc8ba3b Compare October 27, 2016 14:42

mkuznyetsov merged commit e3670d5 into master Oct 27, 2016

mkuznyetsov deleted the che-startup-tcomat branch October 27, 2016 14:42

bmicklea mentioned this pull request Nov 16, 2016

5.0.0-M7 Milestone Plan #2935

Closed

66 tasks

JPinkney pushed a commit to JPinkney/che that referenced this pull request Aug 17, 2017

Merge pull request eclipse-che#2786 from eclipse/che-startup-tcomat

d12b000

Simplify Tomcat che.sh bootsrap script

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify Tomcat che.sh bootsrap script #2786

Simplify Tomcat che.sh bootsrap script #2786

TylerJewell commented Oct 14, 2016 •

edited

Loading

codenvy-ci commented Oct 14, 2016

codenvy-ci commented Oct 14, 2016

codenvy-ci commented Oct 14, 2016

codenvy-ci commented Oct 14, 2016

codenvy-ci commented Oct 15, 2016

codenvy-ci commented Oct 15, 2016

codenvy-ci commented Oct 15, 2016

codenvy-ci commented Oct 15, 2016

benoitf Oct 16, 2016

TylerJewell Oct 16, 2016

benoitf Oct 16, 2016

TylerJewell Oct 16, 2016

benoitf Oct 16, 2016

TylerJewell Oct 16, 2016

benoitf Oct 16, 2016

TylerJewell Oct 16, 2016

benoitf Oct 16, 2016

TylerJewell Oct 16, 2016

benoitf Oct 16, 2016

TylerJewell Oct 16, 2016

benoitf left a comment

codenvy-ci commented Oct 16, 2016

l0rd Oct 19, 2016

TylerJewell Oct 21, 2016

l0rd Oct 21, 2016

TylerJewell commented Oct 24, 2016

bmicklea commented Oct 25, 2016

Simplify Tomcat che.sh bootsrap script #2786

Simplify Tomcat che.sh bootsrap script #2786

Conversation

TylerJewell commented Oct 14, 2016 • edited Loading

What does this PR do?

Build

What issues does this PR fix or reference?

Previous behavior

New behavior

codenvy-ci commented Oct 14, 2016

codenvy-ci commented Oct 14, 2016

codenvy-ci commented Oct 14, 2016

codenvy-ci commented Oct 14, 2016

codenvy-ci commented Oct 15, 2016

codenvy-ci commented Oct 15, 2016

codenvy-ci commented Oct 15, 2016

codenvy-ci commented Oct 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benoitf left a comment

Choose a reason for hiding this comment

codenvy-ci commented Oct 16, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TylerJewell commented Oct 24, 2016

bmicklea commented Oct 25, 2016

TylerJewell commented Oct 14, 2016 •

edited

Loading