-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[1.1] Fix set nofile rlimit error #4277
Merged
kolyshkin
merged 7 commits into
opencontainers:release-1.1
from
lifubang:backport-4265-nofilerlimit
May 20, 2024
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
debf52a
deprecate libct.system.Execv
kolyshkin fbddb71
libct: fix a comment
kolyshkin 83ecd11
runc exec: setupRlimits after syscall.rlimit.init() completed
lifubang 42c2ab2
use go 1.18 in go.mod
lifubang d7a29a3
libct: clean cached rlimit nofile in go runtime
ls-ggg 2992049
update/add some tests for rlimit
lifubang c918058
fix comments for ClearRlimitNofileCache
lifubang File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| module github.com/opencontainers/runc | ||
|
|
||
| go 1.17 | ||
| go 1.18 | ||
|
|
||
| require ( | ||
| github.com/checkpoint-restore/go-criu/v5 v5.3.0 | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| //go:build go1.19 | ||
|
|
||
| package system | ||
|
|
||
| import ( | ||
| "sync/atomic" | ||
| "syscall" | ||
|
|
||
| _ "unsafe" // for go:linkname | ||
| ) | ||
|
|
||
| //go:linkname syscallOrigRlimitNofile syscall.origRlimitNofile | ||
| var syscallOrigRlimitNofile atomic.Pointer[syscall.Rlimit] | ||
|
|
||
| // ClearRlimitNofileCache is to clear go runtime's nofile rlimit cache. | ||
| func ClearRlimitNofileCache() { | ||
| // As reported in issue #4195, the new version of go runtime(since 1.19) | ||
| // will cache rlimit-nofile. Before executing execve, the rlimit-nofile | ||
| // of the process will be restored with the cache. In runc, this will | ||
| // cause the rlimit-nofile setting by the parent process for the container | ||
| // to become invalid. It can be solved by clearing this cache. But | ||
| // unfortunately, go stdlib doesn't provide such function, so we need to | ||
| // link to the private var `origRlimitNofile` in package syscall to hack. | ||
| syscallOrigRlimitNofile.Store(nil) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| //go:build !go1.19 | ||
|
|
||
| package system | ||
|
|
||
| func ClearRlimitNofileCache() { | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| #!/usr/bin/env bats | ||
|
|
||
| load helpers | ||
|
|
||
| function setup() { | ||
| # Do not change the Cur value to be equal to the Max value | ||
| # Because in some environments, the soft and hard nofile limit have the same value. | ||
| [ $EUID -eq 0 ] && prlimit --nofile=1024:65536 -p $$ | ||
| setup_busybox | ||
| } | ||
|
|
||
| function teardown() { | ||
| teardown_bundle | ||
| } | ||
|
|
||
| # Set and check rlimit_nofile for runc run. Arguments are: | ||
| # $1: soft limit; | ||
| # $2: hard limit. | ||
| function run_check_nofile() { | ||
| soft="$1" | ||
| hard="$2" | ||
| update_config ".process.rlimits = [{\"type\": \"RLIMIT_NOFILE\", \"soft\": ${soft}, \"hard\": ${hard}}]" | ||
| update_config '.process.args = ["/bin/sh", "-c", "ulimit -n; ulimit -H -n"]' | ||
|
|
||
| runc run test_rlimit | ||
| [ "$status" -eq 0 ] | ||
| [[ "${lines[0]}" == "${soft}" ]] | ||
| [[ "${lines[1]}" == "${hard}" ]] | ||
| } | ||
|
|
||
| # Set and check rlimit_nofile for runc exec. Arguments are: | ||
| # $1: soft limit; | ||
| # $2: hard limit. | ||
| function exec_check_nofile() { | ||
| soft="$1" | ||
| hard="$2" | ||
| update_config ".process.rlimits = [{\"type\": \"RLIMIT_NOFILE\", \"soft\": ${soft}, \"hard\": ${hard}}]" | ||
|
|
||
| runc run -d --console-socket "$CONSOLE_SOCKET" test_rlimit | ||
| [ "$status" -eq 0 ] | ||
|
|
||
| runc exec test_rlimit /bin/sh -c "ulimit -n; ulimit -H -n" | ||
| [ "$status" -eq 0 ] | ||
| [[ "${lines[0]}" == "${soft}" ]] | ||
| [[ "${lines[1]}" == "${hard}" ]] | ||
| } | ||
|
|
||
| @test "runc run with RLIMIT_NOFILE(The same as system's hard value)" { | ||
| hard=$(ulimit -n -H) | ||
| soft="$hard" | ||
| run_check_nofile "$soft" "$hard" | ||
| } | ||
|
|
||
| @test "runc run with RLIMIT_NOFILE(Bigger than system's hard value)" { | ||
| requires root | ||
| limit=$(ulimit -n -H) | ||
| soft=$((limit + 1)) | ||
| hard=$soft | ||
| run_check_nofile "$soft" "$hard" | ||
| } | ||
|
|
||
| @test "runc run with RLIMIT_NOFILE(Smaller than system's hard value)" { | ||
| limit=$(ulimit -n -H) | ||
| soft=$((limit - 1)) | ||
| hard=$soft | ||
| run_check_nofile "$soft" "$hard" | ||
| } | ||
|
|
||
| @test "runc exec with RLIMIT_NOFILE(The same as system's hard value)" { | ||
| hard=$(ulimit -n -H) | ||
| soft="$hard" | ||
| exec_check_nofile "$soft" "$hard" | ||
| } | ||
|
|
||
| @test "runc exec with RLIMIT_NOFILE(Bigger than system's hard value)" { | ||
| requires root | ||
| limit=$(ulimit -n -H) | ||
| soft=$((limit + 1)) | ||
| hard=$soft | ||
| exec_check_nofile "$soft" "$hard" | ||
| } | ||
|
|
||
| @test "runc exec with RLIMIT_NOFILE(Smaller than system's hard value)" { | ||
| limit=$(ulimit -n -H) | ||
| soft=$((limit - 1)) | ||
| hard=$soft | ||
| exec_check_nofile "$soft" "$hard" | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
(Ideally we should not bump up Go version in backport releases, but it's not a huge deal, as Go 1.17 has already reached EOL)
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.
This is needed here because we rely on a feature that is only available since Go 1.18, and despite the feature being guarded by go1.20 build tag, this bump is still required (see #4277 (comment) above).
Nevertheless, go 1.17 can still be used just fine to compile this (as you can see in CI).
All that doesn't make much sense to me either, but Go developers seem to disagree (golang/go#52880).