Skip to content

Fix aggregated API server readiness race causing sporadic 503s in envtest suites - #1505

Merged
afritzler merged 2 commits into
mainfrom
feat/envtest-aggregated-apiserver-readiness
Jul 27, 2026
Merged

Fix aggregated API server readiness race causing sporadic 503s in envtest suites#1505
afritzler merged 2 commits into
mainfrom
feat/envtest-aggregated-apiserver-readiness

Conversation

@afritzler

@afritzler afritzler commented Jul 6, 2026

Copy link
Copy Markdown
Member

Proposed Changes

Fix aggregated API server readiness race causing sporadic 503s in envtest suites.

Summary by CodeRabbit

  • Bug Fixes
    • Improved aggregated API server readiness checks by running real List round-trip probes for each discovered group/version.
    • Updated readiness polling to require multiple consecutive successful probes before marking endpoints ready, reducing false positives.
    • Refined retry behavior to continue through transient API/transport failures, while stopping immediately on non-transient errors.

@afritzler
afritzler requested a review from a team July 6, 2026 11:15
@github-actions github-actions Bot added size/L bug Something isn't working labels Jul 6, 2026
@afritzler
afritzler force-pushed the feat/envtest-aggregated-apiserver-readiness branch from 5c84f62 to 59f9a9f Compare July 6, 2026 11:20
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds an aggregated API serve-readiness probe to envtest after discovery succeeds. It issues List requests for target group/versions, requires sustained success, retries classified transient failures, and stops on non-transient errors.

Changes

Aggregated API Server Serve Readiness

Layer / File(s) Summary
Invoke serve readiness after discovery
utils/envtest/envtest.go
WaitUntilGroupVersionsDiscoverable now calls waitUntilGroupVersionsServe after discovery succeeds, with imports for error inspection.
Poll List serving readiness
utils/envtest/envtest.go
waitUntilGroupVersionsServe polls List requests until stable success, selects scheme-registered list objects per group/version, retries transient failures, and aborts on non-transient errors.

Estimated code review effort: 3 (Moderate) | ~25 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description only restates the fix and omits the required proposed-change bullets and Fixes # reference. Add the three Proposed Changes bullets with concrete details and include the related issue number in the Fixes # line.
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: fixing envtest aggregated API server readiness races causing sporadic 503s.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/envtest-aggregated-apiserver-readiness

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
utils/envtest/envtest.go (1)

757-767: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Stabilize list target selection. scheme.AllKnownTypes() is map-backed, so firstListTypeForGroupVersion can return a different client.ObjectList kind on different runs. Sort the candidates (for example by Kind) before choosing one.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@utils/envtest/envtest.go` around lines 757 - 767, The list selection in
firstListTypeForGroupVersion is nondeterministic because it iterates
scheme.AllKnownTypes() directly over a map, so the returned client.ObjectList
type can vary between runs. Collect the matching candidates for the requested
schema.GroupVersion, sort them deterministically (for example by GVK.Kind), and
then choose the first matching reflect.Type so the selection is stable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@utils/envtest/envtest.go`:
- Around line 757-767: The list selection in firstListTypeForGroupVersion is
nondeterministic because it iterates scheme.AllKnownTypes() directly over a map,
so the returned client.ObjectList type can vary between runs. Collect the
matching candidates for the requested schema.GroupVersion, sort them
deterministically (for example by GVK.Kind), and then choose the first matching
reflect.Type so the selection is stable.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 517d2afd-a6ff-4922-aad2-8bc8dbf528e6

📥 Commits

Reviewing files that changed from the base of the PR and between a0e22c5 and 5c84f62.

📒 Files selected for processing (1)
  • utils/envtest/envtest.go

@afritzler
afritzler force-pushed the feat/envtest-aggregated-apiserver-readiness branch 2 times, most recently from e74d1ca to 822e018 Compare July 6, 2026 12:05
@afritzler

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Comment thread utils/envtest/envtest.go Outdated
@afritzler
afritzler force-pushed the feat/envtest-aggregated-apiserver-readiness branch from 1d61bfd to 414fad1 Compare July 14, 2026 07:24
@afritzler
afritzler requested a review from a team as a code owner July 14, 2026 07:24
@afritzler
afritzler force-pushed the feat/envtest-aggregated-apiserver-readiness branch 3 times, most recently from 524909f to cf9fd36 Compare July 14, 2026 07:33
…test suites

Signed-off-by: Andreas Fritzler <andreas.fritzler@sap.com>
@afritzler
afritzler force-pushed the feat/envtest-aggregated-apiserver-readiness branch from cf9fd36 to e85c99c Compare July 27, 2026 12:47
Signed-off-by: Andreas Fritzler <andreas.fritzler@sap.com>
@afritzler
afritzler force-pushed the feat/envtest-aggregated-apiserver-readiness branch from e85c99c to 79f5931 Compare July 27, 2026 13:13
@afritzler
afritzler merged commit 9f928e1 into main Jul 27, 2026
11 checks passed
@afritzler
afritzler deleted the feat/envtest-aggregated-apiserver-readiness branch July 27, 2026 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants