From 74a5b8f5766b83c4ea5990421e81a458e51d1225 Mon Sep 17 00:00:00 2001
From: nghialv <nghialv2607@gmail.com>
Date: Fri, 12 Nov 2021 01:02:17 +0900
Subject: [PATCH 1/6] Allow triggering deployment when application is at
 out-of-sync state

---
 pkg/app/piped/planpreview/builder.go |  63 +++--
 pkg/app/piped/trigger/deployment.go  | 171 ++++---------
 pkg/app/piped/trigger/determiner.go  | 118 ++++-----
 pkg/app/piped/trigger/trigger.go     | 361 +++++++++++++++++----------
 pkg/config/deployment.go             |  29 ++-
 5 files changed, 416 insertions(+), 326 deletions(-)

diff --git a/pkg/app/piped/planpreview/builder.go b/pkg/app/piped/planpreview/builder.go
index f9d5f6d148..b7ef20ed0e 100644
--- a/pkg/app/piped/planpreview/builder.go
+++ b/pkg/app/piped/planpreview/builder.go
@@ -19,6 +19,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"path/filepath"
 
 	"go.uber.org/zap"
 	"google.golang.org/grpc/codes"
@@ -305,26 +306,35 @@ func (b *builder) cloneHeadCommit(ctx context.Context, headBranch, headCommit st
 }
 
 func (b *builder) findTriggerApps(ctx context.Context, repo git.Repo, apps []*model.Application, headCommit string) (triggerApps []*model.Application, failedResults []*model.ApplicationPlanPreviewResult, err error) {
-	d := trigger.NewDeterminer(repo, headCommit, b.commitGetter, true, b.logger)
-	for _, app := range apps {
-		shouldTrigger, err := d.ShouldTrigger(ctx, app)
+	d := trigger.NewOnCommitDeterminer(repo, headCommit, b.commitGetter, b.logger)
+	determine := func(app *model.Application) (bool, error) {
+		appCfg, err := loadDeploymentConfiguration(repo.GetPath(), app)
 		if err != nil {
-			// We only need the environment name
-			// so the returned error can be ignorable.
-			var envName string
-			if env, err := b.environmentGetter.Get(ctx, app.EnvId); err == nil {
-				envName = env.Name
-			}
-
-			r := model.MakeApplicationPlanPreviewResult(*app, envName)
-			r.Error = fmt.Sprintf("failed while determining the application should be triggered or not, %v", err)
-			failedResults = append(failedResults, r)
-			continue
+			return false, err
 		}
+		return d.ShouldTrigger(ctx, app, appCfg)
+	}
 
+	for _, app := range apps {
+		shouldTrigger, err := determine(app)
 		if shouldTrigger {
 			triggerApps = append(triggerApps, app)
+			continue
 		}
+		if err == nil {
+			continue
+		}
+
+		// We only need the environment name
+		// so the returned error can be ignorable.
+		var envName string
+		if env, err := b.environmentGetter.Get(ctx, app.EnvId); err == nil {
+			envName = env.Name
+		}
+
+		r := model.MakeApplicationPlanPreviewResult(*app, envName)
+		r.Error = fmt.Sprintf("failed while determining the application should be triggered or not, %v", err)
+		failedResults = append(failedResults, r)
 	}
 	return
 }
@@ -412,3 +422,28 @@ func (b *builder) getMostRecentlySuccessfulDeployment(ctx context.Context, appli
 
 	return deploy.(*model.ApplicationDeploymentReference), nil
 }
+
+func loadDeploymentConfiguration(repoPath string, app *model.Application) (*config.GenericDeploymentSpec, error) {
+	var (
+		relPath = app.GitPath.GetDeploymentConfigFilePath()
+		absPath = filepath.Join(repoPath, relPath)
+	)
+
+	cfg, err := config.LoadFromYAML(absPath)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil, fmt.Errorf("application config file %s was not found in Git", relPath)
+		}
+		return nil, err
+	}
+	if appKind, ok := config.ToApplicationKind(cfg.Kind); !ok || appKind != app.Kind {
+		return nil, fmt.Errorf("invalid application kind in the deployment config file, got: %s, expected: %s", appKind, app.Kind)
+	}
+
+	spec, ok := cfg.GetGenericDeployment()
+	if !ok {
+		return nil, fmt.Errorf("unsupported application kind: %s", app.Kind)
+	}
+
+	return &spec, nil
+}
diff --git a/pkg/app/piped/trigger/deployment.go b/pkg/app/piped/trigger/deployment.go
index 03e6ae0fdc..7a5381f3b4 100644
--- a/pkg/app/piped/trigger/deployment.go
+++ b/pkg/app/piped/trigger/deployment.go
@@ -1,4 +1,4 @@
-// Copyright 2020 The PipeCD Authors.
+// Copyright 2021 The PipeCD Authors.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -18,8 +18,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"os"
-	"path/filepath"
 	"time"
 
 	"github.com/google/uuid"
