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.

Sign up for GitHub

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

Jump to bottom

Convert SIGTERM into SIGINT in e2e test runner #6388

Merged
1 commit merged into from
Apr 3, 2015
Merged

Convert SIGTERM into SIGINT in e2e test runner #6388

1 commit merged into from
Apr 3, 2015
+21 −1

Conversation

ixdy
Copy link
Member

@ixdy ixdy commented Apr 2, 2015

Tested on my Linux workstation by running go run hack/e2e.go -v --test followed by kill -TERM -[pid_of_hack/e2e.go] - Ginkgo printed out its test stats, as it does when normally interrupted by ^C.

I'm not sure whether this change will cause problems on OS X.

@zmerlynn @quinton-hoole

@ghost
Copy link

ghost commented Apr 2, 2015

We should test this on OS X before merging. I can do that, but don't have time right now.

@roberthbailey
Copy link
Contributor

I'll test on OS X.

@zmerlynn
Copy link
Member

zmerlynn commented Apr 2, 2015

I don't think OS X should have any issues with this.

@zmerlynn
Copy link
Member

zmerlynn commented Apr 2, 2015

LGTM

@roberthbailey
Copy link
Contributor

This actually behaves kind of strange on my mac:

Window 1:
go run hack/e2e.go -v --test --check_version_skew=false
lots of output...

Window 2:

$ ps
46116 ttys002    0:00.05 go run hack/e2e.go -v --test --check_version_skew=false
46119 ttys002    0:00.00 /var/folders/00/13pp8000h01000cxqpysvccm004fts/T/go-build346548076/command-line-arguments/_obj/exe/e2e -v --test --check_version_skew=false
46120 ttys002    0:00.01 /bin/bash /Users/robertbailey/Work/go/src/github.com/roberthbailey/kubernetes/hack/e2e-internal/e2e-watch-events.sh
46121 ttys002    0:00.01 /bin/bash /Users/robertbailey/Work/go/src/github.com/roberthbailey/kubernetes/hack/e2e-internal/e2e-status.sh
46167 ttys002    0:00.01 /bin/bash /Users/robertbailey/Work/go/src/github.com/roberthbailey/kubernetes/cluster/kubectl.sh get events --watch-only
46194 ttys002    0:00.20 /Users/robertbailey/Work/go/src/github.com/roberthbailey/kubernetes/cluster/../cluster/gce/../../cluster/../_output/dockerized/bin/darwin/amd64
$ kill -TERM 46116
$ ps
46119 ttys002    0:00.00 /var/folders/00/13pp8000h01000cxqpysvccm004fts/T/go-build346548076/command-line-arguments/_obj/exe/e2e -v --test --check_version_skew=false
46120 ttys002    0:00.01 /bin/bash /Users/robertbailey/Work/go/src/github.com/roberthbailey/kubernetes/hack/e2e-internal/e2e-watch-events.sh
46121 ttys002    0:00.01 /bin/bash /Users/robertbailey/Work/go/src/github.com/roberthbailey/kubernetes/hack/e2e-internal/e2e-status.sh
46167 ttys002    0:00.01 /bin/bash /Users/robertbailey/Work/go/src/github.com/roberthbailey/kubernetes/cluster/kubectl.sh get events --watch-only
46194 ttys002    0:00.20 /Users/robertbailey/Work/go/src/github.com/roberthbailey/kubernetes/cluster/../cluster/gce/../../cluster/../_output/dockerized/bin/darwin/amd64

But then again, it works well to just Ctrl-C from within the same terminal window which does seem to clean up all of the jobs.

The purpose of this is to make Jenkins output better, and I don't think it really makes things any worse on my mac. I propose that we merge.

@ixdy
Copy link
Member Author

ixdy commented Apr 2, 2015

On Linux it also behaves the same as you noted; just sending a signal to the go run pid kills only that process. I killed with - before the pid to send TERM to the process group instead. (I'm not sure exactly what Jenkins does.)

ghost pushed a commit that referenced this pull request Apr 3, 2015
Merge pull request #6388 from ixdy/e2e-sigterm-sigint
dca645d 
Convert SIGTERM into SIGINT in e2e test runner
@ghost ghost merged commit dca645d into kubernetes:master Apr 3, 2015
@ixdy ixdy mentioned this pull request Apr 3, 2015
Closed
@ixdy ixdy deleted the e2e-sigterm-sigint branch August 21, 2015 21:55
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@zmerlynn zmerlynn

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ixdy @roberthbailey @zmerlynn @googlebot