Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -340,5 +340,5 @@ func (t *storageTemplate) UpdateMetadata(meta metadata.Template) error {
return fmt.Errorf("failed to get metafile: %w", err)
}

return meta.ToFile(metafile.Path())
return meta.ReplaceFile(metafile.Path())
}
13 changes: 9 additions & 4 deletions packages/orchestrator/pkg/server/prefetch_harvest.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package server

import (
"context"
"errors"
"fmt"
"maps"
"slices"
Expand Down Expand Up @@ -263,21 +264,25 @@ func (h *prefetchHarvester) run(
// update is enough for a same-node resume, so do it regardless of whether the
// remote upload succeeded.
meta = meta.WithPrefetch(&metadata.Prefetch{Memory: mapping})
var localUpdateErr error
if err := h.templates.UpdateMetadata(buildID, meta); err != nil {
return pages, harvestSuccess, fmt.Errorf("update local metadata: %w", err)
localUpdateErr = fmt.Errorf("update local metadata: %w", err)
if !errors.Is(err, metadata.ErrReplaceCommitted) {
return pages, harvestSuccess, localUpdateErr
}
}

// Only enrich the remote metadata if the snapshot actually landed; on upload
// failure the remote build is incomplete, so there is nothing to enrich (the
// local update above still lets a same-node resume prefetch).
if uploadErr != nil {
return pages, harvestSuccess, nil //nolint:nilerr // remote snapshot did not land; the local update is the most we can do
return pages, harvestSuccess, localUpdateErr
}
if err := h.uploadMetadata(ctx, meta, objectMetadata); err != nil {
return pages, harvestSuccess, fmt.Errorf("re-upload metadata: %w", err)
return pages, harvestSuccess, errors.Join(localUpdateErr, fmt.Errorf("re-upload metadata: %w", err))
}

return pages, harvestSuccess, nil
return pages, harvestSuccess, localUpdateErr
}

// resumeMapping resumes a throwaway warm copy of the just-paused snapshot,
Expand Down
16 changes: 16 additions & 0 deletions packages/orchestrator/pkg/server/prefetch_harvest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package server
import (
"context"
"errors"
"fmt"
"testing"
"time"

Expand Down Expand Up @@ -341,6 +342,21 @@ func TestHarvestRun_UploadFailedKeepsLocalSkipsRemote(t *testing.T) {
require.False(t, p.uploadCalled, "remote re-upload is skipped when the snapshot did not land")
}

func TestHarvestRun_CommittedLocalUpdateErrorStillUpdatesRemote(t *testing.T) {
t.Parallel()

p := newHarvestProbe()
p.tmpls.updateErr = fmt.Errorf("directory sync failed: %w", metadata.ErrReplaceCommitted)

pages, outcome, err := p.run(t.Context(), true)

require.ErrorIs(t, err, metadata.ErrReplaceCommitted)
require.Equal(t, harvestSuccess, outcome)
require.Equal(t, 2, pages)
require.True(t, p.tmpls.updated)
require.True(t, p.uploadCalled, "remote metadata should be updated after the local replacement commits")
}

// TestHarvestRun_DeadlineDuringWaitLeavesMetadataUntouched: if the harvest
// deadline fires while waiting for the upload, neither local nor remote metadata
// is touched (the upload may still be reading/writing them).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,35 +57,36 @@ func TestWithPrefetchSetsMappingAndPreservesFields(t *testing.T) {
}

// TestPrefetchSurvivesMetadataFileRoundTrip is the persist→resume link the
// consume path depends on: the harvest writes the mapping via ToFile/UploadMetadata
// consume path depends on: the harvest writes the mapping via ReplaceFile/UploadMetadata
// and the resume reads it via FromFile. If Prefetch did not survive JSON
// serialization (e.g. a missing/renamed tag), the whole feature would silently
// no-op and every in-memory test above would still pass — so assert the round trip.
func TestPrefetchSurvivesMetadataFileRoundTrip(t *testing.T) {
t.Parallel()

orig := Template{
base := Template{
Version: CurrentVersion,
Template: TemplateMetadata{BuildID: "build-rt", KernelVersion: "6.1", FirecrackerVersion: "1.14"},
Context: Context{User: "root"},
Prefetch: &Prefetch{Memory: &MemoryPrefetchMapping{
Indices: []uint64{3, 1, 2},
AccessTypes: []AccessType{"r", "w", "p"},
BlockSize: 2 << 20,
}},
}
updated := base.WithPrefetch(&Prefetch{Memory: &MemoryPrefetchMapping{
Indices: []uint64{3, 1, 2},
AccessTypes: []AccessType{"r", "w", "p"},
BlockSize: 2 << 20,
}})

path := filepath.Join(t.TempDir(), "metadata.json")
require.NoError(t, orig.ToFile(path))
require.NoError(t, base.ToFile(path))
require.NoError(t, updated.ReplaceFile(path))

got, err := FromFile(path)
require.NoError(t, err)

require.NotNil(t, got.Prefetch, "Prefetch must survive ToFile/FromFile")
require.NotNil(t, got.Prefetch.Memory)
assert.Equal(t, orig.Prefetch.Memory.Indices, got.Prefetch.Memory.Indices, "ordered indices must be preserved")
assert.Equal(t, orig.Prefetch.Memory.AccessTypes, got.Prefetch.Memory.AccessTypes)
assert.Equal(t, orig.Prefetch.Memory.BlockSize, got.Prefetch.Memory.BlockSize)
assert.Equal(t, updated.Prefetch.Memory.Indices, got.Prefetch.Memory.Indices, "ordered indices must be preserved")
assert.Equal(t, updated.Prefetch.Memory.AccessTypes, got.Prefetch.Memory.AccessTypes)
assert.Equal(t, updated.Prefetch.Memory.BlockSize, got.Prefetch.Memory.BlockSize)
}

func TestMemoryPrefetchMappingCount(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ const (
FilesystemOnlyVersion = DeprecatedVersion + 1
)

// ErrReplaceCommitted means the replacement is visible, but parent-directory
// durability could not be confirmed.
var ErrReplaceCommitted = ioutils.ErrAtomicWriteCommitted

var tracer = otel.Tracer("github.com/e2b-dev/infra/packages/orchestrator/pkg/template/metadata")

// AccessType is a compact representation of block access type for JSON serialization.
Expand Down Expand Up @@ -221,6 +225,20 @@ func (t Template) ToFile(path string) error {
return nil
}

func (t Template) ReplaceFile(path string) error {
mr, err := serialize(t)
if err != nil {
return err
}

err = ioutils.WriteToFileFromReaderAtomically(path, mr)
if err != nil {
return fmt.Errorf("failed to replace metadata file: %w", err)
}

return nil
}

func FromFile(path string) (Template, error) {
f, err := os.Open(path)
if err != nil {
Expand Down
82 changes: 77 additions & 5 deletions packages/shared/pkg/ioutils/write_to_file.go
Original file line number Diff line number Diff line change
@@ -1,26 +1,98 @@
package ioutils

import (
"errors"
"fmt"
"io"
"os"
"path/filepath"
)

var ErrAtomicWriteCommitted = errors.New("atomic write committed")

func WriteToFileFromReader(path string, r io.Reader) (err error) {
// Create (truncate if exists) with 0644 perms
f, err := os.Create(path)
if err != nil {
return err
}
// Make sure we return a close error if that's the only error.
defer func() {
if cerr := f.Close(); err == nil && cerr != nil {
err = cerr
if closeErr := f.Close(); err == nil && closeErr != nil {
err = closeErr
}
}()

if _, err = io.Copy(f, r); err != nil {
return err
}

return f.Sync()
}

// WriteToFileFromReaderAtomically replaces an existing regular file without
// exposing partial contents. Errors wrapping ErrAtomicWriteCommitted mean the
// replacement is visible, but parent-directory durability could not be confirmed.
func WriteToFileFromReaderAtomically(path string, r io.Reader) (err error) {
info, err := os.Lstat(path)
if err != nil {
return err
}
if !info.Mode().IsRegular() {
return fmt.Errorf("atomic write target %q is not a regular file", path)
}

dir := filepath.Dir(path)
f, err := os.CreateTemp(dir, ".atomic-write-*")
if err != nil {
return err
}
tempPath := f.Name()
closed := false
committed := false
defer func() {
if !closed {
if closeErr := f.Close(); closeErr != nil {
err = errors.Join(err, fmt.Errorf("close temporary file: %w", closeErr))
}
}
if !committed {
if removeErr := os.Remove(tempPath); removeErr != nil && !errors.Is(removeErr, os.ErrNotExist) {
err = errors.Join(err, fmt.Errorf("remove temporary file: %w", removeErr))
}
}
}()

if _, err = io.Copy(f, r); err != nil {
return err
}
if err = f.Chmod(info.Mode().Perm()); err != nil {
return err
}
if err = f.Sync(); err != nil {
return err
}
err = f.Close()
closed = true
if err != nil {
return err
}

if err = os.Rename(tempPath, path); err != nil {
return err
}
committed = true

if err = syncDirectory(dir); err != nil {
return fmt.Errorf("%w: sync parent directory: %w", ErrAtomicWriteCommitted, err)
}

return nil
}

func syncDirectory(path string) error {
dir, err := os.Open(path)
if err != nil {
return err
}

return f.Sync() // ensure contents hit disk
return errors.Join(dir.Sync(), dir.Close())
}
Loading
Loading