(fix) unsuccessful plan and apply shouldn't report successful status (#292)

* fix unsuccesfull plans and applies reporting good status

Author: Spartakovic

pkg/digger/digger.go

@@ -104,25 +104,23 @@ func RunCommandsPerProject(commandsPerProject []ProjectCommand, repoOwner string
 			case "digger plan":
 				utils.SendUsageRecord(repoOwner, eventName, "plan")
 				ciService.SetStatus(prNumber, "pending", projectCommands.ProjectName+"/plan")
-				err := diggerExecutor.Plan(prNumber)
+				planPerformed, err := diggerExecutor.Plan(prNumber)
 				if err != nil {
 					log.Printf("Failed to run digger plan command. %v", err)
 					ciService.SetStatus(prNumber, "failure", projectCommands.ProjectName+"/plan")
-
 					return false, fmt.Errorf("failed to run digger plan command. %v", err)
-				} else {
+				} else if planPerformed {
 					ciService.SetStatus(prNumber, "success", projectCommands.ProjectName+"/plan")
 				}
 			case "digger apply":
 				utils.SendUsageRecord(repoName, eventName, "apply")
 				ciService.SetStatus(prNumber, "pending", projectCommands.ProjectName+"/apply")
-				err := diggerExecutor.Apply(prNumber)
+				applyPerformed, err := diggerExecutor.Apply(prNumber)
 				if err != nil {
 					log.Printf("Failed to run digger apply command. %v", err)
 					ciService.SetStatus(prNumber, "failure", projectCommands.ProjectName+"/apply")
-
 					return false, fmt.Errorf("failed to run digger apply command. %v", err)
-				} else {
+				} else if applyPerformed {
 					ciService.SetStatus(prNumber, "success", projectCommands.ProjectName+"/apply")
 					appliesPerProject[projectCommands.ProjectName] = true
 				}
@@ -430,13 +428,13 @@ func (d DiggerExecutor) storedPlanFilePath() string {
 	return path.Join(d.RepoOwner, d.planFileName())
 }
 
-func (d DiggerExecutor) Plan(prNumber int) error {
-	res, err := d.ProjectLock.Lock(prNumber)
+func (d DiggerExecutor) Plan(prNumber int) (bool, error) {
+	locked, err := d.ProjectLock.Lock(prNumber)
 	if err != nil {
-		return fmt.Errorf("error locking project: %v", err)
+		return false, fmt.Errorf("error locking project: %v", err)
 	}
-	log.Printf("Lock result: %t\n", res)
-	if res {
+	log.Printf("Lock result: %t\n", locked)
+	if locked {
 		var planSteps []configuration.Step
 
 		if d.PlanStage != nil {
@@ -455,32 +453,32 @@ func (d DiggerExecutor) Plan(prNumber int) error {
 			if step.Action == "init" {
 				_, _, err := d.TerraformExecutor.Init(step.ExtraArgs, d.StateEnvVars)
 				if err != nil {
-					return fmt.Errorf("error running init: %v", err)
+					return false, fmt.Errorf("error running init: %v", err)
 				}
 			}
 			if step.Action == "plan" {
 				planArgs := []string{"-out", d.planFileName()}
 				planArgs = append(planArgs, step.ExtraArgs...)
 				isNonEmptyPlan, stdout, stderr, err := d.TerraformExecutor.Plan(planArgs, d.CommandEnvVars)
 				if err != nil {
-					return fmt.Errorf("error executing plan: %v", err)
+					return false, fmt.Errorf("error executing plan: %v", err)
 				}
 				if d.PlanStorage != nil {
 					planExists, err := d.PlanStorage.PlanExists(d.storedPlanFilePath())
 					if err != nil {
-						return fmt.Errorf("error checking if plan exists: %v", err)
+						return false, fmt.Errorf("error checking if plan exists: %v", err)
 					}
 
 					if planExists {
 						err = d.PlanStorage.DeleteStoredPlan(d.storedPlanFilePath())
 						if err != nil {
-							return fmt.Errorf("error deleting plan: %v", err)
+							return false, fmt.Errorf("error deleting plan: %v", err)
 						}
 					}
 
 					err = d.PlanStorage.StorePlan(d.localPlanFilePath(), d.storedPlanFilePath())
 					if err != nil {
-						return fmt.Errorf("error storing plan: %v", err)
+						return false, fmt.Errorf("error storing plan: %v", err)
 					}
 				}
 				plan := cleanupTerraformPlan(isNonEmptyPlan, err, stdout, stderr)
@@ -496,34 +494,41 @@ func (d DiggerExecutor) Plan(prNumber int) error {
 				log.Printf("Running %v for **%v**\n", step.Value, d.ProjectLock.LockId())
 				_, _, err := d.CommandRunner.Run(d.ProjectPath, step.Shell, commands)
 				if err != nil {
-					return fmt.Errorf("error running command: %v", err)
+					return false, fmt.Errorf("error running command: %v", err)
 				}
 			}
 		}
+		return true, nil
 	}
-	return nil
+	return false, nil
 }
 
-func (d DiggerExecutor) Apply(prNumber int) error {
+func (d DiggerExecutor) Apply(prNumber int) (bool, error) {
 	var plansFilename *string
 	if d.PlanStorage != nil {
 		var err error
 		plansFilename, err = d.PlanStorage.RetrievePlan(d.localPlanFilePath(), d.storedPlanFilePath())
 		if err != nil {
-			return fmt.Errorf("error retrieving plan: %v", err)
+			return false, fmt.Errorf("error retrieving plan: %v", err)
 		}
 	}
 
 	isMergeable, err := d.CIService.IsMergeable(prNumber)
 	if err != nil {
-		return fmt.Errorf("error validating is PR is mergeable: %v", err)
+		return false, fmt.Errorf("error validating is PR is mergeable: %v", err)
 	}
 	if !isMergeable {
 		comment := "Cannot perform Apply since the PR is not currently mergeable."
 		d.CIService.PublishComment(prNumber, comment)
+		return false, nil
 	} else {
+		locked, err := d.ProjectLock.Lock(prNumber)
+
+		if err != nil {
+			return false, fmt.Errorf("error locking project: %v", err)
+		}
 
-		if res, _ := d.ProjectLock.Lock(prNumber); res {
+		if locked {
 			var applySteps []configuration.Step
 
 			if d.ApplyStage != nil {
@@ -543,7 +548,7 @@ func (d DiggerExecutor) Apply(prNumber int) error {
 				if step.Action == "init" {
 					_, _, err := d.TerraformExecutor.Init(step.ExtraArgs, d.StateEnvVars)
 					if err != nil {
-						return fmt.Errorf("error running init: %v", err)
+						return false, fmt.Errorf("error running init: %v", err)
 					}
 				}
 				if step.Action == "apply" {
@@ -553,7 +558,7 @@ func (d DiggerExecutor) Apply(prNumber int) error {
 					d.CIService.PublishComment(prNumber, comment)
 					if err != nil {
 						d.CIService.PublishComment(prNumber, "Error during applying.")
-						return fmt.Errorf("error executing apply: %v", err)
+						return false, fmt.Errorf("error executing apply: %v", err)
 					}
 				}
 				if step.Action == "run" {
@@ -565,13 +570,15 @@ func (d DiggerExecutor) Apply(prNumber int) error {
 					log.Printf("Running %v for **%v**\n", step.Value, d.ProjectLock.LockId())
 					_, _, err := d.CommandRunner.Run(d.ProjectPath, step.Shell, commands)
 					if err != nil {
-						return fmt.Errorf("error running command: %v", err)
+						return false, fmt.Errorf("error running command: %v", err)
 					}
 				}
 			}
+			return true, nil
+		} else {
+			return false, nil
 		}
 	}
-	return nil
 }
 
 func (d DiggerExecutor) Unlock(prNumber int) error {

pkg/gitlab/gitlab.go

@@ -370,24 +370,27 @@ func RunCommandsPerProject(commandsPerProject []digger.ProjectCommand, gitLabCon
 			case "digger plan":
 				utils.SendUsageRecord(repoOwner, eventName, "plan")
 				service.SetStatus(prNumber, "pending", projectCommands.ProjectName+"/plan")
-				err := diggerExecutor.Plan(prNumber)
+				planPerformed, err := diggerExecutor.Plan(prNumber)
 				if err != nil {
 					log.Printf("Failed to run digger plan command. %v", err)
 					service.SetStatus(prNumber, "failure", projectCommands.ProjectName+"/plan")
 
 					return false, fmt.Errorf("failed to run digger plan command. %v", err)
+				} else if !planPerformed {
+					service.SetStatus(prNumber, "pending", projectCommands.ProjectName+"/plan")
 				} else {
 					service.SetStatus(prNumber, "success", projectCommands.ProjectName+"/plan")
 				}
 			case "digger apply":
 				utils.SendUsageRecord(repoName, eventName, "apply")
 				service.SetStatus(prNumber, "pending", projectCommands.ProjectName+"/apply")
-				err := diggerExecutor.Apply(prNumber)
+				applyPerformed, err := diggerExecutor.Apply(prNumber)
 				if err != nil {
 					log.Printf("Failed to run digger apply command. %v", err)
 					service.SetStatus(prNumber, "failure", projectCommands.ProjectName+"/apply")
-
 					return false, fmt.Errorf("failed to run digger apply command. %v", err)
+				} else if !applyPerformed {
+					service.SetStatus(prNumber, "pending", projectCommands.ProjectName+"/apply")
 				} else {
 					service.SetStatus(prNumber, "success", projectCommands.ProjectName+"/apply")
 					appliesPerProject[projectCommands.ProjectName] = true