Skip to content

Commit b4d0073

Browse files
authored
chore: test reliability and speed skill (#133)
* Stabilize integration network and ingress test flakes * Speed up fork-from-running integration tests * Reduce fork test agent wait latency * Add agent prompt * Convert to skill
1 parent 9a87bd5 commit b4d0073

8 files changed

Lines changed: 325 additions & 29 deletions

File tree

lib/ingress/binaries_linux.go

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"log/slog"
1010
"os"
1111
"path/filepath"
12+
"syscall"
1213

1314
"github.com/kernel/hypeman/lib/paths"
1415
)
@@ -57,13 +58,29 @@ func ExtractCaddyBinary(p *paths.Paths) (string, error) {
5758
return "", fmt.Errorf("create caddy binary dir: %w", err)
5859
}
5960

60-
// Write binary to filesystem
61-
if err := os.WriteFile(extractPath, data, 0755); err != nil {
61+
lockFile, err := os.OpenFile(extractPath+".lock", os.O_CREATE|os.O_RDWR, 0644)
62+
if err != nil {
63+
return "", fmt.Errorf("open extraction lock: %w", err)
64+
}
65+
defer lockFile.Close()
66+
if err := syscall.Flock(int(lockFile.Fd()), syscall.LOCK_EX); err != nil {
67+
return "", fmt.Errorf("lock extraction: %w", err)
68+
}
69+
defer syscall.Flock(int(lockFile.Fd()), syscall.LOCK_UN)
70+
71+
// Another process may have extracted it while we waited for the lock.
72+
if _, err := os.Stat(extractPath); err == nil {
73+
if storedHash, err := os.ReadFile(hashPath); err == nil && string(storedHash) == embeddedHash {
74+
return extractPath, nil
75+
}
76+
}
77+
78+
if err := atomicWriteExecutable(extractPath, data); err != nil {
6279
return "", fmt.Errorf("write caddy binary: %w", err)
6380
}
6481

6582
// Write hash file for future comparisons
66-
if err := os.WriteFile(hashPath, []byte(embeddedHash), 0644); err != nil {
83+
if err := atomicWriteFile(hashPath, []byte(embeddedHash), 0644); err != nil {
6784
// Non-fatal - binary is extracted, just won't have hash for next time
6885
// This could cause unnecessary re-extractions but won't break functionality
6986
slog.Info("failed to write caddy binary hash file", "path", hashPath, "error", err)
@@ -76,3 +93,39 @@ func ExtractCaddyBinary(p *paths.Paths) (string, error) {
7693
func GetCaddyBinaryPath(p *paths.Paths) (string, error) {
7794
return ExtractCaddyBinary(p)
7895
}
96+
97+
func atomicWriteExecutable(path string, data []byte) error {
98+
return atomicWriteFile(path, data, 0755)
99+
}
100+
101+
func atomicWriteFile(path string, data []byte, mode os.FileMode) error {
102+
dir := filepath.Dir(path)
103+
tmpFile, err := os.CreateTemp(dir, "caddy-*")
104+
if err != nil {
105+
return fmt.Errorf("create temp file: %w", err)
106+
}
107+
tmpPath := tmpFile.Name()
108+
cleanupTmp := true
109+
defer func() {
110+
if cleanupTmp {
111+
_ = os.Remove(tmpPath)
112+
}
113+
}()
114+
115+
if _, err := tmpFile.Write(data); err != nil {
116+
_ = tmpFile.Close()
117+
return fmt.Errorf("write temp file: %w", err)
118+
}
119+
if err := tmpFile.Chmod(mode); err != nil {
120+
_ = tmpFile.Close()
121+
return fmt.Errorf("chmod temp file: %w", err)
122+
}
123+
if err := tmpFile.Close(); err != nil {
124+
return fmt.Errorf("close temp file: %w", err)
125+
}
126+
if err := os.Rename(tmpPath, path); err != nil {
127+
return fmt.Errorf("install file: %w", err)
128+
}
129+
cleanupTmp = false
130+
return nil
131+
}

lib/instances/firecracker_test.go

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ package instances
44

55
import (
66
"context"
7+
"fmt"
8+
"net"
9+
"net/http"
710
"os"
811
"path/filepath"
912
"strings"
@@ -87,6 +90,31 @@ func createNginxImageAndWait(t *testing.T, ctx context.Context, imageManager ima
8790
t.Fatalf("timed out waiting for image %q to become ready", nginxImage.Name)
8891
}
8992

93+
func startGatewayProbeServer(t *testing.T, gatewayIP string) (string, func()) {
94+
t.Helper()
95+
96+
listener, err := net.Listen("tcp", net.JoinHostPort(gatewayIP, "0"))
97+
require.NoError(t, err)
98+
99+
mux := http.NewServeMux()
100+
mux.HandleFunc("/probe", func(w http.ResponseWriter, r *http.Request) {
101+
_, _ = w.Write([]byte("Connection successful"))
102+
})
103+
104+
server := &http.Server{Handler: mux}
105+
go func() {
106+
_ = server.Serve(listener)
107+
}()
108+
109+
cleanup := func() {
110+
shutdownCtx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
111+
defer cancel()
112+
_ = server.Shutdown(shutdownCtx)
113+
}
114+
115+
return fmt.Sprintf("http://%s/probe", listener.Addr().String()), cleanup
116+
}
117+
90118
func TestFirecrackerStandbyAndRestore(t *testing.T) {
91119
t.Parallel()
92120
requireFirecrackerIntegrationPrereqs(t)
@@ -247,14 +275,17 @@ func TestFirecrackerNetworkLifecycle(t *testing.T) {
247275
_, isBridge := master.(*netlink.Bridge)
248276
assert.True(t, isBridge, "TAP should be attached to a bridge")
249277

278+
probeURL, stopProbeServer := startGatewayProbeServer(t, alloc.Gateway)
279+
t.Cleanup(stopProbeServer)
280+
250281
require.NoError(t, waitForLogMessage(ctx, mgr, inst.Id, "start worker processes", 15*time.Second))
251282
require.NoError(t, waitForLogMessage(ctx, mgr, inst.Id, "[guest-agent] listening", 10*time.Second))
252283

253-
// Retry to reduce flakiness while guest network stack settles.
284+
// Retry while guest network stack settles.
254285
var output string
255286
var exitCode int
256287
for i := 0; i < 10; i++ {
257-
output, exitCode, err = execCommand(ctx, inst, "curl", "-s", "--connect-timeout", "10", "https://public-ping-bucket-kernel.s3.us-east-1.amazonaws.com/index.html")
288+
output, exitCode, err = execCommand(ctx, inst, "curl", "-sS", "--connect-timeout", "10", probeURL)
258289
if err == nil && exitCode == 0 {
259290
break
260291
}
@@ -294,7 +325,7 @@ func TestFirecrackerNetworkLifecycle(t *testing.T) {
294325
assert.Equal(t, uint8(netlink.OperUp), uint8(tapRestored.Attrs().OperState))
295326

296327
for i := 0; i < 10; i++ {
297-
output, exitCode, err = execCommand(ctx, inst, "curl", "-s", "https://public-ping-bucket-kernel.s3.us-east-1.amazonaws.com/index.html")
328+
output, exitCode, err = execCommand(ctx, inst, "curl", "-sS", "--connect-timeout", "10", probeURL)
298329
if err == nil && exitCode == 0 {
299330
break
300331
}
@@ -379,7 +410,6 @@ func TestFirecrackerForkFromRunningNetwork(t *testing.T) {
379410

380411
assertHostCanReachNginx(t, sourceAfterFork.IP, 80, 60*time.Second)
381412
assertHostCanReachNginx(t, forked.IP, 80, 60*time.Second)
382-
assertHostCanReachNginx(t, sourceAfterFork.IP, 80, 60*time.Second)
383413
assert.NotEqual(t, sourceAfterFork.IP, forked.IP)
384414
assert.NotEqual(t, sourceAfterFork.MAC, forked.MAC)
385415
}

lib/instances/fork_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,6 @@ func TestForkCloudHypervisorFromRunningNetwork(t *testing.T) {
419419
assert.NotEqual(t, sourceAfterFork.MAC, forked.MAC)
420420
assertGuestHasOnlyExpectedIPv4(t, forked, forked.IP, 30*time.Second)
421421
assertHostCanReachNginx(t, forked.IP, 80, 60*time.Second)
422-
assertHostCanReachNginx(t, sourceAfterFork.IP, 80, 60*time.Second)
423422
}
424423

425424
func assertHostCanReachNginx(t *testing.T, ip string, port int, timeout time.Duration) {
@@ -495,7 +494,7 @@ func execInInstance(ctx context.Context, inst *Instance, command ...string) (str
495494
Command: command,
496495
Stdout: &stdout,
497496
Stderr: &stderr,
498-
WaitForAgent: 30 * time.Second,
497+
WaitForAgent: 2 * time.Second,
499498
})
500499
if err != nil {
501500
return "", -1, err

lib/instances/qemu_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -955,7 +955,6 @@ func TestQEMUForkFromRunningNetwork(t *testing.T) {
955955
assert.NotEqual(t, sourceAfterFork.IP, forked.IP)
956956
assert.NotEqual(t, sourceAfterFork.MAC, forked.MAC)
957957
assertHostCanReachNginx(t, forked.IP, 80, 60*time.Second)
958-
assertHostCanReachNginx(t, sourceAfterFork.IP, 80, 60*time.Second)
959958
}
960959

961960
func TestQEMUSnapshotFeature(t *testing.T) {

lib/network/allocate.go

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
mathrand "math/rand"
88
"net"
99
"strings"
10+
"time"
1011

1112
"github.com/kernel/hypeman/lib/logger"
1213
)
@@ -22,17 +23,9 @@ func (m *manager) CreateAllocation(ctx context.Context, req AllocateRequest) (*N
2223
log := logger.FromContext(ctx)
2324

2425
// 1. Get default network
25-
network, err := m.getDefaultNetwork(ctx)
26+
network, err := m.getDefaultNetworkWithSelfHeal(ctx)
2627
if err != nil {
27-
// Self-heal if bridge state was externally removed after initialization.
28-
// This keeps allocations robust under highly concurrent test workloads.
29-
if initErr := m.Initialize(ctx, nil); initErr != nil {
30-
return nil, fmt.Errorf("get default network: %w", err)
31-
}
32-
network, err = m.getDefaultNetwork(ctx)
33-
if err != nil {
34-
return nil, fmt.Errorf("get default network: %w", err)
35-
}
28+
return nil, fmt.Errorf("get default network: %w", err)
3629
}
3730

3831
// 2. Check name uniqueness (exclude current instance to allow restarts)
@@ -113,16 +106,9 @@ func (m *manager) RecreateAllocation(ctx context.Context, instanceID string, dow
113106
}
114107

115108
// 2. Get default network details
116-
network, err := m.getDefaultNetwork(ctx)
109+
network, err := m.getDefaultNetworkWithSelfHeal(ctx)
117110
if err != nil {
118-
// Same self-healing behavior as CreateAllocation.
119-
if initErr := m.Initialize(ctx, nil); initErr != nil {
120-
return fmt.Errorf("get default network: %w", err)
121-
}
122-
network, err = m.getDefaultNetwork(ctx)
123-
if err != nil {
124-
return fmt.Errorf("get default network: %w", err)
125-
}
111+
return fmt.Errorf("get default network: %w", err)
126112
}
127113

128114
// 3. Recreate TAP device with same name and rate limits from instance metadata
@@ -239,6 +225,31 @@ func (m *manager) allocateNextIP(ctx context.Context, subnet string) (string, er
239225
return "", fmt.Errorf("no available IPs in subnet %s after %d random attempts and full scan", subnet, maxRetries)
240226
}
241227

228+
func (m *manager) getDefaultNetworkWithSelfHeal(ctx context.Context) (*Network, error) {
229+
network, err := m.getDefaultNetwork(ctx)
230+
if err == nil {
231+
return network, nil
232+
}
233+
234+
// Self-heal if bridge state was externally removed after initialization.
235+
// After re-initialization, kernel bridge/IP state may take a brief moment to become visible.
236+
if initErr := m.Initialize(ctx, nil); initErr != nil {
237+
return nil, err
238+
}
239+
240+
deadline := time.Now().Add(2 * time.Second)
241+
for {
242+
network, err = m.getDefaultNetwork(ctx)
243+
if err == nil {
244+
return network, nil
245+
}
246+
if time.Now().After(deadline) {
247+
return nil, err
248+
}
249+
time.Sleep(100 * time.Millisecond)
250+
}
251+
}
252+
242253
// incrementIP increments IP address by n
243254
func incrementIP(ip net.IP, n int) net.IP {
244255
// Ensure we're working with IPv4 (4 bytes)

skills/test-agent/SKILL.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
name: test-agent
3+
description: Improve repository test quality by eliminating flakes first, then reducing slow-test redundancy, and finally optimizing longest tests while preserving coverage. Use when asked to repeatedly run CI-equivalent no-cache test cycles, diagnose flaky tests, and document findings in this skill's notes.
4+
---
5+
6+
Follow the workflow and constraints documented in [agents/test-agent/PROMPT.md](agents/test-agent/PROMPT.md). Do not modify this file.
7+
8+
Record investigation and timing notes in [agents/test-agent/NOTES.md](agents/test-agent/NOTES.md), use this file as memory of the investigation, reference or search it where needed for context.

0 commit comments

Comments
 (0)