diff --git a/scripts/machine_health.py b/scripts/machine_health.py index a3f4bca2b4d..d5a7176e922 100644 --- a/scripts/machine_health.py +++ b/scripts/machine_health.py @@ -196,8 +196,24 @@ def _write_semaphore_with_new_venv(machine_name: str, reason: str) -> bool: if not os.path.exists(python_exe): # Windows layout (not expected here, but be safe) python_exe = os.path.join(venv_dir, "Scripts", "python.exe") - subprocess.run([python_exe, "-m", "pip", "install", "-q", "-U", "pip"], check=True, timeout=300) - subprocess.run([python_exe, "-m", "pip", "install", "-q", *_AZURE_PACKAGES], check=True, timeout=600) + # Helix sets PYTHONPATH to its own scripts, which include an incompatible partial ``azure`` + # package. Let the temporary interpreter use only its virtual environment packages. + venv_environment = dict(os.environ) + venv_environment.pop("PYTHONHOME", None) + venv_environment.pop("PYTHONPATH", None) + + subprocess.run( + [python_exe, "-m", "pip", "install", "-q", "-U", "pip"], + check=True, + env=venv_environment, + timeout=300, + ) + subprocess.run( + [python_exe, "-m", "pip", "install", "-q", *_AZURE_PACKAGES], + check=True, + env=venv_environment, + timeout=600, + ) # Re-run this module inside the venv to do just the upload, now that azure is available. result = subprocess.run( @@ -208,7 +224,7 @@ def _write_semaphore_with_new_venv(machine_name: str, reason: str) -> bool: "--machine", machine_name, "--reason", reason, ], - env=dict(os.environ), + env=venv_environment, timeout=300, ) return result.returncode == 0 diff --git a/scripts/run_performance_job.py b/scripts/run_performance_job.py index f3b8393240f..67f614a312d 100644 --- a/scripts/run_performance_job.py +++ b/scripts/run_performance_job.py @@ -387,8 +387,8 @@ def get_pre_commands( # If prereqs failed, check for a non-transient corrupted dpkg/apt state. If found, # machine_health takes the machine out of Helix rotation and notifies the perf team. # Runs directly from the correlation payload since the venv/PYTHONPATH are not set up - # yet at this point. Never allowed to change the (already failing) exit code. - 'if [ "x$PERF_PREREQS_INSTALL_FAILED" = "x1" ]; then python3 "$HELIX_CORRELATION_PAYLOAD/performance/scripts/machine_health.py" check || true; fi' + # yet at this point. Preserve the prerequisite failure after the best-effort check. + 'if [ "x$PERF_PREREQS_INSTALL_FAILED" = "x1" ]; then python3 "$HELIX_CORRELATION_PAYLOAD/performance/scripts/machine_health.py" check || true; exit 1; fi' ] # Set MONO_ENV_OPTIONS with for Mono Interpreter runs diff --git a/scripts/tests/test_machine_health.py b/scripts/tests/test_machine_health.py new file mode 100644 index 00000000000..de195560479 --- /dev/null +++ b/scripts/tests/test_machine_health.py @@ -0,0 +1,30 @@ +import os +from unittest.mock import Mock + +from scripts import machine_health + + +def test_fallback_venv_does_not_inherit_python_path(monkeypatch, tmp_path): + venv_dir = tmp_path / "machine-health-venv" + python_exe = venv_dir / "bin" / "python" + python_exe.parent.mkdir(parents=True) + python_exe.touch() + + monkeypatch.setenv("PYTHONHOME", "/helix/python") + monkeypatch.setenv("PYTHONPATH", "/etc/helix/scripts") + monkeypatch.setattr(machine_health.tempfile, "mkdtemp", lambda **_: str(venv_dir)) + monkeypatch.setattr("venv.create", Mock()) + monkeypatch.setattr(machine_health.shutil, "rmtree", Mock()) + + completed = Mock(returncode=0) + run = Mock(return_value=completed) + monkeypatch.setattr(machine_health.subprocess, "run", run) + + assert machine_health._write_semaphore_with_new_venv("PERFVIPER001", "reason") + + assert run.call_count == 3 + for call in run.call_args_list: + environment = call.kwargs["env"] + assert "PYTHONHOME" not in environment + assert "PYTHONPATH" not in environment + assert environment["PATH"] == os.environ["PATH"] diff --git a/scripts/tests/test_run_performance_job.py b/scripts/tests/test_run_performance_job.py index b2d942de79f..aa789318d0e 100644 --- a/scripts/tests/test_run_performance_job.py +++ b/scripts/tests/test_run_performance_job.py @@ -59,3 +59,20 @@ def test_generated_prerequisites_do_not_poll_dpkg_lock(): prerequisites = "\n".join(pre_commands) assert "fuser" not in prerequisites assert "Waiting for dpkg" not in prerequisites + + +def test_linux_prerequisite_failure_remains_failed_after_health_check(): + pre_commands = get_pre_commands( + os_group="linux", + os_distro="ubuntu", + internal=True, + runtime_type="coreclr", + codegen_type="jit", + build_config="Release", + v8_version="12.0.0", + ) + + health_check = next( + command for command in pre_commands if "machine_health.py" in command + ) + assert health_check.endswith("check || true; exit 1; fi")