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: optimize tests by only building binaries once #14012

Merged
merged 3 commits into from
Dec 22, 2024

Conversation

MasonM
Copy link
Contributor

@MasonM MasonM commented Dec 18, 2024

Motivation

All the E2E tests job run make controller to build the workflow controller, and several also run make cli to build the CLI, which takes ~2 minutes. It'll be faster to have a central "build binaries" job that does that and uploads them as an artifact, which each E2E job can then download.

Modifications

Add build-binaries job to build the controller and CLI and upload them as an artifact, then download those artifacts in each E2E job.

Verification

Wait for E2E tests to run.

All the E2E tests job run `make controller` to build the workflow
controller, and several also run `make cli` to build the CLI, which
takes ~2 minutes. It'll be faster to have a central "build binaries" job
that does that and uploads them as an artifact, which each E2E job can
then download.

Signed-off-by: Mason Malone <[email protected]>
@MasonM MasonM force-pushed the centralize-building-binaries branch from 10d427b to 73050de Compare December 18, 2024 05:21
@MasonM MasonM force-pushed the centralize-building-binaries branch from 73050de to 31f6916 Compare December 18, 2024 05:23
@MasonM MasonM marked this pull request as ready for review December 18, 2024 05:55
@MasonM MasonM requested a review from Joibel December 18, 2024 06:10
@tczhao tczhao merged commit 1567ed7 into argoproj:main Dec 22, 2024
30 checks passed
MasonM added a commit to MasonM/argo-workflows that referenced this pull request Dec 23, 2024
`make install` is being redundantly run three times for each E2E test:
1. https://github.com/argoproj/argo-workflows/blob/6699ab396f830210f6dcac4f00a9328a629c142f/.github/workflows/ci-build.yaml#L349-L350
2. As a prerequisite of `make start`:
   https://github.com/argoproj/argo-workflows/blob/6699ab396f830210f6dcac4f00a9328a629c142f/Makefile#L547o
3. As a dependency by Kit: https://github.com/argoproj/argo-workflows/blob/6699ab396f830210f6dcac4f00a9328a629c142f/tasks.yaml#L39

Similarly, the changes in
argoproj#14012 to centralize
binary building aren't effective because the binaries are being rebuilt.

This eliminates the redundancy by removing the unnecessary prerequisites
from the `Makefile` and the redundant `make install` step in
`ci-build.yaml`. Also, I added `make --touch dist/*` to mark everything
as up-to-date so the binaries aren't rebuilts (docs:
https://www.gnu.org/software/make/manual/html_node/Instead-of-Execution.html)

Signed-off-by: Mason Malone <[email protected]>
MasonM added a commit to MasonM/argo-workflows that referenced this pull request Dec 23, 2024
`make install` is being redundantly run three times for each E2E test:
1. In the workflow: https://github.com/argoproj/argo-workflows/blob/6699ab396f830210f6dcac4f00a9328a629c142f/.github/workflows/ci-build.yaml#L349-L350
2. As a prerequisite of `make start`:
   https://github.com/argoproj/argo-workflows/blob/6699ab396f830210f6dcac4f00a9328a629c142f/Makefile#L547o
3. As a dependency by Kit: https://github.com/argoproj/argo-workflows/blob/6699ab396f830210f6dcac4f00a9328a629c142f/tasks.yaml#L39

Similarly, the changes in
argoproj#14012 to centralize
binary building aren't effective because the binaries are being rebuilt.

This eliminates the redundancy by removing the unnecessary prerequisites
from the `Makefile` and the redundant `make install` step in
`ci-build.yaml`. Also, I added `make --touch dist/*` to mark everything
as up-to-date so the binaries aren't rebuilts (docs:
https://www.gnu.org/software/make/manual/html_node/Instead-of-Execution.html)

Signed-off-by: Mason Malone <[email protected]>
MasonM added a commit to MasonM/argo-workflows that referenced this pull request Dec 23, 2024
`make install` is being redundantly run three times for each E2E test:
1. In the workflow: https://github.com/argoproj/argo-workflows/blob/6699ab396f830210f6dcac4f00a9328a629c142f/.github/workflows/ci-build.yaml#L349-L350
2. As a prerequisite of `make start`:
   https://github.com/argoproj/argo-workflows/blob/6699ab396f830210f6dcac4f00a9328a629c142f/Makefile#L547o
3. As a dependency by Kit: https://github.com/argoproj/argo-workflows/blob/6699ab396f830210f6dcac4f00a9328a629c142f/tasks.yaml#L39

Similarly, the changes in
argoproj#14012 to centralize
binary building aren't effective because the binaries are being rebuilt.

This eliminates the redundancy by removing the unnecessary prerequisites
from the `Makefile` and the redundant `make install` step in
`ci-build.yaml`. Also, I added `make --touch dist/*` to mark everything
as up-to-date so the binaries aren't rebuilts (docs:
https://www.gnu.org/software/make/manual/html_node/Instead-of-Execution.html)

Signed-off-by: Mason Malone <[email protected]>
MasonM added a commit to MasonM/argo-workflows that referenced this pull request Dec 23, 2024
`make install` is being redundantly run three times for each E2E test:
1. In the workflow: https://github.com/argoproj/argo-workflows/blob/6699ab396f830210f6dcac4f00a9328a629c142f/.github/workflows/ci-build.yaml#L349-L350
2. As a prerequisite of `make start`:
   https://github.com/argoproj/argo-workflows/blob/6699ab396f830210f6dcac4f00a9328a629c142f/Makefile#L547o
3. As a dependency by Kit: https://github.com/argoproj/argo-workflows/blob/6699ab396f830210f6dcac4f00a9328a629c142f/tasks.yaml#L39

Similarly, the changes in
argoproj#14012 to centralize
binary building aren't effective because the binaries are being rebuilt.

This eliminates the redundancy by removing the unnecessary prerequisites
from the `Makefile` and the redundant `make install` step in
`ci-build.yaml`. Also, I added `make --touch dist/*` to mark everything
as up-to-date so the binaries aren't rebuilts (docs:
https://www.gnu.org/software/make/manual/html_node/Instead-of-Execution.html)

Signed-off-by: Mason Malone <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants