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.

Sign up for GitHub

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

Jump to bottom

Connectivity test + refactoring #2057

Merged
merged 7 commits into from
Aug 8, 2016
Merged

Connectivity test + refactoring #2057

merged 7 commits into from
Aug 8, 2016

Conversation

TylerJewell
Copy link

What does this PR do?

  1. Improves performance by limiting calls to docker.
  2. Adds new networking connectivity test
  3. Publishes "che debug" command with different options

Docs will be added.

Connectivity test + refactoring
2fa7a0e 
1. Added connectivity test to test certain scenarios
2. Added refactoring to make script run faster in debugging scenarios.
@TylerJewell
Copy link
Author

@l0rd @dmytro-ndp - this has an upgraded version of the connectivity test.

@TylerJewell TylerJewell mentioned this pull request Aug 5, 2016
Closed
@codenvy-ci
Copy link

Build finished.
Build success. $BUILD_URL

@codenvy-ci
Copy link

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

@garagatyi
Copy link

Are these exports really needed?

Tyler Jewell added 2 commits August 6, 2016 09:43
Fixup - env variables with spaces
8996b92 
1. Let the user pass environment variables that have strings with spaces.
2. Added pass through of http_proxy, https_proxy, and no_proxy from system into che-launcher
3. Cleaned up various formatting issues
Fix export
de1464c 
Removed unnecessary export tgs
@TylerJewell
Copy link
Author

@garagatyi - only the last two exports are required. The others are not - and they have been removed.

@codenvy-ci
Copy link

Build finished.
Build success. $BUILD_URL

@codenvy-ci
Copy link

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

@TylerJewell
Copy link
Author

Also added:

  1. Consolidated che info and che debug into a single command line offering.
  2. Fixed error where docker run was failing, so commands after docker run syntax were skipped by CLI.
  3. Moved all environment variables into temporary files and used --env-file which respects variables with spaces better.
  4. Added support to che-launcher for inheriting environment variables.

@TylerJewell TylerJewell mentioned this pull request Aug 6, 2016
Closed
@codenvy-ci
Copy link

Build finished.
Build success. $BUILD_URL

@codenvy-ci
Copy link

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

@codenvy-ci
Copy link

Build finished.
Build success. $BUILD_URL

@codenvy-ci
Copy link

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

@l0rd
Copy link
Contributor

l0rd commented Aug 8, 2016

I've tested on Mac with Docker 4 Mac where we still have issue #1644. In this case the expected behaviour should be:

DEBUG: Browser => Workspace Agent : Connection failed

Instead the command hangs for some minutes and then return without showing the test results:

image

This is because the curl command silently fails :
image

To fix that I would use curl option --connect-timout 5 to limit the timeout to 5 seconds and check for curl return value to decide if the connection succeeded or not.

@l0rd
Copy link
Contributor

l0rd commented Aug 8, 2016

The second test in run_connectivity_test(), (TEST 1a: Simulate browser (websockets)) does not add any value IMO.

In other words run_connectivity_test() tests Che connectivity at IP/TCP layer not at application layer.

@TylerJewell
Copy link
Author

@l0rd @riuvshin @tolusha

  1. Removed shell incompatibility from che.sh
  2. Removed container self destruct
  3. Fixed the get_hostname function to always return localhost unless it is boot2docker
  4. Fixed network connectivity tests for mac - added in extra hostname test there as well

This was referenced Aug 8, 2016
Closed
Closed
@TylerJewell TylerJewell merged commit ae639ef into master Aug 8, 2016
@TylerJewell TylerJewell deleted the che-cli-connectivity branch August 8, 2016 19:12
@codenvy-ci
Copy link

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

JPinkney pushed a commit to JPinkney/che that referenced this pull request Aug 17, 2017
Connectivity test + refactoring (eclipse-che#2057)
cec6d09 
* Connectivity test + refactoring
1. Added connectivity test to test certain scenarios
2. Added refactoring to make script run faster in debugging scenarios.
* Added websocket test
* Fixup - env variables with spaces
1. Let the user pass environment variables that have strings with spaces.
2. Added pass through of http_proxy, https_proxy, and no_proxy from system into che-launcher
3. Cleaned up various formatting issues

* Fix export

Removed unnecessary export tgs

* Docker env fixes

* boot2docker warning

* Numerous fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@TylerJewell @codenvy-ci @garagatyi @l0rd