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.

Sign up for GitHub

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

Jump to bottom

feat(cli): support for docker cli flag --add-host #547

Merged
merged 3 commits into from
Mar 9, 2024
Merged

feat(cli): support for docker cli flag --add-host #547

merged 3 commits into from
Mar 9, 2024

Conversation

amountainram
Copy link
Contributor

@amountainram amountainram commented Feb 7, 2024 •
edited
Loading

When spawning multiple docker containers in the same test, communication amongst them can be achieved over the docker default gateway.

In DinD environments, such as when running jobs on a CI pipeline, the docker --add-host can be employed to basically enter new entries in the /etc/hosts files. This allows to avoid binding all container ports to the host environment.

The flag API counterpart is implemented on the RunnableImage struct instead of on the generic image since I reckon it makes more sense within this crate phylosophy, I hope !-).

A test runs 2 containers:

  • an http_server
  • a curl client

and the curl client completes when receiving foo from the server on the alias hostname. In case the hostname wasn't resolve it'd panic.

thomaseizinger
thomaseizinger reviewed Feb 14, 2024
View reviewed changes
Copy link
Collaborator

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

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

Makes sense to me!

A few comments.

testcontainers/src/core/image.rs Outdated Show resolved Hide resolved
testcontainers/src/core/image.rs Outdated Show resolved Hide resolved
testcontainers/tests/cli_client.rs Outdated Show resolved Hide resolved
@amountainram
feat(cli): support for docker cli flag --add-host
227a4d1 
When spawning multiple docker containers in the same test,
communication amongst them can achieved over the docker default
gateway.

In DinD environments such as when running jobs on a CI pipeline, the docker
[`--add-host`](https://docs.docker.com/engine/reference/commandline/container_run/#option)
can be employed to basically enter new entries in the `/etc/hosts`
files. This allows to avoid binding all container ports to the host
environment.

The flag API counterpart is implemented on the `RunnableImage` struct
instead of on the generic image since I reckon it makes more sense
within this crate phylosophy, I hope !-).

A test runs 2 containers:

- an http_server
- a `curl` client

and the `curl` client completes when receiving `foo` from the server on the
alias hostname. In case the hostname wasn't resolve it'd panic.
@amountainram
Copy link
Contributor Author

@thomaseizinger I added the enum as requested and updated the code

thomaseizinger
thomaseizinger reviewed Mar 5, 2024
View reviewed changes
Copy link
Collaborator

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

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

One more request, otherwise LGTM

