-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
force-pushed
the
centralize-building-binaries
branch
from
December 18, 2024 05:21
10d427b
to
73050de
Compare
Signed-off-by: Mason Malone <[email protected]>
MasonM
force-pushed
the
centralize-building-binaries
branch
from
December 18, 2024 05:23
73050de
to
31f6916
Compare
tczhao
reviewed
Dec 19, 2024
tczhao
approved these changes
Dec 19, 2024
blkperl
approved these changes
Dec 20, 2024
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
All the E2E tests job run
make controller
to build the workflow controller, and several also runmake 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.