-
Notifications
You must be signed in to change notification settings - Fork 101
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
Add execution branch for socket activation to correct LISTEN_PID #449
Add execution branch for socket activation to correct LISTEN_PID #449
Conversation
d855821
to
3a3d6b5
Compare
Looks good. Thanks for picking up where I left off. I was hesitant to introduce another whole process into the already somewhat complex way that rootlesskit launches the app as part of #429 especially for a first contribution and given my lack of experience with Go. It is unfortunate that fork and exec are so tightly coupled in Go (though this of course helps with making the language suitable for cross platform development for other projects), since in C, it would be as easy as inserting a call to setenv in the code between fork and exec whereas here we need to introduce a whole other program as a shim and successfully plumb all the relevant info through it, but your implementation looks sane. |
cmd/rootlesskit/main.go
Outdated
iAmChild := os.Getenv(pipeFDEnvKey) != "" | ||
id := "parent" | ||
if iAmChild { | ||
id = "child " // padded to len("parent") | ||
} else if iAmActivationHelper { | ||
id = "activ." |
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.
What is this dot symbol?
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.
Replaced with activation_helper
literal.
Could you squash the commits? |
Signed-off-by: Andrei Lopukhov <andrewlopukhov@gmail.com>
d0094f0
to
ca26493
Compare
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.
Thanks
Proposal on #448
Fixing LISTEN_PID env variable for target process if it equals to PID of [rootlless:parent] process.
Tested with dockerd. Extra $$ script is not required anymore.
Can be demonstated.
Example output for v2.1.0: