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

Adding support to spin up docker in a sibling container (from when Ka… #1603

Merged
merged 6 commits into from
May 22, 2021

Conversation

joelpramos
Copy link
Contributor

Description

Thanks for contributing this Pull Request. Make sure that you submit this Pull Request against the develop branch of this repository, add a brief description, and tag the relevant issue(s) and PR(s) below.

  • Relevant Issues : (compulsory)
  • Relevant PRs : (optional)
  • Type of change :
    • New feature
    • Bug fix for existing feature
    • Code quality improvement
    • Addition or Improvement of tests
    • Addition or Improvement of documentation

@joelpramos
Copy link
Contributor Author

if the following is used:
{ docker: 'ptrthomas/karate-chrome', showDriverLog: true, siblingContainer: true, useDockerHost: true };

An hostname that is different from localhost will be used by the Docker Target but also by the WebSocket URL. If useDockerHost is true then the host "host.docker.internal" will be used (to let docker use its own DNS resolver to resolve the container), otherwise the name of the container that was launched will be used. By default useDockerHost is false meaning that the container name will be used by default. This setting will depend on the Docker network in use.

Also solves for https://stackoverflow.com/questions/59426102/karateui-how-to-add-host-header-into-addoptions-during-the-driver-configurati by including the siblingContainer: true

See following for the implemented workarounds:

@joelpramos
Copy link
Contributor Author

Should be obvious but putting it here for future Googlers - you still need to solve (on your own) the strategy for deploying Karate in a container and providing it access to launch a sibling container. There are several security implications you will need to navigate but the solution that seems to be more accepted is to include the socket of the host into the container where your Karate is running to give it control of the host docker.

Few useful links:
https://devopscube.com/run-docker-in-docker/
https://tomgregory.com/running-docker-in-docker-on-windows/
https://stackoverflow.com/questions/47854463/docker-got-permission-denied-while-trying-to-connect-to-the-docker-daemon-socke

@joelpramos joelpramos marked this pull request as draft May 18, 2021 22:58
@ptrthomas
Copy link
Member

@joelpramos the docker part of the build seems to have failed

…f the socket URL is localhost. If the host/port are localhost no harm no foul. If we're dealing with a remote host (e.g. Device Farm) it shouldn't be localhost to begin with.
@ptrthomas
Copy link
Member

@joelpramos is this ready to merge ?

@joelpramos
Copy link
Contributor Author

@ptrthomas I just need to change the naming of the properties I introduced (e.g. remote target or something instead of sibling container).

Also need to re read the documentation to check whether there was a use case I didn't consider but I'm pretty certain this is fine as the new code is all conditional on that new property.

@joelpramos
Copy link
Contributor Author

I will get it done today

…will present regardless even if you are running your Chrome container somewhere else as found by some stackoverflow answers. This parameter will give control to override the initial header send to the web socket that gets the socket remote URL
… event test scripts are meant to not stop the container immediately but to allow implementation of mechanisms for cleanup of containers on a regular basis)
@joelpramos
Copy link
Contributor Author

@ptrthomas ready for merge.

There is a change for the way the ports are exposed. Instead of relying on the operative system to find us a free port and expose that in the container, we're now relying on docker to expose in a random available port and grabbing that port for use. This enables the use case of sibling containers (as to avoid port clashes).

In addition there are a few optional parameters for when using a chrome in a remote local (e.g. sibling docker container, chrome hosted in another machine etc.).

There are some limitations described here chromedp/chromedp#505 when the host is not localhost. The workaround is applied when setting the property remoteHost to true. I didn't see this as problematic when using the container name as hostname but when using host.docker.internal I came across this issue.

Using host.docker.internal relies on docker's internal DNS resolution - in my particular case as I was just trying out in a VM didn't setup a docker network and was just using the bridge. To overcome that I've added a second parameter useDockerHost. When set to true the host.docker.internal will be used, otherwise the container name (e.g. karate-chrome for http://karate-chrome:9222/json) will be used instead. By default this property is set to false as I assume most folks using containers will be running this in a proper docker network or they'll be using their local machines (in which case none of these parameters are needed).

Sample configuration the docker target:
{ docker: 'ptrthomas/karate-chrome', showDriverLog: true, remoteHost: true, useDockerHost: true };

PS: I only tested with Chrome... I assume that if the others don't use that HOST header property than no harm no foul but in any case the previous behavior did not change unless you are using these new optional parameters.

@joelpramos joelpramos marked this pull request as ready for review May 22, 2021 15:04
@ptrthomas ptrthomas merged commit 2cca3c0 into karatelabs:develop May 22, 2021
@joelpramos joelpramos mentioned this pull request May 23, 2021
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants