Skip to content

intelmqctl: On stop wait longer for bots to be stopped #2598

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
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
7 changes: 7 additions & 0 deletions docs/admin/configuration/intelmq.md
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,13 @@ configured to do so.

(optional, boolean) Verify the TLS certificate of the server. Defaults to true.

**`stop_retry_limit`**

(optional, integer) amount of retries when checking the status of a botnet after issuing `intelmqctl stop`. Each retry
another *0.1s* longer is waited until a maximum of *5s* to sleep in each iteration is reached. Only applies when
stopping a bot*net* (not individual bots).
Defaults to 5.

#### Individual Bot Configuration

!!! info
Expand Down
38 changes: 32 additions & 6 deletions intelmq/bin/intelmqctl.py
Original file line number Diff line number Diff line change
Expand Up @@ -563,12 +563,38 @@ def botnet_stop(self, group=None):
for bot_id in bots:
self.bot_stop(bot_id, getstatus=False)

retval = 0
time.sleep(0.75)
for bot_id in bots:
botnet_status[bot_id] = self.bot_status(bot_id)[1]
if botnet_status[bot_id] not in ['stopped', 'disabled']:
retval = 1
# shallow copy of the list suffices
# only aliasing the list to ease reading the following
stopped_but_still_running_bots = bots

retries = getattr(self._parameters, 'stop_retry_limit', 5)

# parameters (default):
# - sleep 0.75 s with an increment of 0.1
# - at most 5 tries
# => sleep-ing at most 4.75 seconds
sleep_time = 0.75 # in seconds
for _ in range(retries):
# give the bots some time to terminate
time.sleep(sleep_time)
# update the botnet_status
for bot_id in stopped_but_still_running_bots:
botnet_status[bot_id] = self.bot_status(bot_id)[1]
if botnet_status[bot_id] in ['stopped', 'disabled']:
stopped_but_still_running_bots.remove(bot_id)
Comment on lines +581 to +584
Copy link
Member

Choose a reason for hiding this comment

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

You iterate over a list while modifying it. That's now working in the way as you might expect:

>>> d = list(range(5))
>>> for x in d:
...     print(x)
...     if x == 2:
...         d.remove(x)
...         
0
1
2
4

That skips an element.

You need to iterate over a copy.


# check if all bots are stopped -> no need to wait further
if len(stopped_but_still_running_bots) == 0:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if len(stopped_but_still_running_bots) == 0:
if not stopped_but_still_running_bots:

break
# the longer the bots need to terminate the longer we wait to check
# again to avoid long-term load on the system
# but stop at 5 seconds to avoid waiting too long until rechecking
# the status
sleep_time = min(5, sleep_time + 0.1)

retval = 1
if len(stopped_but_still_running_bots) == 0:
retval = 0

self.log_botnet_message('stopped', group)
return retval, botnet_status
Expand Down
Loading