feat: remove wrapperd and launch processes directly #9489
Conversation
| wrapper.stderr, | ||
| } | ||
|
|
||
| // TODO: use pa.Sys.CgroupFD here when we can be sure clone3 is available |
There was a problem hiding this comment.
So implement 2 different pathways? We use this code in container mode as well, and we cannot really expect containers to be ran on kernel 5.5+ which has clone3
Signed-off-by: Dmitry Sharshakov <[email protected]>
Signed-off-by: Dmitry Sharshakov <[email protected]>
Signed-off-by: Dmitry Sharshakov <[email protected]>
Signed-off-by: Dmitry Sharshakov <[email protected]>
…nagement Signed-off-by: Dmitry Sharshakov <[email protected]>
Signed-off-by: Dmitry Sharshakov <[email protected]>
…ling Now everything except dashboard (operation not permitted (?!)) runs Signed-off-by: Dmitry Sharshakov <[email protected]>
Signed-off-by: Dmitry Sharshakov <[email protected]>
Signed-off-by: Dmitry Sharshakov <[email protected]>
Do TIOCSCTTY on stdin, not WRONLY stdout to avoid EPERM Signed-off-by: Dmitry Sharshakov <[email protected]>
Signed-off-by: Dmitry Sharshakov <[email protected]>
dsseng
force-pushed
the
wrapperd-replacement
branch
from
80d9477
to
fdaa3a2
Compare
Validate capabilities are dropped and cgroup, UID, environment and OOM adjustments are set Signed-off-by: Dmitry Sharshakov <[email protected]>
Signed-off-by: Dmitry Sharshakov <[email protected]>
| @@ -105,6 +105,7 @@ linters-settings: | |||
| - golang.zx2c4.com/wireguard | |||
| - golang.zx2c4.com/wireguard/wgctrl | |||
| - cloud.google.com/go | |||
| - kernel.org/pub/linux/libs/security/libcap/cap | |||
There was a problem hiding this comment.
TODO: remove as this was only debug
|
|
||
| // improved error logging | ||
| kernel.org/pub/linux/libs/security/libcap/cap => github.com/dsseng/go-libcap/cap v0.0.0-20241015195416-c3ab072bd718 |
| @@ -224,6 +224,7 @@ func TestProcessSuite(t *testing.T) { | |||
| t.Skip("wrapperd not found") | |||
| } | |||
|
|
|||
| // What's the purpose of this test? Should it be replaced for new subprocess start method? | |||
There was a problem hiding this comment.
process reaper is due to the fact that Talos machine is PID=1 and it should reap all zombies, so it runs a waitpid() loop which breaks Go's wait for the process (as it reaps before Go might have a chance), so unrelated to wrapperd
| @@ -66,7 +66,7 @@ func (d *Dashboard) Runner(r runtime.Runtime) (runner.Runner, error) { | |||
| }), | |||
| runner.WithStdinFile(tty), | |||
| runner.WithStdoutFile(tty), | |||
| runner.WithCtty(1), | |||
| runner.WithCtty(0), | |||
There was a problem hiding this comment.
ctty(1) failed as stdout is opened as write-only. We might want to change that, but perhaps using stdin as ctty is perfectly fine as we pass both for TTY to work
dsseng
changed the title
Signed-off-by: Dmitry Sharshakov <[email protected]>
|
Maybe the test should be a separate PR going in before refactoring. If so, will do later today |
| return fmt.Errorf("failed to load cgroup %s: %w", path, err) | ||
| } | ||
|
|
||
| if err := cgv2.AddProc(uint64(pid)); err != nil { |
There was a problem hiding this comment.
Don't we also use this launch method when running in a container (thus not sure we have clone3 available)? Cgroup in clone3 is actually only available since kernel 5.7
There was a problem hiding this comment.
good point, let's skip it for now
There was a problem hiding this comment.
can we optionally have two paths? like auto-detect?
There was a problem hiding this comment.
Unsure whether that's going to be beneficial, but it's possible for sure
| waitCh := make(chan error) | ||
|
|
||
| go func() { | ||
| waitCh <- reaper.WaitWrapper(usingReaper, notifyCh, cmdWrapper.cmd) | ||
| _, err := process.Wait() |
There was a problem hiding this comment.
should use reaper, don't change this part please
There was a problem hiding this comment.
Okay, refactoring here to accept raw pid and so: https://github.com/siderolabs/go-cmd/blob/main/pkg/cmd/proc/reaper/wait.go#L13-L38
Fixes #9472
TODO: