Skip to content

feat: add option to stop containers with a timeout #779

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

Merged
merged 2 commits into from
May 4, 2025

Conversation

o087D
Copy link
Contributor

@o087D o087D commented Mar 22, 2025

Previously containers were stopped with no timeout (SIGKILL) which was interfering with clean shutdown of processes being tested.

This commit adds a new API of stop_with_timeout(timeout_secconds) in addition to the original stop(). The original stop() interface calls stop_with_timeout using the default '0'.

EDIT:

It would seem my original diagnosis was incorrect and the default timeout is 10 seconds, but my CI system was somehow setting the default to 1 second. I will fix the docs and put in the correct default. I think there is still some value in here - apologies for the time waste getting to this point!

Copy link

netlify bot commented Mar 22, 2025

Deploy Preview for testcontainers-rust ready!

Name Link
🔨 Latest commit 5c63b12
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-rust/deploys/681055732d209500080072fd
😎 Deploy Preview https://deploy-preview-779--testcontainers-rust.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 site configuration.

@o087D o087D force-pushed the stop_with_timeout branch from 96e10b5 to 3d210d5 Compare March 22, 2025 08:56
@DDtKey DDtKey changed the title feat: Adds option to stop containers with a timeout feat: add option to stop containers with a timeout Mar 22, 2025
@DDtKey
Copy link
Collaborator

DDtKey commented Mar 22, 2025

Thank you for the contribution! 👍

I've fixed PR name
Could you, please, fix clippy & compilation warnings and apply rustfmt formatting (nightly)? (see CI)

@o087D o087D force-pushed the stop_with_timeout branch 2 times, most recently from b532967 to e1d188d Compare March 23, 2025 06:16
@o087D
Copy link
Contributor Author

o087D commented Mar 23, 2025

Apologies for that, not sure what happened with the linter and forgot to enable all features!

@o087D o087D force-pushed the stop_with_timeout branch 2 times, most recently from 54bf1fc to e337458 Compare March 31, 2025 16:00
@o087D o087D marked this pull request as draft March 31, 2025 16:02
@o087D o087D force-pushed the stop_with_timeout branch from 8479c44 to 887bcae Compare March 31, 2025 16:06
@o087D
Copy link
Contributor Author

o087D commented Mar 31, 2025

I think this is now there! there is a new clippy lint I dont think I want to get into:

warning: large size difference between variants
  --> testcontainers/src/core/wait/mod.rs:32:1
   |
32 | / pub enum WaitFor {
33 | |     /// An empty condition. Useful for default cases or fallbacks.
34 | |     Nothing,
35 | |     /// Wait for a certain message to appear in the container's logs.
36 | |     Log(LogWaitStrategy),
   | |     -------------------- the second-largest variant contains at least 48 bytes
...  |
44 | |     Http(HttpWaitStrategy),
   | |     ---------------------- the largest variant contains at least 272 bytes
45 | |     /// Wait for the container to exit.
46 | |     Exit(ExitWaitStrategy),
47 | | }
   | |_^ the entire enum is at least 272 bytes
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#large_enum_variant
   = note: `#[warn(clippy::large_enum_variant)]` on by default
help: consider boxing the large fields to reduce the total size of the enum
   |
44 -     Http(HttpWaitStrategy),
44 +     Http(Box<HttpWaitStrategy>),
   |

warning: `testcontainers` (lib) generated 1 warning

I also want to test this on my problematic container just to make sure

@o087D o087D force-pushed the stop_with_timeout branch from beabbb1 to fb908c1 Compare April 1, 2025 05:07
o087D added 2 commits April 28, 2025 16:41
Previously containers were stopped with the system default timeout before
issuing SIGKILL which was interfering with clean shutdown of processes being
tested. This commit adds a new API of stop_with_timeout(Option<timeout_seconds>)
in addition to the original stop() which retains its original functionality.

A value of Some(-1) will issue a SIGTERM then wait indefinitely, a value of
Some(t) will issue a SIGTERM then wait t seconds before issuing a SIGKILL. A
value of None will follow the system default - often waiting 10 seconds, but
this is configurable within the docker engine.
@o087D o087D force-pushed the stop_with_timeout branch from fb908c1 to 5c63b12 Compare April 29, 2025 04:28
@o087D o087D marked this pull request as ready for review April 29, 2025 04:59
Copy link
Collaborator

