Skip to content
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
4f23c7b
Init
Jul 4, 2025
48dff13
Temporaryily remove all other workflows for faster testing
Jul 4, 2025
9bd9207
Try fix
Jul 4, 2025
4997a02
Merge branch 'release-0.293-clp-connector' into xwei/add-release-imag…
anlowee Jul 7, 2025
d74b885
Fix
Jul 7, 2025
4815351
Fix
Jul 7, 2025
e7fe20c
Still build the runtime image when updating the PR, but only publish …
Jul 7, 2025
fc2cc18
Build worker image
Jul 8, 2025
cc2c510
Merge branch 'release-0.293-clp-connector' into xwei/add-release-imag…
anlowee Jul 8, 2025
60dd19d
Restore workflows
Jul 9, 2025
a534bec
Remove the uncessary action
Jul 10, 2025
9e17bc0
Use docker build-and-push action
Jul 14, 2025
aeb4e48
Merge branch 'release-0.293-clp-connector' into xwei/add-release-imag…
anlowee Jul 14, 2025
dbb28fa
Fix
Jul 14, 2025
1e69681
Fix
Jul 14, 2025
4ca014c
Fix
Jul 14, 2025
fec665a
Fix
Jul 14, 2025
20a7054
Merge branch 'release-0.293-clp-connector' into xwei/add-release-imag…
anlowee Jul 15, 2025
283b751
Fix
Jul 15, 2025
64983f0
Merge branch 'xwei/add-release-image-publish-workflow' of github.com:…
Jul 15, 2025
a6ebbaa
Fix
Jul 15, 2025
5ca0420
Apply suggestions from code review
anlowee Jul 18, 2025
fa06d23
Address comments
Jul 18, 2025
d120672
Merge branch 'release-0.293-clp-connector' into xwei/add-release-imag…
anlowee Jul 18, 2025
4c90aad
Merge branch 'release-0.293-clp-connector' into xwei/add-release-imag…
anlowee Jul 18, 2025
3b76c12
Address comments
Jul 21, 2025
b55aeda
Merge branch 'xwei/add-release-image-publish-workflow' of github.com:…
Jul 21, 2025
74787c7
Remove TODO
Jul 21, 2025
313a648
Address comments
Jul 21, 2025
47b9c10
Merge branch 'release-0.293-clp-connector' into xwei/add-release-imag…
anlowee Jul 21, 2025
ceaa809
Minor refactoring.
kirkrodrigues Jul 21, 2025
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
66 changes: 66 additions & 0 deletions .github/workflows/maven-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,71 @@ jobs:
run: |
export MAVEN_OPTS="${MAVEN_INSTALL_OPTS}"
./mvnw install -B -V -T 1C -DskipTests -Dmaven.javadoc.skip=true --no-transfer-progress -P ci -pl '!presto-test-coverage,!:presto-docs'
- name: Upload presto-server
if: matrix.java == '8.0.442'
uses: actions/upload-artifact@v4
with:
name: presto-server
path: presto-server/target/presto-server-0.293.tar.gz
if-no-files-found: "error"
retention-days: 1
- name: Upload presto-cli
if: matrix.java == '8.0.442'
uses: actions/upload-artifact@v4
with:
name: presto-cli
path: presto-cli/target/presto-cli-0.293-executable.jar
if-no-files-found: "error"
retention-days: 1
- name: Clean Maven Output
run: ./mvnw clean -pl '!:presto-server,!:presto-cli,!presto-test-coverage'
presto-coordinator-image:
name: presto-coordinator-image
needs: maven-checks
runs-on: ubuntu-22.04
steps:
- uses: "actions/checkout@v4"
with:
submodules: "recursive"

- name: Download presto-server
uses: actions/download-artifact@v4
with:
name: presto-server
path: ./docker

- name: Download presto-cli
uses: actions/download-artifact@v4
with:
name: presto-cli
path: ./docker

- name: "Login to image registry"
uses: "docker/login-action@v3"
with:
registry: ghcr.io
username: ${{github.actor}}
password: ${{secrets.GITHUB_TOKEN}}

- name: "Set up container image metadata"
id: "meta"
uses: "docker/metadata-action@v5"
with:
images: >-
ghcr.io/${{github.repository}}/coordinator
tags: |-
type=raw,value=dev

- name: "Build and push"
uses: "docker/build-push-action@v6"
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Possible GHCR push failure with mixed-case repo name

${{ github.repository }} keeps capital letters; GHCR requires lower-case paths. Convert to lower-case (e.g., ${{ github.repository }} | toLower).

🤖 Prompt for AI Agents
In .github/workflows/maven-checks.yml around lines 100 to 106, the usage of `${{
github.repository }}` in the image path may cause GHCR push failures because it
retains uppercase letters, while GHCR requires lowercase paths. Fix this by
converting `${{ github.repository }}` to lowercase using the `toLower` function,
ensuring the image path is fully lowercase.

with:
build-args: |-
JMX_PROMETHEUS_JAVA_AGENT_VERSION=0.20.0
PRESTO_VERSION=0.293
context: "./docker/"
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Build args repeat the hard-coded version

