-
Notifications
You must be signed in to change notification settings - Fork 142
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
feat!: support UDP and SCTP port mappings #655
feat!: support UDP and SCTP port mappings #655
Conversation
✅ Deploy Preview for testcontainers-rust ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Initial approach to get early feedback, missing tests. |
# Conflicts: # testcontainers/src/core/image/runnable_image.rs # testcontainers/src/images/generic.rs # testcontainers/src/runners/async_runner.rs
Tests added, updated with main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for jumping on the task! 🙏
I've left some comments, please take a look and let me know WDYT
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please take a look at CI failures, should be minor things.
I'll take a look one more time a bit later, but it looks about ready to be merged!
Thank you for the contribution! 🙏
I really appreciate your help
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much! 🚀
} else { | ||
continue; | ||
}; | ||
let internal_port = internal.parse::<ExposedPort>().expect("Internal port"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JFYI: missed this part, we shouldn't panic anymore.
But don't worry, I'll prepare small refactoring
- rename `ExposedPort` to `ContainerPort` (after #655) - `with_mapped_port`: accept two arguments instead of tuple - avoid possible panic
- rename `ExposedPort` to `ContainerPort` (after #655) - `with_mapped_port`: accept two arguments instead of tuple - introduce `IntoContainerPort` to make a shortcut for conversion from `u16` to `ContainerPort`
- rename `ExposedPort` to `ContainerPort` (after #655) - `with_mapped_port`: accept two arguments instead of tuple - introduce `IntoContainerPort` to make a shortcut for conversion from `u16` to `ContainerPort`
- rename `ExposedPort` to `ContainerPort` (after #655) - `with_mapped_port`: accept two arguments instead of tuple - introduce `IntoContainerPort` to make a shortcut for conversion from `u16` to `ContainerPort`
Follow-up PR after #655 (kudos to @estigma88 🎉 ) - rename `ExposedPort` to `ContainerPort` - minor internal renaming to align terminology - `with_mapped_port`: accept two arguments instead of tuple - introduce `IntoContainerPort` to make a shortcut for conversion from `u16` to `ContainerPort`
## 🤖 New release * `testcontainers`: 0.17.0 -> 0.18.0 <details><summary><i><b>Changelog</b></i></summary><p> <blockquote> ## [0.18.0] - 2024-06-15 ### Details #### Bug Fixes - [❗] Make `DOCKER_CONFIG` usage consistent with Docker CLI ([#654](#654)) #### Features - [❗] Support UDP and SCTP port mappings ([#655](#655)) - Impl `From<u16>` for `ContainerPort` with TCP default ([#658](#658)) - Support HTTP wait strategy ([#659](#659)) - Allow passing `u16` to `Ports` #### Miscellaneous Tasks - Use nightly `rustfmt` ([#657](#657)) #### Refactor - [❗] Get rid of associated type `ImageArgs` and rename to `cmd` ([#649](#649)) - Avoid unnecessary owned structs and boxing ([#651](#651)) - [❗] Add `ImageExt` trait to avoid explicit conversion to `RunnableImage` ([#652](#652)) - [❗] Rename `RunnableImage` to `ContainerRequest` ([#653](#653)) - [❗] Exposed and mapped ports api ([#656](#656)) - Preliminary refactoring of `wait` strategies ([#661](#661)) </blockquote> </p></details> --- This PR was generated with [release-plz](https://github.com/MarcoIeni/release-plz/). ## Migration Guide ### Overview Most of the breaking changes introduced in this version primarily impact developers who implement their own images. For general usage, the API remains mostly the same, with some improvements and enhancements for better performance and flexibility. ### 1. Renaming of `RunnableImage` - **Old**: `RunnableImage` - **New**: `ContainerRequest` - **Update**: The explicit conversion from `Image` to `ContainerRequest` (formerly `RunnableImage`) is no longer necessary. Instead, you can now directly import `testcontainers::ImageExt` and override image parameters as needed. ### 2. Changes to `Image` Methods - **Method**: `Image::tag` and `Image::name` - **Old Return Type**: `String` - **New Return Type**: `&str` - **Update**: Update any code that relies on these methods to handle the new return type `&str`. This change helps improve performance by avoiding unnecessary allocations. ### 3. Changes to `Image::exposed_ports` - **Method**: `Image::exposed_ports` - **Old Return Type**: Implementation-specific or previously different. - **New Return Type**: `&[ContainerPort]` - **Update**: The method now returns a slice of `ContainerPort`, which supports exposing ports with protocols `TCP`, `UDP`, and `SCTP`. Update your code to handle the slice accordingly. ### 4. Removal of Associated Type `Args` in `Image` - **Old**: `Image` had an associated type `Args`. - **New**: The associated type `Args` is removed. - **Update**: Command arguments are now part of `Image::cmd`, which returns `impl IntoIterator<Item = impl Into<String>>`. This change allows more flexibility in specifying command arguments. Ensure your code is updated to work with the new method signature. ### 5. Simplification of Trait Implementation in `Image` - **Old**: `Image` required `Box<dyn ..>` for certain traits. - **New**: Utilizes Return Position `impl` Trait in Trait (RPITIT). - **Update**: Instead of requiring `Box<dyn ..>`, `Image` now uses RPITIT for trait returns. This change simplifies the code and improves readability and performance. Familiarize yourself with [RPITIT](https://rustc-dev-guide.rust-lang.org/return-position-impl-trait-in-trait.html) to understand its benefits and applications in your implementation. ### 6. Changes to `RunnableImage::with_mapped_port` - **Old**: `RunnableImage::with_mapped_port` - **New**: Accessible through `ImageExt::with_mapped_port` - **Update**: This method now accepts two parameters instead of a tuple. Adjust your method calls to pass the parameters separately. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Resolves #234
Migration guide
A new way of defining exposed ports and port mappings is introduced to define the protocol, which could be
Tcp, Udp or Sctp. Changes impact the following methods:
GenericImage.with_exposed_port
GenericImage.with_mapped_port
Container.get_host_port_ipv4
Container.get_host_port_ipv6
ContainerAsync.get_host_port_ipv4
ContainerAsync.get_host_port_ipv6
Exposed ports
Having the following container configuration, that defines a Redis container running on TCP 6379 port:
From now on, we should use
ExposedPort
enum instead, as follows:Notice the
ExposedPort::Tcp(6379)
, where we explicitly say which protocol we want to map the port. Most of your usecases would use Tcp, like Redis container, but we recommend to check the containers documentation to be sure.
Port mappings
Having the following container configuration, that defines a Redis container running on TCP 6379 internal port, mapped to
1000 local port:
From now on, we should use
ExposedPort
enum instead, as follows:Notice the
ExposedPort::Tcp(6379)
, where we explicitly say which protocol we want to map the port. Most of your usecases would use Tcp, like Redis container, but we recommend to check the containers documentation to be sure.
If you are interested on getting the local port mapped to an internal port, before you use the following:
From now on, you must specify the protocol, as follows:
Notice the
ExposedPort::Tcp(6379)
in theget_host_port_ipv4
invocation.