-
Notifications
You must be signed in to change notification settings - Fork 5.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
add kill process method when docker-compose down command #12206
base: main
Are you sure you want to change the base?
Conversation
Add killProcessesUsingExposedPorts call section in runDown func Signed-off-by: DOHYUN KIM <[email protected]>
Signed-off-by: DOHYUN KIM <[email protected]>
cmd := exec.Command("lsof", "-t", "-i", fmt.Sprintf(":%s", port)) | ||
output, err := cmd.Output() | ||
if err != nil { | ||
return fmt.Errorf("failed to find process using port %s: %v", port, err) | ||
} | ||
|
||
pid := strings.TrimSpace(string(output)) | ||
if pid == "" { | ||
return nil | ||
} | ||
|
||
killCmd := exec.Command("kill", "-9", pid) |
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.
This part of the code is not portable and won't work on Windows environment for example.
The fix proposed should work for Linux, Mac and Windows
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.
Also Compose is a client library, while docker engine may run on a remote machine, so this isn't the relevant place to propose such a fix. We already invoke ContainerStop
API which is responsible to stop (or kill after timeout) the target container and release resources, if you found a bug with this, better report on github.com/moby/moby
cmd := exec.Command("lsof", "-t", "-i", fmt.Sprintf(":%s", port)) | ||
output, err := cmd.Output() | ||
if err != nil { | ||
return fmt.Errorf("failed to find process using port %s: %v", port, err) | ||
} | ||
|
||
pid := strings.TrimSpace(string(output)) | ||
if pid == "" { | ||
return nil | ||
} | ||
|
||
killCmd := exec.Command("kill", "-9", pid) |
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.
Also Compose is a client library, while docker engine may run on a remote machine, so this isn't the relevant place to propose such a fix. We already invoke ContainerStop
API which is responsible to stop (or kill after timeout) the target container and release resources, if you found a bug with this, better report on github.com/moby/moby
What I did
Add kill process stage when docker-compose down command
Related issue
fixes 12205