From 066a6e91366b6107a18dc7e45c3f7bc4280d5f54 Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Fri, 14 Feb 2025 13:26:56 +0000 Subject: [PATCH 1/3] test: catch correct exception in memory.py The access to the `firecracker_pid` property causes a read to the vm's pid file. This means that in the scenario where the VM is already dead by the time the monitor starts running, we won't get a `NoSuchProcess` error, we potentially get a `FileNotFoundError` (if the jail is already cleaned up). Thus, to avoid spurious failures, also catch `FileNotFoundError`. We leave the catch for `NoSuchProcess` error in case a VM exits between reading the pid file and the call to the`psutils.Process` constructor (race condition). Signed-off-by: Patrick Roy --- tests/host_tools/memory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/host_tools/memory.py b/tests/host_tools/memory.py index 690cda38701..93380a9321d 100644 --- a/tests/host_tools/memory.py +++ b/tests/host_tools/memory.py @@ -62,7 +62,7 @@ def run(self): guest_mem_bytes = self._vm.mem_size_bytes try: ps = psutil.Process(self._vm.firecracker_pid) - except psutil.NoSuchProcess: + except (psutil.NoSuchProcess, FileNotFoundError): return while not self._should_stop: try: From c8b908130017b0d122a2fb6478b6214e8a72e0e4 Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Fri, 14 Feb 2025 13:28:56 +0000 Subject: [PATCH 2/3] devtool: fix permissions on .python/ after cmd_sandbox Running the sandbox command leaves the .ipython folder as owned by root. Fix this up after exiting so that its can be accesses un-privileged after docker exits (I have a git pre-commit hook that tries to stash all unstaged changes, and it always chokes on this folder). Signed-off-by: Patrick Roy --- tools/devtool | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/devtool b/tools/devtool index 1e53cc6c6bb..7bf8d4ad6fb 100755 --- a/tools/devtool +++ b/tools/devtool @@ -228,7 +228,7 @@ cmd_fix_perms() { run_devctr \ --workdir "$CTR_FC_ROOT_DIR" \ -- \ - chown -f -R "$(id -u):$(id -g)" "$CTR_FC_BUILD_DIR" "$CTR_TEST_RESULTS_DIR" "$CTR_CI_ARTIFACTS_PATH" + chown -f -R "$(id -u):$(id -g)" "$CTR_FC_BUILD_DIR" "$CTR_TEST_RESULTS_DIR" "$CTR_CI_ARTIFACTS_PATH" $@ } # Builds the development container from its Dockerfile. @@ -890,6 +890,7 @@ cmd_sandbox() { cmd_build --release ensure_ci_artifacts cmd_sh "tmux new env PYTEST_ADDOPTS=--pdbcls=IPython.terminal.debugger:TerminalPdb PYTHONPATH=tests IPYTHONDIR=\$PWD/.ipython ipython -i ./tools/sandbox.py $@" + cmd_fix_perms ".ipython" } cmd_sandbox_native() { From 9f74ef77d99a37fefbefd5aa2286cffe44c24144 Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Fri, 14 Feb 2025 13:53:07 +0000 Subject: [PATCH 3/3] test: stop printing host dmesg on test failure Because we're storing dmesg in a local variable, if an exception occurs while killing a microvm, python will end up printing the entirety of the host dmesg output to stderr, because it dumps local variables as part of the backtrace. This is fine in CI because instances are shortlived, but when running tests locally, it takes my terminal multiple minutes to catch up with the amount of text that ends up being printed. Fix this by not storing dmesg in a local variable anymore, and instead inline it to its only use-site. Signed-off-by: Patrick Roy --- tests/conftest.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 552ba25c4ef..e3aea3a8dfa 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -336,11 +336,12 @@ def microvm_factory(request, record_property, results_dir, netns_factory): # if the test failed, save important files from the root of the uVM into `test_results` for troubleshooting report = request.node.stash[PHASE_REPORT_KEY] if "call" in report and report["call"].failed: - dmesg = utils.run_cmd(["dmesg", "-dPx"]) for uvm in uvm_factory.vms: uvm_data = results_dir / uvm.id uvm_data.mkdir() - uvm_data.joinpath("host-dmesg.log").write_text(dmesg.stdout) + uvm_data.joinpath("host-dmesg.log").write_text( + utils.run_cmd(["dmesg", "-dPx"]).stdout + ) shutil.copy(f"/firecracker/build/img/{platform.machine()}/id_rsa", uvm_data) uvm_root = Path(uvm.chroot())