Enable single port exposure on Che #5115

benoitf · 2017-05-17T11:23:54Z

What does this PR do?

Enable single port exposure on Che

Toggle is made by enabling CHE_SINGLE_PORT in the che.env file

By enabling single-port, all browser traffic to Che or any workspace will be routed through the value that you have set to CHE_PORT`, or 8080 if not set. Setting this property will transform the launch sequence of Che to launch a Traefik reverse proxy. The reverse proxy will act as the traffic endpoint for all browser communications. When a new workspace is started or stopped, Che will update Traefik's configuration with rules for how browser traffic should be routed to Che or a workspace.

It’s now using an official Traefik image (before I was using a custom made image (florentbenoit/traefik))

There is an interceptor with a kill switch. It means interceptor is applied only if plug-in is enabled (not only if plug-in is added at compilation)
It is automatically enabled when CHE_SINGLE_PORT is turned on
docker-compose file is handling if the single_port is turned on or off and then add the traefik container and redirect port only if the property is enabled. (not enabled by default)
using --debug flag when launching Eclipse Che is also turning on the traffic web console to view traefik routes
It is not enabled by default, so it means that without user change, there is no overhead, no useless container started, etc.

What issues does this PR fix or reference?

#4361

Changelog

Enable single port exposure on Che

Release Notes

By enabling single-port, all browser traffic to Che or any workspace will be routed through the value that you have set to CHE_PORT`, or 8080 if not set. Setting this property will transform the launch sequence of Che to launch a Traefik reverse proxy. The reverse proxy will act as the traffic endpoint for all browser communications. When a new workspace is started or stopped, Che will update Traefik's configuration with rules for how browser traffic should be routed to Che or a workspace.

Docs PR

eclipse-che/che-docs#229

Change-Id: I12644d9202dadc0b10104f78bb055425ca6611ac
Signed-off-by: Florent BENOIT fbenoit@codenvy.com

…e. (CHE_SINGLE_PORT=true, default is false) By enabling single-port, all browser traffic to Che or any workspace will be routed through the value that you have set to CHE_PORT`, or 8080 if not set. Setting this property will transform the launch sequence of Che to launch a Traefik reverse proxy. The reverse proxy will act as the traffic endpoint for all browser communications. When a new workspace is started or stopped, Che will update Traefik's configuration with rules for how browser traffic should be routed to Che or a workspace. It’s now using an official Traefik image (before I was using a custom made image) There is an interceptor with a kill switch. It means interceptor is applied only if plug-in is enabled (not only if plug-in is added at compilation) It is automatically enabled when CHE_SINGLE_PORT is turned on docker-compose file is handling if the single_port is turned on or off and then add the traefik container and redirect port only if the property is enabled. (not enabled by default) using —debug flag when launching che is also turning on the traffic web console to view traefik routes It is not enabled by default, so it means that without user change, there is no overhead, no useless container started, etc. Change-Id: I12644d9202dadc0b10104f78bb055425ca6611ac Signed-off-by: Florent BENOIT <fbenoit@codenvy.com>

slemeur · 2017-05-17T11:51:53Z

ok for me, just needs the docs ;)

codenvy-ci · 2017-05-17T12:57:56Z

Build # 2617 - FAILED

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

garagatyi

Overall looks good. It is an interesting feature, good work!
Can you elaborate on commented lines to align reviewers view of these changes?

garagatyi · 2017-05-17T12:28:16Z

dockerfiles/init/manifests/che.env

+# The reverse proxy will act as the traffic endpoint for all browser communications.
+# When a new workspace is started or stopped, Che will update Traefik's configuration
+# with rules for how browser traffic should be routed to Che or a workspace.
+CHE_SINGLE_PORT=false


This explanation doesn't describe how it interferes with DNS resolution. I wonder whether it does?

You need extra doc ?

Right now I want to understand how it would work in the different situation. Then I would understand whether I believe additional docs are needed.

I will add docs

garagatyi · 2017-05-17T12:36:32Z

.../plugin-traefik-docker/src/main/java/org/eclipse/che/plugin/traefik/TraefikDockerModule.java

+    protected void configure() {
+
+        // add logic only if plug-in is enabled.
+        if (Boolean.parseBoolean(firstNonNull(System.getenv("CHE_PLUGIN_TRAEFIK_ENABLED"), "false"))) {


I suppose Boolean.parseBoolean(System.getenv("CHE_PLUGIN_TRAEFIK_ENABLED")) would do the same, how do you think?

yes correct, I will fix

garagatyi · 2017-05-17T12:43:00Z

...k-docker/src/main/java/org/eclipse/che/plugin/traefik/TraefikCreateContainerInterceptor.java

+import static java.lang.String.format;
+
+/**
+ * Intercept the calls on createContainer on docker Connector.


Can you add the purpose of such an addition to the docs? Without context, it is not clear why it does so.

garagatyi · 2017-05-17T12:48:33Z

...k-docker/src/main/java/org/eclipse/che/plugin/traefik/TraefikCreateContainerInterceptor.java

+    public Object invoke(MethodInvocation methodInvocation) throws Throwable {
+        ServerEvaluationStrategy serverEvaluationStrategy = serverEvaluationStrategyProvider.get();
+        // Abort if custom server evaluation strategy is not enabled.
+        if (!(CustomServerEvaluationStrategy.class.isInstance(serverEvaluationStrategy))) {


Correct me if I'm mistaken but with this code, the interceptor is enabled whenever the custom strategy is applied.
If so what if the custom strategy is used with some external proxy instead of Traefik.
Then the custom strategy looks rather Traefik oriented than custom while from the configuration point of view it looks like it really custom and can be used in a wide range of situations.

in Guice module, we do not bind the interceptor if we do not enabled traefik.
https://github.com/eclipse/che/blob/683b6f4b8253cc83c8ce7500020c7bf2ee8a8130/plugins/plugin-traefik/plugin-traefik-docker/src/main/java/org/eclipse/che/plugin/traefik/TraefikDockerModule.java#L35-L40
so it's an extra check in case the interceptor is bound but the strategy is not the expected one.

And you can still enable the custom strategy without enabling the traefik plug-in/interceptor

ok then, thx for the explanation

garagatyi · 2017-05-17T12:51:09Z

...k-docker/src/main/java/org/eclipse/che/plugin/traefik/TraefikCreateContainerInterceptor.java

+        DockerConnector dockerConnector = (DockerConnector)methodInvocation.getThis();
+
+        // only one parameter which is CreateContainerParams
+        CreateContainerParams createContainerParams = (CreateContainerParams)methodInvocation.getArguments()[0];


Florent, do you know whether this code works with Openshift based Che? I wonder whether workarounds with replacing DockerConnector with Openshift related entities and this code may work correctly together.

well due to all changes on #5052 I will check once it is merged but either we can drop support for OpenShift connector or if there are issues, it will be fixed.

In this particular comment, I meant the current state of Openshift packaging in master, not from that PR.
I have concerns because DockerConnector is replaced with Openshift patched version and it works in a different way. So I want to ensure that if those components don't work together we know that and document that.

In the current state there is no issue.

garagatyi · 2017-05-17T13:08:50Z

@sunix @gorkem @benoitf Looks like this PR interferes with PR #5052 in the area of single port strategy.
I have concerns that we need 2 strategies with a single port feature at the same time and even in the review process at the same time.
Maybe we can use server strategy from this PR to use in Openshift packaging?

…env file. (CHE_SINGLE_PORT=true, default is false) Change-Id: I06183f745fcb58c4ad6249ffbae435e143ff3fa6 Signed-off-by: Florent BENOIT <fbenoit@codenvy.com>

benoitf · 2017-05-17T16:47:04Z

Added link to documentation PR

codenvy-ci · 2017-05-17T18:28:13Z

Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/2622/

benoitf · 2017-05-18T08:06:38Z

@garagatyi, about

I have concerns that we need 2 strategies with a single port feature at the same time and even in the review process at the same time.
Maybe we can use server strategy from this PR to use in Openshift packaging?

OpenShift doesn't need a front-end (it's what is done in this PR) so this PR doesn't collide with #5052

but#5052 OpenShift PR collide with the existing https://github.com/eclipse/che/blob/52e418bc73c36eaa86c9a427a6014f923de797b7/plugins/plugin-docker/che-plugin-docker-machine/src/main/java/org/eclipse/che/plugin/docker/machine/CustomServerEvaluationStrategy.java that allow to provide a different naming for workspace urls.

sunix · 2017-05-18T08:08:22Z

@garagatyi @gorkem @benoitf yes this interferes with #5052 we should discuss with @l0rd about that

benoitf · 2017-05-18T08:10:39Z

@sunix : it doesn't. #5052 is introducing a custom server evaluation strategy provider. This one is not, it reuse a previously one. It only allow to provide a frontend/proxy which is traefik.
But you may enable only the custom server evaluation strategy provider without enabling traefik frontend.

garagatyi · 2017-05-18T08:13:27Z

Well looks like #5052 interferes with an existing solution that is already merged and not related to this PR.
@sunix @gorkem @l0rd Can you take a look at that strategy to figure out whether it can be reused?

benoitf · 2017-05-18T08:13:37Z

Also the discussion started a long time ago on #4269

sunix · 2017-05-18T12:46:49Z

I works for me as the 2 can live together (we select either one strategy or another in che.env). Though in a second step we should consider using a custom one in openshift connector

gorkem · 2017-05-21T23:44:26Z

@sunix can you create another issue to reuse this and replace the openshift one?

riuvshin · 2017-05-22T07:30:50Z

@benoitf hey, is setting CHE_SINGLE_PORT=true enough to test this? CHE does not work for us here.

* Toggle Che single port by enabling CHE_SINGLE_PORT in the che.env file. (CHE_SINGLE_PORT=true, default is false) By enabling single-port, all browser traffic to Che or any workspace will be routed through the value that you have set to CHE_PORT`, or 8080 if not set. Setting this property will transform the launch sequence of Che to launch a Traefik reverse proxy. The reverse proxy will act as the traffic endpoint for all browser communications. When a new workspace is started or stopped, Che will update Traefik's configuration with rules for how browser traffic should be routed to Che or a workspace. It’s now using an official Traefik image (before I was using a custom made image) There is an interceptor with a kill switch. It means interceptor is applied only if plug-in is enabled (not only if plug-in is added at compilation) It is automatically enabled when CHE_SINGLE_PORT is turned on docker-compose file is handling if the single_port is turned on or off and then add the traefik container and redirect port only if the property is enabled. (not enabled by default) using —debug flag when launching che is also turning on the traffic web console to view traefik routes It is not enabled by default, so it means that without user change, there is no overhead, no useless container started, etc. Change-Id: I12644d9202dadc0b10104f78bb055425ca6611ac Signed-off-by: Florent BENOIT <fbenoit@codenvy.com>

benoitf added kind/enhancement A feature request - must adhere to the feature request template. sprint/current status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. labels May 17, 2017

benoitf requested review from skabashnyuk, slemeur and garagatyi May 17, 2017 11:23

benoitf force-pushed the traefik-plugin branch from 2a5047a to 683b6f4 Compare May 17, 2017 11:25

benoitf mentioned this pull request May 17, 2017

[Wip] Introduce reverse proxy for all http protocol + custom template server strategy #4440

Closed

garagatyi reviewed May 17, 2017

View reviewed changes

fixup! Toggle Che single port by enabling CHE_SINGLE_PORT in the che.…

6ff9718

…env file. (CHE_SINGLE_PORT=true, default is false) Change-Id: I06183f745fcb58c4ad6249ffbae435e143ff3fa6 Signed-off-by: Florent BENOIT <fbenoit@codenvy.com>

benoitf mentioned this pull request May 17, 2017

Add single port routing options eclipse-che/che-docs#229

Merged

slemeur approved these changes May 17, 2017

View reviewed changes

skabashnyuk approved these changes May 18, 2017

View reviewed changes

garagatyi approved these changes May 18, 2017

View reviewed changes

benoitf self-assigned this May 18, 2017

benoitf merged commit a13068e into master May 18, 2017

benoitf deleted the traefik-plugin branch May 18, 2017 12:52

benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label May 18, 2017

benoitf added this to the 5.11.0 milestone May 18, 2017

benoitf removed the sprint/current label May 18, 2017

benoitf mentioned this pull request Jun 14, 2017

Expose Che server and workspaces through one single TCP port #4269

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable single port exposure on Che #5115

Enable single port exposure on Che #5115

benoitf commented May 17, 2017 •

edited

Loading

slemeur commented May 17, 2017

codenvy-ci commented May 17, 2017

garagatyi left a comment

garagatyi May 17, 2017

benoitf May 17, 2017

garagatyi May 17, 2017

benoitf May 17, 2017

benoitf May 17, 2017

garagatyi May 17, 2017

benoitf May 17, 2017

benoitf May 17, 2017

garagatyi May 17, 2017

benoitf May 17, 2017

benoitf May 17, 2017

garagatyi May 17, 2017

benoitf May 17, 2017

garagatyi May 17, 2017

garagatyi May 17, 2017

benoitf May 17, 2017

garagatyi May 17, 2017

benoitf May 17, 2017

garagatyi commented May 17, 2017 •

edited

Loading

benoitf commented May 17, 2017

codenvy-ci commented May 17, 2017

benoitf commented May 18, 2017

sunix commented May 18, 2017

benoitf commented May 18, 2017

garagatyi commented May 18, 2017

benoitf commented May 18, 2017

sunix commented May 18, 2017

gorkem commented May 21, 2017 •

edited

Loading

riuvshin commented May 22, 2017

Enable single port exposure on Che #5115

Enable single port exposure on Che #5115

Conversation

benoitf commented May 17, 2017 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

Changelog

Release Notes

Docs PR

slemeur commented May 17, 2017

codenvy-ci commented May 17, 2017

garagatyi left a 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

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

garagatyi commented May 17, 2017 • edited Loading

benoitf commented May 17, 2017

codenvy-ci commented May 17, 2017

benoitf commented May 18, 2017

sunix commented May 18, 2017

benoitf commented May 18, 2017

garagatyi commented May 18, 2017

benoitf commented May 18, 2017

sunix commented May 18, 2017

gorkem commented May 21, 2017 • edited Loading

riuvshin commented May 22, 2017

benoitf commented May 17, 2017 •

edited

Loading

garagatyi commented May 17, 2017 •

edited

Loading

gorkem commented May 21, 2017 •

edited

Loading