Skip to content

Commit b197409

Browse files
cm-iwatazulinx86
authored andcommitted
test(jailer): enable multi-threaded test
Previously, all tests shared same temporary file/directory, causing concurrency conflicts when running tests in multi-threaded. Resolved test concurrency issues by incorporating random strings into file/directory names. Link: #4412 Signed-off-by: Tomoya Iwata <[email protected]>
1 parent 39a0fae commit b197409

File tree

4 files changed

+134
-91
lines changed

4 files changed

+134
-91
lines changed

src/jailer/src/cgroup.rs

Lines changed: 59 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,6 @@ use regex::Regex;
1313

1414
use crate::{readln_special, writeln_special, JailerError};
1515

16-
const PROC_MOUNTS: &str = if cfg!(test) {
17-
"/tmp/firecracker/test/jailer/proc/mounts"
18-
} else {
19-
"/proc/mounts"
20-
};
21-
2216
// Holds information on a cgroup mount point discovered on the system
2317
#[derive(Debug)]
2418
struct CgroupMountPoint {
@@ -38,15 +32,15 @@ impl CgroupHierarchies {
3832
// It will discover cgroup mount points and hierarchies configured
3933
// on the system and cache the info required to create cgroups later
4034
// within this hierarchies
41-
fn new(ver: u8) -> Result<Self, JailerError> {
35+
fn new(ver: u8, proc_mounts_path: &str) -> Result<Self, JailerError> {
4236
let mut h = CgroupHierarchies {
4337
hierarchies: HashMap::new(),
4438
mount_points: Vec::new(),
4539
};
4640

4741
// search PROC_MOUNTS for cgroup mount points
48-
let f = File::open(PROC_MOUNTS)
49-
.map_err(|err| JailerError::FileOpen(PathBuf::from(PROC_MOUNTS), err))?;
42+
let f = File::open(proc_mounts_path)
43+
.map_err(|err| JailerError::FileOpen(PathBuf::from(proc_mounts_path), err))?;
5044

5145
// Regex courtesy of Filippo.
5246
// This will match on each line from /proc/mounts for both v1 and v2 mount points.
@@ -66,7 +60,7 @@ impl CgroupHierarchies {
6660
).map_err(JailerError::RegEx)?;
6761

6862
for l in BufReader::new(f).lines() {
69-
let l = l.map_err(|err| JailerError::ReadLine(PathBuf::from(PROC_MOUNTS), err))?;
63+
let l = l.map_err(|err| JailerError::ReadLine(PathBuf::from(proc_mounts_path), err))?;
7064
if let Some(capture) = re.captures(&l) {
7165
if ver == 2 && capture["ver"].len() == 1 {
7266
// Found the cgroupv2 unified mountpoint; with cgroupsv2 there is only one
@@ -146,9 +140,9 @@ pub struct CgroupConfigurationBuilder {
146140
impl CgroupConfigurationBuilder {
147141
// Creates the builder object
148142
// It will initialize the CgroupHierarchy cache.
149-
pub fn new(ver: u8) -> Result<Self, JailerError> {
143+
pub fn new(ver: u8, proc_mounts_path: &str) -> Result<Self, JailerError> {
150144
Ok(CgroupConfigurationBuilder {
151-
hierarchies: CgroupHierarchies::new(ver)?,
145+
hierarchies: CgroupHierarchies::new(ver, proc_mounts_path)?,
152146
cgroup_conf: match ver {
153147
1 => Ok(CgroupConfiguration::V1(HashMap::new())),
154148
2 => Ok(CgroupConfiguration::V2(HashMap::new())),
@@ -510,20 +504,19 @@ pub mod test_util {
510504
use std::io::Write;
511505
use std::path::{Path, PathBuf};
512506

513-
use super::PROC_MOUNTS;
507+
use utils::rand;
514508

515509
#[derive(Debug)]
516510
pub struct MockCgroupFs {
517511
mounts_file: File,
512+
pub proc_mounts_path: String,
513+
pub sys_cgroups_path: String,
518514
}
519515

520516
// Helper object that simulates the layout of the cgroup file system
521517
// This can be used for testing regardless of the availability of a particular
522518
// version of cgroups on the system
523519
impl MockCgroupFs {
524-
const MOCK_PROCDIR: &'static str = "/tmp/firecracker/test/jailer/proc";
525-
pub const MOCK_SYS_CGROUPS_DIR: &'static str = "/tmp/firecracker/test/jailer/sys_cgroup";
526-
527520
pub fn create_file_with_contents<P: AsRef<Path> + Debug>(
528521
filename: P,
529522
contents: &str,
@@ -540,16 +533,28 @@ pub mod test_util {
540533
}
541534

542535
pub fn new() -> std::result::Result<MockCgroupFs, std::io::Error> {
536+
let mock_jailer_dir = format!(
537+
"/tmp/firecracker/test/{}/jailer",
538+
rand::rand_alphanumerics(4).into_string().unwrap()
539+
);
540+
let mock_proc_mounts = format!("{}/{}", mock_jailer_dir, "proc/mounts",);
541+
let mock_sys_cgroups = format!("{}/{}", mock_jailer_dir, "sys_cgroup",);
542+
543+
let mock_proc_dir = Path::new(&mock_proc_mounts).parent().unwrap();
544+
543545
// create a mock /proc/mounts file in a temporary directory
544-
fs::create_dir_all(Self::MOCK_PROCDIR)?;
546+
fs::create_dir_all(mock_proc_dir)?;
545547
let file = OpenOptions::new()
546548
.read(true)
547549
.write(true)
548550
.create(true)
549551
.truncate(true)
550-
.open(PROC_MOUNTS)?;
551-
552-
Ok(MockCgroupFs { mounts_file: file })
552+
.open(mock_proc_mounts.clone())?;
553+
Ok(MockCgroupFs {
554+
mounts_file: file,
555+
proc_mounts_path: mock_proc_mounts,
556+
sys_cgroups_path: mock_sys_cgroups,
557+
})
553558
}
554559

555560
// Populate the mocked proc/mounts file with cgroupv2 entries
@@ -558,9 +563,9 @@ pub mod test_util {
558563
writeln!(
559564
self.mounts_file,
560565
"cgroupv2 {}/unified cgroup2 rw,nosuid,nodev,noexec,relatime,nsdelegate 0 0",
561-
Self::MOCK_SYS_CGROUPS_DIR
566+
self.sys_cgroups_path,
562567
)?;
563-
let cg_unified_path = PathBuf::from(format!("{}/unified", Self::MOCK_SYS_CGROUPS_DIR));
568+
let cg_unified_path = PathBuf::from(format!("{}/unified", self.sys_cgroups_path));
564569
fs::create_dir_all(&cg_unified_path)?;
565570
Self::create_file_with_contents(
566571
cg_unified_path.join("cgroup.controllers"),
@@ -584,9 +589,7 @@ pub mod test_util {
584589
writeln!(
585590
self.mounts_file,
586591
"cgroup {}/{} cgroup rw,nosuid,nodev,noexec,relatime,{} 0 0",
587-
Self::MOCK_SYS_CGROUPS_DIR,
588-
c,
589-
c,
592+
self.sys_cgroups_path, c, c,
590593
)?;
591594
}
592595
Ok(())
@@ -596,8 +599,14 @@ pub mod test_util {
596599
// Cleanup created files when object goes out of scope
597600
impl Drop for MockCgroupFs {
598601
fn drop(&mut self) {
599-
let _ = fs::remove_file(PROC_MOUNTS);
600-
let _ = fs::remove_dir_all("/tmp/firecracker/test");
602+
let tmp_dir = Path::new(self.proc_mounts_path.as_str())
603+
.parent()
604+
.unwrap()
605+
.parent()
606+
.unwrap()
607+
.parent()
608+
.unwrap();
609+
let _ = fs::remove_dir_all(tmp_dir);
601610
}
602611
}
603612
}
@@ -629,51 +638,54 @@ mod tests {
629638

630639
#[test]
631640
fn test_cgroup_conf_builder_invalid_version() {
632-
let builder = CgroupConfigurationBuilder::new(0);
641+
let mock_cgroups = MockCgroupFs::new().unwrap();
642+
let builder = CgroupConfigurationBuilder::new(0, mock_cgroups.proc_mounts_path.as_str());
633643
builder.unwrap_err();
634644
}
635645

636646
#[test]
637647
fn test_cgroup_conf_builder_no_mounts() {
638-
let builder = CgroupConfigurationBuilder::new(1);
648+
let mock_cgroups = MockCgroupFs::new().unwrap();
649+
let builder = CgroupConfigurationBuilder::new(1, mock_cgroups.proc_mounts_path.as_str());
639650
builder.unwrap_err();
640651
}
641652

642653
#[test]
643654
fn test_cgroup_conf_builder_v1() {
644655
let mut mock_cgroups = MockCgroupFs::new().unwrap();
645656
mock_cgroups.add_v1_mounts().unwrap();
646-
let builder = CgroupConfigurationBuilder::new(1);
657+
let builder = CgroupConfigurationBuilder::new(1, mock_cgroups.proc_mounts_path.as_str());
647658
builder.unwrap();
648659
}
649660

650661
#[test]
651662
fn test_cgroup_conf_builder_v2() {
652663
let mut mock_cgroups = MockCgroupFs::new().unwrap();
653664
mock_cgroups.add_v2_mounts().unwrap();
654-
let builder = CgroupConfigurationBuilder::new(2);
665+
let builder = CgroupConfigurationBuilder::new(2, mock_cgroups.proc_mounts_path.as_str());
655666
builder.unwrap();
656667
}
657668

658669
#[test]
659670
fn test_cgroup_conf_builder_v2_with_v1_mounts() {
660671
let mut mock_cgroups = MockCgroupFs::new().unwrap();
661672
mock_cgroups.add_v1_mounts().unwrap();
662-
let builder = CgroupConfigurationBuilder::new(2);
673+
let builder = CgroupConfigurationBuilder::new(2, mock_cgroups.proc_mounts_path.as_str());
663674
builder.unwrap_err();
664675
}
665676

666677
#[test]
667678
fn test_cgroup_conf_builder_v2_no_mounts() {
668-
let builder = CgroupConfigurationBuilder::new(2);
679+
let mock_cgroups = MockCgroupFs::new().unwrap();
680+
let builder = CgroupConfigurationBuilder::new(2, mock_cgroups.proc_mounts_path.as_str());
669681
builder.unwrap_err();
670682
}
671683

672684
#[test]
673685
fn test_cgroup_conf_builder_v1_with_v2_mounts() {
674686
let mut mock_cgroups = MockCgroupFs::new().unwrap();
675687
mock_cgroups.add_v2_mounts().unwrap();
676-
let builder = CgroupConfigurationBuilder::new(1);
688+
let builder = CgroupConfigurationBuilder::new(1, mock_cgroups.proc_mounts_path.as_str());
677689
builder.unwrap_err();
678690
}
679691

@@ -684,7 +696,9 @@ mod tests {
684696
mock_cgroups.add_v2_mounts().unwrap();
685697

686698
for v in &[1, 2] {
687-
let mut builder = CgroupConfigurationBuilder::new(*v).unwrap();
699+
let mut builder =
700+
CgroupConfigurationBuilder::new(*v, mock_cgroups.proc_mounts_path.as_str())
701+
.unwrap();
688702

689703
builder
690704
.add_cgroup_property(
@@ -705,7 +719,9 @@ mod tests {
705719
mock_cgroups.add_v2_mounts().unwrap();
706720

707721
for v in &[1, 2] {
708-
let mut builder = CgroupConfigurationBuilder::new(*v).unwrap();
722+
let mut builder =
723+
CgroupConfigurationBuilder::new(*v, mock_cgroups.proc_mounts_path.as_str())
724+
.unwrap();
709725
builder
710726
.add_cgroup_property(
711727
"invalid.cg".to_string(),
@@ -722,7 +738,8 @@ mod tests {
722738
let mut mock_cgroups = MockCgroupFs::new().unwrap();
723739
mock_cgroups.add_v1_mounts().unwrap();
724740

725-
let mut builder = CgroupConfigurationBuilder::new(1).unwrap();
741+
let mut builder =
742+
CgroupConfigurationBuilder::new(1, mock_cgroups.proc_mounts_path.as_str()).unwrap();
726743
builder
727744
.add_cgroup_property(
728745
"cpuset.mems".to_string(),
@@ -733,7 +750,7 @@ mod tests {
733750
.unwrap();
734751
let cg_conf = builder.build();
735752

736-
let cg_root = PathBuf::from(format!("{}/cpuset", MockCgroupFs::MOCK_SYS_CGROUPS_DIR));
753+
let cg_root = PathBuf::from(format!("{}/cpuset", mock_cgroups.sys_cgroups_path));
737754

738755
// with real cgroups these files are created automatically
739756
// since the mock will not do it automatically, we create it here
@@ -756,10 +773,11 @@ mod tests {
756773
fn test_cgroup_conf_v2_write_value() {
757774
let mut mock_cgroups = MockCgroupFs::new().unwrap();
758775
mock_cgroups.add_v2_mounts().unwrap();
759-
let builder = CgroupConfigurationBuilder::new(2);
776+
let builder = CgroupConfigurationBuilder::new(2, mock_cgroups.proc_mounts_path.as_str());
760777
builder.unwrap();
761778

762-
let mut builder = CgroupConfigurationBuilder::new(2).unwrap();
779+
let mut builder =
780+
CgroupConfigurationBuilder::new(2, mock_cgroups.proc_mounts_path.as_str()).unwrap();
763781
builder
764782
.add_cgroup_property(
765783
"cpuset.mems".to_string(),
@@ -769,7 +787,7 @@ mod tests {
769787
)
770788
.unwrap();
771789

772-
let cg_root = PathBuf::from(format!("{}/unified", MockCgroupFs::MOCK_SYS_CGROUPS_DIR));
790+
let cg_root = PathBuf::from(format!("{}/unified", mock_cgroups.sys_cgroups_path));
773791

774792
assert_eq!(builder.get_v2_hierarchy_path().unwrap(), &cg_root);
775793

0 commit comments

Comments
 (0)