Repository navigation
HIVE-29591: Create Hive precommit image and move Dockerfile into the hive repository - #6953
abstractdog wants to merge 7 commits into
Conversation
46def98 to
101acd8
Compare
101acd8 to
84fa9dc
Compare
84fa9dc to
698ecdd
Compare
zabetak
left a comment
There was a problem hiding this comment.
LGTM, just a few small questions and some suggestions but nothing really blocking.
| dev-support/docker/precommit/smoke-test.sh hive-precommit:test | ||
|
|
||
| - name: Login to Docker Hub | ||
| if: github.event_name != 'pull_request' && github.repository == 'apache/hive' |
There was a problem hiding this comment.
Do we want to restrict login/publish to apache/hive repository? Maybe a contributor wants to test the action on their fork would that be a problem? Were you able to test these parts of the action somehow?
There was a problem hiding this comment.
I'd keep the github.repository == 'apache/hive' guard on login and publish: publishing only makes sense for apache/hive-ci-runner, and a fork has neither the DOCKERHUB_* secrets nor push rights to it. A fork still builds the image on its own PRs and manual runs, and with the job-level condition I just added, a push to a fork's master (i.e. syncing the fork) doesn't run the workflow at all. A contributor who wants to try an image doesn't need the workflow: they push it to their own namespace with docker buildx build --push and run precommit with IMAGE=... (README, step 3).
Regarding how I tested: I was able to push the feature branch to the origin, so I can do it, I'll let you know as the docker namespace is already there: https://hub.docker.com/r/apache/hive-ci-runner
There was a problem hiding this comment.
I can set the DOCKERHUB_* secrets on my own fork (e.g., https://github.com/zabetak/hive) and the workflow could theoretically push to https://hub.docker.com/u/zabetak as it happens on .github/workflows/hive-postgres-tpcds-metastore.yml. Anyways its not a big deal I am fine even if we don't allow runs from forks. Just wanted to mention that is feasible and not problematic if we don't point to the apache namespace.
There was a problem hiding this comment.
action that published an image with a testing tag:
https://github.com/apache/hive/actions/runs/43235096035
There was a problem hiding this comment.
action example that failed because tag wasn't overridden, so it won't override the image:
https://github.com/apache/hive/actions/runs/43375951727
apache/hive-ci-runner:HIVE-29591-test is already published, and a published tag is never overwritten. Bump org.opencontainers.image.version in the Dockerfile (or pick another tag for a manual run).
There was a problem hiding this comment.
action example, where the Dockerfile hasn't changed, so there is no reason to push:
https://github.com/apache/hive/actions/runs/43376396112/job/131726682743
Check the tag job says:
Notice: apache/hive-ci-runner:HIVE-29591-test is already published and the Dockerfile did not change: not publishing it again.
There was a problem hiding this comment.
a last commit is pushed that introduced the change:
410a568
so a patch that doesn't change the Dockerfile doesn't make the workflow push a new image
let me know @zabetak , if your +1 still holds
current precommit run:
https://ci.hive.apache.org/job/hive-precommit/job/PR-6953/12/
| if docker manifest inspect "$image" > /dev/null 2>&1; then | ||
| echo "::error::$image is already published, and a published tag is never overwritten. Bump org.opencontainers.image.version in the Dockerfile (or pick another tag for a manual run)." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
We can disable tag overwrite directly in DockerHub if we want to.
- Move the image to dev-support/docker/ci/runner. - Drop the init@<db> stages and precommit-init-metastore.sh (DbInstallBase already covers schematool), and with them Hadoop from the image. - Drop smoke-test.sh. - Version the image with its org.opencontainers.image.version label: publish apache/hive-ci-runner:<version>, never overwrite a published tag, and pin the tag in the Jenkinsfile. - Trim the README to the steps people actually follow. Co-Authored-By: Claude Code <noreply@anthropic.com>
docker 20.10 cannot extract the current mcr.microsoft.com/mssql/server:2019-latest
layers ("archive/tar: invalid tar header", microsoft/mssql-docker#978), which fails
TestSchemaToolForMetastore[mssql].
A push to a fork's master only syncs apache/hive, and the tag check fails there once apache/hive has published the version. Forks still build the image on their PRs and manual runs. Co-Authored-By: Claude Code <noreply@anthropic.com>
… does not need a new version When the tag is published already, the workflow fails only if the Dockerfile changed (against the target branch of a PR, or the previous head of a push); otherwise it builds the image but skips publishing it. A manual run with an existing tag still fails.
22e9acc to
410a568
Compare
|
| # A published tag is never overwritten. When the tag is published already, a change of the | ||
| # Dockerfile needs a new version; any other change (e.g. of this workflow) does not publish. | ||
| - name: Check the tag |
There was a problem hiding this comment.
Still +1 on the PR but all this tag management check and publish seems overly complicated. We could adopt a simple timestamp based tagging (eg. 20261113133625) and get done with it. This way even in the workflow dispatch we can remove the inputs and also users and future maintainers do not have to manually bump the version every time there is a change in the Dockerfile.
There was a problem hiding this comment.
I agree it's overcomplicated; we shall iterate one more time before merging
what tag do you think the Jenkinsfile should contain: latest or pin to a specific timestamp?
-
"do not have to manually bump the version every time" makes me think that the
Jenkinsfileshould always containlatest, in this case 1 PR per change is enough,latestwill point to a certain timestamp tag -
2 PRs per change: a) first one is approved, merged, b) github action deploys an image tag
20261113133625-commithashc) follow-up PR bumpsJenkinsfileto the newer
does it make sense?



