fix: add missing wg.Wait() in flaky storage cache test#2151
Merged
Conversation
The EOF_from_inner_is_returned_to_caller_unchanged subtest was missing a c.wg.Wait() call, causing a race between async cache writeback and t.TempDir() cleanup. This made the test flaky with "directory not empty" errors.
…g chunker test require.Eventually spawns condition goroutines that race with defer chunker.Close(), causing intermittent failures.
levb
added a commit
that referenced
this pull request
Mar 18, 2026
Incorporate main commits: #2150 (validate specified IPs in egress), #2156 (disable request timeout), #2154 (envd init logs), #2151 (storage cache test fix). Resolve conflict in sandbox_create.go by consolidating validation: Egress validation — matches main #2150 + port rejection: - validateEgressRules stays in sandbox_create.go (same location as main) - Uses IsSpecifiedIPOrCIDR from #2150 to reject unspecified addresses (0.0.0.0, ::, 0.0.0.0/24, etc.) while allowing 0.0.0.0/0 - Uses ParseAddressesAndDomains to separate IPs from domains (same as main) - Adds port rejection (egress doesn't support port-specific rules) Ingress validation — new, added next to validateEgressRules: - validateIngressRules + validateIngressEntry in sandbox_create.go - Uses SplitHostPort to separate CIDR from port, validates each part - Uses IsSpecifiedIPOrCIDR for IPv4 + allows ::/0 for IPv6 block-all - Uses ParsePortRange for port/port-range validation - Rejects domains (ingress is IP/CIDR only) - Requires deny-all when allow rules are present Simplify rule.go — remove ParseRule/ParseRules: - Egress validation uses IsSpecifiedIPOrCIDR + ParseAddressesAndDomains directly - Ingress validation uses SplitHostPort + IsIPOrCIDR + ParsePortRange directly - Keep: Rule/ACL structs (hot-path matching), SplitHostPort, ParsePortRange (public) make[1]: Entering directory '/home/lev/dev/infra/iac/provider-gcp' - Remove: ParseRule, ParseRules (tried to be one-size-fits-all, added complexity) Simplify orchestrator ACL building: - newEgressACL: uses parseCIDRs (direct net.ParseCIDR, no ParseRules) - newIngressACL/parseIngressRules: unchanged (builds from proto fields) Update create_instance.go: - buildEgressConfig: uses IsIPOrCIDR for domain detection (was ParseRule) - parseIngressRules: uses SplitHostPort + ParsePortRange (was ParseRule) Test updates: - Error messages updated to match main's IsSpecifiedIPOrCIDR style - Integration tests: replace ::/1/0.0.0.0/1 with ::/0/0.0.0.0/0 (unspecified network addresses now rejected) - TestIsSpecifiedIPOrCIDR preserved from #2150 - TestSplitHostPort, TestParsePortRange replace TestParseRule/TestParseRules
ValentaTomas
pushed a commit
that referenced
this pull request
May 4, 2026
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.
Summary
The
TestCachedSeekable_ReadAt_PreservesEOF/EOF_from_inner_is_returned_to_caller_unchangedsubtest was missing ac.wg.Wait()call, causing a race between async cache writeback andt.TempDir()cleanup. This made the test flaky with "directory not empty" errors during removal.The sibling subtest and all other cache-writeback tests already call
c.wg.Wait()to ensure async operations finish before cleanup.Note
Low Risk
Low risk since changes are limited to tests, but they touch concurrency/timeout coordination and could still mask real timing issues if timeouts are too generous or too strict in CI.
Overview
Stabilizes two concurrency-sensitive tests by ensuring background work completes deterministically:
cachedSeekableEOF preservation now waits for async cache writeback before temp dir cleanup, andStreamingChunkerfull-chunk caching verification replaces a pollingEventuallyloop with a single blockingSliceunder a timeout to avoid races withClose().Written by Cursor Bugbot for commit bcca097. This will update automatically on new commits. Configure here.