-
Notifications
You must be signed in to change notification settings - Fork 338
MOD-13779 | MOD-13574 | MOD-13811 Dockerize CI, add AL2023 and macos-… #1502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…26 (#1494) * MOD-13779 Dockerize CI * MOD-13574 AL2023 * MOD-13811 MACOS-26
| rsync unzip tar awscli clang curl openssl11 openssl11-devel | ||
|
|
||
| RUN source /opt/rh/devtoolset-11/enable | ||
| RUN cp /opt/rh/devtoolset-11/enable /etc/profile.d/scl-devtoolset-11.sh |
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.
Docker RUN source doesn't persist GCC toolset activation
High Severity
RUN source /opt/rh/devtoolset-11/enable is a no-op — each Docker RUN executes in a new shell, so environment changes don't persist to subsequent layers. Only devtoolset-11-gcc is installed (not the base gcc package), so there is no gcc or cc in PATH for later steps. The install_redis.sh step will fail when make tries to invoke cc. The profile.d copy on line 20 only helps login shells, but Docker RUN and the workflow's docker run bash -c both use non-login shells. The fix is to use ENV PATH=/opt/rh/devtoolset-11/root/usr/bin:${PATH} to persistently add the toolset to PATH.
Additional Locations (1)
|
|
||
| # Install git and build tools first | ||
| RUN dnf install -y git make wget openssl openssl-devel which \ | ||
| rsync unzip clang tar |
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.
Missing gcc package prevents Redis build from compiling
High Severity
The package list installs clang but not gcc or any development tools group. On RHEL-based systems, clang does not provide /usr/bin/cc. Since Make's default CC is cc, the install_redis.sh step will fail with a "cc: command not found" error when building Redis. Every other Dockerfile in this PR installs gcc explicitly or through a group like build-essential / Development Tools.
Additional Locations (1)
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.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
| --cap-add=SYS_PTRACE \ | ||
| --security-opt seccomp=unconfined \ | ||
| ${{ env.DOCKER_IMAGE }} \ | ||
| bash -c "cargo test && MODULE=\$(realpath ./target/release/rejson.so) RLTEST_ARGS='--no-progress' \$(realpath ./tests/pytest/tests.sh) VG=${{ inputs.run_valgrind && '1' || '0' }}" |
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.
VG variable passed as argument, not environment variable
High Severity
VG=... is placed after the script path in the bash -c command, making it a positional argument to tests.sh rather than an environment variable. The script checks $VG as an environment variable (line 600 of tests.sh: if [[ $VG == 1 ]]) to decide whether to set up Valgrind, so VG is never actually set and Valgrind silently won't run even when run_valgrind is true. It needs to be placed before the script invocation alongside MODULE and RLTEST_ARGS.
| with: | ||
| arch: arm64 | ||
| redis-ref: ${{needs.prepare-values.outputs.redis-ref}} | ||
| secrets: inherit |
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.
CI builds all platforms without OS restriction
Medium Severity
Neither build-linux-x64 nor build-linux-arm64 specifies an os parameter, so flow-linux.yml falls back to all default platforms (11 x64 + 7 arm64 = 18 builds). The old CI workflow was restricted to just jammy rocky9 amazonlinux2 azurelinux3 for x64 and only azurelinux3 for arm64. This ~3.6x increase in matrix jobs runs on every pull request, significantly increasing CI resource consumption and feedback time.
| arch: x64 | ||
| # os: jammy rocky9 amazonlinux2 | ||
| os: bionic focal jammy rocky8 rocky9 bullseye amazonlinux2 mariner2 azurelinux3 | ||
| os: bionic focal jammy rocky8 rocky9 bullseye amazonlinux2 mariner2 azurelinux3 alpine |
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.
Weekly workflow missing amazonlinux2023 in OS lists
Low Severity
The amazonlinux2023 platform is missing from the weekly workflow's explicit os lists for both x64 and arm64. It's present in the flow-linux.yml defaults (used by CI/nightly) and in the event-tag.yml x64 list, so this appears to be an oversight given the PR's goal of adding AL2023 support.


…26 (#1494)
MOD-13779 Dockerize CI
MOD-13574 AL2023
MOD-13811 MACOS-26
Note
Medium Risk
Replaces multiple per-OS/per-arch GitHub Actions workflows with a new Docker-based build/test/pack pipeline, which can affect CI coverage and artifact publishing across platforms if any Dockerfile or upload step is misconfigured.
Overview
Dockerizes Linux CI and consolidates workflows. PR replaces the old
flow-linux-x86.yml,flow-linux-arm.yml,flow-azurelinux3-arm.yml, andflow-alpine.ymlworkflows with a single parameterizedflow-linux.ymlthat builds per-OS Docker images (Dockerfile.*), runs build/tests inside containers (optionally with valgrind), then packs and uploads artifacts.Event workflows (
event-ci,event-nightly,event-weekly,event-tag) are rewired to call the new Linux flow for bothx64andarm64, expanding the OS matrix (addsalpinebroadly andamazonlinux2023for tags/default matrices) and making nightlyredis-refconfigurable. Artifact uploading is adapted for container runs via a newsbin/upload-artifacts-s3, and Python deps are pinned (setuptools<81) via updated requirements and common install scripts; macOS builds are adjusted to run an arm64 matrix includingmacos-26.Written by Cursor Bugbot for commit a70d4bc. This will update automatically on new commits. Configure here.