CHE-1489: rework workspaces folder usage to fix Che on some configs #1733

garagatyi · 2016-07-14T15:02:59Z

What does this PR do?

Fixes storing FS of workspaces between Che runs in case Che runs in the container.
Removes unneeded provider.

What issues does this PR fix or reference?

Previous Behavior

Che tried to create folders of projects roots of workspace on start of new workspace.
So path of projects root of workspace has to match in Che container and on docker host.

New Behavior

It is possible to define property or environment variable to disable creation of workspace folder by Che.
In that case docker will create folders mounted to workspace container on host. This folder will have root permissions.

Tests written?

Old tests were fixed. No new tests are added yet.

Docs requirements?

Include the content for all the docs changes required.
New instructions on how to run Che in container would be useful.

Signed-off-by: Alexander Garagatyi <[email protected]>

garagatyi · 2016-07-14T15:03:47Z

@eivantsov @TylerJewell @evoevodin @sleshchenko @mmorhun Please review

TylerJewell · 2016-07-14T15:09:49Z

@garagatyi - please document the property name & how it can be configured.

TylerJewell · 2016-07-14T15:36:13Z

@eivantsov - FYI, I am unable to build a docker image for this on a windows box right now. There seems to be some problems with docker for windows and CRLF for newlines. Can you build an image that has this change and put it on docker hub for testing?

codenvy-ci · 2016-07-14T18:44:12Z

Build success. http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/1214/

…nfigs Signed-off-by: Alexander Garagatyi <[email protected]>

mmorhun · 2016-07-15T08:59:35Z

+1

ghost · 2016-07-15T09:23:49Z

@TylerJewell here's how you may try the fix:

docker run --net=host -v /var/run/docker.sock:/var/run/docker.sock -v /c/Users/Tyler/che-storage:/c/Users/Tyler/che-storage -v $(pwd):/home/user/.che -e "CHE_LOCAL_CONF_DIR=/home/user/.che" -e "CHE_ALLOW_CREATION_WORKSPACE_FOLDERS=false" eivantsov/che1

In the current directory, create che.properties file with che.user.workspaces.storage=/c/Users/Tyler/che-storage

voievodin · 2016-07-15T11:05:53Z

.../eclipse/che/plugin/docker/machine/local/node/provider/LocalWorkspaceFolderPathProvider.java

+            final Path cheHome = WindowsHostUtils.ensureCheHomeExist(createFolders);
+            final Path vfs = cheHome.resolve("vfs");
+            if (createFolders) {
+                if (!vfs.toFile().mkdir()) {


There is no need to use nested if statement

voievodin · 2016-07-15T11:10:13Z

LGTM

…some configs Signed-off-by: Alexander Garagatyi <[email protected]>

garagatyi · 2016-07-15T11:15:05Z

@TylerJewell Are you OK with that PR? Can I merge it?

TylerJewell · 2016-07-15T11:21:39Z

@garagatyi:

What is the che.props entry for the same env variable?
Please change CHE_ALLOW_CREATION_WORKSPACE_FOLDERS to CHE_CREATE_WS_FOLDERS.

ghost · 2016-07-15T11:22:07Z

che.user.workspaces.storage.create_folders

…Che on some configs Signed-off-by: Alexander Garagatyi <[email protected]>

garagatyi · 2016-07-15T11:24:21Z

Renamed CHE_ALLOW_CREATION_WORKSPACE_FOLDERS to CHE_CREATE_WS_FOLDERS

TylerJewell · 2016-07-15T11:54:31Z

@garagatyi @eivantsov - it failed for me. Server was unable to boot and gave an error message in the console below.

docker run --net=host --rm --name=che^
 -v /c/users/tyler/che:/c/users/tyler/che^
 -v /c/codenvy:/home/user/.che^
 -e "CHE_LOCAL_CONF_DIR=/home/user/.che"^
 -e "CHE_ALLOW_CREATION_WORKSPACE_FOLDERS=false"^
 -e "DOCKER_HOST=tcp://10.0.75.2:2375"^
 eivantsov/che1

2016-07-15 11:51:05,075[ost-startStop-1]  [INFO ] [o.a.c.startup.HostConfig 910]        - Deploying web application archive /home/user/che/tomcat/webapps/wsmaster.war
2016-07-15 11:51:08,157[ost-startStop-1]  [WARN ] [p.DockerExtConfBindingProvider 51]   - DockerExtConfBindingProvider
2016-07-at org.eclipse.che.plugin.docker.client.InitialAuthConfig.getRegistryPrefix(InitialAuthConfig.java:99)ntext initialized event
 to listat org.eclipse.che.plugin.docker.client.InitialAuthConfig.lambda$new$0(InitialAuthConfig.java:73)
com.googat java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)rors:
        at java.util.HashMap$EntrySpliterator.forEachRemaining(HashMap.java:1691)
1) Errorat java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)rty 'docker.registry.auth.password' is missing '.'. V
alid creat java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
  at orgat java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)java:68)
  at orgat java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)fig.java:48)
  while at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
    for at org.eclipse.che.plugin.docker.client.InitialAuthConfig.<init>(InitialAuthConfig.java:74)t>(DockerConnectorConfiguration.ja
va:123) at org.eclipse.che.plugin.docker.client.InitialAuthConfig$$FastClassByGuice$$6c93a19d.newInstance(<generated>)
  while at com.google.inject.internal.DefaultConstructionProxyFactory$FastClassProxy.newInstance(DefaultConstructionProxyFactory.java:89)for the 1st parameter of org.eclipse.che.plugin.docker.client.DockerConnector.<init>(DockerConnector.java:143)
  at orgat com.google.inject.internal.ConstructorInjector.provision(ConstructorInjector.java:111)

