From 56ba2d8e1d5c46fb8d99e6c680d9a3d50a92ee69 Mon Sep 17 00:00:00 2001 From: misiugodfrey Date: Thu, 7 May 2026 11:45:31 -0700 Subject: [PATCH 1/7] Bake provenance JSON into images and auto-populate post_results.py args Writes /opt/velox-testing/provenance.json into each Presto image at build time (both native_build.dockerfile and provenance_labels.dockerfile), so run_context.py can read it from the container filesystem in both Docker and SLURM/Enroot environments. run_context.py: reads the provenance file and merges presto_sha/branch/repo and velox_sha/branch/repo into the benchmark context dict. post_results.py: auto-populates --velox-branch, --velox-repo, --presto-branch, --presto-repo from the benchmark_result.json context section; CLI args still take precedence. BenchmarkMetadata gains 6 optional provenance fields. presto-build.yml: replaces the labels: input on the deps and coordinator build steps with a second provenance_labels.dockerfile wrapper step, so CI and local builds both write the provenance file (buildx labels: applies OCI metadata only and does not execute RUN steps). --- .github/workflows/presto-build.yml | 54 +++++++++++++------ benchmark_reporting_tools/post_results.py | 20 +++++-- .../performance_benchmarks/benchmark_keys.py | 6 +++ presto/docker/native_build.dockerfile | 5 ++ presto/docker/provenance_labels.dockerfile | 5 ++ .../performance_benchmarks/run_context.py | 23 ++++++++ 6 files changed, 93 insertions(+), 20 deletions(-) diff --git a/.github/workflows/presto-build.yml b/.github/workflows/presto-build.yml index 64179ab6..6f485087 100644 --- a/.github/workflows/presto-build.yml +++ b/.github/workflows/presto-build.yml @@ -130,7 +130,6 @@ jobs: cp -r velox/CMake presto-deps-context/velox/CMake - name: Build and push Presto deps image - id: push uses: docker/build-push-action@263435318d21b8e681c14492fe198d362a7d2c83 # v6.18.0 with: context: presto-deps-context @@ -138,13 +137,6 @@ jobs: build-args: | CUDA_VERSION=${{ matrix.cuda_version }} ARM_BUILD_TARGET=generic - labels: | - velox-testing.presto.sha=${{ inputs.presto_sha }} - velox-testing.presto.branch= - velox-testing.presto.repository=${{ inputs.presto_repository }} - velox-testing.velox.sha=${{ inputs.velox_sha }} - velox-testing.velox.branch= - velox-testing.velox.repository=${{ inputs.velox_repository }} cache-from: type=registry,ref=${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:cache-presto-deps-cuda${{ matrix.cuda_version }}-${{ matrix.arch }} cache-to: type=registry,ref=${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:cache-presto-deps-cuda${{ matrix.cuda_version }}-${{ matrix.arch }},mode=max outputs: type=registry,compression=zstd,force-compression=true,compression-level=15 @@ -152,6 +144,25 @@ jobs: tags: | ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:presto-deps-${{ inputs.presto_short_sha }}-velox-${{ inputs.velox_short_sha }}-cuda${{ matrix.cuda_version }}-${{ inputs.date }}-${{ matrix.arch }} + - name: Apply provenance labels to Presto deps image + id: push + uses: docker/build-push-action@263435318d21b8e681c14492fe198d362a7d2c83 # v6.18.0 + with: + context: velox-testing/presto/docker + file: velox-testing/presto/docker/provenance_labels.dockerfile + build-args: | + BASE_IMAGE=${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:presto-deps-${{ inputs.presto_short_sha }}-velox-${{ inputs.velox_short_sha }}-cuda${{ matrix.cuda_version }}-${{ inputs.date }}-${{ matrix.arch }} + PRESTO_SHA=${{ inputs.presto_sha }} + PRESTO_BRANCH= + PRESTO_REPOSITORY=${{ inputs.presto_repository }} + VELOX_SHA=${{ inputs.velox_sha }} + VELOX_BRANCH= + VELOX_REPOSITORY=${{ inputs.velox_repository }} + outputs: type=registry,compression=zstd,force-compression=true,compression-level=15 + push: true + tags: | + ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:presto-deps-${{ inputs.presto_short_sha }}-velox-${{ inputs.velox_short_sha }}-cuda${{ matrix.cuda_version }}-${{ inputs.date }}-${{ matrix.arch }} + - name: Generate artifact attestation uses: actions/attest-build-provenance@96278af6caaf10aea03fd8d33a09a777ca52d62f # v3.2.0 with: @@ -198,20 +209,31 @@ jobs: echo "tag=presto-coordinator-${PRESTO_SHORT_SHA}-${BUILD_DATE}-${ARCH}" >> "$GITHUB_OUTPUT" - name: Build and push Presto coordinator image - id: push uses: docker/build-push-action@263435318d21b8e681c14492fe198d362a7d2c83 # v6.18.0 with: context: presto/docker file: presto/docker/Dockerfile build-args: | PRESTO_VERSION=${{ inputs.presto_short_sha }} - labels: | - velox-testing.presto.sha=${{ inputs.presto_sha }} - velox-testing.presto.branch= - velox-testing.presto.repository=${{ inputs.presto_repository }} - velox-testing.velox.sha= - velox-testing.velox.branch= - velox-testing.velox.repository= + outputs: type=registry,compression=zstd,force-compression=true,compression-level=15 + push: true + tags: | + ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ steps.tag.outputs.tag }} + + - name: Apply provenance labels to Presto coordinator image + id: push + uses: docker/build-push-action@263435318d21b8e681c14492fe198d362a7d2c83 # v6.18.0 + with: + context: velox-testing/presto/docker + file: velox-testing/presto/docker/provenance_labels.dockerfile + build-args: | + BASE_IMAGE=${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ steps.tag.outputs.tag }} + PRESTO_SHA=${{ inputs.presto_sha }} + PRESTO_BRANCH= + PRESTO_REPOSITORY=${{ inputs.presto_repository }} + VELOX_SHA= + VELOX_BRANCH= + VELOX_REPOSITORY= outputs: type=registry,compression=zstd,force-compression=true,compression-level=15 push: true tags: | diff --git a/benchmark_reporting_tools/post_results.py b/benchmark_reporting_tools/post_results.py index 00602b47..916b603e 100644 --- a/benchmark_reporting_tools/post_results.py +++ b/benchmark_reporting_tools/post_results.py @@ -79,6 +79,12 @@ class BenchmarkMetadata: num_drivers: int | None = None gpu_name: str | None = None image_digest: str | None = None + presto_sha: str | None = None + presto_branch: str | None = None + presto_repo: str | None = None + velox_sha: str | None = None + velox_branch: str | None = None + velox_repo: str | None = None @classmethod def from_parsed(cls, raw: dict) -> "BenchmarkMetadata": @@ -285,22 +291,22 @@ def _parse_args() -> argparse.Namespace: parser.add_argument( "--velox-branch", default=None, - help="Velox branch used to build the worker image.", + help="Velox branch used to build the worker image. Defaults to value from image labels in benchmark_result.json.", ) parser.add_argument( "--velox-repo", default=None, - help="Velox repository used to build the worker image.", + help="Velox repository used to build the worker image. Defaults to value from image labels in benchmark_result.json.", ) parser.add_argument( "--presto-branch", default=None, - help="Presto branch used to build the worker image.", + help="Presto branch used to build the worker image. Defaults to value from image labels in benchmark_result.json.", ) parser.add_argument( "--presto-repo", default=None, - help="Presto repository used to build the worker image.", + help="Presto repository used to build the worker image. Defaults to value from image labels in benchmark_result.json.", ) parser.add_argument( "--concurrency-streams", @@ -625,6 +631,12 @@ async def _process_benchmark_dir( ) return 1 + # Fall back to image provenance labels captured in the benchmark context. + velox_branch = velox_branch or benchmark_metadata.velox_branch + velox_repo = velox_repo or benchmark_metadata.velox_repo + presto_branch = presto_branch or benchmark_metadata.presto_branch + presto_repo = presto_repo or benchmark_metadata.presto_repo + # Resolve config directory: explicit override → auto-detect from variant effective_config_dir = config_dir variant = _ENGINE_TO_VARIANT.get(benchmark_metadata.engine) diff --git a/common/testing/performance_benchmarks/benchmark_keys.py b/common/testing/performance_benchmarks/benchmark_keys.py index 90f1ce35..6290e07b 100644 --- a/common/testing/performance_benchmarks/benchmark_keys.py +++ b/common/testing/performance_benchmarks/benchmark_keys.py @@ -30,3 +30,9 @@ class BenchmarkKeys(str, Enum): KIND_KEY = "kind" GPU_COUNT_KEY = "gpu_count" DATASET_NAME_KEY = "dataset_name" + PRESTO_SHA_KEY = "presto_sha" + PRESTO_BRANCH_KEY = "presto_branch" + PRESTO_REPO_KEY = "presto_repo" + VELOX_SHA_KEY = "velox_sha" + VELOX_BRANCH_KEY = "velox_branch" + VELOX_REPO_KEY = "velox_repo" diff --git a/presto/docker/native_build.dockerfile b/presto/docker/native_build.dockerfile index a64f68a2..71bafbde 100644 --- a/presto/docker/native_build.dockerfile +++ b/presto/docker/native_build.dockerfile @@ -124,5 +124,10 @@ LABEL velox-testing.presto.sha=${PRESTO_SHA} \ velox-testing.velox.sha=${VELOX_SHA} \ velox-testing.velox.branch=${VELOX_BRANCH} \ velox-testing.velox.repository=${VELOX_REPOSITORY} +RUN mkdir -p /opt/velox-testing && \ + printf '{"presto_sha":"%s","presto_branch":"%s","presto_repo":"%s","velox_sha":"%s","velox_branch":"%s","velox_repo":"%s"}\n' \ + "${PRESTO_SHA}" "${PRESTO_BRANCH}" "${PRESTO_REPOSITORY}" \ + "${VELOX_SHA}" "${VELOX_BRANCH}" "${VELOX_REPOSITORY}" \ + > /opt/velox-testing/provenance.json CMD ["bash", "/opt/presto_profiling_wrapper.sh"] diff --git a/presto/docker/provenance_labels.dockerfile b/presto/docker/provenance_labels.dockerfile index 8e38a4f8..4651ea12 100644 --- a/presto/docker/provenance_labels.dockerfile +++ b/presto/docker/provenance_labels.dockerfile @@ -13,3 +13,8 @@ LABEL velox-testing.presto.sha=${PRESTO_SHA} \ velox-testing.velox.sha=${VELOX_SHA} \ velox-testing.velox.branch=${VELOX_BRANCH} \ velox-testing.velox.repository=${VELOX_REPOSITORY} +RUN mkdir -p /opt/velox-testing && \ + printf '{"presto_sha":"%s","presto_branch":"%s","presto_repo":"%s","velox_sha":"%s","velox_branch":"%s","velox_repo":"%s"}\n' \ + "${PRESTO_SHA}" "${PRESTO_BRANCH}" "${PRESTO_REPOSITORY}" \ + "${VELOX_SHA}" "${VELOX_BRANCH}" "${VELOX_REPOSITORY}" \ + > /opt/velox-testing/provenance.json diff --git a/presto/testing/performance_benchmarks/run_context.py b/presto/testing/performance_benchmarks/run_context.py index 3d6435db..6f02d00c 100644 --- a/presto/testing/performance_benchmarks/run_context.py +++ b/presto/testing/performance_benchmarks/run_context.py @@ -144,6 +144,13 @@ def _get_gpu_name_from_worker_logs() -> str | None: "java": "presto-java", } +_PROVENANCE_FILE = Path("/opt/velox-testing/provenance.json") + +_PROVENANCE_FIELDS = frozenset({ + "presto_sha", "presto_branch", "presto_repo", + "velox_sha", "velox_branch", "velox_repo", +}) + def _get_engine(hostname: str, port: int) -> str: """Determine worker engine type from the coordinator's cluster-tag. @@ -209,6 +216,20 @@ def _get_num_drivers() -> int | None: return None +def _get_image_provenance() -> dict: + """Return provenance fields from the baked-in file, or {} if absent or unreadable.""" + if not _PROVENANCE_FILE.is_file(): + return {} + try: + data = json.loads(_PROVENANCE_FILE.read_text()) + result = {k: v for k, v in data.items() if k in _PROVENANCE_FIELDS and v} + _debug(f"provenance from file: {result}") + return result + except Exception as e: + _debug(f"failed to read provenance file: {e}") + return {} + + def gather_run_context( hostname: str, port: int, @@ -252,4 +273,6 @@ def gather_run_context( # Always 1 for single-run invocations; reserved for future multi-execution support. ctx["execution_number"] = 1 + ctx.update(_get_image_provenance()) + return ctx From 7f2f3d0053b7a1d264c5b5bbf4f9c6f7303c32ba Mon Sep 17 00:00:00 2001 From: misiugodfrey Date: Thu, 7 May 2026 15:17:42 -0700 Subject: [PATCH 2/7] Fix InvalidDefaultArgInFrom warning in provenance_labels.dockerfile --- presto/docker/provenance_labels.dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/presto/docker/provenance_labels.dockerfile b/presto/docker/provenance_labels.dockerfile index 4651ea12..8de6a104 100644 --- a/presto/docker/provenance_labels.dockerfile +++ b/presto/docker/provenance_labels.dockerfile @@ -1,4 +1,4 @@ -ARG BASE_IMAGE +ARG BASE_IMAGE=invalid FROM ${BASE_IMAGE} ARG PRESTO_SHA From c1421c81aa3cb249ee733006dfb7ed499f3a3399 Mon Sep 17 00:00:00 2001 From: misiugodfrey Date: Tue, 19 May 2026 15:30:41 -0700 Subject: [PATCH 3/7] Surface image provenance via shared logs dir for host-side pytest The provenance.json baked into images at /opt/velox-testing/provenance.json is only visible from inside the container, but pytest runs on the host in the Docker flow (run_benchmark.sh:294) and inside the coord container in SLURM/Enroot. The host has no /opt/velox-testing/provenance.json, so run_context.py's _get_image_provenance() returned {} and the six provenance fields were silently missing from benchmark_result.json's context. Have each launch script copy the baked-in file into LOGS_DIR at startup (worker_provenance.json from launch_presto_servers.sh, coordinator_provenance.json from launch_coordinator.sh). In Docker the dir is bind-mounted to the host, so pytest reads it directly. In SLURM, add ${LOGS}:/opt/presto-server/logs to both coord and worker srun --container-mounts so the same scheme works across the cluster, and export LOGS_DIR=/opt/presto-server/logs in run_queries so the in-container pytest looks in the right place. run_context.py replaces the hardcoded path constant with a resolver that prefers worker_provenance.json (full presto+velox fields) over coordinator_provenance.json (presto only in CI builds), then falls back to the original /opt/velox-testing/provenance.json for callers running directly inside a container that has the file baked in. --- presto/docker/launch_coordinator.sh | 7 ++++ presto/docker/launch_presto_servers.sh | 6 ++++ presto/slurm/presto-nvl72/functions.sh | 18 +++++++++-- .../performance_benchmarks/run_context.py | 32 ++++++++++++++++--- 4 files changed, 56 insertions(+), 7 deletions(-) diff --git a/presto/docker/launch_coordinator.sh b/presto/docker/launch_coordinator.sh index 020ccef8..1e37fa59 100755 --- a/presto/docker/launch_coordinator.sh +++ b/presto/docker/launch_coordinator.sh @@ -7,6 +7,13 @@ set -e LOGS_DIR="/opt/presto-server/logs" mkdir -p "${LOGS_DIR}" : "${SERVER_START_TIMESTAMP:?SERVER_START_TIMESTAMP must be set before starting the container}" + +# Surface baked-in image provenance in the shared logs dir so the host-side +# pytest (Docker) or in-container pytest (SLURM) can read it via LOGS_DIR. +if [ -f /opt/velox-testing/provenance.json ]; then + cp /opt/velox-testing/provenance.json "${LOGS_DIR}/coordinator_provenance.json" +fi + log_file="${LOGS_DIR}/coordinator_${SERVER_START_TIMESTAMP}.log" exec /opt/presto-server/bin/launcher run >> "${log_file}" 2>&1 diff --git a/presto/docker/launch_presto_servers.sh b/presto/docker/launch_presto_servers.sh index 2494b499..1aa166e9 100644 --- a/presto/docker/launch_presto_servers.sh +++ b/presto/docker/launch_presto_servers.sh @@ -10,6 +10,12 @@ LOGS_DIR="/opt/presto-server/logs" mkdir -p "${LOGS_DIR}" : "${SERVER_START_TIMESTAMP:?SERVER_START_TIMESTAMP must be set before starting the container}" +# Surface baked-in image provenance in the shared logs dir so the host-side +# pytest (Docker) or in-container pytest (SLURM) can read it via LOGS_DIR. +if [ -f /opt/velox-testing/provenance.json ]; then + cp /opt/velox-testing/provenance.json "${LOGS_DIR}/worker_provenance.json" +fi + ETC_BASE="/opt/presto-server/etc" # Resolve the NUMA node for a worker and launch presto_server pinned to it. diff --git a/presto/slurm/presto-nvl72/functions.sh b/presto/slurm/presto-nvl72/functions.sh index 9a66009c..a56a89f1 100755 --- a/presto/slurm/presto-nvl72/functions.sh +++ b/presto/slurm/presto-nvl72/functions.sh @@ -109,7 +109,8 @@ ${CONFIGS}/etc_coordinator/node.properties:/opt/presto-server/etc/node.propertie ${CONFIGS}/etc_coordinator/config_native.properties:/opt/presto-server/etc/config.properties,\ ${CONFIGS}/etc_coordinator/catalog/hive.properties:/opt/presto-server/etc/catalog/hive.properties,\ ${DATA}:/var/lib/presto/data/hive/data/user_data,\ -${VT_ROOT}/.hive_metastore:/var/lib/presto/data/hive/metastore${extra_mounts} \ +${VT_ROOT}/.hive_metastore:/var/lib/presto/data/hive/metastore,\ +${LOGS}:/opt/presto-server/logs${extra_mounts} \ -- bash -lc "unset JAVA_HOME; export JAVA_HOME=/usr/lib/jvm/jre-17-openjdk; export PATH=/usr/lib/jvm/jre-17-openjdk/bin:\$PATH; ${script}" >> ${LOGS}/${log_file} 2>&1 & else srun -w $COORD --ntasks=1 --overlap \ @@ -123,7 +124,8 @@ ${CONFIGS}/etc_coordinator/node.properties:/opt/presto-server/etc/node.propertie ${CONFIGS}/etc_coordinator/config_native.properties:/opt/presto-server/etc/config.properties,\ ${CONFIGS}/etc_coordinator/catalog/hive.properties:/opt/presto-server/etc/catalog/hive.properties,\ ${DATA}:/var/lib/presto/data/hive/data/user_data,\ -${VT_ROOT}/.hive_metastore:/var/lib/presto/data/hive/metastore${extra_mounts} \ +${VT_ROOT}/.hive_metastore:/var/lib/presto/data/hive/metastore,\ +${LOGS}:/opt/presto-server/logs${extra_mounts} \ -- bash -lc "unset JAVA_HOME; export JAVA_HOME=/usr/lib/jvm/jre-17-openjdk; export PATH=/usr/lib/jvm/jre-17-openjdk/bin:\$PATH; ${script}" >> ${LOGS}/${log_file} 2>&1 fi } @@ -146,6 +148,13 @@ unset CONFIG NODE_CONFIG PRESTO_ETC JAVA_TOOL_OPTIONS JDK_JAVA_OPTIONS _JAVA_OPT export JAVA_HOME=/usr export PATH=/usr/bin:$PATH + +# Surface baked-in image provenance into the shared logs dir for the in-container +# pytest in run_benchmark.sh to read via LOGS_DIR. +if [ -f /opt/velox-testing/provenance.json ]; then + cp /opt/velox-testing/provenance.json "/opt/presto-server/logs/coordinator_provenance.json" +fi + /opt/presto-server/bin/launcher run & srv=$! # wait for JVM to appear @@ -237,12 +246,16 @@ ${worker_hive}:/opt/presto-server/etc/catalog/hive.properties,\ ${worker_data}:/var/lib/presto/data,\ ${DATA}:/var/lib/presto/data/hive/data/user_data,\ ${VT_ROOT}/.hive_metastore:/var/lib/presto/data/hive/metastore,\ +${LOGS}:/opt/presto-server/logs,\ /usr/lib/aarch64-linux-gnu/libcuda.so.580.105.08:/usr/local/cuda-13.0/compat/libcuda.so.1,\ /usr/lib/aarch64-linux-gnu/libnvidia-ml.so.580.105.08:/usr/local/lib/libnvidia-ml.so.1 \ -- /bin/bash -c " export LD_LIBRARY_PATH='${CUDF_LIB}':/usr/local/lib:\${LD_LIBRARY_PATH:-} if [[ '${VARIANT_TYPE}' == 'gpu' ]]; then export CUDA_VISIBLE_DEVICES=${gpu_id}; fi echo \"Worker ${worker_id}: CUDA_VISIBLE_DEVICES=\${CUDA_VISIBLE_DEVICES:-none}, NUMA_NODE=${numa_node}\" +if [ -f /opt/velox-testing/provenance.json ]; then + cp /opt/velox-testing/provenance.json /opt/presto-server/logs/worker_provenance.json +fi if [[ '${USE_NUMA}' == '1' ]]; then numactl --cpubind=${numa_node} --membind=${numa_node} /usr/bin/presto_server --etc-dir=/opt/presto-server/etc else @@ -364,6 +377,7 @@ function run_queries { export PRESTO_DATA_DIR=/var/lib/presto/data/hive/data/user_data; \ export MINIFORGE_HOME=/workspace/miniforge3; \ export HOME=/workspace; \ + export LOGS_DIR=/opt/presto-server/logs; \ cd /workspace/presto/scripts; \ ./run_benchmark.sh -b tpch -s tpchsf${scale_factor} -i ${num_iterations} \ --hostname ${COORD} --port $PORT -o /workspace/presto/slurm/presto-nvl72/result_dir --skip-drop-cache" "cli" diff --git a/presto/testing/performance_benchmarks/run_context.py b/presto/testing/performance_benchmarks/run_context.py index 6f02d00c..1794a36e 100644 --- a/presto/testing/performance_benchmarks/run_context.py +++ b/presto/testing/performance_benchmarks/run_context.py @@ -144,14 +144,35 @@ def _get_gpu_name_from_worker_logs() -> str | None: "java": "presto-java", } -_PROVENANCE_FILE = Path("/opt/velox-testing/provenance.json") - _PROVENANCE_FIELDS = frozenset({ "presto_sha", "presto_branch", "presto_repo", "velox_sha", "velox_branch", "velox_repo", }) +def _provenance_file_path() -> Path | None: + """Locate the provenance JSON file. + + The worker/coordinator startup scripts copy /opt/velox-testing/provenance.json + into the shared logs dir as ``worker_provenance.json`` / ``coordinator_provenance.json``. + In Docker the dir is bind-mounted to the host; in SLURM/Enroot both the worker + container and the pytest-running coord container mount ${LOGS} at the same path. + + Prefer the worker file (full presto+velox fields) over the coordinator (presto + only). Fall back to the in-image path for callers running directly inside a + container that has the file baked in. + """ + logs_dir = os.environ.get("LOGS_DIR") + if logs_dir: + ld = Path(logs_dir) + for name in ("worker_provenance.json", "coordinator_provenance.json"): + candidate = ld / name + if candidate.is_file(): + return candidate + fallback = Path("/opt/velox-testing/provenance.json") + return fallback if fallback.is_file() else None + + def _get_engine(hostname: str, port: int) -> str: """Determine worker engine type from the coordinator's cluster-tag. @@ -218,12 +239,13 @@ def _get_num_drivers() -> int | None: def _get_image_provenance() -> dict: """Return provenance fields from the baked-in file, or {} if absent or unreadable.""" - if not _PROVENANCE_FILE.is_file(): + provenance_file = _provenance_file_path() + if provenance_file is None: return {} try: - data = json.loads(_PROVENANCE_FILE.read_text()) + data = json.loads(provenance_file.read_text()) result = {k: v for k, v in data.items() if k in _PROVENANCE_FIELDS and v} - _debug(f"provenance from file: {result}") + _debug(f"provenance from {provenance_file}: {result}") return result except Exception as e: _debug(f"failed to read provenance file: {e}") From bd8831dd8093874684507ae943e29c87b45f1397 Mon Sep 17 00:00:00 2001 From: misiugodfrey Date: Tue, 21 Jul 2026 12:13:02 -0400 Subject: [PATCH 4/7] Fix deps provenance step to reference the pushed image tag The 'Apply provenance labels to Presto deps image' step reconstructed the deps image tag by hand but omitted the build_variant and run_id segments that the build-and-push step includes, so it resolved a tag that was never pushed (buildx: 'not found'). Align BASE_IMAGE and the output tag with the deps build tag. --- .github/workflows/presto-build.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/presto-build.yml b/.github/workflows/presto-build.yml index 00ad1b26..9fdcbbc4 100644 --- a/.github/workflows/presto-build.yml +++ b/.github/workflows/presto-build.yml @@ -139,7 +139,7 @@ jobs: context: velox-testing/presto/docker file: velox-testing/presto/docker/provenance_labels.dockerfile build-args: | - BASE_IMAGE=${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:presto-deps-${{ inputs.presto_short_sha }}-velox-${{ inputs.velox_short_sha }}-cuda${{ matrix.cuda_version }}-${{ inputs.date }}-${{ matrix.arch }} + BASE_IMAGE=${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:presto-deps-${{ inputs.presto_short_sha }}-velox-${{ inputs.velox_short_sha }}-cuda${{ matrix.cuda_version }}-${{ inputs.date }}-${{ inputs.build_variant }}-${{ github.run_id }}-${{ matrix.arch }} PRESTO_SHA=${{ inputs.presto_sha }} PRESTO_BRANCH= PRESTO_REPOSITORY=${{ inputs.presto_repository }} @@ -149,7 +149,7 @@ jobs: outputs: type=registry,compression=zstd,force-compression=true,compression-level=15 push: true tags: | - ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:presto-deps-${{ inputs.presto_short_sha }}-velox-${{ inputs.velox_short_sha }}-cuda${{ matrix.cuda_version }}-${{ inputs.date }}-${{ matrix.arch }} + ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:presto-deps-${{ inputs.presto_short_sha }}-velox-${{ inputs.velox_short_sha }}-cuda${{ matrix.cuda_version }}-${{ inputs.date }}-${{ inputs.build_variant }}-${{ github.run_id }}-${{ matrix.arch }} - name: Generate artifact attestation uses: actions/attest-build-provenance@96278af6caaf10aea03fd8d33a09a777ca52d62f # v3.2.0 From 59527a91b3bf8b09d1dc02ce5932d0a51b0efe36 Mon Sep 17 00:00:00 2001 From: misiugodfrey Date: Tue, 21 Jul 2026 20:56:46 -0400 Subject: [PATCH 5/7] Submit presto/velox SHAs as separate entries and derive commit_hash Add presto_sha/velox_sha as their own engine_config entries alongside the existing branch/repo fields, sourced from the baked image provenance in the benchmark context. Provenance fields are now emitted individually with empties omitted, so a coordinator-only image contributes just its presto entries. Derive query_engine.commit_hash from the SHAs as 'presto--velox-' (or 'presto-' when velox is absent) when --commit-hash is not given; explicit --commit-hash still wins, and it stays 'unknown' when no SHAs exist. Degrades gracefully on older result files with no provenance. --- benchmark_reporting_tools/post_results.py | 37 +++++++++++++++++------ 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/benchmark_reporting_tools/post_results.py b/benchmark_reporting_tools/post_results.py index a12eed8b..2c7de46b 100644 --- a/benchmark_reporting_tools/post_results.py +++ b/benchmark_reporting_tools/post_results.py @@ -393,8 +393,10 @@ def _build_submission_payload( validation_results: dict | None = None, velox_branch: str | None = None, velox_repo: str | None = None, + velox_sha: str | None = None, presto_branch: str | None = None, presto_repo: str | None = None, + presto_sha: str | None = None, labels: list[str] | None = None, notes: str | None = None, ) -> dict: @@ -419,8 +421,15 @@ def _build_submission_payload( # Use placeholders for version info if not provided if version is None: version = "unknown" + # Derive commit_hash from the image provenance SHAs when not explicitly given: + # "presto--velox-" (worker), "presto-" (coordinator, no velox). if commit_hash is None: - commit_hash = "unknown" + commit_parts = [] + if presto_sha: + commit_parts.append(f"presto-{presto_sha}") + if velox_sha: + commit_parts.append(f"velox-{velox_sha}") + commit_hash = "-".join(commit_parts) if commit_parts else "unknown" # Build query logs from results query_logs = [] @@ -510,14 +519,19 @@ def _get_validation_result(query_name): } engine_config_payload = engine_config.serialize() if engine_config else {} - if velox_branch or velox_repo or presto_branch or presto_repo: - engine_config_payload = { - **engine_config_payload, - "velox_branch": velox_branch, - "velox_repo": velox_repo, - "presto_branch": presto_branch, - "presto_repo": presto_repo, - } + # Add image provenance as separate entries; omit empties so a coordinator-only + # image (no velox fields) contributes just its presto entries. + provenance_fields = { + "presto_branch": presto_branch, + "presto_sha": presto_sha, + "presto_repo": presto_repo, + "velox_branch": velox_branch, + "velox_sha": velox_sha, + "velox_repo": velox_repo, + } + provenance_fields = {k: v for k, v in provenance_fields.items() if v} + if provenance_fields: + engine_config_payload = {**engine_config_payload, **provenance_fields} payload: dict = { "sku_name": sku_name, @@ -757,6 +771,9 @@ async def _process_benchmark_dir( velox_repo = velox_repo or benchmark_metadata.velox_repo presto_branch = presto_branch or benchmark_metadata.presto_branch presto_repo = presto_repo or benchmark_metadata.presto_repo + # SHAs come only from the baked image provenance (no CLI override). + velox_sha = benchmark_metadata.velox_sha + presto_sha = benchmark_metadata.presto_sha # Resolve config directory: explicit override → auto-detect from variant effective_config_dir = config_dir @@ -861,8 +878,10 @@ async def _process_benchmark_dir( validation_results=validation_results, velox_branch=velox_branch, velox_repo=velox_repo, + velox_sha=velox_sha, presto_branch=presto_branch, presto_repo=presto_repo, + presto_sha=presto_sha, labels=labels, notes=notes, ) From 849d39c2ad26b2b51f9c708b412e9081e67b5871 Mon Sep 17 00:00:00 2001 From: misiugodfrey Date: Tue, 21 Jul 2026 21:20:32 -0400 Subject: [PATCH 6/7] Extract apply-provenance-labels composite action The deps and coordinator provenance wrapper steps were near-identical build-push-action invocations differing only in base image, tags, and the velox fields. Extract them into .github/actions/apply-provenance-labels so the pinned action SHA, provenance dockerfile wiring, and outputs live in one place (mirroring the repo's existing container-registry-login/delete-arch-tags composite actions), removing the risk of the two steps drifting. Coordinator omits the velox_* inputs (default empty), preserving its presto-only provenance. Behaviour is unchanged. --- .../apply-provenance-labels/action.yml | 61 +++++++++++++++++++ .github/workflows/presto-build.yml | 42 ++++--------- 2 files changed, 73 insertions(+), 30 deletions(-) create mode 100644 .github/actions/apply-provenance-labels/action.yml diff --git a/.github/actions/apply-provenance-labels/action.yml b/.github/actions/apply-provenance-labels/action.yml new file mode 100644 index 00000000..cf5753c0 --- /dev/null +++ b/.github/actions/apply-provenance-labels/action.yml @@ -0,0 +1,61 @@ +name: Apply provenance labels +description: >- + Re-tag an already-built image through provenance_labels.dockerfile so it + carries the velox-testing provenance labels and baked /opt/velox-testing/provenance.json. + +inputs: + base_image: + description: Fully-qualified reference of the already-built image to layer provenance onto. + required: true + tags: + description: Tag(s) to push the provenance-labelled image under (newline-separated). + required: true + presto_sha: + description: Presto commit SHA. + required: true + presto_repository: + description: Presto repository. + required: false + default: "" + presto_branch: + description: Presto branch (empty on CI, which knows only the SHA). + required: false + default: "" + velox_sha: + description: Velox commit SHA (empty for coordinator images, which do not build Velox). + required: false + default: "" + velox_repository: + description: Velox repository. + required: false + default: "" + velox_branch: + description: Velox branch (empty on CI). + required: false + default: "" + +outputs: + digest: + description: Digest of the pushed provenance-labelled image. + value: ${{ steps.push.outputs.digest }} + +runs: + using: composite + steps: + - name: Build and push provenance-labelled image + id: push + uses: docker/build-push-action@263435318d21b8e681c14492fe198d362a7d2c83 # v6.18.0 + with: + context: velox-testing/presto/docker + file: velox-testing/presto/docker/provenance_labels.dockerfile + build-args: | + BASE_IMAGE=${{ inputs.base_image }} + PRESTO_SHA=${{ inputs.presto_sha }} + PRESTO_BRANCH=${{ inputs.presto_branch }} + PRESTO_REPOSITORY=${{ inputs.presto_repository }} + VELOX_SHA=${{ inputs.velox_sha }} + VELOX_BRANCH=${{ inputs.velox_branch }} + VELOX_REPOSITORY=${{ inputs.velox_repository }} + outputs: type=registry,compression=zstd,force-compression=true,compression-level=15 + push: true + tags: ${{ inputs.tags }} diff --git a/.github/workflows/presto-build.yml b/.github/workflows/presto-build.yml index 9fdcbbc4..b374f18f 100644 --- a/.github/workflows/presto-build.yml +++ b/.github/workflows/presto-build.yml @@ -134,22 +134,14 @@ jobs: - name: Apply provenance labels to Presto deps image id: push - uses: docker/build-push-action@263435318d21b8e681c14492fe198d362a7d2c83 # v6.18.0 + uses: ./velox-testing/.github/actions/apply-provenance-labels with: - context: velox-testing/presto/docker - file: velox-testing/presto/docker/provenance_labels.dockerfile - build-args: | - BASE_IMAGE=${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:presto-deps-${{ inputs.presto_short_sha }}-velox-${{ inputs.velox_short_sha }}-cuda${{ matrix.cuda_version }}-${{ inputs.date }}-${{ inputs.build_variant }}-${{ github.run_id }}-${{ matrix.arch }} - PRESTO_SHA=${{ inputs.presto_sha }} - PRESTO_BRANCH= - PRESTO_REPOSITORY=${{ inputs.presto_repository }} - VELOX_SHA=${{ inputs.velox_sha }} - VELOX_BRANCH= - VELOX_REPOSITORY=${{ inputs.velox_repository }} - outputs: type=registry,compression=zstd,force-compression=true,compression-level=15 - push: true - tags: | - ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:presto-deps-${{ inputs.presto_short_sha }}-velox-${{ inputs.velox_short_sha }}-cuda${{ matrix.cuda_version }}-${{ inputs.date }}-${{ inputs.build_variant }}-${{ github.run_id }}-${{ matrix.arch }} + base_image: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:presto-deps-${{ inputs.presto_short_sha }}-velox-${{ inputs.velox_short_sha }}-cuda${{ matrix.cuda_version }}-${{ inputs.date }}-${{ inputs.build_variant }}-${{ github.run_id }}-${{ matrix.arch }} + tags: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:presto-deps-${{ inputs.presto_short_sha }}-velox-${{ inputs.velox_short_sha }}-cuda${{ matrix.cuda_version }}-${{ inputs.date }}-${{ inputs.build_variant }}-${{ github.run_id }}-${{ matrix.arch }} + presto_sha: ${{ inputs.presto_sha }} + presto_repository: ${{ inputs.presto_repository }} + velox_sha: ${{ inputs.velox_sha }} + velox_repository: ${{ inputs.velox_repository }} - name: Generate artifact attestation uses: actions/attest-build-provenance@96278af6caaf10aea03fd8d33a09a777ca52d62f # v3.2.0 @@ -211,22 +203,12 @@ jobs: - name: Apply provenance labels to Presto coordinator image id: push - uses: docker/build-push-action@263435318d21b8e681c14492fe198d362a7d2c83 # v6.18.0 + uses: ./velox-testing/.github/actions/apply-provenance-labels with: - context: velox-testing/presto/docker - file: velox-testing/presto/docker/provenance_labels.dockerfile - build-args: | - BASE_IMAGE=${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ steps.tag.outputs.tag }} - PRESTO_SHA=${{ inputs.presto_sha }} - PRESTO_BRANCH= - PRESTO_REPOSITORY=${{ inputs.presto_repository }} - VELOX_SHA= - VELOX_BRANCH= - VELOX_REPOSITORY= - outputs: type=registry,compression=zstd,force-compression=true,compression-level=15 - push: true - tags: | - ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ steps.tag.outputs.tag }} + base_image: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ steps.tag.outputs.tag }} + tags: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ steps.tag.outputs.tag }} + presto_sha: ${{ inputs.presto_sha }} + presto_repository: ${{ inputs.presto_repository }} - name: Generate artifact attestation uses: actions/attest-build-provenance@96278af6caaf10aea03fd8d33a09a777ca52d62f # v3.2.0 From 6aa30552dca33fde9d13261be5ef74203753f8ad Mon Sep 17 00:00:00 2001 From: misiugodfrey Date: Wed, 22 Jul 2026 14:26:41 -0400 Subject: [PATCH 7/7] Apply ruff format to _PROVENANCE_FIELDS ruff format (v0.14.x) expands the frozenset literal to one element per line; run the formatter to satisfy the check-style pre-commit hook. --- .../testing/performance_benchmarks/run_context.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/presto/testing/performance_benchmarks/run_context.py b/presto/testing/performance_benchmarks/run_context.py index da60f243..fd540700 100644 --- a/presto/testing/performance_benchmarks/run_context.py +++ b/presto/testing/performance_benchmarks/run_context.py @@ -153,10 +153,16 @@ def _get_gpu_name_from_worker_logs() -> str | None: "java": "presto-java", } -_PROVENANCE_FIELDS = frozenset({ - "presto_sha", "presto_branch", "presto_repo", - "velox_sha", "velox_branch", "velox_repo", -}) +_PROVENANCE_FIELDS = frozenset( + { + "presto_sha", + "presto_branch", + "presto_repo", + "velox_sha", + "velox_branch", + "velox_repo", + } +) def _provenance_file_path() -> Path | None: