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

Use host's network stack, not container's. #111

Merged
merged 3 commits into from
May 4, 2023
Merged

Conversation

mario-campos
Copy link
Contributor

Currently, the Docker container uses a "bridged" network stack, which can have strange and unexpected behaviors when those network settings differ from the host's network settings.

For example, supplying an address to an private/internally-resolvable host will cause gh actions-importer to fail to resolve because it does not have those DNS settings.

What's changing?

This PR changes the way that the Docker container is executed by passing the --network=host flag, which will run this container using the host's network stack. In theory, this shouldn't break anything.

How's this tested?

Has not been tested yet.

Without using the host's network stack, the container may behave unexpectedly, such as failing to resolve internal addresses that the host _can_ resolve.
Use hosts's network stack by passing `--network=host` to `docker run`
@mario-campos mario-campos marked this pull request as ready for review April 27, 2023 19:45
@mario-campos mario-campos requested a review from a team as a code owner April 27, 2023 19:45
@mario-campos
Copy link
Contributor Author

Still testing. Will keep PR updated with status.

@stves stves removed their request for review May 2, 2023 17:22
@j-dunham
Copy link
Contributor

j-dunham commented May 4, 2023

I tested with Windows and MacOS and it worked as expected 🎉

@j-dunham
Copy link
Contributor

j-dunham commented May 4, 2023

🚨 Hi @mario-campos please do not merge. It needs to be coordinated with the merging of the labs PR so we do not break them. 🚨

@j-dunham j-dunham requested review from ethanis and Chaseshak May 4, 2023 19:43
@j-dunham j-dunham merged commit 19083bc into github:main May 4, 2023
4 checks passed
@andrewstec
Copy link

This is an awesome PR. Thank you so much.

If you don't mind me asking, will this functionality be released to Github's Self-hosted runner? If so, what version?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants