Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 37 additions & 2 deletions avocado/core/status/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,46 @@
import logging
import os

from avocado.core.output import LOG_JOB
from avocado.core.settings import settings
from avocado.core.status.utils import StatusMsgInvalidJSONError
from avocado.utils.network import ports as network_ports

LOG = logging.getLogger(__name__)


def resolve_listen_uri(uri):
"""
Normalize a status server URI that may contain a port range into
a concrete "host:port" endpoint.
"""
if ":" not in uri:
return uri
host, port_spec = uri.rsplit(":", 1)
if "-" not in port_spec:
return uri

start_s, end_s = port_spec.split("-", 1)
start = int(start_s)
end = int(end_s)
if start > end:
raise ValueError(
f"Invalid port range (start > end) in status server URI: {uri}"
)

port = network_ports.find_free_port(
start_port=start,
end_port=end,
address=host,
sequent=True,
)
Comment on lines +32 to +37

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Bind the selected port before publishing it.

find_free_port() returns only a port number. It does not retain a listening socket. Another Avocado process can bind the selected port before StatusServer.create_server() runs. Two concurrent runs can then select the same port, and one status server fails to start.

Bind and retain the listening socket while scanning the range. Propagate the resolved endpoint only after the bind succeeds.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@avocado/core/status/server.py` around lines 28 - 33, Update the status-server
port allocation around find_free_port and StatusServer.create_server to bind and
retain the listening socket while scanning candidate ports, rather than
returning only a number. Propagate the resolved endpoint only after a bind
succeeds, and ensure the retained socket is used by the server so concurrent
processes cannot select the same port.

if port is None:
raise OSError(
f"Could not bind status server to any port in range {start}-{end} on {host}"
)
return f"{host}:{port}"


class StatusServer:
"""Server that listens for status messages and updates a StatusRepo."""

Expand All @@ -20,7 +54,7 @@ def __init__(self, uri, repo):
messages
:type repo: :class:`avocado.core.status.repo.StatusRepo`
"""
self._uri = uri
self._uri = resolve_listen_uri(uri)
self._repo = repo
self._server_task = None

Expand All @@ -31,7 +65,7 @@ def uri(self):
async def create_server(self):
limit = settings.as_dict().get("run.status_server_buffer_size")
if ":" in self._uri:
host, port = self._uri.split(":")
host, port = self._uri.rsplit(":", 1)
port = int(port)
self._server_task = await asyncio.start_server(
self.cb, host=host, port=port, limit=limit
Expand All @@ -40,6 +74,7 @@ async def create_server(self):
self._server_task = await asyncio.start_unix_server(
self.cb, path=self._uri, limit=limit
)
LOG_JOB.info("Status server listening on %s", self._uri)

async def serve_forever(self):
if self._server_task is None:
Expand Down
59 changes: 25 additions & 34 deletions avocado/plugins/runner_nrunner.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@

import asyncio
import multiprocessing
import os
import platform
import random
import tempfile

from avocado.core.dispatcher import SpawnerDispatcher
from avocado.core.exceptions import JobError, JobFailFast
Expand All @@ -31,11 +28,12 @@
from avocado.core.plugin_interfaces import CLI, Init, SuiteRunner
from avocado.core.settings import settings
from avocado.core.status.repo import StatusRepo
from avocado.core.status.server import StatusServer
from avocado.core.status.server import StatusServer, resolve_listen_uri
from avocado.core.task.runtime import RuntimeTaskGraph
from avocado.core.task.statemachine import TaskStateMachine, Worker

DEFAULT_SERVER_URI = "127.0.0.1:8888"
# Default port range so multiple avocado runs can bind without conflict
DEFAULT_SERVER_URI = "127.0.0.1:8888-9000"


class RunnerInit(Init):
Expand All @@ -55,10 +53,9 @@ def initialize(self):
)

help_msg = (
"If the status server should automatically choose "
'a "status_server_listen" and "status_server_uri" '
"configuration. Default is to auto configure a "
"status server."
"If the status server should automatically choose a listen address "
"from the default port range so multiple runs do not conflict. "
"When disabled, use status_server_listen/status_server_uri."
)
settings.register_option(
section=section,
Expand All @@ -69,10 +66,10 @@ def initialize(self):
)

help_msg = (
'URI where status server will listen on. Usually a "HOST:PORT" '
'string. This is only effective if "status_server_auto" is disabled. '
'If "status_server_uri" is not set, the value from "status_server_listen " '
"will be used."
'URI where status server will listen. "HOST:PORT" or "HOST:START-END" '
"port range (default: 127.0.0.1:8888-9000). Only used when "
'"status_server_auto" is disabled. If "status_server_uri" is not set, '
'"status_server_listen" is used.'
)
settings.register_option(
section=section,
Expand All @@ -83,12 +80,10 @@ def initialize(self):
)

help_msg = (
"URI for connecting to the status server, usually "
'a "HOST:PORT" string. Use this if your status server '
"is in another host, or different port. This is only "
'effective if "status_server_auto" is disabled. '
'If "status_server_listen" is not set, the value from "status_server_uri" '
"will be used."
'URI for connecting to the status server: "HOST:PORT" or "HOST:START-END" '
'port range (default: 127.0.0.1:8888-9000). Only used when "status_server_auto" '
'is disabled. If "status_server_listen" is not set, '
'"status_server_uri" is used.'
)
settings.register_option(
section=section,
Expand Down Expand Up @@ -207,19 +202,8 @@ class Runner(SuiteRunner):
name = "nrunner"
description = "nrunner based implementation of job compliant runner"

def __init__(self):
super().__init__()
self.status_server_dir = None

def _determine_status_server(self, test_suite, config_key):
if test_suite.config.get("run.status_server_auto"):
# no UNIX domain sockets on Windows
if platform.system() != "Windows":
if self.status_server_dir is None:
self.status_server_dir = tempfile.TemporaryDirectory(
prefix="avocado_"
)
return os.path.join(self.status_server_dir.name, ".status_server.sock")
"""Return listen/uri config; default is a port range so multiple runs work."""
return test_suite.config.get(config_key)
Comment on lines 205 to 207

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Honor run.status_server_auto when selecting the endpoint.

When run.status_server_auto is True, this method still returns configured values. For example, a custom run.status_server_uri makes RuntimeTaskGraph send status messages to that custom URI while the server resolves run.status_server_listen. This contradicts the option help and can disconnect tasks from the status server.

When automatic mode is enabled, select the default range for both values. After resolution, update both configuration keys to the same concrete endpoint.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@avocado/plugins/runner_nrunner.py` around lines 205 - 207, Update
_determine_status_server to check run.status_server_auto and, when enabled,
ignore configured endpoint values in favor of the default status-server range
for both listen and URI resolution. After resolving the endpoint, write the same
concrete value back to both run.status_server_listen and run.status_server_uri
so RuntimeTaskGraph and the server use the identical endpoint; preserve
configured values when automatic mode is disabled.


