Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 21 additions & 6 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,34 @@ jobs:
strategy:
fail-fast: false
matrix:
config:
include:
- image: accumulo
test: accumulo
- image: dns
platforms: linux/amd64,linux/arm64
test: dns
- image: centos7-oj17
platforms: linux/amd64,linux/arm64
- image: centos7-oj17-openldap-referrals
platforms: linux/amd64,linux/arm64
test: openldap
- image: spark3-iceberg
platforms: linux/amd64,linux/arm64
test: spark3-iceberg
- image: spark3-delta
platforms: linux/amd64,linux/arm64
test: spark3-delta
- image: spark3-hudi
platforms: linux/amd64,linux/arm64
test: spark3-hudi
- image: kerberos
platforms: linux/amd64,linux/arm64
test: kerberos
- image: gpdb-6
test: gpdb-6
- image: hdp2.6-hive-kerberized-2
- image: hive3.1-hive
platforms: linux/amd64,linux/arm64
test: hive3.1-hive
- image: hdp2.6-hive-kerberized
test: hdp2.6-hive
Expand All @@ -40,13 +48,20 @@ jobs:
- image: cdh5.15-hive-kerberized-kms
# TODO add test https://github.com/trinodb/trino/issues/14543
- image: phoenix5
platforms: linux/amd64,linux/arm64
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 0 # checkout tags so version in Manifest is set properly
- name: Build ${{ matrix.config.image }}
run: make "testing/${{ matrix.config.image }}"
- name: Test ${{ matrix.config.test }}
if: ${{ matrix.config.test != '' }}
- name: Set up QEMU
uses: docker/setup-qemu-action@v2
- name: Build ${{ matrix.image }}
env:
PLATFORMS: ${{ matrix.platforms }}
run: make "testing/${{ matrix.image }}"
- name: Test ${{ matrix.test }}
env:
PLATFORMS: ${{ matrix.platforms }}
if: ${{ matrix.test != '' }}
shell: 'script -q -e -c "bash {0}"'
run: make test IMAGE_TO_TEST="${{ matrix.config.test }}"
run: make test IMAGE_TO_TEST="${{ matrix.test }}"
50 changes: 48 additions & 2 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,55 @@ jobs:
run: |
if [ "$DO_RELEASE" != "true" ]; then
echo "Skipping the release step because not starting from a snapshot version"
else
make release
exit 0
fi
single_arch=(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Now is there some way to make sure that all images get released. Looks like now anything no in either list gets skipped silently.

Is it not reasonable to assume anything not in multi_arch list is single-arch?
Also seems like we now have moved responsibility of images to release and build for in two places (leading to this question).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll try to figure out how to get a list of all available images and add some validation here to throw an error if there are any not mentioned in any of these two lists. I wouldn't make any assumptions about them being single or multi-arch.

Also see this old comment: #143 (comment)

Copy link
Copy Markdown
Member

@hashhar hashhar Oct 26, 2022

Choose a reason for hiding this comment

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

One idea I had was to place a marker file in each directory for the multi-arch supported ones and both ci.yml and release.yml can use that to decide what needs to be done. (Very hacky and I feel dirty for even suggesting it but I don't see better ways yet to be honest).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done! While testing this, I noticed we were missing the recently added centos7-oj11 and spark3-hudi, so it was a great idea, thanks!

testing/accumulo
testing/cdh5.12-hive
testing/cdh5.12-hive-kerberized
testing/cdh5.15-hive
testing/cdh5.15-hive-kerberized
testing/cdh5.15-hive-kerberized-kms
testing/gpdb-6
testing/hdp2.6-hive
testing/hdp2.6-hive-kerberized
testing/hdp2.6-hive-kerberized-2
testing/hdp3.1-hive
testing/hdp3.1-hive-kerberized
)
multi_arch=(
testing/centos7-oj11
testing/centos7-oj17
testing/centos7-oj17-openldap
testing/centos7-oj17-openldap-referrals
testing/dns
testing/hive3.1-hive
testing/kerberos
testing/phoenix5
testing/spark3-delta
testing/spark3-iceberg
testing/spark3-hudi
)
to_release=("${single_arch[@]}" "${multi_arch[@]}")
make meta
read -a images < <(make list)
export LC_ALL=C
mapfile -t ignored < <( \
comm -23 \
<(printf '%s\n' "${images[@]}" | sort) \
<(printf '%s\n' "${to_release[@]}" | sort) \
)
if [ "${#ignored[@]}" -ne 0 ]; then
echo "Images that would not get released: ${ignored[*]}"
echo "Must be explicitly added to either single_arch or multi_arch list"
exit 2
fi
make prepare-release
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

" Extract prepare-release target from release " - should this come before others (or maybe even together with the change which allows releasing multi-arch images)? Also some context is useful.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If I'd put them into a single commit, you'd ask me to extract it :-) I moved it so it's before the commit that adds multi-arch releases, but now it adds a target that's unused until the next commit.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Can you suggest something for more context?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

