Updated recommended runc version to v1.1.12

CHANGELOG.md

@@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Changed the implementation of the lock for the image repository metadata file to a mechanism based on flock(2).
   The new implementation can support both shared locks (a.k.a. read locks) and exclusive locks (a.k.a. write locks),
   and improves the startup time when launching large numbers of containers at scale.
+- Updated recommended runc version to 1.1.12
 - Updated CI integration tests on Rocky 8 to use Python 3.9, solving a problem of missing wheel packages for the previous Python version
 - Updated CI distributed tests to use Docker Compose V2 and Compose file format version 3
 - Updated automatic documentation build to use Sphinx 7.2.6 and Sphinx RTD Theme 2.0.0

CI/installation/install_dep_runc.bash

@@ -4,7 +4,7 @@ pwd_bak=$PWD
 
 # Install runc
 cd /tmp && \
-    wget -O runc.amd64 https://github.com/opencontainers/runc/releases/download/v1.1.9/runc.amd64 && \
+    wget -O runc.amd64 https://github.com/opencontainers/runc/releases/download/v1.1.12/runc.amd64 && \
     chmod 755 runc.amd64 && \
     sudo mv runc.amd64 /usr/local/bin/ && \
     sudo chown root:root /usr/local/bin/runc.amd64 && \

CI/installation/install_packages_fedora:38.sh

@@ -12,7 +12,7 @@ sudo rm -rf /var/cache/dnf
 
 # The following dependencies are not provided via the system's package manager
 # and should be installed manually:
-# - Runc version 1.1.9
+# - Runc version 1.1.12
 # - Umoci
 
 # DOCS: END

CI/installation/install_packages_ubuntu:22.04.sh

