Skip to content

Commit

Permalink
Make networking/http test more resilient (bugfix) (#1213)
Browse files Browse the repository at this point in the history
* Add http connection python script and its unit tests

* Modify networking/http job to call the Python script instead

This job often fails, usually because of connectivity issues.

It is now replaced with a Python script that automatically runs the
`wget` command 3 times, increasing the timeout after each failure to
give more chances to connect succesfully.

Fix CHECKBOX-1419

* Refactor message printing in the script

* Add backoff and jitter to connection function

Following feedback from team, reworking the connection function to use a
random delay, using a backoff and a jitter. Each new attempt will wait
longer than the previous one (up to 60 seconds per attempt).

The jitter is here to prevent the test from choking the infrastructure
if many devices are trying to run this test at the same moment.

* Add unit test to get 100% coverage
  • Loading branch information
pieqq authored Apr 29, 2024
1 parent 6adf790 commit 070d2fc
Show file tree
Hide file tree
Showing 3 changed files with 147 additions and 2 deletions.
84 changes: 84 additions & 0 deletions providers/base/bin/networking_http.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
#!/usr/bin/env python3
#
# This file is part of Checkbox.
#
# Copyright 2024 Canonical Ltd.
# Written by:
# Pierre Equoy <pierre.equoy@canonical.com>
#
# Checkbox is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License version 3,
# as published by the Free Software Foundation.
#
# Checkbox is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with Checkbox. If not, see <http://www.gnu.org/licenses/>.
#

import argparse
import random
import subprocess
import sys
import time


def http_connect(
url, max_attempts: int = 5, initial_delay=1, backoff_factor=2, max_delay=60
):
"""
Use `wget` to try to connect to `url`. If attempt fails, the next one is
made after adding a random delay calculated using a backoff and a jitter
(with a maximum delay of 60 seconds).
"""
for attempt in range(1, max_attempts + 1):
print(
"Trying to connect to {} (attempt {}/{})".format(
url, attempt, max_attempts
)
)
try:
subprocess.run(
[
"wget",
"-SO",
"/dev/null",
url,
],
check=True,
)
return
except subprocess.CalledProcessError as exc:
print("Attempt {} failed: {}".format(attempt, exc))
print()
delay = min(initial_delay * (backoff_factor**attempt), max_delay)
jitter = random.uniform(
0, delay * 0.5
) # Jitter: up to 50% of the delay
final_delay = delay + jitter
print(
"Waiting for {:.2f} seconds before retrying...".format(
final_delay
)
)
time.sleep(final_delay)
raise SystemExit("Failed to connect to {}!".format(url))


def main(args):
parser = argparse.ArgumentParser()
parser.add_argument("url", help="URL to try to connect to")
parser.add_argument(
"--attempts",
default="5",
help="Number of connection attempts (default %(default)s)",
)
args = parser.parse_args(args)
http_connect(args.url, int(args.attempts))


if __name__ == "__main__":
sys.exit(main(sys.argv[1:]))
60 changes: 60 additions & 0 deletions providers/base/tests/test_networking_http.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
#!/usr/bin/env python3
#
# This file is part of Checkbox.
#
# Copyright 2024 Canonical Ltd.
# Written by:
# Pierre Equoy <pierre.equoy@canonical.com>
#
# Checkbox is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License version 3,
# as published by the Free Software Foundation.
#
# Checkbox is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with Checkbox. If not, see <http://www.gnu.org/licenses/>.
#

import subprocess
from unittest import TestCase
from unittest.mock import patch

import networking_http


class NetworkingHTTPTests(TestCase):
@patch("networking_http.subprocess.run")
@patch("networking_http.time.sleep")
def test_http_connect_max_retries(self, mock_sleep, mock_run):
with self.assertRaises(SystemExit):
networking_http.http_connect("test", 0)

@patch("networking_http.subprocess.run")
@patch("networking_http.time.sleep")
def test_http_connect_success(self, mock_sleep, mock_run):
"""
Test that `http_connect` returns safely if the wget command returns 0
"""
self.assertEqual(networking_http.http_connect("test", 3), None)

@patch("networking_http.subprocess.run")
@patch("networking_http.time.sleep")
def test_http_connect_failure(self, mock_sleep, mock_run):
"""
Test that if set to 3 retries, the connection command (wget, run
through subprocess.run) will be called 3 times
"""
mock_run.side_effect = subprocess.CalledProcessError(1, "")
with self.assertRaises(SystemExit):
networking_http.http_connect("test", 3)
self.assertEqual(mock_run.call_count, 3)

@patch("networking_http.http_connect")
def test_main(self, mock_http_connect):
args = ["test", "--attempts", "6"]
networking_http.main(args)
mock_http_connect.assert_called_with("test", 6)
5 changes: 3 additions & 2 deletions providers/base/units/networking/jobs.pxu
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ user: root
plugin: shell
category_id: com.canonical.plainbox::networking
id: networking/http
command: wget -SO /dev/null http://"$TRANSFER_SERVER"
environ: TRANSFER_SERVER
command: networking_http.py http://"$TRANSFER_SERVER"
_description:
Automated test case to make sure that it's possible to download files through HTTP

Expand Down Expand Up @@ -99,4 +100,4 @@ requires:
model_assertion.gadget != "pi"
{%- else %}
lsb.release >= '18'
{% endif -%}
{% endif -%}

0 comments on commit 070d2fc

Please sign in to comment.