Improvements to remove docker-in-docker for che running as image #257

TylerJewell · 2016-02-02T17:52:46Z

Do a docker pull before doing docker run to get latest image.
Launch docker image by volume mounting directories to avoid dind.
Update the docker exec that launches che to copy ws-agent and terminal to right location.

garagatyi · 2016-02-03T07:58:43Z

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

+  echo -e "Cleaning up any zombie containers named ${GREEN}che${NC}..."
+
+  # Force remove any lingering zombie containers with the name che
+  "${DOCKER}" rm -f che &> /dev/null || true


Let's name container eclipseche to avoid possible collisions.

What are the possible collisions?

when some other product uses name che.

I have added a new command line option --container:name which will set the name of the container. The default is che. But it can be overridden.

1. Added CONTAINER variable & command line option for setting name of docker container to use. 2. Refactored numerous strings for simpler reading and reduce duplication. 3. Added in SKIP_JAVA_VERSION check command line option 4. Improved formatting of error output.

TylerJewell · 2016-02-04T20:50:35Z

@garagatyi Numerous improvements made. Please do another review.

danpolanco · 2016-02-05T01:05:20Z

--priveleged will no longer be required?

TylerJewell · 2016-02-05T01:06:49Z

Yes correct. I can post what the new docker command looks like if you want.

danpolanco · 2016-02-05T01:10:05Z

I should be able to figure it out, otherwise I'll let you know.

What's neat is I should be able to get it running on carina 💃

TylerJewell · 2016-02-05T01:17:24Z

There is one significant issue with this new approach. If you run this che container on vbox on Windows, it is able to successfully start and shutdown. But for some reason if you restart the same container, then the vbox VM will peg the CPU and not complete the boot of the Che server. It then requires the VM to be destroyed before the che container can start again. We are researching the root cause, but we think it's a vbox bug.

danpolanco · 2016-02-05T01:39:46Z

I'll take you up on seeing the new docker command now :)

TylerJewell · 2016-02-05T01:43:08Z

This is the quickest way to experience the new container that doesn't use dind.

docker run -ti --net=host -e DOCKER_MACHINE_HOST=<ip-of-host-of-docker> -v /var/run/docker.sock:/var/run/docker.sock -v /home/user/che/lib:/home/user/che/lib-copy -v /home/user/che/workspaces:/home/user/che/workspaces -v /home/user/che/tomcat/temp/local-storage:/home/user/che/tomcat/temp/local-storage codenvy/che:no_dnd

danpolanco · 2016-02-05T02:12:23Z

