fix: execute actions once all pipelines have been executed (#2040)

* fix: execute actions once all pipelines have been executed

* execute actions once all pipelines have been executed
* split long function into multiple small ones

Signed-off-by: Olblak <me@olblak.com>

* chore: change github pullrequest log level message

Signed-off-by: Olblak <me@olblak.com>

* fix: Don't run similar action multiple times

Signed-off-by: Olblak <me@olblak.com>

---------

Signed-off-by: Olblak <me@olblak.com>

pkg/core/engine/actions.go

@@ -0,0 +1,57 @@
+package engine
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/sirupsen/logrus"
+	"github.com/updatecli/updatecli/pkg/core/pipeline/action"
+	"github.com/updatecli/updatecli/pkg/core/result"
+)
+
+// RunActions runs all actions defined in the configuration.
+// To avoid the situation where a pipeline close a pullrequest even thought the next pipeline need
+// to update the same pullrequest, we need to run pipeline actions once all pipelines' targets have been executed.
+// The goal is to respect an order where we first handle pipelines in ATTENTION state, then FAILURE, then SUCCESS and finally SKIPPED.
+// cfr https://github.com/updatecli/updatecli/issues/2039
+
+// 1. ATTENTION: to update existing pull request
+// 2. FAILURE: which may clean up existing pull request
+// 3. SUCCESS: which may clean up existing pull request
+// 4. SKIPPED: which may clean up existing pull request
+//
+// It worth reminding that a pipeline can contain multiple actions.
+// If at least one action is in an attention state,
+// then the pipeline is considered in attention state as well.
+// So the same logic must be applied differently based on the different actions state.
+func (e *Engine) runActions() error {
+
+	errs := []string{}
+
+	logrus.Infof("\n\n%s\n", strings.ToTitle("Actions"))
+	logrus.Infof("%s\n\n", strings.Repeat("=", len("Actions")+1))
+
+	// actionsHashTable is used to avoid running the same action multiple times.
+	actionsHashTable := make(map[uint64]*action.Action)
+
+	for _, pipelineState := range []string{result.ATTENTION, result.FAILURE, result.SUCCESS, result.SKIPPED} {
+		for id := range e.Pipelines {
+			pipeline := e.Pipelines[id]
+			if len(pipeline.Actions) > 0 {
+				if err := pipeline.RunActions(pipelineState, actionsHashTable); err != nil {
+					errs = append(errs, err.Error())
+					pipeline.Report.Result = result.FAILURE
+					logrus.Errorf("action stage:\t%q", err.Error())
+					continue
+				}
+			}
+		}
+	}
+	if len(errs) > 0 {
+		return fmt.Errorf(
+			"errors occurred while running actions:\n\t* %s",
+			strings.Join(errs, "\n\t* "))
+	}
+
+	return nil
+}

pkg/core/engine/reports.go

@@ -0,0 +1,37 @@
+package engine
+
+import (
+	"fmt"
+
+	"github.com/sirupsen/logrus"
+)
+
+// showReports display the reports
+// and return an error if at least one pipeline failed
+func (e *Engine) showReports() error {
+	e.Reports.Sort()
+
+	err := e.Reports.Show()
+	if err != nil {
+		return err
+	}
+	totalSuccessPipeline, totalChangedAppliedPipeline, totalFailedPipeline, totalSkippedPipeline := e.Reports.Summary()
+
+	totalPipeline := totalSuccessPipeline + totalChangedAppliedPipeline + totalFailedPipeline + totalSkippedPipeline
+
+	logrus.Infof("Run Summary")
+	logrus.Infof("===========\n")
+	logrus.Infof("Pipeline(s) run:")
+	logrus.Infof("  * Changed:\t%d", totalChangedAppliedPipeline)
+	logrus.Infof("  * Failed:\t%d", totalFailedPipeline)
+	logrus.Infof("  * Skipped:\t%d", totalSkippedPipeline)
+	logrus.Infof("  * Succeeded:\t%d", totalSuccessPipeline)
+	logrus.Infof("  * Total:\t%d", totalPipeline)
+
+	// Exit on error if at least one pipeline failed
+	if totalFailedPipeline > 0 {
+		return fmt.Errorf("%d over %d pipeline failed", totalFailedPipeline, totalPipeline)
+	}
+
+	return nil
+}

