Cherry-pick #5523 #5540 #5558 #5571 (#5573)

* Correct notification routing for `DEPLOYMENT_STARTED` (#5523) * Correct notification routing for `DEPLOYMENT_STARTED` Signed-off-by: Yuki Okushi <[email protected]> * Harden test case Signed-off-by: Yuki Okushi <[email protected]> --------- Signed-off-by: Yuki Okushi <[email protected]> Signed-off-by: pipecd-bot <[email protected]> * Sort results of plan-preview (#5540) * Sort results of plan-preview Signed-off-by: kj455 <[email protected]> * Ensure the order of list piped Signed-off-by: kj455 <[email protected]> * fix: lint Signed-off-by: kj455 <[email protected]> * fix: move sorting to pipectl Signed-off-by: kj455 <[email protected]> * fix: add testcase Signed-off-by: kj455 <[email protected]> * fix: dev docs Signed-off-by: kj455 <[email protected]> * add docs Signed-off-by: kj455 <[email protected]> --------- Signed-off-by: kj455 <[email protected]> Signed-off-by: pipecd-bot <[email protected]> * Enhanced EventWatcher logs (#5558) * Show push error log earlier than reporting Signed-off-by: t-kikuc <[email protected]> * Use WarnLog in retry Signed-off-by: t-kikuc <[email protected]> * clarify log messages Signed-off-by: t-kikuc <[email protected]> * clarify log messages Signed-off-by: t-kikuc <[email protected]> * add TestDoCalls for asserting counts Signed-off-by: t-kikuc <[email protected]> * add eventIDs in log Signed-off-by: t-kikuc <[email protected]> * enrich logs in updateValues Signed-off-by: t-kikuc <[email protected]> * nits Signed-off-by: t-kikuc <[email protected]> * Revert "add TestDoCalls for asserting counts" This reverts commit de3f112. Signed-off-by: t-kikuc <[email protected]> --------- Signed-off-by: t-kikuc <[email protected]> Signed-off-by: pipecd-bot <[email protected]> * update RELEASE to v0.50.2 with doc update (#5571) Signed-off-by: t-kikuc <[email protected]> Signed-off-by: pipecd-bot <[email protected]> --------- Signed-off-by: Yuki Okushi <[email protected]> Signed-off-by: pipecd-bot <[email protected]> Signed-off-by: kj455 <[email protected]> Signed-off-by: t-kikuc <[email protected]> Co-authored-by: Yuki Okushi <[email protected]> Co-authored-by: Ibuki Kaji <[email protected]> Co-authored-by: Tetsuya KIKUCHI <[email protected]>
pipe-cd · Feb 17, 2025 · ef96027 · ef96027
1 parent 8ff25b1
commit ef96027
Show file tree

Hide file tree

Showing 8 changed files with 244 additions and 13 deletions.
diff --git a/RELEASE b/RELEASE
@@ -1,6 +1,6 @@
 # Generated by `make release` command.
 # DO NOT EDIT.
-tag: v0.50.1
+tag: v0.50.2
 
 releaseNoteGenerator:
   showCommitter: false

diff --git a/docs/content/en/docs-dev/user-guide/plan-preview.md b/docs/content/en/docs-dev/user-guide/plan-preview.md
@@ -38,7 +38,8 @@ pipectl plan-preview \
   --repo-remote-url={ REPO_REMOTE_GIT_SSH_URL } \
   --head-branch={ HEAD_BRANCH } \
   --head-commit={ HEAD_COMMIT } \
-  --base-branch={ BASE_BRANCH }
+  --base-branch={ BASE_BRANCH } \
+  --sort-label-keys={ SORT_LABEL_KEYS }
 ```
 
 You can run it locally or integrate it to your CI system to run automatically when a new pull request is opened/updated. Use `--help` to see more options.
@@ -47,6 +48,13 @@ You can run it locally or integrate it to your CI system to run automatically wh
 pipectl plan-preview --help
 ```
 
+### Order of the results
+
+By default, the results are sorted by PipedID and Application Name.
+
+If you want to sort the results by labels, add `--sort-label-keys` option. For example, when you run with `--sort-label-keys=env,team`, the results will be sorted by PipedID, `env` label, `team` label, and then Application Name.
+
+
 ## GitHub Actions
 
 If you are using GitHub Actions, you can seamlessly integrate our prepared [actions-plan-preview](https://github.com/pipe-cd/actions-plan-preview) to your workflows. This automatically comments the plan-preview result on the pull request when it is opened or updated. You can also trigger to run plan-preview manually by leave a comment `/pipecd plan-preview` on the pull request.
diff --git a/docs/content/en/docs-v0.50.x/user-guide/plan-preview.md b/docs/content/en/docs-v0.50.x/user-guide/plan-preview.md
@@ -38,7 +38,8 @@ pipectl plan-preview \
   --repo-remote-url={ REPO_REMOTE_GIT_SSH_URL } \
   --head-branch={ HEAD_BRANCH } \
   --head-commit={ HEAD_COMMIT } \
-  --base-branch={ BASE_BRANCH }
+  --base-branch={ BASE_BRANCH } \
+  --sort-label-keys={ SORT_LABEL_KEYS }
 ```
 
 You can run it locally or integrate it to your CI system to run automatically when a new pull request is opened/updated. Use `--help` to see more options.
@@ -47,6 +48,13 @@ You can run it locally or integrate it to your CI system to run automatically wh
 pipectl plan-preview --help
 ```
 
+### Order of the results
+
+By default, the results are sorted by PipedID and Application Name.
+
+If you want to sort the results by labels, add `--sort-label-keys` option. For example, when you run with `--sort-label-keys=env,team`, the results will be sorted by PipedID, `env` label, `team` label, and then Application Name.
+
+
 ## GitHub Actions
 
 If you are using GitHub Actions, you can seamlessly integrate our prepared [actions-plan-preview](https://github.com/pipe-cd/actions-plan-preview) to your workflows. This automatically comments the plan-preview result on the pull request when it is opened or updated. You can also trigger to run plan-preview manually by leave a comment `/pipecd plan-preview` on the pull request.
diff --git a/pkg/app/pipectl/cmd/planpreview/planpreview.go b/pkg/app/pipectl/cmd/planpreview/planpreview.go
@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"sort"
 	"strings"
 	"time"
 
@@ -49,6 +50,7 @@ type command struct {
 	timeout            time.Duration
 	pipedHandleTimeout time.Duration
 	checkInterval      time.Duration
+	sortLabelKeys      []string
 
 	clientOptions *client.Options
 }
@@ -75,6 +77,7 @@ func NewCommand() *cobra.Command {
 	cmd.Flags().StringVar(&c.out, "out", c.out, "Write planpreview result to the given path.")
 	cmd.Flags().DurationVar(&c.timeout, "timeout", c.timeout, "Maximum amount of time this command has to complete. Default is 10m.")
 	cmd.Flags().DurationVar(&c.pipedHandleTimeout, "piped-handle-timeout", c.pipedHandleTimeout, "Maximum amount of time that a Piped can take to handle. Default is 5m.")
+	cmd.Flags().StringSliceVar(&c.sortLabelKeys, "sort-label-keys", c.sortLabelKeys, "The application label keys to sort the results by. If not specified, the results will be sorted by only PipedID and ApplicationName.")
 
 	cmd.MarkFlagRequired("repo-remote-url")
 	cmd.MarkFlagRequired("head-branch")
@@ -147,11 +150,32 @@ func (c *command) run(ctx context.Context, _ cli.Input) error {
 				fmt.Printf("Failed to retrieve plan-preview results: %v\n", err)
 				return err
 			}
+			sortResults(results, c.sortLabelKeys)
 			return printResults(results, os.Stdout, c.out)
 		}
 	}
 }
 
+// sortResults sorts the given results by pipedID and the given sortLabelKeys.
+// If sortLabelKeys is not specified or the all values of sortLabelKeys are the same, it sorts by pipedID and ApplicationName.
+func sortResults(allResults []*model.PlanPreviewCommandResult, sortLabelKeys []string) {
+	sort.SliceStable(allResults, func(i, j int) bool {
+		return allResults[i].PipedId < allResults[j].PipedId
+	})
+	for _, resultsPerPiped := range allResults {
+		results := resultsPerPiped.Results
+		sort.SliceStable(results, func(i, j int) bool {
+			a, b := results[i], results[j]
+			for _, key := range sortLabelKeys {
+				if a.Labels[key] != b.Labels[key] {
+					return a.Labels[key] < b.Labels[key]
+				}
+			}
+			return a.ApplicationName < b.ApplicationName
+		})
+	}
+}
+
 func printResults(results []*model.PlanPreviewCommandResult, stdout io.Writer, outFile string) error {
 	r := convert(results)
 

diff --git a/pkg/app/pipectl/cmd/planpreview/planpreview_test.go b/pkg/app/pipectl/cmd/planpreview/planpreview_test.go
@@ -244,3 +244,123 @@ NOTE: An error occurred while building plan-preview for applications of the foll
 		})
 	}
 }