testcontainers/tests/cli_client.rs Outdated Show resolved Hide resolved
@amountainram
fix(test+lint): fix clippy lint warning and attempt on test fix
26b9986 
it appears support of `--add-host` flag in Docker Remote API is somewhat
clumsy, for instance
[here](https://stackoverflow.com/questions/28081421/docker-add-hosts-flag-equivalent-in-remote-api)
and related replies.

It appears that the syntax `host:port` may be supported instead of
`key=value`
auto-merge was automatically disabled March 7, 2024 21:42

Head branch was pushed to by a user without write access

@amountainram
Copy link
Contributor Author

@thomaseizinger i fixed the lint issue but i'm kinda unsure on what to do for the tests.
it appears from the error invalid argument "custom-host=host-gateway" for "--add-host" flag: bad format for add-host: "custom-host=host-gateway" that the supported syntax is {hostname}:{remote-host} instead of {hostname}={remote-host}.

I've tried to run the CI locally with act but I could not make it work.

Can we run again with the new syntax for --add-host flag?

@thomaseizinger
Copy link
Collaborator

I've tried to run the CI locally with act but I could not make it work.

You should just be able to run cargo test locally, assuming you have docker!

@amountainram
Copy link
Contributor Author

I've tried to run the CI locally with act but I could not make it work.

You should just be able to run cargo test locally, assuming you have docker!

Sure thing.. that I did and tests passed both with the syntax host=host and host:host but here in github CI the Docker REST API is used instead of the unix socket to talk with the Docker daemon. It appears that such API understands only host:host for the --add-host flag.

Anyway I see tests pass now so all good 😄!

@thomaseizinger thomaseizinger added this pull request to the merge queue Mar 9, 2024
Merged via the queue into testcontainers:dev with commit 71bbdec Mar 9, 2024
7 checks passed
This was referenced Apr 27, 2024
Closed
Closed
Closed
Merged
DDtKey pushed a commit that referenced this pull request Apr 27, 2024
@github-actions
chore: release v0.16.0 (#597)
7641fbc 
## 🤖 New release
* `testcontainers`: 0.15.0 -> 0.16.0 (⚠️ API breaking changes)

### ⚠️ `testcontainers` breaking changes

```
--- failure constructible_struct_adds_private_field: struct no longer constructible due to new private field ---

Description:
A struct constructible with a struct literal has a new non-public field. It can no longer be constructed using a struct literal outside of its crate.
        ref: https://doc.rust-lang.org/reference/expressions/struct-expr.html
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.31.0/src/lints/constructible_struct_adds_private_field.ron

Failed in:
  field ExecCommand.cmd_ready_condition in /tmp/.tmp8FBROu/testcontainers-rs/testcontainers/src/core/image/exec_command.rs:6
  field ExecCommand.container_ready_conditions in /tmp/.tmp8FBROu/testcontainers-rs/testcontainers/src/core/image/exec_command.rs:7

--- failure enum_missing: pub enum removed or renamed ---

Description:
A publicly-visible enum cannot be imported by its prior path. A `pub use` may have been removed, or the enum itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.31.0/src/lints/enum_missing.ron

Failed in:
  enum testcontainers::core::env::Command, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/env.rs:36

--- failure function_missing: pub fn removed or renamed ---

Description:
A publicly-visible function cannot be imported by its prior path. A `pub use` may have been removed, or the function itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.31.0/src/lints/function_missing.ron

Failed in:
  function testcontainers::core::env::command, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/env.rs:4

--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.31.0/src/lints/inherent_method_missing.ron

Failed in:
  ContainerAsync::get_host_port, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/container_async.rs:68
  ContainerAsync::get_host_port, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/container_async.rs:68
  Container::get_host_port, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/container.rs:124
  Container::get_host_port, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/container.rs:124
  ContainerState::host_port, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/image.rs:120
  GenericImage::with_volume, previously in file /tmp/.tmpkl2iBT/testcontainers/src/images/generic.rs:45
  RunnableImage::inner, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/image.rs:163
  RunnableImage::volumes, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/image.rs:183
  RunnableImage::with_volume, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/image.rs:255
  RunnableImage::inner, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/image.rs:163
  RunnableImage::volumes, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/image.rs:183
  RunnableImage::with_volume, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/image.rs:255

--- failure module_missing: pub module removed or renamed ---

Description:
A publicly-visible module cannot be imported by its prior path. A `pub use` may have been removed, or the module may have been renamed, removed, or made non-public.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.31.0/src/lints/module_missing.ron

Failed in:
  mod testcontainers::core::env, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/env.rs:1
  mod testcontainers::clients, previously in file /tmp/.tmpkl2iBT/testcontainers/src/clients.rs:1

--- failure struct_missing: pub struct removed or renamed ---

Description:
A publicly-visible struct cannot be imported by its prior path. A `pub use` may have been removed, or the struct itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.31.0/src/lints/struct_missing.ron

Failed in:
  struct testcontainers::core::env::Os, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/env.rs:26
  struct testcontainers::clients::Http, previously in file /tmp/.tmpkl2iBT/testcontainers/src/clients/http.rs:23
  struct testcontainers::core::Port, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/image.rs:312
  struct testcontainers::clients::Cli, previously in file /tmp/.tmpkl2iBT/testcontainers/src/clients/cli.rs:22

--- failure struct_pub_field_missing: pub struct's pub field removed or renamed ---

Description:
A publicly-visible struct has at least one public field that is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.31.0/src/lints/struct_pub_field_missing.ron

Failed in:
  field cmd of struct ExecCommand, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/image.rs:101
  field ready_conditions of struct ExecCommand, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/image.rs:103

--- failure struct_pub_field_now_doc_hidden: pub struct field is now #[doc(hidden)] ---

Description:
A pub field of a pub struct is now marked #[doc(hidden)] and is no longer part of the public API.
        ref: https://doc.rust-lang.org/rustdoc/write-documentation/the-doc-attribute.html#hidden
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.31.0/src/lints/struct_pub_field_now_doc_hidden.ron

Failed in:
  field ExecCommand.cmd in file /tmp/.tmp8FBROu/testcontainers-rs/testcontainers/src/core/image/exec_command.rs:4

--- failure trait_method_missing: pub trait method removed or renamed ---

Description:
A trait method is no longer callable, and may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#major-any-change-to-trait-item-signatures
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.31.0/src/lints/trait_method_missing.ron

Failed in:
  method volumes of trait Image, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/image.rs:70
  method volumes of trait Image, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/image.rs:70

--- failure trait_missing: pub trait removed or renamed ---

Description:
A publicly-visible trait cannot be imported by its prior path. A `pub use` may have been removed, or the trait itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.31.0/src/lints/trait_missing.ron

Failed in:
  trait testcontainers::core::env::GetEnvValue, previously in file /tmp/.tmpkl2iBT/testcontainers/src/core/env.rs:20
```

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

## [0.16.0] - 2024-04-27

### Details
#### Bug Fixes
- Use the binds option instead of volumes for mounts
([#581](#581))

#### Documentation
- Mention a way to preserve running containers
([#586](#586))

#### Features
- Support for docker cli flag `--add-host`
([#547](#547))
- Allow to override args in a `RunnableImage`
([#558](#558))
- Add `name` parameter to `RunnableImage`
([#549](#549))
- [❗] Container-centric API with refactored underlying layer
([#575](#575))
- Add ability to pull-image explicitly
([#579](#579))

#### Miscellaneous Tasks
- Remove `spectral` from `dev-dependencies`
([#526](#526))
- Update reqwest requirement from 0.11.14 to 0.12.3
([#569](#569))
- Update bollard and bollard-stubs
([#574](#574))
- Reuse workspace level configs
([#568](#568))

#### Refactor
- [❗] Api for mounts and volumes
([#596](#596))

#### Styling
- Derive `Default` to fix clippy lint
([#525](#525))
- Actualize formatting configs
([#567](#567))
- Enable `StdExternalCrate` grouping
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/MarcoIeni/release-plz/).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomaseizinger thomaseizinger thomaseizinger approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@amountainram @thomaseizinger