garagatyi · 2016-07-15T11:58:59Z

Please send me privately your external conf file

TylerJewell · 2016-07-15T12:35:30Z

@garagatyi @eivantsov - Ok, got things to work after some reconfiguration. We will have to make it really clear for users that they may have to reset their docker for windows credentials to gain access otherwise they may get exception failures.

Can we please change the default che.properties file that ships to have the following comment modified.

# This property works for Docker for Windows, Mac, and Linux.
# On windows with virtualbox, this property is ignored.
che.user.workspaces.storage=/c/users/tyler/che/workspaces
che.user.workspaces.storage.create_folders=false

Can we also rename these two properties to:

che.workspace.storage
che.workspace.storage.create_folders

The run syntax that finally worked:

docker run --net=host --rm --name=che^
 -v /home/user/che/lib:/home/user/che/lib-copy^
 -v /c/users/tyler/che/workspaces:/c/users/tyler/che/workspaces^
 -v /c/users/tyler/che/storage:/home/user/che/storage^
 -v /c/codenvy:/home/user/.che^
 -e "CHE_LOCAL_CONF_DIR=/home/user/.che"^
 -e "CHE_ALLOW_CREATION_WORKSPACE_FOLDERS=false"^
 -e "DOCKER_HOST=tcp://10.0.75.2:2375"^
 eivantsov/che1

garagatyi · 2016-07-15T12:44:00Z

@TylerJewell here is current comment

### Storage and user configuration
# The location where your workspaces (and their projects) are stored.
# This property is ignored on Windows due to limitations with VirtualBox
# On Windows, all workspaces stored in c:\%userprofile%\AppData\Local\Eclipse Che\
che.user.workspaces.storage=${che.home}/workspaces

Do you want to have next comment instead?

### Storage and user configuration
# The location where your workspaces (and their projects) are stored.
# This property works for Docker for Windows, Mac, and Linux.
# On windows with virtualbox, this property is ignored. Instead of that all workspaces stored in c:\%userprofile%\AppData\Local\Eclipse Che\

garagatyi · 2016-07-15T12:46:50Z

If we rename property che.user.workspaces.storage then users with configured external property files will miss the projects after upgrade. Are you OK with that?

TylerJewell · 2016-07-15T12:47:30Z

@garagatyi

### Storage and user configuration
# The location where your workspaces (and their projects) are stored.
# This property works for Docker for Windows, Mac, and Linux.
# On virtualbox for windows, this property is ignored. Instead, they are stored at:
# c:\%userprofile%\AppData\Local\Eclipse Che\

TylerJewell · 2016-07-15T12:48:14Z

@garagatyi - yes, ok with the breaking change. Is there anyway we can quietly support the old configuration in our code? So that there are two properties for the same value? We will only document and show the new one but quietly honor the old?

garagatyi · 2016-07-15T12:49:17Z

We will only document and show the new one but quietly honor the old?

Yes, it is possible

TylerJewell · 2016-07-15T13:04:09Z

@garagatyi - ok, after changes of comments and property names, +1 from me - ok to merge.

garagatyi · 2016-07-15T13:19:54Z

