Skip to content

Commit

Permalink
fix: revert: set rlimit explicitly in wrapperd
Browse files Browse the repository at this point in the history
This reverts commit a2565f6.

The fix done in `a2565f67`, was actually a no-op caused by the
misunderstanding the fix done in Go and backported to [Go 1.20.4](golang/go@ecf7e00).
The fix gave a false confidence that it was working when it was tested
against Talos `main` branch since the PR #7190 bumped `x/sys` package
from [v0.7.0 -> v0.8.0](golang/go@ecf7e00), the actual change in `x/sys` can be found here at https://cs.opensource.google/go/x/sys/+/ff18efa0a3fa22d4fede381b822bbcfe53b7ee7c which meant that when updating Go to 1.20.4 the `x/sys` package should been updated too. The `x/sys` package changed how the syscall to set the rlimit was called, it got moved into the Go stdlib instead of calling rlimit syscall in the `x/sys` package, which meant a combination of using Go 1.20.4 and an older `x/sys` package means `RLIMIT_NOFILE` value would not be set back to the original value.

The Talos 1.4 release branch currently have  `x/sys`
at [v0.7.0(https://github.com/siderolabs/talos/blob/v1.4.3/go.mod#L133),
so the backport would consist of this change along another commit bumping `x/sys` package to `v0.8.0`.

Fixes: #7198
Fixes: #7206

Co-authored-by: Utku Ozdemir <utku.ozdemir@siderolabs.com>
Signed-off-by: Noel Georgi <git@frezbo.dev>
  • Loading branch information
frezbo and utkuozdemir committed May 11, 2023
1 parent a2565f6 commit 4f720d4
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 7 deletions.
3 changes: 3 additions & 0 deletions hack/test/e2e.sh
Expand Up @@ -126,6 +126,9 @@ function create_cluster_capi {
${KUBECTL} get nodes -l node-role.kubernetes.io/control-plane='' && :
sleep 10
done

# verify that RLIMIT_NOFILE is set to 1048576
${KUBECTL} run --rm --restart=Never -it foo --image=alpine -- /bin/sh -c "ulimit -n" | grep -q 1048576
}

function run_talos_integration_test {
Expand Down
7 changes: 0 additions & 7 deletions internal/app/wrapperd/main.go
Expand Up @@ -50,13 +50,6 @@ func Main() {
}
}

// set the rlimit for the process before we drop privileges
// TODO: frezbo: see if we need to drop Rlimit from the boot sequence, the only downside maybe that some very early process might
// not have the higher rlimit set, but it seems we always use the wrapper to start processes.
if err := unix.Setrlimit(unix.RLIMIT_NOFILE, &unix.Rlimit{Max: 1048576, Cur: 1048576}); err != nil {
log.Fatalf("failed to set rlimit: %v", err)
}

// load the cgroup and put the process into the cgroup
if cgroupPath != "" {
if cgroups.Mode() == cgroups.Unified {
Expand Down

0 comments on commit 4f720d4

Please sign in to comment.