Add internal and external API URL environment variables for workspaces #9475

mkuznyetsov · 2018-04-18T06:53:52Z

What does this PR do?

All workspaces will be provided with additional environment variables, that will point to internal or external API URL. On Docker infrastructure:

CHE_API_INTERNAL variable will be provided from "che.infra.docker.master_api_endpoint" (or in case one is not set/set to null, then value from CHE_API env. var will be used).
Same value is provided for CHE_API.
CHE_API_EXTERNAL is provided from "che.api" property, defined in che.properties

In Openshift and Kubernetes,CHE_API, CHE_API_INTERNAL, CHE_API_EXTERNAL will be the same and based from "che.api" property.

What issues does this PR fix or reference?

#9323

Release Notes

Docs PR

…e-9323

sleshchenko · 2018-04-18T06:57:24Z

...ava/org/eclipse/che/api/workspace/server/spi/provision/env/CheApiInternalEnvVarProvider.java

+import org.eclipse.che.api.workspace.server.spi.InfrastructureException;
+import org.eclipse.che.commons.lang.Pair;
+
+/** @author Sergii Leshchenko */


Please fix authorship here

sleshchenko · 2018-04-18T07:21:10Z

...java/org/eclipse/che/workspace/infrastructure/docker/DockerCheApiExternalEnvVarProvider.java

+
+  @Inject
+  public DockerCheApiExternalEnvVarProvider() {
+    apiEnvVar = Pair.of(CHE_API_EXTERNAL_VARIABLE, System.getenv(CHE_API_EXTERNAL_VARIABLE));


Please explain why do you use System.getenv(CHE_API_EXTERNAL_VARIABLE) but not injected che.api constant

sleshchenko · 2018-04-18T07:26:44Z

...java/org/eclipse/che/workspace/infrastructure/docker/DockerCheApiInternalEnvVarProvider.java

+  @Inject
+  public DockerCheApiInternalEnvVarProvider(
+      @Named("che.infra.docker.master_api_endpoint") String apiEndpoint) {
+    String apiEndpointEnvVar = System.getenv(CHE_API_INTERNAL_VARIABLE);


I would say that it should be removed. If a user wants to customize this value he is able to specify CHE_INFRA_DOCKER_MASTER__API__ENDPOINT environment variable.

…into che-9323

…se/che into che-9323

…m:eclipse/che into che-9323

mkuznyetsov · 2018-04-18T09:25:24Z

ci-test

codenvy-ci · 2018-04-18T09:34:52Z

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:9475
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

…thub.com:eclipse/che into che-9323

mkuznyetsov · 2018-04-18T11:41:53Z

ci-test

codenvy-ci · 2018-04-18T12:23:56Z

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:9475
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

skabashnyuk · 2018-04-18T13:33:59Z

dockerfiles/init/modules/che/templates/che.env.erb

@@ -51,7 +51,6 @@ CHE_LOGS_DIR=/logs
 CHE_WORKSPACE_LOGS=/logs/machines
 CHE_TEMPLATE_STORAGE=/data/templates

-


remove this change if not needed

skabashnyuk · 2018-04-18T13:35:09Z

dockerfiles/init/manifests/che.env

@@ -160,7 +160,7 @@
 #
 # Browser --> Che Server
 #    1. Default is 'http://localhost:${SERVER_PORT}/api'.
-#    2. Else use the value of CHE_API
+#    2. Else use the value of CHE_API_EXTERNAL


I guess we need to remove this change too.

skabashnyuk · 2018-04-18T13:35:36Z

...ava/org/eclipse/che/api/workspace/server/spi/provision/env/EnvVarEnvironmentProvisioner.java

@@ -37,6 +37,7 @@ public void provision(RuntimeIdentity id, InternalEnvironment internalEnvironmen
      throws InfrastructureException {
    for (EnvVarProvider envVarProvider : envVarProviders) {
      Pair<String, String> envVar = envVarProvider.get(id);
+


remove this change if not needed

riuvshin · 2018-04-18T14:30:39Z

ci-test

codenvy-ci · 2018-04-18T14:34:13Z

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:9475
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

riuvshin · 2018-04-18T14:41:46Z

ci-test

codenvy-ci · 2018-04-18T14:48:02Z

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:9475
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

riuvshin · 2018-04-18T14:48:11Z

ci-test

codenvy-ci · 2018-04-18T15:32:46Z

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:9475
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

riuvshin · 2018-04-18T16:29:46Z

ci-test

ghost · 2018-04-18T19:58:31Z

ci-test

codenvy-ci · 2018-04-19T01:31:24Z

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:9475
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

sleshchenko · 2018-04-19T08:16:33Z

...ava/org/eclipse/che/api/workspace/server/spi/provision/env/CheApiInternalEnvVarProvider.java

+  String CHE_API_INTERNAL_VARIABLE = "CHE_API_INTERNAL";
+
+  /**
+   * Returns Che API environment variable which should be injected into machines.


Please document what is internal Che API URL

sleshchenko · 2018-04-19T08:19:09Z

...t/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInfraModule.java

@@ -67,6 +71,8 @@ protected void configure() {
    bind(RemoveProjectOnWorkspaceRemove.class).asEagerSingleton();

    bind(CheApiEnvVarProvider.class).to(KubernetesCheApiEnvVarProvider.class);


Do we still need CHE_API env var?
Or it wasn't removed because of backward compatibility? If yes, then maybe it would be better to ensure it on Workspace API level instead of each infrastructure?
I mean creating CheApiEnvVarProvider class that will inject CheApiInternalEnvVarProvider and provide its value as CHE_API. WDYT?

mkuznyetsov · 2018-04-24T21:42:57Z

ci-test

sleshchenko

LGTM

sleshchenko · 2018-04-25T06:58:11Z

...ava/org/eclipse/che/api/workspace/server/spi/provision/env/CheApiInternalEnvVarProvider.java

+
+  /**
+   * Returns Che API environment variable which should be injected into machines.
+   * Internal API URL is meant to be used from the inside of other machine containers.


I wouldn't use containers word here. I think other machines would be fine

codenvy-ci · 2018-04-25T07:24:13Z

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:9475
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

…e-9323

eclipse-che#9475)

mkuznyetsov added 4 commits April 11, 2018 10:17

Provide internal and external api url into workspaces

61e2780

Merge branch 'master' of github.com:eclipse/che into che-9323

3ff7e90

fixup! Merge branch 'master' of github.com:eclipse/che into che-9323

4e07ca8

fixup! fixup! Merge branch 'master' of github.com:eclipse/che into ch…

5b2695c

…e-9323

mkuznyetsov requested review from skabashnyuk and sleshchenko April 18, 2018 06:53

mkuznyetsov requested review from benoitf, garagatyi and riuvshin as code owners April 18, 2018 06:53

sleshchenko reviewed Apr 18, 2018

View reviewed changes

mkuznyetsov added 3 commits April 18, 2018 11:28

fixup! fixup! fixup! Merge branch 'master' of github.com:eclipse/che …

f1c129c

…into che-9323

fixup! fixup! fixup! fixup! Merge branch 'master' of github.com:eclip…

25133b1

…se/che into che-9323

fixup! fixup! fixup! fixup! fixup! Merge branch 'master' of github.co…

f42a5a8

…m:eclipse/che into che-9323

fixup! fixup! fixup! fixup! fixup! fixup! Merge branch 'master' of gi…

308c5a5

…thub.com:eclipse/che into che-9323

skabashnyuk reviewed Apr 18, 2018

View reviewed changes

Merge branch 'master' of github.com:eclipse/che into che-9323

b8fa831

test

8ddeb71

riuvshin requested a review from a user April 18, 2018 16:29

riuvshin requested a review from l0rd as a code owner April 18, 2018 16:29

Tets envs

9ee2ad7

Remove testing echo

aea61c2

benoitf added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/task Internal things, technical debt, and to-do tasks to be performed. labels Apr 19, 2018

sleshchenko reviewed Apr 19, 2018

View reviewed changes

mkuznyetsov added 5 commits April 24, 2018 09:59

Remove odd classes

58fffc9

fixup! Remove odd classes

ab94315

fixup! fixup! Remove odd classes

d64d787

fixup! fixup! fixup! Remove odd classes

fbffe2a

Merge branch 'master' of github.com:eclipse/che into che-9323

e9574ec

fixup! Merge branch 'master' of github.com:eclipse/che into che-9323

5984ec0

sleshchenko approved these changes Apr 25, 2018

View reviewed changes

garagatyi approved these changes Apr 25, 2018

View reviewed changes

fixup! fixup! Merge branch 'master' of github.com:eclipse/che into ch…

773de1e

…e-9323

AndrienkoAleksandr mentioned this pull request Apr 25, 2018

CHE-9265: Implementation Theia terminal plugin. eclipse/che-theia-terminal-extension#1

Merged

Merge branch 'master' of github.com:eclipse/che into che-9323

95da304

mkuznyetsov mentioned this pull request Apr 27, 2018

Add notes about API url environment variables in workspace containers eclipse-che/che-docs#401

Merged

skabashnyuk approved these changes Apr 27, 2018

View reviewed changes

Merge branch 'master' of github.com:eclipse/che into che-9323

08d85d9

mkuznyetsov merged commit b14ec23 into master Apr 27, 2018

mkuznyetsov deleted the che-9323 branch April 27, 2018 07:51

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 Apr 27, 2018

benoitf added this to the 6.5.0 milestone Apr 27, 2018

AndrienkoAleksandr mentioned this pull request Apr 27, 2018

Set correct che api external enviroment variable. Use official released workspace client. eclipse/che-theia-machines-plugin#2

Merged

hbhargav pushed a commit to hbhargav/che that referenced this pull request Dec 5, 2018

Add internal and external API URL environment variables for workspaces (

637f2d4

eclipse-che#9475)

skabashnyuk mentioned this pull request Jan 13, 2020

Can't set CHE_API_INTERNAL. #15663

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add internal and external API URL environment variables for workspaces #9475

Add internal and external API URL environment variables for workspaces #9475

mkuznyetsov commented Apr 18, 2018 •

edited

Loading

sleshchenko Apr 18, 2018

sleshchenko Apr 18, 2018

sleshchenko Apr 18, 2018

mkuznyetsov commented Apr 18, 2018

codenvy-ci commented Apr 18, 2018

mkuznyetsov commented Apr 18, 2018

codenvy-ci commented Apr 18, 2018

skabashnyuk Apr 18, 2018

skabashnyuk Apr 18, 2018

skabashnyuk Apr 18, 2018

riuvshin commented Apr 18, 2018

codenvy-ci commented Apr 18, 2018

riuvshin commented Apr 18, 2018

codenvy-ci commented Apr 18, 2018

riuvshin commented Apr 18, 2018

codenvy-ci commented Apr 18, 2018

riuvshin commented Apr 18, 2018

ghost commented Apr 18, 2018

codenvy-ci commented Apr 19, 2018

sleshchenko Apr 19, 2018

mkuznyetsov Apr 25, 2018

sleshchenko Apr 19, 2018

mkuznyetsov commented Apr 24, 2018

sleshchenko left a comment

sleshchenko Apr 25, 2018

codenvy-ci commented Apr 25, 2018

		@@ -51,7 +51,6 @@ CHE_LOGS_DIR=/logs
		CHE_WORKSPACE_LOGS=/logs/machines
		CHE_TEMPLATE_STORAGE=/data/templates

		@@ -67,6 +71,8 @@ protected void configure() {
		bind(RemoveProjectOnWorkspaceRemove.class).asEagerSingleton();

		bind(CheApiEnvVarProvider.class).to(KubernetesCheApiEnvVarProvider.class);

Add internal and external API URL environment variables for workspaces #9475

Add internal and external API URL environment variables for workspaces #9475

Conversation

mkuznyetsov commented Apr 18, 2018 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

Release Notes

Docs PR

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkuznyetsov commented Apr 18, 2018

codenvy-ci commented Apr 18, 2018

mkuznyetsov commented Apr 18, 2018

codenvy-ci commented Apr 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

riuvshin commented Apr 18, 2018

codenvy-ci commented Apr 18, 2018

riuvshin commented Apr 18, 2018

codenvy-ci commented Apr 18, 2018

riuvshin commented Apr 18, 2018

codenvy-ci commented Apr 18, 2018

riuvshin commented Apr 18, 2018

ghost commented Apr 18, 2018

codenvy-ci commented Apr 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkuznyetsov commented Apr 24, 2018

sleshchenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codenvy-ci commented Apr 25, 2018

mkuznyetsov commented Apr 18, 2018 •

edited

Loading