Skip to content

Commit 0729c7d

Browse files
authored
Add support for using pre-built images (#6737)
* api: add InitialSync type to LiveUpdateSpec Signed-off-by: arnas dundulis <arnas@vinted.com> * tiltfile: add initial_sync() live update step Signed-off-by: arnas dundulis <arnas@vinted.com> * reconciler: implement initial_sync file collection Signed-off-by: arnas dundulis <arnas@vinted.com> * fix: enter maybeSync when initial_sync is configured Without this, maybeSync is gated behind hasChangesToSync which requires file change events or container state changes. On a fresh tilt up, the container starts with no file changes detected, so maybeSync is never entered and isInitialSync is never checked. This means initial_sync silently does nothing. Fix: when InitialSync is configured on the spec, always set hasChangesToSync=true so the reconciler enters maybeSync. The isInitialSync check inside maybeSync still correctly gates on whether the container is new (lastFileTimeSynced is zero). Signed-off-by: arnas dundulis <arnas@vinted.com> * fix: initial_sync tests match wrong container and miss auto-reconciles Two pre-existing test bugs: 1. Test containers use Image "frontend-image" which doesn't match the ImageMap selector (expects "local-registry:12345/frontend-image:my-tag"). Tests were actually syncing to the setupFrontend fixture's "main-id" container, not the test's "container-1". Assertions only checked call count so this was hidden. 2. f.Update() and f.kdUpdateStatus() auto-reconcile via MustReconcile, so initial sync fires during setup before the test's explicit MustReconcile call. Tests now clear f.cu.Calls before the assertion window and account for the correct reconcile sequence. Signed-off-by: arnas dundulis <arnas@vinted.com> * optimize: tar batching for initial_sync During initial sync, instead of collecting all individual file paths, converting them to PathMappings via FilesToPathMappings (O(files x syncs)), statting each for existence via MissingLocalPaths, and then creating a tar from individual file mappings — build the tar archive directly from directory-level sync mappings with the ignore filter passed to TarArchiveForPaths. This means a single directory walk produces the tar stream, skipping the redundant FilesToPathMappings conversion and MissingLocalPaths stat calls. collectAllSyncedFiles still runs for BoilRuns trigger matching. Benchmarks at 14k files: old (individual files): 189ms, 33MB allocs new (tar batched): 164ms, 21MB allocs (14% faster, 37% less memory) Also adds: - buildInitialSyncFilter: composes per-sync-path ignore matchers into a single filter for TarArchiveForPaths - Warnings when sync paths don't exist or dockerignore path has no .dockerignore file during initial sync Signed-off-by: arnas dundulis <arnas@vinted.com> * test: verify tar archive contents during initial_sync Add 6 tests that inspect the actual tar archive produced by initial_sync: - TarBatching_IgnoresFilteredFiles: IgnorePaths exclude files from tar - TarBatching_DockerignoreExcludesFromTar: .dockerignore patterns applied - TarBatching_MultipleSyncPathsInSingleTar: files from all syncs in one tar - TarBatching_NoDeletesDuringInitialSync: ToDelete is always empty - TarBatching_GlobPatternExcludesFromTar: **/testdata glob works - TarBatching_LargeFileTree: 1000 files with 100 ignored, correct count Signed-off-by: arnas dundulis <arnas@vinted.com> * cleanup: rename noMoreInitialSync, update IgnorePaths docs - Rename noMoreInitialSync → seenInitialSync in liveUpdateFromSteps for clarity (the variable tracks whether we've seen an initial_sync step, not whether more are allowed) - Update IgnorePaths field comment to accurately describe dockerignore glob syntax support (**/ patterns, *.ext wildcards) instead of just "exact matches and directory prefixes" Signed-off-by: arnas dundulis <arnas@vinted.com> * generate openapi Signed-off-by: arnas dundulis <arnas@vinted.com> * fix: only force maybeSync when containers still need initial sync The previous approach set hasChangesToSync=true on every reconcile when InitialSync was configured, causing unnecessary work entering and exiting maybeSync after all containers have already been synced. Now uses monitor.needsInitialSync() which returns true only when no containers are tracked yet (first reconcile) or when any tracked container has never been synced (lastFileTimeSynced is zero). Once all containers complete their initial sync, the normal hasChangesToSync gate applies. Signed-off-by: arnas dundulis <arnas@vinted.com> * cleanup: remove unnecessary err2 variable in applyInternal The err2 rename was an artifact of the tar batching refactor. Since err is not used in the outer scope at that point, use err directly with var. Signed-off-by: arnas dundulis <arnas@vinted.com> * fix: use path.IsAbs for ignore path validation (Windows compat) filepath.IsAbs("/absolute/path") returns false on Windows since it's not a Windows absolute path (no drive letter). Use path.IsAbs (POSIX semantics) instead so the validation rejects Unix-style absolute paths on all platforms. Fixes TestLiveUpdate_InitialSync_AbsolutePathError on Windows. Signed-off-by: arnas dundulis <arnas@vinted.com> * ci: retry Signed-off-by: arnas dundulis <arnas@vinted.com> * remove explicit ignore, reuse live_update ignore set Signed-off-by: arnas dundulis <arnas@vinted.com> --------- Signed-off-by: arnas dundulis <arnas@vinted.com>
1 parent c3400c3 commit 0729c7d

