Skip to content

FEATURE: print out compressed image sizes for amd64 #887

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

Merged
merged 4 commits into from
Nov 11, 2024
Merged

FEATURE: print out compressed image sizes for amd64 #887

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
60328e7
FEATURE: print out compressed image sizes for amd64
featheredtoast Nov 8, 2024
fe0347d
use github repository name
featheredtoast Nov 10, 2024
85e769b
remove repository checks
featheredtoast Nov 11, 2024
e057c78
start a service for registry
featheredtoast Nov 11, 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
27 changes: 23 additions & 4 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ env:

jobs:
timestamp:
if: (github.event_name != 'schedule' || github.repository == 'discourse/discourse_docker')
runs-on: ubuntu-latest
outputs:
timestamp: ${{ steps.timestamp.outputs.timestamp }}
Expand All @@ -33,6 +34,7 @@ jobs:
echo "timestamp=$timestamp" >> $GITHUB_OUTPUT

base:
if: (github.event_name != 'schedule' || github.repository == 'discourse/discourse_docker')
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I follow here. Why are we skipping building of the base image when it is a scheduled job?

Copy link
Member Author

Choose a reason for hiding this comment

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

skipping when it's a scheduled job and is a fork - this prevents a bunch of jobs on new forks until actions are disabled but will remove for now from the other comment.

# `unbuntu-22.04-8core` for arch amd64 non-scheduled builds
# `unbuntu-22.04` for arch amd64 scheduled builds
# `unbuntu-22.04-8core-arm` for arch arm64 non-scheduled builds
Expand Down Expand Up @@ -103,8 +105,25 @@ jobs:
run: |
docker images discourse/base

- name: Print compressed summary
if: github.event_name == 'pull_request' && matrix.arch == 'amd64'
run: |
# Push to local repo to compare sizes
docker run --quiet -d --rm -p 5002:5000 registry:2
sleep 5
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of doing this, I think we can follow https://docs.docker.com/build/ci/github-actions/local-registry/ instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh didn't know services were available to us, very cool!

docker tag discourse/base:2.0.${{ env.TIMESTAMP }}-slim-${{ matrix.arch }} localhost:5002/base:2.0.${{ env.TIMESTAMP }}-slim-${{ matrix.arch }}
docker tag discourse/base:2.0.${{ env.TIMESTAMP }}-main-${{ matrix.arch }} localhost:5002/base:2.0.${{ env.TIMESTAMP }}-main-${{ matrix.arch }}
docker push --quiet localhost:5002/base:2.0.${{ env.TIMESTAMP }}-slim-${{ matrix.arch }}
docker push --quiet localhost:5002/base:2.0.${{ env.TIMESTAMP }}-main-${{ matrix.arch }}
# multiarch manifest is an array of schemas - [0] is amd64, [1] is arch64: Compare amd64.
CURRENT_SLIM=$(docker manifest inspect -v discourse/base:slim | jq -r '.[0].SchemaV2Manifest.layers[] | .size / 1024 / 1024 | .*100 | round/100' | awk '{print $0; sum+= $0}; END {print sum}' | tail -n 1)
CURRENT_RELEASE=$(docker manifest inspect -v discourse/base:release | jq -r '.[0].SchemaV2Manifest.layers[] | .size / 1024 / 1024 | .*100 | round/100' | awk '{print $0; sum+= $0}; END {print sum}' | tail -n 1)
NEW_SLIM=$(docker manifest inspect -v --insecure localhost:5002/base:2.0.${{ env.TIMESTAMP }}-slim-${{ matrix.arch }} | jq -r '.SchemaV2Manifest.layers[] | .size / 1024 / 1024 | .*100 | round/100' | awk '{print $0; sum+= $0}; END {print sum}' | tail -n 1)
NEW_RELEASE=$(docker manifest inspect -v --insecure localhost:5002/base:2.0.${{ env.TIMESTAMP }}-main-${{ matrix.arch }} | jq -r '.SchemaV2Manifest.layers[] | .size / 1024 / 1024 | .*100 | round/100' | awk '{print $0; sum+= $0}; END {print sum}' | tail -n 1)
echo "current slim: ${CURRENT_SLIM} release: ${CURRENT_RELEASE}. new slim: ${NEW_SLIM} release: ${NEW_RELEASE}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think including the units will be better here.


- name: push to dockerhub
if: github.ref == 'refs/heads/main'
if: github.ref == 'refs/heads/main' && github.repository == 'discourse/discourse_docker'
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if I agree here. I think forks should just disable actions if they don't want to run them rather than the main repository having to include this conditional.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm of the opinion it's better for a fork to be able to get a passing CI run for experimenting. Forks may be interested in other jobs that are not push-to-discourse/base, such as test builds, running tests, and linting and we should consider allowing that.

We can remove it, but it was quite useful here for this work, so I thought to propose it.

env:
DOCKERHUB_PASSWORD: ${{ secrets.DOCKERHUB_PASSWORD }}
run: |
Expand All @@ -115,7 +134,7 @@ jobs:
docker push discourse/discourse_dev:${{ env.TIMESTAMP }}-${{ matrix.arch }}

- name: Push discourse/base:aarch64 image for backwards compatibility
if: (github.ref == 'refs/heads/main') && (matrix.arch == 'arm64')
if: (github.ref == 'refs/heads/main') && (matrix.arch == 'arm64') && (github.repository == 'discourse/discourse_docker')
run: |
docker tag discourse/base:2.0.${{ env.TIMESTAMP }}-main-${{ matrix.arch }} discourse/base:aarch64
docker push discourse/base:aarch64
Expand All @@ -125,7 +144,7 @@ jobs:
needs: [base, timestamp]
env:
TIMESTAMP: ${{ needs.timestamp.outputs.timestamp }}
if: github.ref == 'refs/heads/main'
if: github.ref == 'refs/heads/main' && github.repository == 'discourse/discourse_docker'
steps:
- name: create and push multiarch manifests
run: |
Expand Down Expand Up @@ -212,7 +231,7 @@ jobs:
run: |
docker images discourse/discourse_test
- name: push to dockerhub
if: success() && (github.ref == 'refs/heads/main')
if: success() && (github.ref == 'refs/heads/main') && (github.repository == 'discourse/discourse_docker')
env:
DOCKERHUB_PASSWORD: ${{ secrets.DOCKERHUB_PASSWORD }}
run: |
Expand Down
Loading