From b41c5e41bc46820912e8789837cca132db65b909 Mon Sep 17 00:00:00 2001 From: Jason Lowe Date: Fri, 13 Oct 2017 15:32:35 -0500 Subject: [PATCH] YARN-7246. Fix the default docker binary path. Contributed by Shane Kumpf --- .../impl/container-executor.c | 17 +++-- .../impl/container-executor.h | 4 ++ .../test/test-container-executor.c | 69 ++++++++++++++----- 3 files changed, 67 insertions(+), 23 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 b83b7687999..3b769646189 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 @@ -75,6 +75,7 @@ static const char* TC_READ_STATS_OPTS [] = { "-s", "-b", NULL}; static const int DEFAULT_DOCKER_SUPPORT_ENABLED = 0; static const int DEFAULT_TC_SUPPORT_ENABLED = 0; +static const char* DEFAULT_DOCKER_BINARY_PATH = "/usr/bin/docker"; //struct to store the user details struct passwd *user_detail = NULL; @@ -1305,9 +1306,17 @@ char* parse_docker_command_file(const char* command_file) { return ret; } +char *get_docker_binary() { + char *docker_binary = get_value(DOCKER_BINARY_KEY, &executor_cfg); + if (docker_binary == NULL) { + docker_binary = strdup(DEFAULT_DOCKER_BINARY_PATH); + } + return docker_binary; +} + int run_docker(const char *command_file) { char* docker_command = parse_docker_command_file(command_file); - char* docker_binary = get_value(DOCKER_BINARY_KEY, &executor_cfg); + char* docker_binary = get_docker_binary(); size_t command_size = MIN(sysconf(_SC_ARG_MAX), 128*1024); char* docker_command_with_binary = calloc(sizeof(char), command_size); @@ -1476,10 +1485,7 @@ int launch_docker_container_as_user(const char * user, const char *app_id, docker_rm_command = calloc(sizeof(char), command_size); char *docker_command = parse_docker_command_file(command_file); - char *docker_binary = get_value(DOCKER_BINARY_KEY, &executor_cfg); - if (docker_binary == NULL) { - docker_binary = "docker"; - } + char *docker_binary = get_docker_binary(); fprintf(LOGFILE, "Creating script paths...\n"); exit_code = create_script_paths( @@ -1653,6 +1659,7 @@ cleanup: free(exit_code_file); free(script_file_dest); free(cred_file_dest); + free(docker_binary); free(docker_command_with_binary); free(docker_wait_command); free(docker_logs_command); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h index 3eaf42e700c..82ce94b8d58 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h @@ -292,6 +292,10 @@ int traffic_control_read_state(char *command_file); */ int traffic_control_read_stats(char *command_file); +/** + * Get the docker binary path. + */ +char *get_docker_binary(); /** * Run a docker command passing the command file as an argument 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 a6e0236875d..438ac94f6c3 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 @@ -52,6 +52,7 @@ static char* username = NULL; static char* yarn_username = NULL; static char** local_dirs = NULL; static char** log_dirs = NULL; +static const char* DEFAULT_DOCKER_BINARY_PATH = "/usr/bin/docker"; /** * Run the command using the effective user id. @@ -102,7 +103,7 @@ void run(const char *cmd) { } } -int write_config_file(char *file_name, int banned) { +int write_config_file(char *file_name, int banned, char *docker_binary_path) { FILE *file; file = fopen(file_name, "w"); if (file == NULL) { @@ -115,6 +116,11 @@ int write_config_file(char *file_name, int banned) { } else { fprintf(file, "min.user.id=0\n"); } + if (docker_binary_path == NULL) { + fprintf(file, "#docker.binary=\n"); + } else { + fprintf(file, "docker.binary=%s\n", docker_binary_path); + } fprintf(file, "allowed.system.users=allowedUser,daemon\n"); fclose(file); return 0; @@ -224,7 +230,12 @@ void test_get_app_log_dir() { free(logdir); } -void test_check_user(int expectedFailure) { +void test_check_user(int expectedFailure, int enable_banned_user_conf) { + if (write_config_file(TEST_ROOT "/test.cfg", enable_banned_user_conf, NULL) != 0) { + exit(1); + } + read_executor_config(TEST_ROOT "/test.cfg"); + printf("\nTesting test_check_user\n"); struct passwd *user = check_user(username); if (user == NULL && !expectedFailure) { @@ -412,7 +423,7 @@ void test_delete_user() { char buffer[100000]; sprintf(buffer, "%s/test.cfg", app_dir); - if (write_config_file(buffer, 1) != 0) { + if (write_config_file(buffer, 1, NULL) != 0) { exit(1); } @@ -923,6 +934,35 @@ void test_sanitize_docker_command() { } } +void test_get_docker_binary() { + if (write_config_file(TEST_ROOT "/docker.cfg", 0, NULL) != 0) { + exit(1); + } + read_executor_config(TEST_ROOT "/docker.cfg"); + + char* docker_binary_null = get_docker_binary(); + printf("Docker binary path when not set: %s\n", docker_binary_null); + if(strcmp(DEFAULT_DOCKER_BINARY_PATH, docker_binary_null) != 0) { + printf("FAIL: expected output %s does not match actual output '%s'\n", DEFAULT_DOCKER_BINARY_PATH, docker_binary_null); + exit(1); + } + free(docker_binary_null); + + char* docker_binary_user = strdup("/bin/docker"); + if (write_config_file(TEST_ROOT "/docker.cfg", 0, docker_binary_user) != 0) { + exit(1); + } + read_executor_config(TEST_ROOT "/docker.cfg"); + char* docker_binary_set = get_docker_binary(); + printf("Docker binary path when set: %s\n", docker_binary_set); + if(strcmp(docker_binary_user, docker_binary_set) != 0) { + printf("FAIL: expected output %s does not match actual output '%s'\n", docker_binary_user, docker_binary_set); + exit(1); + } + free(docker_binary_set); + free(docker_binary_user); +} + // This test is expected to be executed either by a regular // user or by root. If executed by a regular user it doesn't // test all the functions that would depend on changing the @@ -951,14 +991,8 @@ int main(int argc, char **argv) { exit(1); } - if (write_config_file(TEST_ROOT "/test.cfg", 1) != 0) { - exit(1); - } - printf("\nOur executable is %s\n",get_executable(argv[0])); - read_executor_config(TEST_ROOT "/test.cfg"); - local_dirs = extract_values(strdup(NM_LOCAL_DIRS)); log_dirs = extract_values(strdup(NM_LOG_DIRS)); @@ -1017,7 +1051,10 @@ int main(int argc, char **argv) { printf("\nTesting sanitize docker commands()\n"); test_sanitize_docker_command(); - test_check_user(0); + printf("\nTesting get docker binary path()\n"); + test_get_docker_binary(); + + test_check_user(0, 1); #ifdef __APPLE__ printf("OS X: disabling CrashReporter\n"); @@ -1069,23 +1106,19 @@ int main(int argc, char **argv) { free_executor_configurations(); printf("\nTrying banned default user()\n"); - if (write_config_file(TEST_ROOT "/test.cfg", 0) != 0) { - exit(1); - } - read_executor_config(TEST_ROOT "/test.cfg"); #ifdef __APPLE__ username = "_uucp"; - test_check_user(1); + test_check_user(1, 0); username = "_networkd"; - test_check_user(1); + test_check_user(1, 0); #else username = "bin"; - test_check_user(1); + test_check_user(1, 0); username = "sys"; - test_check_user(1); + test_check_user(1, 0); #endif run("rm -fr " TEST_ROOT);