feat(core): Add support for building and running on manylinux_2_28; Add manylinux_2_28 dependency container images. #1041
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| FROM quay.io/pypa/manylinux_2_28_aarch64 | ||
|
Contributor
There was a problem hiding this comment. 🛠️ Refactor suggestion Pin the base image digest for reproducibility Same rationale as for the x86_64 image—lock the manylinux base by digest. 🧰 Tools🪛 Checkov (3.2.334)[LOW] 1-1: Ensure the base image uses a non latest version tag (CKV_DOCKER_7) [LOW] 1-14: Ensure that HEALTHCHECK instructions have been added to container images (CKV_DOCKER_2) [LOW] 1-14: Ensure that a user for the container has been created (CKV_DOCKER_3) 🪛 Hadolint (2.12.0)[warning] 1-1: Always tag the version of an image explicitly (DL3006) 🤖 Prompt for AI Agents |
||
|
|
||
| WORKDIR /root | ||
|
|
||
| RUN mkdir -p ./tools/scripts/lib_install | ||
| COPY ./tools/scripts/lib_install ./tools/scripts/lib_install | ||
|
|
||
| RUN ./tools/scripts/lib_install/manylinux-2.28/install-all.sh | ||
|
|
||
| # Remove cached files | ||
| RUN dnf clean all && rm -rf /var/cache/dnf /tmp/* /var/tmp/* | ||
|
|
||
| # NOTE: Don't flatten the image or else we'll lose any environment modifications from the base | ||
| # image. | ||
|
Contributor
There was a problem hiding this comment. 🧹 Nitpick (assertive) Consider adding HEALTHCHECK and running as non-root • Both are low-effort and align with container best-practices. 🧰 Tools🪛 Checkov (3.2.334)[LOW] 1-1: Ensure the base image uses a non latest version tag (CKV_DOCKER_7) [LOW] 1-14: Ensure that HEALTHCHECK instructions have been added to container images (CKV_DOCKER_2) [LOW] 1-14: Ensure that a user for the container has been created (CKV_DOCKER_3) 🪛 Hadolint (2.12.0)[warning] 1-1: Always tag the version of an image explicitly (DL3006) 🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,27 @@ | ||||||||||||||||||||||||||||||||||
| #!/usr/bin/env bash | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| set -eu | ||||||||||||||||||||||||||||||||||
| set -o pipefail | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )" | ||||||||||||||||||||||||||||||||||
| component_root="${script_dir}/../../../" | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| build_cmd=( | ||||||||||||||||||||||||||||||||||
| docker buildx build | ||||||||||||||||||||||||||||||||||
| --platform linux/arm64 | ||||||||||||||||||||||||||||||||||
| --tag clp-core-dependencies-aarch64-manylinux-2.28:dev | ||||||||||||||||||||||||||||||||||
| "$component_root" | ||||||||||||||||||||||||||||||||||
| --file "${script_dir}/Dockerfile" | ||||||||||||||||||||||||||||||||||
| --load | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment.
-build_cmd=(
- docker buildx build
- --platform linux/arm64
- --tag clp-core-dependencies-aarch64-manylinux-2.28:dev
- "$component_root"
- --file "${script_dir}/Dockerfile"
- --load
-)
+build_cmd=(
+ docker buildx build
+ --platform linux/arm64
+ --tag clp-core-dependencies-aarch64-manylinux-2.28:dev
+ --file "${script_dir}/Dockerfile"
+ --load
+ "$component_root"
+)After this change the command succeeds irrespective of Docker/Buildx version. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if command -v git >/dev/null && git -C "$script_dir" rev-parse --is-inside-work-tree >/dev/null ; | ||||||||||||||||||||||||||||||||||
| then | ||||||||||||||||||||||||||||||||||
| build_cmd+=( | ||||||||||||||||||||||||||||||||||
| --label "org.opencontainers.image.revision=$(git -C "$script_dir" rev-parse HEAD)" | ||||||||||||||||||||||||||||||||||
| --label "org.opencontainers.image.source=$(git -C "$script_dir" remote get-url origin)" | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. 🧹 Nitpick (assertive) Minor: use repo root for Git metadata to avoid submodule edge-cases If this script is moved inside a git-submodule, 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| echo "Running: ${build_cmd[*]}" | ||||||||||||||||||||||||||||||||||
| "${build_cmd[@]}" | ||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,14 @@ | ||||||
| FROM quay.io/pypa/manylinux_2_28_x86_64 | ||||||
|
Contributor
There was a problem hiding this comment. 🛠️ Refactor suggestion Pin the base image to an immutable digest Using only the tag ( -FROM quay.io/pypa/manylinux_2_28_x86_64
+FROM quay.io/pypa/manylinux_2_28_x86_64@sha256:<digest-here>Update whenever you intentionally pull in a new base. 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Checkov (3.2.334)[LOW] 1-1: Ensure the base image uses a non latest version tag (CKV_DOCKER_7) [LOW] 1-14: Ensure that HEALTHCHECK instructions have been added to container images (CKV_DOCKER_2) [LOW] 1-14: Ensure that a user for the container has been created (CKV_DOCKER_3) 🪛 Hadolint (2.12.0)[warning] 1-1: Always tag the version of an image explicitly (DL3006) 🤖 Prompt for AI Agents |
||||||
|
|
||||||
| WORKDIR /root | ||||||
|
|
||||||
| RUN mkdir -p ./tools/scripts/lib_install | ||||||
| COPY ./tools/scripts/lib_install ./tools/scripts/lib_install | ||||||
|
|
||||||
| RUN ./tools/scripts/lib_install/manylinux-2.28/install-all.sh | ||||||
|
||||||
|
|
||||||
| # Remove cached files | ||||||
| RUN dnf clean all && rm -rf /var/cache/dnf /tmp/* /var/tmp/* | ||||||
|
|
||||||
| # NOTE: Don't flatten the image or else we'll lose any environment modifications from the base | ||||||
| # image. | ||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,27 @@ | ||||||||||||||||||||||||||||||||||||
| #!/usr/bin/env bash | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| set -eu | ||||||||||||||||||||||||||||||||||||
| set -o pipefail | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )" | ||||||||||||||||||||||||||||||||||||
| component_root="${script_dir}/../../../" | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| build_cmd=( | ||||||||||||||||||||||||||||||||||||
| docker buildx build | ||||||||||||||||||||||||||||||||||||
| --platform linux/amd64 | ||||||||||||||||||||||||||||||||||||
| --tag clp-core-dependencies-x86-manylinux-2.28:dev | ||||||||||||||||||||||||||||||||||||
| "$component_root" | ||||||||||||||||||||||||||||||||||||
| --file "${script_dir}/Dockerfile" | ||||||||||||||||||||||||||||||||||||
| --load | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. 🧹 Nitpick (assertive) Pin latest base image at build time Adding docker buildx build
--platform linux/amd64
+ --pull
--tag clp-core-dependencies-x86-manylinux-2.28:dev📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Member
There was a problem hiding this comment. @coderabbitai, can you open an issue to address this for all images in another PR?
Contributor
There was a problem hiding this comment. 🧩 Analysis chain@kirkrodrigues I'll create an issue to address this across all Docker images. Let me first check what other Docker build scripts exist in the repository to ensure we capture the full scope. 🏁 Script executed: Length of output: 3813 Now I'll create a GitHub issue to address adding the 🏁 Script executed: Length of output: 2049 @kirkrodrigues I've successfully created issue #1051 to address adding the The issue covers all 6 build scripts I found:
It includes the rationale for the change (ensuring fresh base images with latest security updates), examples of how to implement it, and references back to this PR discussion. The issue has been assigned to you as requested. 🧠 Learnings used |
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if command -v git >/dev/null && git -C "$script_dir" rev-parse --is-inside-work-tree >/dev/null ; | ||||||||||||||||||||||||||||||||||||
| then | ||||||||||||||||||||||||||||||||||||
| build_cmd+=( | ||||||||||||||||||||||||||||||||||||
| --label "org.opencontainers.image.revision=$(git -C "$script_dir" rev-parse HEAD)" | ||||||||||||||||||||||||||||||||||||
| --label "org.opencontainers.image.source=$(git -C "$script_dir" remote get-url origin)" | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. 🧹 Nitpick (assertive) Use repo root for Git metadata When the script lives in a sub-directory, - --label "org.opencontainers.image.revision=$(git -C "$script_dir" rev-parse HEAD)"
- --label "org.opencontainers.image.source=$(git -C "$script_dir" remote get-url origin)"
+ --label "org.opencontainers.image.revision=$(git -C "$component_root" rev-parse HEAD)"
+ --label "org.opencontainers.image.source=$(git -C "$component_root" remote get-url origin)"📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| echo "Running: ${build_cmd[*]}" | ||||||||||||||||||||||||||||||||||||
| "${build_cmd[@]}" | ||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| #!/usr/bin/env bash | ||
|
|
||
| set -eu | ||
| set -o pipefail | ||
|
|
||
| script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )" | ||
|
Contributor
There was a problem hiding this comment. 🧹 Nitpick (assertive) Warn early if the script isn’t run as root The subscripts invoke if [[ $EUID -ne 0 ]]; then
echo "Error: please run this script with root privileges (e.g., via sudo)" >&2
exit 1
fi🤖 Prompt for AI Agents |
||
|
|
||
| "${script_dir}/install-prebuilt-packages.sh" | ||
| "${script_dir}/install-packages-from-source.sh" | ||
|
|
||
| # TODO: https://github.com/y-scope/clp/issues/795 | ||
| "${script_dir}/../check-cmake-version.sh" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| #!/usr/bin/env bash | ||
|
|
||
| set -eu | ||
| set -o pipefail | ||
|
|
||
| script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )" | ||
| lib_install_scripts_dir="${script_dir}/.." | ||
|
|
||
| # NOTE: The remaining installation scripts depend on boost, so we install it beforehand. | ||
| "${lib_install_scripts_dir}/install-boost.sh" 1.87.0 | ||
|
|
||
| # NOTE: | ||
| # 1. libarchive may statically link with LZMA, LZ4, and Zstandard, so we install them beforehand. | ||
| # 2. The versions of libarchive, LZMA, LZ4, and Zstandard available in the manylinux 2.28 package | ||
| # repositories are either dated or don't include static libraries, so we install more recent | ||
| # versions from source. | ||
| "${lib_install_scripts_dir}/liblzma.sh" 5.8.1 | ||
| "${lib_install_scripts_dir}/lz4.sh" 1.10.0 | ||
| "${lib_install_scripts_dir}/zstandard.sh" 1.5.7 | ||
| "${lib_install_scripts_dir}/libarchive.sh" 3.8.0 | ||
|
|
||
| "${lib_install_scripts_dir}/msgpack.sh" 7.0.0 | ||
|
Contributor
There was a problem hiding this comment. 🧹 Nitpick (assertive) Extract version numbers into variables for easier maintenance -# NOTE: The remaining installation scripts depend on boost, so we install it beforehand.
-"${lib_install_scripts_dir}/install-boost.sh" 1.87.0
+# Versions
+BOOST_VER=1.87.0
+LIBLZMA_VER=5.8.1
+LZ4_VER=1.10.0
+ZSTD_VER=1.5.7
+LIBARCHIVE_VER=3.8.0
+MSGPACK_VER=7.0.0
+
+# NOTE: The remaining installation scripts depend on Boost, so we install it beforehand.
+"${lib_install_scripts_dir}/install-boost.sh" "$BOOST_VER"
...
-"${lib_install_scripts_dir}/libarchive.sh" 3.8.0
+"${lib_install_scripts_dir}/libarchive.sh" "$LIBARCHIVE_VER"
...
-"${lib_install_scripts_dir}/msgpack.sh" 7.0.0
+"${lib_install_scripts_dir}/msgpack.sh" "$MSGPACK_VER"This change reduces drift risk when updating dependencies.
🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| #!/usr/bin/env bash | ||
|
|
||
| set -eu | ||
| set -o pipefail | ||
|
|
||
| dnf install -y \ | ||
| gcc-c++ \ | ||
| java-11-openjdk \ | ||
| jq \ | ||
| libcurl-devel \ | ||
| mariadb-connector-c-devel \ | ||
| openssl-devel \ | ||
| zlib-devel \ | ||
| zlib-static | ||
|
|
||
| # Determine architecture for `task` release to install | ||
| rpm_arch=$(rpm --eval "%{_arch}") | ||
| case "$rpm_arch" in | ||
| "x86_64") | ||
| task_pkg_arch="amd64" | ||
| ;; | ||
| "aarch64") | ||
| task_pkg_arch="arm64" | ||
| ;; | ||
| *) | ||
| echo "Error: Unsupported architecture - $rpm_arch" | ||
| exit 1 | ||
| ;; | ||
| esac | ||
|
|
||
| # Install `task` | ||
| # NOTE: We lock `task` to a version < 3.43 to avoid https://github.com/y-scope/clp/issues/872 | ||
| task_pkg_path=$(mktemp -t --suffix ".rpm" task-pkg.XXXXXXXXXX) || exit 1 | ||
| curl \ | ||
| --fail \ | ||
| --location \ | ||
| --output "$task_pkg_path" \ | ||
| --show-error \ | ||
| "https://github.com/go-task/task/releases/download/v3.42.1/task_linux_${task_pkg_arch}.rpm" | ||
| dnf install --assumeyes "$task_pkg_path" | ||
| rm "$task_pkg_path" | ||
|
|
||
| # Downgrade to CMake v3 to work around https://github.com/y-scope/clp/issues/795 | ||
| pipx uninstall cmake | ||
LinZhihao-723 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| pipx install cmake~=3.31 | ||
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| # manylinux 2.28 setup | ||
|
|
||
| To install the dependencies required to build clp-core, follow the steps below. These same steps are | ||
| used by our Docker containers. | ||
|
|
||
| ## Installing dependencies | ||
|
|
||
| :::{caution} | ||
| Before you run any commands below, you should review the scripts to ensure they will not install | ||
| any dependencies or apply any configurations that you don't expect. | ||
| ::: | ||
|
|
||
| To install all dependencies, run the following with elevated privileges: | ||
|
|
||
| ```shell | ||
| components/core/tools/scripts/lib_install/manylinux-2.28/install-all.sh | ||
| ``` | ||
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,38 +1,115 @@ | ||
| # Containers | ||
|
|
||
| We maintain several Docker container images that are useful for building and running CLP. All images | ||
| can be built and used locally, but some are available to download from | ||
| [GitHub Packages][gh-packages]. | ||
|
|
||
| To build an image locally, run the `build.sh` script in the image's directory. | ||
|
|
||
| ## clp-core-dependencies-<arch>-manylinux-2.28 | ||
|
|
||
| Images containing the dependencies necessary to build CLP core in a [manylinux 2.28][manylinux-2.28] | ||
| environment (aarch64 or x86). | ||
|
|
||
| Binaries built on manylinux 2.28 (based on AlmaLinux 8) are expected to be compatible with other | ||
| distros using glibc 2.28+, including: | ||
|
|
||
| * CentOS/RHEL 8+ | ||
| * Debian 10+ | ||
| * Fedora 29+ | ||
| * Ubuntu 18.10+ | ||
|
|
||
| ### clp-core-dependencies-aarch64-manylinux-2.28 | ||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| * Path: | ||
|
|
||
| ```text | ||
| components/core/tools/docker-images/clp-env-base-manylinux-2.28-aarch64 | ||
| ``` | ||
|
|
||
| ### clp-core-dependencies-x86-manylinux-2.28 | ||
|
|
||
| * Path: | ||
|
|
||
| ```text | ||
| components/core/tools/docker-images/clp-env-base-manylinux-2.28-x86_64 | ||
| ``` | ||
|
|
||
|
||
| ## clp-core-dependencies-x86-centos-stream-9 | ||
|
|
||
| An image containing the dependencies necessary to build CLP core in a CentOS Stream 9 x86 | ||
| environment. | ||
|
|
||
| * [GitHub Packages page][core-deps-centos-stream-9] | ||
| * Pull command: | ||
|
|
||
| ```bash | ||
| docker pull ghcr.io/y-scope/clp/clp-core-dependencies-x86-centos-stream-9:main | ||
| ``` | ||
|
|
||
| * Path: | ||
|
|
||
| ```text | ||
| components/core/tools/docker-images/clp-env-base-centos-stream-9 | ||
| ``` | ||
|
|
||
| ## clp-core-dependencies-x86-ubuntu-jammy | ||
|
|
||
| An image containing the dependencies necessary to build CLP core in an Ubuntu Jammy x86 | ||
| environment. | ||
|
|
||
| * [GitHub Packages page][core-deps-ubuntu-jammy] | ||
| * Pull command: | ||
|
|
||
| ```bash | ||
| docker pull ghcr.io/y-scope/clp/clp-core-dependencies-x86-ubuntu-jammy:main | ||
| ``` | ||
|
|
||
| * Path: | ||
|
|
||
| ```text | ||
| components/core/tools/docker-images/clp-env-base-ubuntu-jammy | ||
| ``` | ||
|
|
||
| ## clp-core-x86-ubuntu-jammy | ||
|
|
||
| An image containing the CLP core binaries (`clg`, `clp`, `clp-s`, `glt`, etc.) built in an Ubuntu | ||
| Jammy x86 environment. | ||
|
|
||
| * [GitHub Packages page][core-ubuntu-jammy] | ||
| * Pull command: | ||
|
|
||
| ```bash | ||
| docker pull ghcr.io/y-scope/clp/clp-core-x86-ubuntu-jammy:main | ||
| ``` | ||
|
|
||
| * Path: | ||
|
|
||
| ```text | ||
| components/core/tools/docker-images/clp-core-ubuntu-jammy | ||
| ``` | ||
|
|
||
| ## clp-execution-x86-ubuntu-jammy | ||
|
|
||
| An image containing the dependencies necessary to run the CLP package in an Ubuntu Jammy x86 | ||
| environment. | ||
|
|
||
| * [GitHub Packages page][exe-ubuntu-jammy] | ||
| * Pull command: | ||
|
|
||
| ```bash | ||
| docker pull ghcr.io/y-scope/clp/clp-execution-x86-ubuntu-jammy:main | ||
| ``` | ||
|
|
||
| * Path: | ||
|
|
||
| ```text | ||
| tools/docker-images/clp-execution-base-ubuntu-jammy | ||
| ``` | ||
|
|
||
| [core-deps-centos-stream-9]: https://github.com/y-scope/clp/pkgs/container/clp%2Fclp-core-dependencies-x86-centos-stream-9 | ||
| [core-deps-ubuntu-jammy]: https://github.com/y-scope/clp/pkgs/container/clp%2Fclp-core-dependencies-x86-ubuntu-jammy | ||
| [core-ubuntu-jammy]: https://github.com/y-scope/clp/pkgs/container/clp%2Fclp-core-x86-ubuntu-jammy | ||
| [exe-ubuntu-jammy]: https://github.com/y-scope/clp/pkgs/container/clp%2Fclp-execution-x86-ubuntu-jammy | ||
| [gh-packages]: https://github.com/orgs/y-scope/packages?repo_name=clp | ||
| [manylinux-2.28]: https://github.com/pypa/manylinux?tab=readme-ov-file#manylinux_2_28-almalinux-8-based | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should change
manylinux 2.28tomanylinux_2_28becausemanylinux 2.28is not a distribution + version. 2_28 refers to the glibc version iirc, not the version of many linux.