Skip to content

fix: try to fix more IPv6 handling issues#3198

Merged
mdelapenya merged 6 commits into
testcontainers:mainfrom
ash2k:cleanups
Jun 4, 2025
Merged

fix: try to fix more IPv6 handling issues#3198
mdelapenya merged 6 commits into
testcontainers:mainfrom
ash2k:cleanups

Conversation

@ash2k

@ash2k ash2k commented Jun 3, 2025

Copy link
Copy Markdown
Contributor

What does this PR do?

  • Mostly trying to handle IPv6 correctly in more places. I've ran out of time trying to fix all the modules.
  • Some minor cleanups.

See individual commits.

Why is it important?

Docker seem to prefer IPv6 and I'm still having various issues that seem to be caused by port mapping with IPv6 on the host/container.

Related issues

ash2k added 2 commits June 3, 2025 12:35
Avoid allocating an unbounded buffer only to throw it away immediately.
@ash2k ash2k requested a review from a team as a code owner June 3, 2025 03:15
@netlify

netlify Bot commented Jun 3, 2025

Copy link
Copy Markdown

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit 6a5091b
🔍 Latest deploy log https://app.netlify.com/projects/testcontainers-go/deploys/683fdbe6e1ac9a0008e31cfa
😎 Deploy Preview https://deploy-preview-3198--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@ash2k ash2k changed the title Try to fix more IPv6 handling issues fix: try to fix more IPv6 handling issues Jun 3, 2025
@mdelapenya

Copy link
Copy Markdown
Member

@ash2k thanks for your contribution, indeed, I've been thinking about this to simplify all the modules, but you took the lead! thanks for making it possible.

There is an ongoing PR to remove the readinessCheck for the mapped ports, which is causing flakiness more and more, so let me postpone the merge of this PR until that (I'm on it now).

Now that you showcased the new pattern for getting the host ports in this PR, my suggestion after this is merged would be to send smaller chunks for the rest of the modules, 1-2 per PR, so that it's easier for the CI to pass, and for us to review. WDYT?

@ash2k

ash2k commented Jun 3, 2025

Copy link
Copy Markdown
Contributor Author

@mdelapenya Makes sense, thank you! I'll send smaller PRs next time :)

Regarding the test failures - I think it's a legit failure but I'm not sure how to debug it further. I can reproduce it locally but I have not idea why this might be happening and how to get more information about what is going on. Ideas?

@mdelapenya

Copy link
Copy Markdown
Member

I think we need to merge #3199 to rebase this PR and see the flakiness removed

@ash2k

ash2k commented Jun 3, 2025

Copy link
Copy Markdown
Contributor Author

ack. I'll rebase once it's merged.

@ash2k

ash2k commented Jun 4, 2025

Copy link
Copy Markdown
Contributor Author

@mdelapenya Do you want me to rebase or do you intend to merge it as-is?

@mdelapenya mdelapenya left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@mdelapenya

Copy link
Copy Markdown
Member

@mdelapenya Do you want me to rebase or do you intend to merge it as-is?

No, it's fine as is 👏

Let's work in follow-up PRs for the rest of the modules 🙏

@mdelapenya mdelapenya self-assigned this Jun 4, 2025
@mdelapenya mdelapenya added the bug An issue with the library label Jun 4, 2025
@mdelapenya mdelapenya merged commit ced6c16 into testcontainers:main Jun 4, 2025
203 checks passed
@ash2k ash2k deleted the cleanups branch June 4, 2025 13:37
@ash2k

ash2k commented Jun 4, 2025

Copy link
Copy Markdown
Contributor Author

Thank you! I'll try to get some time to fix the remaining modules soon.

@stevenh

stevenh commented Jun 5, 2025

Copy link
Copy Markdown
Contributor

Nice work on this @ash2k

mdelapenya added a commit to mdelapenya/testcontainers-go that referenced this pull request Jun 10, 2025
* main:
  fix: use PortEndpoint() in a few more modules (testcontainers#3203)
  fix: try to fix more IPv6 handling issues (testcontainers#3198)
  chore!: do not wait for all the exposed ports to be ready (testcontainers#3199)
mdelapenya added a commit to egasimov/testcontainers-go that referenced this pull request Sep 3, 2025
mdelapenya added a commit that referenced this pull request Sep 11, 2025
* feat(nebulagraph): add NebulaGraph module

* Resolve comments 1

* Update document

* Remove static container name

* Resolve comments 2

* docs: refine

* docs: add nebulagraph to mkdocs nav entry

* chore: fix lint (auto)

* chore: append errors to the same slice

* chore: handle state error

* chore: format errors

* chore: do not hide container package

* chore: handle IPv6

See #3198

* chore: simplify

* fix: remove activator on error

* Fix tests 1

* Fix tests 2

---------

Co-authored-by: Manuel de la Peña <mdelapenya@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug An issue with the library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants