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.

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

ci: optimise image building #13027

Merged
merged 5 commits into from
May 20, 2024
Merged
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
64 changes: 21 additions & 43 deletions .github/workflows/ci-build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -151,57 +151,39 @@ jobs:
env:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}

argoexec-image:
name: argoexec-image
argo-images:
name: argo-images
# needs: [ lint ]
runs-on: ubuntu-latest
timeout-minutes: 10
strategy:
fail-fast: false
matrix:
include:
- image: argoexec
- image: argocli
steps:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
- uses: docker/setup-buildx-action@f95db51fddba0c2d1ec667646a06c2ce06100226 # v3.0.0
- name: Build and export
uses: docker/build-push-action@4a13e500e55cf31b7a5d59a38ab2040ab0f42f56 # v5.1.0
with:
context: .
tags: quay.io/argoproj/argoexec:latest
outputs: type=docker,dest=/tmp/argoexec_image.tar
target: argoexec
cache-from: type=gha
cache-to: type=gha,mode=max
- name: Upload
uses: actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32 # v3.1.3
with:
name: argoexec
path: /tmp/argoexec_image.tar
if-no-files-found: error

argocli-image:
name: argocli-image
# needs: [ lint ]
runs-on: ubuntu-latest
timeout-minutes: 15
steps:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
- uses: docker/setup-buildx-action@f95db51fddba0c2d1ec667646a06c2ce06100226 # v3.0.0
- name: Build and export
uses: docker/build-push-action@4a13e500e55cf31b7a5d59a38ab2040ab0f42f56 # v5.1.0
with:
context: .
tags: quay.io/argoproj/argocli:latest
outputs: type=docker,dest=/tmp/argocli_image.tar
target: argocli
tags: quay.io/argoproj/${{matrix.image}}:latest
outputs: type=docker,dest=/tmp/${{matrix.image}}_image.tar
target: ${{matrix.image}}
cache-from: type=gha
cache-to: type=gha,mode=max
- name: Upload
uses: actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32 # v3.1.3
with:
name: argocli
path: /tmp/argocli_image.tar
name: ${{matrix.image}}_image.tar
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
name: ${{matrix.image}}_image.tar
name: ${{matrix.image}}

From the previous code, this should just be the name of the image. Not sure if this was a copy+paste typo?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's deliberate. We're pattern matching this with pattern: '*_image.tar' on line 164. This pattern would be '*' without the extra parts to the name.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I thought that would suffice, no? Or were you trying to be extra careful in case at some point other artifacts were added or something?

path: /tmp/${{matrix.image}}_image.tar
if-no-files-found: error

e2e-tests:
name: E2E Tests
needs: [ changed-files, argoexec-image, argocli-image ]
needs: [ changed-files, argo-images ]
if: ${{ needs.changed-files.outputs.e2e-tests == 'true' }}
runs-on: ubuntu-latest
timeout-minutes: 30
Expand Down Expand Up @@ -276,20 +258,16 @@ jobs:
echo " user:" >> $KUBECONFIG
echo " token: xxxxxx" >> $KUBECONFIG
until kubectl cluster-info ; do sleep 10s ; done
- name: Download argoexec image
uses: actions/download-artifact@9bc31d5ccc31df68ecc42ccf4149144866c47d8a # v3.0.2
with:
name: argoexec
path: /tmp
- name: Load argoexec image
run: docker load < /tmp/argoexec_image.tar
- name: Download argocli image
- name: Download images
uses: actions/download-artifact@9bc31d5ccc31df68ecc42ccf4149144866c47d8a # v3.0.2
with:
name: argocli
pattern: '*_image.tar'
Copy link
Member

@agilgur5 agilgur5 May 27, 2024

Choose a reason for hiding this comment

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

It seems like "download all" is available if you don't specify name and use a directory.

Annnd that appears to be what we're using here since pattern is invalid in actions/download-artifact@v3.

This is getting a warning in CI for every E2E test in the matrix:

[E2E Tests (test-executor, minimal)](https://github.com/argoproj/argo-workflows/actions/runs/9249013492/job/25442227788#step:8:1)
Unexpected input(s) 'pattern', valid inputs are ['name', 'path']

Screenshot 2024-05-27 at 1 44 33 AM

But passes since "no name" == "download all"

path: /tmp
- name: Load argocli image
run: docker load < /tmp/argocli_image.tar
- name: Load images
run: |
set -eux
docker load < /tmp/argoexec_image.tar/argoexec_image.tar
docker load < /tmp/argocli_image.tar/argocli_image.tar
- name: Set-up /etc/hosts
run: |
echo '127.0.0.1 dex' | sudo tee -a /etc/hosts
Expand Down
Loading