+func TestSortResults(t *testing.T) {
+	t.Parallel()
+	testcases := []struct {
+		name          string
+		results       []*model.PlanPreviewCommandResult
+		sortLabelKeys []string
+		expected      []*model.PlanPreviewCommandResult
+	}{
+		{
+			name: "sort by pipedID and application name",
+			results: []*model.PlanPreviewCommandResult{
+				{
+					PipedId: "piped-2",
+					Results: []*model.ApplicationPlanPreviewResult{
+						{ApplicationName: "app-2"},
+						{ApplicationName: "app-1"},
+					},
+				},
+				{
+					PipedId: "piped-1",
+					Results: []*model.ApplicationPlanPreviewResult{
+						{ApplicationName: "app-2"},
+						{ApplicationName: "app-1"},
+					},
+				},
+			},
+			sortLabelKeys: []string{},
+			expected: []*model.PlanPreviewCommandResult{
+				{
+					PipedId: "piped-1",
+					Results: []*model.ApplicationPlanPreviewResult{
+						{ApplicationName: "app-1"},
+						{ApplicationName: "app-2"},
+					},
+				},
+				{
+					PipedId: "piped-2",
+					Results: []*model.ApplicationPlanPreviewResult{
+						{ApplicationName: "app-1"},
+						{ApplicationName: "app-2"},
+					},
+				},
+			},
+		},
+		{
+			name: "sort by label keys",
+			results: []*model.PlanPreviewCommandResult{
+				{
+					PipedId: "piped-1",
+					Results: []*model.ApplicationPlanPreviewResult{
+						{ApplicationName: "app-1", Labels: map[string]string{"env": "staging"}},
+						{ApplicationName: "app-1", Labels: map[string]string{"env": "prod"}},
+					},
+				},
+				{
+					PipedId: "piped-2",
+					Results: []*model.ApplicationPlanPreviewResult{
+						{ApplicationName: "app-3", Labels: map[string]string{"env": "staging"}},
+						{ApplicationName: "app-3", Labels: map[string]string{"env": "prod"}},
+						{ApplicationName: "app-2", Labels: map[string]string{"env": "staging"}},
+						{ApplicationName: "app-2", Labels: map[string]string{"env": "prod"}},
+					},
+				},
+			},
+			sortLabelKeys: []string{"env"},
+			expected: []*model.PlanPreviewCommandResult{
+				{
+					PipedId: "piped-1",
+					Results: []*model.ApplicationPlanPreviewResult{
+						{ApplicationName: "app-1", Labels: map[string]string{"env": "prod"}},
+						{ApplicationName: "app-1", Labels: map[string]string{"env": "staging"}},
+					},
+				},
+				{
+					PipedId: "piped-2",
+					Results: []*model.ApplicationPlanPreviewResult{
+						{ApplicationName: "app-2", Labels: map[string]string{"env": "prod"}},
+						{ApplicationName: "app-3", Labels: map[string]string{"env": "prod"}},
+						{ApplicationName: "app-2", Labels: map[string]string{"env": "staging"}},
+						{ApplicationName: "app-3", Labels: map[string]string{"env": "staging"}},
+					},
+				},
+			},
+		},
+		{
+			name: "sort by multiple label keys",
+			results: []*model.PlanPreviewCommandResult{
+				{
+					PipedId: "piped-1",
+					Results: []*model.ApplicationPlanPreviewResult{
+						{ApplicationName: "app-1", Labels: map[string]string{"env": "prod", "team": "team-2"}},
+						{ApplicationName: "app-1", Labels: map[string]string{"env": "staging", "team": "team-1"}},
+						{ApplicationName: "app-1", Labels: map[string]string{"env": "prod", "team": "team-1"}},
+						{ApplicationName: "app-2", Labels: map[string]string{"env": "prod", "team": "team-2"}},
+					},
+				},
+			},
+			sortLabelKeys: []string{"env", "team"},
+			expected: []*model.PlanPreviewCommandResult{
+				{
+					PipedId: "piped-1",
+					Results: []*model.ApplicationPlanPreviewResult{
+						{ApplicationName: "app-1", Labels: map[string]string{"env": "prod", "team": "team-1"}},
+						{ApplicationName: "app-1", Labels: map[string]string{"env": "prod", "team": "team-2"}},
+						{ApplicationName: "app-2", Labels: map[string]string{"env": "prod", "team": "team-2"}},
+						{ApplicationName: "app-1", Labels: map[string]string{"env": "staging", "team": "team-1"}},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			sortResults(tc.results, tc.sortLabelKeys)
+			assert.Equal(t, tc.expected, tc.results)
+		})
+	}
+}
diff --git a/pkg/app/piped/eventwatcher/eventwatcher.go b/pkg/app/piped/eventwatcher/eventwatcher.go
@@ -183,7 +183,7 @@ func (w *watcher) run(ctx context.Context, repo git.Repo, repoCfg config.PipedRe
 		case <-ticker.C:
 			err := repo.Pull(ctx, repo.GetClonedBranch())
 			if err != nil {
-				w.logger.Error("failed to perform git pull",
+				w.logger.Error("failed to perform git pull. will retry in the next loop",
 					zap.String("repo-id", repoCfg.RepoID),
 					zap.String("branch", repo.GetClonedBranch()),
 					zap.Error(err),
@@ -233,6 +233,7 @@ func (w *watcher) run(ctx context.Context, repo git.Repo, repoCfg config.PipedRe
 				if err := w.updateValues(ctx, repo, repoCfg.RepoID, cfg.Events, commitMsg); err != nil {
 					w.logger.Error("failed to update the values",
 						zap.String("repo-id", repoCfg.RepoID),
+						zap.String("branch", repo.GetClonedBranch()),
 						zap.Error(err),
 					)
 				}
@@ -294,6 +295,7 @@ func (w *watcher) run(ctx context.Context, repo git.Repo, repoCfg config.PipedRe
 			if err := w.execute(ctx, repo, repoCfg.RepoID, cfgs); err != nil {
 				w.logger.Error("failed to execute the event from application configuration",
 					zap.String("repo-id", repoCfg.RepoID),
+					zap.String("branch", repo.GetClonedBranch()),
 					zap.Error(err),
 				)
 			}
@@ -456,28 +458,40 @@ func (w *watcher) execute(ctx context.Context, repo git.Repo, repoID string, eve
 	var responseError error
 	retry := backoff.NewRetry(retryPushNum, backoff.NewConstant(retryPushInterval))
 	for branch, events := range branchHandledEvents {
+		eventIDs := make([]string, 0, len(events))
+		for _, e := range events {
+			eventIDs = append(eventIDs, e.Id)
+		}
+		zlogger := w.logger.With(
+			zap.String("repo-id", repoID),
+			zap.String("branch", tmpRepo.GetClonedBranch()),
+			zap.Strings("event-ids", eventIDs),
+		)
+
 		_, err = retry.Do(ctx, func() (interface{}, error) {
 			if err := tmpRepo.Push(ctx, branch); err != nil {
-				w.logger.Error("failed to push commits", zap.String("repo-id", repoID), zap.String("branch", branch), zap.Error(err))
+				zlogger.Warn(fmt.Sprintf("failed to push commits. retry attempt %d/%d", retry.Calls(), retryPushNum), zap.Error(err))
 				return nil, err
 			}
 			return nil, nil
 		})
 
 		if err == nil {
 			if _, err := w.apiClient.ReportEventStatuses(ctx, &pipedservice.ReportEventStatusesRequest{Events: events}); err != nil {
-				w.logger.Error("failed to report event statuses", zap.Error(err))
+				zlogger.Error("failed to report event statuses", zap.Error(err))
 			}
 			w.executionMilestoneMap.Store(repoID, maxTimestamp)
 			continue
 		}
 
 		// If push fails because the local branch was not fresh, exit to retry again in the next interval.
 		if err == git.ErrBranchNotFresh {
-			w.logger.Warn("failed to push commits", zap.Error(err))
+			zlogger.Warn("failed to push commits. local branch was not up-to-date. will retry in the next loop", zap.Error(err))
 			continue
 		}
 
+		zlogger.Error("failed to push commits", zap.Error(err))
+
 		// If push fails because of the other reason, re-set all statuses to FAILURE.
 		for i := range events {
 			if events[i].Status == model.EventStatus_EVENT_FAILURE {
@@ -487,7 +501,7 @@ func (w *watcher) execute(ctx context.Context, repo git.Repo, repoID string, eve
 			events[i].StatusDescription = fmt.Sprintf("Failed to push changed files: %v", err)
 		}
 		if _, err := w.apiClient.ReportEventStatuses(ctx, &pipedservice.ReportEventStatusesRequest{Events: events}); err != nil {
-			w.logger.Error("failed to report event statuses", zap.Error(err))
+			zlogger.Error("failed to report event statuses", zap.Error(err))
 		}
 		w.executionMilestoneMap.Store(repoID, maxTimestamp)
 		responseError = errors.Join(responseError, err)
@@ -600,17 +614,27 @@ func (w *watcher) updateValues(ctx context.Context, repo git.Repo, repoID string
 		return nil
 	}
 
+	eventIDs := make([]string, 0, len(handledEvents))
+	for _, e := range handledEvents {
+		eventIDs = append(eventIDs, e.Id)
+	}
+	zlogger := w.logger.With(
+		zap.String("repo-id", repoID),
+		zap.String("branch", tmpRepo.GetClonedBranch()),
+		zap.Strings("event-ids", eventIDs),
+	)
+
 	retry := backoff.NewRetry(retryPushNum, backoff.NewConstant(retryPushInterval))
 	_, err = retry.Do(ctx, func() (interface{}, error) {
 		if err := tmpRepo.Push(ctx, tmpRepo.GetClonedBranch()); err != nil {
-			w.logger.Error("failed to push commits", zap.String("repo-id", repoID), zap.String("branch", tmpRepo.GetClonedBranch()), zap.Error(err))
+			zlogger.Warn(fmt.Sprintf("failed to push commits. retry attempt %d/%d", retry.Calls(), retryPushNum), zap.Error(err))
 			return nil, err
 		}
 		return nil, nil
 	})
 	if err == nil {
 		if _, err := w.apiClient.ReportEventStatuses(ctx, &pipedservice.ReportEventStatusesRequest{Events: handledEvents}); err != nil {
-			w.logger.Error("failed to report event statuses", zap.Error(err))
+			zlogger.Error("failed to report event statuses", zap.Error(err))
 			return err
 		}
 		w.milestoneMap.Store(repoID, maxTimestamp)
@@ -619,10 +643,12 @@ func (w *watcher) updateValues(ctx context.Context, repo git.Repo, repoID string
 
 	// If push fails because the local branch was not fresh, exit to retry again in the next interval.
 	if err == git.ErrBranchNotFresh {
-		w.logger.Warn("failed to push commits", zap.Error(err))
+		zlogger.Warn("failed to push commits. local branch was not up-to-date. will retry in the next loop", zap.Error(err))
 		return nil
 	}
 
+	zlogger.Error("failed to push commits", zap.Error(err))
+
 	// If push fails because of the other reason, re-set all statuses to FAILURE.
 	for i := range handledEvents {
 		if handledEvents[i].Status == model.EventStatus_EVENT_FAILURE {
@@ -632,11 +658,10 @@ func (w *watcher) updateValues(ctx context.Context, repo git.Repo, repoID string
 		handledEvents[i].StatusDescription = fmt.Sprintf("Failed to push changed files: %v", err)
 	}
 	if _, err := w.apiClient.ReportEventStatuses(ctx, &pipedservice.ReportEventStatusesRequest{Events: handledEvents}); err != nil {
-		w.logger.Error("failed to report event statuses: %w", zap.Error(err))
+		zlogger.Error("failed to report event statuses: %w", zap.Error(err))
 		return err
 	}
 	w.milestoneMap.Store(repoID, maxTimestamp)
-	w.logger.Error("failed to push commits", zap.Error(err))
 	return err
 }