diff --git a/.github/workflows/windows-build.yml b/.github/workflows/windows-build.yml deleted file mode 100644 index eb554109..00000000 --- a/.github/workflows/windows-build.yml +++ /dev/null @@ -1,81 +0,0 @@ -name: Windows Build - -on: [push, pull_request] - -jobs: - tests: - runs-on: windows-2025 - steps: - - uses: actions/checkout@v4 - with: - submodules: true - - uses: ilammy/msvc-dev-cmd@v1 - - run: cmake --version - - run: nuget ? - - # cache - - name: Cache CMakeCache.txt and Nuget packages - uses: actions/cache@v4 - with: - path: | - CMakeCache.txt - ~/.nuget/packages - key: ${{ runner.os }}-cache - - # install - - run: mkdir -Force libs - - name: Install dependencies and prepare directory structure for cmake - working-directory: libs - run: | - nuget install boost -ExcludeVersion -Version 1.74.0 - mv -Force ./boost/lib/native/include/ ./boost/ - - nuget install boost_system-vc142 -ExcludeVersion -Version 1.74.0 - mv -Force ./boost_system-vc142/lib/native/* ./boost/lib/ - - nuget install boost_program_options-vc142 -ExcludeVersion -Version 1.74.0 - mv -Force ./boost_program_options-vc142/lib/native/* ./boost/lib/ - - nuget install rmt_zlib -ExcludeVersion -Version 1.2.8.7 - - nuget install rmt_libssh2 -ExcludeVersion -Version 1.8.0 - - nuget install rmt_curl -ExcludeVersion -Version 7.51.0 - mkdir -Force curl - cd curl - mkdir -Force include/curl - mkdir -Force lib - cd .. - mv -Force ./rmt_curl/build/native/include/v140/x64/Release/dynamic/* ./curl/include/curl/ - mv -Force ./rmt_curl/build/native/lib/v140/x64/Release/dynamic/* ./curl/lib/ - - nuget install libzmq_vc140 -ExcludeVersion -Version 4.3.2 - mkdir -Force libzmq - cd libzmq - mkdir -Force lib - cd .. - mv -Force ./libzmq_vc140/build/native/include ./libzmq/ - mv -Force ./libzmq_vc140/build/native/bin/libzmq-x64-v140-mt-4_3_2_0.imp.lib ./libzmq/lib/libzmq.lib - - working-directory: libs - run: Invoke-WebRequest -Uri 'https://curl.haxx.se/ca/cacert.pem' -Outfile "curl-ca-bundle.crt" - - # build - - run: cmake -G "Visual Studio 17 2022" "-DBOOST_ROOT=$Env:GITHUB_WORKSPACE\libs\boost" "-DCURL_LIBRARY=$Env:GITHUB_WORKSPACE\libs\curl\lib\libcurl_imp.lib" "-DCURL_INCLUDE_DIR=$Env:GITHUB_WORKSPACE\libs\curl\include" "-DZMQ_LIBRARY_DIR=$Env:GITHUB_WORKSPACE\libs\libzmq\lib" "-DZMQ_INCLUDE_DIR=$Env:GITHUB_WORKSPACE\libs\libzmq\include" . - - run: msbuild "ALL_BUILD.vcxproj" /p:Configuration=Release /m /verbosity:quiet - - # before_test - - name: Move DLLs of libraries to test folder - working-directory: libs - run: | - mv -Force libzmq_vc140/build/native/bin/libzmq-x64-v140-mt-4_3_2_0.dll $Env:GITHUB_WORKSPACE\tests\Release\libzmq.dll - mv -Force rmt_curl/build/native/bin/v140/x64/Release/dynamic/* $Env:GITHUB_WORKSPACE\tests\Release\ - mv -Force rmt_libssh2/build/native/bin/v140/x64/Release/dynamic/* $Env:GITHUB_WORKSPACE\tests\Release\ - mv -Force rmt_zlib/build/native/bin/v140/x64/Release/dynamic/* $Env:GITHUB_WORKSPACE\tests\Release\ - mv -Force curl-ca-bundle.crt $Env:GITHUB_WORKSPACE\tests\ - - # tests - - run: ctest -C Release -E tool_ --output-on-failure - working-directory: tests - - run: ctest -C Release -R tool_ --output-on-failure - continue-on-error: true - working-directory: tests diff --git a/CMakeLists.txt b/CMakeLists.txt index 7b92ebd2..38d3ad86 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.9.0) +set(RECODEX_VERSION 2.0.0) enable_testing() set(EXEC_NAME ${PROJECT_NAME}) @@ -135,6 +135,9 @@ set(SOURCE_FILES ${FILEMAN_DIR}/prefixed_file_manager.h ${SANDBOX_DIR}/sandbox_base.h + ${SANDBOX_DIR}/sandbox_base.cpp + ${SANDBOX_DIR}/guardian_sandbox.h + ${SANDBOX_DIR}/guardian_sandbox.cpp ${SANDBOX_DIR}/isolate_sandbox.h ${SANDBOX_DIR}/isolate_sandbox.cpp @@ -156,8 +159,8 @@ set(SOURCE_FILES ${TASKS_DIR}/internal/mkdir_task.cpp ${TASKS_DIR}/internal/rm_task.h ${TASKS_DIR}/internal/rm_task.cpp - ${TASKS_DIR}/internal/archivate_task.h - ${TASKS_DIR}/internal/archivate_task.cpp + ${TASKS_DIR}/internal/archive_task.h + ${TASKS_DIR}/internal/archive_task.cpp ${TASKS_DIR}/internal/extract_task.h ${TASKS_DIR}/internal/extract_task.cpp ${TASKS_DIR}/internal/fetch_task.h diff --git a/README.md b/README.md index 4949dcad..124bd60b 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,6 @@ # Worker [![Linux Build Status](https://github.com/ReCodEx/worker/workflows/Linux%20Build/badge.svg)](https://github.com/ReCodEx/worker/actions) -[![Windows Build Status](https://github.com/ReCodEx/worker/workflows/Windows%20Build/badge.svg)](https://github.com/ReCodEx/worker/actions) [![codecov](https://codecov.io/gh/ReCodEx/worker/branch/master/graph/badge.svg?token=AYHQA9R8PJ)](https://codecov.io/gh/ReCodEx/worker) [![License](http://img.shields.io/:license-mit-blue.svg)](http://badges.mit-license.org) [![Docs](https://img.shields.io/badge/docs-latest-brightgreen.svg)](http://recodex.github.io/worker/) @@ -45,11 +44,8 @@ use. The package names are for CentOS if not specified otherwise. (`libzmq3-dev` on Debian) - YAML-CPP library, `yaml-cpp` and `yaml-cpp-devel` (`libyaml-cpp0.5v5` and `libyaml-cpp-dev` on Debian) -- libcurl library `libcurl-devel` (`libcurl4-gnutls-dev` on Debian) -- libarchive library as optional dependency. Installing will speed up build - process, otherwise libarchive is built from source during installation. - Package name is `libarchive` and `libarchive-devel` (`libarchive-dev` on - Debian) +- libcurl library `libcurl-devel` (`libcurl4-dev` on Debian) +- libarchive library as optional dependency. Installing will speed up build process, otherwise libarchive is built from source during installation. Package name is `libarchive` and `libarchive-devel` (`libarchive-dev` on Debian) **Isolate** (only for Linux installations) @@ -125,21 +121,13 @@ manageable in long term horizon. #### Install worker on Windows -There are basically two main dependencies needed, **Windows 7** or higher and -**Visual Studio 2019+**. There is a simple installation batch script provided -which should do all the work on Windows machine. The script uses MsBuild from -VS2019 and 64-bit compilation, if you wish to use different compile option, -please revisit mentioned script. +We found no practical applications to run worker on Windows since we rely solely on linux-based (cgroups, namespaces, etc.) sandboxing. Therefore, it was decided to drop Windows support in 2026. The cmake and the code is still maintained in a way, that windows build is possible, but we no longer test it. -The script is placed in *install* directory alongside supportive scripts for -UNIX systems and is named *win-build.cmd*. Provided script will do almost -all the work connected with building and dependency resolving (using -**NuGet** package manager and `msbuild` building system). Script should be -run under 64-bit version of _Developer Command Prompt for VS2019_ and from -*install* directory. +There is a batch script that should help you started with the build process and some old notes are below: -Building and installing of worker is then quite simple, script has command line -parameters which can be used to specify what will be done: +The script is placed in *install* directory alongside supportive scripts for UNIX systems and is named *win-build.cmd*. Provided script will do almost all the work connected with building and dependency resolving (using **NuGet** package manager and `msbuild` building system). Script should be run under 64-bit version of _Developer Command Prompt for VS2019_ and from *install* directory. + +Building and installing of worker is then quite simple, script has command line parameters which can be used to specify what will be done: - *-build* -- It is the default options if none specified. Builds worker and its tests, all is saved in *build* folder and subfolders. @@ -157,11 +145,8 @@ install> win-build.cmd -test install> win-build.cmd -package ``` -All build binaries and cmake temporary files can be found in *build* folder, -classically there will be subfolder *Release* which will contain compiled -application with all needed dlls. Once if clickable installation binary is -created, it can be found in *build* folder under name -*recodex-worker-VERSION-x64.exe*. +All build binaries and cmake temporary files can be found in *build* folder, classically there will be subfolder *Release* which will contain compiled application with all needed dlls. Once if clickable installation binary is created, it can be found in *build* folder under name *recodex-worker-VERSION-x64.exe*. + #### Usage diff --git a/examples/config.yml b/examples/config.yml index c595f184..d845c4b3 100644 --- a/examples/config.yml +++ b/examples/config.yml @@ -35,6 +35,10 @@ logger: level: "debug" # level of logging - one of "debug", "warn", "emerg" max-size: 1048576 # 1 MB; max size of file before log rotation rotations: 3 # number of rotations kept +sandbox: "recodex-guardian" # if not specifies, recodex-guardian is used by default; "isolate" is also supported +sandbox-cpuset: # optional CG cpuset configuration for sandbox (relevant only for recodex-guardian) + cpus: "0-3" # list/range of CPUs to be used by the sandbox (empty/missing = all) + numa-nodes: "0" # list/range of NUMA nodes to be used by the sandbox (empty/missing = all) limits: time: 30 # seconds wall-time: 30 # seconds @@ -58,12 +62,12 @@ limits: # PYTHONHASHSEED: 0 # PYTHONIOENCODING: utf-8 # VIRTUAL_ENV: /path/to/python/venv - bound-directories: # All aditional dirs mapped into sandbox (SDKs, libs, venvs, ...), essential dirs (bin, dev, lib, usr, ...) are mapped automatically + bound-directories: # All additional dirs mapped into sandbox (SDKs, libs, venvs, ...), essential dirs (bin, dev, lib, usr, ...) are mapped automatically - dst: "/tmp" mode: "tmp" # if mode is omitted, dir is mounted as read-only (which is typical for lib dirs) #- src: "/usr/share/something" # actually existing directory # dst: "/something" # where the directory will be mounted in sandbox fs - # mode: "rw" # multiple modes can be separated by comma, see http://www.ucw.cz/moe/isolate.1.html (directory rules - options) + # mode: "rw" # multiple modes can be separated by comma max-output-length: 4096 # in bytes max-carboncopy-length: 1048576 # in bytes cleanup-submission: false # if true, then folders with data concerning submissions will be cleared after evaluation, should be used carefully, can produce huge amount of used disk space diff --git a/examples/job-config.yml b/examples/job-config.yml index c4462585..a75f3453 100644 --- a/examples/job-config.yml +++ b/examples/job-config.yml @@ -38,7 +38,7 @@ tasks: - "test" - "main.c" sandbox: # if defined task is external and will be run in sandbox - name: "isolate" # mandatory information + name: "" # empty string means default sandbox (set in config, recodex-guardian is the fallback) limits: # if not defined, then worker default configuration of limits is loaded # anything of the specified limits can be omitted and will be loaded from worker defaults - hw-group-id: "group1" # determines specific limits for specific machines @@ -68,7 +68,7 @@ tasks: - [100, 200] # an interval (inclusive) # codes 0, 1, 100, 101, ... 199, and 200 will all be accepted sandbox: - name: "isolate" + name: "" limits: - hw-group-id: "group1" # determines specific limits for specific machines time: 1 # seconds diff --git a/recodex-worker.spec b/recodex-worker.spec index 6a8220c4..a8bbc82f 100644 --- a/recodex-worker.spec +++ b/recodex-worker.spec @@ -1,7 +1,7 @@ %define name recodex-worker %define short_name worker -%define version 1.9.1 -%define unmangled_version 885c6bb4b3fa8e21400636f1bee1aefed19956e9 +%define version 2.0.0 +%define unmangled_version e66ddc766c796cda748c6af47ea2c6eef2dcda46 %define release 1 %define spdlog_name spdlog @@ -18,7 +18,7 @@ Prefix: %{_prefix} Vendor: Petr Stefan Url: https://github.com/ReCodEx/worker BuildRequires: systemd gcc-c++ cmake zeromq-devel cppzmq-devel yaml-cpp-devel libcurl-devel libarchive-devel boost-devel -Requires: systemd isolate +Requires: systemd #Source0: %{name}-%{unmangled_version}.tar.gz Source0: https://github.com/ReCodEx/%{short_name}/archive/%{unmangled_version}.tar.gz#/%{short_name}-%{unmangled_version}.tar.gz diff --git a/src/config/sandbox_config.h b/src/config/sandbox_config.h index ace2083f..4593ede3 100644 --- a/src/config/sandbox_config.h +++ b/src/config/sandbox_config.h @@ -16,59 +16,77 @@ class sandbox_config * Name of sandbox which will be used. */ std::string name = ""; + /** * Redirect standard input from given file. * @note Path must be accessible from inside of sandbox. */ std::string std_input = ""; + /** * Redirect standard output to given file. * @note Path must be accessible from inside of sandbox. */ std::string std_output = ""; + /** * Redirect standard error output to given file. * @note Path must be accessible from inside of sandbox. */ std::string std_error = ""; + /** * If true then stderr is redirected to stdout. */ bool stderr_to_stdout = false; + /** * If true then stdout and stderr will be written in the results. */ bool output = false; + /** * File to which stdout will be copied after execution. * Global worker limit for carboncopies is applied. * @note Path is outside the sandbox. */ std::string carboncopy_stdout = ""; + /** * File to which stderr will be copied after execution. * Global worker limit for carboncopies is applied. * @note Path is outside the sandbox. */ std::string carboncopy_stderr = ""; + /** * Change working directory to subdirectory inside the sandbox. * @note Path must be accessible from inside of sandbox. */ std::string chdir = ""; + /** * Working directory relative to the directory with the source files. */ std::string working_directory = ""; + /** * Associative array of loaded limits with textual index identifying its hw group. */ std::map> loaded_limits; + /** + * List of CPU cores to be used by the sandbox (empty = all). + * See worker_config::sandbox_cpus_ for more details. + */ + std::string cpus = ""; /** - * Constructor with defaults. + * List of NUMA nodes to be used by the sandbox (empty = all). + * See worker_config::sandbox_numa_nodes_ for more details. */ + std::string numa_nodes = ""; + sandbox_config() = default; }; diff --git a/src/config/sandbox_limits.h b/src/config/sandbox_limits.h index a983f7da..7e992b4e 100644 --- a/src/config/sandbox_limits.h +++ b/src/config/sandbox_limits.h @@ -29,6 +29,7 @@ struct sandbox_limits { * @warning Not all options must be supported by all sandboxes. Please, consult your sandbox documentation first. */ enum dir_perm : unsigned short { RO = 0, RW = 1, NOEXEC = 2, FS = 4, MAYBE = 8, DEV = 16, TMP = 32, NOREC = 64 }; + /** * Return a mapping between dir_perm enums and their associated string representatives. */ @@ -47,72 +48,86 @@ struct sandbox_limits { } return options; } + /** * Limit memory usage. For Isolate, this limits whole control group (--cg-mem switch). * Memory size is set in kilobytes. */ std::size_t memory_usage = 0; + /** * Extra memory which will be added to memory limit before killing program. * Memory size is set in kilobytes. */ std::size_t extra_memory = 0; + /** * Limit total run time by CPU time. For Isolate, this is for whole control group. * Time is set in seconds and can be fractional. */ float cpu_time = 0; + /** * Limit total run time by wall clock. Time is set in seconds and can be fractional. */ float wall_time = 0; + /** * Set extra time before kill the process. If program finishes in this extra amount of * time, it won't succeeded, but total run time will be reported to results log. This * time is also in (fractional) seconds. */ float extra_time = 0; + /** * Allow to share host computers network. Otherwise, dedicated * local interface will be created. */ bool share_net = false; + /** * Limit stack size. This is additional memory limit, 0 is no special limit for stack, - * global memory rules will aply. Otherwise, max stack size is @a stack_size kilobytes. + * global memory rules will apply. Otherwise, max stack size is @a stack_size kilobytes. */ std::size_t stack_size = 0; + /** * Limit size of created files. This could be useful, if your filesystem doesn't support * quotas. 0 means not set. * @warning This option is deprecated! Use @ref disk_size and @ref disk_files instead. */ std::size_t files_size = 0; + /** * Whether disk quotas (disk_size and disk_files) are enabled. * @warning Keep this false if underlying filesystem does not support quotas. */ bool disk_quotas = false; + /** * Set disk quota to given number of kilobytes. * @warning Underlying filesystem must support quotas. */ std::size_t disk_size = 0; + /** * Set disk quota to given number of files. Actual implementation may vary, for example * on Linux with ext4 filesystem this should be maximum number of used inodes. * @warning Underlying filesystem must support quotas. */ std::size_t disk_files = 0; + /** * Limit number of processes/threads that could be created. * 0 means no limit. */ std::size_t processes = 0; + /** * Set environment variables before run command inside the sandbox. */ std::vector> environ_vars; + /** * Contains local directories that should be bound into the sandbox. */ @@ -160,9 +175,9 @@ struct sandbox_limits { return (memory_usage == second.memory_usage && extra_memory == second.extra_memory && helpers::almost_equal(cpu_time, second.cpu_time) && helpers::almost_equal(wall_time, second.wall_time) && helpers::almost_equal(extra_time, second.extra_time) && stack_size == second.stack_size && - files_size == second.files_size && disk_size == second.disk_size && disk_files == second.disk_files && - processes == second.processes && share_net == second.share_net && environ_vars == second.environ_vars && - bound_dirs == second.bound_dirs); + files_size == second.files_size && disk_quotas == second.disk_quotas && disk_size == second.disk_size && + disk_files == second.disk_files && processes == second.processes && share_net == second.share_net && + environ_vars == second.environ_vars && bound_dirs == second.bound_dirs); } /** diff --git a/src/config/task_results.h b/src/config/task_results.h index 34c2c0ff..760f8f8f 100644 --- a/src/config/task_results.h +++ b/src/config/task_results.h @@ -5,7 +5,7 @@ #include /** - * Return error codes of sandbox. Code names corresponds isolate's meta file error codes. + * Return error codes of sandbox. */ enum class isolate_status { OK, @@ -23,7 +23,7 @@ enum class task_status { OK, FAILED, SKIPPED }; /** * Sandbox results. - * @note Not all items must be returned from sandbox, so some defaults may aply. + * @note Not all items must be returned from sandbox, so some defaults may apply. */ struct sandbox_results { /** @@ -42,7 +42,7 @@ struct sandbox_results { */ float wall_time = 0; /** - * Flag if program exited normaly or was killed. + * Flag if program exited normally or was killed. * Default: false */ bool killed = false; @@ -139,7 +139,7 @@ struct task_results { std::unique_ptr sandbox_status = nullptr; /** - * Constructor with default values initiazation. + * Constructor with default values initialization. */ task_results() = default; /** diff --git a/src/config/worker_config.cpp b/src/config/worker_config.cpp index 4f733da4..fd7f0ba4 100644 --- a/src/config/worker_config.cpp +++ b/src/config/worker_config.cpp @@ -111,7 +111,26 @@ worker_config::worker_config(const YAML::Node &config) } // no throw... can be omitted } // no throw... can be omitted - // load limits + // load default sandbox name (recodex-guardian is used if not defined) + if (config["sandbox"]) { + if (!config["sandbox"].IsScalar()) { throw config_error("Item sandbox must be a string, if present"); } + sandbox_name_ = config["sandbox"].as(); + if (sandbox_name_ != "recodex-guardian" && sandbox_name_ != "isolate") { + throw config_error("Only 'recodex-guardian' and 'isolate' sandboxes are supported for now"); + } + } + + // load sandbox cpus and numa nodes configuration (recodex-guardian only, isolate uses its own config) + if (config["sandbox-cpuset"] && config["sandbox-cpuset"].IsMap()) { + if (config["sandbox-cpuset"]["cpus"] && config["sandbox-cpuset"]["cpus"].IsScalar()) { + sandbox_cpus_ = config["sandbox-cpuset"]["cpus"].as(); + } // no throw... can be omitted + if (config["sandbox-cpuset"]["numa-nodes"] && config["sandbox-cpuset"]["numa-nodes"].IsScalar()) { + sandbox_numa_nodes_ = config["sandbox-cpuset"]["numa-nodes"].as(); + } // no throw... can be omitted + } + + // load sandbox default limits if (config["limits"] && config["limits"].IsMap()) { auto limits = config["limits"]; if (limits["time"] && limits["time"].IsScalar()) { @@ -231,6 +250,21 @@ const std::vector &worker_config::get_filemans_configs() const return filemans_configs_; } +const std::string &worker_config::get_sandbox_name() const +{ + return sandbox_name_; +} + +const std::string &worker_config::get_sandbox_cpus() const +{ + return sandbox_cpus_; +} + +const std::string &worker_config::get_sandbox_numa_nodes() const +{ + return sandbox_numa_nodes_; +} + const sandbox_limits &worker_config::get_limits() const { return limits_; diff --git a/src/config/worker_config.h b/src/config/worker_config.h index b471bc5f..e08d1802 100644 --- a/src/config/worker_config.h +++ b/src/config/worker_config.h @@ -35,7 +35,7 @@ class worker_config worker_config(const YAML::Node &config); /** - * Virtual destructor to avoid memory leaks when dealocating childs. + * Virtual destructor to avoid memory leaks when deallocating childs. */ virtual ~worker_config(); @@ -44,27 +44,32 @@ class worker_config * @return integer which can be used also as identifier/index of sandbox */ virtual std::size_t get_worker_id() const; + /** * Get worker human readable description (name), which will be shown in broker logs. * @return string with the description */ virtual const std::string &get_worker_description() const; + /** * Working directory path defined in config file. * Basically directory which is used as central point of work, all things should be done here. * @return textual representation of path */ virtual const std::string &get_working_directory() const; + /** * Defines address on which broker run. * @return textual representation of address or domain name and port */ virtual const std::string &get_broker_uri() const; + /** * Headers defined in configuration file, which will be sent to broker. * @return associative array */ virtual const header_map_t &get_headers() const; + /** * Gets hwgroup string description. * @return hardware group of this worker @@ -94,11 +99,32 @@ class worker_config * @return constant reference to log_config structure */ virtual const log_config &get_log_config() const; + /** * Get wrapper for file manager configuration. * @return constant reference to fileman_config structure */ virtual const std::vector &get_filemans_configs() const; + + /** + * Get name of the default sandbox which will be used for evaluations, + * if the job configuration doesn't specify any other sandbox. + * @return identifier of the sandbox + */ + virtual const std::string &get_sandbox_name() const; + + /** + * Get the default CPU cores for the sandbox. + * @return string representing the CPU cores + */ + virtual const std::string &get_sandbox_cpus() const; + + /** + * Get the default NUMA nodes for the sandbox. + * @return string representing the NUMA nodes + */ + virtual const std::string &get_sandbox_numa_nodes() const; + /** * Get default worker sandbox limits. Which will be used as defaults if not defined in job configuration. * @return non editable reference to sandbox_limits structure @@ -118,7 +144,7 @@ class worker_config virtual std::size_t get_max_carboncopy_length() const; /** - * Get flag which determines if cleanup is made after sumbission is evaluated. + * Get flag which determines if cleanup is made after submission is evaluated. * @return */ virtual bool get_cleanup_submission() const; @@ -126,32 +152,62 @@ class worker_config private: /** Unique worker number in context of one machine (0-100 preferably) */ std::size_t worker_id_ = 0; + /** Human readable description of the worker for logging purposes */ std::string worker_description_ = ""; + /** Working directory of whole worker used as base directory for all temporary files */ std::string working_directory_ = ""; + /** Broker URI, address where broker is listening */ std::string broker_uri_ = ""; + /** Header which are sent to broker and should specify worker abilities */ header_map_t headers_ = {}; + /** Hwgroup which is sent to broker and is used in job configuration to select right limits */ std::string hwgroup_ = {}; + /** Maximum number of pings in a row without response before the broker is considered disconnected */ std::size_t max_broker_liveness_ = 4; + /** How often should the worker ping the broker */ std::chrono::milliseconds broker_ping_interval_ = std::chrono::milliseconds(1000); + /** The caching directory path */ std::string cache_dir_ = ""; + /** Configuration of logger */ log_config log_config_ = {}; + /** Default configuration of file managers */ std::vector filemans_configs_ = {}; + + /** Default sandbox name */ + std::string sandbox_name_ = "recodex-guardian"; + + /** + * List of CPU cores to be used by the sandbox (empty = all). + * The format must follow linux documentation/cgroups/cpusets.txt. + * Examples: "0-3" first four cores, "0,2,4,6" cores with even IDs, "0-3,8-11" first four cores of two 8-core CPUs. + */ + std::string sandbox_cpus_ = ""; + + /** + * List of NUMA nodes to be used by the sandbox (empty = all). + * The format must follow linux documentation/cgroups/cpusets.txt (similar to cpus above). + */ + std::string sandbox_numa_nodes_ = ""; + /** Default sandbox limits */ sandbox_limits limits_ = {}; + /** Maximal length of output from sandbox which can be written to the results file, in bytes. */ std::size_t max_output_length_ = 0; - /** Maximal lenght of output from sandbox which can be copied into results folder, in bytes */ + + /** Maximal length of output from sandbox which can be copied into results folder, in bytes */ std::size_t max_carboncopy_length_ = 0; + /** If true then all files created during evaluation of job will be deleted at the end. */ bool cleanup_submission_ = true; }; diff --git a/src/helpers/config.cpp b/src/helpers/config.cpp index bb11de1d..41d83bcd 100644 --- a/src/helpers/config.cpp +++ b/src/helpers/config.cpp @@ -159,37 +159,35 @@ std::shared_ptr helpers::build_job_metadata(const YAML::Node &conf if (ctask["sandbox"]["name"] && ctask["sandbox"]["name"].IsScalar()) { sandbox->name = ctask["sandbox"]["name"].as(); - } else { - throw config_exception("Name of sandbox not given"); - } + } // can be omitted, will be filled from worker config... no throw if (ctask["sandbox"]["stdin"] && ctask["sandbox"]["stdin"].IsScalar()) { sandbox->std_input = ctask["sandbox"]["stdin"].as(); - } // can be ommited... no throw + } // can be omitted... no throw if (ctask["sandbox"]["stdout"] && ctask["sandbox"]["stdout"].IsScalar()) { sandbox->std_output = ctask["sandbox"]["stdout"].as(); - } // can be ommited... no throw + } // can be omitted... no throw if (ctask["sandbox"]["stderr"] && ctask["sandbox"]["stderr"].IsScalar()) { sandbox->std_error = ctask["sandbox"]["stderr"].as(); - } // can be ommited... no throw + } // can be omitted... no throw if (ctask["sandbox"]["stderr-to-stdout"] && ctask["sandbox"]["stderr-to-stdout"].IsScalar()) { sandbox->stderr_to_stdout = ctask["sandbox"]["stderr-to-stdout"].as(); - } // can be ommited... no throw + } // can be omitted... no throw if (ctask["sandbox"]["output"] && ctask["sandbox"]["output"].IsScalar()) { sandbox->output = ctask["sandbox"]["output"].as(); - } // can be ommited... no throw + } // can be omitted... no throw if (ctask["sandbox"]["carboncopy-stdout"] && ctask["sandbox"]["carboncopy-stdout"].IsScalar()) { sandbox->carboncopy_stdout = ctask["sandbox"]["carboncopy-stdout"].as(); - } // can be ommited... no throw + } // can be omitted... no throw if (ctask["sandbox"]["carboncopy-stderr"] && ctask["sandbox"]["carboncopy-stderr"].IsScalar()) { sandbox->carboncopy_stderr = ctask["sandbox"]["carboncopy-stderr"].as(); - } // can be ommited... no throw + } // can be omitted... no throw if (ctask["sandbox"]["chdir"] && ctask["sandbox"]["chdir"].IsScalar()) { sandbox->chdir = ctask["sandbox"]["chdir"].as(); - } // can be ommited... no throw + } // can be omitted... no throw if (ctask["sandbox"]["working-directory"] && ctask["sandbox"]["working-directory"].IsScalar()) { sandbox->working_directory = ctask["sandbox"]["working-directory"].as(); - } // can be ommited... no throw + } // can be omitted... no throw // load limits... if they are supplied if (ctask["sandbox"]["limits"]) { diff --git a/src/helpers/filesystem.cpp b/src/helpers/filesystem.cpp index 71d7603b..54121582 100644 --- a/src/helpers/filesystem.cpp +++ b/src/helpers/filesystem.cpp @@ -5,7 +5,7 @@ /** * Try to find matching hardlink in hardlinks map. If src is found in the map, dest is filled with corresponding file. * @param hardlinks the hardlinks map (src -> dst) - * @param src source path being looked up in hardlinks using equvalent func + * @param src source path being looked up in hardlinks using equivalent func * @param dest output arg which is filled in case of success * @return true if the hardlink match is found */ @@ -20,7 +20,7 @@ bool find_matching_hardlink(std::map &hardlinks, const fs::p return false; } -void copy_diretory_internal(const fs::path &src, const fs::path &dest, bool skip_symlinks, std::map &hardlinks) +void copy_directory_internal(const fs::path &src, const fs::path &dest, bool skip_symlinks, std::map &hardlinks) { try { // routine checks @@ -32,7 +32,7 @@ void copy_diretory_internal(const fs::path &src, const fs::path &dest, bool skip if (skip_symlinks && fs::is_symlink(src)) { return; } - + if (!fs::is_directory(fs::symlink_status(src))) { throw helpers::filesystem_exception( "helpers::copy_directory: Source directory is not a directory '" + src.string() + "'"); @@ -54,17 +54,18 @@ void copy_diretory_internal(const fs::path &src, const fs::path &dest, bool skip } if (fs::is_directory(it->symlink_status())) { - copy_diretory_internal(srcPath, destPath, skip_symlinks, hardlinks); - } else { + // recursively copy subdirectory + copy_directory_internal(srcPath, destPath, skip_symlinks, hardlinks); + } else if (fs::is_regular_file(it->symlink_status())) { // prevents copying of special files (sockets, fifos, etc.) // a file may be either copied or hardlinked if (!fs::is_symlink(srcPath) && fs::hard_link_count(srcPath) > 1) { fs::path destPathHardlink; if (find_matching_hardlink(hardlinks, it->path(), destPathHardlink)) { - // another file refering to the same data already exists in dest directory + // another file referring to the same data already exists in dest directory fs::create_hard_link(destPathHardlink, destPath); continue; // move to next file, hardlink replaced copying } else { - // this is the first time we encoutered this data, lets register them in hardlinks map + // this is the first time we encountered this data, lets register them in hardlinks map hardlinks.emplace(std::make_pair(it->path(), destPath)); } } @@ -83,13 +84,13 @@ void copy_diretory_internal(const fs::path &src, const fs::path &dest, bool skip void helpers::copy_directory(const fs::path &src, const fs::path &dest, bool skip_symlinks) { /* - * Hardlinks map provide mapping between files in src and dest which have been harlinked. + * Hardlinks map provide mapping between files in src and dest which have been hardlinked. * Everytime a file with > 1 hardlink count is copied from src to dest, it is registered here. * When files with > 1 hardlinks are encountered, this map is searched and if match is found * the new file is hardlinked inside dest instead of coping it from src. */ std::map hardlinks; - ::copy_diretory_internal(src, dest, skip_symlinks, hardlinks); + ::copy_directory_internal(src, dest, skip_symlinks, hardlinks); } fs::path helpers::normalize_path(const fs::path &path) diff --git a/src/helpers/logger.h b/src/helpers/logger.h index d773e85a..1bc7e75e 100644 --- a/src/helpers/logger.h +++ b/src/helpers/logger.h @@ -24,7 +24,7 @@ namespace helpers * Get unique identification of given log level. * More informative levels (debug, info) has greater values than error levels. * @param lev spdlog level enum type - * @return unique identificator + * @return unique identifier */ int get_log_level_number(spdlog::level::level_enum lev); diff --git a/src/job/job.cpp b/src/job/job.cpp index a868b95b..bff3d27c 100644 --- a/src/job/job.cpp +++ b/src/job/job.cpp @@ -116,8 +116,26 @@ void job::build_job() auto sandbox = task_meta->sandbox; + // + // IMPORTANT (30.9.2026): + // The following condition is temporarily commented, so the sandbox is selected merely from the + // worker configuration (overrides possible sandbox name in the job configuration). + // At the moment, API always chooses isolate, hence, this is necessary to test new recodex-guardian + // and allow old and new instances of the worker to run simultaneously during a transition period. + // TODO: restore the condition after the transition + // + // if (sandbox->name.empty()) { + // if the sandbox is not specified in the job, use worker config instead + sandbox->name = worker_config_->get_sandbox_name(); + // } + + // and let's make sure that one of the sandboxes is specified if (sandbox->name.empty()) { throw job_exception("Sandbox name cannot be empty"); } + // inject sandbox cpuset config from worker configuration + sandbox->cpus = worker_config_->get_sandbox_cpus(); + sandbox->numa_nodes = worker_config_->get_sandbox_numa_nodes(); + // first we have to get appropriate hwgroup limits std::shared_ptr limits; auto hwit = sandbox->loaded_limits.find(worker_config_->get_hwgroup()); @@ -242,6 +260,9 @@ void job::process_task_limits(const std::shared_ptr &limits) } else { if (limits->processes > worker_limits.processes) { throw job_exception("parallel" + msg); } } + + // turn on disk quotas if they are enforced by worker configuration + if (!limits->disk_quotas && worker_limits.disk_quotas) { limits->disk_quotas = true; } if (limits->disk_size == SIZE_MAX) { limits->disk_size = worker_limits.disk_size; } else { diff --git a/src/job/job.h b/src/job/job.h index 095daf07..8b84416a 100644 --- a/src/job/job.h +++ b/src/job/job.h @@ -26,7 +26,7 @@ namespace fs = std::filesystem; * Job is built from configuration in which all information should be provided. * Job building results in task tree and task queue in which task should be evaluated. * @note During construction job_metadata structure is given. This structure is editable and - * there is posibility it can be changed by whoever constructed a job class. + * there is possibility it can be changed by whoever constructed a job class. * In actual ReCodEx worker this situation can never happen. But be aware of this and keep it in mind in other coding. * Also do not change job_metadata or task_metadata structure between construction of tasks and its execution. * If you do it, you should watch your back, devil will be always very close! @@ -41,7 +41,7 @@ class job * @param job_meta * @param worker_conf * @param working_directory Directory for temporary saving of files by tasks. Example - * use case is storing isolate's meta log file. + * use case is storing sandbox's meta log file. * @param source_path path to source codes of submission * @param result_path path to directory containing all results * @param factory used in creation of task objects @@ -81,31 +81,38 @@ class job * Check directories given during construction for existence. */ void check_job_dirs(); + /** * Init system logger for job. Resulting log will be send with other results to frontend. */ void init_logger(); + /** * If given progress callback is nullptr, then make it empty callback so we can call it freely. */ void init_progress_callback(); + /** * Cleanup after job evaluation, should be enough to delete all created files */ void cleanup_job(); + /** * Build job from @a job_meta_. Should be called in constructor. */ void build_job(); + /** * Debug print of queue of tasks. */ void print_job_queue(); + /** * Check limits and in case of undefined values set worker defaults. * @param limits limits which will be checked */ void process_task_limits(const std::shared_ptr &limits); + /** * Given unconnected tasks will be connected according to their dependencies. * If they do not have dependency, they will be assigned to given root task. @@ -120,27 +127,36 @@ class job */ void prepare_job_vars(); /** - * Replace occurences of job config variables and return the resulting string + * Replace occurrences of job config variables and return the resulting string * @param src scanned string for variables * @return new string with all variables replaced with values */ std::string parse_job_var(const std::string &src); + // PRIVATE DATA MEMBERS + /** Information about this job given on construction. */ std::shared_ptr job_meta_; + /** Pointer on default worker config. */ std::shared_ptr worker_config_; + /** Directory, where tasks can create their own subfolders and temporary files. */ fs::path temporary_directory_; + /** Directory where source codes needed in job execution are stored. */ fs::path source_path_; + /** Directory where results and log of job are stored. */ fs::path result_path_; + /** Directory inside sandbox which should be bound as the working one. */ fs::path sandbox_working_path_; + /** Factory for creating tasks. */ std::shared_ptr factory_; + /** Progress callback which is called on some important points */ std::shared_ptr progress_callback_; @@ -149,6 +165,7 @@ class job /** Logical start of every job evaluation */ std::shared_ptr root_task_; + /** Tasks in linear ordering prepared for evaluation */ std::vector> task_queue_; diff --git a/src/sandbox/guardian_sandbox.cpp b/src/sandbox/guardian_sandbox.cpp new file mode 100644 index 00000000..4a16e1ba --- /dev/null +++ b/src/sandbox/guardian_sandbox.cpp @@ -0,0 +1,356 @@ +#ifndef _WIN32 + +#include "guardian_sandbox.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "helpers/filesystem.h" + +namespace fs = std::filesystem; + +guardian_sandbox::guardian_sandbox(std::shared_ptr sandbox_config, + sandbox_limits limits, + std::size_t id, + const std::string &temp_dir, + const std::string &data_dir, + std::shared_ptr logger) + : sandbox_base(sandbox_config, limits, id, temp_dir, data_dir, "recodex-guardian", logger) +{ +} + +void guardian_sandbox::sandbox_init() +{ + meta_file_ = (fs::path(temp_dir_) / "meta.log").string(); + + sandbox_log_pipe stdout_pipe(logger_), stderr_pipe(logger_); + pid_t childpid; + + logger_->debug("Initializing guardian..."); + + childpid = fork(); + + switch (childpid) { + case -1: log_and_throw(logger_, "Fork failed: ", strerror(errno)); break; + case 0: + stdout_pipe.child_dup_to_fd(1); + stderr_pipe.child_dup_to_fd(2); + guardian_init_child(); + break; + + default: + //---Parent--- + auto stdout_stream = stdout_pipe.parent_read_stream(); + auto stderr_stream = stderr_pipe.parent_read_stream(); + + // To prevent deadlocks, read from stderr first, since stdout is guaranteed to fit in PIPE_BUF. + std::string line; + while (std::getline(stderr_stream, line)) { logger_->warn("Guardian stderr: {}", line); } + + if (!std::getline(stdout_stream, sandboxed_dir_)) { + log_and_throw(logger_, "Error reading sandbox path from pipe."); + } + sandboxed_dir_ += "/box"; + + int status; + waitpid(childpid, &status, 0); + if (WEXITSTATUS(status) != 0) { + log_and_throw(logger_, "Guardian init error. Return value: ", WEXITSTATUS(status)); + } + logger_->debug("Guardian initialized in {}", sandboxed_dir_); + break; + } +} + +void guardian_sandbox::guardian_init_child() +{ + std::string box_id_arg("--box-id=" + std::to_string(id_)); + + // Exec guardian init command + std::vector args{ + sandbox_binary_.c_str(), + "--cg", + box_id_arg.c_str(), + }; + + std::string quota_arg; + if (limits_.disk_quotas) { + // Calculate number of required blocks - total number of bytes divided by block size + auto disk_size_blocks = (limits_.disk_size * 1024) / BLOCK_SIZE; // BLOCK_SIZE is from sys/mount.h + quota_arg = "--quota=" + std::to_string(disk_size_blocks) + "," + std::to_string(limits_.disk_files); + args.push_back(quota_arg.c_str()); + } + + args.push_back("--init"); + for (auto &it : args) { logger_->debug(" {}", it); } + args.push_back(nullptr); + + // const_cast is ugly, but this is working with C code - execv does not modify its arguments + execvp(sandbox_binary_.c_str(), const_cast(&args[0])); + + // never reached unless exec explodes in our face + log_and_throw(logger_, "Exec returned to child: ", strerror(errno)); +} + +void guardian_sandbox::sandbox_cleanup() +{ + sandbox_log_pipe stderr_pipe(logger_); + pid_t childpid; + + logger_->debug("Cleaning up guardian..."); + + childpid = fork(); + + switch (childpid) { + case -1: log_and_throw(logger_, "Fork failed: ", strerror(errno)); break; + case 0: + //---Child--- + stderr_pipe.child_dup_to_fd(2); + + // Exec guardian cleanup command + const char *args[5]; + args[0] = sandbox_binary_.c_str(); + args[1] = "--cg"; + args[2] = strdup(("--box-id=" + std::to_string(id_)).c_str()); + args[3] = "--cleanup"; + args[4] = NULL; + + // const_cast is ugly, but this is working with C code - execv does not modify its arguments + execvp(sandbox_binary_.c_str(), const_cast(args)); + + // Never reached + free(const_cast(args[2])); + + log_and_throw(logger_, "Exec returned to child: ", strerror(errno)); + break; + + default: + //---Parent--- + auto stderr_stream = stderr_pipe.parent_read_stream(); + std::string line; + while (std::getline(stderr_stream, line)) { logger_->warn("Guardian stderr: {}", line); } + + int status; + waitpid(childpid, &status, 0); + if (WEXITSTATUS(status) != 0) { + log_and_throw(logger_, "Guardian cleanup error. Return value: ", WEXITSTATUS(status)); + } + logger_->debug("Guardian box {} cleaned up.", id_); + break; + } +} + +void guardian_sandbox::sandbox_run(const std::string &binary, const std::vector &arguments) +{ + pid_t childpid; + + logger_->debug("Running guardian..."); + logger_->debug("Running the first fork"); + + childpid = fork(); + + switch (childpid) { + case -1: log_and_throw(logger_, "Fork failed: ", strerror(errno)); break; + case 0: { + //---Child--- + logger_->debug("Returned from the first fork as child"); + + // Redirect stderr and stdout to /dev/null file + int devnull; + devnull = open("/dev/null", O_WRONLY); + if (devnull == -1) { log_and_throw(logger_, "Cannot open /dev/null file for writing."); } + dup2(devnull, 0); // Don't allow process inside guardian to read from current standard input + dup2(devnull, 1); + dup2(devnull, 2); + + auto args = guardian_run_args(binary, arguments); + execvp(sandbox_binary_.c_str(), args); + + // Never reached + for (char **arg = args; *arg; arg++) { free(*arg); } + delete[] args; + + log_and_throw(logger_, "Exec returned to child: ", strerror(errno)); + } break; + default: { + //---Parent--- + /* Spawn a control process, that will wait given timeout and then kills guardian process. + * When a guardian process finishes before the timeout, parent thread kills control process + * and calls waitpid() to remove zombie from system. + */ + + logger_->debug("Returned from the first fork as parent"); + + pid_t controlpid; + logger_->debug("Running the second fork"); + controlpid = fork(); + switch (controlpid) { + case -1: log_and_throw(logger_, "Fork failed: ", strerror(errno)); break; + case 0: + // Child--- + { + logger_->debug("Returned from the second fork as child (control process)"); + + int remaining = max_timeout_; + // Sleep can be interrupted by signal, so make sure to sleep whole time + while (remaining > 0) { remaining = sleep(remaining); } + kill(childpid, SIGKILL); + } + break; + default: + // Parent--- + logger_->debug("Returned from the second fork as parent"); + + int status; + // Wait for guardian process. Waitpid returns no much longer than timeout if not earlier. + waitpid(childpid, &status, 0); + // Kill control process. If it already exits, nothing will be done + kill(controlpid, SIGKILL); + // Remove zombie from control process. + waitpid(controlpid, NULL, 0); + + // guardian was killed + if (WIFSIGNALED(status)) { + log_and_throw(logger_, "Guardian process was killed by signal ", WTERMSIG(status), " due to timeout."); + } + // guardian exited, but with return value signify internal error + if (WEXITSTATUS(status) != 0 && WEXITSTATUS(status) != 1) { + log_and_throw(logger_, "Guardian run into internal error. Return value: ", WEXITSTATUS(status)); + } + logger_->debug("Guardian box {} ran successfully.", id_); + break; + } + } break; + } +} + +char **guardian_sandbox::guardian_run_args(const std::string &binary, const std::vector &arguments) +{ + std::vector vargs; + + vargs.push_back(sandbox_binary_); // First argument must be binary name + vargs.push_back("--cg"); + vargs.push_back("--cg-timing"); + vargs.push_back("--box-id=" + std::to_string(id_)); + + if (!sandbox_config_->cpus.empty()) { vargs.push_back("--cpuset-cpus=" + sandbox_config_->cpus); } + if (!sandbox_config_->numa_nodes.empty()) { vargs.push_back("--cpuset-mems=" + sandbox_config_->numa_nodes); } + + vargs.push_back("--cg-mem=" + std::to_string(limits_.memory_usage + limits_.extra_memory)); + vargs.push_back("--time=" + std::to_string(limits_.cpu_time)); + vargs.push_back("--wall-time=" + std::to_string(limits_.wall_time)); + vargs.push_back("--extra-time=" + std::to_string(limits_.extra_time)); + if (limits_.stack_size != 0) { vargs.push_back("--stack=" + std::to_string(limits_.stack_size)); } + if (limits_.files_size != 0) { vargs.push_back("--fsize=" + std::to_string(limits_.files_size)); } + if (!sandbox_config_->std_input.empty()) { vargs.push_back("--stdin=" + sandbox_config_->std_input); } + if (!sandbox_config_->std_output.empty()) { vargs.push_back("--stdout=" + sandbox_config_->std_output); } + if (!sandbox_config_->std_error.empty()) { vargs.push_back("--stderr=" + sandbox_config_->std_error); } + if (sandbox_config_->stderr_to_stdout) { vargs.push_back("--stderr-to-stdout"); } + if (!sandbox_config_->chdir.empty()) { + // path is relative to /box inside sandbox ... we want path to be relative to root (/) + vargs.push_back("--chdir=" + (fs::path("..") / sandbox_config_->chdir).string()); + } + if (limits_.processes == 0) { + vargs.push_back("--processes"); + } else { + vargs.push_back("--processes=" + std::to_string(limits_.processes)); + } + if (limits_.share_net) { + vargs.push_back("--share-net"); + vargs.push_back("--dir=/etc"); // shared network requires /etc to work properly + } + for (auto &i : limits_.environ_vars) { vargs.push_back("--env=" + i.first + "=" + i.second); } + for (auto &i : limits_.bound_dirs) { + std::string mode = ""; + auto flags = std::get<2>(i); + for (const auto &kv : sandbox_limits::get_dir_perm_associated_strings()) { + if (flags & kv.first) { mode += ":" + kv.second; } + } + auto src = std::get<0>(i); + auto dst = std::get<1>(i); + std::string dirVal = (src == dst) ? src : (dst + "=" + src); + vargs.push_back(std::string("--dir=") + dirVal + mode); + } + // Bind /etc/alternatives directory if exists + vargs.push_back("--dir=etc/alternatives=/etc/alternatives:maybe"); + + vargs.push_back("--meta=" + meta_file_); + + vargs.push_back("--run"); + vargs.push_back("--"); + vargs.push_back(binary); + for (auto &i : arguments) { vargs.push_back(i); } + + // Convert string to char ** for execv call + char **c_args = new char *[vargs.size() + 1]; + int i = 0; + for (auto &it : vargs) { + c_args[i++] = strdup(it.c_str()); + logger_->debug(" {}", it); + } + c_args[i] = NULL; + return c_args; +} + +sandbox_results guardian_sandbox::extract_results() +{ + sandbox_results results; + + std::ifstream meta_stream; + meta_stream.open(meta_file_); + if (meta_stream.is_open()) { + std::string line; + while (std::getline(meta_stream, line)) { + std::size_t pos = line.find(':'); + std::size_t value_size = line.size() - (pos + 1); + auto first = line.substr(0, pos); + auto second = line.substr(pos + 1, value_size); + if (first == "time") { + results.time = std::stof(second); + } else if (first == "time-wall") { + results.wall_time = std::stof(second); + } else if (first == "killed") { + results.killed = true; + } else if (first == "status") { + if (second == "RE") { + results.status = isolate_status::RE; + } else if (second == "SG") { + results.status = isolate_status::SG; + } else if (second == "TO") { + results.status = isolate_status::TO; + } else if (second == "XX") { + results.status = isolate_status::XX; + } + } else if (first == "message") { + results.message = second; + } else if (first == "exitsig") { + results.exitsig = std::stoi(second); + } else if (first == "exitcode") { + results.exitcode = std::stoi(second); + } else if (first == "cg-mem") { + results.memory = std::stoul(second); + } else if (first == "max-rss") { + results.max_rss = std::stoul(second); + } else if (first == "csw-voluntary") { + results.csw_voluntary = std::stoul(second); + } else if (first == "csw-forced") { + results.csw_forced = std::stoul(second); + } + } + return results; + } else { + log_and_throw(logger_, "Cannot open ", meta_file_, " for reading."); + return results; // never reached + } +} + +#endif diff --git a/src/sandbox/guardian_sandbox.h b/src/sandbox/guardian_sandbox.h new file mode 100644 index 00000000..b3570ba3 --- /dev/null +++ b/src/sandbox/guardian_sandbox.h @@ -0,0 +1,62 @@ +#ifndef RECODEX_WORKER_FILE_GUARDIAN_SANDBOX_H +#define RECODEX_WORKER_FILE_GUARDIAN_SANDBOX_H + +#ifndef _WIN32 + +#include +#include +#include "helpers/logger.h" +#include "sandbox_base.h" +#include "config/sandbox_config.h" + +/** + * Class implementing operations with ReCodEx Guardian sandbox. + * + * Right now, guardian mimics the CLI API of isolate, so it can be used as + * direct replacement. This will be gradually modified in the future. + */ +class guardian_sandbox : public sandbox_base +{ +public: + /** + * Constructor. + * @param sandbox_config General sandbox configuration. + * @param limits Limits for current command. + * @param id Number of current worker. This must be unique for each worker on one machine! + * @param temp_dir Directory to store temporary files (generated sandbox's meta log) + * @param data_dit Directory containing sources which will be copied into sandbox + * @param logger Set system logger (optional). + */ + guardian_sandbox(std::shared_ptr sandbox_config, + sandbox_limits limits, + std::size_t id, + const std::string &temp_dir, + const std::string &data_dir, + std::shared_ptr logger = nullptr); + +private: + /** Path and name of guardian's meta file - here are stored informations about evaluation */ + std::string meta_file_; + + /** Initialize guardian (called in the constructor) */ + void sandbox_init() override; + + /** Run guardian evaluation with sandboxed program inside. */ + void sandbox_run(const std::string &binary, const std::vector &arguments) override; + + /** Cleanup guardian after the evaluation (called by the destructor) */ + void sandbox_cleanup() override; + + /** Actual code for guardian initialization inside a process. Called by sandbox_init(). */ + void guardian_init_child(); + + /** Get guardian command line arguments as plain C string including sandboxed binary with its arguments. */ + char **guardian_run_args(const std::string &binary, const std::vector &arguments); + + /** Parse guardian's meta file with evaluation informations. Must be called after sandbox_run() method. */ + sandbox_results extract_results() override; +}; + + +#endif // _WIN32 +#endif // RECODEX_WORKER_FILE_GUARDIAN_SANDBOX_H diff --git a/src/sandbox/isolate_sandbox.cpp b/src/sandbox/isolate_sandbox.cpp index 88d514b0..ba64d58c 100644 --- a/src/sandbox/isolate_sandbox.cpp +++ b/src/sandbox/isolate_sandbox.cpp @@ -14,137 +14,26 @@ #include #include #include -#include -#include #include "helpers/filesystem.h" +#include "helpers/logger.h" namespace fs = std::filesystem; -namespace -{ - void move_or_throw(std::shared_ptr logger, const std::string &from, const std::string &to) - { - try { - helpers::copy_directory(from, to, true); // true = skip symlinks for security reasons - } catch (fs::filesystem_error &e) { - log_and_throw(logger, "Failed moving ", from, " to ", to, ", error: ", e.what()); - } - - try { - fs::remove_all(from); - } catch (fs::filesystem_error &) { - } - } -} // namespace - isolate_sandbox::isolate_sandbox(std::shared_ptr sandbox_config, sandbox_limits limits, std::size_t id, const std::string &temp_dir, const std::string &data_dir, std::shared_ptr logger) - : sandbox_config_(sandbox_config), limits_(limits), logger_(logger), id_(id), isolate_binary_("isolate"), - data_dir_(data_dir) + : sandbox_base(sandbox_config, limits, id, temp_dir, data_dir, "isolate", logger) { - if (logger_ == nullptr) { logger_ = helpers::create_null_logger(); } - - if (sandbox_config_ == nullptr) { log_and_throw(logger_, "No sandbox configuration provided."); } - - if (data_dir_ == "") { logger_->info("Empty data directory for moving to sandbox."); } - - // Set backup limit (for killing isolate if it hasn't finished yet) - max_timeout_ = limits_.wall_time > limits_.cpu_time ? limits_.wall_time : limits_.cpu_time; - max_timeout_ += 300; // plus 5 minutes (for short tasks) - max_timeout_ *= 1.2; // 20% time more than necessary (better have some spare time) - - temp_dir_ = (fs::path(temp_dir) / std::to_string(id_)).string(); - try { - fs::create_directories(temp_dir_); - } catch (fs::filesystem_error &e) { - log_and_throw(logger_, "Failed to create directory for isolate meta file. Error: ", e.what()); - } - - meta_file_ = (fs::path(temp_dir_) / "meta.log").string(); - - try { - isolate_init(); - } catch (...) { - fs::remove_all(temp_dir_); - throw; - } -} - -isolate_sandbox::~isolate_sandbox() -{ - try { - isolate_cleanup(); - fs::remove_all(temp_dir_); - } catch (...) { - // We don't care if this failed. We can't fix it either. Just don't throw an exception in destructor. - } } -sandbox_results isolate_sandbox::run(const std::string &binary, const std::vector &arguments) +void isolate_sandbox::sandbox_init() { - // move data to isolate directory - if (data_dir_ != "") { move_or_throw(logger_, data_dir_, sandboxed_dir_); } - - try { - // run isolate - isolate_run(binary, arguments); - - // move data from isolate directory back to data directory - if (data_dir_ != "") { move_or_throw(logger_, sandboxed_dir_, data_dir_); } - } catch (const std::exception &) { - // on errors also move data from isolate directory back to data directory - if (data_dir_ != "") { move_or_throw(logger_, sandboxed_dir_, data_dir_); } - - // rethrow the original exception when data are saved - throw; - } - - return process_meta_file(); -} - -class log_pipe { - // A pipe through which a child process sends lines to a parent process. - - int read_fd_, write_fd_; // -1 = closed - std::shared_ptr logger_; - -public: - log_pipe(std::shared_ptr logger) { - logger_ = logger; - int fds[2]; - if (pipe(fds) < 0) { log_and_throw(logger_, "Cannot create pipe: %m"); } - read_fd_ = fds[0]; - write_fd_ = fds[1]; - } - - ~log_pipe() { - if (read_fd_ >= 0) { close(read_fd_); } - if (write_fd_ >= 0) { close(write_fd_); } - } - - void child_dup_to_fd(int fd) { - // Call in child process - dup2(write_fd_, fd); - close(read_fd_); - close(write_fd_); - read_fd_ = write_fd_ = -1; - } - - boost::iostreams::stream parent_read_stream() { - // Call in parent process - close(write_fd_); - write_fd_ = -1; - return boost::iostreams::stream(read_fd_, boost::iostreams::never_close_handle); - } -}; + meta_file_ = (fs::path(temp_dir_) / "meta.log").string(); -void isolate_sandbox::isolate_init() -{ - log_pipe stdout_pipe(logger_), stderr_pipe(logger_); + sandbox_log_pipe stdout_pipe(logger_), stderr_pipe(logger_); pid_t childpid; logger_->debug("Initializing isolate..."); @@ -163,12 +52,9 @@ void isolate_sandbox::isolate_init() auto stdout_stream = stdout_pipe.parent_read_stream(); auto stderr_stream = stderr_pipe.parent_read_stream(); - // To prevent deadlocks, read from stderr first, since stdout is guaranteed - // to fit in PIPE_BUF. + // To prevent deadlocks, read from stderr first, since stdout is guaranteed to fit in PIPE_BUF. std::string line; - while (std::getline(stderr_stream, line)) { - logger_->warn("Isolate: {}", line); - } + while (std::getline(stderr_stream, line)) { logger_->warn("Isolate stderr: {}", line); } if (!std::getline(stdout_stream, sandboxed_dir_)) { log_and_throw(logger_, "Error reading sandbox path from pipe."); @@ -190,8 +76,8 @@ void isolate_sandbox::isolate_init_child() std::string box_id_arg("--box-id=" + std::to_string(id_)); // Exec isolate init command - std::vector args { - isolate_binary_.c_str(), + std::vector args{ + sandbox_binary_.c_str(), "--cg", box_id_arg.c_str(), }; @@ -208,15 +94,15 @@ void isolate_sandbox::isolate_init_child() args.push_back(nullptr); // const_cast is ugly, but this is working with C code - execv does not modify its arguments - execvp(isolate_binary_.c_str(), const_cast(&args[0])); + execvp(sandbox_binary_.c_str(), const_cast(&args[0])); // never reached unless exec explodes in our face log_and_throw(logger_, "Exec returned to child: ", strerror(errno)); } -void isolate_sandbox::isolate_cleanup() +void isolate_sandbox::sandbox_cleanup() { - log_pipe stderr_pipe(logger_); + sandbox_log_pipe stderr_pipe(logger_); pid_t childpid; logger_->debug("Cleaning up isolate..."); @@ -231,14 +117,14 @@ void isolate_sandbox::isolate_cleanup() // Exec isolate cleanup command const char *args[5]; - args[0] = isolate_binary_.c_str(); + args[0] = sandbox_binary_.c_str(); args[1] = "--cg"; args[2] = strdup(("--box-id=" + std::to_string(id_)).c_str()); args[3] = "--cleanup"; args[4] = NULL; // const_cast is ugly, but this is working with C code - execv does not modify its arguments - execvp(isolate_binary_.c_str(), const_cast(args)); + execvp(sandbox_binary_.c_str(), const_cast(args)); // Never reached free(const_cast(args[2])); @@ -249,9 +135,7 @@ void isolate_sandbox::isolate_cleanup() //---Parent--- auto stderr_stream = stderr_pipe.parent_read_stream(); std::string line; - while (std::getline(stderr_stream, line)) { - logger_->warn("Isolate: {}", line); - } + while (std::getline(stderr_stream, line)) { logger_->warn("Isolate: {}", line); } int status; waitpid(childpid, &status, 0); @@ -263,7 +147,7 @@ void isolate_sandbox::isolate_cleanup() } } -void isolate_sandbox::isolate_run(const std::string &binary, const std::vector &arguments) +void isolate_sandbox::sandbox_run(const std::string &binary, const std::vector &arguments) { pid_t childpid; @@ -287,10 +171,10 @@ void isolate_sandbox::isolate_run(const std::string &binary, const std::vector &arguments) { + if (!sandbox_config_->cpus.empty() || !sandbox_config_->numa_nodes.empty()) { + logger_->debug("The worker has sandbox-cpuset parameters configured, but these are ignored by isolate. Isolate " + "uses its own config for that."); + } + std::vector vargs; - vargs.push_back(isolate_binary_); // First argument must be binary name + vargs.push_back(sandbox_binary_); // First argument must be binary name vargs.push_back("--cg"); - vargs.push_back("--cg-timing"); + // vargs.push_back("--cg-timing"); // MJ recommended removing this one in isolate v2 vargs.push_back("--box-id=" + std::to_string(id_)); vargs.push_back("--cg-mem=" + std::to_string(limits_.memory_usage + limits_.extra_memory)); @@ -413,7 +302,7 @@ char **isolate_sandbox::isolate_run_args(const std::string &binary, const std::v return c_args; } -sandbox_results isolate_sandbox::process_meta_file() +sandbox_results isolate_sandbox::extract_results() { sandbox_results results; diff --git a/src/sandbox/isolate_sandbox.h b/src/sandbox/isolate_sandbox.h index cdcfcd50..013614d2 100644 --- a/src/sandbox/isolate_sandbox.h +++ b/src/sandbox/isolate_sandbox.h @@ -5,9 +5,7 @@ #include #include -#include "helpers/logger.h" #include "sandbox_base.h" -#include "config/sandbox_config.h" /** * Class implementing operations with Isolate sandbox. @@ -45,43 +43,28 @@ class isolate_sandbox : public sandbox_base const std::string &temp_dir, const std::string &data_dir, std::shared_ptr logger = nullptr); - /** - * Destructor. - */ - ~isolate_sandbox() override; - sandbox_results run(const std::string &binary, const std::vector &arguments) override; private: - /** General sandbox configuration */ - std::shared_ptr sandbox_config_; - /** Limits for sandboxed program */ - sandbox_limits limits_; - /** Logger */ - std::shared_ptr logger_; - /** Identifier of this isolate's instance. Must be unique on each server. */ - std::size_t id_; - /** Name of isolate binary - defaults "isolate" */ - std::string isolate_binary_; - /** Path to temporary directory used by sandboxes. Subdir with "id_" value will be created. */ - std::string temp_dir_; /** Path and name of isolate's meta file - here are stored informations about evaluation */ std::string meta_file_; - /** Maximum time to run separate isolate process */ - int max_timeout_; - /** Path to the directory containing sources moved to sandbox and back */ - std::string data_dir_; - /** Initialize isolate */ - void isolate_init(); - /** Actual code for isolate initialization inside a process. Called by isolate_init(). */ - void isolate_init_child(); - /** Cleanup isolate after finish evaluation */ - void isolate_cleanup(); + + /** Initialize isolate (called in the constructor) */ + void sandbox_init() override; + /** Run isolate evaluation with sandboxed program inside. */ - void isolate_run(const std::string &binary, const std::vector &arguments); + void sandbox_run(const std::string &binary, const std::vector &arguments) override; + + /** Cleanup isolate after finish evaluation (called in the destructor) */ + void sandbox_cleanup() override; + + /** Actual code for isolate initialization inside a process. Called by sandbox_init(). */ + void isolate_init_child(); + /** Get isolate command line arguments as plain C string including sandboxed binary with its arguments. */ char **isolate_run_args(const std::string &binary, const std::vector &arguments); - /** Parse isolate's meta file with evaluation informations. Must be called after isolate_run() method. */ - sandbox_results process_meta_file(); + + /** Parse isolate's meta file with evaluation informations. Must be called after sandbox_run() method. */ + sandbox_results extract_results() override; }; diff --git a/src/sandbox/sandbox_base.cpp b/src/sandbox/sandbox_base.cpp new file mode 100644 index 00000000..51864c5a --- /dev/null +++ b/src/sandbox/sandbox_base.cpp @@ -0,0 +1,134 @@ +#include "sandbox_base.h" + +#include +#include "helpers/filesystem.h" +#include "helpers/logger.h" + +sandbox_base::sandbox_base(std::shared_ptr sandbox_config, + sandbox_limits limits, + std::size_t id, + const std::string &temp_dir, + const std::string &data_dir, + const std::string &sandbox_binary, + std::shared_ptr logger) + : sandbox_config_(sandbox_config), limits_(limits), logger_(logger), id_(id), data_dir_(data_dir), + sandbox_binary_(sandbox_binary) +{ + if (logger_ == nullptr) { + // logger must not be empty, place dummy instead + logger_ = helpers::create_null_logger(); + } + + if (sandbox_config_ == nullptr) { log_and_throw(logger_, "No sandbox configuration provided."); } + + if (data_dir_ == "") { logger_->info("Empty data directory for moving to sandbox."); } + + // Set backup limit (for killing isolate if it hasn't finished yet) + max_timeout_ = limits_.wall_time > limits_.cpu_time ? limits_.wall_time : limits_.cpu_time; + max_timeout_ += 300; // plus 5 minutes (for short tasks) + max_timeout_ *= 1.2; // 20% time more than necessary (better have some spare time) + + temp_dir_ = (fs::path(temp_dir) / std::to_string(id_)).string(); + try { + fs::create_directories(temp_dir_); + } catch (fs::filesystem_error &e) { + log_and_throw(logger_, "Failed to create temp directory for the sandbox. Error: ", e.what()); + } +} + +sandbox_results sandbox_base::execute_in_sandbox(const std::string &binary, const std::vector &arguments) +{ + try { + sandbox_init(); + } catch (...) { + fs::remove_all(temp_dir_); + throw; + } + + // move data to the sandboxed directory + if (data_dir_ != "") { move_or_throw(logger_, data_dir_, sandboxed_dir_); } + + try { + // run the sandbox (virtual method implemented in child class) + sandbox_run(binary, arguments); + + } catch (const std::exception &e_run) { + try { + // on errors also move data from the sandboxed directory back to data directory + // but we need to do it safely, so the original exception is rethrown after this + if (data_dir_ != "") { move_or_throw(logger_, sandboxed_dir_, data_dir_); } + } catch (const std::exception &e) { + logger_->error("When ", sandbox_binary_, " execution failed... ", e.what()); + } + + // rethrow the original exception when data are saved + throw e_run; + } + + // move data from the sandboxed directory back to data directory (regular case) + if (data_dir_ != "") { move_or_throw(logger_, sandboxed_dir_, data_dir_); } + + auto results = extract_results(); // this is also virtual method + + sandbox_cleanup(); + fs::remove_all(temp_dir_); + + return results; +} + + +/* + * Helper functions + */ + +void move_or_throw(std::shared_ptr logger, const std::string &from, const std::string &to) +{ + try { + helpers::copy_directory(from, to, true); // true = skip symlinks for security reasons + } catch (fs::filesystem_error &e) { + log_and_throw(logger, "Failed moving ", from, " to ", to, ", error: ", e.what()); + } + + try { + fs::remove_all(from); + } catch (fs::filesystem_error &) { + // deliberately ignore this error + } +} + +/* + * Helper class sandbox_log_pipe + */ + +sandbox_log_pipe::sandbox_log_pipe(std::shared_ptr logger) +{ + logger_ = logger; + int fds[2]; + if (pipe(fds) < 0) { log_and_throw(logger_, "Cannot create pipe: %m"); } + read_fd_ = fds[0]; + write_fd_ = fds[1]; +} + +sandbox_log_pipe::~sandbox_log_pipe() +{ + if (read_fd_ >= 0) { close(read_fd_); } + if (write_fd_ >= 0) { close(write_fd_); } +} + +void sandbox_log_pipe::child_dup_to_fd(int fd) +{ + // Call in child process + dup2(write_fd_, fd); + close(read_fd_); + close(write_fd_); + read_fd_ = write_fd_ = -1; +} + +boost::iostreams::stream sandbox_log_pipe::parent_read_stream() +{ + // Call in parent process + close(write_fd_); + write_fd_ = -1; + return boost::iostreams::stream( + read_fd_, boost::iostreams::never_close_handle); +} diff --git a/src/sandbox/sandbox_base.h b/src/sandbox/sandbox_base.h index 8d9d35ef..201f9064 100644 --- a/src/sandbox/sandbox_base.h +++ b/src/sandbox/sandbox_base.h @@ -4,10 +4,12 @@ #include #include #include -#include #include #include +#include +#include #include "spdlog/spdlog.h" +#include "config/sandbox_config.h" #include "config/sandbox_limits.h" #include "config/task_results.h" #include "helpers/format.h" @@ -15,6 +17,14 @@ /** * Base class for all sandbox implementations. + * + * Sandbox is used for security of system running untrusted program. They impose + * sets restrictions to the application like time limit, memory limit or accessible + * files. When any of the limits are reached, the program inside sandbox is killed. + * + * This is a base class for different sandbox implementations that may be used by the worker, + * but the common code is shared in this class. At present, we only support cg-based sandboxes + * for linux systems which is reflected in this interface. */ class sandbox_base { @@ -38,7 +48,7 @@ class sandbox_base * @param arguments Commandline arguments to the binary. * @return Sandbox results. */ - virtual sandbox_results run(const std::string &binary, const std::vector &arguments) = 0; + virtual sandbox_results execute_in_sandbox(const std::string &binary, const std::vector &arguments); protected: /** @@ -46,6 +56,60 @@ class sandbox_base * @warning Must be set in constructor of child class. */ std::string sandboxed_dir_; + + /** General sandbox configuration */ + std::shared_ptr sandbox_config_; + + /** Limits for sandboxed program */ + sandbox_limits limits_; + + /** Logger */ + std::shared_ptr logger_; + + /** Identifier of this sandbox's instance. Must be unique on each server. */ + std::size_t id_; + + /** Path to temporary directory used by sandboxes. Subdir with "id_" value will be created. */ + std::string temp_dir_; + + /** Maximum time to run the separate sandbox process */ + int max_timeout_; + + /** Path to the directory containing sources moved to sandbox and back */ + std::string data_dir_; + + /** Name of sandbox binary (also used to identify the sandbox in logs) */ + std::string sandbox_binary_; + + /** + * Constructor is protected, derived classes should make it public. + * @param sandbox_config General sandbox configuration. + * @param limits Limits for current command. + * @param id Number of current worker. This must be unique for each worker on one machine! + * @param temp_dir Directory to store temporary files (generated isolate's meta log) + * @param data_dir Directory containing sources which will be copied into sandbox + * @param logger Set system logger (optional). + * @param sandbox_binary Name of the sandbox binary (CLI executable) + */ + sandbox_base(std::shared_ptr sandbox_config, + sandbox_limits limits, + std::size_t id, + const std::string &temp_dir, + const std::string &data_dir, + const std::string &sandbox_binary, + std::shared_ptr logger = nullptr); + + /** Initialize the sandbox */ + virtual void sandbox_init() = 0; + + /** Run isolate evaluation with sandboxed program inside. */ + virtual void sandbox_run(const std::string &binary, const std::vector &arguments) = 0; + + /** Cleanup the sandbox after the evaluation */ + virtual void sandbox_cleanup() = 0; + + /** Extract execution results from the sandbox (measurements, errors, ...). */ + virtual sandbox_results extract_results() = 0; }; @@ -89,6 +153,9 @@ class sandbox_exception : public std::exception }; +/** + * Helper function that logs a message and throws an exception with the same message. + */ template void log_and_throw(std::shared_ptr logger, T... args) { std::ostringstream oss; @@ -99,4 +166,37 @@ template void log_and_throw(std::shared_ptr logg } +/** + * Helper function that moves directory from "from" to "to" and throws an exception on failure. + */ +void move_or_throw(std::shared_ptr logger, const std::string &from, const std::string &to); + + +/** + * Helper class that wraps a pipe between a child and a parent process, allowing the child to send lines to the parent. + */ +class sandbox_log_pipe +{ + int read_fd_, write_fd_; // -1 = closed + std::shared_ptr logger_; + +public: + sandbox_log_pipe(std::shared_ptr logger); + ~sandbox_log_pipe(); + + /** + * This is called in the child process to redirect stdout or stderr to the pipe. + * E.g., calling `child_dup_to_fd(1);` will redirect stdout to the pipe. + * This should be called only once! + */ + void child_dup_to_fd(int fd); + + /** + * This is called in the parent process to get a stream to read from the pipe. + * This should be called only once! + */ + boost::iostreams::stream parent_read_stream(); +}; + + #endif // RECODEX_WORKER_FILE_SANDBOX_BASE_H diff --git a/src/tasks/external_task.cpp b/src/tasks/external_task.cpp index 4b7f4fa9..d158e71f 100644 --- a/src/tasks/external_task.cpp +++ b/src/tasks/external_task.cpp @@ -1,4 +1,5 @@ #include "external_task.h" +#include "sandbox/guardian_sandbox.h" #include "sandbox/isolate_sandbox.h" #include "helpers/string_utils.h" #include "helpers/filesystem.h" @@ -37,6 +38,7 @@ void external_task::sandbox_check() bool found = false; #ifndef _WIN32 + if (task_meta_->sandbox->name == "recodex-guardian") { found = true; } if (task_meta_->sandbox->name == "isolate") { found = true; } #endif @@ -46,7 +48,16 @@ void external_task::sandbox_check() void external_task::sandbox_init() { #ifndef _WIN32 - if (task_meta_->sandbox->name == "isolate") { + if (task_meta_->sandbox->name == "recodex-guardian") { + sandbox_limits limits(*limits_); + if (this->get_type() == task_type::INITIATION) { + limits.share_net = true; // initiation (compilation) tasks may use internet to download stuff + + // TODO: a better way would be to make this optional (a job will define, whether it requires net or not) + } + sandbox_ = std::make_shared( + sandbox_config_, limits, worker_config_->get_worker_id(), temp_dir_, evaluation_dir_.string(), logger_); + } else if (task_meta_->sandbox->name == "isolate") { sandbox_limits limits(*limits_); if (this->get_type() == task_type::INITIATION) { limits.share_net = true; // initiation (compilation) tasks may use internet to download stuff @@ -85,8 +96,8 @@ std::shared_ptr external_task::run() make_binary_executable(task_meta_->binary); auto res = std::make_shared(); - res->sandbox_status = - std::unique_ptr(new sandbox_results(sandbox_->run(task_meta_->binary, task_meta_->cmd_args))); + res->sandbox_status = std::unique_ptr( + new sandbox_results(sandbox_->execute_in_sandbox(task_meta_->binary, task_meta_->cmd_args))); // fix status if non-zero exit codes are treated as execution success postprocess_exit_codes(res); @@ -113,7 +124,7 @@ void external_task::postprocess_exit_codes(std::shared_ptr result) 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 + // this happens if zero is not a successfully 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)"; } diff --git a/src/tasks/external_task.h b/src/tasks/external_task.h index 6c9a74a7..1345fe9d 100644 --- a/src/tasks/external_task.h +++ b/src/tasks/external_task.h @@ -11,7 +11,7 @@ /** * Class which handles external tasks, aka tasks which will be executed in sandbox. - * This class have to deal with construction of apropriate sandbox and running program in it. + * This class have to deal with construction of appropriate sandbox and running program in it. */ class external_task : public task_base { @@ -23,11 +23,12 @@ class external_task : public task_base /** * Only way to construct external task is through this constructor. - * Choosing propriate sandbox and constructing it, is also done here. + * Choosing appropriate sandbox and constructing it, is also done here. * @param data Data to create external task class. * @throws task_exception if name of the sandbox in data argument is unknown. */ external_task(const create_params &data); + /** * Destructor, empty right now. */ @@ -36,7 +37,7 @@ class external_task : public task_base /** * Runs given program and parameters in constructed sandbox. * @return @ref task_results with @a sandbox_status item properly set - * @throws sandbox_exception if fatal error occured in sandbox + * @throws sandbox_exception if fatal error occurred in sandbox */ std::shared_ptr run() override; @@ -52,10 +53,12 @@ class external_task : public task_base * stated sandbox). */ void sandbox_check(); + /** - * Construct apropriate sandbox according his name give during construction. + * Construct appropriate sandbox according his name give during construction. */ void sandbox_init(); + /** * Destruction of internal sandbox. */ @@ -98,22 +101,31 @@ class external_task : public task_base /** Worker default configuration */ std::shared_ptr worker_config_; + /** Constructed sandbox itself */ std::shared_ptr sandbox_; + /** General sandbox config */ std::shared_ptr sandbox_config_; + /** Limits for sandbox in which program will be started */ std::shared_ptr limits_; + /** Job system logger */ std::shared_ptr logger_; + /** Directory for temporary files */ std::string temp_dir_; + /** Directory outside sandbox where task will be executed */ fs::path evaluation_dir_; - /** Directory binded to the sandbox as default working dir */ + + /** Directory bound to the sandbox as default working dir */ fs::path sandbox_working_dir_; + /** After execution delete stdout file produced by sandbox */ bool remove_stdout_ = false; + /** After execution delete stderr file produced by sandbox */ bool remove_stderr_ = false; }; diff --git a/src/tasks/internal/archivate_task.cpp b/src/tasks/internal/archive_task.cpp similarity index 73% rename from src/tasks/internal/archivate_task.cpp rename to src/tasks/internal/archive_task.cpp index 46a9de5e..36a2b657 100644 --- a/src/tasks/internal/archivate_task.cpp +++ b/src/tasks/internal/archive_task.cpp @@ -1,8 +1,8 @@ -#include "archivate_task.h" +#include "archive_task.h" #include "archives/archivator.h" -archivate_task::archivate_task(std::size_t id, std::shared_ptr task_meta) : task_base(id, task_meta) +archive_task::archive_task(std::size_t id, std::shared_ptr task_meta) : task_base(id, task_meta) { if (task_meta_->cmd_args.size() != 2) { throw task_exception( @@ -11,7 +11,7 @@ archivate_task::archivate_task(std::size_t id, std::shared_ptr ta } -std::shared_ptr archivate_task::run() +std::shared_ptr archive_task::run() { std::shared_ptr result(new task_results()); diff --git a/src/tasks/internal/archivate_task.h b/src/tasks/internal/archive_task.h similarity index 52% rename from src/tasks/internal/archivate_task.h rename to src/tasks/internal/archive_task.h index 7c422979..ff8d845f 100644 --- a/src/tasks/internal/archivate_task.h +++ b/src/tasks/internal/archive_task.h @@ -1,28 +1,28 @@ -#ifndef RECODEX_WORKER_INTERNAL_ARCHIVATE_TASK_H -#define RECODEX_WORKER_INTERNAL_ARCHIVATE_TASK_H +#ifndef RECODEX_WORKER_INTERNAL_ARCHIVE_TASK_H +#define RECODEX_WORKER_INTERNAL_ARCHIVE_TASK_H #include "tasks/task_base.h" /** - * Create archive using @ref archivator. + * Create archive using @ref archivist. */ -class archivate_task : public task_base +class archive_task : public task_base { public: /** * Constructor with initialization. - * @param id Unique identificator of load order of tasks. + * @param id Unique identifier of load order of tasks. * @param task_meta Variable containing further info about task. It's required that * @a cmd_args entry has just 2 arguments - directory to be archived and name of the archive. - * For more info about archivation see @ref archivator class. + * For more info about activation see @ref archivist class. * @throws task_exception on invalid number of arguments. */ - archivate_task(std::size_t id, std::shared_ptr task_meta); + archive_task(std::size_t id, std::shared_ptr task_meta); /** * Destructor. */ - ~archivate_task() override = default; + ~archive_task() override = default; /** * Run the action. * @return Evaluation results to be pushed back to frontend. @@ -30,4 +30,4 @@ class archivate_task : public task_base std::shared_ptr run() override; }; -#endif // RECODEX_WORKER_INTERNAL_ARCHIVATE_TASK_H +#endif // RECODEX_WORKER_INTERNAL_ARCHIVE_TASK_H diff --git a/src/tasks/task_base.h b/src/tasks/task_base.h index f428972a..c36d5298 100644 --- a/src/tasks/task_base.h +++ b/src/tasks/task_base.h @@ -16,7 +16,7 @@ * about task which always have to be defined. * @note Task can be created only with one constructor which receive pointer to * @ref task_metadata as parameter. This structure is mutable, so it can be changed - * during task execution. This case should never happen in ReCodEx worker, but posibility + * during task execution. This case should never happen in ReCodEx worker, but possibility * is still here. Please keep this in mind if you're editing this code otherwise it can * end up very bad for you! */ @@ -29,7 +29,7 @@ class task_base task_base() = delete; /** * Only possible way of construction which just store all given parameters into private variables. - * @param id Unique identificator of load order of tasks. + * @param id Unique identifier of load order of tasks. * @param task_meta Variable containing further info about task. */ task_base(std::size_t id, std::shared_ptr task_meta); @@ -84,7 +84,7 @@ class task_base std::size_t get_priority(); /** * Get failing policy. If @a true than failure of this task will cause - * mmediate exit of job evaluation. + * immediate exit of job evaluation. * @return If task's failure is fatal for whole job. */ bool get_fatal_failure(); @@ -203,7 +203,7 @@ class task_compare /** * Compare @ref task_base objects by their priority and identifier. This is something like * lesser than operator on @ref task_base objects. - * Its supposed that bigger number of priority is greater priority, so this tasks will be prefered. + * Its supposed that bigger number of priority is greater priority, so this tasks will be preferred. * @param a First task to compare. * @param b Second task to compare. * @return @a true if parameter a is lesser than b diff --git a/src/tasks/task_factory.cpp b/src/tasks/task_factory.cpp index 89427445..d599dc61 100644 --- a/src/tasks/task_factory.cpp +++ b/src/tasks/task_factory.cpp @@ -21,8 +21,8 @@ std::shared_ptr task_factory::create_internal_task(std::size_t id, st task = std::make_shared(id, task_meta); } else if (task_meta->binary == "rm") { task = std::make_shared(id, task_meta); - } else if (task_meta->binary == "archivate") { - task = std::make_shared(id, task_meta); + } else if (task_meta->binary == "archive") { + task = std::make_shared(id, task_meta); } else if (task_meta->binary == "extract") { task = std::make_shared(id, task_meta); } else if (task_meta->binary == "fetch") { diff --git a/src/tasks/task_factory.h b/src/tasks/task_factory.h index 25dc8959..a3d10c39 100644 --- a/src/tasks/task_factory.h +++ b/src/tasks/task_factory.h @@ -5,7 +5,7 @@ #include "task_factory_interface.h" #include "external_task.h" #include "root_task.h" -#include "internal/archivate_task.h" +#include "internal/archive_task.h" #include "internal/cp_task.h" #include "internal/dump_dir_task.h" #include "internal/truncate_task.h" diff --git a/src/worker_core.cpp b/src/worker_core.cpp index f3a14bd2..5d77ce8e 100644 --- a/src/worker_core.cpp +++ b/src/worker_core.cpp @@ -52,7 +52,7 @@ void worker_core::run() logger_->critical("Broker connection thread cannot be started: {}", e.what()); return; } - logger_->info("Broker connection thread created succesfully."); + logger_->info("Broker connection thread created successfully."); logger_->info("Job receiver will now start receiving."); job_receiver_->start_receiving(); diff --git a/src/worker_core.h b/src/worker_core.h index 8f8a24f2..b1c75f74 100644 --- a/src/worker_core.h +++ b/src/worker_core.h @@ -45,7 +45,7 @@ class worker_core worker_core(std::vector args); /** - * All structures which need to be explicitly destructed or unitialized should do it now. + * All structures which need to be explicitly destructed or uninitialized should do it now. */ ~worker_core(); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 6f345159..8aaf9bc7 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -117,12 +117,14 @@ add_test_suite(tasks ${TASKS_DIR}/internal/mkdir_task.cpp ${TASKS_DIR}/internal/rename_task.cpp ${TASKS_DIR}/internal/rm_task.cpp - ${TASKS_DIR}/internal/archivate_task.cpp + ${TASKS_DIR}/internal/archive_task.cpp ${TASKS_DIR}/internal/extract_task.cpp ${TASKS_DIR}/internal/fetch_task.cpp ${TASKS_DIR}/internal/truncate_task.cpp ${TASKS_DIR}/internal/exists_task.cpp ${SRC_DIR}/archives/archivator.cpp + ${SANDBOX_DIR}/sandbox_base.cpp + ${SANDBOX_DIR}/guardian_sandbox.cpp ${SANDBOX_DIR}/isolate_sandbox.cpp ${HELPERS_DIR}/logger.cpp ${HELPERS_DIR}/config.cpp @@ -200,6 +202,7 @@ endif() add_test_suite(tool_isolate_sandbox tests_main.cpp isolate_sandbox.cpp + ${SANDBOX_DIR}/sandbox_base.cpp ${SANDBOX_DIR}/isolate_sandbox.cpp ${HELPERS_DIR}/logger.cpp ${HELPERS_DIR}/filesystem.cpp diff --git a/tests/job.cpp b/tests/job.cpp index 0c57d6ae..8c28a8c9 100644 --- a/tests/job.cpp +++ b/tests/job.cpp @@ -272,13 +272,6 @@ TEST(job_test, empty_tasks_details) task->priority = 1; EXPECT_THROW(job(job_meta, worker_conf, dir_root, dir, temp_directory_path(), factory, nullptr), job_exception); - // empty sandbox name - EXPECT_CALL((*factory), create_internal_task(0, _)).WillOnce(Return(empty_task)); - task->binary = "hello"; - auto sandbox = std::make_shared(); - task->sandbox = sandbox; - EXPECT_THROW(job(job_meta, worker_conf, dir_root, dir, temp_directory_path(), factory, nullptr), job_exception); - // cleanup after yourself remove_all(dir_root); } diff --git a/tests/mocks.h b/tests/mocks.h index 659e5b53..2dcef7cb 100644 --- a/tests/mocks.h +++ b/tests/mocks.h @@ -23,7 +23,7 @@ using namespace testing; /** - * A mock configuration object. Inverval of pinging is 1 second. + * A mock configuration object. Interval of pinging is 1 second. */ class mock_worker_config : public worker_config { diff --git a/tests/tasks.cpp b/tests/tasks.cpp index 51af5ef8..fc611edc 100644 --- a/tests/tasks.cpp +++ b/tests/tasks.cpp @@ -6,7 +6,7 @@ #include #include -#include "tasks/internal/archivate_task.h" +#include "tasks/internal/archive_task.h" #include "tasks/internal/cp_task.h" #include "tasks/internal/extract_task.h" #include "tasks/internal/mkdir_task.h" @@ -67,12 +67,12 @@ std::shared_ptr get_zero_args() return res; } -TEST(Tasks, InternalArchivateTask) +TEST(Tasks, InternalArchiveTask) { - EXPECT_THROW(archivate_task(1, get_three_args()), task_exception); - EXPECT_THROW(archivate_task(1, get_one_args()), task_exception); - EXPECT_THROW(archivate_task(1, get_zero_args()), task_exception); - EXPECT_NO_THROW(archivate_task(1, get_two_args())); + EXPECT_THROW(archive_task(1, get_three_args()), task_exception); + EXPECT_THROW(archive_task(1, get_one_args()), task_exception); + EXPECT_THROW(archive_task(1, get_zero_args()), task_exception); + EXPECT_NO_THROW(archive_task(1, get_two_args())); } TEST(Tasks, InternalCpTask) @@ -206,10 +206,10 @@ TEST(Tasks, TaskFactory) auto meta = get_task_meta(); std::shared_ptr task; - // archivate task - meta->binary = "archivate"; + // archive task + meta->binary = "archive"; task = factory.create_internal_task(0, meta); - EXPECT_NE(std::dynamic_pointer_cast(task), nullptr); + EXPECT_NE(std::dynamic_pointer_cast(task), nullptr); // cp task meta->binary = "cp"; @@ -243,7 +243,7 @@ TEST(Tasks, TaskFactory) // root task // - with explicit nullptr argument - meta->binary = "archivate"; + meta->binary = "archive"; task = factory.create_internal_task(0, nullptr); EXPECT_NE(std::dynamic_pointer_cast(task), nullptr); // - without explicit meta argument @@ -251,7 +251,7 @@ TEST(Tasks, TaskFactory) EXPECT_NE(std::dynamic_pointer_cast(task), nullptr); // unknown internal task - meta->binary = "unknown_internal_bianry"; + meta->binary = "unknown_internal_binary"; task = factory.create_internal_task(0, meta); EXPECT_EQ(task, nullptr);