Repository: osbuild/osbuild
Pull request #1816 (2024-07-11 11:25:11)

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.


Patch:
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 7df91857..590de8c3 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -41,6 +41,8 @@ RPM:
           - aws/fedora-40-aarch64
           - aws/centos-stream-9-x86_64
           - aws/centos-stream-9-aarch64
+          - aws/centos-stream-10-x86_64
+          - aws/centos-stream-10-aarch64
           - aws/rhel-8.10-ga-x86_64
           - aws/rhel-8.10-ga-aarch64
           - aws/rhel-9.4-ga-x86_64
diff --git a/Schutzfile b/Schutzfile
index dce75ee3..438510fa 100644
--- a/Schutzfile
+++ b/Schutzfile
@@ -109,6 +109,47 @@
       }
     ]
   },
+  "centos-stream-10": {
+    "repos": [
+      {
+        "file": "/etc/yum.repos.d/centos.repo",
+        "x86_64": [
+          {
+            "title": "baseos",
+            "name": "baseos",
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el10/cs10-x86_64-baseos-20240701"
+          },
+          {
+            "title": "appstream",
+            "name": "appstream",
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el10/cs10-x86_64-appstream-20240701"
+          },
+          {
+            "title": "crb",
+            "name": "crb",
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el10/cs10-x86_64-crb-20240701"
+          }
+        ],
+        "aarch64": [
+          {
+            "title": "baseos",
+            "name": "baseos",
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el10/cs10-aarch64-baseos-20240701"
+          },
+          {
+            "title": "appstream",
+            "name": "appstream",
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el10/cs10-aarch64-appstream-20240701"
+          },
+          {
+            "title": "crb",
+            "name": "crb",
+            "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/public/el10/cs10-aarch64-crb-20240701"
+          }
+        ]
+      }
+    ]
+  },
   "rhel-9.5": {
     "repos": [
       {
diff --git a/osbuild.spec b/osbuild.spec
index f7229990..93880b17 100644
--- a/osbuild.spec
+++ b/osbuild.spec
@@ -1,7 +1,7 @@
 %global         forgeurl https://github.com/osbuild/osbuild
 %global         selinuxtype targeted
 
-Version:        123
+Version:        124
 
 %forgemeta
 
diff --git a/osbuild/__init__.py b/osbuild/__init__.py
index c9363164..a2b2c711 100644
--- a/osbuild/__init__.py
+++ b/osbuild/__init__.py
@@ -10,7 +10,7 @@ independent of osbuild but used across the osbuild codebase.
 
 from .pipeline import Manifest, Pipeline, Stage
 
-__version__ = "123"
+__version__ = "124"
 
 __all__ = [
     "Manifest",
diff --git a/schutzbot/mockbuild.sh b/schutzbot/mockbuild.sh
index e5717224..be3aa2a5 100755
--- a/schutzbot/mockbuild.sh
+++ b/schutzbot/mockbuild.sh
@@ -107,13 +107,19 @@ if curl --silent --fail --head --output /dev/null "${REPO_URL}/repodata/repomd.x
 fi
 
 # Mock and s3cmd is only available in EPEL for RHEL.
-if [[ $ID == rhel || $ID == centos ]] && ! rpm -q epel-release; then
+# TODO: Adjust this condition, once EPEL-10 is enabled
+if [[ ($ID == rhel || $ID == centos) && ${VERSION_ID%.*} -lt 10 ]] && ! rpm -q epel-release; then
     greenprint "📦 Setting up EPEL repository"
     curl -Ls --retry 5 --output /tmp/epel.rpm \
         https://dl.fedoraproject.org/pub/epel/epel-release-latest-${VERSION_ID%.*}.noarch.rpm
     dnf_install_with_retry /tmp/epel.rpm
 fi
 
+# TODO: Remove this workaround, once EPEL-10 is enabled
+if [[ ($ID == rhel || $ID == centos) && ${VERSION_ID%.*} == 10 ]]; then
+    sudo dnf copr enable -y @osbuild/centpkg "centos-stream-10-$(uname -m)"
+fi
+
 # Install requirements for building RPMs in mock.
 greenprint "📦 Installing mock requirements"
 dnf_install_with_retry createrepo_c make mock python3-pip rpm-build s3cmd
diff --git a/schutzbot/terraform b/schutzbot/terraform
index b56076f0..ec17c2cb 100644
--- a/schutzbot/terraform
+++ b/schutzbot/terraform
@@ -1,1 +1,1 @@
-445f5effb0cd29f2b0610bcec88acc22475f6326
+2530e0ee541687cb5c31f44d1325363e3cc28f78
diff --git a/setup.py b/setup.py
index 26c9f717..d8f5e27a 100644
--- a/setup.py
+++ b/setup.py
@@ -2,7 +2,7 @@ import setuptools
 
 setuptools.setup(
     name="osbuild",
-    version="123",
+    version="124",
     description="A build system for OS images",
     packages=["osbuild", "osbuild.formats", "osbuild.util"],
     license='Apache-2.0',
diff --git a/sources/org.osbuild.curl b/sources/org.osbuild.curl
index db271ff3..c858ff61 100755
--- a/sources/org.osbuild.curl
+++ b/sources/org.osbuild.curl
@@ -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)):


Test patch:
diff --git a/schutzbot/manifest_tests.sh b/schutzbot/manifest_tests.sh
index 4700875c..cfdc8c45 100755
--- a/schutzbot/manifest_tests.sh
+++ b/schutzbot/manifest_tests.sh
@@ -9,7 +9,7 @@ MANIFEST_DB_COMMIT=$(cat Schutzfile | jq -r '.global.dependencies."manifest-db".
 MANIFEST_DB_REPO="https://github.com/osbuild/manifest-db"
 git clone "$MANIFEST_DB_REPO" manifest-db
 cd manifest-db
-git checkout "$MANIFEST_DB_COMMIT"
+git checkout --force "$MANIFEST_DB_COMMIT"
 
 # update selinux labels for the image-info tool
 OSBUILD_LABEL=$(matchpathcon -n /usr/bin/osbuild)
diff --git a/sources/test/test_curl_source.py b/sources/test/test_curl_source.py
index a3fd45f0..5a591a9e 100644
--- a/sources/test/test_curl_source.py
+++ b/sources/test/test_curl_source.py
@@ -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))