Skip to content

Commit 4106863

Browse files
committed
refactor(test): Clarify logic in Microvm.kill()
Explicitly call out that the whole "grep for jailer_id" dance is a regression test. Signed-off-by: Patrick Roy <[email protected]>
1 parent 8b089ce commit 4106863

File tree

1 file changed

+8
-4
lines changed

1 file changed

+8
-4
lines changed

tests/framework/microvm.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -297,17 +297,21 @@ def kill(self):
297297

298298
# if microvm was spawned then check if it gets killed
299299
if self._spawned:
300-
# it is observed that we need to wait some time before
301-
# checking if the process is killed.
302-
time.sleep(1)
300+
# Wait until the Firecracker process is actually dead
301+
utils.wait_process_termination(self.firecracker_pid)
302+
303+
# The following logic guards us against the case where `firecracker_pid` for some
304+
# reason is the wrong PID, e.g. this is a regression test for
305+
# https://github.com/firecracker-microvm/firecracker/pull/4442/commits/d63eb7a65ffaaae0409d15ed55d99ecbd29bc572
306+
303307
# filter ps results for the jailer's unique id
304308
_, stdout, stderr = utils.check_output(
305309
f"ps aux | grep {self.jailer.jailer_id}"
306310
)
307311
# make sure firecracker was killed
308312
assert (
309313
stderr == "" and "firecracker" not in stdout
310-
), f"Firecracker pid {self.firecracker_pid} was not killed as expected"
314+
), f"Firecracker reported its pid {self.firecracker_pid}, which was killed, but there still exist processes using the supposedly dead Firecracker's jailer_id: {stdout}"
311315

312316
# Mark the microVM as not spawned, so we avoid trying to kill twice.
313317
self._spawned = False

0 commit comments

Comments
 (0)