pkg/core/engine/run.go

@@ -1,12 +1,10 @@
 package engine
 
 import (
-	"fmt"
-
 	"github.com/sirupsen/logrus"
 )
 
-// Run runs the full process for one manifest
+// Run runs the full process
 func (e *Engine) Run() (err error) {
 
 	PrintTitle("Pipeline")
@@ -24,29 +22,17 @@ func (e *Engine) Run() (err error) {
 		}
 	}
 
-	e.Reports.Sort()
+	if err = e.runActions(); err != nil {
+		logrus.Errorf("running actions:\n%s", err)
+	}
 
-	err = e.Reports.Show()
-	if err != nil {
-		return err
+	if err = e.publishToUdash(); err != nil {
+		logrus.Errorf("publishing to Udash:\n%s", err)
 	}
-	totalSuccessPipeline, totalChangedAppliedPipeline, totalFailedPipeline, totalSkippedPipeline := e.Reports.Summary()
-
-	totalPipeline := totalSuccessPipeline + totalChangedAppliedPipeline + totalFailedPipeline + totalSkippedPipeline
-
-	logrus.Infof("Run Summary")
-	logrus.Infof("===========\n")
-	logrus.Infof("Pipeline(s) run:")
-	logrus.Infof("  * Changed:\t%d", totalChangedAppliedPipeline)
-	logrus.Infof("  * Failed:\t%d", totalFailedPipeline)
-	logrus.Infof("  * Skipped:\t%d", totalSkippedPipeline)
-	logrus.Infof("  * Succeeded:\t%d", totalSuccessPipeline)
-	logrus.Infof("  * Total:\t%d", totalPipeline)
-
-	// Exit on error if at least one pipeline failed
-	if totalFailedPipeline > 0 {
-		return fmt.Errorf("%d over %d pipeline failed", totalFailedPipeline, totalPipeline)
+
+	if err = e.showReports(); err != nil {
+		return err
 	}
 
-	return err
+	return nil
 }

pkg/core/engine/udash.go

@@ -0,0 +1,41 @@
+package engine
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+
+	"github.com/updatecli/updatecli/pkg/core/cmdoptions"
+	"github.com/updatecli/updatecli/pkg/core/udash"
+)
+
+// publishToUdash publish pipeline reports to the Udash service.
+// This service is still experimental and should be used with caution.
+// More information on https://github.com/updatecli/udash
+func (e *Engine) publishToUdash() error {
+
+	errs := []string{}
+
+	if !cmdoptions.Experimental {
+		return nil
+	}
+
+	for id := range e.Pipelines {
+		pipeline := e.Pipelines[id]
+		if err := udash.Publish(&pipeline.Report); err != nil &&
+			!errors.Is(err, udash.ErrNoUdashBearerToken) &&
+			!errors.Is(err, udash.ErrNoUdashAPIURL) {
+			errs = append(errs, pipeline.Name+err.Error())
+		}
+		e.Pipelines[id] = pipeline
+	}
+
+	if len(errs) > 0 {
+		return fmt.Errorf(
+			"errors occurred while publishing to Udash:\n\t* %s",
+			strings.Join(errs, "\n\t* "),
+		)
+	}
+
+	return nil
+}

pkg/core/pipeline/actions.go

@@ -5,36 +5,56 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/mitchellh/hashstructure"
 	"github.com/sirupsen/logrus"
 	"github.com/updatecli/updatecli/pkg/core/pipeline/action"
 	"github.com/updatecli/updatecli/pkg/core/reports"
 	"github.com/updatecli/updatecli/pkg/core/result"
 )
 