@TylerJewell What then do if both old and new properties are set?

TylerJewell · 2016-07-15T13:32:16Z

@garagatyi - accept new one, I think.

garagatyi · 2016-07-15T13:33:51Z

Looks like we will have to accept the old one because it will be the case when user overrided old property and then upgraded Che.

codenvy-ci · 2016-07-15T14:40:51Z

Build success. http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/1222/

…to fix Che on some configs Signed-off-by: Alexander Garagatyi <[email protected]>

sleshchenko · 2016-07-15T16:04:31Z

.../eclipse/che/plugin/docker/machine/local/node/provider/LocalWorkspaceFolderPathProvider.java

-        checkProps(workspacesMountPoint, hostProjectsFolder);
+        this.isWindows = SystemInfo.isWindows();
+        String allowFoldersCreationEnvVar = System.getenv(ALLOW_FOLDERS_CREATION_ENV_VARIABLE);
+        if (allowFoldersCreationEnvVar != null) {


You can remove this check. Or merge with next by &&
I prefer first variant

sleshchenko · 2016-07-15T16:07:46Z

LGTM

TylerJewell · 2016-07-15T16:09:48Z

@garagatyi - did this improvement remove the need for the volume mount? If so, once eugene creates a new image, I can test. Otherwise LGTM.

… usage to fix Che on some configs Signed-off-by: Alexander Garagatyi <[email protected]>

garagatyi · 2016-07-15T16:14:21Z

Yes. You don't have to mount workspaces folder into Che container if folders creation is disabled (e.g. in your case).

TylerJewell · 2016-07-15T18:19:25Z

@garagatyi - can we add an environment variable for che.workspace.storage property named CHE_WORKSPACE_STORAGE? By having both properties available as environment variables, we can configure both of these with -e commands without the user having to create a custom che.properties file. So right now, Windows users will always have to create a custom properties file and we should give them environment variables only.

Also, to start the process of more consistency - I updated the name of the properties and the environment variables that match. Sorry for moving this around. I had to write some docs on this and get a feel before I could settle in on a standard.

che.workspace.storage
che.workspace.storage.create_folders

CHE_WORKSPACE_STORAGE
CHE_WORKSPACE_STORAGE_CREATE_FOLDERS

TylerJewell · 2016-07-15T18:26:54Z

Documentation update:
https://eclipse-che.readme.io/v4.4/docs/usage-docker-for-windows

codenvy-ci · 2016-07-15T18:37:49Z

Build # 1227 - FAILED

Please check console output at http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/1227/ to view the results.

… folder usage to fix Che on some configs Signed-off-by: Alexander Garagatyi <[email protected]>

codenvy-ci · 2016-07-18T07:54:29Z

Build # 1228 - FAILED

Please check console output at http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/1228/ to view the results.

garagatyi · 2016-07-18T10:32:41Z

Now mounting /workspaces folder into Che container is not needed. storage path can be set with env variable

CHE-1489: rework workspaces folder usage to fix Che on some configs

4619733

Signed-off-by: Alexander Garagatyi <[email protected]>

fixup! CHE-1489: rework workspaces folder usage to fix Che on some co…

f0b88bc

…nfigs Signed-off-by: Alexander Garagatyi <[email protected]>

voievodin reviewed Jul 15, 2016
View reviewed changes

fixup! fixup! CHE-1489: rework workspaces folder usage to fix Che on …

7fe692f

…some configs Signed-off-by: Alexander Garagatyi <[email protected]>

fixup! fixup! fixup! CHE-1489: rework workspaces folder usage to fix …

0b0b8b9

…Che on some configs Signed-off-by: Alexander Garagatyi <[email protected]>

This was referenced Jul 15, 2016

Unable to create workspaces #240

Closed

how can I get the same workspace which I was working on when I restart Che server. #779

Closed

fixup! fixup! fixup! fixup! CHE-1489: rework workspaces folder usage …

b167d27

…to fix Che on some configs Signed-off-by: Alexander Garagatyi <[email protected]>

sleshchenko reviewed Jul 15, 2016
View reviewed changes

fixup! fixup! fixup! fixup! fixup! CHE-1489: rework workspaces folder…

0de9376

… usage to fix Che on some configs Signed-off-by: Alexander Garagatyi <[email protected]>

fixup! fixup! fixup! fixup! fixup! fixup! CHE-1489: rework workspaces…

b26d654

… folder usage to fix Che on some configs Signed-off-by: Alexander Garagatyi <[email protected]>

garagatyi merged commit 3eb38cd into master Jul 18, 2016

garagatyi deleted the CHE-1489 branch July 18, 2016 10:03

TylerJewell modified the milestone: 4.6.0 Jul 22, 2016

TylerJewell mentioned this pull request Jul 22, 2016

How to restore workspaces when restarting Che? #1561

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CHE-1489: rework workspaces folder usage to fix Che on some configs #1733

CHE-1489: rework workspaces folder usage to fix Che on some configs #1733

garagatyi commented Jul 14, 2016

garagatyi commented Jul 14, 2016

TylerJewell commented Jul 14, 2016

TylerJewell commented Jul 14, 2016

codenvy-ci commented Jul 14, 2016

mmorhun commented Jul 15, 2016

ghost commented Jul 15, 2016 •

edited by ghost

Loading

voievodin Jul 15, 2016 •

edited

Loading

voievodin commented Jul 15, 2016

garagatyi commented Jul 15, 2016

TylerJewell commented Jul 15, 2016

ghost commented Jul 15, 2016

garagatyi commented Jul 15, 2016 •

edited by TylerJewell

Loading

TylerJewell commented Jul 15, 2016

garagatyi commented Jul 15, 2016

TylerJewell commented Jul 15, 2016

garagatyi commented Jul 15, 2016

garagatyi commented Jul 15, 2016

TylerJewell commented Jul 15, 2016

TylerJewell commented Jul 15, 2016

garagatyi commented Jul 15, 2016 •

edited

Loading

TylerJewell commented Jul 15, 2016

garagatyi commented Jul 15, 2016

TylerJewell commented Jul 15, 2016

garagatyi commented Jul 15, 2016

codenvy-ci commented Jul 15, 2016

sleshchenko Jul 15, 2016

sleshchenko commented Jul 15, 2016

TylerJewell commented Jul 15, 2016

garagatyi commented Jul 15, 2016

TylerJewell commented Jul 15, 2016

TylerJewell commented Jul 15, 2016

codenvy-ci commented Jul 15, 2016

codenvy-ci commented Jul 18, 2016

garagatyi commented Jul 18, 2016

CHE-1489: rework workspaces folder usage to fix Che on some configs #1733

CHE-1489: rework workspaces folder usage to fix Che on some configs #1733

Conversation

garagatyi commented Jul 14, 2016

What does this PR do?

What issues does this PR fix or reference?

Previous Behavior

New Behavior

Tests written?

Docs requirements?

garagatyi commented Jul 14, 2016

TylerJewell commented Jul 14, 2016

TylerJewell commented Jul 14, 2016

codenvy-ci commented Jul 14, 2016

mmorhun commented Jul 15, 2016

ghost commented Jul 15, 2016 • edited by ghost Loading

voievodin Jul 15, 2016 • edited Loading

Choose a reason for hiding this comment

voievodin commented Jul 15, 2016

garagatyi commented Jul 15, 2016

TylerJewell commented Jul 15, 2016

ghost commented Jul 15, 2016

garagatyi commented Jul 15, 2016 • edited by TylerJewell Loading

TylerJewell commented Jul 15, 2016

garagatyi commented Jul 15, 2016

TylerJewell commented Jul 15, 2016

garagatyi commented Jul 15, 2016

garagatyi commented Jul 15, 2016

TylerJewell commented Jul 15, 2016

TylerJewell commented Jul 15, 2016

garagatyi commented Jul 15, 2016 • edited Loading

TylerJewell commented Jul 15, 2016

garagatyi commented Jul 15, 2016

TylerJewell commented Jul 15, 2016

garagatyi commented Jul 15, 2016

codenvy-ci commented Jul 15, 2016

sleshchenko Jul 15, 2016

Choose a reason for hiding this comment

sleshchenko commented Jul 15, 2016

TylerJewell commented Jul 15, 2016

garagatyi commented Jul 15, 2016

TylerJewell commented Jul 15, 2016

TylerJewell commented Jul 15, 2016

codenvy-ci commented Jul 15, 2016

codenvy-ci commented Jul 18, 2016

garagatyi commented Jul 18, 2016

ghost commented Jul 15, 2016 •

edited by ghost

Loading

voievodin Jul 15, 2016 •

edited

Loading

garagatyi commented Jul 15, 2016 •

edited by TylerJewell

Loading

garagatyi commented Jul 15, 2016 •

edited

Loading