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(fork-test.c) Call MANAGERPID from env var when systemd run with --user flag #3149

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ScriptSathi
Copy link
Contributor

@ScriptSathi ScriptSathi commented Nov 20, 2024

Fix #3148

Description

A nil pointer is returned by ctx.Err() because ctx is not done when the error is displayed.

More details

On some distributions, like Archlinux, systemd run with the flag --user. Thus, makes the user manager process a child of systemd process 1. So when you search for PPID, it will always be this child process.
According to the documentation below, systemd expose an environnement variable to help finding this children. So if we compare the value of this environment variable with the ppid, we should get the same result as doing ppid == 1

https://www.freedesktop.org/software/systemd/man/latest/systemd.exec.html#%24MANAGERPID

@ScriptSathi ScriptSathi requested a review from a team as a code owner November 20, 2024 14:49
@ScriptSathi ScriptSathi force-pushed the pr/ScriptSathi/fix-fork_test-return-nil-on-error branch 2 times, most recently from 8e71878 to d245eec Compare November 21, 2024 16:15
@ScriptSathi
Copy link
Contributor Author

@kkourt, here is the fix working for me on Archlinux.

I manage to get the output of make test by now.

fork_test.go:62: failed to parse child2 PID

Thank you for your help on this 💯

@ScriptSathi ScriptSathi force-pushed the pr/ScriptSathi/fix-fork_test-return-nil-on-error branch from d245eec to 109719d Compare November 21, 2024 16:37
@ScriptSathi ScriptSathi changed the title [fork_test.go:55] Remove ctx.Err() to avoid waiting ctx to be done [fork-test.c] Call MANAGERPID from env var when systemd run with --user flag Nov 21, 2024
@ScriptSathi ScriptSathi changed the title [fork-test.c] Call MANAGERPID from env var when systemd run with --user flag fix(fork-test.c) Call MANAGERPID from env var when systemd run with --user flag Nov 21, 2024
contrib/tester-progs/fork-tester.c Outdated Show resolved Hide resolved
@mtardy mtardy added the release-note/misc This PR makes changes that have no direct user impact. label Nov 21, 2024
@ScriptSathi ScriptSathi force-pushed the pr/ScriptSathi/fix-fork_test-return-nil-on-error branch from 109719d to ad77730 Compare November 21, 2024 20:41
Copy link

netlify bot commented Nov 21, 2024

Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit 96abbdc
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/6741e600f67d7c0007eb3187
😎 Deploy Preview https://deploy-preview-3149--tetragon.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@ScriptSathi ScriptSathi force-pushed the pr/ScriptSathi/fix-fork_test-return-nil-on-error branch from ad77730 to 6c0b4b3 Compare November 21, 2024 20:58
@ScriptSathi ScriptSathi force-pushed the pr/ScriptSathi/fix-fork_test-return-nil-on-error branch 2 times, most recently from f154425 to 987c58e Compare November 22, 2024 11:59
…ppid

  The initial goal of fork-tester.c is to test that subchild (child2) find
systemd pid has new parent with hardcoded pid 1. But we can produce the
same behaviour by controlling the new parent of a subchild process when
his parent dies by using `PR_SET_CHILD_SUBREAPER`.
  The main process register as reaper, which means that any childrens and
childrens of childrens will have this main process as parent when their
closest parent dies. So here with have main process as reaper, child1
creates a subchild (child2) then dies. But child2 is no more orphan with
systemd as parent, main process catch it. This add more control over the
process tree and should work for any cases.

Signed-off-by: Tristan d'Audibert <[email protected]>
This update comes after updating the `fork-tester.c`. The parent
inherent grand children child 2 when child 1 dies. In order to ensure
that this specific behaviour occurs, we need to compare parent PID with
new parent PID of child 2 which is suppose to be parent PID instead of
child PID.

Signed-off-by: Tristan d'Audibert <[email protected]>
@ScriptSathi ScriptSathi force-pushed the pr/ScriptSathi/fix-fork_test-return-nil-on-error branch from 987c58e to 96abbdc Compare November 23, 2024 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[fork_test.go:55] Return error <nil>
3 participants