From ec0b453c5241c6baedc650b9bf50ed9b704353ef Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 6 Aug 2019 14:41:17 -0700 Subject: [PATCH 1/3] Watch all artifact workspaces, including those outside of the working dir This should fix #2613: Watcher doesn't detect changes when artifacts are outside the directory from where skaffold is run --- pkg/skaffold/trigger/triggers.go | 18 ++++++++++++++++-- pkg/skaffold/trigger/triggers_test.go | 25 +++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/pkg/skaffold/trigger/triggers.go b/pkg/skaffold/trigger/triggers.go index 75619c12047..fdb00dd87eb 100644 --- a/pkg/skaffold/trigger/triggers.go +++ b/pkg/skaffold/trigger/triggers.go @@ -22,6 +22,7 @@ import ( "fmt" "io" "os" + "path/filepath" "strings" "sync/atomic" "time" @@ -47,8 +48,13 @@ func NewTrigger(runctx *runcontext.RunContext) (Trigger, error) { Interval: time.Duration(runctx.Opts.WatchPollInterval) * time.Millisecond, }, nil case "notify": + 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, + Interval: time.Duration(runctx.Opts.WatchPollInterval) * time.Millisecond, + workspaces: workspaces, }, nil case "manual": return &manualTrigger{}, nil @@ -134,7 +140,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 +163,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. diff --git a/pkg/skaffold/trigger/triggers_test.go b/pkg/skaffold/trigger/triggers_test.go index 02444448b1b..6b3a31ea023 100644 --- a/pkg/skaffold/trigger/triggers_test.go +++ b/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{})) + } }) } } From 6a7125577446e8b1898cb42978e8aea6ed184aa3 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 6 Aug 2019 15:46:56 -0700 Subject: [PATCH 2/3] reorganize into newFSNotifyTrigger function --- pkg/skaffold/trigger/triggers.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/pkg/skaffold/trigger/triggers.go b/pkg/skaffold/trigger/triggers.go index fdb00dd87eb..ec32291aca4 100644 --- a/pkg/skaffold/trigger/triggers.go +++ b/pkg/skaffold/trigger/triggers.go @@ -48,14 +48,7 @@ func NewTrigger(runctx *runcontext.RunContext) (Trigger, error) { Interval: time.Duration(runctx.Opts.WatchPollInterval) * time.Millisecond, }, nil case "notify": - 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, - }, nil + return newFSNotifyTrigger(runctx) case "manual": return &manualTrigger{}, nil default: @@ -63,6 +56,17 @@ func NewTrigger(runctx *runcontext.RunContext) (Trigger, error) { } } +func newFSNotifyTrigger(runctx *runcontext.RunContext) (*fsNotifyTrigger, error) { + 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, + }, nil +} + // pollTrigger watches for changes on a given interval of time. type pollTrigger struct { Interval time.Duration From 19addcbdf4c05d37000e4e2e46495737ac0c9bee Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Thu, 8 Aug 2019 12:13:05 -0700 Subject: [PATCH 3/3] fix lint --- pkg/skaffold/trigger/triggers.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/skaffold/trigger/triggers.go b/pkg/skaffold/trigger/triggers.go index ec32291aca4..5ca046f32b1 100644 --- a/pkg/skaffold/trigger/triggers.go +++ b/pkg/skaffold/trigger/triggers.go @@ -48,7 +48,7 @@ func NewTrigger(runctx *runcontext.RunContext) (Trigger, error) { Interval: time.Duration(runctx.Opts.WatchPollInterval) * time.Millisecond, }, nil case "notify": - return newFSNotifyTrigger(runctx) + return newFSNotifyTrigger(runctx), nil case "manual": return &manualTrigger{}, nil default: @@ -56,7 +56,7 @@ func NewTrigger(runctx *runcontext.RunContext) (Trigger, error) { } } -func newFSNotifyTrigger(runctx *runcontext.RunContext) (*fsNotifyTrigger, error) { +func newFSNotifyTrigger(runctx *runcontext.RunContext) *fsNotifyTrigger { workspaces := map[string]struct{}{} for _, a := range runctx.Cfg.Build.Artifacts { workspaces[a.Workspace] = struct{}{} @@ -64,7 +64,7 @@ func newFSNotifyTrigger(runctx *runcontext.RunContext) (*fsNotifyTrigger, error) return &fsNotifyTrigger{ Interval: time.Duration(runctx.Opts.WatchPollInterval) * time.Millisecond, workspaces: workspaces, - }, nil + } } // pollTrigger watches for changes on a given interval of time.