@DDtKey DDtKey left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution!
Really appreciate it!

Sorry for the delay, we probably should have fix the lints in separate PR before.

But I don't mind merging as part of this PR

@DDtKey DDtKey merged commit 6d55d23 into testcontainers:main May 4, 2025
12 checks passed
@github-actions github-actions bot mentioned this pull request May 4, 2025
DDtKey pushed a commit that referenced this pull request May 4, 2025
## 🤖 New release

* `testcontainers`: 0.23.3 -> 0.24.0 (⚠ API breaking changes)

### ⚠ `testcontainers` breaking changes

```text
--- failure enum_variant_added: enum variant added on exhaustive enum ---

Description:
A publicly-visible enum without #[non_exhaustive] has a new variant.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#enum-variant-new
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.40.0/src/lints/enum_variant_added.ron

Failed in:
  variant CmdWaitFor:Exit in /tmp/.tmpPG1hwc/testcontainers-rs/testcontainers/src/core/wait/cmd_wait.rs:16
  variant ClientError:PauseContainer in /tmp/.tmpPG1hwc/testcontainers-rs/testcontainers/src/core/client.rs:81
  variant ClientError:UnpauseContainer in /tmp/.tmpPG1hwc/testcontainers-rs/testcontainers/src/core/client.rs:83
  variant ClientError:PauseContainer in /tmp/.tmpPG1hwc/testcontainers-rs/testcontainers/src/core/client.rs:81
  variant ClientError:UnpauseContainer in /tmp/.tmpPG1hwc/testcontainers-rs/testcontainers/src/core/client.rs:83

--- failure enum_variant_missing: pub enum variant removed or renamed ---

Description:
A publicly-visible enum has at least one variant 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.40.0/src/lints/enum_variant_missing.ron

Failed in:
  variant CmdWaitFor::ExitCode, previously in file /tmp/.tmp7IxuhL/testcontainers/src/core/wait/cmd_wait.rs:16

--- failure trait_method_added: pub trait method added ---

Description:
A non-sealed public trait added a new method without a default implementation, which breaks downstream implementations of the trait
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#trait-new-item-no-default
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.40.0/src/lints/trait_method_added.ron

Failed in:
  trait method testcontainers::core::ImageExt::with_user in file /tmp/.tmpPG1hwc/testcontainers-rs/testcontainers/src/core/image/image_ext.rs:171
  trait method testcontainers::core::ImageExt::with_readonly_rootfs in file /tmp/.tmpPG1hwc/testcontainers-rs/testcontainers/src/core/image/image_ext.rs:174
  trait method testcontainers::core::ImageExt::with_security_opt in file /tmp/.tmpPG1hwc/testcontainers-rs/testcontainers/src/core/image/image_ext.rs:177
  trait method testcontainers::ImageExt::with_user in file /tmp/.tmpPG1hwc/testcontainers-rs/testcontainers/src/core/image/image_ext.rs:171
  trait method testcontainers::ImageExt::with_readonly_rootfs in file /tmp/.tmpPG1hwc/testcontainers-rs/testcontainers/src/core/image/image_ext.rs:174
  trait method testcontainers::ImageExt::with_security_opt in file /tmp/.tmpPG1hwc/testcontainers-rs/testcontainers/src/core/image/image_ext.rs:177
```

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

<blockquote>

## [0.24.0] - 2025-05-04

### Details
#### Features
- Support waiting for commands exit regardless of exit code (#771)
- Add user configuration for container commands (#784)
- Add option to stop containers with a timeout (#779)
- Support `pause` and `unpause` container (#785)
- Allow `security_opt` and `readonly_rootfs` to be configured (#787)

#### Miscellaneous Tasks
- Update etcetera requirement from 0.8.0 to 0.9.0 (#773)
- Update etcetera requirement from 0.9.0 to 0.10.0 (#775)
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/release-plz/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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants