filewatch: avoid accepting stale/duplicate events

internal/engine/fswatch/action.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"strings"
-	"time"
 
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -123,27 +122,9 @@ func processFileWatchStatus(ctx context.Context, state *store.EngineState, fw *f
 		return
 	}
 
-	// NOTE(nick): BuildController uses these timestamps to determine which files
-	// to clear after a build. In particular, it:
-	//
-	// 1) Grabs the pending files
-	// 2) Runs a live update
-	// 3) Clears the pending files with timestamps before the live update started.
-	//
-	// Here's the race condition: suppose a file changes, but it doesn't get into
-	// the EngineState until after step (2). That means step (3) will clear the file
-	// even though it wasn't live-updated properly. Because as far as we can tell,
-	// the file must have been in the EngineState before the build started.
-	//
-	// Ideally, BuildController should be do more bookkeeping to keep track of
-	// which files it consumed from which FileWatches. But we're changing
-	// this architecture anyway. For now, we record the time it got into
-	// the EngineState, rather than the time it was originally changed.
-	now := time.Now()
-
 	if targetID.Type == model.TargetTypeConfigs {
 		for _, f := range latestEvent.SeenFiles {
-			state.PendingConfigFileChanges[f] = now
+			state.PendingConfigFileChanges[f] = latestEvent.Time.Time
 		}
 		return
 	}
@@ -155,9 +136,8 @@ func processFileWatchStatus(ctx context.Context, state *store.EngineState, fw *f
 			return
 		}
 
-		status := ms.MutableBuildStatus(targetID)
 		for _, f := range latestEvent.SeenFiles {
-			status.PendingFileChanges[f] = now
+			ms.AddPendingFileChange(targetID, f, latestEvent.Time.Time)
 		}
 	}
 }

internal/store/engine_state.go

@@ -595,6 +595,43 @@ func (ms *ManifestState) PodWithID(pid k8s.PodID) (*Pod, bool) {
 	return nil, false
 }
 
+func (ms *ManifestState) AddPendingFileChange(targetID model.TargetID, file string, timestamp time.Time) {
+	if timestamp.Before(ms.LastBuild().StartTime) {
+		// this file change occurred before the last build even started, so it's stale and was already processed
+		return
+	} else if !ms.CurrentBuild.Empty() {
+		if timestamp.Before(ms.CurrentBuild.StartTime) {
+			// this file change occurred before the build started, but if the current build already knows
+			// about it (from another target or rapid successive changes that weren't de-duped), it can be ignored
+			for _, edit := range ms.CurrentBuild.Edits {
+				if edit == file {
+					return
+				}
+			}
+		}
+		// NOTE(nick): BuildController uses these timestamps to determine which files
+		// to clear after a build. In particular, it:
+		//
+		// 1) Grabs the pending files
+		// 2) Runs a live update
+		// 3) Clears the pending files with timestamps before the live update started.
+		//
+		// Here's the race condition: suppose a file changes, but it doesn't get into
+		// the EngineState until after step (2). That means step (3) will clear the file
+		// even though it wasn't live-updated properly. Because as far as we can tell,
+		// the file must have been in the EngineState before the build started.
+		//
+		// Ideally, BuildController should be do more bookkeeping to keep track of
+		// which files it consumed from which FileWatches. But we're changing
+		// this architecture anyway. For now, we record the time it got into
+		// the EngineState, rather than the time it was originally changed.
+		timestamp = time.Now()
+	}
+
+	bs := ms.MutableBuildStatus(targetID)
+	bs.PendingFileChanges[file] = timestamp
+}
+
 func (ms *ManifestState) HasPendingFileChanges() bool {
 	for _, status := range ms.BuildStatuses {
 		if len(status.PendingFileChanges) > 0 {