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 support to multi host port binding #116

Merged
merged 7 commits into from
Sep 9, 2024
Merged

Add support to multi host port binding #116

merged 7 commits into from
Sep 9, 2024

Conversation

rgiuse
Copy link
Contributor

@rgiuse rgiuse commented Sep 1, 2024

Add support to multi host port binding.

Add support to multiple host port binding
@lavie
Copy link
Owner

lavie commented Sep 2, 2024

Thank you for this enhancement. Please add test coverage for this new behavior so that it can be merged.

@rgiuse
Copy link
Contributor Author

rgiuse commented Sep 4, 2024

New test case
Added support for recorded fixure in test

@lavie
Copy link
Owner

lavie commented Sep 4, 2024

Thank you for adding test coverage. I must ask, though, that you use the existing style of fixture in the project using the minimal set of command line switches. As it is, the 6th fixture you've added has over 300 lines of configuration and nearly all of them are irrelevant to the project and the test case it's meant to cover.

@rgiuse
Copy link
Contributor Author

rgiuse commented Sep 4, 2024

You typically use a standard fixture style, where a running Docker container is generated to perform tests directly.

In this case, I need to bind two different existing IP addresses of your machine to the same port:

One is the universal localhost address, 127.0.0.1.
The other can be one of your local IP addresses.

Only valid existing addresses can be used.

I need to peek a valid local ip address to make sure the test is consistent.

I ran the tests with my configuration and added a "static" fixture test functionality to handle this.

However, I haven't found other valid solutions.

Do you have any suggestions?

@lavie
Copy link
Owner

lavie commented Sep 4, 2024

These tests are supposed to be run as part of the CI process using GitHub Actions. I don't run them locally.

It's an interesting challenge. Off the top of my head maybe a custom network can be added in GitHub Actions and then the fixture container could be associated with a specific IP address?

e.g.

- name: Create custom Docker network
  run: docker network create --subnet=10.10.0.0/16 custom-net

- name: Run Docker container with custom network
  run: |
    docker run -d --name my-container --network custom-net --ip 10.10.10.10 my-container

@rgiuse
Copy link
Contributor Author

rgiuse commented Sep 4, 2024

I can try, but I'm not sure it will work. From what I understand, you can only bind a container port to an IP address of the Docker host.

I don't think you can use the -p option to map a port directly to a Docker network.

Interesting idea in any case!

@rgiuse
Copy link
Contributor Author

rgiuse commented Sep 4, 2024

Creating a docker network on 10.10.0.0/16 you can map a port to 10.10.0.1

@lavie
Copy link
Owner

lavie commented Sep 6, 2024

@rgiuse I'm not sure what the status of the PR is. Were you able to run the tests by creating a custom network?

@rgiuse
Copy link
Contributor Author

rgiuse commented Sep 6, 2024

Yes, I was able to run all the test.

@lavie
Copy link
Owner

lavie commented Sep 6, 2024

Oh good. If you want to add the changes to GitHub Actions please go ahead, otherwise I'll do it. (when I find time..)

Add runlike_fixure6
Add runlike_custom-net creation
@lavie
Copy link
Owner

lavie commented Sep 9, 2024

Nice work! 🙏

@lavie lavie merged commit 80f4634 into lavie:master Sep 9, 2024
1 check passed
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