Distribute agents from separate container, not from API #1346

tolusha · 2016-12-14T08:55:12Z

What does this PR do?

Distribute agents from separate container, not from API

What issues does this PR fix or reference?

#1153

Previous Behavior

Agents were being downloaded from master.

New Behavior

Agents will be downloaded from dedicated docker container.

Tests written?

Manual testing

benoitf · 2016-12-14T09:27:52Z

Is that 9000 port is a port that clients need to connect ? (if yes we have to check the port locally)
https://github.com/codenvy/codenvy/blob/master/dockerfiles/cli/scripts/cli.sh#L130-L135
it seems we should check the port in dev mode

To provide the agents, in the PR, you're using alpine + lighthttpd
as we're already requiring nginx image, shouldn't it be better to use nginx image ? so we don't require/need a new image

riuvshin · 2016-12-14T09:32:10Z

dockerfiles/agents/Dockerfile

+    && echo "server.port = 9000" >> /etc/lighttpd/lighttpd.conf \
+    && ln -s /home/user/che /var/www/localhost/htdocs/agent-binaries
+
+RUN addgroup -S user && adduser -S -g user user


do we really need user here?

Can be removed.

riuvshin · 2016-12-14T09:33:26Z

dockerfiles/agents/Dockerfile

+
+EXPOSE 9000
+
+CMD ["lighttpd", "-f", "/etc/lighttpd/lighttpd.conf", "-D"]


where is lighttpd.conf ? maybe we need generate that config with puppet?

Agree. should be managed by puppet

riuvshin · 2016-12-14T09:35:49Z

@benoitf no 9000 port is internal all connections will be to haproxy like <API_END_POINT>/agent-binaries

benoitf · 2016-12-14T09:37:01Z

@riuvshin

+<% if scope.lookupvar('compose::env') != 'production' -%>
 +    ports:
 +      - '9000:9000'
 +<% end -%>

riuvshin · 2016-12-14T09:40:45Z

yes this is for dev mode, I've asked to add this because I thought it will be tomcat and we will need be able to debug it in dev mode. so we have to remove that part.
that container with agents was added as haproxy backend so all clients will connect to haproxy

tolusha · 2016-12-14T12:08:11Z

@benoitf
The task is about to add a dedicated container not to reuse existed one.

riuvshin

LGTM

benoitf · 2016-12-14T12:48:51Z

@tolusha yes I was talking about image, not the container

benoitf · 2016-12-14T13:21:09Z

dockerfiles/init/modules/compose/templates/docker-compose.yml.erb

+      - '<%= scope.lookupvar('compose::codenvy_folder') -%>/config/lighttpd/lighttpd.conf:/etc/lighttpd/lighttpd.conf'
+<% if scope.lookupvar('compose::env') != 'production' -%>
+    ports:
+      - '9000:9000'


if this code is still there
then
https://github.com/codenvy/codenvy/blob/master/dockerfiles/cli/scripts/cli.sh#L130-L135 need to check 9000 port

if the local listener is dropped, then no need to check this port

Anatoly wanted to keep open that port in dev mode to test server without codenvy.
We don;t need check this port I think as it is only in dev mode
(but I still want OK to remove that.)

either code is removed or either the port is checked in dev mode (as we do for debug port)

benoitf · 2016-12-14T13:24:13Z

dockerfiles/agents/Dockerfile

+
+RUN apk update \
+    && apk add lighttpd \
+    && ln -s /data /var/www/localhost/htdocs/agent-binaries


to reduce size of the image, cache of apk is usually dropped as /tmp folder :
rm -rf /var/cache/apk/* and sometimes /tmp folder

benoitf · 2016-12-14T13:28:08Z

dockerfiles/agents/build.sh

+    mv ${DEPENDENCY_DIR}/${prefix}/terminal/exec-agent-${prefix}.tar.gz ${DEPENDENCY_DIR}/${prefix}/terminal/websocket-terminal-${prefix}.tar.gz
+done
+
+mvn dependency:copy -Dartifact=com.codenvy.onpremises:onpremises-ide-packaging-tomcat-ext-server:${POM_VERSION}:tar.gz \


could we use a maven pom + assembly.xml file to copy all the required dependencies ?
then we could only call a single maven command

benoitf · 2016-12-14T13:29:11Z

dockerfiles/init/modules/lighttpd/manifests/init.pp

+class lighttpd {
+  file { "/opt/codenvy/config/lighttpd":
+    ensure  => "directory",
+    mode    => "755",


do we need execute permissions ?

this is common folder permission we always use that mode for folders we manage

benoitf · 2016-12-14T13:29:35Z

dockerfiles/init/modules/lighttpd/templates/lighttpd.conf.erb

@@ -0,0 +1,323 @@
+###############################################################################
+# Default lighttpd.conf for Gentoo.


I'm not sure comment is accurate as we're based on alpine

this is default content of lighttpd.conf it have info for all operations systems I think

yes but here we can keep only what is interesting

benoitf · 2016-12-14T13:30:18Z

dockerfiles/init/modules/lighttpd/templates/lighttpd.conf.erb

+# for >= linux-2.6
+#   server.event-handler = "linux-sysepoll"
+# for FreeBSD
+#   server.event-handler = "freebsd-kqueue"


here it's linux so no need to have comments around FreeBSD

this is default content of lighttpd.conf

but we don't need default values

benoitf · 2016-12-14T13:31:21Z

dockerfiles/agents/build.sh

+rm -rf ${DEPENDENCY_DIR}
+
+TERMINAL_AGENTS=("org.eclipse.che:exec-agent:${POM_VERSION}:tar.gz:linux_amd64"
+                 "org.eclipse.che:exec-agent:${POM_VERSION}:tar.gz:linux_arm7");


in which case the linux_arm7 will work ?
as all docker containers being based on linux/amd64 I don't see a usecase where arm arch will work ?

The only case I can see is in creating ssh machine (artic device)

benoitf · 2016-12-14T13:31:56Z

dockerfiles/init/modules/lighttpd/templates/lighttpd.conf.erb

+# debug.log-file-not-found   = "enable"
+# }}}
+
+# vim: set ft=conf foldmethod=marker et :


maybe we could drop tons of the commented lines ?

this is default content of that config, Im OK to drop comments.

tolusha · 2016-12-15T07:09:16Z

@benoitf
Updated

riuvshin · 2016-12-15T10:37:10Z

@tolusha Please prvide PR for customer-saas same way, you may need @benoitf help

skabashnyuk

I have a couple of question

Why don't you use existed nginx image?
In future how are you going to provide authentication for this client. Maybe you should take a look on some http server implemented on GO. we have authentication implemented for go based terminal

tolusha · 2016-12-15T13:49:29Z

lighttpd is the most light weight web serve I've found. That's was the purpose of using it.

tolusha · 2016-12-16T06:11:14Z

@skabashnyuk
Whose answer you are waiting for?
There weren't any requirements concerning authentication or specific webserver.
If you need this feel free to create a new issue.

skabashnyuk · 2016-12-16T08:26:43Z

ok up to you

* CODENVY-1153: Distribute agents from separate container, not from API

Anatoliy Bazko added 4 commits December 13, 2016 11:33

CODENVY-1153: Distribute agents from separate container, not from API

5f1dd5d

Fix up

1091df5

Fix up

0e1a9d3

Fix up

f41d32a

tolusha changed the title ~~Codenvy 1153~~ Distribute agents from separate container, not from API Dec 14, 2016

tolusha requested review from benoitf, skabashnyuk, riuvshin and garagatyi December 14, 2016 08:55

riuvshin reviewed Dec 14, 2016

View reviewed changes

Add template for lighttpd.conf

6405fe0

riuvshin approved these changes Dec 14, 2016

View reviewed changes

benoitf suggested changes Dec 14, 2016

View reviewed changes

benoitf reviewed Dec 14, 2016

View reviewed changes

Anatoliy Bazko added 2 commits December 15, 2016 08:25

Fix up

8adec5f

Merge master

d83a0bd

benoitf approved these changes Dec 15, 2016

View reviewed changes

skabashnyuk approved these changes Dec 15, 2016

View reviewed changes

Anatoliy Bazko added 3 commits December 15, 2016 15:09

Fix up

8fc32f6

Merge branch 'master' into CODENVY-1153

8087cf9

Fix up build

7afdb97

skabashnyuk reviewed Dec 15, 2016

View reviewed changes

skabashnyuk self-requested a review December 15, 2016 13:44

skabashnyuk requested review from skabashnyuk and removed request for skabashnyuk December 16, 2016 08:26

tolusha removed the request for review from garagatyi December 19, 2016 08:23

Merge master

3998ed8

tolusha merged commit ab5383c into master Dec 19, 2016

tolusha deleted the CODENVY-1153 branch December 19, 2016 08:28

benoitf pushed a commit that referenced this pull request Dec 20, 2016

Distribute agents from separate container, not from API (#1346)

bfd7a6a

* CODENVY-1153: Distribute agents from separate container, not from API


		EXPOSE 9000

		CMD ["lighttpd", "-f", "/etc/lighttpd/lighttpd.conf", "-D"]

		@@ -0,0 +1,323 @@
		###############################################################################
		# Default lighttpd.conf for Gentoo.

Distribute agents from separate container, not from API #1346

Distribute agents from separate container, not from API #1346

Conversation

tolusha commented Dec 14, 2016 • edited by JamesDrummond

What does this PR do?

What issues does this PR fix or reference?

Previous Behavior

New Behavior

Tests written?

benoitf commented Dec 14, 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

riuvshin commented Dec 14, 2016 • edited

benoitf commented Dec 14, 2016

riuvshin commented Dec 14, 2016

tolusha commented Dec 14, 2016

riuvshin left a comment

Choose a reason for hiding this comment

benoitf commented Dec 14, 2016

benoitf Dec 14, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benoitf Dec 14, 2016 • edited

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tolusha commented Dec 15, 2016

riuvshin commented Dec 15, 2016

skabashnyuk left a comment

Choose a reason for hiding this comment

tolusha commented Dec 15, 2016

tolusha commented Dec 16, 2016 • edited

skabashnyuk commented Dec 16, 2016

tolusha commented Dec 14, 2016 •

edited by JamesDrummond

riuvshin commented Dec 14, 2016 •

edited

benoitf Dec 14, 2016 •

edited

benoitf Dec 14, 2016 •

edited

tolusha commented Dec 16, 2016 •

edited