Conversation
The konflux build system has trouble in 2 areas. 1) It cuts of network access at certain times. The solution was to build everything in one shot. 2) Konflux causesbazel to do things differently which results in not being able to find libovms_shared.so or capi_benchmark. So, it was changed to try to put it in a known spot for searching. These changes do not affect the build done by Docker or podman.
There was a problem hiding this comment.
Pull request overview
This PR modifies the Dockerfile.redhat to accommodate Konflux build system constraints, specifically addressing issues with network access cutoffs and binary location detection during the build process.
Changes:
- Consolidated multiple bazel build commands into a single RUN instruction to work around Konflux network cutoffs
- Introduced explicit bazel output_base configuration for certain builds to ensure built binaries are placed in predictable locations
- Added logic to find, verify, and copy built artifacts (libovms_shared.so and capi_benchmark) from custom output locations
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # 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) |
There was a problem hiding this comment.
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.
| RUN mkdir -p /tmp/bazel-output && \ | ||
| bazel --output_base=/tmp/bazel-output \ | ||
| build --jobs=$JOBS ${CAPI_FLAGS} //src:ovms_shared && \ |
There was a problem hiding this comment.
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.
| bazel --output_base=/tmp/bazel-output \ | ||
| build --jobs=$JOBS ${CAPI_FLAGS} //src:ovms_shared && \ | ||
| find /tmp/bazel-output -name libovms_shared.so && \ | ||
| LIB_PATH=$(find /tmp/bazel-output -name libovms_shared.so | head -n1) && \ |
There was a problem hiding this comment.
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; }
| 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; } && \ |
| 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) && \ |
There was a problem hiding this comment.
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; }
| 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; } && \ |
| 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"] |
There was a problem hiding this comment.
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.
| SHELL ["/bin/bash", "-o", "pipefail", "-c"] | |
| SHELL ["/bin/bash", "-xo", "pipefail", "-c"] |
|
|
||
| # C api app with bazel | ||
| # hadolint ignore=DL3059 | ||
| RUN bazel build --jobs $JOBS ${CAPI_FLAGS} //src:capi_cpp_example |
There was a problem hiding this comment.
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.
| 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 |
2 participants
🛠 Summary
JIRA/Issue if applicable.
Describe the changes.
🧪 Checklist
``