Che nightly does not start on Windows 10

[ghost]

Syntax used:

docker run -ti -v /c/Users/Codenvy/che-conf:/data -v /var/run/docker.sock:/var/run/docker.sock eclipse/che-cli:nightly start --pull

When starting a workspace, server and agent can connect to each other, but the browser cannot reach workspace agent:

For whatever reason, it tries to connect to VM's docker0 network interface IP instead of the VM IP which is 10.0.75.2

Playing with CHE_HOST did not help.

It does not happen with eclipse/che-cli:latest

[TylerJewell]

@eivantsov - I have the same issue on my Windows 10 Home OS system as well. Confirmed.

[garagatyi]

@eivantsov Have you tried 5.0.0? nightly now uses 5.1.0, so can you check if it persists on 5.0.0 to identify milestone?

[riuvshin]

reproduced on mac

[TylerJewell]

@garagatyi - ok with 5.0.0. So this issue is only on nightly - it's not on our tagged versions.

[riuvshin]

@TylerJewell @garagatyi confirmed on mac 5.0.0 tag - works. that means that we will not have this bug in 5.0.1 because we don't cherry-pick those changes.

[TylerJewell]

Great - dropped the milestone marker.

[riuvshin]

oki but we must have this for 5.1.0

[ghost]

@garagatyi and myself have found the problem. By default CHE_DOCKER_IP_EXTERNAL is identical to CHE_DOCKER_IP which is calculated as IP of docker0 network interface.

So, in CLI we should detect if it's Mac or Windows and set CHE_DOCKER_IP_EXTERNAL to be localhost or VM IP. But since Docker does all the routing itself, localhost is totally ok.

I have also replied here - #3657 (comment)

[skabashnyuk]

@benoitf @TylerJewell can you take a look if it's possible to fix in CLI?

[riuvshin]

@skabashnyuk are you sure it is CLI issue?

[garagatyi]

As @eivantsov described we don't set external ip property so it is proper for product to use internal IP in that case. If external IP is set everything works

[TylerJewell]

Can you explain why this issue does not happen for 5.0 but is happening for nightly?

I don't believe this is a cli issue and instead it is an issue for eclipse/che-server which is where all os and vm detection occurs

[riuvshin]

yeah I have exactly same feeling that this is not CLI issue because it works for 5.0.0 tag

[garagatyi]

We have changes in code that probably changed something. It was quite complex refactoring. QA team didn't find the problem since it doesn't use non-Linux OSs for QA cycle for now.

Since Che runs in container it is logical that it doesn't detect OS of host system. And CLI sequence can do OS related checks and put appropriate preperties in che.env.

[garagatyi]

I reopened this issue since fix that @benoitf pushed is not complete. I have needed changes but they are not checked yet.
If you think that fix from @benoitf is enough to make it not blocker you can move this issue to the next milestone

[benoitf]

@garagatyi I don't see different technical changes in your PR.
Also AFAIK docker che launcher is not maintained.

[garagatyi]

I think there are few:
https://github.com/eclipse/che/compare/master...garagatyi:CHE-3686?expand=1#diff-1da4013f60e1b92b5613cf9680423a88R84
https://github.com/eclipse/che/compare/master...garagatyi:CHE-3686?expand=1#diff-4c85837520f3c25f0f4f10f1243c2f5bR99
https://github.com/eclipse/che/compare/master...garagatyi:CHE-3686?expand=1#diff-50536b2139a639ba6012158866aa5e7dR168
https://github.com/eclipse/che/compare/master...garagatyi:CHE-3686?expand=1#diff-eae0d593d9d11b19a0c27f9e468d957bR67

[garagatyi]

If launcher is not maintained can you create an issue to remove it? Can you point in that issue what files should be removed. Thanks.

[benoitf]

@garagatyi AFAIK in the changes you showed,
che.sh is part of the native launch of che (which doesn't work on windows or Mac so it's not related to this issue. (And BTW I think it's gonna be removed when CLI was stable enough) : #3318 (comment)

about che-launcher : #3423 (comment)

[garagatyi]

So, what about
https://github.com/eclipse/che/compare/master...garagatyi:CHE-3686?expand=1#diff-1da4013f60e1b92b5613cf9680423a88R84
https://github.com/eclipse/che/compare/master...garagatyi:CHE-3686?expand=1#diff-eae0d593d9d11b19a0c27f9e468d957bR67

[benoitf]

for the 1st one I would expect it's an issue in che aliases
for the second one it's a refactoring

[garagatyi]

I just made code behaves as it did before we merged PR with refactoring.
Can you elaborate on what do you want to do? Do you want me to close issue and don't do these changes?

[benoitf]

no there are interesting doc updates.
Also you should have good reason for the code refactoring.
I'm just saying that the issue is already solved without these nice improvements

[garagatyi]

I would not count it as a real refactoring. I moved methods a bit, but it is just small code cleanup.
And I changed behavior of default server address evaluation strategy to match previous behavior. Unfortunately I didn't notice that it was changed in that PR. So now I just changed it back to no increase support tickets number because of changes in Che behavior.
As I said if issue with Che on Windows and Mac is fixed by your commit you can move this issue to next milestone.
But since this issue is assigned to me and I spent my time on investigation which commit broke Che and why I against closing this issue because of your fix.

[riuvshin]

@benoitf @garagatyi so guys what we are going to do? this is issue is still blocker for 5.1.0. should we remove blocker and move this to next ver after @benoitf changes? or we still wait something for 5.1.0?

[TylerJewell]

1. Remove the blocker because the product is usable without @garagatyi fixes.
2. Drop any milestone
3. Then let TLs and @garagatyi decide scheduling of other implementation.

@garagatyi - can you please rework the description to describe the work to be done now? It describes as a bug, but now that it's a different kind of issue, so let's just be explicit on what to do now vs. what we had to do before.

[amisevsk]

@garagatyi +1 on your changes. In #3282, I didn't realize that the environment variables were still used to set parameters, and likely the only reason the code worked in 5.0.0 was that LocalDockerInstanceRuntimeInfo was counterintuitively using those env vars to override properties. I only saw this issue now, sorry I couldn't add input earlier.

While it sounds like this is not a blocker for 5.1.0, I think it is still a good change to make, for correctness' sake. Those env vars should have been removed completely when the switch to defining everything in che.properties (and now che.env) was made. Env vars CHE_DOCKER_IP and CHE_DOCKER_IP_EXTERNAL should have the exact same effect as CHE_DOCKER_MACHINE_HOST and CHE_DOCKER_MACHINE_HOST_EXTERNAL did in 5.0.0. Further, this issue likely means that the property che.docker.ip.external did not have an effect in 5.0.0 when CHE_DOCKER_MACHINE_HOST_EXTERNAL was set.

[garagatyi]

@amisevsk I agree with you. I also didn't notice that.

[garagatyi]

Should be fixed already