Merge pull request #1439 from ltouati/fsnotify

Implement a notification based watcher

Author: dgageot

Gopkg.lock

@@ -66,3 +66,7 @@
 [[constraint]]
   name = "github.com/bmatcuk/doublestar"
   revision = "v1.1.1"
+
+[[constraint]]
+  name = "github.com/rjeczalik/notify"
+  version = "0.9.2"

Gopkg.toml

@@ -55,7 +55,7 @@ $(BUILD_DIR)/$(PROJECT): $(BUILD_DIR)/$(PROJECT)-$(GOOS)-$(GOARCH)
 	cp $(BUILD_DIR)/$(PROJECT)-$(GOOS)-$(GOARCH) $@
 
 $(BUILD_DIR)/$(PROJECT)-%-$(GOARCH): $(GO_FILES) $(BUILD_DIR)
-	GOOS=$* GOARCH=$(GOARCH) CGO_ENABLED=0 go build -ldflags $(GO_LDFLAGS) -gcflags $(GO_GCFLAGS) -asmflags $(GO_ASMFLAGS) -o $@ $(BUILD_PACKAGE)
+	GOOS=$* GOARCH=$(GOARCH) CGO_ENABLED=1 go build -ldflags $(GO_LDFLAGS) -gcflags $(GO_GCFLAGS) -asmflags $(GO_ASMFLAGS) -o $@ $(BUILD_PACKAGE)
 
 %.sha256: %
 	shasum -a 256 $< > $@
@@ -81,7 +81,7 @@ test:
 
 .PHONY: install
 install: $(GO_FILES) $(BUILD_DIR)
-	GOOS=$(GOOS) GOARCH=$(GOARCH) CGO_ENABLED=0 go install -ldflags $(GO_LDFLAGS) -gcflags $(GO_GCFLAGS) -asmflags $(GO_ASMFLAGS) $(BUILD_PACKAGE)
+	GOOS=$(GOOS) GOARCH=$(GOARCH) CGO_ENABLED=1 go install -ldflags $(GO_LDFLAGS) -gcflags $(GO_GCFLAGS) -asmflags $(GO_ASMFLAGS) $(BUILD_PACKAGE)
 
 .PHONY: integration
 integration: install $(BUILD_DIR)/$(PROJECT)

Makefile

@@ -38,7 +38,7 @@ func NewCmdDev(out io.Writer) *cobra.Command {
 	}
 	AddRunDevFlags(cmd)
 	cmd.Flags().BoolVar(&opts.TailDev, "tail", true, "Stream logs from deployed objects")
-	cmd.Flags().StringVar(&opts.Trigger, "trigger", "polling", "How are changes detected? (polling or manual)")
+	cmd.Flags().StringVar(&opts.Trigger, "trigger", "polling", "How are changes detected? (polling, manual or notify)")
 	cmd.Flags().BoolVar(&opts.Cleanup, "cleanup", true, "Delete deployments after dev mode is interrupted")
 	cmd.Flags().StringArrayVarP(&opts.Watch, "watch-image", "w", nil, "Choose which artifacts to watch. Artifacts with image names that contain the expression will be watched only. Default is to watch sources for all artifacts")
 	cmd.Flags().IntVarP(&opts.WatchPollInterval, "watch-poll-interval", "i", 1000, "Interval (in ms) between two checks for file changes")

cmd/skaffold/app/cmd/dev.go

@@ -213,7 +213,7 @@ Flags:
       --skip-tests                Whether to skip the tests after building
       --tail                      Stream logs from deployed objects (default true)
       --toot                      Emit a terminal beep after the deploy is complete
-      --trigger string            How are changes detected? (polling or manual) (default "polling")
+      --trigger string            How are changes detected? (polling, manual or notify) (default "polling")
   -w, --watch-image stringArray   Choose which artifacts to watch. Artifacts with image names that contain the expression will be watched only. Default is to watch sources for all artifacts
   -i, --watch-poll-interval int   Interval (in ms) between two checks for file changes (default 1000)
 

docs/content/en/docs/references/cli/_index.md

@@ -17,14 +17,14 @@
 cp docs/content/en/docs/references/cli/index_header docs/content/en/docs/references/cli/_index.md
 go run cmd/skaffold/man/man.go >> docs/content/en/docs/references/cli/_index.md
 
-readonly CLI_CHANGES=`git status -s | grep "docs/" | wc -l`
+readonly CLI_CHANGES=`git status -s | grep "docs/" | grep -x vendor | wc -l`
 
 if [ $CLI_CHANGES -gt 0 ]; then
   echo "You have skaffold command changes but haven't generated the CLI reference docs. Please run hack/check-docs.sh and commit the results!"
   exit 1
 fi
 
-readonly DOCS_CHANGES=`git diff --name-status master | grep "docs/" | wc -l`
+readonly DOCS_CHANGES=`git diff --name-status master | grep "docs/" | grep -x vendor | wc -l`
 
 if [ $DOCS_CHANGES -gt 0 ]; then
   echo "There are $DOCS_CHANGES changes in docs, testing site generation..."

hack/check-docs.sh

@@ -18,20 +18,23 @@ package watch
 
 import (
 	"bufio"
+	"context"
 	"fmt"
 	"io"
 	"os"
 	"strings"
+	"sync/atomic"
 	"time"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
+	"github.com/rjeczalik/notify"
 	"github.com/sirupsen/logrus"
 )
 
 // Trigger describes a mechanism that triggers the watch.
 type Trigger interface {
-	Start() (<-chan bool, func())
+	Start(context.Context) (<-chan bool, error)
 	WatchForChanges(io.Writer)
 	Debounce() bool
 }
@@ -41,7 +44,11 @@ func NewTrigger(opts *config.SkaffoldOptions) (Trigger, error) {
 	switch strings.ToLower(opts.Trigger) {
 	case "polling":
 		return &pollTrigger{
-			Interval: time.Duration(opts.WatchPollInterval) * time.Millisecond,
+			interval: time.Duration(opts.WatchPollInterval) * time.Millisecond,
+		}, nil
+	case "notify":
+		return &fsNotifyTrigger{
+			interval: time.Duration(opts.WatchPollInterval) * time.Millisecond,
 		}, nil
 	case "manual":
 		return &manualTrigger{}, nil
@@ -52,7 +59,7 @@ func NewTrigger(opts *config.SkaffoldOptions) (Trigger, error) {
 
 // pollTrigger watches for changes on a given interval of time.
 type pollTrigger struct {
-	Interval time.Duration
+	interval time.Duration
 }
 
 // Debounce tells the watcher to debounce rapid sequence of changes.
@@ -61,27 +68,30 @@ func (t *pollTrigger) Debounce() bool {
 }
 
 func (t *pollTrigger) WatchForChanges(out io.Writer) {
-	color.Yellow.Fprintf(out, "Watching for changes every %v...\n", t.Interval)
+	color.Yellow.Fprintf(out, "Watching for changes every %v...\n", t.interval)
 }
 
 // Start starts a timer.
-func (t *pollTrigger) Start() (<-chan bool, func()) {
+func (t *pollTrigger) Start(ctx context.Context) (<-chan bool, error) {
 	trigger := make(chan bool)
 
-	ticker := time.NewTicker(t.Interval)
+	ticker := time.NewTicker(t.interval)
 	go func() {
 		for {
-			<-ticker.C
-			trigger <- true
+			select {
+			case <-ticker.C:
+				trigger <- true
+			case <-ctx.Done():
+				ticker.Stop()
+			}
 		}
 	}()
 
-	return trigger, ticker.Stop
+	return trigger, nil
 }
 
 // manualTrigger watches for changes when the user presses a key.
-type manualTrigger struct {
-}
+type manualTrigger struct{}
 
 // Debounce tells the watcher to not debounce rapid sequence of changes.
 func (t *manualTrigger) Debounce() bool {
@@ -93,19 +103,78 @@ func (t *manualTrigger) WatchForChanges(out io.Writer) {
 }
 
 // Start starts listening to pressed keys.
-func (t *manualTrigger) Start() (<-chan bool, func()) {
+func (t *manualTrigger) Start(ctx context.Context) (<-chan bool, error) {
 	trigger := make(chan bool)
 
+	var stopped int32
+	go func() {
+		<-ctx.Done()
+		atomic.StoreInt32(&stopped, 1)
+	}()
+
 	reader := bufio.NewReader(os.Stdin)
 	go func() {
 		for {
 			_, _, err := reader.ReadRune()
 			if err != nil {
 				logrus.Debugf("manual trigger error: %s", err)
 			}
+
+			// Wait until the context is cancelled.
+			if atomic.LoadInt32(&stopped) == 1 {
+				return
+			}
 			trigger <- true
 		}
 	}()
 
-	return trigger, func() {}
+	return trigger, nil
+}
+
+// notifyTrigger watches for changes with fsnotify
+type fsNotifyTrigger struct {
+	interval time.Duration
+}
+
+// Debounce tells the watcher to not debounce rapid sequence of changes.
+func (t *fsNotifyTrigger) Debounce() bool {
+	// This trigger has built-in debouncing.
+	return false
+}
+
+func (t *fsNotifyTrigger) WatchForChanges(out io.Writer) {
+	color.Yellow.Fprintln(out, "Watching for changes...")
+}
+
+// Start Listening for file system changes
+func (t *fsNotifyTrigger) Start(ctx context.Context) (<-chan bool, error) {
+	// TODO(@dgageot): If file changes happen too quickly, events might be lost
+	c := make(chan notify.EventInfo, 100)
+
+	// Watch current directory recursively
+	if err := notify.Watch("./...", c, notify.All); err != nil {
+		return nil, err
+	}
+
+	trigger := make(chan bool)
+	go func() {
+		timer := time.NewTimer(1<<63 - 1) // Forever
+
+		for {
+			select {
+			case e := <-c:
+				logrus.Debugln("Change detected", e)
+
+				// Wait t.interval before triggering.
+				// This way, rapid stream of events will be grouped.
+				timer.Reset(t.interval)
+			case <-timer.C:
+				trigger <- true
+			case <-ctx.Done():
+				timer.Stop()
+			}
+		}
+	}()
+
+	return trigger, nil
 }

pkg/skaffold/watch/triggers.go

@@ -68,8 +68,13 @@ func (w *watchList) Register(deps func() ([]string, error), onChange func(Events
 
 // Run watches files until the context is cancelled or an error occurs.
 func (w *watchList) Run(ctx context.Context, out io.Writer, onChange func() error) error {
-	t, cleanup := w.trigger.Start()
-	defer cleanup()
+	ctxTrigger, cancelTrigger := context.WithCancel(ctx)
+	defer cancelTrigger()
+
+	t, err := w.trigger.Start(ctxTrigger)
+	if err != nil {
+		return errors.Wrap(err, "unable to start trigger")
+	}
 
 	changedComponents := map[int]bool{}
 

pkg/skaffold/watch/watch.go

@@ -72,7 +72,7 @@ func TestWatch(t *testing.T) {
 
 			// Watch folder
 			watcher := NewWatcher(&pollTrigger{
-				Interval: 10 * time.Millisecond,
+				interval: 10 * time.Millisecond,
 			})
 			err := watcher.Register(folder.List, folderChanged.call)
 			testutil.CheckError(t, false, err)