Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable watch trigger only when at least one of autoBuild, autoSync or autoDeploy is enabled #4676

Merged
merged 2 commits into from
Aug 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pkg/skaffold/runner/intent.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,9 @@ func (i *intents) GetIntents() (bool, bool, bool) {
defer i.lock.Unlock()
return i.build, i.sync, i.deploy
}

func (i *intents) IsAnyAutoEnabled() bool {
i.lock.Lock()
defer i.lock.Unlock()
return i.autoBuild || i.autoSync || i.autoDeploy
}
9 changes: 4 additions & 5 deletions pkg/skaffold/runner/new.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,15 @@ func NewForConfig(runCtx *runcontext.RunContext) (*SkaffoldRunner, error) {
deployer = WithNotification(deployer)
}

trigger, err := trigger.NewTrigger(runCtx)
if err != nil {
return nil, fmt.Errorf("creating watch trigger: %w", err)
}

event.InitializeState(runCtx.Pipeline(), runCtx.GetKubeContext(), runCtx.AutoBuild(), runCtx.AutoDeploy(), runCtx.AutoSync())
event.LogMetaEvent()

monitor := filemon.NewMonitor()
intents, intentChan := setupIntents(runCtx)
trigger, err := trigger.NewTrigger(runCtx, intents.IsAnyAutoEnabled)
if err != nil {
return nil, fmt.Errorf("creating watch trigger: %w", err)
}

