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 CmdModifierTest#testMemoryLimitModified in cgroup2 environment. #4375

Merged
merged 3 commits into from
Sep 14, 2021

Conversation

cac03
Copy link
Contributor

@cac03 cac03 commented Aug 16, 2021

The testMemoryLimitModified was failing in cgroup2 environment because of the missing
/sys/fs/cgroup/memory/memory.limit_in_bytes file.

In the cgroup2 environment a new file should be checked instead -- /sys/fs/cgroup/memory.max.

This commit intrdocues changes to cat both files:

  1. /sys/fs/cgroup/memory/memory.limit_in_bytes from the first version of cgroup
  2. /sys/fs/cgroup/memory.max from the second

Similar issues:

  1. /sys/fs/cgroup/memory/memory.limit_in_bytes: No such file or directory oracle/docker-images#1939
  2. ENOENT: no such file or directory, open '/sys/fs/cgroup/memory/memory.limit_in_bytes / aarch64  apify/crawlee#693

In the test both files are catted, but only one of them will succeed.
The attempt to cat the other one result in an error added to ExecResult#stderr.
Is this acceptable in these tests?

@cac03 cac03 marked this pull request as ready for review August 17, 2021 12:24
@bsideup
Copy link
Member

bsideup commented Aug 31, 2021

Hi @cac03,

Thanks for submitting a PR! I have an idea:
We can read docker info (see DockerClient#infoCmd) and then check cgroup version (there is no field but you can use getRawObject().get("CgroupVersion"), and then depending on the value, use the correct path. WDYT?

@cac03
Copy link
Contributor Author

cac03 commented Aug 31, 2021

Hi @bsideup !

WDYT?

I think that's cleaner solution
I'll update the PR this week

Thank you for the hint!

@cac03 cac03 marked this pull request as draft August 31, 2021 19:53
The `testMemoryLimitModified` was failing in cgroup2 environment because of the missing
`/sys/fs/cgroup/memory/memory.limit_in_bytes` file.

In the cgroup2 environment a new file should be checked instead -- `/sys/fs/cgroup/memory.max`.

This commit intrdocues changes to `cat` both files:

1. `/sys/fs/cgroup/memory/memory.limit_in_bytes` from the first version of cgroup
2. `/sys/fs/cgroup/memory.max` from the second

Similar issues:

1. oracle/docker-images#1939
2. apify/crawlee#693
@cac03 cac03 marked this pull request as ready for review September 4, 2021 18:12
@cac03
Copy link
Contributor Author

cac03 commented Sep 4, 2021

@bsideup I've updated the PR

Can you please take a look?

@cac03 cac03 requested a review from rnorth September 7, 2021 16:54
@bsideup bsideup added this to the next milestone Sep 14, 2021
@bsideup bsideup merged commit 79b064b into testcontainers:master Sep 14, 2021
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.

None yet

3 participants