Awesome! Leaving this hear in case anyone else is curious... (I haven't tested to make sure everything works as expected yet)

editor_container:
  image: "codenvy/che:no_dnd"
  restart: "always"
  environment:
    - "VIRTUAL_HOST=sub.yourdomain.org"
    - "VIRTUAL_PORT=8080"
    - "TZ=America/Mountains"
  volumes:
    - "/var/run/docker.sock:/var/run/docker.sock"
  volumes_from:
    - "editor_data"

editor_data:
  image: "codenvy/che:no_dnd"
  environment:
    - "TZ=America/Mountains"
  volumes:
    - "/home/user/che/lib-copy"
    - "/home/user/che/workspaces"
    - "/home/user/che/tomcat/temp/local-storage"
  entrypoint: "true"

Should close #238.

TylerJewell · 2016-02-05T04:06:22Z

thanks for the contribution @DanTheColoradan. May we post this suggestion on the docs when we update our Docker page with the new instructions?

garagatyi · 2016-02-05T07:35:32Z

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

+  if [[ "${SKIP_JAVA_VERSION}" == false ]]; then
+    # Che requires Java version 1.8 or higher.
+	JAVA_VERSION=$("${JAVA_HOME}/bin/java" -version 2>&1 | awk -F '"' '/version/ {print $2}')
+	if [[  -z "${JAVA_VERSION}" || "${JAVA_VERSION}" < "1.8" ]]; then


Please fix indentation

garagatyi · 2016-02-05T07:48:27Z

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

+	if ${USE_DEBUG}; then
+	  set +x
+	fi
+	return


it is useless

Necessary to avoid flawed output if script execution is interrupted

garagatyi · 2016-02-05T07:52:41Z

@TylerJewell It is nice that you changed functions declaration syntax but it won't help to fix #262.
I believe that only one thing that we can do is disable POSIX compliant mode in Che script. If we won't do that we have to fix dozens of operations in the script to fix that issue.

danpolanco · 2016-02-05T17:03:39Z

What permissions is /home/user/che supposed to have? It looks like they are being set back to root somehow?

danpolanco · 2016-02-05T17:06:57Z

Nvm.

garagatyi · 2016-02-08T09:09:43Z

ok for me. @skabashnyuk @vparfonov WDYT?

TylerJewell · 2016-02-08T17:02:23Z

Hold on any merge. We have a new commit that is coming that will need re-testing.

Made numerous improvements. We now have the ability to start / stop repeatedly a Che server running in an image without corruption. Finally!

TylerJewell · 2016-02-08T17:37:52Z

@garagatyi @skabashnyuk @vparfonov I am ready to merge this. Made numerous improvements and now Che in a docker image is stable even after many stop / restart cycles.

garagatyi · 2016-02-09T08:22:22Z

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

+                                            `"mkdir -p /home/user/che/lib-copy/ && "`
+                                            `"sudo chown -R user:user /home/user && "`
+                                            `"cp -rf /home/user/che/lib/* /home/user/che/lib-copy && "`
+                                            `"sudo sed -i 's/random/urandom/g' /opt/jre1.8.0_65/lib/security/java.security && "`


Please explaine why you added this sed.

http://stackoverfklow.com/questions/26431922/tomcat7-starts-too-late-on-ubuntu-14-04-x64-digitalocean

Looks like this change decreses security of random generator. @skabashnyuk WDYT is it ok to accept that change?

ghost · 2016-02-09T17:57:14Z

I think this one isn't necessary - http://stackoverfklow.com/questions/26431922/tomcat7-starts-too-late-on-ubuntu-14-04-x64-digitalocean however, it is harmless in terms of affecting Che. It is just an exec. I thought this might be the reason why container was unresponsive.

1. Will only stop the che server if che is in a docker container. 2. Added --stop-container option which will also stop docker container running che 3. Removed unnecessary sed command in che start

TylerJewell · 2016-02-10T18:32:48Z

@garagatyi Ready to merge. Last updates made.

skabashnyuk · 2016-02-10T22:21:28Z

ok

Improvements to remove docker-in-docker for che running as image

Change how docker is launched

bff5685

This was referenced Feb 2, 2016

Don't use Docker-in-Docker when running as a Docker Container #244

Closed

Passing argument to docker image #170

Closed

garagatyi reviewed Feb 3, 2016
View reviewed changes

This was referenced Feb 4, 2016

Eclipse Che che.sh script not compatible with bash #262

Closed

Too many open files with codenvy/che:nightly #143

Closed

danpolanco mentioned this pull request Feb 5, 2016

Documentation for running Che using docker-compose + potentially nginx-proxy & on carina #269

Closed

garagatyi reviewed Feb 5, 2016
View reviewed changes

Add JAVA_VERSION to output, add posix check

7b32af0

TylerJewell mentioned this pull request Feb 8, 2016

Can't restart workspace after stopping it #286

Closed

Tyler Jewell added 2 commits February 8, 2016 09:03

tests for new docker type

b13adf0

Updated to work with boot2docker v1.10

26f8fe0

Made numerous improvements. We now have the ability to start / stop repeatedly a Che server running in an image without corruption. Finally!

TylerJewell mentioned this pull request Feb 8, 2016

What is the right way to stop che? #287

Closed

garagatyi reviewed Feb 9, 2016
View reviewed changes

This was referenced Feb 10, 2016

Che will not start when JAVA_HOME does not match "java -version" on Windows #295

Closed

Launching Che with docker as explained in documentation doesn't work #297

Closed

Add --stop-container option

9a55efb

1. Will only stop the che server if che is in a docker container. 2. Added --stop-container option which will also stop docker container running che 3. Removed unnecessary sed command in che start

ghost mentioned this pull request Feb 10, 2016

Creating workspace with che running inside a docker container fails with certificates https error #300

Closed

TylerJewell pushed a commit that referenced this pull request Feb 10, 2016

Merge pull request #257 from codenvy/idex-4266

abe68ed

Improvements to remove docker-in-docker for che running as image

TylerJewell merged commit abe68ed into master Feb 10, 2016

skabashnyuk deleted the idex-4266 branch February 11, 2016 12:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvements to remove docker-in-docker for che running as image #257

Improvements to remove docker-in-docker for che running as image #257

TylerJewell commented Feb 2, 2016

garagatyi Feb 3, 2016

TylerJewell Feb 4, 2016

garagatyi Feb 4, 2016

TylerJewell Feb 4, 2016

TylerJewell commented Feb 4, 2016

danpolanco commented Feb 5, 2016

TylerJewell commented Feb 5, 2016

danpolanco commented Feb 5, 2016

TylerJewell commented Feb 5, 2016

danpolanco commented Feb 5, 2016

TylerJewell commented Feb 5, 2016

danpolanco commented Feb 5, 2016

TylerJewell commented Feb 5, 2016

garagatyi Feb 5, 2016

garagatyi Feb 5, 2016

TylerJewell Feb 5, 2016

garagatyi Feb 5, 2016

garagatyi commented Feb 5, 2016

danpolanco commented Feb 5, 2016

danpolanco commented Feb 5, 2016

garagatyi commented Feb 8, 2016

TylerJewell commented Feb 8, 2016

TylerJewell commented Feb 8, 2016

garagatyi Feb 9, 2016

TylerJewell Feb 9, 2016

garagatyi Feb 9, 2016

ghost commented Feb 9, 2016

TylerJewell commented Feb 10, 2016

skabashnyuk commented Feb 10, 2016

Improvements to remove docker-in-docker for che running as image #257

Improvements to remove docker-in-docker for che running as image #257

Conversation

TylerJewell commented Feb 2, 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

TylerJewell commented Feb 4, 2016

danpolanco commented Feb 5, 2016

TylerJewell commented Feb 5, 2016

danpolanco commented Feb 5, 2016

TylerJewell commented Feb 5, 2016

danpolanco commented Feb 5, 2016

TylerJewell commented Feb 5, 2016

danpolanco commented Feb 5, 2016

TylerJewell commented Feb 5, 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

garagatyi commented Feb 5, 2016

danpolanco commented Feb 5, 2016

danpolanco commented Feb 5, 2016

garagatyi commented Feb 8, 2016

TylerJewell commented Feb 8, 2016

TylerJewell commented Feb 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Feb 9, 2016

TylerJewell commented Feb 10, 2016

skabashnyuk commented Feb 10, 2016