Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Skip building HotROD for all platforms for pull requests #5765

Merged
merged 8 commits into from
Jul 19, 2024
Merged

Skip building HotROD for all platforms for pull requests #5765

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d81ab88
Enable build_flags for hotrod workflow
Manoramsharma Jul 19, 2024
9e7f0ef
Merge remote-tracking branch 'upstream/main' into hotrod-image-build
Manoramsharma Jul 19, 2024
dc68116
Need local flag only when the test runs on a PR
Manoramsharma Jul 19, 2024
d847dff
Publish Image with LOCAL_FLAG after successful tests
Manoramsharma Jul 19, 2024
6f855c4
fix
yurishkuro Jul 19, 2024
e1fb84c
fix
yurishkuro Jul 19, 2024
a8b1d1b
fix
yurishkuro Jul 19, 2024
7d6539f
fix
yurishkuro Jul 19, 2024
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
13 changes: 12 additions & 1 deletion .github/workflows/ci-docker-hotrod.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,19 @@ jobs:

- uses: docker/setup-qemu-action@5927c834f5b4fdf503fca6f4c7eccda82949e1ee # v3.1.0

- name: Define BUILD_FLAGS var if running on a Pull Request
run: |
case ${GITHUB_EVENT_NAME} in
pull_request)
echo "BUILD_FLAGS=-l -D -p linux/amd64" >> ${GITHUB_ENV}
;;
*)
echo "BUILD_FLAGS=" >> ${GITHUB_ENV}
;;
esac

- name: Build, test, and publish hotrod image
run: bash scripts/hotrod-integration-test.sh
run: bash scripts/hotrod-integration-test.sh ${{ env.BUILD_FLAGS }}
env:
DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }}
QUAY_TOKEN: ${{ secrets.QUAY_TOKEN }}
Expand Down
36 changes: 32 additions & 4 deletions scripts/hotrod-integration-test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,34 @@

set -euxf -o pipefail

print_help() {
echo "Usage: $0 [-l] [-D] [-p platforms] [-h]"
echo "-D: Disable building of images with debugger"
echo "-h: Print help"
echo "-l: Enable local-only mode that only pushes images to local registry"
echo "-p: Comma-separated list of platforms to build for (default: all supported)"
exit 1
}

docker_compose_file="./examples/hotrod/docker-compose.yml"
platforms="linux/amd64,linux/s390x,linux/ppc64le,linux/arm64"
BINARY='example-hotrod'
LOCAL_FLAG=''

while getopts "lp:h" opt; do
case "${opt}" in
l)
# in the local-only mode the images will only be pushed to local registry
LOCAL_FLAG='-l'
;;
p)
platforms=${OPTARG}
;;
?)
print_help
;;
esac
done

teardown() {
echo "Tearing down..."
Expand All @@ -21,13 +47,15 @@ make create-baseimg

# Loop through each platform (separated by commas)
for platform in $(echo "$platforms" | tr ',' ' '); do
# Extract the operating system from the platform string
os=${platform%%/*} #remove everything after the last slash
# Extract the architecture from the platform string
arch=${platform##*/} # Remove everything before the last slash
make "build-all-in-one" GOOS=linux GOARCH="${arch}"
make "build-all-in-one" GOOS="${os}" GOARCH="${arch}"
Copy link
Member

Choose a reason for hiding this comment

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

This is actually incorrect. This script is for building & testing hotrod. You are not actually reducing platforms on which the hotrod is build (L40-43). On the other hand, all-in-one does not need to be built for other platforms at all, only once for the current platform "$(go env GOOS)/$(go env GOARCH)", because we only use it for e2e test and then discard.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay so I should leave build-all-in-one with the single build just for linux and should bring back the implementation for hotrod, something like this >

# Build for each platform for platform in $(echo "$PLATFORMS" | tr ',' ' '); do os=${platform%/*} # Extract OS (part before the slash) arch=${platform##*/} # Extract architecture (part after the slash) make build-examples GOOS="${os}" GOARCH="${arch}" done

Copy link
Member

Choose a reason for hiding this comment

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

Yes the loop is for hotrod, allinone only needs to be built locally

done

# Build image locally (-l) for integration test
bash scripts/build-upload-a-docker-image.sh -l -c example-hotrod -d examples/hotrod -p "${platforms}"
bash scripts/build-upload-a-docker-image.sh -l -c ${BINARY} -d examples/hotrod -p "${platforms}"
bash scripts/build-upload-a-docker-image.sh -l -b -c all-in-one -d cmd/all-in-one -p "${platforms}" -t release

JAEGER_VERSION=$GITHUB_SHA REGISTRY="localhost:5000/" docker compose -f "$docker_compose_file" up -d
Expand Down Expand Up @@ -86,5 +114,5 @@ if [[ "$span_count" -lt "$EXPECTED_SPANS" ]]; then
exit 1
fi

# Ensure the image is published after successful test (no -l flag)
bash scripts/build-upload-a-docker-image.sh -c example-hotrod -d examples/hotrod -p "${platforms}"
# Ensure the image is published after successful test (maybe with -l flag if on a pull request)
bash scripts/build-upload-a-docker-image.sh ${LOCAL_FLAG} -c example-hotrod -d examples/hotrod -p "${platforms}"
Loading