From 2301b25899b5ae293719f4b4dcb8584c20a36bd5 Mon Sep 17 00:00:00 2001 From: Eric Yang Date: Fri, 10 Jan 2020 19:04:04 -0500 Subject: [PATCH] YARN-10019. Improved container-executor exec() calls. Contributed by Peter Bacsko --- .../impl/container-executor.c | 77 ++++++++----------- .../container-executor/impl/runc/runc.c | 2 +- .../native/container-executor/impl/util.h | 3 +- .../impl/utils/docker-util.c | 3 +- 4 files changed, 39 insertions(+), 46 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 3de736529a0..d69acf33abe 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 @@ -1610,7 +1610,7 @@ int exec_container(const char *command_file) { } } else { if (rc < 0) { - if (errno==5) { + if (errno == EIO) { fprintf(stderr, "Remote Connection Closed.\n"); exit(0); } else { @@ -1644,21 +1644,21 @@ int exec_container(const char *command_file) { tcsetattr (fds, TCSANOW, &new_term_settings); // The slave side of the PTY becomes the standard input and outputs of the child process - close(0); // Close standard input (current terminal) - close(1); // Close standard output (current terminal) - close(2); // Close standard error (current terminal) + close(STDIN_FILENO); // Close standard input (current terminal) + close(STDOUT_FILENO); // Close standard output (current terminal) + close(STDERR_FILENO); // Close standard error (current terminal) if (dup(fds) == -1) { // PTY becomes standard input (0) - exit(DOCKER_EXEC_FAILED); + _exit(DOCKER_EXEC_FAILED); } if (dup(fds) == -1) { // PTY becomes standard output (1) - exit(DOCKER_EXEC_FAILED); + _exit(DOCKER_EXEC_FAILED); } if (dup(fds) == -1) { // PTY becomes standard error (2) - exit(DOCKER_EXEC_FAILED); + _exit(DOCKER_EXEC_FAILED); } // Now the original file descriptor is useless @@ -1669,8 +1669,8 @@ int exec_container(const char *command_file) { setsid(); } else { exit_code = set_user(user); - if (exit_code!=0) { - goto cleanup; + if (exit_code != 0) { + _exit(exit_code); } } @@ -1679,24 +1679,20 @@ int exec_container(const char *command_file) { ioctl(0, TIOCSCTTY, 1); if (docker) { ret = execvp(binary, args); + fprintf(ERRORFILE, "exec failed - %s\n", strerror(errno)); + _exit(DOCKER_EXEC_FAILED); } else { if (change_user(user_detail->pw_uid, user_detail->pw_gid) != 0) { - exit_code = DOCKER_EXEC_FAILED; - goto cleanup; + _exit(DOCKER_EXEC_FAILED); } ret = chdir(workdir); if (ret != 0) { - exit_code = DOCKER_EXEC_FAILED; - goto cleanup; + fprintf(ERRORFILE, "chdir failed - %s", strerror(errno)); + _exit(DOCKER_EXEC_FAILED); } - ret = execve(binary, args, env); - } - if (ret != 0) { - fprintf(ERRORFILE, "Couldn't execute the container launch with args %s - %s\n", - binary, strerror(errno)); - exit_code = DOCKER_EXEC_FAILED; - } else { - exit_code = 0; + execve(binary, args, env); + fprintf(ERRORFILE, "exec failed - %s\n", strerror(errno)); + _exit(DOCKER_EXEC_FAILED); } } @@ -1708,7 +1704,8 @@ cleanup: free_values(args); free_values(env); free_configuration(&command_config); - return exit_code; + + return exit_code; // we reach this point only if an error occurs } int exec_docker_command(char *docker_command, char **argv, int argc) { @@ -2113,14 +2110,14 @@ int launch_docker_container_as_user(const char * user, const char *app_id, if (so_fd == NULL) { fprintf(ERRORFILE, "Could not append to %s\n", so); exit_code = UNABLE_TO_EXECUTE_CONTAINER_SCRIPT; - goto cleanup; + _exit(exit_code); } FILE* se_fd = fopen(se, "a+"); if (se_fd == NULL) { fprintf(ERRORFILE, "Could not append to %s\n", se); exit_code = UNABLE_TO_EXECUTE_CONTAINER_SCRIPT; fclose(so_fd); - goto cleanup; + _exit(exit_code); } // if entry point is enabled, clone docker command output // to stdout.txt and stderr.txt for yarn. @@ -2130,19 +2127,19 @@ int launch_docker_container_as_user(const char * user, const char *app_id, if (dup2(fileno(so_fd), fileno(stdout)) == -1) { fprintf(ERRORFILE, "Could not append to stdout.txt\n"); fclose(so_fd); - return UNABLE_TO_EXECUTE_CONTAINER_SCRIPT; + _exit(UNABLE_TO_EXECUTE_CONTAINER_SCRIPT); } if (dup2(fileno(se_fd), fileno(stderr)) == -1) { fprintf(ERRORFILE, "Could not append to stderr.txt\n"); fclose(se_fd); - return UNABLE_TO_EXECUTE_CONTAINER_SCRIPT; + _exit(UNABLE_TO_EXECUTE_CONTAINER_SCRIPT); } } fclose(so_fd); fclose(se_fd); execvp(docker_binary, docker_command); fprintf(ERRORFILE, "failed to execute docker command! error: %s\n", strerror(errno)); - return UNABLE_TO_EXECUTE_CONTAINER_SCRIPT; + _exit(UNABLE_TO_EXECUTE_CONTAINER_SCRIPT); } else { if (use_entry_point) { int pid = 0; @@ -2332,16 +2329,14 @@ int launch_container_as_user(const char *user, const char *app_id, // setsid pid_t pid = setsid(); if (pid == -1) { - exit_code = SETSID_OPER_FAILED; - goto cleanup; + _exit(SETSID_OPER_FAILED); } fprintf(LOGFILE, "Writing pid file...\n"); // write pid to pidfile if (pid_file == NULL || write_pid_to_file_as_nm(pid_file, pid) != 0) { - exit_code = WRITE_PIDFILE_FAILED; - goto cleanup; + _exit(WRITE_PIDFILE_FAILED); } #ifdef __linux @@ -2354,8 +2349,7 @@ int launch_container_as_user(const char *user, const char *app_id, *cgroup_ptr != NULL; ++cgroup_ptr) { if (strcmp(*cgroup_ptr, "none") != 0 && write_pid_to_cgroup_as_root(*cgroup_ptr, pid) != 0) { - exit_code = WRITE_CGROUP_FAILED; - goto cleanup; + _exit(WRITE_CGROUP_FAILED); } } } @@ -2368,7 +2362,7 @@ int launch_container_as_user(const char *user, const char *app_id, container_file_source, cred_file_source, keystore_file_source, truststore_file_source); if (exit_code != 0) { fprintf(ERRORFILE, "Could not create local files and directories\n"); - goto cleanup; + _exit(exit_code); } fprintf(LOGFILE, "Launching container...\n"); @@ -2385,13 +2379,10 @@ int launch_container_as_user(const char *user, const char *app_id, #endif umask(0027); - if (execlp(script_file_dest, script_file_dest, NULL) != 0) { - fprintf(LOGFILE, "Couldn't execute the container launch file %s - %s\n", - script_file_dest, strerror(errno)); - exit_code = UNABLE_TO_EXECUTE_CONTAINER_SCRIPT; - goto cleanup; - } - exit_code = 0; + execlp(script_file_dest, script_file_dest, NULL); + fprintf(LOGFILE, "Couldn't execute the container launch file %s - %s\n", + script_file_dest, strerror(errno)); + _exit(UNABLE_TO_EXECUTE_CONTAINER_SCRIPT); cleanup: free(exit_code_file); @@ -2982,7 +2973,7 @@ static int run_traffic_control(const char *opts[], char *command_file) { execv(TC_BIN, (char**)args); //if we reach here, exec failed fprintf(LOGFILE, "failed to execute tc command! error: %s\n", strerror(errno)); - return TRAFFIC_CONTROL_EXECUTION_FAILED; + _exit(TRAFFIC_CONTROL_EXECUTION_FAILED); } } @@ -3233,7 +3224,7 @@ int remove_docker_container(char**argv, int argc) { if (child_pid == 0) { // child int rc = exec_docker_command("rm", args, 2); - exit_code = rc; // Only get here if exec fails + _exit(rc); } else { // parent exit_code = wait_and_get_exit_code(child_pid); if (exit_code != 0) { diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/runc/runc.c b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/runc/runc.c index cde6d8add0b..0b252de403c 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/runc/runc.c +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/runc/runc.c @@ -885,7 +885,7 @@ int run_runc_container(const char* command_file) { pid_t child_pid = fork(); if (child_pid == 0) { exec_runc(rlc->container_id, runc_config_path, rlc->pid_file); - exit(1); // just in case exec_runc returns somehow + _exit(1); // just in case exec_runc returns somehow } else if (child_pid == -1) { fprintf(ERRORFILE, "Error cannot fork: %s\n", strerror(errno)); rc = OUT_OF_MEMORY; diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/util.h b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/util.h index b984a2337a3..920888f1eff 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/util.h +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/util.h @@ -103,7 +103,8 @@ enum errorcodes { DOCKER_SERVICE_MODE_DISABLED = 75, ERROR_RUNC_SETUP_FAILED = 76, ERROR_RUNC_RUN_FAILED = 77, - ERROR_RUNC_REAP_LAYER_MOUNTS_FAILED = 78 + ERROR_RUNC_REAP_LAYER_MOUNTS_FAILED = 78, + ERROR_DOCKER_CONTAINER_EXEC_FAILED = 79 }; /* Macros for min/max. */ diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.c b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.c index f35fd88bfbd..8bc66b30f64 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.c +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.c @@ -1169,7 +1169,8 @@ static int check_privileges(const char *user) { int child_pid = fork(); if (child_pid == 0) { execl("/usr/bin/sudo", "sudo", "-U", user, "-n", "-l", "docker", NULL); - exit(INITIALIZE_USER_FAILED); + fprintf(ERRORFILE, "could not invoke docker with sudo - %s\n", strerror(errno)); + _exit(INITIALIZE_USER_FAILED); } else { while ((waitid = waitpid(child_pid, &statval, 0)) != child_pid) { if (waitid == -1 && errno != EINTR) {