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

Check container's status if WaitStrategy fails. #1837

Merged
merged 11 commits into from
Nov 9, 2019

Conversation

bsideup
Copy link
Member

@bsideup bsideup commented Sep 8, 2019

Fixes #1834

@bsideup bsideup added this to the next milestone Sep 8, 2019
@bsideup bsideup requested a review from a team September 8, 2019 14:59
.withCreateContainerCmdModifier(it -> {
it.getHostConfig().withMemory(4 * FileUtils.ONE_MB);
})
.withCommand("sh", "-c", "usleep 100; dd if=/dev/urandom bs=128GB count=1 > test.txt")
Copy link
Member

Choose a reason for hiding this comment

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

Gulp... Better get this right 😅 😂

At first sight I was perplexed as to why writing to disk would cause OOMKill at all, but realised that actually the only thing that matters here is having the blocksize be >4MB, as dd buffers into memory before writing.

If that's the case, could we use > /dev/null or of=/dev/null to just be explicit about blackholing the data?

This might be slightly less alarming for anyone reading the code in future who fears we're about to fill their disk with garbage 😄.

Copy link
Member

@rnorth rnorth left a comment

Choose a reason for hiding this comment

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

Only minor comments from me, but this LGTM as a whole.

@rnorth
Copy link
Member

rnorth commented Sep 23, 2019

Ohoh, build is failing. Looks like we're not getting OOMKilled like we want to.

@bsideup
Copy link
Member Author

bsideup commented Sep 24, 2019

@rnorth yeah, will try to debug it (works locally)

@bsideup bsideup modified the milestones: 1.12.3, next Oct 26, 2019
@rnorth rnorth self-assigned this Nov 1, 2019
@rnorth
Copy link
Member

rnorth commented Nov 1, 2019

Trying to diagnose why this org.testcontainers.containers.GenericContainerTest test is behaving strangely

On CircleCI there were two problems:
* `dd: /dev/urandom: Bad address` when `/dev/urandom` was used, which
  would cause exit code 1, not the desired OOMKill
* Once worked-around, `dd` was only processing a partial block:

```
0+1 records in

0+1 records out
```

I suspect that `dd` might internally be respecting, or suffering limited
buffers due to the cgroup memory limit, though I'm not clear why this
behaviour only appeared on CircleCI and Azure pipelines.
@bsideup bsideup merged commit f3011b6 into master Nov 9, 2019
@delete-merged-branch delete-merged-branch bot deleted the check_status_after_wait branch November 9, 2019 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

If WaitStrategy fails, we should log if the container is still running or failed
2 participants