Skip to content

Commit

Permalink
Merge pull request buildbot#7924 from mifo123/max_lines_restriction
Browse files Browse the repository at this point in the history
buildbot_worker: Add `maxLines` parameter to shell command.
  • Loading branch information
p12tic authored Aug 17, 2024
2 parents b18f245 + 65e3fc5 commit 83dd162
Show file tree
Hide file tree
Showing 16 changed files with 96 additions and 2 deletions.
9 changes: 9 additions & 0 deletions master/buildbot/process/buildstep.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ def __init__(self, **kwargs):
self._acquiringLocks = []
self.stopped = False
self.timed_out = False
self.max_lines_reached = False
self.master = None
self.statistics = {}
self.logs = {}
Expand Down Expand Up @@ -405,6 +406,8 @@ def getResultSummary(self):
stepsumm += f' ({statusToString(self.results)})'
if self.timed_out:
stepsumm += " (timed out)"
elif self.max_lines_reached:
stepsumm += " (max lines reached)"

return {'step': stepsumm}

Expand Down Expand Up @@ -832,6 +835,8 @@ def runCommand(self, command):
res = yield command.run(self, self.remote, self.build.builder.name)
if command.remote_failure_reason in ("timeout", "timeout_without_output"):
self.timed_out = True
elif command.remote_failure_reason in ("max_lines_failure",):
self.max_lines_reached = True
finally:
self.cmd = None
return res
Expand Down Expand Up @@ -894,6 +899,7 @@ class ShellMixin:
lazylogfiles = {}
timeout = 1200
maxTime = None
max_lines = None
logEnviron = True
interruptSignal = 'KILL'
sigtermTime = None
Expand All @@ -911,6 +917,7 @@ class ShellMixin:
('lazylogfiles', check_param_bool),
('timeout', check_param_number_none),
('maxTime', check_param_number_none),
('max_lines', check_param_number_none),
('logEnviron', check_param_bool),
('interruptSignal', check_param_str_none),
('sigtermTime', check_param_number_none),
Expand Down Expand Up @@ -1030,6 +1037,8 @@ def getResultSummary(self):
summary += f' ({statusToString(self.results)})'
if self.timed_out:
summary += " (timed out)"
elif self.max_lines_reached:
summary += " (max lines)"
return {'step': summary}
return super().getResultSummary()

Expand Down
2 changes: 2 additions & 0 deletions master/buildbot/process/remotecommand.py
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,7 @@ def __init__(
want_stderr=1,
timeout=20 * 60,
maxTime=None,
max_lines=None,
sigtermTime=None,
logfiles=None,
usePTY=None,
Expand Down Expand Up @@ -491,6 +492,7 @@ def obfuscate(arg):
'logfiles': logfiles,
'timeout': timeout,
'maxTime': maxTime,
'max_lines': max_lines,
'sigtermTime': sigtermTime,
'usePTY': usePTY,
'logEnviron': logEnviron,
Expand Down
1 change: 1 addition & 0 deletions master/buildbot/steps/shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ def __init__(self, **kwargs):
'want_stderr',
'timeout',
'maxTime',
'max_lines',
'sigtermTime',
'logfiles',
'lazylogfiles',
Expand Down
2 changes: 2 additions & 0 deletions master/buildbot/test/steps.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ def __init__(
initial_stdin=None,
timeout=20 * 60,
max_time=None,
max_lines=None,
sigterm_time=None,
logfiles=None,
use_pty=False,
Expand All @@ -292,6 +293,7 @@ def __init__(
'initial_stdin': initial_stdin,
'timeout': timeout,
'maxTime': max_time,
'max_lines': max_lines,
'logfiles': logfiles,
'usePTY': use_pty,
'logEnviron': log_environ,
Expand Down
1 change: 1 addition & 0 deletions master/buildbot/test/unit/process/test_remotecommand.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ def __init__(
want_stderr=1,
timeout=20 * 60,
maxTime=None,
max_lines=None,
sigtermTime=None,
logfiles=None,
usePTY=None,
Expand Down
1 change: 1 addition & 0 deletions master/docs/developer/cls-buildsteps.rst
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,7 @@ This class can only be used in new-style steps.
.. py:attribute:: lazylogfiles
.. py:attribute:: timeout
.. py:attribute:: maxTime
.. py:attribute:: max_lines
.. py:attribute:: logEnviron
.. py:attribute:: interruptSignal
.. py:attribute:: sigtermTime
Expand Down
3 changes: 2 additions & 1 deletion master/docs/developer/cls-remotecommands.rst
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ RemoteCommand

Add data to a logfile other than ``stdio``.

.. py:class:: RemoteShellCommand(workdir, command, env=None, want_stdout=True, want_stderr=True, timeout=20*60, maxTime=None, sigtermTime=None, logfiles={}, usePTY=None, logEnviron=True, collectStdio=False, collectStderr=False, interruptSignal=None, initialStdin=None, decodeRC=None, stdioLogName='stdio')
.. py:class:: RemoteShellCommand(workdir, command, env=None, want_stdout=True, want_stderr=True, timeout=20*60, maxTime=None, max_lines=None, sigtermTime=None, logfiles={}, usePTY=None, logEnviron=True, collectStdio=False, collectStderr=False, interruptSignal=None, initialStdin=None, decodeRC=None, stdioLogName='stdio')
:param workdir: directory in which the command should be executed, relative to the builder's basedir
:param command: shell command to run
Expand All @@ -185,6 +185,7 @@ RemoteCommand
:param want_stderr: If false, then no updates will be sent for stderr
:param timeout: Maximum time without output before the command is killed
:param maxTime: Maximum overall time from the start before the command is killed
:param max_lines: Maximum lines command can produce to stdout, then command is killed.
:param sigtermTime: Try to kill the command with SIGTERM and wait for sigtermTime seconds before firing ``interruptSignal`` or SIGKILL if it's not defined.
If None, SIGTERM will not be fired
:param env: A dictionary of environment variables to augment or replace the existing environment on the worker
Expand Down
8 changes: 8 additions & 0 deletions master/docs/developer/master-worker-msgpack.rst
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,12 @@ Runs a ``shell`` command on the worker.
Even if command is still running and giving the output, ``maxTime`` variable sets the maximum time the command is allowed to be performing.
If ``maxTime`` is set to ``None``, command runs for as long as it needs unless ``timeout`` specifies otherwise.

``max_lines``
Value is an integer and is optional.
If value is not specified, the default is ``None``.
It represents, how many produced lines a worker should wait before killing a process.
If ``max_lines`` is set to ``None``, command runs for as long as it needs unless ``timeout`` or ``maxTime`` specifies otherwise.

``sigtermTime``
Value is an integer and is optional.
If value is not specified, the default is ``None``.
Expand Down Expand Up @@ -829,6 +835,7 @@ Runs a ``shell`` command on the worker.

- ``timeout`` if the command timed out due to time specified by the ``maxTime`` parameter being exceeded.
- ``timeout_without_output`` if the command timed out due to time specified by the ``timeout`` parameter being exceeded.
- ``max_lines_failure`` if the command is killed due to the number of lines specified by the ``max_lines`` parameter being exceeded.

The basic structure of worker ``update`` message is explained in section :ref:`MsgPack_Keys_And_Values_Message`.

Expand Down Expand Up @@ -1159,6 +1166,7 @@ Commands may have their own update names so only common ones are described here.

- ``timeout`` if the command timed out due to time specified by the ``maxTime`` parameter being exceeded.
- ``timeout_without_output`` if the command timed out due to time specified by the ``timeout`` parameter being exceeded.
- ``max_lines_failure`` if the command is killed due to the number of lines specified by the ``max_lines`` parameter being exceeded.

``header``
Value is a string of a header.
Expand Down
5 changes: 5 additions & 0 deletions master/docs/developer/master-worker.rst
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,10 @@ Runs a shell command on the worker. This command takes the following arguments:

Maximum overall time from the start before the command is killed.

``max_lines``

Maximum overall produced lines by the command, then it is killed.

``logfiles``

A dictionary specifying logfiles other than stdio. Keys are the logfile
Expand Down Expand Up @@ -306,6 +310,7 @@ The ``shell`` command sends the following updates:

- ``timeout`` if the command timed out due to time specified by the ``maxTime`` parameter being exceeded.
- ``timeout_without_output`` if the command timed out due to time specified by the ``timeout`` parameter being exceeded.
- ``max_lines_failure`` if the command is killed due to the number of lines specified by the ``max_lines`` parameter being exceeded.

``log``

Expand Down
4 changes: 4 additions & 0 deletions master/docs/manual/configuration/steps/shell_command.rst
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,10 @@ The :bb:step:`ShellCommand` arguments are:
If the command takes longer than this many seconds, it will be killed.
This is disabled by default.

``max_lines``
If the command outputs more lines than this maximum lines, it will be killed.
This is disabled by default.

``logEnviron``
If ``True`` (the default), then the step's logfile will describe the environment variables on the worker.
In situations where the environment is not relevant and is long, it may be easier to set it to ``False``.
Expand Down
1 change: 1 addition & 0 deletions newsfragments/max-lines-parameter-shell.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added `max_lines` parameter to the shell command, allowing processes to be terminated if they exceed a specified line count.
3 changes: 2 additions & 1 deletion worker/buildbot_worker/commands/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from buildbot_worker.interfaces import IWorkerCommand

# The following identifier should be updated each time this file is changed
command_version = "3.2"
command_version = "3.3"

# version history:
# >=1.17: commands are interruptable
Expand Down Expand Up @@ -68,6 +68,7 @@
# command.
# >= 3.1: rmfile command added to remove a file
# >= 3.2: shell command now reports failure reason in case the command timed out.
# >= 3.3: shell command now supports max_lines parameter.


@implementer(IWorkerCommand)
Expand Down
1 change: 1 addition & 0 deletions worker/buildbot_worker/commands/shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ def start(self):
environ=args.get('env'),
timeout=args.get('timeout', None),
maxTime=args.get('maxTime', None),
max_lines=args.get('max_lines', None),
sigtermTime=args.get('sigtermTime', None),
sendStdout=args.get('want_stdout', True),
sendStderr=args.get('want_stderr', True),
Expand Down
22 changes: 22 additions & 0 deletions worker/buildbot_worker/runprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ def __init__(
sendRC=True,
timeout=None,
maxTime=None,
max_lines=None,
sigtermTime=None,
initialStdin=None,
keepStdout=False,
Expand Down Expand Up @@ -351,6 +352,8 @@ def to_bytes(cmd):
self.unicode_encoding = unicode_encoding
self.send_update = send_update
self.process = None
self.line_count = 0
self.max_line_kill = False
if not os.path.exists(workdir):
os.makedirs(workdir)
if environ:
Expand Down Expand Up @@ -396,6 +399,7 @@ def subst(match):
self.ioTimeoutTimer = None
self.sigtermTime = sigtermTime
self.maxTime = maxTime
self.max_lines = max_lines
self.maxTimeoutTimer = None
self.killTimer = None
self.keepStdout = keepStdout
Expand Down Expand Up @@ -681,6 +685,7 @@ def unlink_temp(result):

def addStdout(self, data):
if self.sendStdout:
self._check_max_lines(data)
self.send_update([('stdout', data)])

if self.keepStdout:
Expand All @@ -690,6 +695,7 @@ def addStdout(self, data):

def addStderr(self, data):
if self.sendStderr:
self._check_max_lines(data)
self.send_update([('stderr', data)])

if self.keepStderr:
Expand Down Expand Up @@ -754,6 +760,22 @@ def doMaxTimeout(self):
self.send_update([("failure_reason", "timeout")])
self.kill(msg)

def _check_max_lines(self, data):
if self.max_lines is not None:
self.line_count += len(re.findall(r"\r\n|\r|\n", data))
if self.line_count > self.max_lines and not self.max_line_kill:
self.pp.transport.closeStdout()
self.max_line_kill = True
self.do_max_lines()

def do_max_lines(self):
msg = (
f"command exceeds max lines: {self.line_count}/{self.max_lines} "
f"written/allowed running {self.fake_command}"
)
self.send_update([("failure_reason", "max_lines_failure")])
self.kill(msg)

def isDead(self):
if self.process.pid is None:
return True
Expand Down
1 change: 1 addition & 0 deletions worker/buildbot_worker/test/fake/runprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ def __init__(self, command_id, command, workdir, unicode_encoding, send_update,
"sendRC": True,
"timeout": None,
"maxTime": None,
"max_lines": None,
"sigtermTime": None,
"initialStdin": None,
"keepStdout": False,
Expand Down
34 changes: 34 additions & 0 deletions worker/buildbot_worker/test/unit/test_runprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,21 @@ def printArgsCommand():
return [sys.executable, '-c', 'import sys; sys.stdout.write(repr(sys.argv[1:]))']


def print_text_command(lines, phrase):
return [
sys.executable,
'-c',
f'''
import time
import sys
for _ in range({lines}):
sys.stdout.write("{phrase}\\n")
sys.stdout.flush()
time.sleep(0.2)
''',
]


# windows returns rc 1, because exit status cannot indicate "signalled";
# posix returns rc -1 for "signalled"
FATAL_RC = -1
Expand Down Expand Up @@ -334,6 +349,25 @@ def testCommandMaxTime(self):
self.assertTrue(('rc', FATAL_RC) in self.updates, self.show())
self.assertTrue(("failure_reason", "timeout") in self.updates, self.show())

@defer.inlineCallbacks
def test_command_max_lines(self):
s = runprocess.RunProcess(
0,
print_text_command(5, 'hello'),
self.basedir,
'utf-8',
self.send_update,
sendStdout=True,
max_lines=1,
)

d = s.start()
yield d

self.assertTrue(('stdout', nl('hello\n')) in self.updates, self.show())
self.assertTrue(('rc', FATAL_RC) in self.updates, self.show())
self.assertTrue(("failure_reason", "max_lines_failure") in self.updates, self.show())

@compat.skipUnlessPlatformIs("posix")
@defer.inlineCallbacks
def test_stdin_closed(self):
Expand Down

0 comments on commit 83dd162

Please sign in to comment.