fix: collect bridge IP address correctly

[dlen]

I bumped into this issue while using https://github.com/testcontainers/testcontainers-rs-modules-community/tree/main/src/mysql
and the latest version of testcontainers-rs.

get_bridge_ip_address was not returning the correct IP address value. When running a container on GNU/Linux systems
NetworkSettings.bridge might be an empty string therefore it is not useful as a default value.

This PR tries to provide a default value for such scenarios where:

• No --bridge flag has been explicitly provided when starting the container
• NetworkSettings.bridge is an empty string
• Container has not been launched with a specific network and therefore defaults to whatever is configured on the docker daemon

Thanks for all the efforts in this project!

[DDtKey]

Thank you for the contribution! 🙏

The changes makes sense to me, I've left a couple of minor comments

[dlen]

Wow @DDtKey thanks a lot for the prompt response!! I have pushed your suggestions

[DDtKey]

Could you take a look at falling tests?

[dlen]

Yes, np

[dlen]

Found some issues with the old approach so I had to switch to determine the network attached to the container by using HostConfig.NetworkMode. Even though it is named NetworkMode it contains the name of the network attached and not its mode. So, when we have the network name it a matter of inspect that network and find its driver to finally conclude if it is a bridged network in order to obtain the bridge ip. I think the test should pass on the CI now.

[DDtKey]

I think we should consider this change as breaking one.

Thus, I'm going to include this into 0.17.0.
JFYI: I want to include couple of changes in the next major release, so it will probably will take some time before releasing (not too long)

[dlen]

No problem. Thanks for your time man!

…

On Thu, May 9, 2024, 9:35 PM Artem Medvedev ***@***.***> wrote: I think we should consider this change as breaking one. I mean it's definitely fixes the described issue, but the behavior has changed. Thus, I'm going to include this onto 0.17.0. JFYI: I want to include couple of changes in the next major release, so it will probably will take some time before releasing (not too long) — Reply to this email directly, view it on GitHub <#626 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALQ6LBPCM5FLBW2UJGMKFTZBPFYPAVCNFSM6AAAAABHO4LUI2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBTGI4TGOBVGA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[DDtKey]

Hi @dlen!
I was about to merge the PR, but wanted to double-check the test coverage and current behavior in main.
And understood that these tests are passing against it.

I'm just wondering if this could be covered better, because if we change this back - we don't see any failures.
Sorry for the late notice

[dlen]

Yeah I could add more tests, no problem. However I won't be able to look into it till later next week. If you have something specific in mind let me know.

…

On Fri, May 10, 2024, 11:02 PM Artem Medvedev ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In testcontainers/src/runners/async_runner.rs <#626 (comment)> : > + #[tokio::test] + async fn async_should_rely_on_network_mode_when_network_is_provided_and_settings_bridge_empty() Hi @dlen <https://github.com/dlen>! I was about to merge the PR, but wanted to double-check the test coverage and current behavior in main. And understood that these tests are passing against it. I'm just wondering if this could be covered better, because if we change this back - we don't see the crash, however we are aware of some issues. Sorry for the late notice — Reply to this email directly, view it on GitHub <#626 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALQ6LBH2QET3YPQ4HBBVHTZBUYX5AVCNFSM6AAAAABHO4LUI2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANJQHA3TGOJZHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[DDtKey]

Nothing specific at the moment, but I'll think of it a bit later.

The main goal is to make a test which fails against main, but passes with new changes. That would be ideal option, as we consider this a fix

But in fact, I don't rush you. We even can proceed with this and add tests separately