From fe10caee8dcb850bdb97db404947337e6918e2ed Mon Sep 17 00:00:00 2001 From: Jason Lowe Date: Wed, 25 May 2016 21:37:31 +0000 Subject: [PATCH] YARN-4459. container-executor should only kill process groups. Contributed by Jun Gong (cherry picked from commit 1ba31fe9e906dbd093afd4b254216601967a4a7b) Conflicts: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c --- .../impl/container-executor.c | 19 +++------ .../test/test-container-executor.c | 42 ------------------- 2 files changed, 5 insertions(+), 56 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c index 4e547705f65..d583b7efaa5 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c @@ -1524,21 +1524,13 @@ int signal_container_as_user(const char *user, int pid, int sig) { } //Don't continue if the process-group is not alive anymore. - int has_group = 1; if (kill(-pid,0) < 0) { - if (kill(pid, 0) < 0) { - if (errno == ESRCH) { - return INVALID_CONTAINER_PID; - } - fprintf(LOGFILE, "Error signalling container %d with %d - %s\n", - pid, sig, strerror(errno)); - return -1; - } else { - has_group = 0; - } + fprintf(LOGFILE, "Error signalling not exist process group %d " + "with signal %d\n", pid, sig); + return INVALID_CONTAINER_PID; } - if (kill((has_group ? -1 : 1) * pid, sig) < 0) { + if (kill(-pid, sig) < 0) { if(errno != ESRCH) { fprintf(LOGFILE, "Error signalling process group %d with signal %d - %s\n", @@ -1552,8 +1544,7 @@ int signal_container_as_user(const char *user, int pid, int sig) { return INVALID_CONTAINER_PID; } } - fprintf(LOGFILE, "Killing process %s%d with %d\n", - (has_group ? "group " :""), pid, sig); + fprintf(LOGFILE, "Killing process group %d with %d\n", pid, sig); return 0; } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c index 3db75eab072..0306cb6fc6a 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c @@ -469,47 +469,6 @@ void run_test_in_child(const char* test_name, void (*func)()) { } } -void test_signal_container() { - printf("\nTesting signal_container\n"); - fflush(stdout); - fflush(stderr); - pid_t child = fork(); - if (child == -1) { - printf("FAIL: fork failed\n"); - exit(1); - } else if (child == 0) { - printf("\nSwitching to user %d\n", user_detail->pw_uid); - if (change_user(user_detail->pw_uid, user_detail->pw_gid) != 0) { - exit(1); - } - sleep(3600); - exit(0); - } else { - printf("Child container launched as %" PRId64 "\n", (int64_t)child); - printf("Signaling container as user %s\n", yarn_username); - // there's a race condition for child calling change_user and us - // calling signal_container_as_user, hence sleeping - sleep(3); - if (signal_container_as_user(yarn_username, child, SIGQUIT) != 0) { - exit(1); - } - int status = 0; - if (waitpid(child, &status, 0) == -1) { - printf("FAIL: waitpid failed - %s\n", strerror(errno)); - exit(1); - } - if (!WIFSIGNALED(status)) { - printf("FAIL: child wasn't signalled - %d\n", status); - exit(1); - } - if (WTERMSIG(status) != SIGQUIT) { - printf("FAIL: child was killed with %d instead of %d\n", - WTERMSIG(status), SIGQUIT); - exit(1); - } - } -} - void test_signal_container_group() { printf("\nTesting group signal_container\n"); fflush(stdout); @@ -825,7 +784,6 @@ int main(int argc, char **argv) { // the tests that change user need to be run in a subshell, so that // when they change user they don't give up our privs - run_test_in_child("test_signal_container", test_signal_container); run_test_in_child("test_signal_container_group", test_signal_container_group); // init app and run container can't be run if you aren't testing as root