From 4679e1b983f1003d1524b66f3a13f32f6d06046d Mon Sep 17 00:00:00 2001 From: Sudan Landge Date: Thu, 8 Feb 2024 17:10:14 +0000 Subject: [PATCH 1/2] chore: test to check if killing microvm works as expected Change: Add a test to make sure there is no change in behaviour in killing firecracker_pid. Reason: In our CI we pass `--new-pid-ns` along with `--daemonize` and so firecracker pid can be read from the file `firecracker.pid` present in the jailers root directory. However, if there is a change in behaviour that leads to this file not having the actual firecracker pid then it could cause a regression. Signed-off-by: Sudan Landge --- tests/framework/microvm.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/framework/microvm.py b/tests/framework/microvm.py index b6ebb2bd4bd..686a2098f1e 100644 --- a/tests/framework/microvm.py +++ b/tests/framework/microvm.py @@ -255,6 +255,20 @@ def kill(self): LOG.exception("Process not found: %d", self.firecracker_pid) except FileNotFoundError: LOG.exception("PID file not found") + + # if microvm was spawned then check if it gets killed + if self._spawned: + # it is observed that we need to wait some time before + # checking if the process is killed. + time.sleep(1) + # filter ps results for the jailer's unique id + rc, stdout, stderr = utils.run_cmd( + f"ps aux | grep {self.jailer.jailer_id}" + ) + # make sure firecracker was killed + assert ( + rc == 0 and stderr == "" and stdout.find("firecracker") == -1 + ), f"Firecracker pid {self.firecracker_pid} was not killed as expected" else: # Killing screen will send SIGHUP to underlying Firecracker. # Needed to avoid false positives in case kill() is called again. From 81b541e1b6549a61d104c5e65ce6f7d7ce7bea1d Mon Sep 17 00:00:00 2001 From: Sudan Landge Date: Thu, 8 Feb 2024 17:53:27 +0000 Subject: [PATCH 2/2] doc: update known limitation of jailer da92bf6d4afa678565dcf9df2e4e00d2fa273d96 commit changed the behaviour of the pid returned by the jailer so, add a known limitation explaining that and suggest a workaround to get the Firecracker pid. Signed-off-by: Sudan Landge --- docs/jailer.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/jailer.md b/docs/jailer.md index 97c8c0b104a..2ebb64b0fbc 100644 --- a/docs/jailer.md +++ b/docs/jailer.md @@ -277,6 +277,14 @@ Note: default value for `` is `/run/firecracker.socket`. associated with `--daemonize` runs towards the end, instead of the very beginning. We are working on adding better logging capabilities. +### Known limitations + +- When passing the --daemonize option to Firecracker without the --new-ns-pid + option, the Firecracker process will have a different pid than the Jailer + process. The suggested workaround to get Firecracker process's pid in this + case is using `--new-pid-ns` flag and read Firecracker's pid from the + `firecracker.pid` file present in the jailer's root directory. + ## Caveats - If all the cgroup controllers are bunched up on a single mount point using the