Merge pull request #2614 from priyawadhwa/filewatch-bug

Watch all artifact workspaces, including those outside of the working directory

pkg/skaffold/trigger/triggers.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"path/filepath"
 	"strings"
 	"sync/atomic"
 	"time"
@@ -47,16 +48,25 @@ func NewTrigger(runctx *runcontext.RunContext) (Trigger, error) {
 			Interval: time.Duration(runctx.Opts.WatchPollInterval) * time.Millisecond,
 		}, nil
 	case "notify":
-		return &fsNotifyTrigger{
-			Interval: time.Duration(runctx.Opts.WatchPollInterval) * time.Millisecond,
-		}, nil
+		return newFSNotifyTrigger(runctx), nil
 	case "manual":
 		return &manualTrigger{}, nil
 	default:
 		return nil, fmt.Errorf("unsupported trigger: %s", runctx.Opts.Trigger)
 	}
 }
 
+func newFSNotifyTrigger(runctx *runcontext.RunContext) *fsNotifyTrigger {
+	workspaces := map[string]struct{}{}
+	for _, a := range runctx.Cfg.Build.Artifacts {
+		workspaces[a.Workspace] = struct{}{}
+	}
+	return &fsNotifyTrigger{
+		Interval:   time.Duration(runctx.Opts.WatchPollInterval) * time.Millisecond,
+		workspaces: workspaces,
+	}
+}
+
 // pollTrigger watches for changes on a given interval of time.
 type pollTrigger struct {
 	Interval time.Duration
@@ -134,7 +144,8 @@ func (t *manualTrigger) Start(ctx context.Context) (<-chan bool, error) {
 
 // notifyTrigger watches for changes with fsnotify
 type fsNotifyTrigger struct {
-	Interval time.Duration
+	Interval   time.Duration
+	workspaces map[string]struct{}
 }
 
 // Debounce tells the watcher to not debounce rapid sequence of changes.
@@ -156,6 +167,13 @@ func (t *fsNotifyTrigger) Start(ctx context.Context) (<-chan bool, error) {
 		return nil, err
 	}
 
+	// Watch all workspaces recursively
+	for w := range t.workspaces {
+		if err := notify.Watch(filepath.Join(w, "..."), c, notify.All); err != nil {
+			return nil, err
+		}
+	}
+
 	// Since the file watcher runs in a separate go routine
 	// and can take some time to start, it can lose the very first change.
 	// As a mitigation, we act as if a change was detected.

pkg/skaffold/trigger/triggers_test.go

@@ -23,7 +23,9 @@ import (
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/testutil"
+	"github.com/google/go-cmp/cmp"
 )
 
 func TestNewTrigger(t *testing.T) {
@@ -45,6 +47,10 @@ func TestNewTrigger(t *testing.T) {
 			opts:        config.SkaffoldOptions{Trigger: "notify", WatchPollInterval: 1},
 			expected: &fsNotifyTrigger{
 				Interval: time.Duration(1) * time.Millisecond,
+				workspaces: map[string]struct{}{
+					"../workspace":            {},
+					"../some/other/workspace": {},
+				},
 			},
 		},
 		{
@@ -62,11 +68,26 @@ func TestNewTrigger(t *testing.T) {
 		testutil.Run(t, test.description, func(t *testutil.T) {
 			runCtx := &runcontext.RunContext{
 				Opts: test.opts,
+				Cfg: latest.Pipeline{
+					Build: latest.BuildConfig{
+						Artifacts: []*latest.Artifact{
+							{
+								Workspace: "../workspace",
+							}, {
+								Workspace: "../workspace",
+							}, {
+								Workspace: "../some/other/workspace",
+							},
+						},
+					},
+				},
 			}
 
 			got, err := NewTrigger(runCtx)
-
-			t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expected, got)
+			t.CheckError(test.shouldErr, err)
+			if !test.shouldErr {
+				t.CheckDeepEqual(test.expected, got, cmp.AllowUnexported(fsNotifyTrigger{}))
+			}
 		})
 	}
 }