PRESTO_VERSION=0.293 duplicates the literal again. Once you introduce a workflow-level PRESTO_VERSION, reference it here instead.

-            PRESTO_VERSION=0.293
+            PRESTO_VERSION=${{ env.PRESTO_VERSION }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
build-args: |-
JMX_PROMETHEUS_JAVA_AGENT_VERSION=0.20.0
PRESTO_VERSION=0.293
context: "./docker/"
build-args: |-
JMX_PROMETHEUS_JAVA_AGENT_VERSION=0.20.0
PRESTO_VERSION=${{ env.PRESTO_VERSION }}
context: "./docker/"
🤖 Prompt for AI Agents
In .github/workflows/maven-checks.yml around lines 108 to 111, the build
argument PRESTO_VERSION is hard-coded as 0.293, duplicating the version literal.
Replace this hard-coded value with a reference to the workflow-level
PRESTO_VERSION variable to avoid repetition and improve maintainability.

file: "./docker/Dockerfile"
push: >-
${{github.event_name != 'pull_request'
&& github.ref == 'refs/heads/release-0.293-clp-connector'}}
tags: "${{steps.meta.outputs.tags}}"
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Consider skipping the heavy Docker build on pull-requests
You already guard the push, but the image is still fully built during PRs, adding ~5-10 min to CI. Add the same condition to the build step or wrap the whole job in if: github.event_name == 'push' && github.ref == 'refs/heads/release-0.293-clp-connector'.

🤖 Prompt for AI Agents
In .github/workflows/maven-checks.yml around lines 104 to 115, the Docker image
build runs during pull requests, causing unnecessary CI delays. To fix this, add
a condition to skip the build step during pull requests by adding an if clause
that checks if the event is a push to the release-0.293-clp-connector branch, or
wrap the entire job with this condition. This ensures the heavy Docker build
only runs on relevant pushes, not on PRs.

labels: "${{steps.meta.outputs.labels}}"
72 changes: 72 additions & 0 deletions .github/workflows/prestissimo-worker-images-build.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
name: prestissimo-worker-images-build

on:
pull_request:
push:

jobs:
prestissimo-worker-images-build:
name: prestissimo-worker-images-build
runs-on: ubuntu-22.04
steps:
- uses: "actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683"
with:
submodules: "recursive"

- name: "Login to image registry"
uses: "docker/login-action@74a5d142397b4f367a81961eba4e8cd7edddf772"
with:
Comment on lines +12 to +18
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Mixed pinning style: commit SHAs here, version tags elsewhere

This workflow pins actions to SHAs while maven-checks.yml uses version tags (@v4, @v5). Pick one convention (preferably SHA pinning) across all workflows for consistency and supply-chain safety.

🤖 Prompt for AI Agents
In .github/workflows/prestissimo-worker-images-build.yml around lines 12 to 18,
the actions are pinned using commit SHAs while other workflows use version tags.
To maintain consistency and improve supply-chain security, update all action
references in this workflow to use commit SHAs instead of version tags, matching
the preferred SHA pinning style used elsewhere.

registry: ghcr.io
username: ${{github.actor}}
password: ${{secrets.GITHUB_TOKEN}}

- name: "Set up metadata for dependency image"
id: "metadata-deps-image"
uses: "docker/metadata-action@902fa8ec7d6ecbf8d84d538b9b233a880e428804"
with:
images: >-
ghcr.io/${{github.repository}}/prestissimo-worker-dev-env
tags: |-
type=raw,value=dev
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Upper-case repository names break GHCR pushes

Convert ${{ github.repository }} to lower-case (or just use ${{ github.repository_owner }}) to avoid auth errors.

-            ghcr.io/${{github.repository}}/prestissimo-worker-dev-env
+            ghcr.io/${{ github.repository_owner }}/prestissimo-worker-dev-env
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
images: >-
ghcr.io/${{github.repository}}/prestissimo-worker-dev-env
tags: |-
type=raw,value=dev
images: >-
ghcr.io/${{ github.repository_owner }}/prestissimo-worker-dev-env
tags: |-
type=raw,value=dev
🤖 Prompt for AI Agents
In .github/workflows/prestissimo-worker-images-build.yml around lines 28 to 31,
the use of `${{ github.repository }}` can cause GHCR push failures if the
repository name contains upper-case letters. To fix this, convert `${{
github.repository }}` to lower-case using a function or replace it with `${{
github.repository_owner }}` which is always lower-case, ensuring the image name
is valid and avoiding authentication errors.

- name: "Build and push dependency image"
uses: "docker/build-push-action@471d1dc4e07e5cdedd4c2171150001c434f0b7a4"
with:
context: "./presto-native-execution/"
file: "./presto-native-execution/scripts/dockerfiles/ubuntu-22.04-dependency.dockerfile"
push: >-
${{github.event_name != 'pull_request'
&& github.ref == 'refs/heads/release-0.293-clp-connector'}}
tags: "${{steps.metadata-deps-image.outputs.tags}}"
labels: "${{steps.metadata-deps-image.outputs.labels}}"

- name: "Set up metadata for runtime image"
id: "metadata-runtime-image"
uses: "docker/metadata-action@902fa8ec7d6ecbf8d84d538b9b233a880e428804"
with:
images: >-
ghcr.io/${{github.repository}}/prestissimo-worker
tags: |-
type=raw,value=dev
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Same lowercase issue for the runtime image path

Apply the same fix as above to keep both images consistent and push-able.

🤖 Prompt for AI Agents
In .github/workflows/prestissimo-worker-images-build.yml around lines 48 to 51,
the runtime image path uses uppercase letters which can cause inconsistencies
and push failures. Modify the image path to use all lowercase letters, matching
the fix applied to the other image path, ensuring both image paths are
consistent and pushable.

- name: Get number of cores
id: get-cores
run: |-
echo "num_cores=$(nproc)" >> $GITHUB_OUTPUT
- name: "Build and push runtime image"
uses: "docker/build-push-action@471d1dc4e07e5cdedd4c2171150001c434f0b7a4"
with:
build-args: |-
BASE_IMAGE=ubuntu:22.04
DEPENDENCY_IMAGE=${{steps.metadata-deps-image.outputs.tags}}
EXTRA_CMAKE_FLAGS=-DPRESTO_ENABLE_TESTING=OFF -DPRESTO_ENABLE_PARQUET=ON -DPRESTO_ENABLE_S3=ON
NUM_THREADS=${{steps.get-cores.outputs.num_cores}}
OSNAME=ubuntu
context: "./presto-native-execution/"
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

DEPENDENCY_IMAGE may contain newlines, confusing BuildKit
docker/metadata-action outputs tags as a newline-separated list. Passing that raw into a single BUILD_ARG can inject stray line breaks and break the build. Grab the first tag explicitly:

-            DEPENDENCY_IMAGE=${{steps.metadata-deps-image.outputs.tags}}
+            DEPENDENCY_IMAGE=${{ steps.metadata-deps-image.outputs.tags }}
+            # If multiple tags are present, keep only the first one
+            DEPENDENCY_IMAGE=${DEPENDENCY_IMAGE%%$'\n'*}

or parse it in a separate step and set an output.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In .github/workflows/prestissimo-worker-images-build.yml around lines 58 to 66,
the DEPENDENCY_IMAGE build argument is set directly from
steps.metadata-deps-image.outputs.tags, which may contain multiple
newline-separated tags causing BuildKit to fail. Fix this by extracting only the
first tag from the tags output before passing it as DEPENDENCY_IMAGE, either by
using shell commands to select the first line or by adding a separate step to
parse and set a single tag output, then reference that sanitized output in the
build-args.

file: "./presto-native-execution/scripts/dockerfiles/prestissimo-runtime.dockerfile"
push: >-
${{github.event_name != 'pull_request'
&& github.ref == 'refs/heads/release-0.293-clp-connector'}}
tags: "${{steps.metadata-runtime-image.outputs.tags}}"
labels: "${{steps.metadata-runtime-image.outputs.labels}}"
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

ARG DEPENDENCY_IMAGE=presto/prestissimo-dependency:centos9
ARG BASE_IMAGE=quay.io/centos/centos:stream9
FROM ${DEPENDENCY_IMAGE} as prestissimo-image

Check warning on line 15 in presto-native-execution/scripts/dockerfiles/prestissimo-runtime.dockerfile

View workflow job for this annotation

GitHub Actions / prestissimo-worker-images-build

The 'as' keyword should match the case of the 'from' keyword

FromAsCasing: 'as' and 'FROM' keywords' casing do not match More info: https://docs.docker.com/go/dockerfile/rule/from-as-casing/

ARG OSNAME=centos
ARG BUILD_TYPE=Release
Expand All @@ -39,6 +39,15 @@
ENV BUILD_BASE_DIR=_build
ENV BUILD_DIR=""

# Temporary fix for https://github.com/prestodb/presto/issues/25531
# TODO: Update this code when there's a proper fix
RUN apt-get update && \
DEBIAN_FRONTEND=noninteractive apt-get install -y tzdata && \
ln -snf /usr/share/zoneinfo/America/Toronto /etc/localtime && \
echo "America/New_York" > /etc/timezone && \
Copy link
Member

Choose a reason for hiding this comment

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

What error do we get if we don't have these lines?

Copy link
Author

Choose a reason for hiding this comment

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

prestodb#25531
In the description

apt-get clean && \
rm -rf /var/lib/apt/lists/*

COPY --chmod=0775 --from=prestissimo-image /prestissimo/${BUILD_BASE_DIR}/${BUILD_DIR}/presto_cpp/main/presto_server /usr/bin/
COPY --chmod=0775 --from=prestissimo-image /runtime-libraries/* /usr/lib64/prestissimo-libs/
COPY --chmod=0755 ./etc /opt/presto-server/etc
Expand Down
Loading