cibuildwheel icon indicating copy to clipboard operation
cibuildwheel copied to clipboard

fix: enforce minimum version of docker/podman

Open mayeut opened this issue 1 year ago • 4 comments

This allows to always pass --platform to the OCI engine thus fixing issues with multiarch images. It also allows to use docker cp instead of system tar workaround.

fix #1771 fix #1957 fix #1962 close #1968

Last time this was attempted, it failed on some CI (c.f. https://github.com/pypa/cibuildwheel/pull/1773), let's check the status now.

mayeut avatar Aug 11 '24 12:08 mayeut

gitlab might or might not fail if this goes in. @joerick, we might want to push this to a branch of the pypa repo rather than my fork in order to test gitlab (If I remember correctly, that's the only way to test gitlab) ?

Travis-CI is failing (despite result being OK). AppVeyor is another issue entirely, I don't think it's being tested anymore ? (since the move to pypa ?)

mayeut avatar Aug 12 '24 18:08 mayeut

So far, the only CI that exhibited issues is Travis CI on graviton2 VM (still missing gitlab tests & Travis CI ppc64le/s390x). It uses docker 19.03 on Ubuntu Focal (Jammy not available yet).

We can either allow docker 19.03 but print a warning or require TravisCI graviton2 VM users to update docker. I'd rather see users update docker (it's a matter of adding 8 lines in .travis.yml) as this might also allow docker cp without any workarounds for legacy behavior.

@henryiii, @joerick any thoughts ?

mayeut avatar Aug 16 '24 10:08 mayeut

AppVeyor build can be seen successful on my fork: https://ci.appveyor.com/project/mayeut/cibuildwheel/builds/50422602 CirrusCI build also (no more compute credits here): https://cirrus-ci.com/build/5147426338635776 I tested gitlab by pushing my branch to the pypa repo.

mayeut avatar Aug 17 '24 09:08 mayeut

I'd rather see users update docker (it's a matter of adding 8 lines in .travis.yml) as this might also allow docker cp without any workarounds for legacy behavior.

That sounds fine to me. I assume this is the incantation to upgrade docker on that platform?

.travis.yml

addons:
  apt:
    sources:
      - sourceline: 'deb https://download.docker.com/linux/ubuntu $(lsb_release -cs) stable'
    packages:
    - docker-ce docker-ce-cli containerd.io

joerick avatar Aug 22 '24 08:08 joerick

There's an issue with s390x & ppc64le on Travis CI so this requires a bit more work...

mayeut avatar Sep 03 '24 15:09 mayeut

It seems the installation of QEMU is failing on s390x & ppc64le which leads to the oci_container_test timeouts.

mayeut avatar Sep 04 '24 05:09 mayeut

Converting to draft until the remaining (weird) issue on s390x gets solved.

mayeut avatar Sep 06 '24 18:09 mayeut

The weird issue is that docker container ls does not seem to list the just created/running container... Adding a retry, the unit test passes:

diff --git a/unit_test/oci_container_test.py b/unit_test/oci_container_test.py
index e913c203..803b09bd 100644
--- a/unit_test/oci_container_test.py
+++ b/unit_test/oci_container_test.py
@@ -8,6 +8,7 @@
 import subprocess
 import sys
 import textwrap
+import time
 from pathlib import Path, PurePath, PurePosixPath
 
 import pytest
@@ -134,16 +135,23 @@ def test_container_removed(container_engine):
     with OCIContainer(
         engine=container_engine, image=DEFAULT_IMAGE, oci_platform=DEFAULT_OCI_PLATFORM
     ) as container:
-        docker_containers_listing = subprocess.run(
-            f"{container.engine.name} container ls",
-            shell=True,
-            check=True,
-            stdout=subprocess.PIPE,
-            text=True,
-        ).stdout
         assert container.name is not None
-        assert container.name in docker_containers_listing
         old_container_name = container.name
+        retry_count = 3
+        while retry_count > 0:
+            retry_count -= 1
+            docker_containers_listing = subprocess.run(
+                f"{container.engine.name} container ls",
+                shell=True,
+                check=True,
+                stdout=subprocess.PIPE,
+                text=True,
+            ).stdout
+            if container.name in docker_containers_listing:
+                break
+            if retry_count == 0:
+                pytest.xfail("s390x travis...")
+            time.sleep(0.5)
 
     docker_containers_listing = subprocess.run(
         f"{container.engine.name} container ls",

I will let the debug run finish before pushing again to this branch skipping this test on Travis CI s390x...

mayeut avatar Sep 06 '24 19:09 mayeut

Skipping the flaky test on s390x allowed to run all tests successfully. Installing only test dependencies of cibuildwheel allows not to build some wheels on ppc64le/s390x (cryptography, pynacl, pyaml, ...) reducing setup time.

mayeut avatar Sep 07 '24 08:09 mayeut

FYI This change causes a build failure for mypy on manylinux and musllinux with Github actions and ubuntu-latest. https://github.com/mypyc/mypy_mypyc-wheels/actions/runs/10852483095/job/30118641732#step:4:461

For the build, the mypy repo is first cloned in a previous step and cibuildwheel subsequently run with package-dir: mypy.

The error

fatal: detected dubious ownership in repository at '/project/mypy'

-- I was able to work around it by adding this to linux.before-all:

git config --global --add safe.directory {project}/mypy

The link to the PR: https://github.com/mypyc/mypy_mypyc-wheels/pull/82

cdce8p avatar Sep 13 '24 17:09 cdce8p

Happy to report that this change exposed a bug in our automation, revealing a mistake I made 4 years ago and never noticed: https://github.com/ansible/pylibssh/pull/692.

The problem looks as follows:

  e897bff8bb4b: Pull complete
  Digest: sha256:d1a2a5447db38774458dc669abadcff717eaa2f6ee5d9797144c659a3f4ae60d
  Status: Downloaded newer image for ghcr.io/ansible/pylibssh-manylinux_2_28_x86_64:libssh-v0.9.6
  image with reference ghcr.io/ansible/pylibssh-manylinux_2_28_x86_64:libssh-v0.9.6 was found but does not match the specified platform: wanted linux/amd64, actual: linux/linux/amd64

I figured I'd document it because it was a pain to google the thing and I never got a hit with somebody else seeing this output with a double prefix.

webknjaz avatar Mar 18 '25 22:03 webknjaz