def _sync_status_server_urls(self, config):
Expand All @@ -240,10 +224,19 @@ def _sync_status_server_urls(self, config):
def _create_status_server(self, test_suite, job):
self._sync_status_server_urls(test_suite.config)
listen = self._determine_status_server(test_suite, "run.status_server_listen")
try:
resolved_listen = resolve_listen_uri(listen)
except (ValueError, OSError) as exc:
raise JobError(str(exc)) from exc
if resolved_listen != listen:
test_suite.config["run.status_server_listen"] = resolved_listen
server_uri = test_suite.config.get("run.status_server_uri")
if server_uri in (listen, DEFAULT_SERVER_URI):
test_suite.config["run.status_server_uri"] = resolved_listen
# pylint: disable=W0201
self.status_repo = StatusRepo(job.unique_id)
# pylint: disable=W0201
self.status_server = StatusServer(listen, self.status_repo)
self.status_server = StatusServer(resolved_listen, self.status_repo)

async def _update_status(self, job):
message_handler = MessageHandler()
Expand Down Expand Up @@ -400,8 +393,6 @@ def run_suite(self, job, test_suite):
loop.run_until_complete(task)
except asyncio.CancelledError:
pass
if self.status_server_dir is not None:
self.status_server_dir.cleanup()

# Update the overall summary with found test statuses, which will
# determine the Avocado command line exit status
Expand Down
31 changes: 16 additions & 15 deletions man/avocado.rst
Original file line number Diff line number Diff line change
Expand Up @@ -179,23 +179,24 @@ Options for subcommand `run` (`avocado run --help`)::
nrunner specific options:
--shuffle Shuffle the tasks to be executed
--status-server-disable-auto
If the status server should automatically choose a
"status_server_listen" and "status_server_uri"
configuration. Default is to auto configure a status
server.
Disable automatic status server port selection. By
default, a port range (127.0.0.1:8888-9000) is used
so multiple avocado runs can run without port
conflicts. When disabled, use
--status-server-listen/--status-server-uri.
--status-server-listen HOST_PORT
URI where status server will listen on. Usually a
"HOST:PORT" string. This is only effective if
"status_server_auto" is disabled. If
"status_server_uri" is not set, the value from
"status_server_listen " will be used.
URI where status server will listen: "HOST:PORT" or
"HOST:START-END" (default 127.0.0.1:8888-9000). An
available port in the range is chosen. Only used
when --status-server-disable-auto is set. If
"status_server_uri" is not set,
"status_server_listen" is used.
--status-server-uri HOST_PORT
URI for connecting to the status server, usually a
"HOST:PORT" string. Use this if your status server is
in another host, or different port. This is only
effective if "status_server_auto" is disabled. If
"status_server_listen" is not set, the value from
"status_server_uri" will be used.
URI for connecting to the status server: "HOST:PORT"
or "HOST:START-END". Only used when
--status-server-disable-auto is set. If
"status_server_listen" is not set,
"status_server_uri" is used.
--max-parallel-tasks NUMBER_OF_TASKS
Number of maximum number tasks running in parallel.
You can disable parallel execution by setting this to
Expand Down
Loading