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

Fixed container init process not re-parent to youki main process #1637

Merged
merged 1 commit into from
Mar 11, 2023

Conversation

yihuaf
Copy link
Collaborator

@yihuaf yihuaf commented Mar 8, 2023

Fix #1601

In this PR, I decided to clone intermediate process as child and then clone the container init process as sibling of the intermediate process and child of youki main process. In this way, when intermediate process exits, the container init process will not re-parent to pid 1 and become zombie. The caller of youki can decide to use subreaper flag to adopt the container init process when youki main process exits.

This method cleaner because youki main process is guaranteed to be the parent of both intermediate process and container init process at all time. There is no extra re-parenting.

This PR is only part of the fix. We do need to revisit the detached and foreground mode. Not everything is fixed, but this PR should maintain the old behavior on anything that it does not fix.

@yihuaf yihuaf self-assigned this Mar 8, 2023
@yihuaf yihuaf marked this pull request as draft March 8, 2023 05:36
@yihuaf
Copy link
Collaborator Author

yihuaf commented Mar 8, 2023

See this comment for more discussion: #1601 (comment)

@codecov-commenter
Copy link

codecov-commenter commented Mar 8, 2023

Codecov Report

Merging #1637 (9632c06) into main (ff5dba9) will increase coverage by 0.01%.
The diff coverage is 0.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1637      +/-   ##
==========================================
+ Coverage   68.93%   68.95%   +0.01%     
==========================================
  Files         120      120              
  Lines       13156    13157       +1     
==========================================
+ Hits         9069     9072       +3     
+ Misses       4087     4085       -2     

@yihuaf yihuaf force-pushed the yihuaf/fix-exec branch from cfb32ae to e9ef2ca Compare March 8, 2023 06:42
@yihuaf yihuaf marked this pull request as ready for review March 8, 2023 06:49
@yihuaf yihuaf requested review from utam0k, Furisto and YJDoc2 March 8, 2023 06:49
@yihuaf yihuaf force-pushed the yihuaf/fix-exec branch 2 times, most recently from 5369ab7 to 9632c06 Compare March 8, 2023 06:57
@yihuaf yihuaf force-pushed the yihuaf/fix-exec branch from 9632c06 to 4fb1d41 Compare March 9, 2023 16:54
Copy link
Member

@utam0k utam0k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@utam0k utam0k merged commit ef5daba into youki-dev:main Mar 11, 2023
@yihuaf yihuaf deleted the yihuaf/fix-exec branch March 11, 2023 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use clone(2) instead of fork(2)
3 participants