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

fix: expose default exposed ports if the NetworkMode is not container #560

Merged
merged 1 commit into from Oct 7, 2022

Conversation

clive-jevons
Copy link
Contributor

docker cannot do container network mode with exposed ports.
in order to make it work, only calculate the default exposed
ports if the NetworkMode is NOT container.

docker cannot do container network mode with exposed ports.
in order to make it work, only calculate the default exposed
ports if the NetworkMode is NOT container.
@mdelapenya
Copy link
Collaborator

This PR is related to #468, as it introduced a change in which all ports will be exposed if no exposed ports were defined at the container request

@codecov
Copy link

codecov bot commented Oct 7, 2022

Codecov Report

Merging #560 (c3513ea) into main (66fed81) will increase coverage by 0.23%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #560      +/-   ##
==========================================
+ Coverage   69.01%   69.24%   +0.23%     
==========================================
  Files          22       22              
  Lines        2172     2172              
==========================================
+ Hits         1499     1504       +5     
+ Misses        535      532       -3     
+ Partials      138      136       -2     
Impacted Files Coverage Δ
docker.go 72.19% <100.00%> (+0.51%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

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

Thanks for identifying this bug and having the time to send a fix

LGTM

@mdelapenya mdelapenya merged commit dc09559 into main Oct 7, 2022
@mdelapenya mdelapenya changed the title fix container NetworkMode usage fix: expose default exposed ports if the NetworkMode is not container Oct 7, 2022
@mdelapenya mdelapenya deleted the fix-container-network-mode-usage branch November 15, 2022 12:19
mdelapenya referenced this pull request in mdelapenya/testcontainers-go Dec 21, 2022
* main:
  Add system requirements parent docs page for podman and colima (#562)
  Support for cap-add/cap-drop (#555)
  fix container NetworkMode usage (#560)
  chore: use hashed versions of test-summary action (#556)
  chore: use container.State() function in tests (#543)
  Log docker server info (#548)
  docs: add docs regarding Colima usage (#547)
  chore: add emoji to breaking changes in release drafter (#542)
  chore: add CONTRIBUTING file (#539)
  issue #537 Rename the wait/multi.go file to wait/all.go (#541)
  docs: add a basic layout for wait strategies in docs (#536)
  docs: improve consistency and fix typos (#534)
  chore: do not skip test (#528)
  chore: include test flakiness in the release drafter (#535)
  chore: retire old versions of Go (#530)
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.

None yet

3 participants