Use a docker launcher image to simplify start/stop/update/restart commands #1683

l0rd · 2016-07-07T14:36:47Z

The goal of this PR is to simplify the docker run command to start/stop/update/restart Che. To test it:

Build the launcher image: docker build -t codenvy/che:launcher -f Dockerfile.launcher .
Run Che: docker run --rm -v /var/run/docker.sock:/var/run/docker.sock codenvy/che-launcher start
Update Che: docker run --rm -v /var/run/docker.sock:/var/run/docker.sock codenvy/che-launcher update
Restart Che: docker run --rm -v /var/run/docker.sock:/var/run/docker.sock codenvy/che-launcher restart
Stop Che: docker run --rm -v /var/run/docker.sock:/var/run/docker.sock codenvy/che-launcher stop

codenvy-ci · 2016-07-07T14:39:03Z

Can one of the admins verify this patch?

garagatyi · 2016-07-07T15:42:59Z

Dockerfile.launcher

+#  `docker run --rm \
+#       -v /var/run/docker.sock:/var/run/docker.sock \
+#        codenvy/che-launcher [start|stop|restart|update]
+FROM docker:1.11


If user has docker 1.10 or 1.12 on his machine/server, will he get exception due to incompatible docker client/server?

That's a problem. I've tested it with 1.12rc2 and it worked but I got the exception with v1.10. I can think about 2 possible solutions:

Mount the docker binary as well:
docker run -v $(which docker):/usr/local/bin/docker -v /var/run/docker.sock:/var/run/docker.sock codenvy/che-launcher start

Use a tool like dvm to install multiple Docker clients in the image and update launch.sh to test which docker client works well.

What do you think?

We can also support several tags of launcher image with different versions of docker. WDYT?

How would you tag these images with different Docker version? codenvy/che-launcher:release-docker1.11 ?

My main concern is that users should check their Docker version before running Che and adapt the docker run command accordingly. I would prefer copy/paste a simple command without any editing.

Let me check what Rancher and Docker UCP do because they must have the same problem.

The $(which docker) syntax will fail on Docker for Windows. But you can do a string parse using docker version. There is go format syntax accepted as part of the command. But this does seem like an inelegant solution. All this work to simplify the run syntax to just introduce a complicated mount to handle the docker client binary.

Seems like we need to embed smart logic within the launcher.sh to figure out what docker daemon version is running, test for version compatibility, and then use the right docker client within the launcher.

TylerJewell · 2016-07-07T15:57:23Z

I am such a huge fan of this approach to simplifying the run syntax. I think we have to develop a strategy that inverses some of the files.

We need to move codenvy/che --> codenvy/che-server.
We need this image to be codenvy/che (not codenvy/che-launcher)
We need to find a location in the Che source repository to hold all of our Dockerfiles. Maybe we should have:
/docker/Dockerfile.server
/Dockerfile (represents the launcher)
/docker/Dockerfile. for when we add Dockerfiles for version check, volume-from, etc.

One issue with this approach is that it is not easy to build the images. The server Dockerfile assumes that there is a Che assembly in a certain sub-directory. Also the file naming becomes inconsistent.

Subsequently, if there are executing scriptions that need to be packaged into each image, how do we store those? The launcher.sh is specific only to the outer launcher image. The server image, for example, has /bin/che.sh inside of it for launching the Tomcat server.

garagatyi · 2016-07-07T15:58:48Z

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

+
+  for command_line_option in "$@"
+  do
+  case ${command_line_option} in


Personally I prefer leaving do in the same line where for is placed and addition of indentation of body of cycle.
It makes shell script more similar to java code. Please consider such reformatting.

garagatyi · 2016-07-07T16:21:46Z

I like simplified Che management approach. Good job, @l0rd!

Here are some possible improvements (not obviously for that PR):

we should consider possibility to launch Che from stable channel, so user should be able to choose
also running che as a daemon would help some users. I mean starting container with restart policy and maybe provide possibility to store logs at specified file.

garagatyi · 2016-07-07T16:24:31Z

I'm not sure how launcher container will behave if user call start of the server and then press CTRL-C at some point. Will it stop the server? Will it react somehow at all?

@l0rd Can you clarify?

l0rd · 2016-07-07T17:22:17Z

@garagatyi thanks for your feedback! I will made changes accordingly and answer to your questions tomorrow.

benoitf · 2016-07-08T12:54:05Z

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

+                    grep 'inet ' | \
+                    cut -d/ -f1 | \
+                    awk '{ print $2}')
+}


with docker for mac it returns the correct ip (I tried with both implementations)

but it doesn't work with docker toolbox for mac

$ docker run --rm --net host alpine sh -c ifconfig
docker0 Link encap:Ethernet HWaddr 02:42:BD:D7:32:B8
inet addr:172.17.0.1 Bcast:0.0.0.0 Mask:255.255.0.0
inet6 addr: fe80::42:bdff:fed7:32b8%32597/64 Scope:Link
UP BROADCAST MULTICAST MTU:1500 Metric:1
RX packets:49 errors:0 dropped:0 overruns:0 frame:0
TX packets:39 errors:0 dropped:0 overruns:0 carrier:0
collisions:0 txqueuelen:0
RX bytes:4277 (4.1 KiB) TX bytes:5483 (5.3 KiB)

eth0 Link encap:Ethernet HWaddr 08:00:27:A2:AF:A6
inet addr:10.0.2.15 Bcast:10.0.2.255 Mask:255.255.255.0
inet6 addr: fe80::a00:27ff:fea2:afa6%32597/64 Scope:Link
UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1
RX packets:3609 errors:0 dropped:0 overruns:0 frame:0
TX packets:1934 errors:0 dropped:0 overruns:0 carrier:0
collisions:0 txqueuelen:1000
RX bytes:2580871 (2.4 MiB) TX bytes:159603 (155.8 KiB)

eth1 Link encap:Ethernet HWaddr 08:00:27:70:6A:D8
inet addr:192.168.99.100 Bcast:192.168.99.255 Mask:255.255.255.0
inet6 addr: fe80::a00:27ff:fe70:6ad8%32597/64 Scope:Link
UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1
RX packets:1170 errors:0 dropped:0 overruns:0 frame:0
TX packets:909 errors:0 dropped:0 overruns:0 carrier:0
collisions:0 txqueuelen:1000
RX bytes:182053 (177.7 KiB) TX bytes:265568 (259.3 KiB)

lo Link encap:Local Loopback
inet addr:127.0.0.1 Mask:255.0.0.0
inet6 addr: ::1%32597/128 Scope:Host
UP LOOPBACK RUNNING MTU:65536 Metric:1
RX packets:0 errors:0 dropped:0 overruns:0 frame:0
TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
collisions:0 txqueuelen:1
RX bytes:0 (0.0 B) TX bytes:0 (0.0 B)

the working ip is 192.168.99.100 (eth1) in my case

@benoitf thanks for testing. I'll look at that.

l0rd · 2016-07-08T15:06:01Z

Che server container is now started in background (-d parameter).

CTRL+C works if both Che server and Che launcher containers are run with -ti parameter (this way Docker forward signal signals to PID 1 process in container). But running it in the background makes more sense in my opinion.

Let me add a function in launcher.sh to retrieve the tag (i.e. version) of the Che launcher image and set CHE_IMAGE_VERSION. Running codenvy/che-launcher:v123 will spawn codenvy/che:v123.

TylerJewell · 2016-07-08T15:08:25Z

@l0rd - if you launch the server in the background where people cannot get the logs, should we:

Add in some sort of test like we do in the vagrant files to print out a message in the launcher when the server is ready, what the URL is for the client to reach it?
Should we print out a message on where a user can monitor the server startup (which log file is being streamed)? This is pretty hard because the log file will be in a container that a user will have to get into.

l0rd · 2016-07-08T15:16:38Z

@TylerJewell Would you think a message like the following would help?

Che server has been started. Run docker logs -f che to see Che server logs

TylerJewell · 2016-07-08T15:18:10Z

Yeah, definitely that is super useful. I think that combined with the test to verify the server is up, and the printout of the client URL is perfect.

TylerJewell · 2016-07-08T16:20:24Z

@l0rd - Based upon this comment, it dawns on me that we have many people who are running into issues because the directory where /lib-copy will get mounted is not writable. In one case, i saw a windows user who just didn't have his directory activated for mounting. And in a second case, the directory was mountable but not writable.

Should we put in a check to test to make sure that the directories are writable for the following two locations? If these values are not provided in a che.properties, then we are using ${che.home}. If they are provided, then they are overrides.

If you do not have write permissions to these folders, this is when the /mnt errors show up, and they are very mysterious to most people.

machine.server.terminal.path_to_archive.linux_amd64=${che.home}/lib/linux_amd64/terminal
machine.server.terminal.path_to_archive.linux_arm7=${che.home}/lib/linux_arm7/terminal
machine.server.ext.archive=${che.home}/lib/ws-agent.zip

LeDominik · 2016-07-08T18:19:18Z

@TylerJewell I think you also need a configuration for the machine-workspaces location...

garagatyi · 2016-07-09T06:18:44Z

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

+
+# See: https://sipb.mit.edu/doc/safe-shell/
+set -e
+trap exit SIGHUP SIGINT SIGTERM


What about moving these lines to line 109, WDYT?

Since you don't have exit function in this script you don't need trapping

And actually script fails because of bad trapping on my machine

@garagatyi I agree. Do we have the same issue in che.sh?

Yes we have the same bug in che.sh

TylerJewell · 2016-07-09T06:24:40Z

Hi @l0rd - I have started a list of to-dos, and also started to lay down some of the requirements that I think we need to zero in on for this to be mergeable.

Note - this comment has been heavily edited. We now reference #1697 for the broader checklist of items to complete.

Image Name	Container Name	Purpose
`codenvy/che`	`che`	Launcher container
`codenvy/che-server`	`che-server`	Launches Che via Tomcat
`codenvy/che-version`	`che-version`	Checks for new versions of Che and downloads images
`codenvy/che-sdk`	`che-sdk`	A container that can build extensions and custom assemblies
`codenvy/che-file`	`che-file`	A container that will run the Chefile utility
`codenvy/che-install`	`che-install`	A container that installs platform scripts / binaries.

The repository of Che would be restructured as:

/
  che.sh   # Utility script to simplify executing "docker run ..."
  che.bat  # Utility script
  dockerfiles/
  dockerfiles/<image-name>/Dockerfile
  dockerfiles/<image-name>/<something-for-build>

We would remove the default Dockerfile out of the root of the repository and place it in the right /dockerfiles folder.

List of environment variables for launcher:

CHE_HOST_LOCATION, default = ${che.home}
CHE_WORKSPACES_LOCATION, default = ${CHE_HOST_LOCATION}/workspaces
CHE_TERMINAL_MOUNT_LOCATION, default = ${CHE_HOST_LOCATION}/lib/
CHE_AGENT_ZIP_LOCATION, default = {CHE_HOST_LOCATION}/lib/ws-agent.zip
CHE_DOCKER_HOST_IP, default = find_docker_host_ip() method
CHE_PORT, default = 8080
CHE_VERSION, default = latest
CHE_PROPERTIES_LOCATION, default = /home/user/che
CHE_RESTART_POLICY, default = always

For the policy on che.properties. We can allow the user to pass in an environment variable which provides a fully qualified path to a local che.properties file. If that is present, we should mount that into the Che server and use that.

For the environment variables that are involved with changing the mount location, if any of the environment variables requires a che.properties modification, take the file passed in by the user and use sed to alter the values so that the environment variables are overrides. If no che.properties exists, then we can create a new one and append the appropriate property entries.

garagatyi · 2016-07-09T06:27:57Z

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

+  USAGE="
+Usage:
+  `basename "$0"` [COMMAND]
+     start                              Starts Che server in new console


Container is started as detached docker container. I'm not sure it is precise and meaningful to say that it is launched in a new console

garagatyi · 2016-07-09T09:15:50Z

If che container is stopped neither launcher.sh start nor launcher.sh restart is working. Both returns errors. @l0rd Please ensure these situations are handled properly.

garagatyi · 2016-07-09T09:19:21Z

Doesn't work on my linux because of problems with permissions

bin[simpler-docker-command !]$ ./launcher.sh start
cabf1976df166d651006a434c9da916f60e71ed604205003f6afded0c2d9292e
bin[simpler-docker-command !]$ docker ps
CONTAINER ID        IMAGE               COMMAND             CREATED             STATUS              PORTS               NAMES
bin[simpler-docker-command !]$ docker ps -a
CONTAINER ID        IMAGE                 COMMAND                  CREATED             STATUS                      PORTS               NAMES
cabf1976df16        codenvy/che:nightly   "/home/user/che/bin/c"   12 seconds ago      Exited (0) 11 seconds ago                       che
bin[simpler-docker-command !]$ docker logs che

!!!
!!! Running 'docker' succeeded, but 'docker ps' failed.
The file /var/run/docker.sock does not have appropriate permissions.
OWNER READ:  r
OWNER WRITE: w
GROUP READ:  r
GROUP WRITE: w
OTHER READ:  -
OTHER WRITE: -
Run 'sudo chmod 666 /var/run/docker.sock' to give the right permissions.
!!!

bin[simpler-docker-command !]$

garagatyi · 2016-07-18T15:39:49Z

Everything else is awesome. What a complex PR. @l0rd super cool contribution!

benoitf · 2016-07-18T16:32:32Z

dockerfiles/che-cli/files-to-install/che.sh

+}
+
+info() {
+  printf  "${GREEN}INFO:${NC} %s\n" "${1}"


multiple spaces after printf

Trailing spaces removed

garagatyi · 2016-07-18T19:04:48Z

And cool improvement from @TylerJewell, good job guys!

garagatyi · 2016-07-19T06:30:19Z

@l0rd @TylerJewell I'm curious how are you going to marge that PR with 30 commits? Are you going to squash it or leave these 30 commits?

garagatyi · 2016-07-19T06:46:24Z

dockerfiles/che-launcher/launcher.sh

+}
+
+server_is_booted() {
+  HTTP_STATUS_CODE=$(curl -I http://"${CHE_HOST_IP}":"${CHE_PORT}"/api/ 2>/dev/null \


you can also use specific options of curl instead of manual parsing of output
curl http://"${CHE_HOST_IP}":"${CHE_PORT}"/api/ -s -o /dev/null --write-out "%{http_code}"
where
-s do not print additional info
-o /dev/null suppress response body
--write-out "%{http_code}"` print status code

l0rd · 2016-07-19T08:50:12Z

@garagatyi I've updated curl command with your suggestions (and I kept option -I to just retrieve the header). On both Vagrantfile and launcher.sh.

I personally don't mind leaving the 30 commits like that (authors and history are preserved) but if you thinks it's more appropriate I have no problem squashing all of them.

garagatyi · 2016-07-19T08:52:32Z

@skabashnyuk @riuvshin @TylerJewell how do you think is it better to squash commits or we are ok if this goes into master with such a huge history tree?

skabashnyuk · 2016-07-19T08:53:20Z

squash

riuvshin · 2016-07-19T08:53:55Z

--squash

…/restart Che Signed-off-by: Mario Loriedo <[email protected]>

l0rd · 2016-07-19T09:23:29Z

@garagatyi @riuvshin @skabashnyuk done 😃

garagatyi · 2016-07-19T09:40:05Z

@l0rd Thanks!
I think that all commits should be signed because you are not eclipse che committer, but one of them is not signed. Please check that.

l0rd · 2016-07-19T09:42:35Z

The only commit that is not signed is @TylerJewell's commit. Do I need to sign that commit with my signature?

garagatyi · 2016-07-19T09:47:18Z

I believe so. This commit is pushed by you, so it doesn't prove that it is authored by @TylerJewell.

Signed-off-by: Mario Loriedo <[email protected]>

l0rd · 2016-07-19T09:56:48Z

Done

garagatyi · 2016-07-19T10:41:39Z

Thanks

garagatyi · 2016-07-19T10:42:34Z

@l0rd @TylerJewell is this PR ready or other changes are still needed?

Signed-off-by: Mario Loriedo <[email protected]>

TylerJewell · 2016-07-19T13:26:39Z

Congratulations @l0rd!

l0rd force-pushed the simpler-docker-command branch from 658d34e to abd2488 Compare July 7, 2016 14:37

garagatyi reviewed Jul 7, 2016
View reviewed changes

benoitf reviewed Jul 8, 2016
View reviewed changes

TylerJewell mentioned this pull request Jul 8, 2016

I cannot create a fully functional CHE workspace using Docker beta for mac #1482

Closed

garagatyi reviewed Jul 9, 2016
View reviewed changes

benoitf reviewed Jul 18, 2016
View reviewed changes

TylerJewell mentioned this pull request Jul 18, 2016

Find way to distinguish between Docker for Mac and Windows #1744

Closed

garagatyi reviewed Jul 19, 2016
View reviewed changes

Use a launcher image to simplify docker commands to start/stop/update…

8dbffa2

…/restart Che Signed-off-by: Mario Loriedo <[email protected]>

l0rd force-pushed the simpler-docker-command branch from fb38bdb to 01ee7bc Compare July 19, 2016 09:21

Added new features to Launcher and introduced Che CLI

ff50a29

Signed-off-by: Mario Loriedo <[email protected]>

l0rd force-pushed the simpler-docker-command branch from 01ee7bc to cdd2e5f Compare July 19, 2016 09:55

l0rd mentioned this pull request Jul 19, 2016

Downgrade docker installed in Che image #1833

Merged

Improvements to launcher.sh after code reviews and che-dev Docker image

61b6442

Signed-off-by: Mario Loriedo <[email protected]>

l0rd force-pushed the simpler-docker-command branch from cdd2e5f to 61b6442 Compare July 19, 2016 13:23

TylerJewell merged commit de60983 into eclipse-che:master Jul 19, 2016

bmicklea mentioned this pull request Jul 20, 2016

Milestone Summary 4.6.0 #1870

Closed

64 tasks

TylerJewell added the kind/epic A long-lived, PM-driven feature request. Must include a checklist of items that must be completed. label Jul 22, 2016

TylerJewell added this to the 4.6.0 milestone Jul 22, 2016

Use a docker launcher image to simplify start/stop/update/restart commands #1683

Use a docker launcher image to simplify start/stop/update/restart commands #1683

Conversation

l0rd commented Jul 7, 2016

codenvy-ci commented Jul 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

l0rd Jul 8, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TylerJewell commented Jul 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garagatyi commented Jul 7, 2016

garagatyi commented Jul 7, 2016

l0rd commented Jul 7, 2016

benoitf Jul 8, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

l0rd commented Jul 8, 2016

TylerJewell commented Jul 8, 2016

l0rd commented Jul 8, 2016

TylerJewell commented Jul 8, 2016

TylerJewell commented Jul 8, 2016

LeDominik commented Jul 8, 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

TylerJewell commented Jul 9, 2016 • edited by vinokurig Loading

Choose a reason for hiding this comment

garagatyi commented Jul 9, 2016

garagatyi commented Jul 9, 2016

garagatyi commented Jul 18, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garagatyi commented Jul 18, 2016

garagatyi commented Jul 19, 2016

Choose a reason for hiding this comment

l0rd commented Jul 19, 2016

garagatyi commented Jul 19, 2016

skabashnyuk commented Jul 19, 2016

riuvshin commented Jul 19, 2016

l0rd commented Jul 19, 2016

garagatyi commented Jul 19, 2016

l0rd commented Jul 19, 2016

garagatyi commented Jul 19, 2016

l0rd commented Jul 19, 2016

garagatyi commented Jul 19, 2016

garagatyi commented Jul 19, 2016

TylerJewell commented Jul 19, 2016

l0rd Jul 8, 2016 •

edited

Loading

benoitf Jul 8, 2016 •

edited

Loading

TylerJewell commented Jul 9, 2016 •

edited by vinokurig

Loading