From 2636822524a95f8a84870e6beb4144305b0e11f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Sun, 14 Jul 2024 23:13:10 +0200 Subject: [PATCH 1/3] Adding success_exit_codes task config parameter to allow also non-zero exit codes (for external commands) to be treated as successful tasks. --- examples/job-config.yml | 28 ++++++++++++++++++++-- src/config/task_metadata.h | 16 ++++++++++++- src/helpers/config.cpp | 48 +++++++++++++++++++++++++++++++++++++ src/tasks/external_task.cpp | 30 +++++++++++++++++------ src/tasks/external_task.h | 9 +++++++ 5 files changed, 121 insertions(+), 10 deletions(-) diff --git a/examples/job-config.yml b/examples/job-config.yml index f64da67f..114a7512 100644 --- a/examples/job-config.yml +++ b/examples/job-config.yml @@ -25,7 +25,7 @@ tasks: args: - "/tmp/main.c" - "/tmp/hello_from_codex_worker/main.c" - - task-id: "eval_test01" + - task-id: "compile_test01" priority: 3 fatal-failure: false dependencies: @@ -54,9 +54,33 @@ tasks: - src: /tmp/hello_from_codex_worker dst: "evaluate" mode: RW - - task-id: "remove_created_dir" + - task-id: "run_test01" priority: 4 fatal-failure: false + dependencies: + - "compile_test01" + cmd: + bin: "./test" + args: [] + success_exit_codes: # list of exit codes which will be accepted as success + - 0 # zero is success by default, but if success_exit_codes is present, it must be added explicitly. + - 1 # single code + - [100, 200] # an interval (inclusive) + # codes 0, 1, 100, 101, ... 199, and 200 will all be accepted + sandbox: + name: "isolate" + limits: + - hw-group-id: "group1" # determines specific limits for specific machines + time: 1 # seconds + wall-time: 1 # seconds + extra-time: 1 # seconds + parallel: 0 # time and memory limits are merged from all potential processes/threads + environ-variable: + ISOLATE_BOX: "/box" + PATH: "/usr/bin" + - task-id: "remove_created_dir" + priority: 5 + fatal-failure: false cmd: bin: "rm" args: diff --git a/src/config/task_metadata.h b/src/config/task_metadata.h index 2e1d0588..578d4c07 100644 --- a/src/config/task_metadata.h +++ b/src/config/task_metadata.h @@ -37,7 +37,7 @@ class task_metadata std::shared_ptr sandbox = nullptr, std::string test_id = "") : task_id(task_id), priority(priority), dependencies(deps), test_id(test_id), type(type), fatal_failure(fatal), - binary(cmd), cmd_args(args), sandbox(sandbox) + binary(cmd), cmd_args(args), success_exit_codes({true}), sandbox(sandbox) { } @@ -59,6 +59,20 @@ class task_metadata std::string binary; /** Arguments for executed command. */ std::vector cmd_args; + /** + * List of command exit codes which should be treated as "success" (denoted by true value at their index). + * By default, the list has only one item - true at index 0. + */ + std::vector success_exit_codes; + + /** + * Safely checks whether given exit code is deemed successful (handling corner cases). + */ + bool is_success_exit_code(int code) const + { + // indices outside success_exit_codes range are handled as false + return code >= 0 && ((std::size_t) code) < success_exit_codes.size() && success_exit_codes[code]; + } /** If not null than this task is external and will be executed in given sandbox. */ std::shared_ptr sandbox; diff --git a/src/helpers/config.cpp b/src/helpers/config.cpp index 45255504..24cf630d 100644 --- a/src/helpers/config.cpp +++ b/src/helpers/config.cpp @@ -7,6 +7,50 @@ #include "config.h" +/** + * Properly add an exit-code or an interval of exit-codes in the bitmap. + */ +void add_exit_codes(std::vector &success_exit_codes, int from_index, int to_index = -1) +{ + if (to_index == -1) { to_index = from_index; } + + if (from_index < 0 || to_index > 255 || from_index > to_index) { return; } + + if (success_exit_codes.size() <= ((std::size_t) to_index)) { success_exit_codes.resize(to_index + 1); } + + for (int i = from_index; i <= to_index; ++i) { success_exit_codes[i] = true; } +} + + +/** + * Process the config node with success exit codes and fill a bitmap with their enabled indices. + * The config must be a single int value or a list of values. In case of a list, each item should be either integer or a + * tuple of two integers representing from-to range (inclusive). + * @param node root of yaml sub-tree with the exit codes. + * @param success_exit_codes a bitmap being constructed + */ +void load_task_success_exit_codes(const YAML::Node &node, std::vector &success_exit_codes) +{ + success_exit_codes.clear(); + if (node.IsScalar()) { + add_exit_codes(success_exit_codes, node.as()); + } else if (node.IsSequence()) { + for (auto &subnode : node) { + if (subnode.IsScalar()) { + add_exit_codes(success_exit_codes, subnode.as()); + } else if (subnode.IsSequence() && subnode.size() == 2) { + add_exit_codes(success_exit_codes, subnode[0].as(), subnode[1].as()); + } else { + throw helpers::config_exception( + "Success exit code must be a scalar (int) value or an interval (two integers in a list)"); + } + } + } else { + throw helpers::config_exception("Task command success_exit_codes must be an integer or a list."); + } +} + + std::shared_ptr helpers::build_job_metadata(const YAML::Node &conf) { std::shared_ptr job_meta = std::make_shared(); @@ -80,6 +124,10 @@ std::shared_ptr helpers::build_job_metadata(const YAML::Node &conf if (ctask["cmd"]["args"] && ctask["cmd"]["args"].IsSequence()) { task_meta->cmd_args = ctask["cmd"]["args"].as>(); } // can be omitted... no throw + + if (ctask["cmd"]["success_exit_codes"]) { + load_task_success_exit_codes(ctask["cmd"]["success_exit_codes"], task_meta->success_exit_codes); + } } else { throw config_exception("Command in task is not a map"); } diff --git a/src/tasks/external_task.cpp b/src/tasks/external_task.cpp index 67ab1667..4b7f4fa9 100644 --- a/src/tasks/external_task.cpp +++ b/src/tasks/external_task.cpp @@ -88,6 +88,9 @@ std::shared_ptr external_task::run() res->sandbox_status = std::unique_ptr(new sandbox_results(sandbox_->run(task_meta_->binary, task_meta_->cmd_args))); + // fix status if non-zero exit codes are treated as execution success + postprocess_exit_codes(res); + // get output from stdout and stderr get_results_output(res); @@ -102,6 +105,20 @@ std::shared_ptr external_task::run() return res; } +void external_task::postprocess_exit_codes(std::shared_ptr result) +{ + bool success = task_meta_->is_success_exit_code(result->sandbox_status->exitcode); + if (success && result->sandbox_status->status == isolate_status::RE) { + // we need to override the status to ok, based on acceptable exit-code + result->sandbox_status->status = isolate_status::OK; + result->sandbox_status->message = ""; + } else if (!success && result->sandbox_status->status == isolate_status::OK) { + // this happens if zero is not a successfuly exit-code + result->sandbox_status->status = isolate_status::RE; + result->sandbox_status->message = "Exited with code 0, which is not considered a success (exit codes override)"; + } +} + std::shared_ptr external_task::get_limits() { return limits_; @@ -168,14 +185,10 @@ void external_task::process_results_output( std_err.read(&result_stderr[0], max_length); // if there was something in stdout, write it to result - if (std_out.gcount() != 0) { - result->output_stdout = result_stdout.substr(0, std_out.gcount()); - } + if (std_out.gcount() != 0) { result->output_stdout = result_stdout.substr(0, std_out.gcount()); } // if there was something in stderr, write it to result - if (std_err.gcount() != 0) { - result->output_stderr = result_stderr.substr(0, std_err.gcount()); - } + if (std_err.gcount() != 0) { result->output_stderr = result_stderr.substr(0, std_err.gcount()); } // be nice and close streams std_out.close(); @@ -227,7 +240,10 @@ void external_task::make_binary_executable(const std::string &binary) // determine if file has executable bits set fs::file_status stat = status(binary_path); - if ((stat.permissions() & (fs::perms::owner_exec | fs::perms::group_exec | fs::perms::others_exec)) != fs::perms::none) { return; } + if ((stat.permissions() & (fs::perms::owner_exec | fs::perms::group_exec | fs::perms::others_exec)) != + fs::perms::none) { + return; + } fs::permissions( binary_path, fs::perms::owner_exec | fs::perms::group_exec | fs::perms::others_exec, fs::perm_options::add); diff --git a/src/tasks/external_task.h b/src/tasks/external_task.h index 9c029f26..6c9a74a7 100644 --- a/src/tasks/external_task.h +++ b/src/tasks/external_task.h @@ -74,17 +74,26 @@ class external_task : public task_base */ void make_binary_executable(const std::string &binary); + /** + * A task may accept different exit codes (than 0) as success. However, sandbox results may indicated nonzero exit + * codes as failures. This method fixes such situations. + */ + void postprocess_exit_codes(std::shared_ptr result); + /** * Initialize output if requested. */ void results_output_init(); + /** * Get configuration limited content of the stdout and stderr and return it. * @param result to which stdout and err will be assigned */ void get_results_output(std::shared_ptr result); + void process_results_output( const std::shared_ptr &result, const fs::path &stdout_path, const fs::path &stderr_path); + void process_carboncopy_output(const fs::path &stdout_path, const fs::path &stderr_path); /** Worker default configuration */ From 0315bd94fd145e95513fc4c5e8aa92c11c23197e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Sun, 14 Jul 2024 23:37:05 +0200 Subject: [PATCH 2/3] Adding tests for success exit codes. --- tests/job_config.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/job_config.cpp b/tests/job_config.cpp index 7c707535..3d254fe4 100644 --- a/tests/job_config.cpp +++ b/tests/job_config.cpp @@ -192,6 +192,14 @@ TEST(job_config_test, config_data) " args:\n" " - -v\n" " - \"-f 01.in\"\n" + " success_exit_codes:\n" + " - 1\n" + " - [3,5]\n" + " - [10,12]\n" + " - [11,14]\n" + " - [21,23]\n" + " - [22,24]\n" + " - [20,25]\n" " sandbox:\n" " name: fake\n" " stdin: 01.in\n" @@ -251,6 +259,14 @@ TEST(job_config_test, config_data) ASSERT_EQ(task2->cmd_args[0], "-v"); ASSERT_EQ(task2->cmd_args[1], "-f 01.in"); + std::vector codes{1, 3, 4, 5, 10, 11, 12, 13, 14, 20, 21, 22, 23, 24, 25}; + for (auto code : codes) { ASSERT_TRUE(task2->is_success_exit_code(code)); } + std::size_t false_count = 0; + for (int i = 0; i < 256; ++i) { + if (!task2->is_success_exit_code(i)) { ++false_count; } + } + ASSERT_EQ(false_count, 256 - codes.size()); + ASSERT_NE(task2->sandbox, nullptr); ASSERT_EQ(task2->sandbox->name, "fake"); ASSERT_EQ(task2->sandbox->std_input, "01.in"); From 5c69ab1373d1728220a8f5e82eef0d9b77140e37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Sun, 14 Jul 2024 23:46:16 +0200 Subject: [PATCH 3/3] Preparing and finalizing for build. --- CMakeLists.txt | 2 +- examples/job-config.yml | 4 ++-- src/config/task_results.h | 8 +++++++- src/helpers/config.cpp | 6 +++--- tests/job_config.cpp | 2 +- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 77a36714..b030187b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,6 +1,6 @@ cmake_minimum_required(VERSION 3.11.0) project(recodex-worker) -set(RECODEX_VERSION 1.8.0) +set(RECODEX_VERSION 1.9.0) enable_testing() set(EXEC_NAME ${PROJECT_NAME}) diff --git a/examples/job-config.yml b/examples/job-config.yml index 114a7512..c4462585 100644 --- a/examples/job-config.yml +++ b/examples/job-config.yml @@ -62,8 +62,8 @@ tasks: cmd: bin: "./test" args: [] - success_exit_codes: # list of exit codes which will be accepted as success - - 0 # zero is success by default, but if success_exit_codes is present, it must be added explicitly. + success-exit-codes: # list of exit codes which will be accepted as success + - 0 # zero is success by default, but if success-exit-codes is present, it must be added explicitly. - 1 # single code - [100, 200] # an interval (inclusive) # codes 0, 1, 100, 101, ... 199, and 200 will all be accepted diff --git a/src/config/task_results.h b/src/config/task_results.h index cec299ad..34c2c0ff 100644 --- a/src/config/task_results.h +++ b/src/config/task_results.h @@ -7,7 +7,13 @@ /** * Return error codes of sandbox. Code names corresponds isolate's meta file error codes. */ -enum class isolate_status { OK, RE, SG, TO, XX }; +enum class isolate_status { + OK, + RE, // run-time error, i.e., exited with a non-zero exit code + SG, // program died on a signal + TO, // timed out + XX, // internal error of the sandbox +}; /** * Status of whole task after execution. diff --git a/src/helpers/config.cpp b/src/helpers/config.cpp index 24cf630d..bb11de1d 100644 --- a/src/helpers/config.cpp +++ b/src/helpers/config.cpp @@ -46,7 +46,7 @@ void load_task_success_exit_codes(const YAML::Node &node, std::vector &suc } } } else { - throw helpers::config_exception("Task command success_exit_codes must be an integer or a list."); + throw helpers::config_exception("Task command success-exit-codes must be an integer or a list."); } } @@ -125,8 +125,8 @@ std::shared_ptr helpers::build_job_metadata(const YAML::Node &conf task_meta->cmd_args = ctask["cmd"]["args"].as>(); } // can be omitted... no throw - if (ctask["cmd"]["success_exit_codes"]) { - load_task_success_exit_codes(ctask["cmd"]["success_exit_codes"], task_meta->success_exit_codes); + if (ctask["cmd"]["success-exit-codes"]) { + load_task_success_exit_codes(ctask["cmd"]["success-exit-codes"], task_meta->success_exit_codes); } } else { throw config_exception("Command in task is not a map"); diff --git a/tests/job_config.cpp b/tests/job_config.cpp index 3d254fe4..f470fa0a 100644 --- a/tests/job_config.cpp +++ b/tests/job_config.cpp @@ -192,7 +192,7 @@ TEST(job_config_test, config_data) " args:\n" " - -v\n" " - \"-f 01.in\"\n" - " success_exit_codes:\n" + " success-exit-codes:\n" " - 1\n" " - [3,5]\n" " - [10,12]\n"