-func (p *Pipeline) RunActions() error {
+// RunActions runs all actions defined in the configuration.
+// pipelineState is used to skip actions that are not related to the current pipeline state.
+func (p *Pipeline) RunActions(pipelineState string, actionHashTable map[uint64]*action.Action) error {
 
-	if len(p.Targets) == 0 {
-		logrus.Debugln("no target found, skipping action")
+	// Early return
+	if len(p.Targets) == 0 || len(p.Actions) == 0 {
 		return nil
 	}
 
-	if len(p.Actions) == 0 {
-		logrus.Debugln("no action found, skipping")
-		return nil
-	}
+	for id := range p.Actions {
 
-	if len(p.Actions) > 0 {
-		logrus.Infof("\n\n%s\n", strings.ToTitle("Actions"))
-		logrus.Infof("%s\n\n", strings.Repeat("=", len("Actions")+1))
-	}
+		action := p.Actions[id]
+
+		// Hash the action configuration to avoid running the same action multiple times.
+		// We use the hash of the action configuration. It worth mentioning because of:
+		// https://github.com/updatecli/updatecli/pull/1395
+		// it's difficult today to generate an unique hash for each action.
+		// so we may have situation where different actions doing the same thing
+		// will be applied multiple times.
+		actionHash, err := hashstructure.Hash(action.Config, nil)
+		if err != nil {
+			return err
+		}
+
+		// Skip action if already executed
+		if _, ok := actionHashTable[actionHash]; ok {
+			continue
+		}
+
+		actionHashTable[actionHash] = &action
 
-	for id, action := range p.Actions {
 		relatedTargets, err := p.SearchAssociatedTargetsID(id)
 		if err != nil {
 			logrus.Errorf(err.Error())
 			continue
 		}
 
+		for i := range relatedTargets {
+			if p.Targets[relatedTargets[i]].Result.Result != pipelineState {
+				continue
+			}
+		}
+
 		// Update pipeline before each condition run
 		err = p.Update()
 		if err != nil {
@@ -99,6 +119,9 @@ func (p *Pipeline) RunActions() error {
 			action.Report.Targets = append(action.Report.Targets, actionTarget)
 		}
 
+		logrus.Infof("\n%s - %s", p.Name, id)
+		logrus.Infof("%s\n\n", strings.Repeat("-", len(p.Name)+len(id)+3))
+
 		// No need to execute the action if no target require attention
 		if len(action.Report.Targets) == 0 {
 			if !p.Options.Target.DryRun {
@@ -108,6 +131,7 @@ func (p *Pipeline) RunActions() error {
 					return err
 				}
 			}
+			logrus.Infof("No additional step needed\n")
 			continue
 		}
 

pkg/core/pipeline/main.go

@@ -1,12 +1,10 @@
 package pipeline
 
 import (
-	"errors"
 	"fmt"
 	"strings"
 
 	"github.com/sirupsen/logrus"
-	"github.com/updatecli/updatecli/pkg/core/cmdoptions"
 	"github.com/updatecli/updatecli/pkg/core/config"
 	"github.com/updatecli/updatecli/pkg/core/pipeline/action"
 	"github.com/updatecli/updatecli/pkg/core/pipeline/condition"
@@ -15,7 +13,6 @@ import (
 	"github.com/updatecli/updatecli/pkg/core/pipeline/target"
 	"github.com/updatecli/updatecli/pkg/core/reports"
 	"github.com/updatecli/updatecli/pkg/core/result"
-	"github.com/updatecli/updatecli/pkg/core/udash"
 )
 
 // Pipeline represent an updatecli run for a specific configuration
@@ -221,22 +218,6 @@ func (p *Pipeline) Run() error {
 		}
 	}
 
-	if len(p.Actions) > 0 {
-		if err := p.RunActions(); err != nil {
-			p.Report.Result = result.FAILURE
-			return fmt.Errorf("action stage:\t%q", err.Error())
-		}
-	}
-
-	if cmdoptions.Experimental {
-		if err := udash.Publish(&p.Report); err != nil &&
-			!errors.Is(err, udash.ErrNoUdashBearerToken) &&
-			!errors.Is(err, udash.ErrNoUdashAPIURL) {
-			logrus.Infof("Skipping report publishing")
-			logrus.Debugf("publish report: %s", err)
-		}
-	}
-
 	return nil
 
 }

pkg/plugins/scms/github/pullrequest.go

@@ -573,7 +573,7 @@ func (p *PullRequest) getRemotePullRequest(resetBody bool) error {
 		logrus.Debugf("Resetting pull-request body with new report")
 	}
 
-	logrus.Debugf("Existing pull-request found: %s", p.remotePullRequest.Url)
+	logrus.Infof("Existing GitHub pull request found: %s", p.remotePullRequest.Url)
 
 	return nil
 }