adds a target that's unused until the next commit.

Sorry I has misordered commits while reviewing. It's all good here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For context I'd probably say:

With multi-arch releases images for all platforms must be built before they can be published - which requires decoupling prepare-release from release goal.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sorry I has misordered commits while reviewing. It's all good here.

But I moved it, so are you sure?

make "${single_arch[@]/%/@$VERSION}"
make "${multi_arch[@]/%/@$VERSION}" PLATFORM="linux/amd64,linux/arm64"
remote=("${to_release[@]/#/ghcr.io/trinodb/}")
remote=("${remote[@]/%/:$VERSION}")
./bin/push.sh "${remote[@]}"

- name: Set next development version
run: |
Expand Down
28 changes: 17 additions & 11 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ LABEL := io.trino.git.hash=$(shell git rev-parse HEAD)

DEPEND_SH=bin/depend.sh
FLAG_SH=bin/flag.sh
BUILD_SH=$(realpath bin/build.sh)
TAG_SH=$(realpath bin/tag.sh)
PUSH_SH=bin/push.sh
TEST_SH=bin/test.sh
BUILDDIR=build
Expand Down Expand Up @@ -95,9 +97,10 @@ images: $(LATEST_TAGS)
#
# Release images to Dockerhub
#
.PHONY: release push-release snapshot build-snapshot push-snapshot
.PHONY: prepare-release release push-release snapshot build-snapshot push-snapshot

release: require-clean-repo require-on-master require-release-version push-release
prepare-release: require-clean-repo require-on-master require-release-version
release: prepare-release push-release

push-release: $(RELEASE_TAGS)
$(PUSH_SH) $(call docker-tag,$(call resolved-image-name,$^))
Expand Down Expand Up @@ -179,7 +182,7 @@ $(FLAGDIR)/%.flags: %/Dockerfile $(FLAG_SH)
# supposed to mean.
# - TODO replace :unlabelled with something more appropriate.
$(UNLABELLED_TAGS): %@unlabelled: %/Dockerfile %@latest
docker tag $*:latest $(call docker-tag,$@)
$(TAG_SH) $*:latest $(call docker-tag,$@)

#
# We don't need to specify any (real) dependencies other than the Dockerfile
Expand All @@ -192,18 +195,17 @@ $(LATEST_TAGS): %@latest: %/Dockerfile %-parent-check
@echo
@echo "Building [$@] image using buildkit"
@echo
cd $* && time $(SHELL) -c "( docker buildx build --compress --progress=plain --add-host hadoop-master:127.0.0.2 ${BUILD_ARGS} $(DBFLAGS_$*) -t $(call docker-tag,$@) --label $(LABEL) . )"
docker history $(call docker-tag,$@)
cd $* && time $(BUILD_SH) $(call docker-tag,$@) ${BUILD_ARGS} $(DBFLAGS_$*) --label $(LABEL)

$(VERSION_TAGS): %@$(VERSION): %@latest
docker tag $(call docker-tag,$^) $(call docker-tag,$@)
docker tag $(call docker-tag,$^) $(call docker-tag,$(call resolved-image-name,$^))
docker tag $(call docker-tag,$@) $(call docker-tag,$(call resolved-image-name,$@))
$(TAG_SH) $(call docker-tag,$^) $(call docker-tag,$@)
$(TAG_SH) $(call docker-tag,$^) $(call docker-tag,$(call resolved-image-name,$^))
$(TAG_SH) $(call docker-tag,$@) $(call docker-tag,$(call resolved-image-name,$@))

$(GIT_HASH_TAGS): %@$(GIT_HASH): %@latest
docker tag $(call docker-tag,$^) $(call docker-tag,$@)
docker tag $(call docker-tag,$^) $(call docker-tag,$(call resolved-image-name,$^))
docker tag $(call docker-tag,$@) $(call docker-tag,$(call resolved-image-name,$@))
$(TAG_SH) $(call docker-tag,$^) $(call docker-tag,$@)
$(TAG_SH) $(call docker-tag,$^) $(call docker-tag,$(call resolved-image-name,$^))
$(TAG_SH) $(call docker-tag,$@) $(call docker-tag,$(call resolved-image-name,$@))

#
# Verify that the parent image specified in the Dockerfile is either
Expand Down Expand Up @@ -258,3 +260,7 @@ test:
.PHONY: clean
clean:
-rm -r $(BUILDDIR)

.PHONY: list
list:
@echo $(IMAGE_DIRS)
43 changes: 43 additions & 0 deletions bin/build.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
#!/usr/bin/env bash

set -xeuo pipefail

usage() {
echo "$0 {image} [args]" >&2
}

if [ $# -lt 1 ]; then
usage
exit 1
fi

image=$1
shift

if [ -z "${PLATFORMS:-}" ]; then
docker buildx build \
--compress \
--progress=plain \
--add-host hadoop-master:127.0.0.2 \
-t "$image" \
--load \
"$@" \
.
exit 0
fi

IFS=, read -ra platforms <<<"$PLATFORMS"
export ARCH
for platform in "${platforms[@]}"; do
IFS=: read -r name tag <<<"$image"
ARCH="-${platform//\//-}"
docker buildx build \
--platform "$platform" \
--compress \
--progress=plain \
--add-host hadoop-master:127.0.0.2 \
-t "${name}:${tag}${ARCH}" \
--load \
"$@" \
.
done
2 changes: 2 additions & 0 deletions bin/depend.sh
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ shift
known_images="$*"

parent_image_tag=$(find_parent "$target_dockerfile")
# remove the $ARCH arg since it should be resolved to an empty string anyway
parent_image_tag=${parent_image_tag//\$ARCH/}
ec=$?
case $ec in
0) ;;
Expand Down
22 changes: 22 additions & 0 deletions bin/lib.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#!/usr/bin/env bash

set -xeuo pipefail

function expand_multiarch_tags() {
local platforms
local name
local tag=$1
shift

if [ -z "${PLATFORMS:-}" ]; then
echo "$tag"
return
fi

IFS=, read -ra platforms <<<"$PLATFORMS"
IFS=: read -r name tag <<<"$tag"

for platform in "${platforms[@]}"; do
echo "${name}:${tag}-${platform//\//-}"
done
}
29 changes: 25 additions & 4 deletions bin/push.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,31 @@

set -xeuo pipefail

while [ "$#" -gt 0 ]; do
while ! docker push "$1"; do
echo "Failed to push $1, retrying in 30s..."
function push_retry() {
local image=$1

while ! docker push "$image"; do
echo "Failed to push $image, retrying in 30s..."
sleep 30
done
shift
}

if [ -z "$PLATFORMS" ]; then
for image in "$@"; do
push_retry "$image"
done
exit 0
fi

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)"
# shellcheck source=bin/lib.sh
source "$SCRIPT_DIR/lib.sh"

for image in "$@"; do
mapfile -t expanded_names < <(expand_multiarch_tags "$image")
for name in "${expanded_names[@]}"; do
push_retry "$name"
done
docker manifest create "$image" "${expanded_names[@]}"
docker manifest push "$image"
done
25 changes: 25 additions & 0 deletions bin/tag.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#!/usr/bin/env bash

set -xeuo pipefail

usage() {
echo "$0 {src} {dst}" >&2
}

if [ $# -lt 2 ]; then
usage
exit 1
fi

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)"
# shellcheck source=bin/lib.sh
source "$SCRIPT_DIR/lib.sh"

mapfile -t src_tags < <(expand_multiarch_tags "$1")
mapfile -t dst_tags < <(expand_multiarch_tags "$2")

for i in "${!src_tags[@]}"; do
src=${src_tags[$i]}
dst=${dst_tags[$i]}
docker tag "$src" "$dst"
done
Loading