Skip to content

Fix some signal forwarder issues #4532

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.

Sign up for GitHub

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

Jump to bottom
Merged
fuweid merged 2 commits into from
Sep 6, 2020
Merged

Fix some signal forwarder issues #4532

fuweid merged 2 commits into from
Sep 6, 2020

Conversation

@cpuguy83
Copy link
Member

@cpuguy83 cpuguy83 commented Sep 4, 2020

Exit signal forward if process not found

Previously the signal loop can end up racing with the process exiting.
Intead of logging and continuing the loop, exit early.

Ignore SIGURG signals in signal forwarder

Starting with go1.14, the go runtime hijacks SIGURG but with no way to
not send to other signal handlers.

In practice, we get this signal frequently.
I found this while testing out go1.15 with ctr and multiple execs with
only echo hello. When the process exits quickly, if the previous
commit is not applied, you end up with an error message that it couldn't
forward SIGURG to the container (due to the process being gone).

@cpuguy83
Exit signal forward if process not found
6650510 
Previously the signal loop can end up racing with the process exiting.
Intead of logging and continuing the loop, exit early.

Signed-off-by: Brian Goff <[email protected]>
@cpuguy83 cpuguy83 force-pushed the forward_signal_not_found branch from ddaeb55 to 59f8a3a Compare September 4, 2020 23:17
@cpuguy83
Ignore SIGURG signals in signal forwarder
899b4e3 
Starting with go1.14, the go runtime hijacks SIGURG but with no way to
not send to other signal handlers.

In practice, we get this signal frequently.
I found this while testing out go1.15 with ctr and multiple execs with
only `echo hello`. When the process exits quickly, if the previous
commit is not applied, you end up with an error message that it couldn't
forward SIGURG to the container (due to the process being gone).

Signed-off-by: Brian Goff <[email protected]>
@cpuguy83 cpuguy83 force-pushed the forward_signal_not_found branch from 59f8a3a to 899b4e3 Compare September 4, 2020 23:19
@theopenlab-ci
Copy link

theopenlab-ci bot commented Sep 4, 2020

Build succeeded.

estesp
estesp approved these changes Sep 5, 2020
View reviewed changes
Copy link
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

fuweid
fuweid approved these changes Sep 6, 2020
View reviewed changes
Copy link
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

@fuweid fuweid merged commit 7ce2386 into containerd:master Sep 6, 2020
@cpuguy83 cpuguy83 deleted the forward_signal_not_found branch September 6, 2020 04:37
This was referenced Sep 8, 2020
Merged
Merged
@fuweid fuweid added cherry-picked/1.3.x cherry-picked/1.4.x PR commits are cherry picked into the release/1.4 branch and removed cherry-pick/1.3.x labels Sep 9, 2020
@pothos pothos mentioned this pull request Jan 11, 2021
Closed
pothos added a commit to flatcar-archive/coreos-overlay that referenced this pull request Jan 12, 2021
@pothos
app-emulation/(docker*|containerd): Revert to Go 1.13
cd05fb7 
When Docker/containerd binaries are compiled with Go 1.15 the
containers generate many signal 23 (SIGURG) events which flood
monitoring systems:
  kubernetes/kops#10388
The SIGURG signal does not kill the process but is generated by Go
runtime scheduling:
  https://go.googlesource.com/proposal/+/master/design/24543-non-cooperative-preemption.md)
Because the Go runtime does not know if the process expects external
SIGURG signals, the signal is not filtered out but reported to the
process: golang/go#37942
The process has to filter this signal out itself before forwarding it
to, e.g,. children processes or logs.
This change was introduced with the Go 1.15 update (actually Go 1.14
but Flatcar skipped that for Stable), however, while containerd has
some workarounds in place, e.g., in
containerd/containerd#4532 but there are still
areas where the signal is not handled correctly.
Until this is the case, downgrade to use the Go 1.13 compiler for
Docker/containerd binaries.

See flatcar/Flatcar#315
@pothos pothos mentioned this pull request Jan 12, 2021
Merged
pothos added a commit to flatcar-archive/coreos-overlay that referenced this pull request Jan 12, 2021
@pothos
app-emulation/(docker*|containerd): Revert to Go 1.13
2a8304c 
When Docker/containerd binaries are compiled with Go 1.15 the
containers generate many signal 23 (SIGURG) events which flood
monitoring systems:
  kubernetes/kops#10388
The SIGURG signal does not kill the process but is generated by Go
runtime scheduling:
  https://go.googlesource.com/proposal/+/master/design/24543-non-cooperative-preemption.md)
Because the Go runtime does not know if the process expects external
SIGURG signals, the signal is not filtered out but reported to the
process: golang/go#37942
The process has to filter this signal out itself before forwarding it
to, e.g,. children processes or logs.
This change was introduced with the Go 1.15 update (actually Go 1.14
but Flatcar skipped that for Stable), however, while containerd has
some workarounds in place, e.g., in
containerd/containerd#4532 but there are still
areas where the signal is not handled correctly.
Until this is the case, downgrade to use the Go 1.13 compiler for
Docker/containerd binaries.

See flatcar/Flatcar#315
pothos added a commit to flatcar-archive/coreos-overlay that referenced this pull request Jan 13, 2021
@pothos
app-emulation/(docker*|containerd): Revert to Go 1.13
9c17edd 
When Docker/containerd binaries are compiled with Go 1.15 the
containers generate many signal 23 (SIGURG) events which flood
monitoring systems:
  kubernetes/kops#10388
