Skip to content

Commit 74d2f2b

Browse files
committed
fix: clamp pull_conns; surface silent reseed skips; test the retry rule
- EffectivePullConns was only lower-bounded; a misconfigured pull_conns (e.g. 100000) fanned out that many connections/goroutines and exhausted fds. Clamp to maxPullConns (64). splitRanges also guards n<1 so it can never divide by zero. - The no-vsock skip on the auto clone/restore path logged at Debug, so a legacy VM silently kept the snapshot's CRNG state. Warn instead. - Add TestReseedVM_AgentRejectionNotRetried: a mock agent that answers then rejects proves a live reply is final and not billed the dial retry budget (the exact rule 2010949 introduced, previously untested).
1 parent 257529a commit 74d2f2b

4 files changed

Lines changed: 66 additions & 3 deletions

File tree

cmd/vm/reseed.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,8 @@ func signalReseed(ctx context.Context, vm *types.VM, regenMachineID bool) bool {
9292
return false
9393
}
9494
if vm.VsockSocket == "" {
95-
logger.Debug(ctx, "skip reseed signal: vsock not configured")
95+
// Not Debug: a legacy VM silently keeps the snapshot's CRNG state.
96+
logger.Warnf(ctx, "skip reseed for %s: no vsock; this clone keeps the snapshot's CRNG state — recreate the VM to enable the agent", vm.ID)
9697
return false
9798
}
9899
if err := reseedVM(ctx, vm, regenMachineID); err != nil {

cmd/vm/reseed_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,19 @@
11
package vm
22

33
import (
4+
"bufio"
45
"context"
56
"errors"
7+
"io"
8+
"net"
9+
"os"
10+
"path/filepath"
11+
"strings"
12+
"sync/atomic"
613
"testing"
714

15+
"github.com/cocoonstack/cocoon-agent/agent"
16+
817
"github.com/cocoonstack/cocoon/types"
918
)
1019

@@ -39,3 +48,51 @@ func TestReseedVM_CtxCanceled(t *testing.T) {
3948
t.Fatalf("got err=%v, want context.Canceled", err)
4049
}
4150
}
51+
52+
// TestReseedVM_AgentRejectionNotRetried pins the dial-vs-rejection distinction:
53+
// once the agent answers, its reply is final and must not be billed the retry budget.
54+
func TestReseedVM_AgentRejectionNotRetried(t *testing.T) {
55+
// A short dir: unix socket paths are capped at ~104 bytes, which t.TempDir() blows past.
56+
dir, err := os.MkdirTemp("/tmp", "rsd")
57+
if err != nil {
58+
t.Fatalf("temp dir: %v", err)
59+
}
60+
defer func() { _ = os.RemoveAll(dir) }()
61+
sock := filepath.Join(dir, "s.sock")
62+
ln, err := net.Listen("unix", sock)
63+
if err != nil {
64+
t.Fatalf("listen: %v", err)
65+
}
66+
defer func() { _ = ln.Close() }()
67+
68+
var accepts atomic.Int32
69+
done := make(chan struct{})
70+
go func() {
71+
defer close(done)
72+
for {
73+
conn, err := ln.Accept()
74+
if err != nil {
75+
return
76+
}
77+
accepts.Add(1)
78+
br := bufio.NewReader(conn)
79+
_, _ = br.ReadString('\n') // CONNECT <port>
80+
_, _ = io.WriteString(conn, "OK 1\n") // hybrid-vsock handshake ack
81+
_, _ = br.ReadString('\n') // reseed opening frame
82+
// A live agent rejects (e.g. version skew); reseedVM must surface it, not retry.
83+
_ = agent.NewEncoder(conn).Encode(agent.Message{Type: agent.MsgError, Message: "version skew"})
84+
_ = conn.Close()
85+
}
86+
}()
87+
88+
vm := &types.VM{ID: "vm1", VsockSocket: sock}
89+
err = reseedVM(t.Context(), vm, false)
90+
if err == nil || !strings.Contains(err.Error(), "version skew") {
91+
t.Fatalf("err = %v, want it to carry the agent's rejection", err)
92+
}
93+
_ = ln.Close()
94+
<-done
95+
if n := accepts.Load(); n != 1 {
96+
t.Errorf("agent accepted %d times; a live rejection must not be retried (want 1)", n)
97+
}
98+
}

config/config.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ const (
1616

1717
// defaultPullConns is the default concurrent Range connections per cloud-image download.
1818
defaultPullConns = 8
19+
// maxPullConns caps it so a misconfigured pull_conns can't exhaust file descriptors.
20+
maxPullConns = 64
1921
)
2022

2123
// HypervisorType identifies the selected hypervisor backend.
@@ -69,9 +71,9 @@ func (c *Config) EffectivePoolSize() int {
6971
return utils.PoolSizeOrDefault(c.PoolSize)
7072
}
7173

72-
// EffectivePullConns returns PullConns if set, otherwise defaultPullConns.
74+
// EffectivePullConns returns PullConns (defaultPullConns when unset) clamped to maxPullConns.
7375
func (c *Config) EffectivePullConns() int {
74-
return utils.OrDefault(c.PullConns, defaultPullConns)
76+
return min(utils.OrDefault(c.PullConns, defaultPullConns), maxPullConns)
7577
}
7678

7779
// Validate checks that all config fields are within acceptable ranges.

images/cloudimg/pull.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,9 @@ func hashDigest(dst *os.File) (string, error) {
313313

314314
// splitRanges divides [0,size) into up to n contiguous, inclusive-ended byte ranges.
315315
func splitRanges(size int64, n int) []byteRange {
316+
if n < 1 {
317+
n = 1
318+
}
316319
chunk := (size + int64(n) - 1) / int64(n)
317320
ranges := make([]byteRange, 0, n)
318321
for start := int64(0); start < size; start += chunk {

0 commit comments

Comments
 (0)