diff --git a/tests/framework/defs.py b/tests/framework/defs.py index 3d16325f71c..b075ecc0deb 100644 --- a/tests/framework/defs.py +++ b/tests/framework/defs.py @@ -48,3 +48,7 @@ # Absolute path to the test results folder TEST_RESULTS_DIR = FC_WORKSPACE_DIR / "test_results" + +# Name of the file that stores firecracker's PID when launched by jailer with +# `--new-pid-ns`. +FC_PID_FILE_NAME = "firecracker.pid" diff --git a/tests/framework/microvm.py b/tests/framework/microvm.py index b76422ca8d7..fdb8f9155b6 100644 --- a/tests/framework/microvm.py +++ b/tests/framework/microvm.py @@ -28,7 +28,8 @@ import host_tools.network as net_tools import framework.utils as utils -from framework.defs import MICROVM_KERNEL_RELPATH, MICROVM_FSFILES_RELPATH +from framework.defs import MICROVM_KERNEL_RELPATH, MICROVM_FSFILES_RELPATH, \ + FC_PID_FILE_NAME from framework.http import Session from framework.jailer import JailerContext from framework.resources import Actions, Balloon, BootSource, Drive, \ @@ -174,6 +175,15 @@ def kill(self): utils.run_cmd( 'kill -9 {} || true'.format(self.screen_pid)) + # Check if Firecracker was launched by the jailer in a new pid ns. + fc_pid_in_new_ns = self.pid_in_new_ns + + if fc_pid_in_new_ns: + # We need to explicitly kill the Firecracker pid, since it's + # different from the jailer pid that was previously killed. + utils.run_cmd(f'kill -9 {fc_pid_in_new_ns}', + ignore_return_code=True) + if self._memory_monitor and self._memory_monitor.is_alive(): self._memory_monitor.signal_stop() self._memory_monitor.join(timeout=1) @@ -289,6 +299,22 @@ def memory_monitor(self, monitor): """Set the memory monitor.""" self._memory_monitor = monitor + @property + def pid_in_new_ns(self): + """Get the pid of the Firecracker process in the new namespace. + + Returns None if Firecracker was not launched in a new pid ns. + """ + fc_pid = None + + pid_file_path = f"{self.jailer.chroot_path()}/{FC_PID_FILE_NAME}" + if os.path.exists(pid_file_path): + # Read the PID stored inside the file. + with open(pid_file_path) as file: + fc_pid = int(file.readline()) + + return fc_pid + def flush_metrics(self, metrics_fifo): """Flush the microvm metrics. diff --git a/tests/integration_tests/security/test_jail.py b/tests/integration_tests/security/test_jail.py index ccd3a97c838..05410d80d85 100644 --- a/tests/integration_tests/security/test_jail.py +++ b/tests/integration_tests/security/test_jail.py @@ -10,15 +10,13 @@ # These are the permissions that all files/dirs inside the jailer have. REG_PERMS = stat.S_IRUSR | stat.S_IWUSR | \ - stat.S_IXUSR | stat.S_IRGRP | stat.S_IXGRP | \ - stat.S_IROTH | stat.S_IXOTH + stat.S_IXUSR | stat.S_IRGRP | stat.S_IXGRP | \ + stat.S_IROTH | stat.S_IXOTH DIR_STATS = stat.S_IFDIR | stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR FILE_STATS = stat.S_IFREG | REG_PERMS SOCK_STATS = stat.S_IFSOCK | REG_PERMS # These are the stats of the devices created by tha jailer. CHAR_STATS = stat.S_IFCHR | stat.S_IRUSR | stat.S_IWUSR -# Name of the file that stores firecracker's PID. -PID_FILE_NAME = "firecracker.pid" def check_stats(filepath, stats, uid, gid): @@ -204,14 +202,8 @@ def test_new_pid_namespace(test_microvm_with_ssh): test_microvm.spawn() # Check that the PID file exists. - pid_file_path = "{}/{}".format(test_microvm.jailer.chroot_path(), - PID_FILE_NAME) - assert os.path.exists(pid_file_path) - - # Read the PID stored inside the file. - with open(pid_file_path) as file: - fc_pid = int(file.readline()) - file.close() + fc_pid = test_microvm.pid_in_new_ns + assert fc_pid is not None # Validate the PID. stdout = subprocess.check_output("pidof firecracker", shell=True) @@ -221,9 +213,9 @@ def test_new_pid_namespace(test_microvm_with_ssh): # Firecracker process is a member of. nstgid_cmd = "cat /proc/{}/status | grep NStgid".format(fc_pid) nstgid_list = subprocess.check_output( - nstgid_cmd, - shell=True - ).decode('utf-8').strip().split("\t")[1:] + nstgid_cmd, + shell=True + ).decode('utf-8').strip().split("\t")[1:] # Check that Firecracker's PID namespace is nested. `NStgid` should # report two values and the last one should be 1, because Firecracker