Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add networking pre-flight check #3756

Merged
merged 8 commits into from
Jan 23, 2017
Merged

Add networking pre-flight check #3756

merged 8 commits into from
Jan 23, 2017

Conversation

TylerJewell
Copy link

@TylerJewell TylerJewell commented Jan 17, 2017

What does this PR do?

  1. A working branch that includes the epic improvements of CHE-3748, an epic dedicated to detecting possible networking issues earlier in the startup sequence, interrupting operating, and providing helpful feedback to end users.

  2. Preflight check that performs two sets of tests: a simulation of browser => agent and a simulation of Che server => agent. In the first set of tests, they check both localhost and external IP connectivity and only a single test must pass. In the second set, we test connectivity from Che using both Docker internal and external IP addresses and both must pass for Che to continue. Sample output in the start sequence:

INFO: (che start): Preflight checks
         port 8080 (http):       [AVAILABLE]
         conn (browser => ws):   [NOT OK]
         conn (server => ws):    [OK]

ERROR: Try 'docker run <options> eclipse/che:nightly info --network' for more tests.
  1. Now skips preflight checks if user provides --fast command.

  2. Updates the info --network command to make use of the same test framework.

To Be Included - post-flight checks that run in the background of the Che server after the server has successfully started to test creating a minimal workspace, with small workspace agent, and test ws=>agent connectivity.

To Be Included - a UD extension that adds "Diagnostics" button to the bottom next to "docs" that will enable a set of tests to be run from a user's browser to test web sockets, open workspace ports, and ws agent connectivity.

To Be Included - smarter set of error messages that tell users what to check or what to configure to overcome a particular issue.

I believe that this issue will require a matching Codenvy issue as it has a custom cmd_start with its own form of port checks. Can we reuse Che's utility function?

What issues does this PR fix or reference?

#3748

Changelog

Adds CLI preflight check that simulates connections from browser and server to agent
Adds ability to apply --fast to skip preflight checks upon boot

Release Notes

We have added two additional preflight checks to the startup sequence that creates simulated connectiosn from a browser to a workspace and from Che to a workspace. These connections are simulated using short-lived Docker containers that we launch to test a connection. These connections will use some of your host's native networking and can give an early indication if something is not configured properly. You can skip these tests with --fast on the command line.

You will see the tests run during boot with the following output. If any of the tests fail, you can get more details by running the info --network command, and the output of each simulation will be provided.

conn (browser => ws):   [OK]"
conn (server => ws):    [OK]"

Docs PR

Nothing to add to docs on this

@TylerJewell TylerJewell added kind/enhancement A feature request - must adhere to the feature request template. status/in-progress This issue has been taken by an engineer and is under active development. status/open-for-dev An issue has had its specification reviewed and confirmed. Waiting for an engineer to take it. labels Jan 17, 2017
@codenvy-ci
Copy link

@benoitf
Copy link
Contributor

benoitf commented Jan 18, 2017

I've error with command when my /data mounted folder does not exists:

docker run -it --rm -v /tmp/che-cli:/data -v /var/run/docker.sock:/var/run/docker.sock eclipse/che-cli:nightly start --fast
docker: open /data/che.env: no such file or directory.
See 'docker run --help'.
INFO: Proxy: HTTP_PROXY=, HTTPS_PROXY=, NO_PROXY=*.local, 169.254/16
WARN: Skipping dockerhub network check...
INFO: (che cli): Loading cli...
WARN: Skipping nightly image check...
WARN: Skipping version compatibility check...
docker: open /data/che.env: no such file or directory.
See 'docker run --help'.
WARN: (che init): 'nightly' installations cannot be upgraded to non-nightly versions

preflight check is working

         conn (browser => ws):   [OK]
         conn (server => ws):    [OK]

info --network is failing on the first test on my side : Browser => Workspace Agent (localhost): Connection failed

NFO: Proxy: HTTP_PROXY=, HTTPS_PROXY=, NO_PROXY=*.local, 169.254/16
INFO: (che cli): Checking network... (hint: '--fast' skips version, network, preflight and nightly checks)
INFO: (che cli): Loading cli...
INFO: (che cli): Checking registry for version 'nightly' images
INFO: 
INFO: ---------------------------------------
INFO: --------   CONNECTIVITY TEST   --------
INFO: ---------------------------------------
INFO: Browser    => Workspace Agent (localhost): Connection failed
INFO: Browser    => Workspace Agent (192.168.65.2): Connection succeeded
INFO: Server     => Workspace Agent (External IP): Connection succeeded
INFO: Server     => Workspace Agent (Internal IP): Connection succeeded

@TylerJewell
Copy link
Author

Yes, the localhost test fails for Mac and Windows in the default configuration. When we do preflight check, we do an OR test of the first two - we only need a single one to work properly.

We should fail if :/data is not present. So that may be a bug.

@benoitf
Copy link
Contributor

benoitf commented Jan 18, 2017

I mean :/data is volume mounted but the directory on the host does not exists yet. AFAIK until now it was created by the CLI in init phase

@TylerJewell
Copy link
Author

I think we hvae always assumed that the directory mounted to :/data exists already. I don't think we have taken the action to create that folder if it doesn't exist. I think we should stop with error message if it isn't there and ask user to create it first.

@benoitf
Copy link
Contributor

benoitf commented Jan 18, 2017

I have the same error if the folder is created but is empty (I think it's the default case of users)

@benoitf
Copy link
Contributor

benoitf commented Jan 18, 2017

$  mkdir /tmp/che-cli
$ docker run -it --rm -v /tmp/che-cli:/data -v /var/run/docker.sock:/var/run/docker.sock eclipse/che-cli:nightly start
WARNING: No kernel memory limit support
INFO: Proxy: HTTP_PROXY=, HTTPS_PROXY=, NO_PROXY=*.local, 169.254/16
INFO: (che cli): Checking network... (hint: '--fast' skips version, network, preflight and nightly checks)
INFO: (che cli): Loading cli...
docker: open /data/che.env: no such file or directory.
See 'docker run --help'.

@TylerJewell
Copy link
Author

Looks like something is trying to access the environment file before the CLI has been fully bootstrapped. Will investigate!

@benoitf
Copy link
Contributor

benoitf commented Jan 18, 2017

it seems it's not linked to this PR

@codenvy-ci
Copy link

Tyler Jewell added 4 commits January 22, 2017 09:43
Signed-off-by: Tyler Jewell <tjewell@codenvy.com>
Signed-off-by: Tyler Jewell <tjewell@codenvy.com>
@codenvy-ci
Copy link

@TylerJewell TylerJewell changed the title WIP: Add networking pre-flight, post-flight, and inside-the-UD checks Add networking pre-flight check Jan 23, 2017
@TylerJewell TylerJewell removed status/in-progress This issue has been taken by an engineer and is under active development. status/open-for-dev An issue has had its specification reviewed and confirmed. Waiting for an engineer to take it. labels Jan 23, 2017
@TylerJewell TylerJewell added this to the 5.2.0 milestone Jan 23, 2017
@TylerJewell TylerJewell merged commit c749231 into master Jan 23, 2017
@TylerJewell TylerJewell deleted the CHE-3748 branch January 23, 2017 20:16
@codenvy-ci
Copy link

JPinkney pushed a commit to JPinkney/che that referenced this pull request Aug 17, 2017
Signed-off-by: Tyler Jewell <tjewell@codenvy.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants