From bcecdd72cea9481aa58d9163a4b0cd954adbf78d Mon Sep 17 00:00:00 2001
From: Andrew Lavery <laverya@umich.edu>
Date: Mon, 10 Jun 2019 17:05:44 -0700
Subject: [PATCH] avoid deadlocks when starting the terraform lifecycle step

---
 pkg/lifecycle/daemon/routes_navcycle.go       | 23 ++++++++++++++-----
 .../daemon/routes_navcycle_completestep.go    |  5 ++--
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/pkg/lifecycle/daemon/routes_navcycle.go b/pkg/lifecycle/daemon/routes_navcycle.go
index 2666a26c1..f5939858a 100644
--- a/pkg/lifecycle/daemon/routes_navcycle.go
+++ b/pkg/lifecycle/daemon/routes_navcycle.go
@@ -127,14 +127,22 @@ func (d *NavcycleRoutes) shutdown(c *gin.Context) {
 
 // returns false if aborted
 func (d *NavcycleRoutes) maybeAbortDueToMissingRequirement(requires []string, c *gin.Context, requestedStepID string) (ok bool) {
-	required, err := d.getRequiredButIncompleteStepFor(requires)
+	required, err := d.getRequiredButIncompleteStepFor(requires, false)
 	if err != nil {
 		c.AbortWithError(500, errors.Wrapf(err, "check requirements for step %s", requestedStepID))
 		return false
 	}
 	if required != "" {
-		d.errRequired(required, c)
-		return false
+		// if there is a required step that has not been completed, wait for any currently running steps to complete before checking again
+		required, err = d.getRequiredButIncompleteStepFor(requires, true)
+		if err != nil {
+			c.AbortWithError(500, errors.Wrapf(err, "check requirements for step %s", requestedStepID))
+			return false
+		}
+		if required != "" {
+			d.errRequired(required, c)
+			return false
+		}
 	}
 	return true
 }
@@ -142,14 +150,17 @@ func (d *NavcycleRoutes) maybeAbortDueToMissingRequirement(requires []string, c
 // this will return an incomplete step that is present in the list of required steps.
 // if there are multiple required but incomplete steps, this will return the first one,
 // although from a UI perspective the order is probably not strictly defined
-func (d *NavcycleRoutes) getRequiredButIncompleteStepFor(requires []string) (string, error) {
+func (d *NavcycleRoutes) getRequiredButIncompleteStepFor(requires []string, withLock bool) (string, error) {
 	debug := level.Debug(log.With(d.Logger, "method", "getRequiredButIncompleteStepFor"))
 	if len(requires) == 0 {
 		return "", nil
 	}
 
-	d.completeMut.Lock()
-	defer d.completeMut.Unlock()
+	if withLock {
+		debug.Log("event", "completeMut.lock")
+		d.completeMut.Lock()
+		defer d.completeMut.Unlock()
+	}
 
 	stepsCompleted := map[string]interface{}{}
 	currentState, err := d.StateManager.TryLoad()
diff --git a/pkg/lifecycle/daemon/routes_navcycle_completestep.go b/pkg/lifecycle/daemon/routes_navcycle_completestep.go
index dbc1d467f..4145f7710 100644
--- a/pkg/lifecycle/daemon/routes_navcycle_completestep.go
+++ b/pkg/lifecycle/daemon/routes_navcycle_completestep.go
@@ -75,6 +75,7 @@ func (d *NavcycleRoutes) handleAsync(errChan chan error, debug log.Logger, step
 	_, err := d.StateManager.StateUpdate(func(currentState state.State) (state.State, error) {
 		return currentState.WithCompletedStep(step), nil
 	})
+
 	if err != nil {
 		level.Error(d.Logger).Log("event", "state.save.fail", "err", err, "step.id", stepID)
 		return
@@ -97,11 +98,11 @@ func (d *NavcycleRoutes) awaitAsyncStep(errChan chan error, debug log.Logger, st
 				level.Error(debug).Log("event", "async.fail", "err", err, "errorWithStack", errToPrint, "progress", d.progress(step))
 				return err
 			}
-			level.Info(debug).Log("event", "task.complete", "progess", d.progress(step))
+			level.Info(debug).Log("event", "task.complete", "progress", d.progress(step))
 			return nil
 		// debug log progress every ten seconds
 		case <-time.After(10 * time.Second):
-			debug.Log("event", "task.running", "progess", d.progress(step))
+			debug.Log("event", "task.running", "progress", d.progress(step))
 		}
 	}
 }