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 systemd main pid handling when running with --log #4190

Merged
merged 1 commit into from
Oct 12, 2021

Conversation

brandond
Copy link
Member

@brandond brandond commented Oct 11, 2021

Fixes regression introduced in #4115, reported by @kralicky

Proposed Changes

Using MAINPID breaks systemd's exit detection, as it stops watching the
original pid, but is unable to watch the new pid as it is not a child
of systemd itself. The best we can do is just notify when execing the child
process.

We also need to consolidate forking into a single place so that we don't
end up with multiple levels of child processes if both redirecting log
output and reaping child processes.

Types of Changes

bugfix

Verification

  • start k3s in docker without the docker --init flag; note that it does not leave behind zombie containerd-shim processes.
  • start k3s as systemd service with the --log flag; note that logs are written to the specified log file.
  • start k3s as systemd service with the --log flag; note that it is restarted properly when killed.

Linked Issues

User-Facing Change

Fixed an issue that caused k3s to not be restarted by systemd when started with the `--log` flag.

Further Comments

Using MAINPID breaks systemd's exit detection, as it stops watching the
original pid, but is unable to watch the new pid as it is not a child
of systemd itself. The best we can do is just notify when execing the child
process.

We also need to consolidate forking into a sigle place so that we don't
end up with multiple levels of child processes if both redirecting log
output and reaping child processes.

Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
@brandond brandond requested a review from a team as a code owner October 11, 2021 22:05
@brandond brandond changed the title Refactor log and reaper exec to omit MAINPID Fix systemd main pid handling when running with --log Oct 11, 2021
@codecov-commenter
Copy link

codecov-commenter commented Oct 11, 2021

Codecov Report

Merging #4190 (ac3759f) into master (ac7a8d8) will increase coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4190   +/-   ##
=======================================
  Coverage   11.46%   11.47%           
=======================================
  Files         134      135    +1     
  Lines        9242     9237    -5     
=======================================
  Hits         1060     1060           
+ Misses       7956     7951    -5     
  Partials      226      226           
Flag Coverage Δ
inttests 0.67% <0.00%> (+<0.01%) ⬆️
unittests 11.12% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/cli/agent/agent.go 0.00% <0.00%> (ø)
pkg/cli/cmds/init_linux.go 0.00% <0.00%> (ø)
pkg/cli/cmds/log.go 0.00% <0.00%> (ø)
pkg/cli/cmds/log_linux.go 0.00% <0.00%> (ø)
pkg/cli/server/server.go 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ac7a8d8...ac3759f. Read the comment docs.

@brandond brandond merged commit dc18ef2 into k3s-io:master Oct 12, 2021
@brandond brandond deleted the fix_log_exit branch June 6, 2024 21:18
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.

5 participants