Skip to content

Commit

Permalink
Merge pull request #601 from golemfactory/blue/ensure-run-command-lea…
Browse files Browse the repository at this point in the history
…ves-no-process

ensure `run_command_on_host` doesn't leave lingering processes
  • Loading branch information
shadeofblue authored Dec 20, 2022
2 parents 2a1d9d2 + 94cd5c3 commit b19cf77
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 51 deletions.
47 changes: 0 additions & 47 deletions goth/default-assets/docker/docker-compose-hybrid-net.yml

This file was deleted.

4 changes: 2 additions & 2 deletions goth/default-assets/docker/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ services:

router:
image: yagna-goth
entrypoint: /usr/bin/ya-sb-router
command: ["-l", "tcp://0.0.0.0:7477"]
entrypoint: /usr/bin/ya-relay-server
command: ["-a", "udp://0.0.0.0:7477", "--ip-checker-port" , "7476"]

proxy-nginx:
# A service that runs `nginx` and routes API calls to
Expand Down
2 changes: 1 addition & 1 deletion goth/default-assets/goth-config-testing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

docker-compose:
# Path to compose file to be used, relative to `docker-dir`
compose-file: "docker-compose-hybrid-net.yml"
compose-file: "docker-compose.yml"
docker-dir: "docker/"

build-environment:
Expand Down
13 changes: 12 additions & 1 deletion goth/runner/probe/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import logging
import os
from pathlib import Path
import shlex
import signal
import traceback
from typing import (
AsyncIterator,
Expand Down Expand Up @@ -389,7 +391,7 @@ async def run_command_on_host(

cmd_task = asyncio.create_task(
process.run_command(
command.split(),
shlex.split(command),
cmd_env,
log_level=logging.INFO,
cmd_logger=cmd_logger,
Expand All @@ -411,6 +413,15 @@ async def run_command_on_host(

finally:
await cmd_monitor.stop()

# ensure the process is killed before we leave
proc = await process_monitor.get_process()
try:
proc.send_signal(signal.SIGKILL)
await proc.wait()
except ProcessLookupError:
pass

for assertion in cmd_monitor.failed:
raise TemporalAssertionError(assertion.name)

Expand Down
15 changes: 15 additions & 0 deletions test/unit/runner/probe/test_run_command_on_host.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import asyncio
import os
import pytest
import signal
from unittest.mock import MagicMock

from goth.address import YAGNA_BUS_URL, YAGNA_REST_URL, YAGNA_REST_PORT
Expand Down Expand Up @@ -76,3 +77,17 @@ async def test_run_command_on_host(mock_probe):
):

await monitor.wait_for_pattern(".*eChO", timeout=10)


@pytest.mark.asyncio
async def test_run_command_on_host_exception_leaves_no_process(mock_probe):
"""Test if the started command doesn't linger on exception."""
with pytest.raises(RuntimeError):
async with mock_probe.run_command_on_host(
'python -c "import time; time.sleep(10)"',
) as (_task, monitor, process_monitor):
proc: asyncio.subprocess.Process = await process_monitor.get_process()
raise RuntimeError("intentional")

with pytest.raises(ProcessLookupError):
proc.send_signal(signal.SIGKILL)

0 comments on commit b19cf77

Please sign in to comment.