@@ -52,6 +52,8 @@ class ContainerManagerUnittest : public testing::Test {
5252 void TestLoadContainerInfoVersionHandling () const ;
5353 void TestSaveContainerInfoWithVersion () const ;
5454 void TestContainerMatchingConsistency () const ;
55+ void TestcomputeMatchedContainersDiffWithSnapshot () const ;
56+ void TestNullRawContainerInfoHandling () const ;
5557 void runTestFile (const std::string& testFilePath) const ;
5658
5759private:
@@ -1291,6 +1293,275 @@ bool ContainerManagerUnittest::parseContainerFilterConfigFromTestJson(const Json
12911293 return true ;
12921294}
12931295
1296+ void ContainerManagerUnittest::TestcomputeMatchedContainersDiffWithSnapshot () const {
1297+ // This test verifies that computeMatchedContainersDiff uses a snapshot of mContainerMap
1298+ // to avoid race conditions when the map is modified concurrently
1299+ ContainerManager containerManager;
1300+
1301+ // Set up initial containers in mContainerMap
1302+ RawContainerInfo info1;
1303+ info1.mID = " snapshot1" ;
1304+ info1.mLogPath = " /var/lib/docker/containers/snapshot1/logs" ;
1305+ info1.mStatus = " running" ;
1306+ containerManager.mContainerMap [" snapshot1" ] = std::make_shared<RawContainerInfo>(info1);
1307+
1308+ RawContainerInfo info2;
1309+ info2.mID = " snapshot2" ;
1310+ info2.mLogPath = " /var/lib/docker/containers/snapshot2/logs" ;
1311+ info2.mStatus = " running" ;
1312+ containerManager.mContainerMap [" snapshot2" ] = std::make_shared<RawContainerInfo>(info2);
1313+
1314+ RawContainerInfo info3;
1315+ info3.mID = " snapshot3" ;
1316+ info3.mLogPath = " /var/lib/docker/containers/snapshot3/logs" ;
1317+ info3.mStatus = " exited" ;
1318+ containerManager.mContainerMap [" snapshot3" ] = std::make_shared<RawContainerInfo>(info3);
1319+
1320+ // Test 1: Verify that only running containers are added (isStdio = false)
1321+ {
1322+ std::set<std::string> fullList;
1323+ std::unordered_map<std::string, std::shared_ptr<RawContainerInfo>> matchList;
1324+ ContainerFilters filters;
1325+ ContainerDiff diff;
1326+
1327+ containerManager.computeMatchedContainersDiff (fullList, matchList, filters, false , diff);
1328+
1329+ // Should only add running containers to both diff.mAdded and fullList
1330+ EXPECT_EQ (diff.mAdded .size (), 2 );
1331+ EXPECT_EQ (fullList.size (), 2 ); // Only running containers are added to fullList when isStdio=false
1332+
1333+ std::set<std::string> addedIds;
1334+ for (const auto & container : diff.mAdded ) {
1335+ addedIds.insert (container->mID );
1336+ }
1337+ EXPECT_TRUE (addedIds.find (" snapshot1" ) != addedIds.end ());
1338+ EXPECT_TRUE (addedIds.find (" snapshot2" ) != addedIds.end ());
1339+ EXPECT_TRUE (addedIds.find (" snapshot3" ) == addedIds.end ()); // exited container should not be added
1340+
1341+ // Verify fullList only contains running containers
1342+ EXPECT_TRUE (fullList.find (" snapshot1" ) != fullList.end ());
1343+ EXPECT_TRUE (fullList.find (" snapshot2" ) != fullList.end ());
1344+ EXPECT_TRUE (fullList.find (" snapshot3" ) == fullList.end ());
1345+ }
1346+
1347+ // Test 2: Verify that isStdio=true includes non-running containers
1348+ {
1349+ std::set<std::string> fullList;
1350+ std::unordered_map<std::string, std::shared_ptr<RawContainerInfo>> matchList;
1351+ ContainerFilters filters;
1352+ ContainerDiff diff;
1353+
1354+ containerManager.computeMatchedContainersDiff (fullList, matchList, filters, true , diff);
1355+
1356+ // Should add all containers when isStdio=true (including exited)
1357+ EXPECT_EQ (diff.mAdded .size (), 3 );
1358+ EXPECT_EQ (fullList.size (), 3 ); // All containers are added to fullList when isStdio=true
1359+
1360+ std::set<std::string> addedIds;
1361+ for (const auto & container : diff.mAdded ) {
1362+ addedIds.insert (container->mID );
1363+ }
1364+ EXPECT_TRUE (addedIds.find (" snapshot1" ) != addedIds.end ());
1365+ EXPECT_TRUE (addedIds.find (" snapshot2" ) != addedIds.end ());
1366+ EXPECT_TRUE (addedIds.find (" snapshot3" ) != addedIds.end ());
1367+
1368+ // Verify fullList contains all containers
1369+ EXPECT_TRUE (fullList.find (" snapshot1" ) != fullList.end ());
1370+ EXPECT_TRUE (fullList.find (" snapshot2" ) != fullList.end ());
1371+ EXPECT_TRUE (fullList.find (" snapshot3" ) != fullList.end ());
1372+ }
1373+
1374+ // Test 3: Verify snapshot behavior - containers removed from fullList are detected
1375+ {
1376+ std::set<std::string> fullList;
1377+ fullList.insert (" snapshot1" );
1378+ fullList.insert (" removed1" ); // This container exists in fullList but not in mContainerMap
1379+
1380+ std::unordered_map<std::string, std::shared_ptr<RawContainerInfo>> matchList;
1381+ RawContainerInfo removedInfo;
1382+ removedInfo.mID = " removed1" ;
1383+ removedInfo.mLogPath = " /var/lib/docker/containers/removed1/logs" ;
1384+ matchList[" removed1" ] = std::make_shared<RawContainerInfo>(removedInfo);
1385+
1386+ ContainerFilters filters;
1387+ ContainerDiff diff;
1388+
1389+ containerManager.computeMatchedContainersDiff (fullList, matchList, filters, false , diff);
1390+
1391+ // The removed container should be in mRemoved list
1392+ EXPECT_TRUE (std::find (diff.mRemoved .begin (), diff.mRemoved .end (), " removed1" ) != diff.mRemoved .end ());
1393+ // The removed container should no longer be in fullList
1394+ EXPECT_EQ (fullList.count (" removed1" ), 0 );
1395+
1396+ // snapshot1 should still be in fullList
1397+ EXPECT_TRUE (fullList.find (" snapshot1" ) != fullList.end ());
1398+
1399+ // snapshot2 should be added to fullList (running container not previously in fullList)
1400+ EXPECT_TRUE (fullList.find (" snapshot2" ) != fullList.end ());
1401+
1402+ // snapshot3 should NOT be in fullList (exited container with isStdio=false)
1403+ EXPECT_TRUE (fullList.find (" snapshot3" ) == fullList.end ());
1404+
1405+ // Final fullList size should be 2 (snapshot1 and snapshot2)
1406+ EXPECT_EQ (fullList.size (), 2 );
1407+
1408+ // diff.mAdded should contain snapshot2
1409+ EXPECT_EQ (diff.mAdded .size (), 1 );
1410+ if (diff.mAdded .size () > 0 ) {
1411+ EXPECT_EQ (diff.mAdded [0 ]->mID , " snapshot2" );
1412+ }
1413+ }
1414+
1415+ // Test 4: Verify snapshot behavior - modified containers are detected
1416+ {
1417+ std::set<std::string> fullList;
1418+ fullList.insert (" snapshot1" );
1419+
1420+ std::unordered_map<std::string, std::shared_ptr<RawContainerInfo>> matchList;
1421+ RawContainerInfo oldInfo;
1422+ oldInfo.mID = " snapshot1" ;
1423+ oldInfo.mLogPath = " /old/path" ; // Different from current mContainerMap
1424+ matchList[" snapshot1" ] = std::make_shared<RawContainerInfo>(oldInfo);
1425+
1426+ ContainerFilters filters;
1427+ ContainerDiff diff;
1428+
1429+ containerManager.computeMatchedContainersDiff (fullList, matchList, filters, false , diff);
1430+
1431+ // The container should be in mModified list
1432+ EXPECT_EQ (diff.mModified .size (), 1 );
1433+ if (diff.mModified .size () > 0 ) {
1434+ EXPECT_EQ (diff.mModified [0 ]->mID , " snapshot1" );
1435+ EXPECT_EQ (diff.mModified [0 ]->mLogPath , " /var/lib/docker/containers/snapshot1/logs" );
1436+ }
1437+ }
1438+ }
1439+
1440+ void ContainerManagerUnittest::TestNullRawContainerInfoHandling () const {
1441+ // This test verifies that null mRawContainerInfo pointers are handled gracefully
1442+ // as added in the race condition fix
1443+
1444+ // Note: This test is limited because ContainerInfo is typically managed internally
1445+ // and we cannot easily inject null mRawContainerInfo in the current implementation.
1446+ // However, we can test the defensive checks are in place by verifying the code
1447+ // doesn't crash with various edge cases.
1448+
1449+ ContainerManager containerManager;
1450+
1451+ // Test 1: Empty container map should not cause issues
1452+ {
1453+ std::set<std::string> fullList;
1454+ std::unordered_map<std::string, std::shared_ptr<RawContainerInfo>> matchList;
1455+ ContainerFilters filters;
1456+ ContainerDiff diff;
1457+
1458+ // Should not crash with empty map
1459+ containerManager.computeMatchedContainersDiff (fullList, matchList, filters, false , diff);
1460+ EXPECT_EQ (diff.mAdded .size (), 0 );
1461+ EXPECT_EQ (diff.mRemoved .size (), 0 );
1462+ EXPECT_EQ (diff.mModified .size (), 0 );
1463+ }
1464+
1465+ // Test 2: Container with minimal info
1466+ {
1467+ RawContainerInfo minimalInfo;
1468+ minimalInfo.mID = " minimal1" ;
1469+ minimalInfo.mStatus = " running" ;
1470+ containerManager.mContainerMap [" minimal1" ] = std::make_shared<RawContainerInfo>(minimalInfo);
1471+
1472+ std::set<std::string> fullList;
1473+ std::unordered_map<std::string, std::shared_ptr<RawContainerInfo>> matchList;
1474+ ContainerFilters filters;
1475+ ContainerDiff diff;
1476+
1477+ containerManager.computeMatchedContainersDiff (fullList, matchList, filters, false , diff);
1478+ EXPECT_EQ (diff.mAdded .size (), 1 );
1479+ if (diff.mAdded .size () > 0 ) {
1480+ EXPECT_EQ (diff.mAdded [0 ]->mID , " minimal1" );
1481+ }
1482+ }
1483+
1484+ // Test 3: Multiple containers with various states
1485+ {
1486+ containerManager.mContainerMap .clear ();
1487+
1488+ RawContainerInfo running1;
1489+ running1.mID = " running1" ;
1490+ running1.mStatus = " running" ;
1491+ running1.mLogPath = " /var/log/running1" ;
1492+ containerManager.mContainerMap [" running1" ] = std::make_shared<RawContainerInfo>(running1);
1493+
1494+ RawContainerInfo exited1;
1495+ exited1.mID = " exited1" ;
1496+ exited1.mStatus = " exited" ;
1497+ exited1.mLogPath = " /var/log/exited1" ;
1498+ containerManager.mContainerMap [" exited1" ] = std::make_shared<RawContainerInfo>(exited1);
1499+
1500+ RawContainerInfo created1;
1501+ created1.mID = " created1" ;
1502+ created1.mStatus = " created" ;
1503+ created1.mLogPath = " /var/log/created1" ;
1504+ containerManager.mContainerMap [" created1" ] = std::make_shared<RawContainerInfo>(created1);
1505+
1506+ std::set<std::string> fullList;
1507+ std::unordered_map<std::string, std::shared_ptr<RawContainerInfo>> matchList;
1508+ ContainerFilters filters;
1509+ ContainerDiff diff;
1510+
1511+ // With isStdio=false, only running containers should be added
1512+ containerManager.computeMatchedContainersDiff (fullList, matchList, filters, false , diff);
1513+ EXPECT_EQ (diff.mAdded .size (), 1 );
1514+ EXPECT_EQ (fullList.size (), 1 ); // Only running container in fullList
1515+ if (diff.mAdded .size () > 0 ) {
1516+ EXPECT_EQ (diff.mAdded [0 ]->mID , " running1" );
1517+ }
1518+
1519+ // With isStdio=true, all containers should be added
1520+ fullList.clear ();
1521+ matchList.clear ();
1522+ ContainerDiff diff2;
1523+ containerManager.computeMatchedContainersDiff (fullList, matchList, filters, true , diff2);
1524+ EXPECT_EQ (diff2.mAdded .size (), 3 );
1525+ EXPECT_EQ (fullList.size (), 3 ); // All containers in fullList when isStdio=true
1526+ }
1527+
1528+ // Test 4: Verify thread-safe snapshot usage
1529+ {
1530+ containerManager.mContainerMap .clear ();
1531+
1532+ // Add some containers
1533+ for (int i = 0 ; i < 10 ; i++) {
1534+ RawContainerInfo info;
1535+ info.mID = " container" + std::to_string (i);
1536+ info.mStatus = (i % 2 == 0 ) ? " running" : " exited" ;
1537+ info.mLogPath = " /var/log/container" + std::to_string (i);
1538+ containerManager.mContainerMap [info.mID ] = std::make_shared<RawContainerInfo>(info);
1539+ }
1540+
1541+ std::set<std::string> fullList;
1542+ std::unordered_map<std::string, std::shared_ptr<RawContainerInfo>> matchList;
1543+ ContainerFilters filters;
1544+ ContainerDiff diff;
1545+
1546+ // This should use a snapshot and not be affected by concurrent modifications
1547+ containerManager.computeMatchedContainersDiff (fullList, matchList, filters, false , diff);
1548+
1549+ // Only running containers (even indices: 0, 2, 4, 6, 8) should be added
1550+ EXPECT_EQ (diff.mAdded .size (), 5 );
1551+ EXPECT_EQ (fullList.size (), 5 ); // Only running containers are added to fullList when isStdio=false
1552+
1553+ // Verify that only running containers are in fullList
1554+ for (int i = 0 ; i < 10 ; i++) {
1555+ std::string containerId = " container" + std::to_string (i);
1556+ if (i % 2 == 0 ) {
1557+ EXPECT_TRUE (fullList.find (containerId) != fullList.end ());
1558+ } else {
1559+ EXPECT_TRUE (fullList.find (containerId) == fullList.end ());
1560+ }
1561+ }
1562+ }
1563+ }
1564+
12941565void ContainerManagerUnittest::parseLabelFilters (const Json::Value& filtersJson, MatchCriteriaFilter& filter) const {
12951566 // Clear existing filters to ensure clean state
12961567 filter.mIncludeFields .mFieldsMap .clear ();
@@ -1352,6 +1623,8 @@ UNIT_TEST_CASE(ContainerManagerUnittest, TestLoadContainerInfoFromContainersForm
13521623UNIT_TEST_CASE (ContainerManagerUnittest, TestLoadContainerInfoVersionHandling)
13531624UNIT_TEST_CASE (ContainerManagerUnittest, TestSaveContainerInfoWithVersion)
13541625UNIT_TEST_CASE (ContainerManagerUnittest, TestContainerMatchingConsistency)
1626+ UNIT_TEST_CASE (ContainerManagerUnittest, TestcomputeMatchedContainersDiffWithSnapshot)
1627+ UNIT_TEST_CASE (ContainerManagerUnittest, TestNullRawContainerInfoHandling)
13551628
13561629} // namespace logtail
13571630
0 commit comments