sources(curl): error if curl exists 0 but there are downloads left

As part of the investigation of the CI failure in
https://github.com/osbuild/osbuild-composer/pull/4247
we noticed that curl can return a return_code of `0` even
when it did not downloaded all the urls in a `--config` provided
file. This seems to be curl version dependent, I had a hard
time writing a test-case with the real curl (8.6.0) that
reproduces this so I went with mocking it. We definietly saw
this failure with the centos 9 version (7.76).

Our current code is buggy and assumes that the exit status
of curl is always non-zero if any download fails but that is
only the case when `--fail-early` is used.

The extra paranoia will not hurt even when relying on the
exit code of curl is fixed.
This commit is contained in:
Michael Vogt 2024-07-08 16:32:54 +02:00 committed by Achilleas Koutsou
parent 2b2fec85b2
commit 16667ef260
2 changed files with 17 additions and 0 deletions

View file

@ -317,6 +317,9 @@ class CurlSource(sources.SourceService):
failed_urls = ",".join([itm[1]["url"] for itm in dl_pairs])
raise RuntimeError(f"curl: error downloading {failed_urls}: error code {return_code}")
if len(dl_pairs) > 0:
raise RuntimeError(f"curl: finished with return_code {return_code} but {dl_pairs} left to download")
def _fetch_all_old_curl(self, amended):
with concurrent.futures.ThreadPoolExecutor(max_workers=self.max_workers) as executor:
for _ in executor.map(self.fetch_one, *zip(*amended)):

View file

@ -9,6 +9,7 @@ from unittest.mock import patch
import pytest
import osbuild.testutil
from osbuild.testutil.net import http_serve_directory
SOURCES_NAME = "org.osbuild.curl"
@ -306,3 +307,16 @@ def test_curl_has_parallel_download(mocked_check_output, monkeypatch, sources_mo
mocked_check_output.return_value = OLD_CURL_OUTPUT
assert not sources_module.curl_has_parallel_downloads()
# this check is only done in the "parallel=True" case
@pytest.mark.parametrize("curl_parallel", [True], indirect=["curl_parallel"])
def test_curl_result_is_double_checked(tmp_path, curl_parallel):
test_sources = make_test_sources(tmp_path, 1234, 5)
# simulate that curl returned an exit code 0 even though not all
# sources got downloaded
with osbuild.testutil.mock_command("curl", ""):
with pytest.raises(RuntimeError) as exp:
curl_parallel.fetch_all(test_sources)
assert re.match(r"curl: finished with return_code 0 but .* left to download", str(exp.value))