fix(layer): handle terraform errors in runner (#149)

* fix(layer): check plan sum in IsPlanArtifactUpToDate

* fix(layer): change condition reason when plan failed

* fix(layer): change condition IsApplyUpToDate when plan failed

* fix(runner): do not put an empty plan checksum

* fix(runner): put exit code to 1 on error

* fix(runner): try to not use os.exit

* fix(runner): make Exec func return with err

* fix(runner): new Exec function with better error handling

* revert: fix(runner): new Exec function with better error handling

This reverts commit d17f524.

* fix(runner): propagate error through cobra

* fix(runner): error propagation

* fix(mispell): some mispell in error messages

* test(layer): adapt fail plan test case to new condition

Author: corrieriluca

cmd/runner/start.go

@@ -12,9 +12,10 @@ func buildRunnerStartCmd(app *burrito.App) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "start",
 		Short: "Start Burrito runner",
+		// Do not display usage on program error
+		SilenceUsage: true,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			app.StartRunner()
-			return nil
+			return app.StartRunner()
 		},
 	}
 

internal/burrito/burrito.go

@@ -26,7 +26,7 @@ type Server interface {
 }
 
 type Runner interface {
-	Exec()
+	Exec() error
 }
 
 type Controllers interface {

internal/burrito/runner.go

@@ -1,5 +1,5 @@
 package burrito
 
-func (app *App) StartRunner() {
-	app.Runner.Exec()
+func (app *App) StartRunner() error {
+	return app.Runner.Exec()
 }

internal/controllers/terraformlayer/conditions.go

@@ -26,6 +26,13 @@ func (r *Reconciler) IsPlanArtifactUpToDate(t *configv1alpha1.TerraformLayer) (m
 		condition.Status = metav1.ConditionFalse
 		return condition, false
 	}
+	planHash, ok := t.Annotations[annotations.LastPlanSum]
+	if !ok || planHash == "" {
+		condition.Reason = "LastPlanFailed"
+		condition.Message = "Last plan run has failed"
+		condition.Status = metav1.ConditionFalse
+		return condition, false
+	}
 	lastPlanDate, err := time.Parse(time.UnixDate, value)
 	if err != nil {
 		condition.Reason = "ParseError"
@@ -95,16 +102,16 @@ func (r *Reconciler) IsApplyUpToDate(t *configv1alpha1.TerraformLayer) (metav1.C
 		LastTransitionTime: metav1.NewTime(time.Now()),
 	}
 	planHash, ok := t.Annotations[annotations.LastPlanSum]
-	if !ok {
+	if !ok || planHash == "" {
 		condition.Reason = "NoPlanHasRunYet"
 		condition.Message = "No plan has run on this layer yet"
 		condition.Status = metav1.ConditionTrue
 		return condition, true
 	}
 	applyHash, ok := t.Annotations[annotations.LastApplySum]
 	if !ok {
-		condition.Reason = "NoApplyHasRan"
-		condition.Message = "Apply has not ran yet but a plan is available, launching apply"
+		condition.Reason = "NoApplyHasRun"
+		condition.Message = "Apply has not run yet but a plan is available, launching apply"
 		condition.Status = metav1.ConditionFalse
 		return condition, false
 	}

internal/controllers/terraformlayer/controller_test.go

@@ -367,6 +367,10 @@ var _ = Describe("Layer", func() {
 		It("should end in PlanNeeded state", func() {
 			Expect(layer.Status.State).To(Equal("PlanNeeded"))
 		})
+		It("should have the condition IsPlanArtifactUpToDate set to false", func() {
+			Expect(layer.Status.Conditions[0].Reason).To(Equal("LastPlanFailed"))
+			Expect(string(layer.Status.Conditions[0].Status)).To(Equal("False"))
+		})
 		It("should set RequeueAfter to WaitAction", func() {
 			Expect(result.RequeueAfter).To(Equal(reconciler.Config.Controller.Timers.WaitAction))
 		})

internal/controllers/terraformpullrequest/states.go

@@ -111,7 +111,7 @@ func (s *CommentNeeded) getHandler() func(ctx context.Context, r *Reconciler, re
 		comment := comment.NewDefaultComment(layers, r.Storage)
 		err = provider.Comment(repository, pr, comment)
 		if err != nil {
-			log.Errorf("an error occured while commenting pull request: %s", err)
+			log.Errorf("an error occurred while commenting pull request: %s", err)
 			return ctrl.Result{RequeueAfter: r.Config.Controller.Timers.OnError}
 		}
 		err = annotations.Add(ctx, r.Client, pr, map[string]string{annotations.LastCommentedCommit: pr.Annotations[annotations.LastDiscoveredCommit]})

internal/runner/runner.go

@@ -66,18 +66,24 @@ func (r *Runner) unlock() {
 	if err != nil {
 		log.Fatalf("could not remove lease lock for terraform layer %s: %s", r.layer.Name, err)
 	}
+	log.Infof("successfully removed lease lock for terraform layer %s", r.layer.Name)
 }
 
-func (r *Runner) Exec() {
+func (r *Runner) Exec() error {
 	defer r.unlock()
+
 	var sum string
-	err := r.init()
+	var commit string
 	ann := map[string]string{}
+
+	err := r.init()
 	if err != nil {
 		log.Errorf("error initializing runner: %s", err)
 	}
-	ref, _ := r.gitRepository.Head()
-	commit := ref.Hash().String()
+	if r.gitRepository != nil {
+		ref, _ := r.gitRepository.Head()
+		commit = ref.Hash().String()
+	}
 
 	switch r.config.Runner.Action {
 	case "plan":
@@ -95,8 +101,9 @@ func (r *Runner) Exec() {
 			ann[annotations.LastApplyCommit] = commit
 		}
 	default:
-		err = errors.New("unrecognized runner action, If this is happening there might be a version mismatch between the controller and runner")
+		err = errors.New("unrecognized runner action, if this is happening there might be a version mismatch between the controller and runner")
 	}
+
 	if err != nil {
 		log.Errorf("error during runner execution: %s", err)
 		n, ok := r.layer.Annotations[annotations.Failure]
@@ -109,10 +116,14 @@ func (r *Runner) Exec() {
 	} else {
 		ann[annotations.Failure] = "0"
 	}
-	err = annotations.Add(context.TODO(), r.client, r.layer, ann)
-	if err != nil {
+
+	annotErr := annotations.Add(context.TODO(), r.client, r.layer, ann)
+	if annotErr != nil {
 		log.Errorf("could not update terraform layer annotations: %s", err)
 	}
+	log.Infof("successfully updated terraform layer annotations")
+
+	return err
 }
 
 func (r *Runner) getLayerAndRepository() error {
@@ -195,6 +206,8 @@ func (r *Runner) init() error {
 	log.Infof("cloning repository %s %s branch", r.repository.Spec.Repository.Url, r.layer.Spec.Branch)
 	r.gitRepository, err = clone(r.config.Runner.Repository, r.repository.Spec.Repository.Url, r.layer.Spec.Branch, r.layer.Spec.Path)
 	if err != nil {
+		r.gitRepository = nil // reset git repository for the caller
+		log.Errorf("error cloning repository: %s", err)
 		return err
 	}
 	log.Infof("repository cloned successfully")
@@ -211,14 +224,18 @@ func (r *Runner) init() error {
 	log.Infof("Launching terraform init in %s", workingDir)
 	err = r.exec.Init(workingDir)
 	if err != nil {
-		log.Errorf("")
+		log.Errorf("error executing terraform init: %s", err)
 		return err
 	}
 	return nil
 }
 
 func (r *Runner) plan() (string, error) {
 	log.Infof("starting terraform plan")
+	if r.exec == nil {
+		err := errors.New("terraform or terragrunt binary not installed")
+		return "", err
+	}
 	err := r.exec.Plan()
 	if err != nil {
 		log.Errorf("error executing terraform plan: %s", err)
@@ -246,7 +263,7 @@ func (r *Runner) plan() (string, error) {
 		log.Errorf("error parsing terraform json plan: %s", err)
 		return "", err
 	}
-	diff, shortDiff := getDiff(plan)
+	_, shortDiff := getDiff(plan)
 	planJsonKey := storage.GenerateKey(storage.LastPlannedArtifactJson, r.layer)
 	log.Infof("setting plan json into storage at key %s", planJsonKey)
 	err = r.storage.Set(planJsonKey, planJsonBytes, 3600)
@@ -257,10 +274,6 @@ func (r *Runner) plan() (string, error) {
 	if err != nil {
 		log.Errorf("could not put short plan in cache: %s", err)
 	}
-	if !diff {
-		log.Infof("terraform plan diff empty, no subsequent apply should be launched")
-		return "", nil
-	}
 	planBin, err := os.ReadFile(PlanArtifact)
 	if err != nil {
 		log.Errorf("could not read plan output: %s", err)
@@ -280,6 +293,10 @@ func (r *Runner) plan() (string, error) {
 
 func (r *Runner) apply() (string, error) {
 	log.Infof("starting terraform apply")
+	if r.exec == nil {
+		err := errors.New("terraform or terragrunt binary not installed")
+		return "", err
+	}
 	planBinKey := storage.GenerateKey(storage.LastPlannedArtifactBin, r.layer)
 	log.Infof("getting plan binary in cache at key %s", planBinKey)
 	plan, err := r.storage.Get(planBinKey)

internal/webhook/github/provider.go

@@ -36,7 +36,7 @@ func (g *Github) GetEvent(r *http.Request) (event.Event, error) {
 		return nil, err
 	}
 	if err != nil {
-		log.Errorf("an error occured during request parsing: %s", err)
+		log.Errorf("an error occurred during request parsing: %s", err)
 		return nil, err
 	}
 

internal/webhook/gitlab/provider.go

@@ -36,7 +36,7 @@ func (g *Gitlab) GetEvent(r *http.Request) (event.Event, error) {
 		return nil, err
 	}
 	if err != nil {
-		log.Errorf("an error occured during event parsing: %s", err)
+		log.Errorf("an error occurred during event parsing: %s", err)
 		return nil, err
 	}
 	switch payload := p.(type) {