From 779febfb9d87ddf6c38a88570fc8d778257cb672 Mon Sep 17 00:00:00 2001 From: Noel Georgi Date: Thu, 11 May 2023 23:11:48 +0530 Subject: [PATCH] fix: revert: set rlimit explicitly in wrapperd This reverts commit a2565f67416e9b9bc22f2d5506df9ea7771c0c8c. 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](https://github.com/golang/go/commit/ecf7e00db8b8f5fff622f232bd8c515814c2ecc6). 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](https://github.com/golang/go/commit/ecf7e00db8b8f5fff622f232bd8c515814c2ecc6), 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 Signed-off-by: Noel Georgi (cherry picked from commit 4f720d46532af39165fc5051052d5c42595d91af) --- hack/test/e2e.sh | 3 +++ internal/app/wrapperd/main.go | 7 ------- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/hack/test/e2e.sh b/hack/test/e2e.sh index 28ac28719d..3b34b5b5db 100755 --- a/hack/test/e2e.sh +++ b/hack/test/e2e.sh @@ -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 { diff --git a/internal/app/wrapperd/main.go b/internal/app/wrapperd/main.go index 435e256c52..8db49359ee 100644 --- a/internal/app/wrapperd/main.go +++ b/internal/app/wrapperd/main.go @@ -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 {