return &SkaffoldRunner{
builder: builder,
Expand Down
51 changes: 43 additions & 8 deletions pkg/skaffold/trigger/triggers.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,35 +42,40 @@ type Trigger interface {
}

// NewTrigger creates a new trigger.
func NewTrigger(runCtx *runcontext.RunContext) (Trigger, error) {
func NewTrigger(runCtx *runcontext.RunContext, isActive func() bool) (Trigger, error) {
switch strings.ToLower(runCtx.Trigger()) {
case "polling":
return &pollTrigger{
Interval: time.Duration(runCtx.WatchPollInterval()) * time.Millisecond,
isActive: isActive,
}, nil
case "notify":
return newFSNotifyTrigger(runCtx), nil
return newFSNotifyTrigger(runCtx, isActive), nil
case "manual":
return &manualTrigger{}, nil
return &manualTrigger{
isActive: isActive,
}, nil
default:
return nil, fmt.Errorf("unsupported trigger: %s", runCtx.Trigger())
}
}

func newFSNotifyTrigger(runCtx *runcontext.RunContext) *fsNotifyTrigger {
func newFSNotifyTrigger(runCtx *runcontext.RunContext, isActive func() bool) *fsNotifyTrigger {
workspaces := map[string]struct{}{}
for _, a := range runCtx.Pipeline().Build.Artifacts {
workspaces[a.Workspace] = struct{}{}
}
return &fsNotifyTrigger{
Interval: time.Duration(runCtx.WatchPollInterval()) * time.Millisecond,
workspaces: workspaces,
isActive: isActive,
}
}

// pollTrigger watches for changes on a given interval of time.
type pollTrigger struct {
Interval time.Duration
isActive func() bool
}

// Debounce tells the watcher to debounce rapid sequence of changes.
Expand All @@ -79,7 +84,11 @@ func (t *pollTrigger) Debounce() bool {
}

func (t *pollTrigger) LogWatchToUser(out io.Writer) {
color.Yellow.Fprintf(out, "Watching for changes every %v...\n", t.Interval)
if t.isActive() {
color.Yellow.Fprintf(out, "Watching for changes every %v...\n", t.Interval)
} else {
color.Yellow.Fprintln(out, "Not watching for changes...")
}
}

// Start starts a timer.
Expand All @@ -91,6 +100,11 @@ func (t *pollTrigger) Start(ctx context.Context) (<-chan bool, error) {
for {
select {
case <-ticker.C:

// Ignore if trigger is inactive
if !t.isActive() {
continue
}
trigger <- true
case <-ctx.Done():
ticker.Stop()
Expand All @@ -103,15 +117,21 @@ func (t *pollTrigger) Start(ctx context.Context) (<-chan bool, error) {
}

// manualTrigger watches for changes when the user presses a key.
type manualTrigger struct{}
type manualTrigger struct {
isActive func() bool
}

// Debounce tells the watcher to not debounce rapid sequence of changes.
func (t *manualTrigger) Debounce() bool {
return false
}

func (t *manualTrigger) LogWatchToUser(out io.Writer) {
color.Yellow.Fprintln(out, "Press any key to rebuild/redeploy the changes")
if t.isActive() {
color.Yellow.Fprintln(out, "Press any key to rebuild/redeploy the changes")
} else {
color.Yellow.Fprintln(out, "Not watching for changes...")
}
}

// Start starts listening to pressed keys.
Expand All @@ -136,6 +156,11 @@ func (t *manualTrigger) Start(ctx context.Context) (<-chan bool, error) {
if atomic.LoadInt32(&stopped) == 1 {
return
}

// Ignore if trigger is inactive
if !t.isActive() {
continue
}
trigger <- true
}
}()
Expand All @@ -147,6 +172,7 @@ func (t *manualTrigger) Start(ctx context.Context) (<-chan bool, error) {
type fsNotifyTrigger struct {
Interval time.Duration
workspaces map[string]struct{}
isActive func() bool
}

// Debounce tells the watcher to not debounce rapid sequence of changes.
Expand All @@ -156,7 +182,11 @@ func (t *fsNotifyTrigger) Debounce() bool {
}

func (t *fsNotifyTrigger) LogWatchToUser(out io.Writer) {
color.Yellow.Fprintln(out, "Watching for changes...")
if t.isActive() {
color.Yellow.Fprintln(out, "Watching for changes...")
} else {
color.Yellow.Fprintln(out, "Not watching for changes...")
}
}

// Start listening for file system changes
Expand Down Expand Up @@ -197,6 +227,11 @@ func (t *fsNotifyTrigger) Start(ctx context.Context) (<-chan bool, error) {
for {
select {
case e := <-c:

// Ignore detected changes if not active
if !t.isActive() {
continue
}
logrus.Debugln("Change detected", e)

// Wait t.interval before triggering.
Expand Down
102 changes: 85 additions & 17 deletions pkg/skaffold/trigger/triggers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,10 @@ func TestNewTrigger(t *testing.T) {
},
}

got, err := NewTrigger(runCtx)
got, err := NewTrigger(runCtx, nil)
t.CheckError(test.shouldErr, err)
if !test.shouldErr {
t.CheckDeepEqual(test.expected, got, cmp.AllowUnexported(fsNotifyTrigger{}))
t.CheckDeepEqual(test.expected, got, cmp.AllowUnexported(fsNotifyTrigger{}), cmp.AllowUnexported(manualTrigger{}), cmp.AllowUnexported(pollTrigger{}))
}
})
}
Expand All @@ -100,13 +100,36 @@ func TestPollTrigger_Debounce(t *testing.T) {
}

func TestPollTrigger_LogWatchToUser(t *testing.T) {
out := new(bytes.Buffer)
tests := []struct {
description string
isActive bool
expected string
}{
{
description: "active polling trigger",
isActive: true,
expected: "Watching for changes every 10ns...\n",
},
{
description: "inactive polling trigger",
isActive: false,
expected: "Not watching for changes...\n",
},
}
for _, test := range tests {
out := new(bytes.Buffer)

trigger := &pollTrigger{Interval: 10}
trigger.LogWatchToUser(out)
trigger := &pollTrigger{
Interval: 10,
isActive: func() bool {
return test.isActive
},
}
trigger.LogWatchToUser(out)

got, want := out.String(), "Watching for changes every 10ns...\n"
testutil.CheckDeepEqual(t, want, got)
got, want := out.String(), test.expected
testutil.CheckDeepEqual(t, want, got)
}
}

func TestNotifyTrigger_Debounce(t *testing.T) {
Expand All @@ -116,13 +139,36 @@ func TestNotifyTrigger_Debounce(t *testing.T) {
}

func TestNotifyTrigger_LogWatchToUser(t *testing.T) {
out := new(bytes.Buffer)
tests := []struct {
description string
isActive bool
expected string
}{
{
description: "active notify trigger",
isActive: true,
expected: "Watching for changes...\n",
},
{
description: "inactive notify trigger",
isActive: false,
expected: "Not watching for changes...\n",
},
}
for _, test := range tests {
out := new(bytes.Buffer)

trigger := &fsNotifyTrigger{Interval: 10}
trigger.LogWatchToUser(out)
trigger := &fsNotifyTrigger{
Interval: 10,
isActive: func() bool {
return test.isActive
},
}
trigger.LogWatchToUser(out)

got, want := out.String(), "Watching for changes...\n"
testutil.CheckDeepEqual(t, want, got)
got, want := out.String(), test.expected
testutil.CheckDeepEqual(t, want, got)
}
}

func TestManualTrigger_Debounce(t *testing.T) {
Expand All @@ -132,11 +178,33 @@ func TestManualTrigger_Debounce(t *testing.T) {
}

func TestManualTrigger_LogWatchToUser(t *testing.T) {
out := new(bytes.Buffer)
tests := []struct {
description string
isActive bool
expected string
}{
{
description: "active manual trigger",
isActive: true,
expected: "Press any key to rebuild/redeploy the changes\n",
},
{
description: "inactive manual trigger",
isActive: false,
expected: "Not watching for changes...\n",
},
}
for _, test := range tests {
out := new(bytes.Buffer)

trigger := &manualTrigger{}
trigger.LogWatchToUser(out)
trigger := &manualTrigger{
isActive: func() bool {
return test.isActive
},
}
trigger.LogWatchToUser(out)

got, want := out.String(), "Press any key to rebuild/redeploy the changes\n"
testutil.CheckDeepEqual(t, want, got)
got, want := out.String(), test.expected
testutil.CheckDeepEqual(t, want, got)
}
}