-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Allow signals to be sent to gateway exec containers #2590
Conversation
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.
Looks good
func (ctrProc *containerProcess) Signal(_ context.Context, sig syscall.Signal) error { | ||
name := sigToName[sig] | ||
if name == "" { | ||
return errors.Errorf("Unknown signal %v", sig) |
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.
nit: lowercase errors
executor/runcexecutor/executor.go
Outdated
// WaitForStart will record the pid reported by Runc via the channel. | ||
// We wait for up to 10s for the runc process to start. If the started | ||
// callback is non-nil it will be called after receiving the pid. | ||
func (p *startingProcess) WaitForStart(ctx context.Context, startedCh chan int, started func()) error { |
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.
These definitions could use a channel direction constraint
executor/runcexecutor/executor.go
Outdated
// startingProcess is to track the os process so we can send signals to it. | ||
type startingProcess struct { | ||
Process *os.Process | ||
found chan struct{} |
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.
Maybe ready
better name in here
Signed-off-by: Cory Bennett <cbennett@netflix.com>
21f6dc5
to
559d079
Compare
Fixed the comments, should be good now. |
fixes #2566