perf(ci): remove container from go, go-postgres, go-bench jobs#19743
Draft
davdhacs wants to merge 1 commit intodavdhacs/host-runner-test-fixesfrom
Draft
perf(ci): remove container from go, go-postgres, go-bench jobs#19743davdhacs wants to merge 1 commit intodavdhacs/host-runner-test-fixesfrom
davdhacs wants to merge 1 commit intodavdhacs/host-runner-test-fixesfrom
Conversation
Run Go test jobs directly on ubuntu-latest instead of in the CI container image. This saves ~55s container init overhead per job and enables faster cache I/O (no overlay filesystem). Changes per job: - Remove container: block - Add actions/setup-go with GOTOOLCHAIN=auto override - Replace su postgres with docker run postgres - Add pg_isready retry loop (docker postgres needs warmup time) - go-bench: skip on PRs (if: github.event_name == 'push') Depends on #19712 (host-runner test fixes). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Skipping CI for Draft Pull Request. |
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
docker run ... --name postgresin bothgo-postgresandgo-benchjobs never stops/removes the container explicitly; consider adding a cleanup step (e.g.,docker stop postgres || true) to avoid name/port conflicts on reused runners or when debugging failures. - The Postgres setup and readiness loop are duplicated between
go-postgresandgo-bench; consider extracting this into a reusable action or composite step to keep the workflow DRY and make future changes to the DB bootstrap logic less error-prone.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `docker run ... --name postgres` in both `go-postgres` and `go-bench` jobs never stops/removes the container explicitly; consider adding a cleanup step (e.g., `docker stop postgres || true`) to avoid name/port conflicts on reused runners or when debugging failures.
- The Postgres setup and readiness loop are duplicated between `go-postgres` and `go-bench`; consider extracting this into a reusable action or composite step to keep the workflow DRY and make future changes to the DB bootstrap logic less error-prone.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
davdhacs
added a commit
that referenced
this pull request
Apr 1, 2026
Split the monolithic go job into 5 shards (pkg-helm, pkg-other, central-1, central-2, rest) and go-postgres into 2 shards (main, migrator) for parallel execution. Also split integration and operator tests into separate jobs (go-integration, go-operator-integration) so they run in parallel with unit tests instead of sequentially after them. Depends on #19743 (container removal) and #19742 (SKIP_DEPS + postgres subtargets). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 1, 2026
Contributor
|
Images are ready for the commit at f0fa0ff. To use with deploy scripts, first |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## davdhacs/host-runner-test-fixes #19743 +/- ##
===================================================================
- Coverage 49.59% 49.59% -0.01%
===================================================================
Files 2756 2756
Lines 208036 208036
===================================================================
- Hits 103180 103179 -1
- Misses 97196 97197 +1
Partials 7660 7660
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
Run Go test jobs directly on
ubuntu-latestinstead of in theapollo-cicontainer image. Saves ~55s container init overhead per job and enables faster GHA cache I/O (no overlay filesystem).Changes per job:
setup-go+GOTOOLCHAIN=autosu postgrestodocker run postgres, add readiness retry loopif: github.event_name == 'push')Test commands unchanged — still uses
make go-unit-tests,make go-postgres-unit-tests, etc.Stacked on #19712 (host-runner test fixes — required for tests to pass outside containers).
Split from #19678.
Testing and quality
Automated testing
How I validated my change
CI validation. Previously validated on #19678 (combined PR).
🤖 Generated with Claude Code