Skip to content

Commit 43fb717

Browse files
committed
test: Add retries to get firecracker PID in new ns
When `pid_in_new_ns` is referenced before the pid file is created by jailer with `--new-pid-ns` flag, it returns `None` and some jailer integration tests fail. To avoid this supurious failures, adds reties to `pid_in_new_ns`. Signed-off-by: Takahiro Itazuri <[email protected]>
1 parent 0bd543a commit 43fb717

File tree

2 files changed

+10
-14
lines changed

2 files changed

+10
-14
lines changed

tests/framework/microvm.py

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -214,12 +214,10 @@ def kill(self):
214214
self._validate_api_response_times()
215215

216216
# Check if Firecracker was launched by the jailer in a new pid ns.
217-
fc_pid_in_new_ns = self.pid_in_new_ns
218-
219-
if fc_pid_in_new_ns:
217+
if self.jailer.new_pid_ns:
220218
# We need to explicitly kill the Firecracker pid, since it's
221219
# different from the jailer pid that was previously killed.
222-
utils.run_cmd(f"kill -9 {fc_pid_in_new_ns}", ignore_return_code=True)
220+
utils.run_cmd(f"kill -9 {self.pid_in_new_ns}", ignore_return_code=True)
223221

224222
if self.memory_monitor:
225223
if self.memory_monitor.is_alive():
@@ -340,20 +338,18 @@ def started(self):
340338
return json.loads(self.desc_inst.get().content)["started"]
341339

342340
@property
341+
@retry(delay=0.1, tries=5)
343342
def pid_in_new_ns(self):
344343
"""Get the pid of the Firecracker process in the new namespace.
345344
346-
Returns None if Firecracker was not launched in a new pid ns.
345+
Reads the pid from a file created by jailer with `--new-pid-ns` flag.
347346
"""
348-
fc_pid = None
349-
350-
pid_file_path = f"{self.jailer.chroot_path()}/{FC_PID_FILE_NAME}"
351-
if os.path.exists(pid_file_path):
352-
# Read the PID stored inside the file.
353-
with open(pid_file_path, encoding="utf-8") as file:
354-
fc_pid = int(file.readline())
347+
# Check if the pid file exists.
348+
pid_file_path = Path(f"{self.jailer.chroot_path()}/{FC_PID_FILE_NAME}")
349+
assert pid_file_path.exists()
355350

356-
return fc_pid
351+
# Read the PID stored inside the file.
352+
return int(pid_file_path.read_text(encoding="ascii"))
357353

358354
def flush_metrics(self, metrics_fifo):
359355
"""Flush the microvm metrics.

tests/integration_tests/security/test_jail.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,7 @@ def test_new_pid_ns_resource_limits(test_microvm_with_api):
625625

626626
# Get Firecracker's PID.
627627
fc_pid = test_microvm.pid_in_new_ns
628+
628629
# Check limit values were correctly set.
629630
check_limits(fc_pid, NOFILE, FSIZE)
630631

@@ -641,7 +642,6 @@ def test_new_pid_namespace(test_microvm_with_api):
641642

642643
# Check that the PID file exists.
643644
fc_pid = test_microvm.pid_in_new_ns
644-
assert fc_pid is not None
645645

646646
# Validate the PID.
647647
stdout = subprocess.check_output("pidof firecracker", shell=True)

0 commit comments

Comments
 (0)