@@ -36,96 +34,44 @@ const notificationsKey = "DeploymentNotification"
 func (t *Trigger) triggerDeployment(
 	ctx context.Context,
 	app *model.Application,
+	appCfg *config.GenericDeploymentSpec,
 	branch string,
 	commit git.Commit,
 	commander string,
 	syncStrategy model.SyncStrategy,
-) (deployment *model.Deployment, err error) {
-	n, err := t.getNotification(app.GitPath)
-	if err != nil {
-		t.logger.Error("failed to get the list of mentions", zap.Error(err))
-		t.reportDeploymentFailed(app, fmt.Sprintf("failed to find the list of mentions from %s: %v", app.GitPath.GetDeploymentConfigFilePath(), err), commit)
-		return
-	}
+) (*model.Deployment, error) {
 
-	deployment, err = buildDeployment(app, branch, commit, commander, syncStrategy, time.Now(), n)
+	// Build deployment model to trigger.
+	deployment, err := buildDeployment(
+		app,
+		branch,
+		commit,
+		commander,
+		syncStrategy,
+		time.Now(),
+		appCfg.DeploymentNotification,
+	)
 	if err != nil {
-		t.logger.Error("failed to build the deployment", zap.Error(err))
-		t.reportDeploymentFailed(app, fmt.Sprintf("failed to build the deployment: %v", err), commit)
-		return
-	}
-
-	var as []string
-	if n != nil {
-		as = n.FindSlackAccounts(model.NotificationEventType_EVENT_DEPLOYMENT_TRIGGERED)
+		return nil, fmt.Errorf("could not initialize deployment: %w", err)
 	}
 
-	defer func() {
-		if err != nil {
-			t.reportDeploymentFailed(app, fmt.Sprintf("%v", err), commit)
-			return
-		}
-		env, err := t.environmentLister.Get(ctx, deployment.EnvId)
-		if err != nil {
-			t.reportDeploymentFailed(app, fmt.Sprintf("%v", err), commit)
-			return
-		}
-		t.notifier.Notify(model.NotificationEvent{
-			Type: model.NotificationEventType_EVENT_DEPLOYMENT_TRIGGERED,
-			Metadata: &model.NotificationEventDeploymentTriggered{
-				Deployment:        deployment,
-				EnvName:           env.Name,
-				MentionedAccounts: as,
-			},
-		})
-	}()
-
-	t.logger.Info(fmt.Sprintf("application %s will be triggered to sync", app.Id),
-		zap.String("commit-hash", commit.Hash),
-	)
-	req := &pipedservice.CreateDeploymentRequest{
+	// Send deployment model to control-plane to trigger.
+	t.logger.Info(fmt.Sprintf("application %s will be triggered to sync", app.Id), zap.String("commit", commit.Hash))
+	_, err = t.apiClient.CreateDeployment(ctx, &pipedservice.CreateDeploymentRequest{
 		Deployment: deployment,
-	}
-	if _, err = t.apiClient.CreateDeployment(ctx, req); err != nil {
-		t.logger.Error("failed to create deployment", zap.Error(err))
-		t.reportDeploymentFailed(app, fmt.Sprintf("failed to create deployment: %v", err), commit)
-		return
+	})
+	if err != nil {
+		return nil, fmt.Errorf("cound not register a new deployment to control-plane: %w", err)
 	}
 
 	// TODO: Find a better way to ensure that the application should be updated correctly
 	// when the deployment was successfully triggered.
-	if e := t.reportMostRecentlyTriggeredDeployment(ctx, deployment); e != nil {
+	// This error is ignored because the deployment was already registered successfully.
+	if e := reportMostRecentlyTriggeredDeployment(ctx, t.apiClient, deployment); e != nil {
 		t.logger.Error("failed to report most recently triggered deployment", zap.Error(e))
 	}
 
-	return
-}
-
-func (t *Trigger) reportMostRecentlyTriggeredDeployment(ctx context.Context, d *model.Deployment) error {
-	var (
-		err error
-		req = &pipedservice.ReportApplicationMostRecentDeploymentRequest{
-			ApplicationId: d.ApplicationId,
-			Status:        model.DeploymentStatus_DEPLOYMENT_PENDING,
-			Deployment: &model.ApplicationDeploymentReference{
-				DeploymentId: d.Id,
-				Trigger:      d.Trigger,
-				Summary:      d.Summary,
-				Version:      d.Version,
-				StartedAt:    d.CreatedAt,
-				CompletedAt:  d.CompletedAt,
-			},
-		}
-		retry = pipedservice.NewRetry(10)
-	)
-
-	for retry.WaitNext(ctx) {
-		if _, err = t.apiClient.ReportApplicationMostRecentDeployment(ctx, req); err == nil {
-			return nil
-		}
-		err = fmt.Errorf("failed to report most recent successful deployment: %w", err)
-	}
-	return err
+	return deployment, nil
 }
 
 func buildDeployment(
@@ -135,21 +81,23 @@ func buildDeployment(
 	commander string,
 	syncStrategy model.SyncStrategy,
 	now time.Time,
-	notification *config.DeploymentNotification,
+	noti *config.DeploymentNotification,
 ) (*model.Deployment, error) {
-	commitURL := ""
+
+	var commitURL string
 	if r := app.GitPath.Repo; r != nil {
-		var err error
-		commitURL, err = git.MakeCommitURL(r.Remote, commit.Hash)
+		url, err := git.MakeCommitURL(r.Remote, commit.Hash)
 		if err != nil {
 			return nil, err
 		}
+		commitURL = url
 	}
+
 	metadata := make(map[string]string)
-	if notification != nil {
-		value, err := json.Marshal(notification)
+	if noti != nil {
+		value, err := json.Marshal(noti)
 		if err != nil {
-			return nil, fmt.Errorf("failed to save mention config to deployment metadata: %w", err)
+			return nil, fmt.Errorf("failed to save notification config to deployment metadata: %w", err)
 		}
 		metadata[notificationsKey] = string(value)
 	}
@@ -188,40 +136,29 @@ func buildDeployment(
 	return deployment, nil
 }
 
-func (t *Trigger) reportDeploymentFailed(app *model.Application, reason string, commit git.Commit) {
-	t.notifier.Notify(model.NotificationEvent{
-		Type: model.NotificationEventType_EVENT_DEPLOYMENT_TRIGGER_FAILED,
-		Metadata: &model.NotificationEventDeploymentTriggerFailed{
-			Application:   app,
-			CommitHash:    commit.Hash,
-			CommitMessage: commit.Message,
-			Reason:        reason,
-		},
-	})
-}
-
-func (t *Trigger) getNotification(p *model.ApplicationGitPath) (*config.DeploymentNotification, error) {
-	// Find the application repo from pre-loaded ones.
-	repo, ok := t.gitRepos[p.Repo.Id]
-	if !ok {
-		t.logger.Warn("detected some applications binding with a non existent repository", zap.String("repo-id", p.Repo.Id))
-		return nil, fmt.Errorf("unknown repo %q is set to the deployment", p.Repo.Id)
-	}
-
-	absPath := filepath.Join(repo.GetPath(), p.GetDeploymentConfigFilePath())
-
-	cfg, err := config.LoadFromYAML(absPath)
-	if err != nil {
-		if os.IsNotExist(err) {
-			return nil, fmt.Errorf("deployment config file %s was not found", p.GetDeploymentConfigFilePath())
+func reportMostRecentlyTriggeredDeployment(ctx context.Context, client apiClient, d *model.Deployment) error {
+	var (
+		err error
+		req = &pipedservice.ReportApplicationMostRecentDeploymentRequest{
+			ApplicationId: d.ApplicationId,
+			Status:        model.DeploymentStatus_DEPLOYMENT_PENDING,
+			Deployment: &model.ApplicationDeploymentReference{
+				DeploymentId: d.Id,
+				Trigger:      d.Trigger,
+				Summary:      d.Summary,
+				Version:      d.Version,
+				StartedAt:    d.CreatedAt,
+				CompletedAt:  d.CompletedAt,
+			},
 		}
-		return nil, err
-	}
+		retry = pipedservice.NewRetry(10)
+	)
 
-	spec, ok := cfg.GetGenericDeployment()
-	if !ok {
-		return nil, fmt.Errorf("unsupported application kind: %s", cfg.Kind)
+	for retry.WaitNext(ctx) {
+		if _, err = client.ReportApplicationMostRecentDeployment(ctx, req); err == nil {
+			return nil
+		}
+		err = fmt.Errorf("failed to report most recent successful deployment: %w", err)
 	}
-
-	return spec.DeploymentNotification, nil
+	return err
 }
diff --git a/pkg/app/piped/trigger/determiner.go b/pkg/app/piped/trigger/determiner.go
index 5758f681f8..2c2dad938b 100644
--- a/pkg/app/piped/trigger/determiner.go
+++ b/pkg/app/piped/trigger/determiner.go
@@ -17,8 +17,6 @@ package trigger
 import (
 	"context"
 	"fmt"
-	"os"
-	"path/filepath"
 	"strings"
 
 	"go.uber.org/zap"
@@ -29,50 +27,81 @@ import (
 	"github.com/pipe-cd/pipe/pkg/model"
 )
 
+type Determiner interface {
+	ShouldTrigger(ctx context.Context, app *model.Application, appCfg *config.GenericDeploymentSpec) (bool, error)
+}
+
+type determiners struct {
+	onCommand   Determiner
+	onOutOfSync Determiner
+	onCommit    Determiner
+}
+
+func (ds *determiners) Determiner(ck candidateKind) Determiner {
+	switch ck {
+	case commandCandidate:
+		return ds.onCommand
+	case outOfSyncCandidate:
+		return ds.onOutOfSync
+	default:
+		return ds.onCommit
+	}
+}
+
+type OnCommandDeterminer struct {
+}
+
+// ShouldTrigger decides whether a given application should be triggered or not.
+func (d *OnCommandDeterminer) ShouldTrigger(ctx context.Context, app *model.Application, appCfg *config.GenericDeploymentSpec) (bool, error) {
+	if appCfg.Trigger.OnCommand.Disabled {
+		return false, nil
+	}
+
+	return true, nil
+}
+
+type OnOutOfSyncDeterminer struct {
+}
+
+// ShouldTrigger decides whether a given application should be triggered or not.
+func (d *OnOutOfSyncDeterminer) ShouldTrigger(ctx context.Context, app *model.Application, appCfg *config.GenericDeploymentSpec) (bool, error) {
+	if appCfg.Trigger.OnOutOfSync.Disabled {
+		return false, nil
+	}
+
+	return true, nil
+}
+
 type LastTriggeredCommitGetter interface {
 	Get(ctx context.Context, applicationID string) (string, error)
 }
 
-type Determiner struct {
+type OnCommitDeterminer struct {
 	repo         git.Repo
 	targetCommit string
 	commitGetter LastTriggeredCommitGetter
-	// Flag `ignoreUserConfig` set to `true` will force check changes and use it to determine
-	// the application deployment should be triggered or not, regardless of the user's configuration.
-	ignoreUserConfig bool
-	logger           *zap.Logger
+	logger       *zap.Logger
 }
 
-func NewDeterminer(repo git.Repo, targetCommit string, cg LastTriggeredCommitGetter, ignoreUserConfig bool, logger *zap.Logger) *Determiner {
-	return &Determiner{
-		repo:             repo,
-		targetCommit:     targetCommit,
-		commitGetter:     cg,
-		ignoreUserConfig: ignoreUserConfig,
-		logger:           logger.Named("determiner"),
+func NewOnCommitDeterminer(repo git.Repo, targetCommit string, cg LastTriggeredCommitGetter, logger *zap.Logger) Determiner {
+	return &OnCommitDeterminer{
+		repo:         repo,
+		targetCommit: targetCommit,
+		commitGetter: cg,
+		logger:       logger.Named("determiner"),
 	}
 }
 
 // ShouldTrigger decides whether a given application should be triggered or not.
-func (d *Determiner) ShouldTrigger(ctx context.Context, app *model.Application) (bool, error) {
+func (d *OnCommitDeterminer) ShouldTrigger(ctx context.Context, app *model.Application, appCfg *config.GenericDeploymentSpec) (bool, error) {
 	logger := d.logger.With(
 		zap.String("app", app.Name),
 		zap.String("app-id", app.Id),
 		zap.String("target-commit", d.targetCommit),
 	)
 
-	// TODO: Add logic to determine trigger or not based on other configuration than onCommit.
-	return d.shouldTriggerOnCommit(ctx, app, logger)
-}
-
-func (d *Determiner) shouldTriggerOnCommit(ctx context.Context, app *model.Application, logger *zap.Logger) (bool, error) {
-	deployConfig, err := loadDeploymentConfiguration(d.repo.GetPath(), app)
-	if err != nil {
-		return false, err
-	}
-
 	// Not trigger in case users disable auto trigger deploy on change and the user config is unignorable.
-	if deployConfig.Trigger.OnCommit.Disabled && !d.ignoreUserConfig {
+	if appCfg.Trigger.OnCommit.Disabled {
 		logger.Info(fmt.Sprintf("auto trigger deployment disabled for application, hash: %s", d.targetCommit))
 		return false, nil
 	}
@@ -104,19 +133,19 @@ func (d *Determiner) shouldTriggerOnCommit(ctx context.Context, app *model.Appli
 		return false, err
 	}
 
-	// TODO: Remove deprecated `deployConfig.TriggerPaths` configuration.
-	checkingPaths := make([]string, 0, len(deployConfig.Trigger.OnCommit.Paths)+len(deployConfig.TriggerPaths))
-	// Note: deployConfig.TriggerPaths or deployConfig.Trigger.OnCommit.Paths may contain "" (empty string)
+	// TODO: Remove deprecated `appCfg.TriggerPaths` configuration.
+	checkingPaths := make([]string, 0, len(appCfg.Trigger.OnCommit.Paths)+len(appCfg.TriggerPaths))
+	// Note: appCfg.TriggerPaths or appCfg.Trigger.OnCommit.Paths may contain "" (empty string)
 	// in case users use one of them without the other, that cause unexpected "" path in the checkingPaths list
 	// leads to always trigger deployment since "" path matched all other paths.
 	// The below logic is to remove that "" path from checking path list, will remove after remove the
-	// deprecated deployConfig.TriggerPaths.
-	for _, p := range deployConfig.Trigger.OnCommit.Paths {
+	// deprecated appCfg.TriggerPaths.
+	for _, p := range appCfg.Trigger.OnCommit.Paths {
 		if p != "" {
 			checkingPaths = append(checkingPaths, p)
 		}
 	}
-	for _, p := range deployConfig.TriggerPaths {
+	for _, p := range appCfg.TriggerPaths {
 		if p != "" {
 			checkingPaths = append(checkingPaths, p)
 		}
@@ -135,31 +164,6 @@ func (d *Determiner) shouldTriggerOnCommit(ctx context.Context, app *model.Appli
 	return true, nil
 }
 
-func loadDeploymentConfiguration(repoPath string, app *model.Application) (*config.GenericDeploymentSpec, error) {
-	var (
-		relPath = app.GitPath.GetDeploymentConfigFilePath()
-		absPath = filepath.Join(repoPath, relPath)
-	)
-
-	cfg, err := config.LoadFromYAML(absPath)
-	if err != nil {
-		if os.IsNotExist(err) {
-			return nil, fmt.Errorf("deployment config file %s was not found", relPath)
-		}
-		return nil, err
-	}
-	if appKind, ok := config.ToApplicationKind(cfg.Kind); !ok || appKind != app.Kind {
-		return nil, fmt.Errorf("invalid application kind in the deployment config file, got: %s, expected: %s", appKind, app.Kind)
-	}
-
-	spec, ok := cfg.GetGenericDeployment()
-	if !ok {
-		return nil, fmt.Errorf("unsupported application kind: %s", app.Kind)
-	}
-
-	return &spec, nil
-}
-
 func isTouchedByChangedFiles(appDir string, changes []string, changedFiles []string) (bool, error) {
 	if !strings.HasSuffix(appDir, "/") {
 		appDir += "/"
diff --git a/pkg/app/piped/trigger/trigger.go b/pkg/app/piped/trigger/trigger.go
index e9822c545d..d325feef89 100644
--- a/pkg/app/piped/trigger/trigger.go
+++ b/pkg/app/piped/trigger/trigger.go
@@ -1,4 +1,4 @@
-// Copyright 2020 The PipeCD Authors.
+// Copyright 2021 The PipeCD Authors.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -20,6 +20,8 @@ package trigger
 import (
 	"context"
 	"fmt"
+	"os"
+	"path/filepath"
 	"time"
 
 	"go.uber.org/zap"
@@ -33,12 +35,9 @@ import (
 )
 
 const (
-	commandCheckInterval                = 10 * time.Second
+	ondemandCheckInterval               = 10 * time.Second
 	defaultLastTriggeredCommitCacheSize = 500
-)
-
-const (
-	triggeredDeploymentIDKey = "TriggeredDeploymentID"
+	triggeredDeploymentIDKey            = "TriggeredDeploymentID"
 )
 
 type apiClient interface {
@@ -68,6 +67,20 @@ type notifier interface {
 	Notify(event model.NotificationEvent)
 }
 
+type candidateKind int
+
+const (
+	commitCandidate candidateKind = iota
+	commandCandidate
+	outOfSyncCandidate
+)
+
+type candidate struct {
+	application *model.Application
+	kind        candidateKind
+	command     model.ReportableCommand
+}
+
 type Trigger struct {
 	apiClient         apiClient
 	gitClient         gitClient
@@ -82,7 +95,6 @@ type Trigger struct {
 	logger            *zap.Logger
 }
 
-// NewTrigger creates a new instance for Trigger.
 func NewTrigger(
 	apiClient apiClient,
 	gitClient gitClient,
@@ -121,211 +133,288 @@ func NewTrigger(
 	return t, nil
 }
 
-// Run starts running Trigger until the specified context has done.
-// This also waits for its cleaning up before returning.
 func (t *Trigger) Run(ctx context.Context) error {
 	t.logger.Info("start running deployment trigger")
 
-	// Pre-clone to cache the registered git repositories.
+	// Pre cloning to cache the registered git repositories.
 	t.gitRepos = make(map[string]git.Repo, len(t.config.Repositories))
 	for _, r := range t.config.Repositories {
 		repo, err := t.gitClient.Clone(ctx, r.RepoID, r.Remote, r.Branch, "")
 		if err != nil {
-			t.logger.Error("failed to clone repository",
-				zap.String("repo-id", r.RepoID),
-				zap.Error(err),
-			)
+			t.logger.Error(fmt.Sprintf("failed to clone git repository %s", r.RepoID), zap.Error(err))
 			return err
 		}
 		t.gitRepos[r.RepoID] = repo
 	}
 
-	commitTicker := time.NewTicker(time.Duration(t.config.SyncInterval))
-	defer commitTicker.Stop()
+	syncTicker := time.NewTicker(time.Duration(t.config.SyncInterval))
+	defer syncTicker.Stop()
 
-	commandTicker := time.NewTicker(commandCheckInterval)
-	defer commandTicker.Stop()
+	ondemandTicker := time.NewTicker(ondemandCheckInterval)
+	defer ondemandTicker.Stop()
 
-L:
 	for {
 		select {
-
-		case <-commandTicker.C:
-			t.checkNewCommands(ctx)
-
-		case <-commitTicker.C:
-			t.checkNewCommits(ctx)
+		case <-syncTicker.C:
+			var (
+				commitCandidates    = t.listCommitCandidates(ctx)
+				outOfSyncCandidates = t.listOutOfSyncCandidates(ctx)
+				candidates          = append(commitCandidates, outOfSyncCandidates...)
+			)
+			t.logger.Info(fmt.Sprintf("found %d candidates: %d commit candidates and %d out_of_sync candidates",
+				len(candidates),
+				len(commitCandidates),
+				len(outOfSyncCandidates),
+			))
+			t.checkCandidates(ctx, candidates)
+
+		case <-ondemandTicker.C:
+			candidates := t.listCommandCandidates(ctx)
+			t.logger.Info(fmt.Sprintf("found %d command candidates", len(candidates)))
+			t.checkCandidates(ctx, candidates)
 
 		case <-ctx.Done():
-			break L
+			t.logger.Info("deployment trigger has been stopped")
+			return nil
 		}
 	}
-
-	t.logger.Info("deployment trigger has been stopped")
-	return nil
 }
 
-func (t *Trigger) GetLastTriggeredCommitGetter() LastTriggeredCommitGetter {
-	return t.commitStore
+func (t *Trigger) checkCandidates(ctx context.Context, cs []candidate) (err error) {
+	// Group candidates by repository to reduce the number of Git operations on each repo.
+	csm := make(map[string][]candidate)
+	for _, c := range cs {
+		repoId := c.application.GitPath.Repo.Id
+		if _, ok := csm[repoId]; !ok {
+			csm[repoId] = []candidate{c}
+			continue
+		}
+		csm[repoId] = append(csm[repoId], c)
+	}
+
+	// Iterate each repository and check its candidates.
+	// Only the last error will be returned.
+	for repoID, cs := range csm {
+		if e := t.checkRepoCandidates(ctx, repoID, cs); e != nil {
+			t.logger.Error(fmt.Sprintf("failed while checking applications in repo %s", repoID), zap.Error(e))
+			err = e
+		}
+	}
+	return
 }
 
-func (t *Trigger) checkNewCommands(ctx context.Context) error {
-	commands := t.commandLister.ListApplicationCommands()
+func (t *Trigger) checkRepoCandidates(ctx context.Context, repoID string, cs []candidate) error {
+	gitRepo, branch, headCommit, err := t.updateRepoToLatest(ctx, repoID)
+	if err != nil {
+		// TODO: Find a better way to skip the CANCELLED error log while shutting down.
+		if ctx.Err() != context.Canceled {
+			t.logger.Error(fmt.Sprintf("failed to update git repository %s to latest", repoID), zap.Error(err))
+		}
+		return err
+	}
 
-	for _, cmd := range commands {
-		syncCmd := cmd.GetSyncApplication()
-		if syncCmd == nil {
+	ds := &determiners{
+		onCommand:   &OnCommandDeterminer{},
+		onOutOfSync: &OnOutOfSyncDeterminer{},
+		onCommit:    NewOnCommitDeterminer(gitRepo, headCommit.Hash, t.commitStore, t.logger),
+	}
+
+	for _, c := range cs {
+		app := c.application
+
+		appCfg, err := loadDeploymentConfiguration(gitRepo.GetPath(), app)
+		if err != nil {
+			msg := fmt.Sprintf("failed to load application config file at %s: %v", app.GitPath.GetDeploymentConfigFilePath(), err)
+			t.notifyDeploymentTriggerFailed(app, msg, headCommit)
 			continue
 		}
 
-		app, ok := t.applicationLister.Get(syncCmd.ApplicationId)
-		if !ok {
-			t.logger.Warn("detected an AppSync command for an unregistered application",
-				zap.String("command", cmd.Id),
-				zap.String("app-id", syncCmd.ApplicationId),
-				zap.String("commander", cmd.Commander),
-			)
+		shouldTrigger, err := ds.Determiner(c.kind).ShouldTrigger(ctx, app, appCfg)
+		if err != nil {
+			msg := fmt.Sprintf("failed while determining whether application %s should be triggered or not: %s", app.Name, err)
+			t.notifyDeploymentTriggerFailed(app, msg, headCommit)
 			continue
 		}
 
-		d, err := t.syncApplication(ctx, app, cmd.Commander, syncCmd.SyncStrategy)
-		if err != nil {
-			t.logger.Error("failed to sync application",
-				zap.String("app-id", app.Id),
-				zap.Error(err),
-			)
-			if err := cmd.Report(ctx, model.CommandStatus_COMMAND_FAILED, nil, nil); err != nil {
-				t.logger.Error("failed to report command status", zap.Error(err))
-			}
+		if !shouldTrigger {
+			t.commitStore.Put(app.Id, headCommit.Hash)
 			continue
 		}
 
-		metadata := map[string]string{
-			triggeredDeploymentIDKey: d.Id,
+		// Build deployment model and send a request to API to create a new deployment.
+		deployment, err := t.triggerDeployment(ctx, app, appCfg, branch, headCommit, "", model.SyncStrategy_AUTO)
+		if err != nil {
+			msg := fmt.Sprintf("failed to trigger application %s: %v", app.Id, err)
+			t.notifyDeploymentTriggerFailed(app, msg, headCommit)
+			continue
 		}
-		if err := cmd.Report(ctx, model.CommandStatus_COMMAND_SUCCEEDED, metadata, nil); err != nil {
-			t.logger.Error("failed to report command status", zap.Error(err))
+
+		t.commitStore.Put(app.Id, headCommit.Hash)
+		t.notifyDeploymentTriggered(ctx, app, appCfg, deployment)
+
+		// Mask command as handled since the deployment has been triggered successfully.
+		if c.kind == commandCandidate {
+			metadata := map[string]string{
+				triggeredDeploymentIDKey: deployment.Id,
+			}
+			if err := c.command.Report(ctx, model.CommandStatus_COMMAND_SUCCEEDED, metadata, nil); err != nil {
+				t.logger.Error("failed to report command status", zap.Error(err))
+			}
 		}
 	}
 
 	return nil
 }
 
-func (t *Trigger) checkNewCommits(ctx context.Context) error {
-	if len(t.gitRepos) == 0 {
-		t.logger.Info("no repositories were configured for this piped")
-		return nil
-	}
-
-	// List all applications that should be handled by this piped
-	// and then group them by repository.
-	var applications = t.listApplications()
+// listCommandCandidates finds all applications that have been commanded to sync.
+func (t *Trigger) listCommandCandidates(ctx context.Context) []candidate {
+	var (
+		cmds = t.commandLister.ListApplicationCommands()
+		apps = make([]candidate, 0)
+	)
 
-	// ENHANCEMENT: We may want to apply worker model here to run them concurrently.
-	for repoID, apps := range applications {
-		gitRepo, branch, headCommit, err := t.updateRepoToLatest(ctx, repoID)
-		if err != nil {
+	for _, cmd := range cmds {
+		// Filter out commands that are not SYNC command.
+		syncCmd := cmd.GetSyncApplication()
+		if syncCmd == nil {
 			continue
 		}
-		d := NewDeterminer(gitRepo, headCommit.Hash, t.commitStore, false, t.logger)
-
-		for _, app := range apps {
-			shouldTrigger, err := d.ShouldTrigger(ctx, app)
-			if err != nil {
-				t.logger.Error(fmt.Sprintf("failed to check application: %s", app.Id), zap.Error(err))
-				t.reportDeploymentFailed(app, fmt.Sprintf("failed to find the list of mentions from %s: %v", app.GitPath.GetDeploymentConfigFilePath(), err), headCommit)
-				continue
-			}
 
-			if !shouldTrigger {
-				t.commitStore.Put(app.Id, headCommit.Hash)
-				continue
-			}
-
-			// Build deployment model and send a request to API to create a new deployment.
-			t.logger.Info("application should be synced because of the new commit")
-			if _, err := t.triggerDeployment(ctx, app, branch, headCommit, "", model.SyncStrategy_AUTO); err != nil {
-				t.logger.Error(fmt.Sprintf("failed to trigger application: %s", app.Id), zap.Error(err))
-			}
-			t.commitStore.Put(app.Id, headCommit.Hash)
+		// Find the target application specified in command.
+		app, ok := t.applicationLister.Get(syncCmd.ApplicationId)
+		if !ok {
+			t.logger.Warn("detected an AppSync command for an unregistered application",
+				zap.String("command", cmd.Id),
+				zap.String("app-id", syncCmd.ApplicationId),
+				zap.String("commander", cmd.Commander),
+			)
+			continue
 		}
+
+		apps = append(apps, candidate{
+			application: app,
+			kind:        commandCandidate,
+			command:     cmd,
+		})
 	}
 
-	return nil
+	return apps
 }
 
-func (t *Trigger) syncApplication(ctx context.Context, app *model.Application, commander string, syncStrategy model.SyncStrategy) (*model.Deployment, error) {
-	_, branch, headCommit, err := t.updateRepoToLatest(ctx, app.GitPath.Repo.Id)
-	if err != nil {
-		return nil, err
+// listOutOfSyncCandidates finds all applications that are staying at OUT_OF_SYNC state.
+func (t *Trigger) listOutOfSyncCandidates(ctx context.Context) []candidate {
+	var (
+		list = t.applicationLister.List()
+		apps = make([]candidate, 0)
+	)
+	for _, app := range list {
+		if app.SyncState.Status != model.ApplicationSyncStatus_OUT_OF_SYNC {
+			continue
+		}
+		apps = append(apps, candidate{
+			application: app,
+			kind:        outOfSyncCandidate,
+		})
 	}
+	return apps
+}
 
-	// Build deployment model and send a request to API to create a new deployment.
-	t.logger.Info(fmt.Sprintf("application %s will be synced because of a sync command", app.Id),
-		zap.String("head-commit", headCommit.Hash),
+// listCommitCandidates finds all applications that have potentiality
+// to be candidates by the changes of new commits.
+// They are all applications managed by this Piped.
+func (t *Trigger) listCommitCandidates(ctx context.Context) []candidate {
+	var (
+		list = t.applicationLister.List()
+		apps = make([]candidate, 0)
 	)
-	d, err := t.triggerDeployment(ctx, app, branch, headCommit, commander, syncStrategy)
-	if err != nil {
-		return nil, err
+	for _, app := range list {
+		apps = append(apps, candidate{
+			application: app,
+			kind:        commitCandidate,
+		})
 	}
-	t.commitStore.Put(app.Id, headCommit.Hash)
-
-	return d, nil
+	return apps
 }
 
+// updateRepoToLatest ensures that the local data of the given Git repository should be up-to-date.
 func (t *Trigger) updateRepoToLatest(ctx context.Context, repoID string) (repo git.Repo, branch string, headCommit git.Commit, err error) {
 	var ok bool
 
-	// Find the application repo from pre-loaded ones.
+	// Find the repository from the previously loaded list.
 	repo, ok = t.gitRepos[repoID]
 	if !ok {
-		t.logger.Warn("detected some applications binding with a non existent repository", zap.String("repo-id", repoID))
-		err = fmt.Errorf("missing repository")
+		err = fmt.Errorf("the repository was not registered in Piped configuration")
 		return
 	}
 	branch = repo.GetClonedBranch()
 
-	// Fetch to update the repository and then
-	if err = repo.Pull(ctx, branch); err != nil {
-		if ctx.Err() != context.Canceled {
-			t.logger.Error("failed to update repository branch",
-				zap.String("repo-id", repoID),
-				zap.Error(err),
-			)
-		}
+	// Fetch to update the repository.
+	err = repo.Pull(ctx, branch)
+	if err != nil {
 		return
 	}
 
 	// Get the head commit of the repository.
 	headCommit, err = repo.GetLatestCommit(ctx)
-	if err != nil {
-		// TODO: Find a better way to skip the CANCELLED error log while shutting down.
-		if ctx.Err() != context.Canceled {
-			t.logger.Error("failed to get head commit hash",
-				zap.String("repo-id", repoID),
-				zap.Error(err),
-			)
-		}
-		return
+	return
+}
+
+func (t *Trigger) GetLastTriggeredCommitGetter() LastTriggeredCommitGetter {
+	return t.commitStore
+}
+
+func (t *Trigger) notifyDeploymentTriggered(ctx context.Context, app *model.Application, appCfg *config.GenericDeploymentSpec, d *model.Deployment) {
+	var mentions []string
+	if n := appCfg.DeploymentNotification; n != nil {
+		mentions = n.FindSlackAccounts(model.NotificationEventType_EVENT_DEPLOYMENT_TRIGGERED)
 	}
 
-	return
+	if env, err := t.environmentLister.Get(ctx, d.EnvId); err == nil {
+		t.notifier.Notify(model.NotificationEvent{
+			Type: model.NotificationEventType_EVENT_DEPLOYMENT_TRIGGERED,
+			Metadata: &model.NotificationEventDeploymentTriggered{
+				Deployment:        d,
+				EnvName:           env.Name,
+				MentionedAccounts: mentions,
+			},
+		})
+	}
 }
 
-// listApplications retrieves all applications those should be handled by this piped
-// and then groups them by repoID.
-func (t *Trigger) listApplications() map[string][]*model.Application {
+func (t *Trigger) notifyDeploymentTriggerFailed(app *model.Application, reason string, commit git.Commit) {
+	t.notifier.Notify(model.NotificationEvent{
+		Type: model.NotificationEventType_EVENT_DEPLOYMENT_TRIGGER_FAILED,
+		Metadata: &model.NotificationEventDeploymentTriggerFailed{
+			Application:   app,
+			CommitHash:    commit.Hash,
+			CommitMessage: commit.Message,
+			Reason:        reason,
+		},
+	})
+	t.logger.Error(reason)
+}
+
+func loadDeploymentConfiguration(repoPath string, app *model.Application) (*config.GenericDeploymentSpec, error) {
 	var (
-		apps = t.applicationLister.List()
-		m    = make(map[string][]*model.Application)
+		relPath = app.GitPath.GetDeploymentConfigFilePath()
+		absPath = filepath.Join(repoPath, relPath)
 	)
-	for _, app := range apps {
-		repoId := app.GitPath.Repo.Id
-		if _, ok := m[repoId]; !ok {
-			m[repoId] = []*model.Application{app}
-		} else {
-			m[repoId] = append(m[repoId], app)
+
+	cfg, err := config.LoadFromYAML(absPath)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil, fmt.Errorf("application config file %s was not found in Git", relPath)
 		}
+		return nil, err
 	}
-	return m
+	if appKind, ok := config.ToApplicationKind(cfg.Kind); !ok || appKind != app.Kind {
+		return nil, fmt.Errorf("invalid application kind in the deployment config file, got: %s, expected: %s", appKind, app.Kind)
+	}
+
+	spec, ok := cfg.GetGenericDeployment()
+	if !ok {
+		return nil, fmt.Errorf("unsupported application kind: %s", app.Kind)
+	}
+
+	return &spec, nil
 }
diff --git a/pkg/config/deployment.go b/pkg/config/deployment.go
index e9659c0435..30741aa14d 100644
--- a/pkg/config/deployment.go
+++ b/pkg/config/deployment.go
@@ -59,18 +59,43 @@ type DeploymentPlanner struct {
 }
 
 type Trigger struct {
-	// Configuration in case changes on commit are found.
+	// Configurable fields used while deciding the application
+	// should be triggered or not based on commit changes.
 	OnCommit OnCommit `json:"onCommit"`
+	// Configurable fields used while deciding the application
+	// should be triggered or not based on received SYNC command.
+	OnCommand OnCommand `json:"onCommand"`
+	// Configurable fields used while deciding the application
+	// should be triggered or not based on OUT_OF_SYNC state.
+	OnOutOfSync OnOutOfSync `json:"onOutOfSync"`
 }
 
 type OnCommit struct {
-	// Control trigger new deployment on Git change or not.
+	// Whether to exclude application from triggering target
+	// when a new commit touched the application.
+	// Default is false.
 	Disabled bool `json:"disabled,omitempty"`
 	// List of directories or files where their changes will trigger the deployment.
 	// Regular expression can be used.
 	Paths []string `json:"paths,omitempty"`
 }
 
+type OnCommand struct {
+	// Whether to exclude application from triggering target
+	// when received a new SYNC command.
+	// Default is false.
+	Disabled bool `json:"disabled,omitempty"`
+}
+
+type OnOutOfSync struct {
+	// Whether to exclude application from triggering target
+	// when application is at OUT_OF_SYNC state.
+	// Default is true.
+	Disabled bool `json:"disabled,omitempty" default:"true"`
+	// TODO: Add a field to control the trigger frequency.
+	// MinWindow Duration `json:"minWindow,omitempty"`
+}
+
 func (s *GenericDeploymentSpec) Validate() error {
 	if s.Pipeline != nil {
 		for _, stage := range s.Pipeline.Stages {

From f558738bc2a29e701aa9c34c23b25d1cbf090f92 Mon Sep 17 00:00:00 2001
From: nghialv <nghialv2607@gmail.com>
Date: Fri, 12 Nov 2021 01:22:04 +0900
Subject: [PATCH 2/6] Fix tests

---
 pkg/app/piped/trigger/determiner.go      |  4 ++--
 pkg/app/piped/trigger/trigger.go         | 16 ++++++++--------
 pkg/config/deployment_cloudrun_test.go   |  6 ++++++
 pkg/config/deployment_ecs_test.go        |  6 ++++++
 pkg/config/deployment_kubernetes_test.go |  6 ++++++
 pkg/config/deployment_lambda_test.go     |  6 ++++++
 pkg/config/deployment_terraform_test.go  | 24 ++++++++++++++++++++++++
 7 files changed, 58 insertions(+), 10 deletions(-)

diff --git a/pkg/app/piped/trigger/determiner.go b/pkg/app/piped/trigger/determiner.go
index 2c2dad938b..714429f857 100644
--- a/pkg/app/piped/trigger/determiner.go
+++ b/pkg/app/piped/trigger/determiner.go
@@ -52,7 +52,7 @@ type OnCommandDeterminer struct {
 }
 
 // ShouldTrigger decides whether a given application should be triggered or not.
-func (d *OnCommandDeterminer) ShouldTrigger(ctx context.Context, app *model.Application, appCfg *config.GenericDeploymentSpec) (bool, error) {
+func (d *OnCommandDeterminer) ShouldTrigger(_ context.Context, app *model.Application, appCfg *config.GenericDeploymentSpec) (bool, error) {
 	if appCfg.Trigger.OnCommand.Disabled {
 		return false, nil
 	}
@@ -64,7 +64,7 @@ type OnOutOfSyncDeterminer struct {
 }
 
 // ShouldTrigger decides whether a given application should be triggered or not.
-func (d *OnOutOfSyncDeterminer) ShouldTrigger(ctx context.Context, app *model.Application, appCfg *config.GenericDeploymentSpec) (bool, error) {
+func (d *OnOutOfSyncDeterminer) ShouldTrigger(_ context.Context, app *model.Application, appCfg *config.GenericDeploymentSpec) (bool, error) {
 	if appCfg.Trigger.OnOutOfSync.Disabled {
 		return false, nil
 	}
diff --git a/pkg/app/piped/trigger/trigger.go b/pkg/app/piped/trigger/trigger.go
index d325feef89..e05d73268d 100644
--- a/pkg/app/piped/trigger/trigger.go
+++ b/pkg/app/piped/trigger/trigger.go
@@ -157,8 +157,8 @@ func (t *Trigger) Run(ctx context.Context) error {
 		select {
 		case <-syncTicker.C:
 			var (
-				commitCandidates    = t.listCommitCandidates(ctx)
-				outOfSyncCandidates = t.listOutOfSyncCandidates(ctx)
+				commitCandidates    = t.listCommitCandidates()
+				outOfSyncCandidates = t.listOutOfSyncCandidates()
 				candidates          = append(commitCandidates, outOfSyncCandidates...)
 			)
 			t.logger.Info(fmt.Sprintf("found %d candidates: %d commit candidates and %d out_of_sync candidates",
@@ -169,7 +169,7 @@ func (t *Trigger) Run(ctx context.Context) error {
 			t.checkCandidates(ctx, candidates)
 
 		case <-ondemandTicker.C:
-			candidates := t.listCommandCandidates(ctx)
+			candidates := t.listCommandCandidates()
 			t.logger.Info(fmt.Sprintf("found %d command candidates", len(candidates)))
 			t.checkCandidates(ctx, candidates)
 
@@ -250,7 +250,7 @@ func (t *Trigger) checkRepoCandidates(ctx context.Context, repoID string, cs []c
 		}
 
 		t.commitStore.Put(app.Id, headCommit.Hash)
-		t.notifyDeploymentTriggered(ctx, app, appCfg, deployment)
+		t.notifyDeploymentTriggered(ctx, appCfg, deployment)
 
 		// Mask command as handled since the deployment has been triggered successfully.
 		if c.kind == commandCandidate {
@@ -267,7 +267,7 @@ func (t *Trigger) checkRepoCandidates(ctx context.Context, repoID string, cs []c
 }
 
 // listCommandCandidates finds all applications that have been commanded to sync.
-func (t *Trigger) listCommandCandidates(ctx context.Context) []candidate {
+func (t *Trigger) listCommandCandidates() []candidate {
 	var (
 		cmds = t.commandLister.ListApplicationCommands()
 		apps = make([]candidate, 0)
@@ -302,7 +302,7 @@ func (t *Trigger) listCommandCandidates(ctx context.Context) []candidate {
 }
 
 // listOutOfSyncCandidates finds all applications that are staying at OUT_OF_SYNC state.
-func (t *Trigger) listOutOfSyncCandidates(ctx context.Context) []candidate {
+func (t *Trigger) listOutOfSyncCandidates() []candidate {
 	var (
 		list = t.applicationLister.List()
 		apps = make([]candidate, 0)
@@ -322,7 +322,7 @@ func (t *Trigger) listOutOfSyncCandidates(ctx context.Context) []candidate {
 // listCommitCandidates finds all applications that have potentiality
 // to be candidates by the changes of new commits.
 // They are all applications managed by this Piped.
-func (t *Trigger) listCommitCandidates(ctx context.Context) []candidate {
+func (t *Trigger) listCommitCandidates() []candidate {
 	var (
 		list = t.applicationLister.List()
 		apps = make([]candidate, 0)
@@ -363,7 +363,7 @@ func (t *Trigger) GetLastTriggeredCommitGetter() LastTriggeredCommitGetter {
 	return t.commitStore
 }
 
-func (t *Trigger) notifyDeploymentTriggered(ctx context.Context, app *model.Application, appCfg *config.GenericDeploymentSpec, d *model.Deployment) {
+func (t *Trigger) notifyDeploymentTriggered(ctx context.Context, appCfg *config.GenericDeploymentSpec, d *model.Deployment) {
 	var mentions []string
 	if n := appCfg.DeploymentNotification; n != nil {
 		mentions = n.FindSlackAccounts(model.NotificationEventType_EVENT_DEPLOYMENT_TRIGGERED)
diff --git a/pkg/config/deployment_cloudrun_test.go b/pkg/config/deployment_cloudrun_test.go
index 23ff9bf0f3..638a7c50cf 100644
--- a/pkg/config/deployment_cloudrun_test.go
+++ b/pkg/config/deployment_cloudrun_test.go
@@ -41,6 +41,12 @@ func TestCloudRunDeploymentConfig(t *testing.T) {
 						OnCommit: OnCommit{
 							Disabled: false,
 						},
+						OnCommand: OnCommand{
+							Disabled: false,
+						},
+						OnOutOfSync: OnOutOfSync{
+							Disabled: true,
+						},
 					},
 				},
 				Input: CloudRunDeploymentInput{
diff --git a/pkg/config/deployment_ecs_test.go b/pkg/config/deployment_ecs_test.go
index d62a32dafa..453a2f1184 100644
--- a/pkg/config/deployment_ecs_test.go
+++ b/pkg/config/deployment_ecs_test.go
@@ -42,6 +42,12 @@ func TestECSDeploymentConfig(t *testing.T) {
 						OnCommit: OnCommit{
 							Disabled: false,
 						},
+						OnCommand: OnCommand{
+							Disabled: false,
+						},
+						OnOutOfSync: OnOutOfSync{
+							Disabled: true,
+						},
 					},
 				},
 				Input: ECSDeploymentInput{
diff --git a/pkg/config/deployment_kubernetes_test.go b/pkg/config/deployment_kubernetes_test.go
index e3f681efae..0e78c10b9b 100644
--- a/pkg/config/deployment_kubernetes_test.go
+++ b/pkg/config/deployment_kubernetes_test.go
@@ -83,6 +83,12 @@ func TestKubernetesDeploymentConfig(t *testing.T) {
 						OnCommit: OnCommit{
 							Disabled: false,
 						},
+						OnCommand: OnCommand{
+							Disabled: false,
+						},
+						OnOutOfSync: OnOutOfSync{
+							Disabled: true,
+						},
 					},
 				},
 				Input: KubernetesDeploymentInput{
diff --git a/pkg/config/deployment_lambda_test.go b/pkg/config/deployment_lambda_test.go
index 111f3bc3a0..2352d2c380 100644
--- a/pkg/config/deployment_lambda_test.go
+++ b/pkg/config/deployment_lambda_test.go
@@ -43,6 +43,12 @@ func TestLambdaDeploymentConfig(t *testing.T) {
 						OnCommit: OnCommit{
 							Disabled: false,
 						},
+						OnCommand: OnCommand{
+							Disabled: false,
+						},
+						OnOutOfSync: OnOutOfSync{
+							Disabled: true,
+						},
 					},
 				},
 				Input: LambdaDeploymentInput{
diff --git a/pkg/config/deployment_terraform_test.go b/pkg/config/deployment_terraform_test.go
index a79ef43209..bf93b8c1bb 100644
--- a/pkg/config/deployment_terraform_test.go
+++ b/pkg/config/deployment_terraform_test.go
@@ -43,6 +43,12 @@ func TestTerraformDeploymentConfig(t *testing.T) {
 						OnCommit: OnCommit{
 							Disabled: false,
 						},
+						OnCommand: OnCommand{
+							Disabled: false,
+						},
+						OnOutOfSync: OnOutOfSync{
+							Disabled: true,
+						},
 					},
 				},
 				Input: TerraformDeploymentInput{},
@@ -60,6 +66,12 @@ func TestTerraformDeploymentConfig(t *testing.T) {
 						OnCommit: OnCommit{
 							Disabled: false,
 						},
+						OnCommand: OnCommand{
+							Disabled: false,
+						},
+						OnOutOfSync: OnOutOfSync{
+							Disabled: true,
+						},
 					},
 				},
 				Input: TerraformDeploymentInput{
@@ -87,6 +99,12 @@ func TestTerraformDeploymentConfig(t *testing.T) {
 						OnCommit: OnCommit{
 							Disabled: false,
 						},
+						OnCommand: OnCommand{
+							Disabled: false,
+						},
+						OnOutOfSync: OnOutOfSync{
+							Disabled: true,
+						},
 					},
 				},
 				Input: TerraformDeploymentInput{
@@ -127,6 +145,12 @@ func TestTerraformDeploymentConfig(t *testing.T) {
 						OnCommit: OnCommit{
 							Disabled: false,
 						},
+						OnCommand: OnCommand{
+							Disabled: false,
+						},
+						OnOutOfSync: OnOutOfSync{
+							Disabled: true,
+						},
 					},
 				},
 				Input: TerraformDeploymentInput{

From bb43c35eb7067431167b0bed99819ebfb8570e20 Mon Sep 17 00:00:00 2001
From: nghialv <nghialv2607@gmail.com>
Date: Mon, 15 Nov 2021 10:16:58 +0900
Subject: [PATCH 3/6] Fix tests

---
 pkg/config/deployment_lambda_test.go | 10 ++++++++++
 pkg/config/deployment_test.go        |  3 +++
 2 files changed, 13 insertions(+)

diff --git a/pkg/config/deployment_lambda_test.go b/pkg/config/deployment_lambda_test.go
index 2352d2c380..a246b05249 100644
--- a/pkg/config/deployment_lambda_test.go
+++ b/pkg/config/deployment_lambda_test.go
@@ -91,6 +91,11 @@ func TestLambdaDeploymentConfig(t *testing.T) {
 							},
 						},
 					},
+					Trigger: Trigger{
+						OnOutOfSync: OnOutOfSync{
+							Disabled: true,
+						},
+					},
 				},
 				Input: LambdaDeploymentInput{
 					FunctionManifestFile: "function.yaml",
@@ -123,6 +128,11 @@ func TestLambdaDeploymentConfig(t *testing.T) {
 							},
 						},
 					},
+					Trigger: Trigger{
+						OnOutOfSync: OnOutOfSync{
+							Disabled: true,
+						},
+					},
 				},
 				Input: LambdaDeploymentInput{
 					FunctionManifestFile: "function.yaml",
diff --git a/pkg/config/deployment_test.go b/pkg/config/deployment_test.go
index 8afb997882..18cddfb01f 100644
--- a/pkg/config/deployment_test.go
+++ b/pkg/config/deployment_test.go
@@ -276,6 +276,9 @@ func TestGenericTriggerConfiguration(t *testing.T) {
 								"deployment.yaml",
 							},
 						},
+						OnOutOfSync: OnOutOfSync{
+							Disabled: true,
+						},
 					},
 				},
 				Input: KubernetesDeploymentInput{

From 13450f6811ed3163b85f554a17a59f3d68bbf2da Mon Sep 17 00:00:00 2001
From: nghialv <nghialv2607@gmail.com>
Date: Mon, 15 Nov 2021 10:19:59 +0900
Subject: [PATCH 4/6] Remove unused parameters

---
 pkg/app/piped/trigger/determiner.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pkg/app/piped/trigger/determiner.go b/pkg/app/piped/trigger/determiner.go
index 714429f857..64832c60e6 100644
--- a/pkg/app/piped/trigger/determiner.go
+++ b/pkg/app/piped/trigger/determiner.go
@@ -52,7 +52,7 @@ type OnCommandDeterminer struct {
 }
 
 // ShouldTrigger decides whether a given application should be triggered or not.
-func (d *OnCommandDeterminer) ShouldTrigger(_ context.Context, app *model.Application, appCfg *config.GenericDeploymentSpec) (bool, error) {
+func (d *OnCommandDeterminer) ShouldTrigger(_ context.Context, _ *model.Application, appCfg *config.GenericDeploymentSpec) (bool, error) {
 	if appCfg.Trigger.OnCommand.Disabled {
 		return false, nil
 	}
@@ -64,7 +64,7 @@ type OnOutOfSyncDeterminer struct {
 }
 
 // ShouldTrigger decides whether a given application should be triggered or not.
-func (d *OnOutOfSyncDeterminer) ShouldTrigger(_ context.Context, app *model.Application, appCfg *config.GenericDeploymentSpec) (bool, error) {
+func (d *OnOutOfSyncDeterminer) ShouldTrigger(_ context.Context, _ *model.Application, appCfg *config.GenericDeploymentSpec) (bool, error) {
 	if appCfg.Trigger.OnOutOfSync.Disabled {
 		return false, nil
 	}

From 0ed5f99394f7b52d727f0bebe75e5577cb05722c Mon Sep 17 00:00:00 2001
From: nghialv <nghialv2607@gmail.com>
Date: Mon, 15 Nov 2021 10:28:03 +0900
Subject: [PATCH 5/6] Refine notification

---
 pkg/app/piped/trigger/trigger.go | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/pkg/app/piped/trigger/trigger.go b/pkg/app/piped/trigger/trigger.go
index e05d73268d..339a3e0f7f 100644
--- a/pkg/app/piped/trigger/trigger.go
+++ b/pkg/app/piped/trigger/trigger.go
@@ -224,8 +224,13 @@ func (t *Trigger) checkRepoCandidates(ctx context.Context, repoID string, cs []c
 
 		appCfg, err := loadDeploymentConfiguration(gitRepo.GetPath(), app)
 		if err != nil {
-			msg := fmt.Sprintf("failed to load application config file at %s: %v", app.GitPath.GetDeploymentConfigFilePath(), err)
-			t.notifyDeploymentTriggerFailed(app, msg, headCommit)
+			t.logger.Error("failed to load application config file", zap.Error(err))
+			// Do not notify this event to external services because it may cause annoying
+			// when one application is missing or having an invalid configuration file.
+			// So instead of notifying this as a notification,
+			// we should show this problem on the web with a status like INVALID_CONFIG.
+			//
+			// t.notifyDeploymentTriggerFailed(app, msg, headCommit)
 			continue
 		}
 
@@ -233,6 +238,7 @@ func (t *Trigger) checkRepoCandidates(ctx context.Context, repoID string, cs []c
 		if err != nil {
 			msg := fmt.Sprintf("failed while determining whether application %s should be triggered or not: %s", app.Name, err)
 			t.notifyDeploymentTriggerFailed(app, msg, headCommit)
+			t.logger.Error(msg, zap.Error(err))
 			continue
 		}
 
@@ -246,6 +252,7 @@ func (t *Trigger) checkRepoCandidates(ctx context.Context, repoID string, cs []c
 		if err != nil {
 			msg := fmt.Sprintf("failed to trigger application %s: %v", app.Id, err)
 			t.notifyDeploymentTriggerFailed(app, msg, headCommit)
+			t.logger.Error(msg, zap.Error(err))
 			continue
 		}
 
@@ -391,7 +398,6 @@ func (t *Trigger) notifyDeploymentTriggerFailed(app *model.Application, reason s
 			Reason:        reason,
 		},
 	})
-	t.logger.Error(reason)
 }
 
 func loadDeploymentConfiguration(repoPath string, app *model.Application) (*config.GenericDeploymentSpec, error) {

From 12396182979ad9287880d483f882b9c81ad519a5 Mon Sep 17 00:00:00 2001
From: Le Van Nghia <nghialv2607@gmail.com>
Date: Mon, 15 Nov 2021 15:09:35 +0900
Subject: [PATCH 6/6] Update deployment.go

---
 pkg/config/deployment.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/pkg/config/deployment.go b/pkg/config/deployment.go
index 30741aa14d..11b292b7d4 100644
--- a/pkg/config/deployment.go
+++ b/pkg/config/deployment.go
@@ -91,6 +91,7 @@ type OnOutOfSync struct {
 	// Whether to exclude application from triggering target
 	// when application is at OUT_OF_SYNC state.
 	// Default is true.
+	// TODO: Ensure that the given value will not be overridden by the default one.
 	Disabled bool `json:"disabled,omitempty" default:"true"`
 	// TODO: Add a field to control the trigger frequency.
 	// MinWindow Duration `json:"minWindow,omitempty"`