Skip to content

fix: add retry loop for nginx startup in port forward tests#213

Closed
claude-claude[bot] wants to merge 3 commits intowork-from-mainfrom
claude/fix-21616317063
Closed

fix: add retry loop for nginx startup in port forward tests#213
claude-claude[bot] wants to merge 3 commits intowork-from-mainfrom
claude/fix-21616317063

Conversation

@claude-claude
Copy link
Copy Markdown
Contributor

@claude-claude claude-claude bot commented Feb 3, 2026

CI Fix

Fixes CI #21615994113

Problem

Tests test_port_forward_rootless and test_port_forward_bridged were flaky, sometimes passing on retry but other times failing all 3 attempts. The tests would fail with "Rootless port forwarding via loopback IP should work" / "Direct access to guest should work".

Root cause: PR #212 changed health checks from HTTP-based (which verified nginx was responding on port 80) to container-ready file mechanism (which only verifies the container started). This created a race condition where:

  1. Container starts and creates the ready file → VM marked as "Healthy"
  2. Test immediately tries to curl nginx
  3. Nginx hasn't finished binding to port 80 yet → curl fails

This is why the tests were flaky - sometimes nginx was fast enough, sometimes not.

Solution

Added 30-second retry loops (with 500ms intervals) to both port forwarding tests. The tests now:

  1. Wait for VM to become healthy (container started)
  2. Retry curling nginx for up to 30 seconds, allowing nginx time to bind to port 80
  3. Only fail if nginx doesn't respond within 30 seconds

This aligns with the new health check behavior while ensuring tests reliably verify port forwarding functionality.


Generated by Claude | Fix Run

ejc3 and others added 3 commits February 3, 2026 03:26
Add slirp_ipv6 test module with tests for:
- libslirp version detection (4.7+ supports native IPv6 DNS)
- DNS resolution in VMs
- IPv6 connectivity on eth0

Change health check default: don't auto-assign HTTP health check URL
from network config. HTTP health checks require an HTTP server in
the container. Use container-ready file mechanism by default instead.
Users can explicitly set --health-check for HTTP checks.

Also add find_available_port() helper to tests/common for port scanning.

Tested:
  make test-root FILTER=slirp_ipv6 - all 3 tests pass
  make test-root FILTER=sanity - sanity test still passes
- Switch from wget to curl for HTTPS testing (busybox wget doesn't
  properly support HTTPS through HTTP proxy)
- Use dynamic port allocation (find_available_high_port) instead of
  hardcoded 8080 to avoid conflicts with system services
- Add --vm flag to clone tests for nslookup/curl (run in VM, not container)
- Update test URLs: google.com → facebook.com, httpbin.org → checkip.amazonaws.com

Tests verified:
  test_egress_fresh_rootless - passed
  test_exec_rootless - passed
  test_port_forward_rootless - passed
The PR changed health checks from HTTP-based (which verified nginx was responding)
to container-ready file (which only verifies container started). This causes a race
condition where the container is marked healthy but nginx hasn't finished binding
to port 80 yet.

Add 30-second retry loops with 500ms intervals to both test_port_forward_bridged
and test_port_forward_rootless, giving nginx time to start after the container
becomes ready.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@ejc3
Copy link
Copy Markdown
Owner

ejc3 commented Feb 3, 2026

Not needed - pasta was removed from branch

@ejc3 ejc3 closed this Feb 3, 2026
@ejc3 ejc3 deleted the claude/fix-21616317063 branch February 8, 2026 18:28
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.

1 participant