The SIGURG signal does not kill the process but is generated by Go
runtime scheduling:
  https://go.googlesource.com/proposal/+/master/design/24543-non-cooperative-preemption.md)
Because the Go runtime does not know if the process expects external
SIGURG signals, the signal is not filtered out but reported to the
process: golang/go#37942
The process has to filter this signal out itself before forwarding it
to, e.g,. children processes or logs.
This change was introduced with the Go 1.15 update (actually Go 1.14
but Flatcar skipped that for Stable), however, while containerd has
some workarounds in place, e.g., in
containerd/containerd#4532 but there are still
areas where the signal is not handled correctly.
Until this is the case, downgrade to use the Go 1.13 compiler for
Docker/containerd binaries.

See flatcar/Flatcar#315
pothos added a commit to flatcar-archive/coreos-overlay that referenced this pull request Jan 18, 2021
@pothos
app-emulation/(docker*|containerd): Revert to Go 1.13
13c11b3 
When Docker/containerd binaries are compiled with Go 1.15 the
containers generate many signal 23 (SIGURG) events which flood
monitoring systems:
  kubernetes/kops#10388
The SIGURG signal does not kill the process but is generated by Go
runtime scheduling:
  https://go.googlesource.com/proposal/+/master/design/24543-non-cooperative-preemption.md)
Because the Go runtime does not know if the process expects external
SIGURG signals, the signal is not filtered out but reported to the
process: golang/go#37942
The process has to filter this signal out itself before forwarding it
to, e.g,. children processes or logs.
This change was introduced with the Go 1.15 update (actually Go 1.14
but Flatcar skipped that for Stable), however, while containerd has
some workarounds in place, e.g., in
containerd/containerd#4532 but there are still
areas where the signal is not handled correctly.
Until this is the case, downgrade to use the Go 1.13 compiler for
Docker/containerd binaries.

See flatcar/Flatcar#315
pothos added a commit to flatcar-archive/coreos-overlay that referenced this pull request Jan 18, 2021
@pothos
app-emulation/(docker*|containerd): Revert to Go 1.13
fd30d44 
When Docker/containerd binaries are compiled with Go 1.15 the
containers generate many signal 23 (SIGURG) events which flood
monitoring systems:
  kubernetes/kops#10388
The SIGURG signal does not kill the process but is generated by Go
runtime scheduling:
  https://go.googlesource.com/proposal/+/master/design/24543-non-cooperative-preemption.md)
Because the Go runtime does not know if the process expects external
SIGURG signals, the signal is not filtered out but reported to the
process: golang/go#37942
The process has to filter this signal out itself before forwarding it
to, e.g,. children processes or logs.
This change was introduced with the Go 1.15 update (actually Go 1.14
but Flatcar skipped that for Stable), however, while containerd has
some workarounds in place, e.g., in
containerd/containerd#4532 but there are still
areas where the signal is not handled correctly.
Until this is the case, downgrade to use the Go 1.13 compiler for
Docker/containerd binaries.

See flatcar/Flatcar#315
pothos added a commit to flatcar-archive/coreos-overlay that referenced this pull request Jan 18, 2021
@pothos
app-emulation/(docker*|containerd): Revert to Go 1.13
dc0052f 
When Docker/containerd binaries are compiled with Go 1.15 the
containers generate many signal 23 (SIGURG) events which flood
monitoring systems:
  kubernetes/kops#10388
The SIGURG signal does not kill the process but is generated by Go
runtime scheduling:
  https://go.googlesource.com/proposal/+/master/design/24543-non-cooperative-preemption.md)
Because the Go runtime does not know if the process expects external
SIGURG signals, the signal is not filtered out but reported to the
process: golang/go#37942
The process has to filter this signal out itself before forwarding it
to, e.g,. children processes or logs.
This change was introduced with the Go 1.15 update (actually Go 1.14
but Flatcar skipped that for Stable), however, while containerd has
some workarounds in place, e.g., in
containerd/containerd#4532 but there are still
areas where the signal is not handled correctly.
Until this is the case, downgrade to use the Go 1.13 compiler for
Docker/containerd binaries.

See flatcar/Flatcar#315
pothos added a commit to flatcar-archive/coreos-overlay that referenced this pull request Feb 1, 2021
@pothos
app-emulation/(docker*|containerd): Revert to Go 1.13
ce21e7e 
When Docker/containerd binaries are compiled with Go 1.15 the
containers generate many signal 23 (SIGURG) events which flood
monitoring systems:
  kubernetes/kops#10388
The SIGURG signal does not kill the process but is generated by Go
runtime scheduling:
  https://go.googlesource.com/proposal/+/master/design/24543-non-cooperative-preemption.md)
Because the Go runtime does not know if the process expects external
SIGURG signals, the signal is not filtered out but reported to the
process: golang/go#37942
The process has to filter this signal out itself before forwarding it
to, e.g,. children processes or logs.
This change was introduced with the Go 1.15 update (actually Go 1.14
but Flatcar skipped that for Stable), however, while containerd has
some workarounds in place, e.g., in
containerd/containerd#4532 but there are still
areas where the signal is not handled correctly.
Until this is the case, downgrade to use the Go 1.13 compiler for
Docker/containerd binaries.

See flatcar/Flatcar#315
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@estesp estesp estesp approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

@fuweid fuweid fuweid approved these changes

Assignees

No one assigned

Labels

cherry-picked/1.4.x PR commits are cherry picked into the release/1.4 branch

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cpuguy83 @estesp @AkihiroSuda @fuweid