Skip to content

Stevegrubb konflux changes #3995

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.

Sign up for GitHub

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

Open
dtrawins wants to merge 3 commits into
base: main
Choose a base branch
from
+26 −10
Open

Stevegrubb konflux changes #3995

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1617ce8
Updates to the Red Hat Dockerfile for Konflux
stevegrubb Feb 10, 2026
1fe1361
hadolint ignoring
dtrawins Feb 11, 2026
2a28d1b
correct hadolint
dtrawins Feb 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 26 additions & 10 deletions Dockerfile.redhat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,6 @@ COPY *\.bzl /ovms/
COPY yarn.lock /ovms/
COPY package.json /ovms/

# prebuild dependencies before copying sources
# hadolint ignore=DL3059
RUN bazel build --jobs=$JOBS ${debug_bazel_flags} //:ovms_dependencies @com_google_googletest//:gtest

# hadolint ignore=DL3059
RUN cp -v /etc/ssl/certs/ca-bundle.crt /etc/ssl/certs/ca-certificates.crt

Expand Down Expand Up @@ -310,17 +306,18 @@ RUN bash -c "sed -i -e 's|REPLACE_PROJECT_VERSION|${PROJECT_VERSION}|g' /ovms/sr
if [ "$ov_use_binary" == "0" ] ; then sed -i -e "s#REPLACE_OPENVINO_NAME#$(git --git-dir /openvino/.git log -n 1 | head -n 1 | cut -d' ' -f2 | head -c 12)#g" /ovms/src/version.hpp ; fi && \
bash -c "sed -i -e 's|REPLACE_BAZEL_BUILD_FLAGS|${debug_bazel_flags}${minitrace_flags}|g' /ovms/src/version.hpp"

# Custom Nodes
RUN bazel build --jobs=$JOBS ${debug_bazel_flags} //src:release_custom_nodes

# OVMS
ARG OPTIMIZE_BUILDING_TESTS=0
RUN rm -f /usr/lib64/cmake/OpenSSL/OpenSSLConfig.cmake

# Builds unit tests together with ovms server in one step
# It speeds up CI when tests are executed outside of the image building
# Konflux build system cuts off network access at times. Build all in 1 shot.
# hadolint ignore=SC2046
RUN bazel build --jobs=$JOBS ${debug_bazel_flags} ${minitrace_flags} //src:ovms $(if [ "$OPTIMIZE_BUILDING_TESTS" == "1" ] ; then echo -n //src:ovms_test; fi)
RUN bazel build --jobs=$JOBS ${debug_bazel_flags} //:ovms_dependencies @com_google_googletest//:gtest && \
bazel build --jobs=$JOBS ${debug_bazel_flags} //src:release_custom_nodes && \
bazel build --jobs=$JOBS ${debug_bazel_flags} ${minitrace_flags} //src:ovms \
$(if [ "$OPTIMIZE_BUILDING_TESTS" == "1" ] ; then echo -n //src:ovms_test; fi)
Comment on lines +315 to +320
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment mentions building all in one shot due to Konflux network cutoff, but this consolidation removes the ability to leverage Docker layer caching for individual build steps. If the custom nodes build fails, the entire consolidated build will need to be re-run, including the dependencies that were already built. Consider whether the benefits of working around Konflux network issues outweigh the loss of granular caching, or document this tradeoff explicitly.

Copilot uses AI. Check for mistakes.

# Tests execution
COPY ci/check_coverage.bat /ovms/
Expand Down Expand Up @@ -348,14 +345,33 @@ ARG CAPI_FLAGS="--strip=always --config=mp_off_py_off --//:distro=redhat"
ARG JOBS=40
ARG LTO_ENABLE=OFF
WORKDIR /ovms
RUN bazel build --jobs $JOBS ${CAPI_FLAGS} //src:ovms_shared
# Konflux has a hard time locating the built binaries. Need to put it
# in a known location.
SHELL ["/bin/bash", "-o", "pipefail", "-c"]
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SHELL directive here uses "-o pipefail" without the "-x" flag, which differs from the Ubuntu Dockerfile (line 346) that uses "-xo pipefail". The "-x" flag provides debug output by printing each command before execution, which is helpful for troubleshooting build issues. For consistency and better debugging capabilities, consider adding the "-x" flag here as well.

Suggested change
SHELL ["/bin/bash", "-o", "pipefail", "-c"]
SHELL ["/bin/bash", "-xo", "pipefail", "-c"]

Copilot uses AI. Check for mistakes.
RUN mkdir -p /tmp/bazel-output && \
bazel --output_base=/tmp/bazel-output \
build --jobs=$JOBS ${CAPI_FLAGS} //src:ovms_shared && \
Comment on lines +351 to +353
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a custom --output_base=/tmp/bazel-output changes Bazel's workspace structure and may have unintended consequences. Each invocation with this custom output_base creates a separate Bazel workspace, which means build artifacts and dependencies won't be shared with the build at line 362 that uses the default output_base. This could lead to redundant downloads and builds. Additionally, using /tmp/bazel-output for multiple builds (lines 352 and 368) might cause conflicts if they're run in parallel or if the directory isn't cleaned between builds. Consider using unique output_base paths for each build, or ensure the builds are sequential and the directory is cleaned appropriately.

Copilot uses AI. Check for mistakes.
find /tmp/bazel-output -name libovms_shared.so && \
LIB_PATH=$(find /tmp/bazel-output -name libovms_shared.so | head -n1) && \
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The find command here could fail silently if libovms_shared.so is not found in the expected location. If the find returns no results, LIB_PATH will be empty, and the subsequent cp command will fail with a cryptic error message. Consider adding error checking after the find command to ensure the file was found, or use set -e to exit on errors. For example, add a check like: [ -n "$LIB_PATH" ] || { echo "Error: libovms_shared.so not found"; exit 1; }

Suggested change
LIB_PATH=$(find /tmp/bazel-output -name libovms_shared.so | head -n1) && \
LIB_PATH=$(find /tmp/bazel-output -name libovms_shared.so | head -n1) && \
[ -n "$LIB_PATH" ] || { echo "Error: libovms_shared.so not found in /tmp/bazel-output"; exit 1; } && \

Copilot uses AI. Check for mistakes.
echo "Found built file: $LIB_PATH" && \
mkdir -p /ovms_release/lib && \
cp -v "$LIB_PATH" /ovms_release/lib/

# C api app with bazel
# hadolint ignore=DL3059
RUN bazel build --jobs $JOBS ${CAPI_FLAGS} //src:capi_cpp_example
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This RUN command at line 362 does not use the custom --output_base=/tmp/bazel-output like the other bazel build commands in this stage (lines 352 and 368). According to the comments in this PR, Konflux has difficulty locating built binaries, which is why the custom output_base was introduced. For consistency and to avoid potential issues with Konflux, this build should also use the custom output_base approach, or there should be a comment explaining why this particular build doesn't need it.

Suggested change
RUN bazel build --jobs $JOBS ${CAPI_FLAGS} //src:capi_cpp_example
RUN mkdir -p /tmp/bazel-output && \
bazel --output_base=/tmp/bazel-output \
build --jobs=$JOBS ${CAPI_FLAGS} //src:capi_cpp_example

Copilot uses AI. Check for mistakes.

# C-API benchmark app
RUN bazel build --jobs=$JOBS ${CAPI_FLAGS} //src:capi_benchmark && ./bazel-bin/src/capi_benchmark --niter 2 --nstreams 1 --servable_name "dummy"
# Konflux has a hard time locating the built binaries. Need to put it
# in a known location.
RUN mkdir -p /tmp/bazel-output && \
bazel --output_base=/tmp/bazel-output \
build --jobs=$JOBS ${CAPI_FLAGS} //src:capi_benchmark && \
CAPI_BIN=$(find /tmp/bazel-output -type f -name capi_benchmark | head -n1) && \
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The find command here could fail silently if capi_benchmark is not found in the expected location. If the find returns no results, CAPI_BIN will be empty, and the subsequent chmod and execution will fail. Consider adding error checking after the find command to ensure the file was found, or use set -e to exit on errors. For example, add a check like: [ -n "$CAPI_BIN" ] || { echo "Error: capi_benchmark not found"; exit 1; }

Suggested change
CAPI_BIN=$(find /tmp/bazel-output -type f -name capi_benchmark | head -n1) && \
CAPI_BIN=$(find /tmp/bazel-output -type f -name capi_benchmark | head -n1) && \
[ -n "$CAPI_BIN" ] || { echo "Error: capi_benchmark not found"; exit 1; } && \

Copilot uses AI. Check for mistakes.
echo "Found built capi_benchmark: $CAPI_BIN" && \
chmod +x "$CAPI_BIN" && \
"$CAPI_BIN" --niter 2 --nstreams 1 --servable_name "dummy"

# C-api C/C++ app with gcc
COPY MakefileCapi /ovms/
RUN make -f MakefileCapi cpp CAPI_FLAGS="${CAPI_FLAGS}" && \
Expand Down