Skip to content

Commit e43d9a2

Browse files
sjmiller609claude
andauthored
Add optional snapshot compression defaults and standby integration (#149)
* Add configurable snapshot compression with async standby support * Add CH snapshot compression restore coverage * Skip transient compression temp files during snapshot copy * Clarify async snapshot compression restore behavior * Reduce snapshot compression test races * Restore compression test parallelism * Unify snapshot restore cancellation behavior * Apply suggestions from code review Co-authored-by: Steven Miller <sjmiller609@gmail.com> * Add snapshot compression metrics * Fix snapshot compression review feedback * Update Stainless model config * Serialize shared initrd rebuilds * Fix disabled snapshot defaults fallback * Make snapshot compression fully opt-in * Fix snapshot compression restore races * Fix optional standby body handling * Fix standby snapshot compression races * Normalize standby snapshot compression copies * Handle optional standby bodies outside generated code * Fix snapshot compression cleanup races * Clarify snapshot compression metrics state * Return bad request for invalid standby input * Use native-first snapshot codecs with Go fallback * Normalize snapshot compression algorithms case-insensitively * Tighten standby compression validation handling * Reduce compression levels in integration tests to avoid CI timeout The compression integration tests were using zstd level 19 and lz4 level 9, which are very slow for compressing ~1GB memory files. After merging main (which added more integration tests to lib/instances), the total package test time exceeded the 20-minute CI timeout. Reduce to level 3 for both zstd and lz4 high-level cases. The tests still exercise the full compression/decompression pipeline across both algorithms and multiple levels (1 and 3 for zstd, 0 and 3 for lz4). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Reduce compression integration test cycles to fit CI timeout Five compression cycles (each involving VM standby + compress + restore + boot + exec readiness) consistently exceed the 20-minute CI timeout after merging main. Reduce to three cycles: one in-flight zstd, one completed zstd, and one completed lz4. This still exercises both algorithms and both the in-flight/completed code paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Address PR review feedback: add OpenAPI descriptions, fix dst.Close() error handling - Add descriptions to snapshot_policy and compression fields in openapi.yaml per Steven's review comments - Check dst.Close() errors in runGoCompression and runGoDecompression to prevent silently corrupt snapshot files on delayed write failures Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Address review feedback: add server-side compression validation, log metadata errors - Validate algorithm (zstd/lz4) and per-algorithm level ranges in toDomainSnapshotCompressionConfig instead of passing through unchecked - Log metadata update errors in compression jobs instead of silently discarding them - Normalize algorithm to lowercase in config struct after validation - Fix misleading test name (OmitsLevel -> PreservesLevel) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix snapshot compression review follow-ups * Fix standby fork compression race --------- Co-authored-by: sjmiller609 <7516283+sjmiller609@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 75c3289 commit e43d9a2

51 files changed

Lines changed: 4138 additions & 308 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

cmd/api/api/api_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func newTestService(t *testing.T) *ApiService {
4343
limits := instances.ResourceLimits{
4444
MaxOverlaySize: 100 * 1024 * 1024 * 1024, // 100GB
4545
}
46-
instanceMgr := instances.NewManager(p, imageMgr, systemMgr, networkMgr, deviceMgr, volumeMgr, limits, "", nil, nil)
46+
instanceMgr := instances.NewManager(p, imageMgr, systemMgr, networkMgr, deviceMgr, volumeMgr, limits, "", instances.SnapshotPolicy{}, nil, nil)
4747

4848
// Initialize network manager (creates bridge for network-enabled tests)
4949
if err := networkMgr.Initialize(ctx(), nil); err != nil {

cmd/api/api/instances.go

Lines changed: 103 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/kernel/hypeman/lib/network"
1818
"github.com/kernel/hypeman/lib/oapi"
1919
"github.com/kernel/hypeman/lib/resources"
20+
"github.com/kernel/hypeman/lib/snapshot"
2021
"github.com/kernel/hypeman/lib/vm_metrics"
2122
"github.com/samber/lo"
2223
)
@@ -301,6 +302,16 @@ func (s *ApiService) CreateInstance(ctx context.Context, request oapi.CreateInst
301302
SkipKernelHeaders: request.Body.SkipKernelHeaders != nil && *request.Body.SkipKernelHeaders,
302303
SkipGuestAgent: request.Body.SkipGuestAgent != nil && *request.Body.SkipGuestAgent,
303304
}
305+
if request.Body.SnapshotPolicy != nil {
306+
snapshotPolicy, err := toInstanceSnapshotPolicy(*request.Body.SnapshotPolicy)
307+
if err != nil {
308+
return oapi.CreateInstance400JSONResponse{
309+
Code: "invalid_snapshot_policy",
310+
Message: err.Error(),
311+
}, nil
312+
}
313+
domainReq.SnapshotPolicy = snapshotPolicy
314+
}
304315

305316
inst, err := s.InstanceManager.CreateInstance(ctx, domainReq)
306317
if err != nil {
@@ -438,9 +449,26 @@ func (s *ApiService) StandbyInstance(ctx context.Context, request oapi.StandbyIn
438449
}
439450
log := logger.FromContext(ctx)
440451

441-
result, err := s.InstanceManager.StandbyInstance(ctx, inst.Id)
452+
standbyReq := instances.StandbyInstanceRequest{}
453+
if request.Body != nil && request.Body.Compression != nil {
454+
compression, err := toDomainSnapshotCompressionConfig(*request.Body.Compression)
455+
if err != nil {
456+
return oapi.StandbyInstance400JSONResponse{
457+
Code: "invalid_snapshot_compression",
458+
Message: err.Error(),
459+
}, nil
460+
}
461+
standbyReq.Compression = compression
462+
}
463+
464+
result, err := s.InstanceManager.StandbyInstance(ctx, inst.Id, standbyReq)
442465
if err != nil {
443466
switch {
467+
case errors.Is(err, instances.ErrInvalidRequest):
468+
return oapi.StandbyInstance400JSONResponse{
469+
Code: "invalid_request",
470+
Message: err.Error(),
471+
}, nil
444472
case errors.Is(err, instances.ErrInvalidState):
445473
return oapi.StandbyInstance409JSONResponse{
446474
Code: "invalid_state",
@@ -951,6 +979,10 @@ func instanceToOAPI(inst instances.Instance) oapi.Instance {
951979
if len(inst.Tags) > 0 {
952980
oapiInst.Tags = toOAPITags(inst.Tags)
953981
}
982+
if inst.SnapshotPolicy != nil {
983+
oapiPolicy := toOAPISnapshotPolicy(*inst.SnapshotPolicy)
984+
oapiInst.SnapshotPolicy = &oapiPolicy
985+
}
954986

955987
// Convert volume attachments
956988
if len(inst.Volumes) > 0 {
@@ -985,3 +1017,73 @@ func instanceToOAPI(inst instances.Instance) oapi.Instance {
9851017

9861018
return oapiInst
9871019
}
1020+
1021+
func toDomainSnapshotCompressionConfig(cfg oapi.SnapshotCompressionConfig) (*snapshot.SnapshotCompressionConfig, error) {
1022+
out := &snapshot.SnapshotCompressionConfig{
1023+
Enabled: cfg.Enabled,
1024+
}
1025+
if cfg.Algorithm != nil {
1026+
algo := snapshot.SnapshotCompressionAlgorithm(strings.ToLower(string(*cfg.Algorithm)))
1027+
switch algo {
1028+
case snapshot.SnapshotCompressionAlgorithmZstd, snapshot.SnapshotCompressionAlgorithmLz4:
1029+
default:
1030+
return nil, fmt.Errorf("algorithm must be one of zstd or lz4, got %q", *cfg.Algorithm)
1031+
}
1032+
out.Algorithm = algo
1033+
}
1034+
if cfg.Level != nil {
1035+
level := *cfg.Level
1036+
algo := out.Algorithm
1037+
if algo == "" {
1038+
algo = snapshot.SnapshotCompressionAlgorithmZstd
1039+
}
1040+
switch algo {
1041+
case snapshot.SnapshotCompressionAlgorithmZstd:
1042+
if level < snapshot.MinSnapshotCompressionZstdLevel || level > snapshot.MaxSnapshotCompressionZstdLevel {
1043+
return nil, fmt.Errorf("level must be between %d and %d for zstd, got %d", snapshot.MinSnapshotCompressionZstdLevel, snapshot.MaxSnapshotCompressionZstdLevel, level)
1044+
}
1045+
case snapshot.SnapshotCompressionAlgorithmLz4:
1046+
if level < snapshot.MinSnapshotCompressionLz4Level || level > snapshot.MaxSnapshotCompressionLz4Level {
1047+
return nil, fmt.Errorf("level must be between %d and %d for lz4, got %d", snapshot.MinSnapshotCompressionLz4Level, snapshot.MaxSnapshotCompressionLz4Level, level)
1048+
}
1049+
}
1050+
out.Level = &level
1051+
}
1052+
return out, nil
1053+
}
1054+
1055+
func toInstanceSnapshotPolicy(policy oapi.SnapshotPolicy) (*instances.SnapshotPolicy, error) {
1056+
out := &instances.SnapshotPolicy{}
1057+
if policy.Compression != nil {
1058+
compression, err := toDomainSnapshotCompressionConfig(*policy.Compression)
1059+
if err != nil {
1060+
return nil, err
1061+
}
1062+
out.Compression = compression
1063+
}
1064+
return out, nil
1065+
}
1066+
1067+
func toOAPISnapshotCompressionConfig(cfg snapshot.SnapshotCompressionConfig) oapi.SnapshotCompressionConfig {
1068+
out := oapi.SnapshotCompressionConfig{
1069+
Enabled: cfg.Enabled,
1070+
}
1071+
if cfg.Algorithm != "" {
1072+
algo := oapi.SnapshotCompressionConfigAlgorithm(cfg.Algorithm)
1073+
out.Algorithm = &algo
1074+
}
1075+
if cfg.Level != nil {
1076+
level := *cfg.Level
1077+
out.Level = &level
1078+
}
1079+
return out
1080+
}
1081+
1082+
func toOAPISnapshotPolicy(policy instances.SnapshotPolicy) oapi.SnapshotPolicy {
1083+
out := oapi.SnapshotPolicy{}
1084+
if policy.Compression != nil {
1085+
compression := toOAPISnapshotCompressionConfig(*policy.Compression)
1086+
out.Compression = &compression
1087+
}
1088+
return out
1089+
}

cmd/api/api/instances_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,14 @@ type captureForkManager struct {
204204
err error
205205
}
206206

207+
type captureStandbyManager struct {
208+
instances.Manager
209+
lastID string
210+
lastReq *instances.StandbyInstanceRequest
211+
result *instances.Instance
212+
err error
213+
}
214+
207215
type captureUpdateManager struct {
208216
instances.Manager
209217
lastID string
@@ -222,6 +230,16 @@ func (m *captureForkManager) ForkInstance(ctx context.Context, id string, req in
222230
return m.result, nil
223231
}
224232

233+
func (m *captureStandbyManager) StandbyInstance(ctx context.Context, id string, req instances.StandbyInstanceRequest) (*instances.Instance, error) {
234+
reqCopy := req
235+
m.lastID = id
236+
m.lastReq = &reqCopy
237+
if m.err != nil {
238+
return nil, m.err
239+
}
240+
return m.result, nil
241+
}
242+
225243
func (m *captureUpdateManager) UpdateInstance(ctx context.Context, id string, req instances.UpdateInstanceRequest) (*instances.Instance, error) {
226244
reqCopy := req
227245
m.lastID = id
@@ -677,6 +695,46 @@ func TestForkInstance_InvalidRequest(t *testing.T) {
677695
assert.Equal(t, "invalid_request", badReq.Code)
678696
}
679697

698+
func TestStandbyInstance_InvalidRequest(t *testing.T) {
699+
t.Parallel()
700+
svc := newTestService(t)
701+
702+
source := instances.Instance{
703+
StoredMetadata: instances.StoredMetadata{
704+
Id: "standby-src",
705+
Name: "standby-src",
706+
Image: "docker.io/library/alpine:latest",
707+
CreatedAt: time.Now(),
708+
HypervisorType: hypervisor.TypeCloudHypervisor,
709+
},
710+
State: instances.StateStopped,
711+
}
712+
713+
mockMgr := &captureStandbyManager{
714+
Manager: svc.InstanceManager,
715+
err: fmt.Errorf("%w: invalid snapshot compression level", instances.ErrInvalidRequest),
716+
}
717+
svc.InstanceManager = mockMgr
718+
719+
resp, err := svc.StandbyInstance(
720+
mw.WithResolvedInstance(ctx(), source.Id, source),
721+
oapi.StandbyInstanceRequestObject{
722+
Id: source.Id,
723+
Body: &oapi.StandbyInstanceRequest{
724+
Compression: &oapi.SnapshotCompressionConfig{
725+
Enabled: true,
726+
},
727+
},
728+
},
729+
)
730+
require.NoError(t, err)
731+
732+
badReq, ok := resp.(oapi.StandbyInstance400JSONResponse)
733+
require.True(t, ok, "expected 400 response")
734+
assert.Equal(t, "invalid_request", badReq.Code)
735+
assert.Contains(t, badReq.Message, "invalid snapshot compression level")
736+
}
737+
680738
func TestForkInstance_FromRunningFlagForwarded(t *testing.T) {
681739
t.Parallel()
682740
svc := newTestService(t)
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package api
2+
3+
import (
4+
"io"
5+
"net/http"
6+
"strings"
7+
)
8+
9+
// NormalizeOptionalStandbyBody rewrites empty standby POST bodies to "{}"
10+
// so the generated strict handler can decode them without special casing.
11+
func NormalizeOptionalStandbyBody(next http.Handler) http.Handler {
12+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
13+
if r.Method != http.MethodPost {
14+
next.ServeHTTP(w, r)
15+
return
16+
}
17+
if isStandbyRoutePath(r.URL.Path) && requestBodyIsEmpty(r) {
18+
r.Body = io.NopCloser(strings.NewReader(`{}`))
19+
r.ContentLength = 2
20+
if r.Header.Get("Content-Type") == "" {
21+
r.Header.Set("Content-Type", "application/json")
22+
}
23+
}
24+
25+
next.ServeHTTP(w, r)
26+
})
27+
}
28+
29+
func isStandbyRoutePath(path string) bool {
30+
if !strings.HasPrefix(path, "/instances/") || !strings.HasSuffix(path, "/standby") {
31+
return false
32+
}
33+
34+
instanceID := strings.TrimPrefix(path, "/instances/")
35+
instanceID = strings.TrimSuffix(instanceID, "/standby")
36+
return instanceID != "" && !strings.Contains(instanceID, "/")
37+
}
38+
39+
func requestBodyIsEmpty(r *http.Request) bool {
40+
if r == nil {
41+
return true
42+
}
43+
if r.Body == nil || r.Body == http.NoBody {
44+
return true
45+
}
46+
return r.ContentLength == 0 && len(r.TransferEncoding) == 0
47+
}
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
package api
2+
3+
import (
4+
"bytes"
5+
"io"
6+
"net/http"
7+
"net/http/httptest"
8+
"testing"
9+
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
func TestNormalizeOptionalStandbyBody(t *testing.T) {
15+
t.Parallel()
16+
17+
t.Run("empty standby body becomes empty JSON object", func(t *testing.T) {
18+
t.Parallel()
19+
20+
var gotBody []byte
21+
var gotContentType string
22+
next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
23+
var err error
24+
gotBody, err = io.ReadAll(r.Body)
25+
require.NoError(t, err)
26+
gotContentType = r.Header.Get("Content-Type")
27+
w.WriteHeader(http.StatusNoContent)
28+
})
29+
30+
req := httptest.NewRequest(http.MethodPost, "/instances/test/standby", nil)
31+
rec := httptest.NewRecorder()
32+
33+
NormalizeOptionalStandbyBody(next).ServeHTTP(rec, req)
34+
35+
assert.Equal(t, http.StatusNoContent, rec.Code)
36+
assert.Equal(t, []byte(`{}`), gotBody)
37+
assert.Equal(t, "application/json", gotContentType)
38+
})
39+
40+
t.Run("existing standby body is preserved", func(t *testing.T) {
41+
t.Parallel()
42+
43+
var gotBody []byte
44+
next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
45+
var err error
46+
gotBody, err = io.ReadAll(r.Body)
47+
require.NoError(t, err)
48+
w.WriteHeader(http.StatusNoContent)
49+
})
50+
51+
req := httptest.NewRequest(http.MethodPost, "/instances/test/standby", bytes.NewBufferString(`{"compression":{"enabled":true}}`))
52+
rec := httptest.NewRecorder()
53+
54+
NormalizeOptionalStandbyBody(next).ServeHTTP(rec, req)
55+
56+
assert.Equal(t, http.StatusNoContent, rec.Code)
57+
assert.Equal(t, []byte(`{"compression":{"enabled":true}}`), gotBody)
58+
})
59+
60+
t.Run("non-standby route is untouched", func(t *testing.T) {
61+
t.Parallel()
62+
63+
var gotBody []byte
64+
next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
65+
var err error
66+
gotBody, err = io.ReadAll(r.Body)
67+
require.NoError(t, err)
68+
w.WriteHeader(http.StatusNoContent)
69+
})
70+
71+
req := httptest.NewRequest(http.MethodPost, "/instances/test/start", nil)
72+
rec := httptest.NewRecorder()
73+
74+
NormalizeOptionalStandbyBody(next).ServeHTTP(rec, req)
75+
76+
assert.Equal(t, http.StatusNoContent, rec.Code)
77+
assert.Empty(t, gotBody)
78+
})
79+
80+
t.Run("non-post request skips standby normalization", func(t *testing.T) {
81+
t.Parallel()
82+
83+
var gotBody []byte
84+
next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
85+
var err error
86+
gotBody, err = io.ReadAll(r.Body)
87+
require.NoError(t, err)
88+
w.WriteHeader(http.StatusNoContent)
89+
})
90+
91+
req := httptest.NewRequest(http.MethodGet, "/instances/test/standby", nil)
92+
rec := httptest.NewRecorder()
93+
94+
NormalizeOptionalStandbyBody(next).ServeHTTP(rec, req)
95+
96+
assert.Equal(t, http.StatusNoContent, rec.Code)
97+
assert.Empty(t, gotBody)
98+
})
99+
100+
t.Run("standby route matcher only accepts single path segment ids", func(t *testing.T) {
101+
t.Parallel()
102+
103+
assert.True(t, isStandbyRoutePath("/instances/test/standby"))
104+
assert.False(t, isStandbyRoutePath("/instances/test/start"))
105+
assert.False(t, isStandbyRoutePath("/instances/test/standby/extra"))
106+
assert.False(t, isStandbyRoutePath("/instances/test/nested/standby"))
107+
})
108+
}

0 commit comments

Comments
 (0)