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

Terminate child process by cleanup() in local-up-cluster.sh #8040

Merged
merged 1 commit into from
May 11, 2015
Merged

Terminate child process by cleanup() in local-up-cluster.sh #8040

merged 1 commit into from
May 11, 2015

Conversation

nak3
Copy link
Contributor

@nak3 nak3 commented May 10, 2015

Nevertheless some services are run with sudo in $./hack/local-up-cluster.sh, cleanup() desn't kill the child processes. Therefore some processes remains after stop terminate the running script.
This PR will change to kill the child processes and terminate the script cleanly.

@gmarek gmarek self-assigned this May 11, 2015
@gmarek gmarek added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 11, 2015
nikhiljindal added a commit that referenced this pull request May 11, 2015
@nikhiljindal
Merge pull request #8040 from nak3/terminate-child-process-local-up-c…
3d8f94e 
…luster

Terminate child process by cleanup() in local-up-cluster.sh
@nikhiljindal nikhiljindal merged commit 3d8f94e into kubernetes:master May 11, 2015
@nikhiljindal
Copy link
Contributor

Cleanup seems to be failing after syncing this change:
I get the following output:

Cleaning up...

Usage:
 kill [options] <pid> [...]

Options:
 <pid> [...]            send signal to every <pid> listed
 -<signal>, -s, --signal <signal>
                        specify the <signal> to be sent
 -l, --list=[<signal>]  list all signal names, or convert one to a name
 -L, --table            list all signal names in a nice table

 -h, --help     display this help and exit
 -V, --version  output version information and exit

For more details see kill(1).

Note that ./hack/local-up-cluster is broken right now (#7101 (comment)), so you can easily reproduce this on head.

@nak3 : Can you please take a look?

@nak3
Copy link
Contributor Author

nak3 commented May 13, 2015

It looks like ps --pid=PID --pid=PID doesn't work depending on the environment. I will fix them ASAP.

@pmorie
Copy link
Member

pmorie commented May 14, 2015

Just ran into this and created #8227

@nak3
Copy link
Contributor Author

nak3 commented May 14, 2015

I sent pull/8234

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@gmarek gmarek

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@nak3 @nikhiljindal @pmorie @gmarek @googlebot