@@ -18,7 +18,7 @@ sudo rm -rf /var/lib/apt/lists/*
 
 # The following dependencies are not provided via the system's package manager
 # and should be installed manually:
-# - Runc version 1.1.9
+# - Runc version 1.1.12
 
 # DOCS: END
 

CI/src/integration_tests/test_hook_stdout_stderr.py

@@ -6,6 +6,7 @@
 # SPDX-License-Identifier: BSD-3-Clause
 
 import unittest
+import pytest
 import subprocess
 import os
 import re
@@ -49,6 +50,7 @@ def _enable_hook(cls):
     def _disable_hook(cls):
         subprocess.call(["sudo", "rm", cls._OCIHOOK_CONFIG_FILE])
 
+    @pytest.mark.xfail(reason="Hooks stdout/err are not captured by Pytest after changes for runc 1.1.12")
     def test_stdout(self):
         # DEBUG log level
         expected = [
@@ -74,6 +76,7 @@ def test_stdout(self):
         stdout = self._get_sarus_stdout()
         self._check_output_matches(stdout, expected)
 
+    @pytest.mark.xfail(reason="Hooks stdout/err are not captured by Pytest after changes for runc 1.1.12")
     def test_stderr(self):
         expected = [
             r"^hook's stderr$",

CI/src/integration_tests/test_mpi_hook.py

@@ -127,6 +127,7 @@ def test_mpich_compatible_symlink(self):
         number_of_expected_mounts = len(self._HOST_MPI_LIBS) + len(self._HOST_MPI_DEPENDENCY_LIBS)
         assert hashes.count(self._HOST_LIB_HASH) == number_of_expected_mounts
 
+    @pytest.mark.xfail(reason="Hooks stdout/err are not captured by Pytest after changes for runc 1.1.12")
     def test_mpich_minor_incompatible(self):
         self._mpi_command_line_option = True
         self._container_image = "quay.io/ethcscs/sarus-integration-tests:mpich_minor_incompatible"

CI/utility_functions.bash

@@ -120,7 +120,7 @@ build_sarus_archive() {
 
     # runc is statically linked with libseccomp, which is licensed under GNU LGPL-2.1.
     # To comply with LGPL-2.1 (§6(a)), include the libseccomp source code in the licenses folder.
-    wget -O ${prefix_dir}/licenses/libseccomp-2.5.4.tar.gz https://github.com/opencontainers/runc/releases/download/v1.1.9/libseccomp-2.5.4.tar.gz
+    wget -O ${prefix_dir}/licenses/libseccomp-2.5.4.tar.gz https://github.com/opencontainers/runc/releases/download/v1.1.12/libseccomp-2.5.4.tar.gz
 
     mkdir -pv ${prefix_dir}/var/OCIBundleDir
 

doc/install/requirements.rst

@@ -146,7 +146,7 @@ Sarus internally relies on an OCI-compliant runtime to spawn a container.
 
 Here we will provide some indications to install `runc
 <https://github.com/opencontainers/runc>`_, the reference implementation from
-the Open Container Initiative. The recommended version is **v1.1.9**.
+the Open Container Initiative. The recommended version is **v1.1.12**.
 
 .. note::
     runc versions from 1.1.0 to 1.1.2 have a bug which causes an error when

src/hooks/common/Utility.cpp

@@ -54,11 +54,15 @@ void applyLoggingConfigIfAvailable(const rapidjson::Document& json) {
     }
 
     if(json["annotations"].HasMember("com.hooks.logging.stdoutfd")) {
-        replaceFd(1, std::stoi(json["annotations"]["com.hooks.logging.stdoutfd"].GetString()));
+        int fd = open(json["annotations"]["com.hooks.logging.stdoutfd"].GetString(), O_WRONLY);
+        replaceFd(1, fd);
+        close(fd);
     }
 
     if(json["annotations"].HasMember("com.hooks.logging.stderrfd")) {
-        replaceFd(2, std::stoi(json["annotations"]["com.hooks.logging.stderrfd"].GetString()));
+        int fd = open(json["annotations"]["com.hooks.logging.stderrfd"].GetString(), O_WRONLY);
+        replaceFd(2, fd);
+        close(fd);
     }
 }
 

src/runtime/ConfigsMerger.cpp

@@ -128,6 +128,27 @@ std::unordered_map<std::string, std::string> ConfigsMerger::getBundleAnnotations
     auto level = static_cast<IntType>(common::Logger::getInstance().getLevel());
     annotations["com.hooks.logging.level"] = std::to_string(level);
 
+    // Resolve paths of the engine's stdout/stderr (if available) to be used by hooks
+    // as their stdout/err file descriptors
+    auto* realPtr = realpath("/dev/stdout", NULL);
+    if (realPtr != nullptr) {
+        annotations["com.hooks.logging.stdoutfd"] = realPtr;
+        free(realPtr);
+    }
+    else {
+        auto message = boost::format("Failed to find real path for /dev/stdout: %s") % strerror(errno);
+        utility::logMessage(message, common::LogLevel::DEBUG);
+    }
+    realPtr = realpath("/dev/stderr", NULL);
+    if (realPtr != nullptr) {
+        annotations["com.hooks.logging.stderrfd"] = realPtr;
+        free(realPtr);
+    }
+    else {
+        auto message = boost::format("Failed to find real path for /dev/stderr: %s") % strerror(errno);
+        utility::logMessage(message, common::LogLevel::DEBUG);
+    }
+
     // Merge custom annotations (from the CLI or other components like the FileDescriptorHandler),
     // which should have priority over any others.
     // With C++17, instead of operator[] we could use insert_or_assign()

src/runtime/FileDescriptorHandler.cpp

@@ -42,14 +42,6 @@ void FileDescriptorHandler::preservePMIFdIfAny() {
     }
 }
 
-void FileDescriptorHandler::passStdoutAndStderrToHooks() {
-    // Note: force duplication of stdout and stderr file descriptors because runc
-    // replaces them prior executing the hooks, i.e. the Sarus's stdout and stderr
-    // wouldn't be accessible from the hooks if not duplicated.
-    fileDescriptorsToPreserve[1] = { "stdout", {}, std::string{"com.hooks.logging.stdoutfd"}, true };
-    fileDescriptorsToPreserve[2] = { "stderr", {}, std::string{"com.hooks.logging.stderrfd"}, true };
-}
-
 void FileDescriptorHandler::applyChangesToFdsAndEnvVariablesAndBundleAnnotations() {
     utility::logMessage("Applying changes to file descriptors, container's environment variables and bundle's annotations",
                         common::LogLevel::INFO);

src/runtime/FileDescriptorHandler.hpp

@@ -22,7 +22,6 @@ class FileDescriptorHandler {
 public:
     FileDescriptorHandler(std::shared_ptr<common::Config>);
     void preservePMIFdIfAny();
-    void passStdoutAndStderrToHooks();
     void applyChangesToFdsAndEnvVariablesAndBundleAnnotations();
     int getExtraFileDescriptors() const {return extraFileDescriptors;};
 

src/runtime/Runtime.cpp

@@ -62,7 +62,6 @@ void Runtime::setupOCIBundle() {
     performDeviceMounts();
     remountRootfsWithNoSuid();
     fdHandler.preservePMIFdIfAny();
-    fdHandler.passStdoutAndStderrToHooks();
     fdHandler.applyChangesToFdsAndEnvVariablesAndBundleAnnotations();
     bundleConfig.generateConfigFile();
 

src/runtime/test/expected_config.json

@@ -160,7 +160,6 @@
 	},
 	"annotations": {
 		"com.test.dummy_key": "dummy_value",
-		"com.test.image.key": "image_value",
-		"com.hooks.logging.level": "2"
+		"com.test.image.key": "image_value"
 	}
 }

src/runtime/test/test_ConfigsMerger.cpp

@@ -294,6 +294,8 @@ TEST(ConfigsMergerTestGroup, pmix_environment) {
 
 TEST(ConfigsMergerTestGroup, bundle_annotations) {
     auto metadata = common::ImageMetadata{};
+    auto* realStdout = realpath("/dev/stdout", NULL);
+    auto* realStderr = realpath("/dev/stderr", NULL);
 
     // No hooks enabled
     {
@@ -303,6 +305,8 @@ TEST(ConfigsMergerTestGroup, bundle_annotations) {
             {"com.test.dummy_key", "dummy_value"},
             {"com.hooks.logging.level", "2"}
         };
+        if (realStdout != nullptr) expectedAnnotations["com.hooks.logging.stdoutfd"] = realStdout;
+        if (realStderr != nullptr) expectedAnnotations["com.hooks.logging.stderrfd"] = realStderr;
         CHECK((ConfigsMerger{config, metadata}.getBundleAnnotations() == expectedAnnotations));
     }
     // glibc hook enabled
@@ -315,6 +319,8 @@ TEST(ConfigsMergerTestGroup, bundle_annotations) {
             {"com.hooks.logging.level", "2"},
             {"com.hooks.glibc.enabled", "true"}
         };
+        if (realStdout != nullptr) expectedAnnotations["com.hooks.logging.stdoutfd"] = realStdout;
+        if (realStderr != nullptr) expectedAnnotations["com.hooks.logging.stderrfd"] = realStderr;
         CHECK((ConfigsMerger{config, metadata}.getBundleAnnotations() == expectedAnnotations));
     }
     // MPI hook enabled
@@ -328,6 +334,8 @@ TEST(ConfigsMergerTestGroup, bundle_annotations) {
             {"com.hooks.glibc.enabled", "true"},
             {"com.hooks.mpi.enabled", "true"}
         };
+        if (realStdout != nullptr) expectedAnnotations["com.hooks.logging.stdoutfd"] = realStdout;
+        if (realStderr != nullptr) expectedAnnotations["com.hooks.logging.stderrfd"] = realStderr;
         CHECK((ConfigsMerger{config, metadata}.getBundleAnnotations() == expectedAnnotations));
 
         // Explicit MPI type
@@ -340,6 +348,8 @@ TEST(ConfigsMergerTestGroup, bundle_annotations) {
             {"com.hooks.mpi.enabled", "true"},
             {"com.hooks.mpi.type", "mpi0"}
         };
+        if (realStdout != nullptr) expectedAnnotations["com.hooks.logging.stdoutfd"] = realStdout;
+        if (realStderr != nullptr) expectedAnnotations["com.hooks.logging.stderrfd"] = realStderr;
         CHECK((ConfigsMerger{config, metadata}.getBundleAnnotations() == expectedAnnotations));
     }
     // SSH hook enabled
@@ -353,6 +363,8 @@ TEST(ConfigsMergerTestGroup, bundle_annotations) {
             {"com.hooks.slurm-global-sync.enabled", "true"},
             {"com.hooks.ssh.enabled", "true"}
         };
+        if (realStdout != nullptr) expectedAnnotations["com.hooks.logging.stdoutfd"] = realStdout;
+        if (realStderr != nullptr) expectedAnnotations["com.hooks.logging.stderrfd"] = realStderr;
         CHECK((ConfigsMerger{config, metadata}.getBundleAnnotations() == expectedAnnotations));
     }
     // Image annotations
@@ -365,6 +377,8 @@ TEST(ConfigsMergerTestGroup, bundle_annotations) {
             {"com.hooks.logging.level", "2"},
             {"com.test.image.key", "image_value"}
         };
+        if (realStdout != nullptr) expectedAnnotations["com.hooks.logging.stdoutfd"] = realStdout;
+        if (realStderr != nullptr) expectedAnnotations["com.hooks.logging.stderrfd"] = realStderr;
         CHECK((ConfigsMerger{config, metadata}.getBundleAnnotations() == expectedAnnotations));
         metadata.labels.erase("com.test.image.key");
     }
@@ -377,11 +391,16 @@ TEST(ConfigsMergerTestGroup, bundle_annotations) {
         config->commandRun.ociAnnotations["com.hooks.logging.level"] = "0";
         auto expectedAnnotations = std::unordered_map<std::string, std::string>{
             {"com.test.dummy_key", "dummy_value"},
-            {"com.hooks.logging.level", "0"}
+            {"com.hooks.logging.level", "0"},
         };
+        if (realStdout != nullptr) expectedAnnotations["com.hooks.logging.stdoutfd"] = realStdout;
+        if (realStderr != nullptr) expectedAnnotations["com.hooks.logging.stderrfd"] = realStderr;
         auto annot = ConfigsMerger{config, metadata}.getBundleAnnotations();
         CHECK((ConfigsMerger{config, metadata}.getBundleAnnotations() == expectedAnnotations));
     }
+
+    free(realStdout);
+    free(realStderr);
 }
 
 TEST(ConfigsMergerTestGroup, command_to_execute) {

src/runtime/test/test_FileDescriptorHandler.cpp

@@ -99,32 +99,6 @@ IGNORE_TEST(RuntimeTestGroup, applyChangesToFdsAndEnvVariablesAndBundleAnnotatio
     CHECK_EQUAL(std::to_string(3), config->commandRun.hostEnvironment["PMI_FD"]);
     CHECK_EQUAL(testFiles[2].string(), boost::filesystem::canonical("/proc/self/fd/3").string());
     closeFiles(testFDs);
-
-    // test Sarus stdout stderr fds
-    testFDs = openFiles(testFiles);
-    handler = runtime::FileDescriptorHandler{config};
-    handler.passStdoutAndStderrToHooks();
-    handler.applyChangesToFdsAndEnvVariablesAndBundleAnnotations();
-    CHECK_EQUAL(2, handler.getExtraFileDescriptors());
-    CHECK_EQUAL(std::to_string(3), config->commandRun.ociAnnotations["com.hooks.logging.stdoutfd"]);
-    CHECK_EQUAL(std::to_string(4), config->commandRun.ociAnnotations["com.hooks.logging.stderrfd"]);
-    CHECK(boost::filesystem::canonical("/proc/self/fd/1") == boost::filesystem::canonical("/proc/self/fd/3"));
-    CHECK(boost::filesystem::canonical("/proc/self/fd/2") == boost::filesystem::canonical("/proc/self/fd/4"));
-
-    // test Sarus stdout stderr + PMI
-    testFDs = openFiles(testFiles);
-    config->commandRun.hostEnvironment["PMI_FD"] = std::to_string(testFDs[2]);
-    handler = runtime::FileDescriptorHandler{config};
-    handler.passStdoutAndStderrToHooks();
-    handler.preservePMIFdIfAny();
-    handler.applyChangesToFdsAndEnvVariablesAndBundleAnnotations();
-    CHECK_EQUAL(3, handler.getExtraFileDescriptors());
-    CHECK_EQUAL(std::to_string(3), config->commandRun.ociAnnotations["com.hooks.logging.stdoutfd"]);
-    CHECK_EQUAL(std::to_string(4), config->commandRun.ociAnnotations["com.hooks.logging.stderrfd"]);
-    CHECK_EQUAL(std::to_string(5), config->commandRun.hostEnvironment["PMI_FD"]);
-    CHECK(boost::filesystem::canonical("/proc/self/fd/1") == boost::filesystem::canonical("/proc/self/fd/3"));
-    CHECK(boost::filesystem::canonical("/proc/self/fd/2") == boost::filesystem::canonical("/proc/self/fd/4"));
-    CHECK_EQUAL(testFiles[2].string(), boost::filesystem::canonical("/proc/self/fd/5").string());
 }
 
 SARUS_UNITTEST_MAIN_FUNCTION();

src/runtime/test/test_OCIBundleConfig.cpp

@@ -190,6 +190,26 @@ static void enterTestDeviceFilesInJson(rapidjson::Document* json) {
     (*json)["linux"]["resources"]["devices"].PushBack(testDevice1Rule, *allocator);
 }
 
+static void enterHooksLoggingAnnotationsInJson(rapidjson::Document* json) {
+    auto* allocator = &json->GetAllocator();
+
+    auto* realStderr = realpath("/dev/stderr", NULL);
+    if (realStderr != nullptr) {
+        auto realStderrValue = rj::Value{realStderr, *allocator};
+        (*json)["annotations"].AddMember("com.hooks.logging.stderrfd", realStderrValue, *allocator);
+        free(realStderr);
+    }
+
+    auto* realStdout = realpath("/dev/stdout", NULL);
+    if (realStdout != nullptr) {
+        auto realStdoutValue = rj::Value{realStdout, *allocator};
+        (*json)["annotations"].AddMember("com.hooks.logging.stdoutfd", realStdoutValue, *allocator);
+        free(realStdout);
+    }
+
+    (*json)["annotations"].AddMember("com.hooks.logging.level", "2", *allocator);
+}
+
 TEST(OCIBundleConfigTestGroup, OCIBundleConfig) {
     // create test config
     auto configRAII = test_utility::config::makeConfig();
@@ -207,7 +227,10 @@ TEST(OCIBundleConfigTestGroup, OCIBundleConfig) {
     auto actualJson = common::readJSON(actualConfigFile);
     sortJsonEnvironmentArray(actualJson);
 
-    compareJsonObjects(actualJson, getExpectedJson());
+    auto expectedJson = getExpectedJson();
+    enterHooksLoggingAnnotationsInJson(&expectedJson);
+
+    compareJsonObjects(actualJson, expectedJson);
 }
 
 #ifdef ASROOT
@@ -234,6 +257,7 @@ IGNORE_TEST(OCIBundleConfigTestGroup, allowed_devices) {
 
     auto expectedJson = getExpectedJson();
     enterTestDeviceFilesInJson(&expectedJson);
+    enterHooksLoggingAnnotationsInJson(&expectedJson);
 
     compareJsonObjects(actualJson, expectedJson);
 }