11 files changed

Lines changed: 1151 additions & 30 deletions

File tree

internal/controllers/core/liveupdate/input.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package liveupdate
33
import (
44
"github.com/tilt-dev/tilt/internal/build"
55
"github.com/tilt-dev/tilt/internal/store/liveupdates"
6+
"github.com/tilt-dev/tilt/pkg/model"
67

78
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
89
)
@@ -18,4 +19,12 @@ type Input struct {
1819
ChangedFiles []build.PathMapping
1920

2021
LastFileTimeSynced metav1.MicroTime
22+
23+
// InitialSync is set during initial sync to enable tar batching directly from
24+
// the configured sync mappings instead of individual file PathMappings.
25+
InitialSync bool
26+
27+
// InitialSyncFilter applies the same ignore semantics used by the source
28+
// FileWatch objects during the initial filesystem walk and tar creation.
29+
InitialSyncFilter model.PathMatcher
2130
}

internal/controllers/core/liveupdate/monitor.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ type monitor struct {
3434

3535
type monitorSource struct {
3636
modTimeByPath map[string]metav1.MicroTime
37+
ignores []v1alpha1.IgnoreDef
3738
lastImageStatus *v1alpha1.ImageMapStatus
3839
lastFileEvent *v1alpha1.FileEvent
3940
}
@@ -44,6 +45,22 @@ type monitorContainerKey struct {
4445
namespace string
4546
}
4647

48+
// needsInitialSync reports whether any container may still need an initial
49+
// sync. Returns true when no containers are tracked yet (first reconcile,
50+
// or after garbage collection) or when any tracked container has never
51+
// been synced.
52+
func (m *monitor) needsInitialSync() bool {
53+
if len(m.containers) == 0 {
54+
return true
55+
}
56+
for _, c := range m.containers {
57+
if c.lastFileTimeSynced.IsZero() {
58+
return true
59+
}
60+
}
61+
return false
62+
}
63+
4764
type monitorContainerStatus struct {
4865
lastFileTimeSynced metav1.MicroTime
4966

internal/controllers/core/liveupdate/reconciler.go

Lines changed: 154 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"io/fs"
8+
"os"
9+
"path/filepath"
710
"sync"
811
"time"
912

@@ -27,6 +30,7 @@ import (
2730
"github.com/tilt-dev/tilt/internal/controllers/apis/configmap"
2831
"github.com/tilt-dev/tilt/internal/controllers/apis/liveupdate"
2932
"github.com/tilt-dev/tilt/internal/controllers/indexer"
33+
"github.com/tilt-dev/tilt/internal/ignore"
3034
"github.com/tilt-dev/tilt/internal/k8s"
3135
"github.com/tilt-dev/tilt/internal/ospath"
3236
"github.com/tilt-dev/tilt/internal/sliceutils"
@@ -181,6 +185,16 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
181185
monitor.hasChangesToSync = true
182186
}
183187

188+
// When initial_sync is configured, enter maybeSync if there may be
189+
// unsynced containers. This handles the case where tilt starts and
190+
// finds an already-running container — no file changes or k8s changes
191+
// are detected, but the container still needs its initial sync.
192+
// Once all tracked containers have been synced, skip this to avoid
193+
// unnecessary work on every reconcile.
194+
if lu.Spec.InitialSync != nil && monitor.needsInitialSync() {
195+
monitor.hasChangesToSync = true
196+
}
197+
184198
if monitor.hasChangesToSync {
185199
status := r.maybeSync(ctx, lu, monitor)
186200
if status.Failed != nil {
@@ -307,12 +321,25 @@ func (r *Reconciler) reconcileOneSource(ctx context.Context, monitor *monitor, s
307321
fwn := source.FileWatch
308322
imn := source.ImageMap
309323

324+
var mSource *monitorSource
325+
if fwn != "" {
326+
var ok bool
327+
mSource, ok = monitor.sources[fwn]
328+
if !ok {
329+
mSource = &monitorSource{
330+
modTimeByPath: make(map[string]metav1.MicroTime),
331+
}
332+
monitor.sources[fwn] = mSource
333+
}
334+
}
335+
310336
var fw v1alpha1.FileWatch
311337
if fwn != "" {
312338
err := r.client.Get(ctx, types.NamespacedName{Name: fwn}, &fw)
313339
if err != nil {
314340
return false, err
315341
}
342+
mSource.ignores = append([]v1alpha1.IgnoreDef(nil), fw.Spec.Ignores...)
316343
}
317344

318345
var im v1alpha1.ImageMap
@@ -328,14 +355,6 @@ func (r *Reconciler) reconcileOneSource(ctx context.Context, monitor *monitor, s
328355
return false, nil
329356
}
330357

331-
mSource, ok := monitor.sources[fwn]
332-
if !ok {
333-
mSource = &monitorSource{
334-
modTimeByPath: make(map[string]metav1.MicroTime),
335-
}
336-
monitor.sources[fwn] = mSource
337-
}
338-
339358
newImageStatus := im.Status
340359
imageChanged := false
341360
if imn != "" {
@@ -697,6 +716,29 @@ func (r *Reconciler) maybeSync(ctx context.Context, lu *v1alpha1.LiveUpdate, mon
697716
}
698717
}
699718

719+
// Initial sync: on new container, collect ALL files from sync paths.
720+
// We collect the file list for trigger matching (BoilRuns), but the
721+
// actual tar archive is built directly from directory-level sync
722+
// mappings, avoiding a second walk into individual file mappings.
723+
isInitialSync := lu.Spec.InitialSync != nil && (!ok || cStatus.lastFileTimeSynced.IsZero())
724+
initialSyncFilter := model.EmptyMatcher
725+
if isInitialSync {
726+
initialSyncFilter = r.buildInitialSyncFilter(monitor)
727+
var err error
728+
filesChanged, err = r.collectAllSyncedFiles(ctx, lu.Spec, initialSyncFilter)
729+
if err != nil {
730+
status.Failed = createFailedState(lu, "InitialSyncError",
731+
fmt.Sprintf("Failed to collect files for initial sync: %v", err))
732+
status.Containers = nil
733+
return true
734+
}
735+
newHighWaterMark = apis.NowMicro()
736+
// Set low water mark to reconciler start time so that any file changes
737+
// between startup and initial sync completion are re-processed on the
738+
// next reconcile, ensuring no changes are missed.
739+
newLowWaterMark = r.startedTime
740+
}
741+
700742
// Sort the files so that they're deterministic.
701743
filesChanged = sliceutils.DedupedAndSorted(filesChanged)
702744
if len(filesChanged) > 0 {
@@ -778,6 +820,8 @@ func (r *Reconciler) maybeSync(ctx context.Context, lu *v1alpha1.LiveUpdate, mon
778820
ChangedFiles: plan.SyncPaths,
779821
Containers: []liveupdates.Container{c},
780822
LastFileTimeSynced: newHighWaterMark,
823+
InitialSync: isInitialSync,
824+
InitialSyncFilter: initialSyncFilter,
781825
})
782826
filesApplied = true
783827
}
@@ -906,27 +950,45 @@ func (r *Reconciler) applyInternal(
906950
return result
907951
}
908952

909-
// rm files from container
910-
toRemove, toArchive, err := build.MissingLocalPaths(ctx, changedFiles)
911-
if err != nil {
912-
result.Failed = &v1alpha1.LiveUpdateStateFailed{
913-
Reason: "Invalid",
914-
Message: fmt.Sprintf("Mapping paths: %v", err),
953+
// For initial sync, build a tar directly from sync directories. This avoids
954+
// the overhead of collecting all files into individual PathMappings and then
955+
// re-walking them to build the tar.
956+
var toRemove []build.PathMapping
957+
var toArchive []build.PathMapping
958+
archiveFilter := model.EmptyMatcher
959+
if input.InitialSync {
960+
// No files to remove during initial sync — all files exist locally.
961+
toRemove = nil
962+
toArchive = build.SyncsToPathMappings(liveupdate.SyncSteps(spec))
963+
archiveFilter = input.InitialSyncFilter
964+
l.Infof("Initial sync: will copy sync paths to container%s: %s", suffix, names)
965+
for _, pm := range toArchive {
966+
l.Infof("- %s", pm.PrettyStr())
967+
}
968+
} else {
969+
// rm files from container
970+
var err error
971+
toRemove, toArchive, err = build.MissingLocalPaths(ctx, changedFiles)
972+
if err != nil {
973+
result.Failed = &v1alpha1.LiveUpdateStateFailed{
974+
Reason: "Invalid",
975+
Message: fmt.Sprintf("Mapping paths: %v", err),
976+
}
977+
return result
915978
}
916-
return result
917-
}
918979

919-
if len(toRemove) > 0 {
920-
l.Infof("Will delete %d file(s) from container%s: %s", len(toRemove), suffix, names)
921-
for _, pm := range toRemove {
922-
l.Infof("- '%s' (matched local path: '%s')", pm.ContainerPath, pm.LocalPath)
980+
if len(toRemove) > 0 {
981+
l.Infof("Will delete %d file(s) from container%s: %s", len(toRemove), suffix, names)
982+
for _, pm := range toRemove {
983+
l.Infof("- '%s' (matched local path: '%s')", pm.ContainerPath, pm.LocalPath)
984+
}
923985
}
924-
}
925986

926-
if len(toArchive) > 0 {
927-
l.Infof("Will copy %d file(s) to container%s: %s", len(toArchive), suffix, names)
928-
for _, pm := range toArchive {
929-
l.Infof("- %s", pm.PrettyStr())
987+
if len(toArchive) > 0 {
988+
l.Infof("Will copy %d file(s) to container%s: %s", len(toArchive), suffix, names)
989+
for _, pm := range toArchive {
990+
l.Infof("- %s", pm.PrettyStr())
991+
}
930992
}
931993
}
932994

@@ -935,7 +997,7 @@ func (r *Reconciler) applyInternal(
935997
// TODO(nick): We should try to distinguish between cases where the tar writer
936998
// fails (which is recoverable) vs when the server-side unpacking
937999
// fails (which may not be recoverable).
938-
archive := build.TarArchiveForPaths(ctx, toArchive, nil)
1000+
archive := build.TarArchiveForPaths(ctx, toArchive, archiveFilter)
9391001
err = cu.UpdateContainer(ctx, cInfo, archive,
9401002
build.PathMappingsToContainerPaths(toRemove), boiledSteps, hotReload)
9411003
_ = archive.Close()
@@ -1134,3 +1196,69 @@ func indexLiveUpdate(obj ctrlclient.Object) []indexer.Key {
11341196
}
11351197
return result
11361198
}
1199+
1200+
func (r *Reconciler) buildInitialSyncFilter(monitor *monitor) model.PathMatcher {
1201+
if len(monitor.spec.Sources) == 0 {
1202+
return model.EmptyMatcher
1203+
}
1204+
1205+
var allIgnores []v1alpha1.IgnoreDef
1206+
for _, source := range monitor.spec.Sources {
1207+
mSource, ok := monitor.sources[source.FileWatch]
1208+
if !ok {
1209+
continue
1210+
}
1211+
allIgnores = append(allIgnores, mSource.ignores...)
1212+
}
1213+
return ignore.CreateFileChangeFilter(allIgnores)
1214+
}
1215+
1216+
// collectAllSyncedFiles walks all sync paths and collects all files.
1217+
func (r *Reconciler) collectAllSyncedFiles(ctx context.Context, spec v1alpha1.LiveUpdateSpec, filter model.PathMatcher) ([]string, error) {
1218+
l := logger.Get(ctx)
1219+
var allFiles []string
1220+
1221+
if filter == nil {
1222+
filter = model.EmptyMatcher
1223+
}
1224+
1225+
for _, syncSpec := range spec.Syncs {
1226+
localPath := syncSpec.LocalPath
1227+
if !filepath.IsAbs(localPath) {
1228+
localPath = filepath.Join(spec.BasePath, localPath)
1229+
}
1230+
1231+
if _, err := os.Stat(localPath); os.IsNotExist(err) {
1232+
l.Warnf("initial_sync: sync path %q does not exist, skipping", localPath)
1233+
continue
1234+
} else if err != nil {
1235+
return nil, fmt.Errorf("stat %s: %w", localPath, err)
1236+
}
1237+
1238+
err := filepath.WalkDir(localPath, func(path string, d fs.DirEntry, err error) error {
1239+
if err != nil {
1240+
return err
1241+
}
1242+
if d.IsDir() {
1243+
if matches, err := filter.MatchesEntireDir(path); err != nil {
1244+
return err
1245+
} else if matches && path != localPath {
1246+
return filepath.SkipDir
1247+
}
1248+
return nil
1249+
}
1250+
if matches, err := filter.Matches(path); err != nil {
1251+
return err
1252+
} else if matches {
1253+
return nil
1254+
}
1255+
allFiles = append(allFiles, path)
1256+
return nil
1257+
})
1258+
if err != nil {
1259+
return nil, fmt.Errorf("walking sync path %s: %w", localPath, err)
1260+
}
1261+
}
1262+
1263+
return allFiles, nil
1264+
}

0 commit comments

Comments
 (0)