What changes were proposed in this pull request?
A precommit image in
dev-support/docker/ci/runner. It is based on Debian trixie-slim and contains:A GitHub Actions workflow,
precommit-image.yml. On PRs that change the Dockerfile, it builds the image. On master it publishes the image for amd64 and arm64 asapache/hive-ci-runner:<version>, where<version>is theorg.opencontainers.image.versionlabel of the Dockerfile. A published the workflow fails if the tag already exists. A manual run can publish a candidate under atag of its own.The Jenkinsfile runs in
apache/hive-ci-runner:1instead of `ayushtkn/hive-dev-box:esw java,/etc/profile.d/*.sh, `git config extrMAVEN_ARGS(Maven 3.9+), in place of the dev-boxmvnwrapper.IMAGEbuild parameter lets a run use a candidate image.The
init@derbyandinit@postgresstages are removed.DbInstallBasealready coversschematoolon more databases, andInformationSchemaWithPrivilegeTestBasecovers the SYS/INFORMATION_SCHEMAinitialization. This also keeps Hadoop out of the image.
cachingprintstream.qandstack_trace.qnormalize the exec failure message withqt:replace:error=2, No such file or directory.This line is the only difference in the output, so the tests still check the same failure.
A README on how to change, try and roll out the image.
The docker and helm action pins in
docker-images.ymlandhive-postgres-tpcds-metastore.ymlare bumped to the versionsprecommit-image.ymluses. Some of the old pins were close to their expiry datesin the ASF allowlist.
Why are the changes needed?
The precommit image used to be built from a separate repository (hive-dev-box) and pushed to a personal Docker Hub account. Keeping the Dockerfile in the Hive repository and publishing it from CI makes the
image reviewable and reproducible, and removes the dependency on a personal account. See HI
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Locally, in the image with a
docker:dindsidecar:mvn install -DskipTests -Pitests,distsucceeds.newline.q(pythonTRANSFORM) onTestMiniLlapCliDriverjdbc_project_pushdown.q(postgres in docker) onTestMiniLlapLocalCliDriverstack_trace.qonTestNegativeLlapCliDrivercachingprintstream.qonTestNegativeLlapLocalCliDriverA full precommit run with the candidate image (
IMAGE=abstractdog/hive-precommit:HIVE-29591-v3):https://ci.hive.apache.org/job/hive-precommit/job/PR-6953/12/