Skip to content

Commit 696d736

Browse files
linrunqi08linrunqi08
committed
add ut
1 parent cda43c9 commit 696d736

File tree

1 file changed

+150
-33
lines changed

1 file changed

+150
-33
lines changed

‎core/unittest/container_manager/ContainerManagerUnittest.cpp‎

Lines changed: 150 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,14 @@
1414

1515

1616
#include <algorithm>
17+
#include <atomic>
1718
#include <boost/regex.hpp>
19+
#include <chrono>
1820
#include <fstream>
1921
#include <memory>
2022
#include <set>
2123
#include <string>
24+
#include <thread>
2225
#include <unordered_map>
2326
#include <vector>
2427

@@ -54,6 +57,7 @@ class ContainerManagerUnittest : public testing::Test {
5457
void TestContainerMatchingConsistency() const;
5558
void TestcomputeMatchedContainersDiffWithSnapshot() const;
5659
void TestNullRawContainerInfoHandling() const;
60+
void TestConcurrentContainerMapAccess() const;
5761
void runTestFile(const std::string& testFilePath) const;
5862

5963
private:
@@ -1329,15 +1333,15 @@ void ContainerManagerUnittest::TestcomputeMatchedContainersDiffWithSnapshot() co
13291333
// Should only add running containers to both diff.mAdded and fullList
13301334
EXPECT_EQ(diff.mAdded.size(), 2);
13311335
EXPECT_EQ(fullList.size(), 2); // Only running containers are added to fullList when isStdio=false
1332-
1336+
13331337
std::set<std::string> addedIds;
13341338
for (const auto& container : diff.mAdded) {
13351339
addedIds.insert(container->mID);
13361340
}
13371341
EXPECT_TRUE(addedIds.find("snapshot1") != addedIds.end());
13381342
EXPECT_TRUE(addedIds.find("snapshot2") != addedIds.end());
13391343
EXPECT_TRUE(addedIds.find("snapshot3") == addedIds.end()); // exited container should not be added
1340-
1344+
13411345
// Verify fullList only contains running containers
13421346
EXPECT_TRUE(fullList.find("snapshot1") != fullList.end());
13431347
EXPECT_TRUE(fullList.find("snapshot2") != fullList.end());
@@ -1356,15 +1360,15 @@ void ContainerManagerUnittest::TestcomputeMatchedContainersDiffWithSnapshot() co
13561360
// Should add all containers when isStdio=true (including exited)
13571361
EXPECT_EQ(diff.mAdded.size(), 3);
13581362
EXPECT_EQ(fullList.size(), 3); // All containers are added to fullList when isStdio=true
1359-
1363+
13601364
std::set<std::string> addedIds;
13611365
for (const auto& container : diff.mAdded) {
13621366
addedIds.insert(container->mID);
13631367
}
13641368
EXPECT_TRUE(addedIds.find("snapshot1") != addedIds.end());
13651369
EXPECT_TRUE(addedIds.find("snapshot2") != addedIds.end());
13661370
EXPECT_TRUE(addedIds.find("snapshot3") != addedIds.end());
1367-
1371+
13681372
// Verify fullList contains all containers
13691373
EXPECT_TRUE(fullList.find("snapshot1") != fullList.end());
13701374
EXPECT_TRUE(fullList.find("snapshot2") != fullList.end());
@@ -1376,13 +1380,13 @@ void ContainerManagerUnittest::TestcomputeMatchedContainersDiffWithSnapshot() co
13761380
std::set<std::string> fullList;
13771381
fullList.insert("snapshot1");
13781382
fullList.insert("removed1"); // This container exists in fullList but not in mContainerMap
1379-
1383+
13801384
std::unordered_map<std::string, std::shared_ptr<RawContainerInfo>> matchList;
13811385
RawContainerInfo removedInfo;
13821386
removedInfo.mID = "removed1";
13831387
removedInfo.mLogPath = "/var/lib/docker/containers/removed1/logs";
13841388
matchList["removed1"] = std::make_shared<RawContainerInfo>(removedInfo);
1385-
1389+
13861390
ContainerFilters filters;
13871391
ContainerDiff diff;
13881392

@@ -1392,19 +1396,19 @@ void ContainerManagerUnittest::TestcomputeMatchedContainersDiffWithSnapshot() co
13921396
EXPECT_TRUE(std::find(diff.mRemoved.begin(), diff.mRemoved.end(), "removed1") != diff.mRemoved.end());
13931397
// The removed container should no longer be in fullList
13941398
EXPECT_EQ(fullList.count("removed1"), 0);
1395-
1399+
13961400
// snapshot1 should still be in fullList
13971401
EXPECT_TRUE(fullList.find("snapshot1") != fullList.end());
1398-
1402+
13991403
// snapshot2 should be added to fullList (running container not previously in fullList)
14001404
EXPECT_TRUE(fullList.find("snapshot2") != fullList.end());
1401-
1405+
14021406
// snapshot3 should NOT be in fullList (exited container with isStdio=false)
14031407
EXPECT_TRUE(fullList.find("snapshot3") == fullList.end());
1404-
1408+
14051409
// Final fullList size should be 2 (snapshot1 and snapshot2)
14061410
EXPECT_EQ(fullList.size(), 2);
1407-
1411+
14081412
// diff.mAdded should contain snapshot2
14091413
EXPECT_EQ(diff.mAdded.size(), 1);
14101414
if (diff.mAdded.size() > 0) {
@@ -1416,13 +1420,13 @@ void ContainerManagerUnittest::TestcomputeMatchedContainersDiffWithSnapshot() co
14161420
{
14171421
std::set<std::string> fullList;
14181422
fullList.insert("snapshot1");
1419-
1423+
14201424
std::unordered_map<std::string, std::shared_ptr<RawContainerInfo>> matchList;
14211425
RawContainerInfo oldInfo;
14221426
oldInfo.mID = "snapshot1";
14231427
oldInfo.mLogPath = "/old/path"; // Different from current mContainerMap
14241428
matchList["snapshot1"] = std::make_shared<RawContainerInfo>(oldInfo);
1425-
1429+
14261430
ContainerFilters filters;
14271431
ContainerDiff diff;
14281432

@@ -1440,82 +1444,82 @@ void ContainerManagerUnittest::TestcomputeMatchedContainersDiffWithSnapshot() co
14401444
void ContainerManagerUnittest::TestNullRawContainerInfoHandling() const {
14411445
// This test verifies that null mRawContainerInfo pointers are handled gracefully
14421446
// as added in the race condition fix
1443-
1447+
14441448
// Note: This test is limited because ContainerInfo is typically managed internally
14451449
// and we cannot easily inject null mRawContainerInfo in the current implementation.
14461450
// However, we can test the defensive checks are in place by verifying the code
14471451
// doesn't crash with various edge cases.
1448-
1452+
14491453
ContainerManager containerManager;
1450-
1454+
14511455
// Test 1: Empty container map should not cause issues
14521456
{
14531457
std::set<std::string> fullList;
14541458
std::unordered_map<std::string, std::shared_ptr<RawContainerInfo>> matchList;
14551459
ContainerFilters filters;
14561460
ContainerDiff diff;
1457-
1461+
14581462
// Should not crash with empty map
14591463
containerManager.computeMatchedContainersDiff(fullList, matchList, filters, false, diff);
14601464
EXPECT_EQ(diff.mAdded.size(), 0);
14611465
EXPECT_EQ(diff.mRemoved.size(), 0);
14621466
EXPECT_EQ(diff.mModified.size(), 0);
14631467
}
1464-
1468+
14651469
// Test 2: Container with minimal info
14661470
{
14671471
RawContainerInfo minimalInfo;
14681472
minimalInfo.mID = "minimal1";
14691473
minimalInfo.mStatus = "running";
14701474
containerManager.mContainerMap["minimal1"] = std::make_shared<RawContainerInfo>(minimalInfo);
1471-
1475+
14721476
std::set<std::string> fullList;
14731477
std::unordered_map<std::string, std::shared_ptr<RawContainerInfo>> matchList;
14741478
ContainerFilters filters;
14751479
ContainerDiff diff;
1476-
1480+
14771481
containerManager.computeMatchedContainersDiff(fullList, matchList, filters, false, diff);
14781482
EXPECT_EQ(diff.mAdded.size(), 1);
14791483
if (diff.mAdded.size() > 0) {
14801484
EXPECT_EQ(diff.mAdded[0]->mID, "minimal1");
14811485
}
14821486
}
1483-
1487+
14841488
// Test 3: Multiple containers with various states
14851489
{
14861490
containerManager.mContainerMap.clear();
1487-
1491+
14881492
RawContainerInfo running1;
14891493
running1.mID = "running1";
14901494
running1.mStatus = "running";
14911495
running1.mLogPath = "/var/log/running1";
14921496
containerManager.mContainerMap["running1"] = std::make_shared<RawContainerInfo>(running1);
1493-
1497+
14941498
RawContainerInfo exited1;
14951499
exited1.mID = "exited1";
14961500
exited1.mStatus = "exited";
14971501
exited1.mLogPath = "/var/log/exited1";
14981502
containerManager.mContainerMap["exited1"] = std::make_shared<RawContainerInfo>(exited1);
1499-
1503+
15001504
RawContainerInfo created1;
15011505
created1.mID = "created1";
15021506
created1.mStatus = "created";
15031507
created1.mLogPath = "/var/log/created1";
15041508
containerManager.mContainerMap["created1"] = std::make_shared<RawContainerInfo>(created1);
1505-
1509+
15061510
std::set<std::string> fullList;
15071511
std::unordered_map<std::string, std::shared_ptr<RawContainerInfo>> matchList;
15081512
ContainerFilters filters;
15091513
ContainerDiff diff;
1510-
1514+
15111515
// With isStdio=false, only running containers should be added
15121516
containerManager.computeMatchedContainersDiff(fullList, matchList, filters, false, diff);
15131517
EXPECT_EQ(diff.mAdded.size(), 1);
15141518
EXPECT_EQ(fullList.size(), 1); // Only running container in fullList
15151519
if (diff.mAdded.size() > 0) {
15161520
EXPECT_EQ(diff.mAdded[0]->mID, "running1");
15171521
}
1518-
1522+
15191523
// With isStdio=true, all containers should be added
15201524
fullList.clear();
15211525
matchList.clear();
@@ -1524,11 +1528,11 @@ void ContainerManagerUnittest::TestNullRawContainerInfoHandling() const {
15241528
EXPECT_EQ(diff2.mAdded.size(), 3);
15251529
EXPECT_EQ(fullList.size(), 3); // All containers in fullList when isStdio=true
15261530
}
1527-
1531+
15281532
// Test 4: Verify thread-safe snapshot usage
15291533
{
15301534
containerManager.mContainerMap.clear();
1531-
1535+
15321536
// Add some containers
15331537
for (int i = 0; i < 10; i++) {
15341538
RawContainerInfo info;
@@ -1537,19 +1541,19 @@ void ContainerManagerUnittest::TestNullRawContainerInfoHandling() const {
15371541
info.mLogPath = "/var/log/container" + std::to_string(i);
15381542
containerManager.mContainerMap[info.mID] = std::make_shared<RawContainerInfo>(info);
15391543
}
1540-
1544+
15411545
std::set<std::string> fullList;
15421546
std::unordered_map<std::string, std::shared_ptr<RawContainerInfo>> matchList;
15431547
ContainerFilters filters;
15441548
ContainerDiff diff;
1545-
1549+
15461550
// This should use a snapshot and not be affected by concurrent modifications
15471551
containerManager.computeMatchedContainersDiff(fullList, matchList, filters, false, diff);
1548-
1552+
15491553
// Only running containers (even indices: 0, 2, 4, 6, 8) should be added
15501554
EXPECT_EQ(diff.mAdded.size(), 5);
15511555
EXPECT_EQ(fullList.size(), 5); // Only running containers are added to fullList when isStdio=false
1552-
1556+
15531557
// Verify that only running containers are in fullList
15541558
for (int i = 0; i < 10; i++) {
15551559
std::string containerId = "container" + std::to_string(i);
@@ -1562,6 +1566,118 @@ void ContainerManagerUnittest::TestNullRawContainerInfoHandling() const {
15621566
}
15631567
}
15641568

1569+
void ContainerManagerUnittest::TestConcurrentContainerMapAccess() const {
1570+
// This test simulates concurrent access to mContainerMap to reproduce race conditions
1571+
// that the fix was designed to prevent.
1572+
ContainerManager containerManager;
1573+
1574+
// Initialize with some containers
1575+
for (int i = 0; i < 20; i++) {
1576+
RawContainerInfo info;
1577+
info.mID = "container_" + std::to_string(i);
1578+
info.mStatus = "running";
1579+
info.mLogPath = "/var/log/container_" + std::to_string(i);
1580+
containerManager.mContainerMap[info.mID] = std::make_shared<RawContainerInfo>(info);
1581+
}
1582+
1583+
std::atomic<bool> testRunning{true};
1584+
std::atomic<int> iterationCount{0};
1585+
std::atomic<int> errorCount{0};
1586+
1587+
// Thread 1: Continuously modify mContainerMap
1588+
auto modifyThread = [&]() {
1589+
int counter = 100;
1590+
while (testRunning) {
1591+
try {
1592+
// Add new containers
1593+
RawContainerInfo newInfo;
1594+
newInfo.mID = "dynamic_" + std::to_string(counter);
1595+
newInfo.mStatus = "running";
1596+
newInfo.mLogPath = "/var/log/dynamic_" + std::to_string(counter);
1597+
1598+
{
1599+
std::lock_guard<std::mutex> lock(containerManager.mContainerMapMutex);
1600+
containerManager.mContainerMap[newInfo.mID] = std::make_shared<RawContainerInfo>(newInfo);
1601+
}
1602+
1603+
// Remove old containers
1604+
if (counter > 105) {
1605+
std::string oldId = "dynamic_" + std::to_string(counter - 5);
1606+
std::lock_guard<std::mutex> lock(containerManager.mContainerMapMutex);
1607+
containerManager.mContainerMap.erase(oldId);
1608+
}
1609+
1610+
counter++;
1611+
std::this_thread::sleep_for(std::chrono::microseconds(100));
1612+
} catch (const std::exception& e) {
1613+
errorCount++;
1614+
}
1615+
}
1616+
};
1617+
1618+
// Thread 2: Continuously call computeMatchedContainersDiff (reads mContainerMap)
1619+
auto readThread = [&]() {
1620+
while (testRunning) {
1621+
try {
1622+
std::set<std::string> fullList;
1623+
std::unordered_map<std::string, std::shared_ptr<RawContainerInfo>> matchList;
1624+
ContainerFilters filters;
1625+
ContainerDiff diff;
1626+
1627+
// This should use a snapshot to avoid iterator invalidation
1628+
containerManager.computeMatchedContainersDiff(fullList, matchList, filters, false, diff);
1629+
1630+
iterationCount++;
1631+
std::this_thread::sleep_for(std::chrono::microseconds(100));
1632+
} catch (const std::exception& e) {
1633+
errorCount++;
1634+
}
1635+
}
1636+
};
1637+
1638+
// Thread 3: Another reader thread to increase contention
1639+
auto readThread2 = [&]() {
1640+
while (testRunning) {
1641+
try {
1642+
std::set<std::string> fullList;
1643+
std::unordered_map<std::string, std::shared_ptr<RawContainerInfo>> matchList;
1644+
ContainerFilters filters;
1645+
ContainerDiff diff;
1646+
1647+
containerManager.computeMatchedContainersDiff(fullList, matchList, filters, true, diff);
1648+
1649+
std::this_thread::sleep_for(std::chrono::microseconds(100));
1650+
} catch (const std::exception& e) {
1651+
errorCount++;
1652+
}
1653+
}
1654+
};
1655+
1656+
// Start threads
1657+
std::thread t1(modifyThread);
1658+
std::thread t2(readThread);
1659+
std::thread t3(readThread2);
1660+
1661+
// Let them run for a short time
1662+
std::this_thread::sleep_for(std::chrono::milliseconds(500));
1663+
1664+
// Stop threads
1665+
testRunning = false;
1666+
1667+
t1.join();
1668+
t2.join();
1669+
t3.join();
1670+
1671+
// With the fix (snapshot), there should be no errors
1672+
EXPECT_EQ(errorCount.load(), 0) << "Concurrent access caused " << errorCount.load() << " errors";
1673+
1674+
// Verify we actually ran many iterations
1675+
EXPECT_GT(iterationCount.load(), 100) << "Test should run many iterations to stress test concurrency";
1676+
1677+
std::cout << "Concurrent test completed: " << iterationCount.load() << " iterations with " << errorCount.load()
1678+
<< " errors" << std::endl;
1679+
}
1680+
15651681
void ContainerManagerUnittest::parseLabelFilters(const Json::Value& filtersJson, MatchCriteriaFilter& filter) const {
15661682
// Clear existing filters to ensure clean state
15671683
filter.mIncludeFields.mFieldsMap.clear();
@@ -1625,6 +1741,7 @@ UNIT_TEST_CASE(ContainerManagerUnittest, TestSaveContainerInfoWithVersion)
16251741
UNIT_TEST_CASE(ContainerManagerUnittest, TestContainerMatchingConsistency)
16261742
UNIT_TEST_CASE(ContainerManagerUnittest, TestcomputeMatchedContainersDiffWithSnapshot)
16271743
UNIT_TEST_CASE(ContainerManagerUnittest, TestNullRawContainerInfoHandling)
1744+
UNIT_TEST_CASE(ContainerManagerUnittest, TestConcurrentContainerMapAccess)
16281745

16291746
} // namespace logtail
16301747

0 commit comments

Comments
 (0)