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

Allow signals to be sent to gateway exec containers #2590

Merged
merged 1 commit into from
Feb 3, 2022

Conversation

coryb
Copy link
Collaborator

@coryb coryb commented Feb 1, 2022

fixes #2566

@tonistiigi tonistiigi added this to the v0.10.0 milestone Feb 1, 2022
Copy link
Member

@tonistiigi tonistiigi left a 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)
Copy link
Member

Choose a reason for hiding this comment

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

nit: lowercase errors

// 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 {
Copy link
Member

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

// startingProcess is to track the os process so we can send signals to it.
type startingProcess struct {
Process *os.Process
found chan struct{}
Copy link
Member

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>
@coryb
Copy link
Collaborator Author

coryb commented Feb 2, 2022

Fixed the comments, should be good now.

@coryb coryb merged commit a640b47 into moby:master Feb 3, 2022
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.

gateway: exec API should support sending signals to processes
2 participants