Skip to content
This repository has been archived by the owner on Mar 9, 2022. It is now read-only.

Fix ip leakage #949

Merged
merged 3 commits into from Oct 12, 2018
Merged

Fix ip leakage #949

merged 3 commits into from Oct 12, 2018

Conversation

Random-Liu
Copy link
Member

@Random-Liu Random-Liu commented Oct 10, 2018

Fixes #948.
Based on containerd/go-cni#31.

We should cherrypick this PR into all supported branches.

Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

see comment..

also .. need to update vendor I think

@@ -295,6 +298,15 @@ func KillProcess(name string) error {
return nil
}

// KillPid kills the process by pid. kill is used.
func KillPid(pid int) error {
output, err := exec.Command("kill", "-9", strconv.Itoa(pid)).CombinedOutput()
Copy link
Member

Choose a reason for hiding this comment

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

i think you wanted to send a SIGTERM here (-15) not a SIGKILL? or maybe the KillProcess is wrong as I see it's using default which I believe is SIGTERM.

Copy link
Member Author

Choose a reason for hiding this comment

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

Use kill directly, which uses SIGTERM by default.

Copy link
Member

Choose a reason for hiding this comment

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

:-) good

Signed-off-by: Lantao Liu <lantaol@google.com>
@Random-Liu Random-Liu force-pushed the fix-ip-leakage branch 2 times, most recently from 8fa56d0 to 79ae65e Compare October 10, 2018 20:12
Signed-off-by: Lantao Liu <lantaol@google.com>
@Random-Liu
Copy link
Member Author

need to update vendor I think

Done.

Signed-off-by: Lantao Liu <lantaol@google.com>
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

/LGTM

@Random-Liu Random-Liu merged commit 728f636 into containerd:master Oct 12, 2018
@Random-Liu Random-Liu deleted the fix-ip-leakage branch October 12, 2018 08:18
Random-Liu added a commit that referenced this pull request Oct 12, 2018
Random-Liu added a commit that referenced this pull request Oct 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants