Conversation
The controller-protocol.version marker was only added to the Gradle buildZip packaging path, but the Linux artifacts consumed by CI and DRA are produced by dev-tools/docker/docker_entrypoint.sh (platform zip) and .buildkite/scripts/steps/create_dra.sh (-deps/-nodeps split), neither of which staged the marker. As a result the -nodeps bundle lacked the file and Elasticsearch's new verifyControllerProtocolVersion gate rejected it, failing the nightly PyTorch build's triggered Java integration tests. Stage the marker at the bundle root in the Docker packaging step and add it to the create_dra.sh -nodeps include list, with guarded fast-fail checks in both paths so a future omission fails loudly at packaging time rather than as an opaque downstream Elasticsearch build failure. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Pinging @elastic/ml-core (Team:ML) |
… helper Both the Linux Docker packaging path (dev-tools/docker/docker_entrypoint.sh) and the DRA assembly step (.buildkite/scripts/steps/create_dra.sh) duplicated the same unzip/grep fast-fail check for the controller-protocol.version marker. Extract it into dev-tools/verify_controller_protocol_version.sh and source it from both, keeping the unzip-availability guard in one place. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Critical issues remain in the DRA all-platform archive and marker verification pipeline.
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
Adds controller-protocol.version packaging and validation for Linux and DRA artifacts.
Changes:
- Adds shared archive marker verification.
- Stages the marker in Linux bundles.
- Includes and validates it in DRA packaging.
| File | Summary |
|---|---|
dev-tools/verify_controller_protocol_version.sh |
Validates the controller protocol marker in archives; the pipefail/SIGPIPE interaction needs correction. |
dev-tools/docker/docker_entrypoint.sh |
Stages the marker in Docker-built Linux artifacts. |
.buildkite/scripts/steps/create_dra.sh |
Adds the marker to split bundles, but the all-platform archive still omits it. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- create_dra.sh: include controller-protocol.version in the all-platform ml-cpp-<version>.zip so the DRA uber artifact carries the marker too, matching the Gradle buildUberZip task, and verify it alongside -nodeps. - verify_controller_protocol_version.sh: capture the unzip listing and match it via a here-string instead of piping into 'grep -q'. Under set -o pipefail grep could close the pipe early and SIGPIPE unzip, making a present marker be reported as missing. Co-authored-by: Cursor <cursoragent@cursor.com>
edsavage
enabled auto-merge (squash)
September 27, 2026 22:48
edsavage
disabled auto-merge
September 27, 2026 22:48
This was referenced Sep 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
controller-protocol.versionmarker was only wired into the GradlebuildZippackaging path, but the Linux artifacts that CI and DRA actually produce/consume are built bydev-tools/docker/docker_entrypoint.sh(the platform zip) and.buildkite/scripts/steps/create_dra.sh(the-deps/-nodepssplit) — neither of which staged the marker. As a result the resolved-nodepsbundle contained no marker, and Elasticsearch's newverifyControllerProtocolVersiongate (from elastic/elasticsearch#159052) rejected it:This surfaced in the nightly
ml-cpp-pytorch-builds#832, whose triggeredml-cpp-pr-builds#3252 failed all three Java integration steps (multi-node, YAML REST, inference) on this gate.ml-cpp-snapshot-buildsand DRA/staging stay green only because they don't run the Elasticsearch Java integration tests — but they were still publishing marker-less-nodeps/-depsbundles, so this fixes the shipped artifacts too.Changes
dev-tools/docker/docker_entrypoint.sh: stage3rd_party/controller-protocol.versionat the bundle root before zipping (parity with the GradlebuildZiptask).set -emakes a missing marker source fail here..buildkite/scripts/steps/create_dra.sh: addcontroller-protocol.versionto the-nodepsinclude list so the marker ships alongside the controller it describes.Test plan
ml-cpp-pr-builds(the Java integration steps that runverifyControllerProtocolVersionnow pass)ml-cpp-<version>-linux-x86_64.zipandml-cpp-<version>-nodeps.zipcontaincontroller-protocol.version(unzip -l ... | grep controller-protocol.version)Verified against the artifacts from ml-cpp-pr-builds #3255:
ml-cpp-9.6.0-SNAPSHOT-linux-aarch64.zip—unzip -lshowscontroller-protocol.version(30 bytes) at the zip root.ml-cpp-9.6.0-SNAPSHOT-linux-x86_64.zip— contains thecontroller-protocol.versionentry.ml-cpp-<version>.zipand-nodeps.zipfor the ES integration tests (run_es_tests.sh), so the passingverifyControllerProtocolVersiongate validated exactly this artifact. The dedicatedcreate_dra.sh-nodeps/uber assembly runs in the snapshot/DRA pipeline and is now guarded by the new fast-fail marker checks.Made with Cursor