From 90c154beb9b6b929be7a793089343910e3c08a8f Mon Sep 17 00:00:00 2001 From: Gurdal Oruklu Date: Mon, 30 Mar 2026 12:25:50 -0700 Subject: [PATCH 1/8] Optimize iptables setup and cache external interfaces - Apply IPv4 and IPv6 iptables rules concurrently to reduce startup latency. - Cache available external interfaces to avoid repeated I/O and parsing. --- .../Networking/include/NetworkingPlugin.h | 5 +++ .../Networking/source/NetworkingPlugin.cpp | 43 ++++++++++++++++--- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/rdkPlugins/Networking/include/NetworkingPlugin.h b/rdkPlugins/Networking/include/NetworkingPlugin.h index 973c10f76..eaa3affe8 100644 --- a/rdkPlugins/Networking/include/NetworkingPlugin.h +++ b/rdkPlugins/Networking/include/NetworkingPlugin.h @@ -74,6 +74,11 @@ class NetworkingPlugin : public RdkPluginBase std::shared_ptr mHelper; std::shared_ptr mNetfilter; + + // Cached result of GetAvailableExternalInterfaces() - the settings file and + // available interface set do not change during a container lifetime, so we + // compute this once and reuse it across all hook calls. + mutable std::vector mExtIfaces; }; #endif // !defined(NETWORKINGPLUGIN_H) diff --git a/rdkPlugins/Networking/source/NetworkingPlugin.cpp b/rdkPlugins/Networking/source/NetworkingPlugin.cpp index 650e3aeb8..7cc92215a 100644 --- a/rdkPlugins/Networking/source/NetworkingPlugin.cpp +++ b/rdkPlugins/Networking/source/NetworkingPlugin.cpp @@ -31,6 +31,7 @@ #include #include #include +#include REGISTER_RDK_PLUGIN(NetworkingPlugin); @@ -165,9 +166,11 @@ bool NetworkingPlugin::createRuntime() return false; } - // check if another container has already initialised the bridge device for us - std::shared_ptr netlink = std::make_shared(); - bool bridgeExists = netlink->ifaceExists(std::string(BRIDGE_NAME)); + // Check whether another container already set up the bridge device. + // A direct sysfs access() is far cheaper than opening a Netlink socket + // (which allocates a socket fd + opens /sys/class/net) just to query + // a single interface name. + bool bridgeExists = (access("/sys/class/net/" BRIDGE_NAME, F_OK) == 0); if (!bridgeExists) { @@ -305,8 +308,15 @@ bool NetworkingPlugin::createRuntime() } } - // apply iptables changes - if (!mNetfilter->applyRules(AF_INET) || !mNetfilter->applyRules(AF_INET6)) + // Apply IPv4 and IPv6 iptables rules concurrently. The two rule sets + // are completely independent - running them in parallel overlaps the + // fork/exec cost of iptables-save and iptables-restore for both + // address families, halving the iptables portion of startup latency. + auto ipv4Future = std::async(std::launch::async, + [this]() { return mNetfilter->applyRules(AF_INET); }); + const bool ipv6Success = mNetfilter->applyRules(AF_INET6); + const bool ipv4Success = ipv4Future.get(); + if (!ipv4Success || !ipv6Success) { AI_LOG_ERROR_EXIT("failed to apply iptables rules"); return false; @@ -511,8 +521,13 @@ bool NetworkingPlugin::postHalt() } } - // apply iptables changes - if (!mNetfilter->applyRules(AF_INET) || !mNetfilter->applyRules(AF_INET6)) + // Apply IPv4 and IPv6 iptables rules concurrently (same rationale as + // createRuntime - the two address families are fully independent). + auto ipv4HaltFuture = std::async(std::launch::async, + [this]() { return mNetfilter->applyRules(AF_INET); }); + const bool ipv6HaltSuccess = mNetfilter->applyRules(AF_INET6); + const bool ipv4HaltSuccess = ipv4HaltFuture.get(); + if (!ipv4HaltSuccess || !ipv6HaltSuccess) { AI_LOG_ERROR_EXIT("failed to apply iptables rules"); return false; @@ -558,6 +573,15 @@ std::vector NetworkingPlugin::getDependencies() const */ std::vector NetworkingPlugin::GetAvailableExternalInterfaces() const { + // Return the cached result if already computed. Reading and YAJL- + // parsing /etc/dobby.json plus scanning /sys/class/net on every call + // adds measurable latency; the result is stable for the lifetime of + // the container. + if (!mExtIfaces.empty()) + { + return mExtIfaces; + } + std::vector externalIfaces = GetExternalInterfacesFromSettings(); if (externalIfaces.size() == 0) @@ -606,6 +630,11 @@ std::vector NetworkingPlugin::GetAvailableExternalInterfaces() cons { AI_LOG_ERROR("None of the external interfaces defined in the settings file are available"); } + else + { + // Cache the result so subsequent hook calls pay no I/O cost. + mExtIfaces = externalIfaces; + } return externalIfaces; } From f5dca9fc91631ccd496496929e68c87e53d14ce1 Mon Sep 17 00:00:00 2001 From: Gurdal Oruklu Date: Tue, 31 Mar 2026 13:32:23 -0700 Subject: [PATCH 2/8] made container swap limit configurable --- README.md | 45 ++++ bundle/lib/include/DobbySpecConfig.h | 1 + bundle/lib/source/DobbySpecConfig.cpp | 64 ++++- .../OciConfigJson1.0.2-dobby.template | 2 +- .../OciConfigJsonVM1.0.2-dobby.template | 2 +- .../tests/DobbySpecConfigTest/CMakeLists.txt | 72 ++++++ .../DobbySpecConfigTest.cpp | 221 ++++++++++++++++++ tests/L2_testing/dobby_specs/swap_limit.json | 29 +++ .../test_runner/swap_limit_tests.py | 110 +++++++++ 9 files changed, 543 insertions(+), 3 deletions(-) create mode 100644 tests/L1_testing/tests/DobbySpecConfigTest/CMakeLists.txt create mode 100644 tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigTest.cpp create mode 100644 tests/L2_testing/dobby_specs/swap_limit.json create mode 100644 tests/L2_testing/test_runner/swap_limit_tests.py diff --git a/README.md b/README.md index 1d75db49d..ed1fadf6d 100644 --- a/README.md +++ b/README.md @@ -125,6 +125,51 @@ Usage: DobbyBundleGenerator -o, --outputDirectory=PATH Where to save the generated OCI bundle ``` +## Dobby Spec Format +When using `DobbyDaemon` or `DobbyBundleGenerator`, containers are described using a Dobby-specific JSON spec file. Example specs can be found in `tests/L2_testing/dobby_specs/`. + +The table below lists the supported top-level fields. Fields marked **mandatory** must always be present. + +| Field | Type | Mandatory | Description | +|-------|------|-----------|-------------| +| `version` | string | Yes | Spec version. Currently `"1.0"` or `"1.1"`. | +| `args` | array | Yes | Command and arguments to run inside the container. | +| `user` | object | Yes | `uid` and `gid` the container process runs as. | +| `memLimit` | integer | Yes | Memory limit in bytes (`memory.limit_in_bytes`). Must be ≥ 256 KiB. | +| `swapLimit` | integer | No | Swap+memory limit in bytes (`memory.memsw.limit_in_bytes`). Must be ≥ `memLimit`. Defaults to `memLimit` (no extra swap). | +| `env` | array | No | Environment variables in `"KEY=VALUE"` format. | +| `cwd` | string | No | Working directory inside the container. | +| `console` | object | No | Console log settings: `path` and `limit` (bytes). | +| `etc` | object | No | Inline `/etc` file content (`passwd`, `group`, `hosts`, `services`, `ld.so.preload`). | +| `network` | string | No | Network mode: `"nat"`, `"open"`, or `"private"`. Defaults to `"private"`. | +| `mounts` | array | No | Additional bind-mounts into the container. | +| `cpu` | object | No | CPU cgroup settings: `shares` (percentage 1–100) and `cores` (bitmask string). | +| `rtPriority` | object | No | Real-time scheduling priority settings. | +| `userNs` | boolean | No | Enable user namespacing. Defaults to `true`. | +| `gpu` | object | No | GPU device node access settings. | +| `vpu` | object | No | VPU device node access settings. | +| `devices` | array | No | Additional device nodes to whitelist. | +| `capabilities` | array | No | Linux capabilities to grant the container. | +| `seccomp` | object | No | Seccomp syscall filter profile. | +| `syslog` | object | No | Syslog plugin configuration. | +| `dbus` | object | No | D-Bus access configuration. | +| `restartOnCrash` | boolean | No | Restart the container automatically if it crashes. | +| `plugins` | object | No | Legacy plugin configuration (prefer `rdkPlugins`). | + +### Memory configuration example + +```json +{ + "version": "1.0", + "args": [ "/usr/bin/myapp" ], + "user": { "uid": 1000, "gid": 1000 }, + "memLimit": 67108864, + "swapLimit": 134217728 +} +``` + +`swapLimit` sets the combined memory+swap ceiling enforced by the kernel cgroup (`memory.memsw.limit_in_bytes`). When omitted, swap is capped at the same value as `memLimit`, effectively disabling extra swap for the container. + ## DobbyTool This is a simple command line tool that is used for debugging purporses. It connects to the Dobby daemon over dbus and allows for debugging and testing containers. diff --git a/bundle/lib/include/DobbySpecConfig.h b/bundle/lib/include/DobbySpecConfig.h index bc10574ab..8f6bede21 100644 --- a/bundle/lib/include/DobbySpecConfig.h +++ b/bundle/lib/include/DobbySpecConfig.h @@ -135,6 +135,7 @@ class DobbySpecConfig : public DobbyConfig JSON_FIELD_PROCESSOR(processMounts); JSON_FIELD_PROCESSOR(processLegacyPlugins); JSON_FIELD_PROCESSOR(processMemLimit); + JSON_FIELD_PROCESSOR(processSwapLimit); JSON_FIELD_PROCESSOR(processGpu); JSON_FIELD_PROCESSOR(processVpu); JSON_FIELD_PROCESSOR(processDbus); diff --git a/bundle/lib/source/DobbySpecConfig.cpp b/bundle/lib/source/DobbySpecConfig.cpp index 326d428a3..e46ee1d91 100644 --- a/bundle/lib/source/DobbySpecConfig.cpp +++ b/bundle/lib/source/DobbySpecConfig.cpp @@ -62,6 +62,8 @@ static const ctemplate::StaticTemplateString USERNS_DISABLED = static const ctemplate::StaticTemplateString MEM_LIMIT = STS_INIT(MEM_LIMIT, "MEM_LIMIT"); +static const ctemplate::StaticTemplateString MEM_SWAP = + STS_INIT(MEM_SWAP, "MEM_SWAP"); static const ctemplate::StaticTemplateString CPU_SHARES_ENABLED = STS_INIT(CPU_SHARES_ENABLED, "CPU_SHARES_ENABLED"); @@ -187,6 +189,7 @@ static const ctemplate::StaticTemplateString SECCOMP_SYSCALLS = #define JSON_FLAG_FILECAPABILITIES (0x1U << 20) #define JSON_FLAG_VPU (0x1U << 21) #define JSON_FLAG_SECCOMP (0x1U << 22) +#define JSON_FLAG_SWAPLIMIT (0x1U << 23) int DobbySpecConfig::mNumCores = -1; @@ -504,7 +507,8 @@ bool DobbySpecConfig::parseSpec(ctemplate::TemplateDictionary* dictionary, { "cpu", { JSON_FLAG_CPU, &DobbySpecConfig::processCpu } }, { "devices", { JSON_FLAG_DEVICES, &DobbySpecConfig::processDevices } }, { "capabilities", { JSON_FLAG_CAPABILITIES, &DobbySpecConfig::processCapabilities } }, - { "seccomp", { JSON_FLAG_SECCOMP, &DobbySpecConfig::processSeccomp } } + { "seccomp", { JSON_FLAG_SECCOMP, &DobbySpecConfig::processSeccomp } }, + { "swapLimit", { JSON_FLAG_SWAPLIMIT, &DobbySpecConfig::processSwapLimit } } }; // step 1 - parse the 'dobby' spec document @@ -627,6 +631,16 @@ bool DobbySpecConfig::parseSpec(ctemplate::TemplateDictionary* dictionary, dictionary->SetIntValue(RLIMIT_RTPRIO, 0); } + if (!(flags & JSON_FLAG_SWAPLIMIT)) + { + // swapLimit not supplied: default swap to memLimit (no extra swap) + const Json::Value& memLimitVal = mSpec["memLimit"]; + if (memLimitVal.isIntegral()) + { + dictionary->SetIntValue(MEM_SWAP, memLimitVal.asUInt()); + } + } + if (!(flags & JSON_FLAG_CAPABILITIES)) { dictionary->SetValue(NO_NEW_PRIVS, "true"); @@ -1278,6 +1292,54 @@ bool DobbySpecConfig::processMemLimit(const Json::Value& value, return true; } +// ----------------------------------------------------------------------------- +/** + * @brief Processes the optional swap limit field. + * + * When present, this value is used as the cgroup memory.memsw.limit_in_bytes, + * allowing swap to be configured independently of the memory limit. When + * absent the swap limit defaults to the same value as memLimit (i.e. no + * extra swap beyond the memory limit). + * + * The kernel requires swap >= memLimit, so an error is returned if the + * supplied value is smaller than the memLimit already set. + * + * Example json: + * + * "swapLimit": 2097152 + * + * + * + * @param[in] value The json spec document from the client + * @param[in] dictionary Pointer to the OCI dictionary to populate + * + * @return true if correctly processed the value, otherwise false. + */ +bool DobbySpecConfig::processSwapLimit(const Json::Value& value, + ctemplate::TemplateDictionary* dictionary) +{ + if (!value.isIntegral()) + { + AI_LOG_ERROR("invalid swapLimit field"); + return false; + } + + unsigned memSwap = value.asUInt(); + + // the kernel requires memory.memsw.limit_in_bytes >= memory.limit_in_bytes + const Json::Value& memLimitVal = mSpec["memLimit"]; + if (memLimitVal.isIntegral() && (memSwap < memLimitVal.asUInt())) + { + AI_LOG_ERROR("swapLimit (%u) must be >= memLimit (%u)", + memSwap, memLimitVal.asUInt()); + return false; + } + + dictionary->SetIntValue(MEM_SWAP, memSwap); + + return true; +} + // ----------------------------------------------------------------------------- /** * @brief Adds the GPU device nodes (if any) to supplied dictionary. diff --git a/bundle/lib/source/templates/OciConfigJson1.0.2-dobby.template b/bundle/lib/source/templates/OciConfigJson1.0.2-dobby.template index 6cbdabd43..907ac8bec 100644 --- a/bundle/lib/source/templates/OciConfigJson1.0.2-dobby.template +++ b/bundle/lib/source/templates/OciConfigJson1.0.2-dobby.template @@ -328,7 +328,7 @@ static const char* ociJsonTemplate = R"JSON( ], "memory": { "limit": {{MEM_LIMIT}}, - "swap": {{MEM_LIMIT}}, + "swap": {{MEM_SWAP}}, "swappiness": 60 }, "cpu": { diff --git a/bundle/lib/source/templates/OciConfigJsonVM1.0.2-dobby.template b/bundle/lib/source/templates/OciConfigJsonVM1.0.2-dobby.template index 21fe91d38..7b6537d7f 100644 --- a/bundle/lib/source/templates/OciConfigJsonVM1.0.2-dobby.template +++ b/bundle/lib/source/templates/OciConfigJsonVM1.0.2-dobby.template @@ -339,7 +339,7 @@ static const char* ociJsonTemplate = R"JSON( ], "memory": { "limit": {{MEM_LIMIT}}, - "swap": {{MEM_LIMIT}}, + "swap": {{MEM_SWAP}}, "swappiness": 60 }, "cpu": { diff --git a/tests/L1_testing/tests/DobbySpecConfigTest/CMakeLists.txt b/tests/L1_testing/tests/DobbySpecConfigTest/CMakeLists.txt new file mode 100644 index 000000000..e0f7c5756 --- /dev/null +++ b/tests/L1_testing/tests/DobbySpecConfigTest/CMakeLists.txt @@ -0,0 +1,72 @@ +# If not stated otherwise in this file or this component's LICENSE file the +# following copyright and licenses apply: +# +# Copyright 2024 Sky UK +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +cmake_minimum_required(VERSION 3.7) +project(DobbySpecConfigL1Test) + +set(CMAKE_CXX_STANDARD 14) + +find_package(GTest REQUIRED) +find_package(ctemplate REQUIRED) +find_package(jsoncpp REQUIRED) + +include_directories(${GTEST_INCLUDE_DIRS}) + +add_library(DobbySpecConfigTestLib SHARED STATIC + ../../../../bundle/lib/source/DobbySpecConfig.cpp + ../../../../bundle/lib/source/DobbyTemplate.cpp + ../../../../AppInfrastructure/Logging/source/Logging.cpp + ../../mocks/ContainerIdMock.cpp + ../../mocks/DobbyBundleMock.cpp + ../../mocks/DobbyUtilsMock.cpp + ) + +target_include_directories(DobbySpecConfigTestLib + PUBLIC + ../../mocks + ../../../../utils/include + ../../../../utils/source + ../../../../AppInfrastructure/Logging/include + ../../../../AppInfrastructure/Common/include + ../../../../bundle/lib/include + ../../../../bundle/lib/source + ../../../../ipcUtils/include + ../../../../settings/include + ../../../../daemon/lib/include + ../../../../libocispec/generated_output + ../../../../pluginLauncher/lib/include + ../../../../protocol/include + ../../../../build/AppInfrastructure/Tracing + /usr/include/jsoncpp + ) + +file(GLOB TESTS *.cpp) + +add_executable(${PROJECT_NAME} ${TESTS}) + +target_link_libraries(${PROJECT_NAME} + PRIVATE + DobbySpecConfigTestLib + GTest::gmock + GTest::GTest + GTest::Main + ctemplate + pthread + jsoncpp +) + +install(TARGETS ${PROJECT_NAME} DESTINATION bin) diff --git a/tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigTest.cpp b/tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigTest.cpp new file mode 100644 index 000000000..cf740a76f --- /dev/null +++ b/tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigTest.cpp @@ -0,0 +1,221 @@ +/* +* If not stated otherwise in this file or this component's LICENSE file the +* following copyright and licenses apply: +* +* Copyright 2024 Sky UK +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +// Open private members so processSwapLimit / mSpec / mDictionary are accessible +#define private public +#include "DobbySpecConfig.h" + +#include +#include +#include +#include + +#include "DobbySettingsMock.h" +#include "DobbyBundleMock.h" +#include "DobbyUtilsMock.h" +#include "ContainerIdMock.h" + +using ::testing::NiceMock; +using ::testing::Return; + +// ── Static impl pointers required by the mock delegate pattern ──────────────── +DobbyBundleImpl* DobbyBundle::impl = nullptr; +DobbyUtilsImpl* DobbyUtils::impl = nullptr; +ContainerIdImpl* ContainerId::impl = nullptr; + +// ── Minimal valid Dobby spec strings ───────────────────────────────────────── +// (mandatory fields: version, args, user, memLimit) + +static const char* kSpecMemOnly = R"({ + "version": "1.0", + "args": ["/bin/true"], + "user": { "uid": 1000, "gid": 1000 }, + "memLimit": 2998272 +})"; + +static const char* kSpecWithSwap = R"({ + "version": "1.0", + "args": ["/bin/true"], + "user": { "uid": 1000, "gid": 1000 }, + "memLimit": 2998272, + "swapLimit": 5996544 +})"; + +static const char* kSpecSwapEqualsLimit = R"({ + "version": "1.0", + "args": ["/bin/true"], + "user": { "uid": 1000, "gid": 1000 }, + "memLimit": 2998272, + "swapLimit": 2998272 +})"; + +static const char* kSpecSwapBelowLimit = R"({ + "version": "1.0", + "args": ["/bin/true"], + "user": { "uid": 1000, "gid": 1000 }, + "memLimit": 5996544, + "swapLimit": 2998272 +})"; + +// ── Inline ctemplate used to read MEM_LIMIT / MEM_SWAP out of the dictionary ── +static const char* kMemTemplateName = "test_swap_memory"; +static const char* kMemTemplateStr = "LIMIT={{MEM_LIMIT}} SWAP={{MEM_SWAP}}"; + +// ── Test fixture ────────────────────────────────────────────────────────────── + +class DobbySpecConfigTest : public ::testing::Test +{ +protected: + NiceMock* p_settingsMock = nullptr; + NiceMock* p_bundleMock = nullptr; + NiceMock* p_utilsMock = nullptr; + + std::shared_ptr mSettings; + std::shared_ptr mBundle; + std::shared_ptr mUtils; + + void SetUp() override + { + p_settingsMock = new NiceMock(); + p_bundleMock = new NiceMock(); + p_utilsMock = new NiceMock(); + + DobbyBundle::setImpl(p_bundleMock); + DobbyUtils::setImpl(p_utilsMock); + + // GPU/VPU settings not needed for these tests + ON_CALL(*p_settingsMock, gpuAccessSettings()) + .WillByDefault(Return(nullptr)); + ON_CALL(*p_settingsMock, vpuAccessSettings()) + .WillByDefault(Return(nullptr)); + ON_CALL(*p_settingsMock, defaultPlugins()) + .WillByDefault(Return(std::vector{})); + ON_CALL(*p_settingsMock, rdkPluginsData()) + .WillByDefault(Return(Json::Value(Json::objectValue))); + ON_CALL(*p_settingsMock, extraEnvVariables()) + .WillByDefault(Return(std::map{})); + + // dirFd() returns -1 so the template-write at step 8 of parseSpec + // fails, but all dictionary values are populated by the preceding + // steps. isValid() will be false for all configs constructed here. + ON_CALL(*p_bundleMock, dirFd()) + .WillByDefault(Return(-1)); + + // Use a no-op deleter so mock lifetime is managed by this fixture + mSettings = std::shared_ptr(p_settingsMock, + [](IDobbySettings*){}); + mBundle = std::make_shared(); + mUtils = std::make_shared(); + + // Register a tiny inline template so we can read MEM_LIMIT / MEM_SWAP + // back out of the dictionary without expanding the full OCI template. + ctemplate::StringToTemplateCache( + kMemTemplateName, + kMemTemplateStr, + ctemplate::DO_NOT_STRIP); + } + + void TearDown() override + { + DobbyBundle::setImpl(nullptr); + DobbyUtils::setImpl(nullptr); + + delete p_bundleMock; + delete p_utilsMock; + delete p_settingsMock; + } + + // Construct a DobbySpecConfig from the given JSON string. + std::unique_ptr makeConfig(const std::string& specJson) + { + return std::make_unique(mUtils, mSettings, mBundle, specJson); + } + + // Expand the test template against the config's ctemplate dictionary. + // Returns a string of the form "LIMIT= SWAP=". + std::string expandMemTemplate(DobbySpecConfig& cfg) + { + std::string out; + ctemplate::ExpandTemplate( + kMemTemplateName, + ctemplate::DO_NOT_STRIP, + cfg.mDictionary, + &out); + return out; + } +}; + +// ── Test cases ──────────────────────────────────────────────────────────────── + +/** + * When 'swapLimit' is absent, MEM_SWAP must default to MEM_LIMIT + * (no extra swap beyond the memory limit). + */ +TEST_F(DobbySpecConfigTest, SwapLimit_DefaultsToMemLimit) +{ + auto cfg = makeConfig(kSpecMemOnly); + EXPECT_EQ(expandMemTemplate(*cfg), "LIMIT=2998272 SWAP=2998272"); +} + +/** + * When 'swapLimit' is greater than 'memLimit', MEM_SWAP must be set to the + * supplied swap limit independently of MEM_LIMIT. + */ +TEST_F(DobbySpecConfigTest, SwapLimit_SetIndependently) +{ + auto cfg = makeConfig(kSpecWithSwap); + EXPECT_EQ(expandMemTemplate(*cfg), "LIMIT=2998272 SWAP=5996544"); +} + +/** + * When 'swapLimit' equals 'memLimit' (minimum valid value), parsing must + * succeed and MEM_SWAP must equal the shared value. + */ +TEST_F(DobbySpecConfigTest, SwapLimit_EqualToMemLimit_Succeeds) +{ + auto cfg = makeConfig(kSpecSwapEqualsLimit); + EXPECT_EQ(expandMemTemplate(*cfg), "LIMIT=2998272 SWAP=2998272"); +} + +/** + * When 'swapLimit' < 'memLimit', processSwapLimit must return false because + * the kernel rejects memory.memsw.limit_in_bytes < memory.limit_in_bytes. + */ +TEST_F(DobbySpecConfigTest, SwapLimit_LessThanMemLimit_Fails) +{ + // Construct a config so mSpec is populated with memLimit=5996544 + DobbySpecConfig cfg(mUtils, mSettings, mBundle, kSpecSwapBelowLimit); + + // Directly invoke processSwapLimit with a value that is below memLimit + ctemplate::TemplateDictionary dict("test_below"); + Json::Value badSwap(2998272); // 2998272 < 5996544 + EXPECT_FALSE(cfg.processSwapLimit(badSwap, &dict)); +} + +/** + * When 'swapLimit' is not an integer, processSwapLimit must return false. + */ +TEST_F(DobbySpecConfigTest, SwapLimit_NonIntegral_Fails) +{ + DobbySpecConfig cfg(mUtils, mSettings, mBundle, kSpecMemOnly); + + ctemplate::TemplateDictionary dict("test_nonint"); + Json::Value badSwap("not-a-number"); + EXPECT_FALSE(cfg.processSwapLimit(badSwap, &dict)); +} diff --git a/tests/L2_testing/dobby_specs/swap_limit.json b/tests/L2_testing/dobby_specs/swap_limit.json new file mode 100644 index 000000000..12502b32c --- /dev/null +++ b/tests/L2_testing/dobby_specs/swap_limit.json @@ -0,0 +1,29 @@ +{ + "version": "1.0", + "cwd": "/", + "args": [ + "cat", + "/sys/fs/cgroup/memory/memory.memsw.limit_in_bytes" + ], + "env": [], + "user": { + "uid": 1000, + "gid": 1000 + }, + "console": { + "limit": 65536, + "path": "/tmp/swap_limit.log" + }, + "etc": { + "group": [ + "root:x:0:" + ], + "passwd": [ + "root::0:0:root:/:/bin/false" + ] + }, + "memLimit": 2998272, + "swapLimit": 5996544, + "network": "nat", + "mounts": [] +} diff --git a/tests/L2_testing/test_runner/swap_limit_tests.py b/tests/L2_testing/test_runner/swap_limit_tests.py new file mode 100644 index 000000000..2da7df7fc --- /dev/null +++ b/tests/L2_testing/test_runner/swap_limit_tests.py @@ -0,0 +1,110 @@ +# If not stated otherwise in this file or this component's LICENSE file the +# following copyright and licenses apply: +# +# Copyright 2024 Sky UK +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import test_utils + +tests = [ + test_utils.Test( + "Swap limit default", + "ram", + str(2998272), + "Starts a container with only memLimit set and verifies that " + "memory.memsw.limit_in_bytes equals memLimit (no extra swap)"), + test_utils.Test( + "Swap limit override", + "swap_limit", + str(5996544), + "Starts a container with swapLimit > memLimit and verifies that " + "memory.memsw.limit_in_bytes reflects the independent swapLimit value"), +] + + +def execute_test(): + with test_utils.dobby_daemon(): + output_table = [] + + for test in tests: + result = test_container(test.container_id, test.expected_output) + output = test_utils.create_simple_test_output(test, result[0], result[1]) + output_table.append(output) + test_utils.print_single_result(output) + + return test_utils.count_print_results(output_table) + + +def test_container(container_id, expected_swap): + """Launch a container and verify its cgroup swap limit. + + The container cats /sys/fs/cgroup/memory/memory.memsw.limit_in_bytes + to its console log, which is then read and compared against the + expected value. + + Parameters: + container_id (str): name of the spec (without .json extension) + expected_swap (str): expected numeric string from the cgroup file + + Returns: + (bool, str): (passed, message) + """ + test_utils.print_log("Running swap limit test for '%s'" % container_id, + test_utils.Severity.debug) + + spec_path = test_utils.get_container_spec_path(container_id) + launched = test_utils.launch_container(container_id, spec_path) + + if not launched: + return False, "Container '%s' failed to launch" % container_id + + return validate_swap_limit(container_id, expected_swap) + + +def validate_swap_limit(container_id, expected_swap): + """Read the container log and compare the swap cgroup value. + + If the log is empty (swap accounting disabled in the kernel), the test + is treated as a skip rather than a failure so CI does not break on + platforms where 'swapaccount=1' has not been set on the kernel cmdline. + + Parameters: + container_id (str): container whose log to inspect + expected_swap (str): expected value as a decimal string + + Returns: + (bool, str): (passed, message) + """ + log = test_utils.get_container_log(container_id) + + if not log: + test_utils.print_log( + "No log output from '%s' – swap accounting may be disabled " + "(kernel cmdline requires 'swapaccount=1')" % container_id, + test_utils.Severity.warning) + return True, "Skipped – swap accounting not available on this platform" + + actual = log.strip() + + if actual == expected_swap: + return True, "Test passed" + + return (False, + "Swap limit mismatch for '%s': expected %s, got %s" + % (container_id, expected_swap, actual)) + + +if __name__ == "__main__": + test_utils.parse_arguments(__file__, True) + execute_test() From 8e4394b16fda91e3131c727ba6bb96b1fe064a9a Mon Sep 17 00:00:00 2001 From: Gurdal Oruklu Date: Tue, 31 Mar 2026 15:35:42 -0700 Subject: [PATCH 3/8] fixed swaplimit L1 tests --- .github/workflows/L1-tests.yml | 56 ++++---- tests/L1_testing/mocks/DobbyBundle.h | 4 + tests/L1_testing/mocks/DobbyBundleMock.cpp | 11 ++ tests/L1_testing/mocks/DobbyBundleMock.h | 2 + tests/L1_testing/tests/CMakeLists.txt | 4 +- .../tests/DobbySpecConfigTest/CMakeLists.txt | 19 ++- .../DobbySpecConfigLinkStubs.cpp | 56 ++++++++ .../DobbySpecConfigTest.cpp | 121 +++++++++--------- .../DobbyTemplateStubs.cpp | 47 +++++++ 9 files changed, 232 insertions(+), 88 deletions(-) create mode 100644 tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigLinkStubs.cpp create mode 100644 tests/L1_testing/tests/DobbySpecConfigTest/DobbyTemplateStubs.cpp diff --git a/.github/workflows/L1-tests.yml b/.github/workflows/L1-tests.yml index 43a9059f3..fbf1d252e 100755 --- a/.github/workflows/L1-tests.yml +++ b/.github/workflows/L1-tests.yml @@ -11,8 +11,8 @@ jobs: strategy: fail-fast: false matrix: - compiler: [ gcc, clang ] - coverage: [ with-coverage, without-coverage ] + compiler: [gcc, clang] + coverage: [with-coverage, without-coverage] exclude: - compiler: clang coverage: with-coverage @@ -25,7 +25,17 @@ jobs: # If adding a RUN_TESTS cmake option, it will build with enabling optional_flags and run the L1 tests # matrix runs both versions build_type: ["Release", "Debug"] - extra_flags: [ "RUN_TESTS", "-DLEGACY_COMPONENTS=ON", "-DLEGACY_COMPONENTS=OFF", "-DUSE_SYSTEMD=ON", "-DUSE_SYSTEMD=OFF", "-DDOBBY_HIBERNATE_MEMCR_IMPL=ON -DDOBBY_HIBERNATE_MEMCR_PARAMS_ENABLED=OFF", "-DDOBBY_HIBERNATE_MEMCR_IMPL=ON -DDOBBY_HIBERNATE_MEMCR_PARAMS_ENABLED=ON", "-DDOBBY_HIBERNATE_MEMCR_IMPL=OFF"] + extra_flags: + [ + "RUN_TESTS", + "-DLEGACY_COMPONENTS=ON", + "-DLEGACY_COMPONENTS=OFF", + "-DUSE_SYSTEMD=ON", + "-DUSE_SYSTEMD=OFF", + "-DDOBBY_HIBERNATE_MEMCR_IMPL=ON -DDOBBY_HIBERNATE_MEMCR_PARAMS_ENABLED=OFF", + "-DDOBBY_HIBERNATE_MEMCR_IMPL=ON -DDOBBY_HIBERNATE_MEMCR_PARAMS_ENABLED=ON", + "-DDOBBY_HIBERNATE_MEMCR_IMPL=OFF", + ] name: Build in ${{ matrix.build_type }} Mode (${{ matrix.extra_flags }}) steps: - name: checkout @@ -48,27 +58,27 @@ jobs: - name: Install gmock run: | - cd $GITHUB_WORKSPACE - git clone https://github.com/google/googletest.git -b release-1.11.0 - cd googletest - mkdir build - cd build - cmake .. - make - sudo make install + cd $GITHUB_WORKSPACE + git clone https://github.com/google/googletest.git -b release-1.11.0 + cd googletest + mkdir build + cd build + cmake .. + make + sudo make install - name: build dobby run: | - cd $GITHUB_WORKSPACE - mkdir build - cd build - if [ ${{ matrix.extra_flags }} = "RUN_TESTS" ] - then - cmake -DCMAKE_TOOLCHAIN_FILE="${{ env.TOOLCHAIN_FILE }}" -DRDK_PLATFORM=DEV_VM -DCMAKE_INSTALL_PREFIX:PATH=/usr -DENABLE_DOBBYL1TEST=ON -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} ${{ env.optional_flags }} ${{ env.optional_plugins }} .. - else - cmake -DCMAKE_TOOLCHAIN_FILE="${{ env.TOOLCHAIN_FILE }}" -DRDK_PLATFORM=DEV_VM -DCMAKE_INSTALL_PREFIX:PATH=/usr -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} ${{ matrix.extra_flags }} ${{ env.optional_plugins }} .. - fi - make -j $(nproc) + cd $GITHUB_WORKSPACE + mkdir build + cd build + if [ ${{ matrix.extra_flags }} = "RUN_TESTS" ] + then + cmake -DCMAKE_TOOLCHAIN_FILE="${{ env.TOOLCHAIN_FILE }}" -DRDK_PLATFORM=DEV_VM -DCMAKE_INSTALL_PREFIX:PATH=/usr -DENABLE_DOBBYL1TEST=ON -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} ${{ env.optional_flags }} ${{ env.optional_plugins }} .. + else + cmake -DCMAKE_TOOLCHAIN_FILE="${{ env.TOOLCHAIN_FILE }}" -DRDK_PLATFORM=DEV_VM -DCMAKE_INSTALL_PREFIX:PATH=/usr -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} ${{ matrix.extra_flags }} ${{ env.optional_plugins }} .. + fi + make -j $(nproc) - name: run l1-tests if: ${{ matrix.extra_flags == 'RUN_TESTS' && matrix.build_type == 'Debug' }} @@ -76,6 +86,7 @@ jobs: sudo valgrind --tool=memcheck --leak-check=yes --show-reachable=yes --track-fds=yes --fair-sched=try $GITHUB_WORKSPACE/build/tests/L1_testing/tests/DobbyTest/DobbyL1Test --gtest_output="json:$(pwd)/DobbyL1TestResults.json" sudo $GITHUB_WORKSPACE/build/tests/L1_testing/tests/DobbyUtilsTest/DobbyUtilsL1Test --gtest_output="json:$(pwd)/DobbyUtilsL1TestResults.json" sudo valgrind --tool=memcheck --leak-check=yes --show-reachable=yes --track-fds=yes --fair-sched=try $GITHUB_WORKSPACE/build/tests/L1_testing/tests/DobbyManagerTest/DobbyManagerL1Test --gtest_output="json:$(pwd)/DobbyManagerL1TestResults.json" + sudo valgrind --tool=memcheck --leak-check=yes --show-reachable=yes --track-fds=yes --fair-sched=try $GITHUB_WORKSPACE/build/tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigL1Test --gtest_output="json:$(pwd)/DobbySpecConfigL1TestResults.json" - name: Generate coverage if: ${{ matrix.coverage == 'with-coverage' && matrix.extra_flags == 'RUN_TESTS' && matrix.build_type == 'Debug' }} @@ -83,7 +94,7 @@ jobs: lcov --rc geninfo_unexecuted_blocks=1 --ignore-errors source - --ignore-errors mismatch + --ignore-errors mismatch -c -o coverage.info -d $GITHUB_WORKSPACE @@ -108,5 +119,6 @@ jobs: DobbyL1TestResults.json DobbyUtilsL1TestResults.json DobbyManagerL1TestResults.json + DobbySpecConfigL1TestResults.json coverage if-no-files-found: warn diff --git a/tests/L1_testing/mocks/DobbyBundle.h b/tests/L1_testing/mocks/DobbyBundle.h index 69357b93a..a0dbbc7df 100644 --- a/tests/L1_testing/mocks/DobbyBundle.h +++ b/tests/L1_testing/mocks/DobbyBundle.h @@ -31,6 +31,8 @@ class DobbyBundleImpl { virtual ~DobbyBundleImpl() = default; virtual void setPersistence(bool persist) = 0; + virtual bool getPersistence() const = 0; + virtual int dirFd() const = 0; virtual bool isValid() const = 0; virtual const std::string& path() const = 0; @@ -58,6 +60,8 @@ class DobbyBundle { static void setImpl(DobbyBundleImpl* newImpl); void setPersistence(bool persist); + bool getPersistence() const; + int dirFd() const; bool isValid() const; }; diff --git a/tests/L1_testing/mocks/DobbyBundleMock.cpp b/tests/L1_testing/mocks/DobbyBundleMock.cpp index b4e4a6fec..126ffbcd2 100755 --- a/tests/L1_testing/mocks/DobbyBundleMock.cpp +++ b/tests/L1_testing/mocks/DobbyBundleMock.cpp @@ -47,3 +47,14 @@ const std::string& DobbyBundle::path() const return impl->path(); } +bool DobbyBundle::getPersistence() const +{ + EXPECT_NE(impl, nullptr); + return impl->getPersistence(); +} + +int DobbyBundle::dirFd() const +{ + EXPECT_NE(impl, nullptr); + return impl->dirFd(); +} diff --git a/tests/L1_testing/mocks/DobbyBundleMock.h b/tests/L1_testing/mocks/DobbyBundleMock.h index 996e439e8..368fb0936 100644 --- a/tests/L1_testing/mocks/DobbyBundleMock.h +++ b/tests/L1_testing/mocks/DobbyBundleMock.h @@ -28,6 +28,8 @@ class DobbyBundleMock : public DobbyBundleImpl { virtual ~DobbyBundleMock() = default; MOCK_METHOD(void, setPersistence, (bool persist), (override)); + MOCK_METHOD(bool, getPersistence, (), (const,override)); + MOCK_METHOD(int, dirFd, (), (const,override)); MOCK_METHOD(bool, isValid, (), (const,override)); MOCK_METHOD((const std::string&), path, (), (const,override)); }; diff --git a/tests/L1_testing/tests/CMakeLists.txt b/tests/L1_testing/tests/CMakeLists.txt index 26010cf2e..b30693b92 100644 --- a/tests/L1_testing/tests/CMakeLists.txt +++ b/tests/L1_testing/tests/CMakeLists.txt @@ -17,4 +17,6 @@ add_subdirectory(DobbyUtilsTest) add_subdirectory(DobbyTest) -add_subdirectory(DobbyManagerTest) \ No newline at end of file +add_subdirectory(DobbyManagerTest) +add_subdirectory(DobbySpecConfigTest) + diff --git a/tests/L1_testing/tests/DobbySpecConfigTest/CMakeLists.txt b/tests/L1_testing/tests/DobbySpecConfigTest/CMakeLists.txt index e0f7c5756..5a7b67807 100644 --- a/tests/L1_testing/tests/DobbySpecConfigTest/CMakeLists.txt +++ b/tests/L1_testing/tests/DobbySpecConfigTest/CMakeLists.txt @@ -26,24 +26,30 @@ find_package(jsoncpp REQUIRED) include_directories(${GTEST_INCLUDE_DIRS}) +# Real sources only – no PIMPL mock wrappers for DobbyBundle or DobbyTemplate. +# DobbySettingsMock is the only mock used; it implements IDobbySettings via gmock. add_library(DobbySpecConfigTestLib SHARED STATIC ../../../../bundle/lib/source/DobbySpecConfig.cpp ../../../../bundle/lib/source/DobbyTemplate.cpp + ../../../../bundle/lib/source/DobbyBundle.cpp ../../../../AppInfrastructure/Logging/source/Logging.cpp - ../../mocks/ContainerIdMock.cpp - ../../mocks/DobbyBundleMock.cpp - ../../mocks/DobbyUtilsMock.cpp + ../../mocks/DobbyConfigMock.cpp + ../../mocks/IpcFileDescriptorMock.cpp + ../../mocks/rt_dobby_schema.c + DobbySpecConfigLinkStubs.cpp ) +# Real bundle/lib headers MUST come before the mocks directory so that the +# real DobbyTemplate.h (with ctemplate members) is found instead of the +# stripped-down mock version. target_include_directories(DobbySpecConfigTestLib PUBLIC - ../../mocks + ../../../../bundle/lib/include + ../../../../bundle/lib/source ../../../../utils/include ../../../../utils/source ../../../../AppInfrastructure/Logging/include ../../../../AppInfrastructure/Common/include - ../../../../bundle/lib/include - ../../../../bundle/lib/source ../../../../ipcUtils/include ../../../../settings/include ../../../../daemon/lib/include @@ -51,6 +57,7 @@ target_include_directories(DobbySpecConfigTestLib ../../../../pluginLauncher/lib/include ../../../../protocol/include ../../../../build/AppInfrastructure/Tracing + ../../mocks /usr/include/jsoncpp ) diff --git a/tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigLinkStubs.cpp b/tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigLinkStubs.cpp new file mode 100644 index 000000000..90a1b59ba --- /dev/null +++ b/tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigLinkStubs.cpp @@ -0,0 +1,56 @@ +/* +* If not stated otherwise in this file or this component's LICENSE file the +* following copyright and licenses apply: +* +* Copyright 2024 Sky UK +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +// Link-time stubs for symbols referenced by DobbySpecConfig.cpp that are +// never called during unit testing. The 4-arg DobbySpecConfig constructor +// does not invoke convertToCompliant or rt_dobby_schema_parse_file, and +// addGpuDevNodes / addVpuDevNodes are only reached when gpuAccessSettings() +// returns non-null (our mock returns nullptr). + +#include "IpcCommon.h" // IAsyncReplySender / IAsyncReplySenderApiImpl +#include "ContainerId.h" +#include +#include +#include + +// ── IPC ─────────────────────────────────────────────────────────────────────── +// Required by the IpcCommon.h PIMPL inline functions that are instantiated +// during test compilation even though sendReply is never called. +AI_IPC::IAsyncReplySenderApiImpl* AI_IPC::IAsyncReplySender::impl = nullptr; + +// ── DobbyConfig ─────────────────────────────────────────────────────────────── +// Weak stubs – never called because (a) we use the 4-arg constructor which +// skips convertToCompliant, and (b) gpuAccessSettings() returns nullptr so +// addGpuDevNodes / addVpuDevNodes are never entered. + +#include "DobbyConfig.h" + +bool DobbyConfig::convertToCompliant( + const ContainerId& /*id*/, + std::shared_ptr /*cfg*/, + const std::string& /*rootfsPath*/) +{ + return true; // never called in these tests +} + +std::list DobbyConfig::scanDevNodes( + const std::list& /*devNodes*/) +{ + return {}; // never called in these tests +} diff --git a/tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigTest.cpp b/tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigTest.cpp index cf740a76f..ddfa43fea 100644 --- a/tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigTest.cpp +++ b/tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigTest.cpp @@ -17,30 +17,30 @@ * limitations under the License. */ -// Open private members so processSwapLimit / mSpec / mDictionary are accessible -#define private public -#include "DobbySpecConfig.h" - +// All system and third-party headers must come BEFORE #define private public +// to prevent the macro from mangling standard library internals. #include #include #include #include +#include +#include +#include +// DobbySettingsMock.h pulls in gmock which eventually includes ; +// it must be included before #define private public. #include "DobbySettingsMock.h" -#include "DobbyBundleMock.h" -#include "DobbyUtilsMock.h" -#include "ContainerIdMock.h" + +// Open up private members of DobbySpecConfig so tests can access +// mDictionary directly and call processSwapLimit. +#define private public +#include "DobbySpecConfig.h" +#include "DobbyTemplate.h" using ::testing::NiceMock; using ::testing::Return; -// ── Static impl pointers required by the mock delegate pattern ──────────────── -DobbyBundleImpl* DobbyBundle::impl = nullptr; -DobbyUtilsImpl* DobbyUtils::impl = nullptr; -ContainerIdImpl* ContainerId::impl = nullptr; - // ── Minimal valid Dobby spec strings ───────────────────────────────────────── -// (mandatory fields: version, args, user, memLimit) static const char* kSpecMemOnly = R"({ "version": "1.0", @@ -73,33 +73,32 @@ static const char* kSpecSwapBelowLimit = R"({ "swapLimit": 2998272 })"; -// ── Inline ctemplate used to read MEM_LIMIT / MEM_SWAP out of the dictionary ── +// ── Inline ctemplate for reading MEM_LIMIT / MEM_SWAP back from the dict ───── static const char* kMemTemplateName = "test_swap_memory"; static const char* kMemTemplateStr = "LIMIT={{MEM_LIMIT}} SWAP={{MEM_SWAP}}"; -// ── Test fixture ────────────────────────────────────────────────────────────── +// ── Fixture ─────────────────────────────────────────────────────────────────── class DobbySpecConfigTest : public ::testing::Test { protected: - NiceMock* p_settingsMock = nullptr; - NiceMock* p_bundleMock = nullptr; - NiceMock* p_utilsMock = nullptr; + char mTmpDir[64]; + NiceMock* p_settingsMock = nullptr; std::shared_ptr mSettings; std::shared_ptr mBundle; - std::shared_ptr mUtils; void SetUp() override { + // Reserve a unique path for the bundle directory. + // mkdtemp creates the directory; we immediately remove it so that + // DobbyBundle(path, persist) can create it itself via mkdir(). + std::strcpy(mTmpDir, "/tmp/dobby_spectest_XXXXXX"); + ASSERT_NE(mkdtemp(mTmpDir), nullptr) << "mkdtemp failed"; + ::rmdir(mTmpDir); // let DobbyBundle create the dir + + // Settings mock – return empty/null for GPU, VPU, plugins. p_settingsMock = new NiceMock(); - p_bundleMock = new NiceMock(); - p_utilsMock = new NiceMock(); - - DobbyBundle::setImpl(p_bundleMock); - DobbyUtils::setImpl(p_utilsMock); - - // GPU/VPU settings not needed for these tests ON_CALL(*p_settingsMock, gpuAccessSettings()) .WillByDefault(Return(nullptr)); ON_CALL(*p_settingsMock, vpuAccessSettings()) @@ -111,20 +110,23 @@ class DobbySpecConfigTest : public ::testing::Test ON_CALL(*p_settingsMock, extraEnvVariables()) .WillByDefault(Return(std::map{})); - // dirFd() returns -1 so the template-write at step 8 of parseSpec - // fails, but all dictionary values are populated by the preceding - // steps. isValid() will be false for all configs constructed here. - ON_CALL(*p_bundleMock, dirFd()) - .WillByDefault(Return(-1)); - - // Use a no-op deleter so mock lifetime is managed by this fixture + // Use a no-op deleter; fixture owns the raw pointer. mSettings = std::shared_ptr(p_settingsMock, [](IDobbySettings*){}); - mBundle = std::make_shared(); - mUtils = std::make_shared(); + + // Real DobbyBundle pointing at the temp directory. + // utils is not used in the constructor body so nullptr is safe. + mBundle = std::make_shared( + std::shared_ptr(nullptr), + std::string(mTmpDir), + /*persist=*/true); + + // Initialise the DobbyTemplate singleton with our settings before any + // DobbySpecConfig is constructed (parseSpec calls applyAt internally). + DobbyTemplate::setSettings(mSettings); // Register a tiny inline template so we can read MEM_LIMIT / MEM_SWAP - // back out of the dictionary without expanding the full OCI template. + // from the populated dictionary without parsing the full OCI JSON. ctemplate::StringToTemplateCache( kMemTemplateName, kMemTemplateStr, @@ -133,22 +135,24 @@ class DobbySpecConfigTest : public ::testing::Test void TearDown() override { - DobbyBundle::setImpl(nullptr); - DobbyUtils::setImpl(nullptr); - - delete p_bundleMock; - delete p_utilsMock; + // Remove any config.json written during the test. + std::string cfg = std::string(mTmpDir) + "/config.json"; + ::remove(cfg.c_str()); + ::rmdir(mTmpDir); delete p_settingsMock; } - // Construct a DobbySpecConfig from the given JSON string. std::unique_ptr makeConfig(const std::string& specJson) { - return std::make_unique(mUtils, mSettings, mBundle, specJson); + return std::make_unique( + std::shared_ptr(nullptr), + mSettings, + mBundle, + specJson); } - // Expand the test template against the config's ctemplate dictionary. - // Returns a string of the form "LIMIT= SWAP=". + // Expand the mini template against the config's ctemplate dictionary. + // Returns e.g. "LIMIT=2998272 SWAP=2998272". std::string expandMemTemplate(DobbySpecConfig& cfg) { std::string out; @@ -161,15 +165,16 @@ class DobbySpecConfigTest : public ::testing::Test } }; -// ── Test cases ──────────────────────────────────────────────────────────────── +// ── Tests ───────────────────────────────────────────────────────────────────── /** - * When 'swapLimit' is absent, MEM_SWAP must default to MEM_LIMIT - * (no extra swap beyond the memory limit). + * When 'swapLimit' is absent, MEM_SWAP must default to the same value as + * MEM_LIMIT (no extra swap beyond the memory limit). */ TEST_F(DobbySpecConfigTest, SwapLimit_DefaultsToMemLimit) { auto cfg = makeConfig(kSpecMemOnly); + EXPECT_TRUE(cfg->isValid()); EXPECT_EQ(expandMemTemplate(*cfg), "LIMIT=2998272 SWAP=2998272"); } @@ -180,6 +185,7 @@ TEST_F(DobbySpecConfigTest, SwapLimit_DefaultsToMemLimit) TEST_F(DobbySpecConfigTest, SwapLimit_SetIndependently) { auto cfg = makeConfig(kSpecWithSwap); + EXPECT_TRUE(cfg->isValid()); EXPECT_EQ(expandMemTemplate(*cfg), "LIMIT=2998272 SWAP=5996544"); } @@ -190,32 +196,29 @@ TEST_F(DobbySpecConfigTest, SwapLimit_SetIndependently) TEST_F(DobbySpecConfigTest, SwapLimit_EqualToMemLimit_Succeeds) { auto cfg = makeConfig(kSpecSwapEqualsLimit); + EXPECT_TRUE(cfg->isValid()); EXPECT_EQ(expandMemTemplate(*cfg), "LIMIT=2998272 SWAP=2998272"); } /** - * When 'swapLimit' < 'memLimit', processSwapLimit must return false because - * the kernel rejects memory.memsw.limit_in_bytes < memory.limit_in_bytes. + * When 'swapLimit' < 'memLimit', processSwapLimit must reject the value + * and parsing must fail (kernel requires memsw >= mem). */ TEST_F(DobbySpecConfigTest, SwapLimit_LessThanMemLimit_Fails) { - // Construct a config so mSpec is populated with memLimit=5996544 - DobbySpecConfig cfg(mUtils, mSettings, mBundle, kSpecSwapBelowLimit); - - // Directly invoke processSwapLimit with a value that is below memLimit - ctemplate::TemplateDictionary dict("test_below"); - Json::Value badSwap(2998272); // 2998272 < 5996544 - EXPECT_FALSE(cfg.processSwapLimit(badSwap, &dict)); + auto cfg = makeConfig(kSpecSwapBelowLimit); + EXPECT_FALSE(cfg->isValid()); } /** * When 'swapLimit' is not an integer, processSwapLimit must return false. + * Verify by calling the private method directly. */ TEST_F(DobbySpecConfigTest, SwapLimit_NonIntegral_Fails) { - DobbySpecConfig cfg(mUtils, mSettings, mBundle, kSpecMemOnly); + auto cfg = makeConfig(kSpecMemOnly); ctemplate::TemplateDictionary dict("test_nonint"); Json::Value badSwap("not-a-number"); - EXPECT_FALSE(cfg.processSwapLimit(badSwap, &dict)); + EXPECT_FALSE(cfg->processSwapLimit(badSwap, &dict)); } diff --git a/tests/L1_testing/tests/DobbySpecConfigTest/DobbyTemplateStubs.cpp b/tests/L1_testing/tests/DobbySpecConfigTest/DobbyTemplateStubs.cpp new file mode 100644 index 000000000..a5eb6fefa --- /dev/null +++ b/tests/L1_testing/tests/DobbySpecConfigTest/DobbyTemplateStubs.cpp @@ -0,0 +1,47 @@ +/* +* If not stated otherwise in this file or this component's LICENSE file the +* following copyright and licenses apply: +* +* Copyright 2024 Sky UK +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +// Minimal stubs for DobbyTemplate static methods used by DobbySpecConfig. +// +// DobbySpecConfig::parseSpec calls DobbyTemplate::applyAt at step 8 to write +// the rendered config.json. In unit tests we pass dirFd=-1 so we only need +// this to return false (which is correct behaviour for an invalid fd) – the +// preceding steps have already populated the ctemplate dictionary, which is +// what the tests actually verify. + +#include "DobbyTemplate.h" + +// Required by DobbyTemplateMock.cpp's delegate pattern +DobbyTemplateImpl* DobbyTemplate::impl = nullptr; + +bool DobbyTemplate::applyAt(int /* dirFd */, + const std::string& /* fileName */, + const ctemplate::TemplateDictionaryInterface* /* dict */, + bool /* prettyPrint */) +{ + // Stub: always returns false so parseSpec returns false, but the + // dictionary is fully populated by the time this is called. + return false; +} + +std::string DobbyTemplate::apply(const ctemplate::TemplateDictionaryInterface* /* dict */, + bool /* prettyPrint */) +{ + return ""; +} From abd7d8603f3328edcfec996e70decdac9c66baa9 Mon Sep 17 00:00:00 2001 From: Gurdal Oruklu Date: Tue, 31 Mar 2026 15:44:25 -0700 Subject: [PATCH 4/8] Remove stale DobbyTemplateStubs.cpp left over from debugging The final DobbySpecConfigTest build uses the real DobbyTemplate.cpp directly (with bundle/lib/include ordered before the mocks dir so the real header is found first). The stub file was created during an intermediate debugging step and was never needed in the final approach, but it was accidentally left on disk. file(GLOB TESTS *.cpp) in the CMakeLists was picking it up, causing a CI build failure because DobbyTemplateImpl is not defined when the real DobbyTemplate.h is in scope. --- .../DobbyTemplateStubs.cpp | 47 ------------------- 1 file changed, 47 deletions(-) delete mode 100644 tests/L1_testing/tests/DobbySpecConfigTest/DobbyTemplateStubs.cpp diff --git a/tests/L1_testing/tests/DobbySpecConfigTest/DobbyTemplateStubs.cpp b/tests/L1_testing/tests/DobbySpecConfigTest/DobbyTemplateStubs.cpp deleted file mode 100644 index a5eb6fefa..000000000 --- a/tests/L1_testing/tests/DobbySpecConfigTest/DobbyTemplateStubs.cpp +++ /dev/null @@ -1,47 +0,0 @@ -/* -* If not stated otherwise in this file or this component's LICENSE file the -* following copyright and licenses apply: -* -* Copyright 2024 Sky UK -* -* Licensed under the Apache License, Version 2.0 (the "License"); -* you may not use this file except in compliance with the License. -* You may obtain a copy of the License at -* -* http://www.apache.org/licenses/LICENSE-2.0 -* -* Unless required by applicable law or agreed to in writing, software -* distributed under the License is distributed on an "AS IS" BASIS, -* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -* See the License for the specific language governing permissions and -* limitations under the License. -*/ - -// Minimal stubs for DobbyTemplate static methods used by DobbySpecConfig. -// -// DobbySpecConfig::parseSpec calls DobbyTemplate::applyAt at step 8 to write -// the rendered config.json. In unit tests we pass dirFd=-1 so we only need -// this to return false (which is correct behaviour for an invalid fd) – the -// preceding steps have already populated the ctemplate dictionary, which is -// what the tests actually verify. - -#include "DobbyTemplate.h" - -// Required by DobbyTemplateMock.cpp's delegate pattern -DobbyTemplateImpl* DobbyTemplate::impl = nullptr; - -bool DobbyTemplate::applyAt(int /* dirFd */, - const std::string& /* fileName */, - const ctemplate::TemplateDictionaryInterface* /* dict */, - bool /* prettyPrint */) -{ - // Stub: always returns false so parseSpec returns false, but the - // dictionary is fully populated by the time this is called. - return false; -} - -std::string DobbyTemplate::apply(const ctemplate::TemplateDictionaryInterface* /* dict */, - bool /* prettyPrint */) -{ - return ""; -} From 30d0b057bf82a78ade916d4e3d2d4c7965edc32e Mon Sep 17 00:00:00 2001 From: Gurdal Oruklu Date: Tue, 31 Mar 2026 15:48:07 -0700 Subject: [PATCH 5/8] Revert "Optimize iptables setup and cache external interfaces" This reverts commit 90c154beb9b6b929be7a793089343910e3c08a8f. --- .../Networking/include/NetworkingPlugin.h | 5 --- .../Networking/source/NetworkingPlugin.cpp | 43 +++---------------- 2 files changed, 7 insertions(+), 41 deletions(-) diff --git a/rdkPlugins/Networking/include/NetworkingPlugin.h b/rdkPlugins/Networking/include/NetworkingPlugin.h index eaa3affe8..973c10f76 100644 --- a/rdkPlugins/Networking/include/NetworkingPlugin.h +++ b/rdkPlugins/Networking/include/NetworkingPlugin.h @@ -74,11 +74,6 @@ class NetworkingPlugin : public RdkPluginBase std::shared_ptr mHelper; std::shared_ptr mNetfilter; - - // Cached result of GetAvailableExternalInterfaces() - the settings file and - // available interface set do not change during a container lifetime, so we - // compute this once and reuse it across all hook calls. - mutable std::vector mExtIfaces; }; #endif // !defined(NETWORKINGPLUGIN_H) diff --git a/rdkPlugins/Networking/source/NetworkingPlugin.cpp b/rdkPlugins/Networking/source/NetworkingPlugin.cpp index 7cc92215a..650e3aeb8 100644 --- a/rdkPlugins/Networking/source/NetworkingPlugin.cpp +++ b/rdkPlugins/Networking/source/NetworkingPlugin.cpp @@ -31,7 +31,6 @@ #include #include #include -#include REGISTER_RDK_PLUGIN(NetworkingPlugin); @@ -166,11 +165,9 @@ bool NetworkingPlugin::createRuntime() return false; } - // Check whether another container already set up the bridge device. - // A direct sysfs access() is far cheaper than opening a Netlink socket - // (which allocates a socket fd + opens /sys/class/net) just to query - // a single interface name. - bool bridgeExists = (access("/sys/class/net/" BRIDGE_NAME, F_OK) == 0); + // check if another container has already initialised the bridge device for us + std::shared_ptr netlink = std::make_shared(); + bool bridgeExists = netlink->ifaceExists(std::string(BRIDGE_NAME)); if (!bridgeExists) { @@ -308,15 +305,8 @@ bool NetworkingPlugin::createRuntime() } } - // Apply IPv4 and IPv6 iptables rules concurrently. The two rule sets - // are completely independent - running them in parallel overlaps the - // fork/exec cost of iptables-save and iptables-restore for both - // address families, halving the iptables portion of startup latency. - auto ipv4Future = std::async(std::launch::async, - [this]() { return mNetfilter->applyRules(AF_INET); }); - const bool ipv6Success = mNetfilter->applyRules(AF_INET6); - const bool ipv4Success = ipv4Future.get(); - if (!ipv4Success || !ipv6Success) + // apply iptables changes + if (!mNetfilter->applyRules(AF_INET) || !mNetfilter->applyRules(AF_INET6)) { AI_LOG_ERROR_EXIT("failed to apply iptables rules"); return false; @@ -521,13 +511,8 @@ bool NetworkingPlugin::postHalt() } } - // Apply IPv4 and IPv6 iptables rules concurrently (same rationale as - // createRuntime - the two address families are fully independent). - auto ipv4HaltFuture = std::async(std::launch::async, - [this]() { return mNetfilter->applyRules(AF_INET); }); - const bool ipv6HaltSuccess = mNetfilter->applyRules(AF_INET6); - const bool ipv4HaltSuccess = ipv4HaltFuture.get(); - if (!ipv4HaltSuccess || !ipv6HaltSuccess) + // apply iptables changes + if (!mNetfilter->applyRules(AF_INET) || !mNetfilter->applyRules(AF_INET6)) { AI_LOG_ERROR_EXIT("failed to apply iptables rules"); return false; @@ -573,15 +558,6 @@ std::vector NetworkingPlugin::getDependencies() const */ std::vector NetworkingPlugin::GetAvailableExternalInterfaces() const { - // Return the cached result if already computed. Reading and YAJL- - // parsing /etc/dobby.json plus scanning /sys/class/net on every call - // adds measurable latency; the result is stable for the lifetime of - // the container. - if (!mExtIfaces.empty()) - { - return mExtIfaces; - } - std::vector externalIfaces = GetExternalInterfacesFromSettings(); if (externalIfaces.size() == 0) @@ -630,11 +606,6 @@ std::vector NetworkingPlugin::GetAvailableExternalInterfaces() cons { AI_LOG_ERROR("None of the external interfaces defined in the settings file are available"); } - else - { - // Cache the result so subsequent hook calls pay no I/O cost. - mExtIfaces = externalIfaces; - } return externalIfaces; } From 85d83e8e91d7acd2e31dbca02706f828ff1fb89e Mon Sep 17 00:00:00 2001 From: Gurdal Oruklu Date: Tue, 31 Mar 2026 16:17:56 -0700 Subject: [PATCH 6/8] Fix PicklingError in basic_sanity_tests: use threading instead of multiprocessing multiprocessing.Process requires pickling its target function. Python cannot pickle nested (local) functions, so on environments where the multiprocessing start method is 'spawn' (or where pickling is otherwise attempted) this raises: _pickle.PicklingError: Can't pickle local object read_asynchronous..wait_for_string The wait_for_string helper reads lines from a subprocess pipe until a string is found -- this is I/O-bound work that needs no separate process. Replace multiprocessing.Process with threading.Thread(daemon=True): - No pickling required; the thread closure captures proc and string_to_find directly from the enclosing scope. - daemon=True ensures the thread is silently reaped if it is still blocked on readline() when the timeout expires. - The is_alive() / join(timeout) logic is unchanged. --- .../test_runner/basic_sanity_tests.py | 26 +++++++------------ 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/tests/L2_testing/test_runner/basic_sanity_tests.py b/tests/L2_testing/test_runner/basic_sanity_tests.py index f98e60fe9..946a4152e 100755 --- a/tests/L2_testing/test_runner/basic_sanity_tests.py +++ b/tests/L2_testing/test_runner/basic_sanity_tests.py @@ -19,7 +19,7 @@ from subprocess import check_output import subprocess from time import sleep -import multiprocessing +import threading from os.path import basename tests = ( @@ -99,19 +99,11 @@ def read_asynchronous(proc, string_to_find, timeout): """ - # as this function should not be used outside asynchronous read, it is moved inside it - def wait_for_string(proc, string_to_find): - """Waits indefinitely until string is found in process. Must be run with timeout multiprocess. - - Parameters: - proc (process): process in which we want to read - string_to_find (string): what we want to find in process - - Returns: - None: Returns nothing if found, never ends if not found - - """ - + # Use a daemon thread so the nested target function is never pickled. + # multiprocessing.Process requires pickling the target, which fails + # for nested functions on spawn-based environments (newer Python/OS). + # Threading shares the address space so no serialisation is needed. + def wait_for_string(): while True: # notice that all data are in stderr not in stdout, this is DobbyDaemon design output = proc.stderr.readline() @@ -120,14 +112,14 @@ def wait_for_string(proc, string_to_find): return found = False - reader = multiprocessing.Process(target=wait_for_string, args=(proc, string_to_find), kwargs={}) - test_utils.print_log("Starting multithread read", test_utils.Severity.debug) + reader = threading.Thread(target=wait_for_string, daemon=True) + test_utils.print_log("Starting async read thread", test_utils.Severity.debug) reader.start() reader.join(timeout) # if thread still running if reader.is_alive(): test_utils.print_log("Reader still exists, closing", test_utils.Severity.debug) - reader.terminate() + # daemon=True: thread is abandoned and reaped when the process exits test_utils.print_log("Not found string \"%s\"" % string_to_find, test_utils.Severity.error) else: found = True From 8cbd8764dc1bb4343903e3d99d009eb932972c74 Mon Sep 17 00:00:00 2001 From: Gurdal Oruklu Date: Tue, 31 Mar 2026 16:24:20 -0700 Subject: [PATCH 7/8] Register swap_limit_tests in L2 test runner swap_limit_tests.py was added in an earlier commit but was never wired into runner.py, so the swap-limit integration checks were silently skipped in CI. Add the import and register the module in supported_tests so it runs as part of the full L2 suite. --- tests/L2_testing/test_runner/runner.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/L2_testing/test_runner/runner.py b/tests/L2_testing/test_runner/runner.py index 4c9dfc9fc..4e177d38f 100755 --- a/tests/L2_testing/test_runner/runner.py +++ b/tests/L2_testing/test_runner/runner.py @@ -28,6 +28,7 @@ import pid_limit_tests import memcr_tests import annotation_tests +import swap_limit_tests import sys import json @@ -44,7 +45,8 @@ network_tests, gui_containers, pid_limit_tests, - memcr_tests] + memcr_tests, + swap_limit_tests] def run_all_tests(): success_count = 0 From 6fa35561739258713a1b7b6e435ba660a14caeeb Mon Sep 17 00:00:00 2001 From: ks734 Date: Tue, 28 Apr 2026 06:54:02 +0000 Subject: [PATCH 8/8] RDKEMW-16534: Fix copilot comments --- README.md | 2 +- bundle/lib/source/DobbySpecConfig.cpp | 40 +++++++++++++++---- .../tests/DobbySpecConfigTest/CMakeLists.txt | 5 ++- .../DobbySpecConfigTest.cpp | 7 ++++ .../dobby_specs/swap_limit_default.json | 28 +++++++++++++ .../test_runner/basic_sanity_tests.py | 3 ++ .../test_runner/swap_limit_tests.py | 2 +- 7 files changed, 77 insertions(+), 10 deletions(-) create mode 100644 tests/L2_testing/dobby_specs/swap_limit_default.json diff --git a/README.md b/README.md index ed1fadf6d..0d373e06b 100644 --- a/README.md +++ b/README.md @@ -135,7 +135,7 @@ The table below lists the supported top-level fields. Fields marked **mandatory* | `version` | string | Yes | Spec version. Currently `"1.0"` or `"1.1"`. | | `args` | array | Yes | Command and arguments to run inside the container. | | `user` | object | Yes | `uid` and `gid` the container process runs as. | -| `memLimit` | integer | Yes | Memory limit in bytes (`memory.limit_in_bytes`). Must be ≥ 256 KiB. | +| `memLimit` | integer | Yes | Memory limit in bytes (`memory.limit_in_bytes`). Values below 256 KiB are accepted but will only generate a warning and may not be effective. | | `swapLimit` | integer | No | Swap+memory limit in bytes (`memory.memsw.limit_in_bytes`). Must be ≥ `memLimit`. Defaults to `memLimit` (no extra swap). | | `env` | array | No | Environment variables in `"KEY=VALUE"` format. | | `cwd` | string | No | Working directory inside the container. | diff --git a/bundle/lib/source/DobbySpecConfig.cpp b/bundle/lib/source/DobbySpecConfig.cpp index 8bbfa4232..101a9ea87 100644 --- a/bundle/lib/source/DobbySpecConfig.cpp +++ b/bundle/lib/source/DobbySpecConfig.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -1317,26 +1318,51 @@ bool DobbySpecConfig::processMemLimit(const Json::Value& value, * @return true if correctly processed the value, otherwise false. */ bool DobbySpecConfig::processSwapLimit(const Json::Value& value, - ctemplate::TemplateDictionary* dictionary) + ctemplate::TemplateDictionary* dictionary) { + // Reject non-numeric values up front. if (!value.isIntegral()) { AI_LOG_ERROR("invalid swapLimit field"); return false; } - unsigned memSwap = value.asUInt(); + // JsonCpp's isIntegral() returns true for negative integers too. A + // negative value would silently wrap to a huge unsigned number and bypass + // the swap >= memLimit guard, so we must check sign before casting. + const int64_t memSwapSigned = value.asInt64(); + if (memSwapSigned < 0) + { + AI_LOG_ERROR("swapLimit must be non-negative, got %" PRId64, memSwapSigned); + return false; + } - // the kernel requires memory.memsw.limit_in_bytes >= memory.limit_in_bytes + // The kernel requires memory.memsw.limit_in_bytes >= memory.limit_in_bytes. const Json::Value& memLimitVal = mSpec["memLimit"]; - if (memLimitVal.isIntegral() && (memSwap < memLimitVal.asUInt())) + if (memLimitVal.isIntegral()) + { + const int64_t memLimitSigned = memLimitVal.asInt64(); + if (memLimitSigned < 0) + { + AI_LOG_ERROR("memLimit is negative; cannot validate swapLimit"); + return false; + } + if (memSwapSigned < memLimitSigned) + { + AI_LOG_ERROR("swapLimit (%" PRId64 ") must be >= memLimit (%" PRId64 ")", + memSwapSigned, memLimitSigned); + return false; + } + } + + if (memSwapSigned > static_cast(UINT_MAX)) { - AI_LOG_ERROR("swapLimit (%u) must be >= memLimit (%u)", - memSwap, memLimitVal.asUInt()); + AI_LOG_ERROR("swapLimit (%" PRId64 ") exceeds maximum supported value for template field (%u)", + memSwapSigned, UINT_MAX); return false; } - dictionary->SetIntValue(MEM_SWAP, memSwap); + dictionary->SetIntValue(MEM_SWAP, static_cast(memSwapSigned)); return true; } diff --git a/tests/L1_testing/tests/DobbySpecConfigTest/CMakeLists.txt b/tests/L1_testing/tests/DobbySpecConfigTest/CMakeLists.txt index 5a7b67807..894e3ac53 100644 --- a/tests/L1_testing/tests/DobbySpecConfigTest/CMakeLists.txt +++ b/tests/L1_testing/tests/DobbySpecConfigTest/CMakeLists.txt @@ -28,7 +28,7 @@ include_directories(${GTEST_INCLUDE_DIRS}) # Real sources only – no PIMPL mock wrappers for DobbyBundle or DobbyTemplate. # DobbySettingsMock is the only mock used; it implements IDobbySettings via gmock. -add_library(DobbySpecConfigTestLib SHARED STATIC +add_library(DobbySpecConfigTestLib STATIC ../../../../bundle/lib/source/DobbySpecConfig.cpp ../../../../bundle/lib/source/DobbyTemplate.cpp ../../../../bundle/lib/source/DobbyBundle.cpp @@ -62,6 +62,9 @@ target_include_directories(DobbySpecConfigTestLib ) file(GLOB TESTS *.cpp) +# DobbySpecConfigLinkStubs.cpp is already compiled into DobbySpecConfigTestLib; +# including it again in the test executable would cause duplicate symbol errors. +list(REMOVE_ITEM TESTS "${CMAKE_CURRENT_SOURCE_DIR}/DobbySpecConfigLinkStubs.cpp") add_executable(${PROJECT_NAME} ${TESTS}) diff --git a/tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigTest.cpp b/tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigTest.cpp index ddfa43fea..9cc219793 100644 --- a/tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigTest.cpp +++ b/tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigTest.cpp @@ -23,9 +23,13 @@ #include #include #include +#include +#include +#include #include #include #include +#include // DobbySettingsMock.h pulls in gmock which eventually includes ; // it must be included before #define private public. @@ -36,6 +40,9 @@ #define private public #include "DobbySpecConfig.h" #include "DobbyTemplate.h" +// Undefine immediately so the macro does not leak into gtest/gmock headers +// below, which can cause hard-to-diagnose build failures on some compilers. +#undef private using ::testing::NiceMock; using ::testing::Return; diff --git a/tests/L2_testing/dobby_specs/swap_limit_default.json b/tests/L2_testing/dobby_specs/swap_limit_default.json new file mode 100644 index 000000000..434f00a18 --- /dev/null +++ b/tests/L2_testing/dobby_specs/swap_limit_default.json @@ -0,0 +1,28 @@ +{ + "version": "1.0", + "cwd": "/", + "args": [ + "cat", + "/sys/fs/cgroup/memory/memory.memsw.limit_in_bytes" + ], + "env": [], + "user": { + "uid": 1000, + "gid": 1000 + }, + "console": { + "limit": 65536, + "path": "/tmp/swap_limit_default.log" + }, + "etc": { + "group": [ + "root:x:0:" + ], + "passwd": [ + "root::0:0:root:/:/bin/false" + ] + }, + "memLimit": 2998272, + "network": "nat", + "mounts": [] +} diff --git a/tests/L2_testing/test_runner/basic_sanity_tests.py b/tests/L2_testing/test_runner/basic_sanity_tests.py index 946a4152e..479fdc2a9 100755 --- a/tests/L2_testing/test_runner/basic_sanity_tests.py +++ b/tests/L2_testing/test_runner/basic_sanity_tests.py @@ -107,6 +107,9 @@ def wait_for_string(): while True: # notice that all data are in stderr not in stdout, this is DobbyDaemon design output = proc.stderr.readline() + if not output: + # EOF – subprocess closed its stderr pipe + return if string_to_find in output: test_utils.print_log("Found string \"%s\"" % string_to_find, test_utils.Severity.debug) return diff --git a/tests/L2_testing/test_runner/swap_limit_tests.py b/tests/L2_testing/test_runner/swap_limit_tests.py index 2da7df7fc..c565458e4 100644 --- a/tests/L2_testing/test_runner/swap_limit_tests.py +++ b/tests/L2_testing/test_runner/swap_limit_tests.py @@ -20,7 +20,7 @@ tests = [ test_utils.Test( "Swap limit default", - "ram", + "swap_limit_default", str(2998272), "Starts a container with only memLimit set and verifies that " "memory.memsw.limit_in_bytes equals memLimit (no extra swap)"),