Merge pull request #29685 from bboreham/kill-setpgid-3

Automatic merge from submit-queue

Fix killing child sudo process in e2e_node tests

Fixes #29211; re-doing #29380 which was reverted due to cross-platform build failure #29669.

The context is we are trying to kill a process started as `sudo kube-apiserver`, but `sudo` ignores signals from the same process group. Applying `Setpgid` means the `sudo kill` process won't be in the same process group, so will not fall foul of this nifty feature.

~~I also took the liberty of removing some code setting `Pdeathsig` because it claims to be doing something  in the same area, but actually it doesn't do that at all.  The setting is applied to the forked process, i.e. `sudo`, and it means the `sudo` will get killed if we (`e2e_node.test`) die.  This (a) isn't what the comment says and (b) doesn't help because sending SIGKILL to the sudo process leaves sudo's child alive.~~

We do need to use the same 'hack' to access `Setpgid` as the `e2e_node.test` program gets built on Windows (although it doesn't run there).

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.kubernetes.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.kubernetes.io/reviews/kubernetes/kubernetes/29685)
<!-- Reviewable:end -->
This commit is contained in:
Kubernetes Submit Queue 2016-08-11 05:38:17 -07:00 committed by GitHub
commit 6f7cc12c3c

View File

@ -312,12 +312,12 @@ func (es *e2eService) startServer(cmd *healthCheckCommand) error {
cmd.Cmd.Stdout = outfile
cmd.Cmd.Stderr = outfile
// Killing the sudo command should kill the server as well.
// Death of this test process should kill the server as well.
attrs := &syscall.SysProcAttr{}
// Hack to set linux-only field without build tags.
deathSigField := reflect.ValueOf(attrs).Elem().FieldByName("Pdeathsig")
if deathSigField.IsValid() {
deathSigField.Set(reflect.ValueOf(syscall.SIGKILL))
deathSigField.Set(reflect.ValueOf(syscall.SIGTERM))
} else {
cmdErrorChan <- fmt.Errorf("Failed to set Pdeathsig field (non-linux build)")
return
@ -389,7 +389,20 @@ func (k *killCmd) Kill() error {
const timeout = 10 * time.Second
for _, signal := range []string{"-TERM", "-KILL"} {
glog.V(2).Infof("Killing process %d (%s) with %s", pid, name, signal)
_, err := exec.Command("sudo", "kill", signal, strconv.Itoa(pid)).Output()
cmd := exec.Command("sudo", "kill", signal, strconv.Itoa(pid))
// Run the 'kill' command in a separate process group so sudo doesn't ignore it
attrs := &syscall.SysProcAttr{}
// Hack to set unix-only field without build tags.
setpgidField := reflect.ValueOf(attrs).Elem().FieldByName("Setpgid")
if setpgidField.IsValid() {
setpgidField.Set(reflect.ValueOf(true))
} else {
return fmt.Errorf("Failed to set Setpgid field (non-unix build)")
}
cmd.SysProcAttr = attrs
_, err := cmd.Output()
if err != nil {
glog.Errorf("Error signaling process %d (%s) with %s: %v", pid, name, signal, err)
continue