Revise Process lifecycle state machine

- removed StateFailed
- StateStarting --> StartStopped is OK
- updated tests

Author: mostlygeek

proxy/process.go

@@ -24,9 +24,6 @@ const (
 	StateReady    ProcessState = ProcessState("ready")
 	StateStopping ProcessState = ProcessState("stopping")
 
-	// failed a health check on start and will not be recovered
-	StateFailed ProcessState = ProcessState("failed")
-
 	// process is shutdown and will not be restarted
 	StateShutdown ProcessState = ProcessState("shutdown")
 )
@@ -70,6 +67,9 @@ type Process struct {
 
 	// used for testing to override the default value
 	gracefulStopTimeout time.Duration
+
+	// track the number of failed starts
+	failedStartCount int
 }
 
 func NewProcess(ID string, healthCheckTimeout int, config ModelConfig, processLogger *LogMonitor, proxyLogger *LogMonitor) *Process {
@@ -137,13 +137,11 @@ func isValidTransition(from, to ProcessState) bool {
 	case StateStopped:
 		return to == StateStarting
 	case StateStarting:
-		return to == StateReady || to == StateFailed || to == StateStopping
+		return to == StateReady || to == StateStopping || to == StateStopped
 	case StateReady:
 		return to == StateStopping
 	case StateStopping:
 		return to == StateStopped || to == StateShutdown
-	case StateFailed:
-		return to == StateStopping
 	case StateShutdown:
 		return false // No transitions allowed from these states
 	}
@@ -202,11 +200,13 @@ func (p *Process) start() error {
 	p.cancelUpstream = ctxCancelUpstream
 	p.cmdWaitChan = make(chan struct{})
 
+	p.failedStartCount++ // this will be reset to zero when the process has successfully started
 	err = p.cmd.Start()
 
 	// Set process state to failed
 	if err != nil {
-		if curState, swapErr := p.swapState(StateStarting, StateFailed); swapErr != nil {
+		if curState, swapErr := p.swapState(StateStarting, StateStopped); swapErr != nil {
+			p.state = StateStopped // force it into a stopped state
 			return fmt.Errorf(
 				"failed to start command and state swap failed. command error: %v, current state: %v, state swap error: %v",
 				err, curState, swapErr,
@@ -299,6 +299,7 @@ func (p *Process) start() error {
 	if curState, err := p.swapState(StateStarting, StateReady); err != nil {
 		return fmt.Errorf("failed to set Process state to ready: current state: %v, error: %v", curState, err)
 	} else {
+		p.failedStartCount = 0
 		return nil
 	}
 }
@@ -323,18 +324,9 @@ func (p *Process) StopImmediately() {
 	}
 
 	p.proxyLogger.Debugf("<%s> Stopping process, current state: %s", p.ID, p.CurrentState())
-	currentState := p.CurrentState()
-
-	if currentState == StateFailed {
-		if curState, err := p.swapState(StateFailed, StateStopping); err != nil {
-			p.proxyLogger.Infof("<%s> Stop() Failed -> StateStopping err: %v, current state: %v", p.ID, err, curState)
-			return
-		}
-	} else {
-		if curState, err := p.swapState(StateReady, StateStopping); err != nil {
-			p.proxyLogger.Infof("<%s> Stop() Ready -> StateStopping err: %v, current state: %v", p.ID, err, curState)
-			return
-		}
+	if curState, err := p.swapState(StateReady, StateStopping); err != nil {
+		p.proxyLogger.Infof("<%s> Stop() Ready -> StateStopping err: %v, current state: %v", p.ID, err, curState)
+		return
 	}
 
 	p.stopCommand()
@@ -401,7 +393,7 @@ func (p *Process) ProxyRequest(w http.ResponseWriter, r *http.Request) {
 
 	// prevent new requests from being made while stopping or irrecoverable
 	currentState := p.CurrentState()
-	if currentState == StateFailed || currentState == StateShutdown || currentState == StateStopping {
+	if currentState == StateShutdown || currentState == StateStopping {
 		http.Error(w, fmt.Sprintf("Process can not ProxyRequest, state is %s", currentState), http.StatusServiceUnavailable)
 		return
 	}

proxy/process_test.go

@@ -106,8 +106,8 @@ func TestProcess_BrokenModelConfig(t *testing.T) {
 
 	w = httptest.NewRecorder()
 	process.ProxyRequest(w, req)
-	assert.Equal(t, http.StatusServiceUnavailable, w.Code)
-	assert.Contains(t, w.Body.String(), "Process can not ProxyRequest, state is failed")
+	assert.Equal(t, http.StatusBadGateway, w.Code)
+	assert.Contains(t, w.Body.String(), "start() failed: ")
 }
 
 func TestProcess_UnloadAfterTTL(t *testing.T) {
@@ -248,18 +248,14 @@ func TestProcess_SwapState(t *testing.T) {
 	}{
 		{"Stopped to Starting", StateStopped, StateStopped, StateStarting, nil, StateStarting},
 		{"Starting to Ready", StateStarting, StateStarting, StateReady, nil, StateReady},
-		{"Starting to Failed", StateStarting, StateStarting, StateFailed, nil, StateFailed},
 		{"Starting to Stopping", StateStarting, StateStarting, StateStopping, nil, StateStopping},
+		{"Starting to Stopped", StateStarting, StateStarting, StateStopped, nil, StateStopped},
 		{"Ready to Stopping", StateReady, StateReady, StateStopping, nil, StateStopping},
 		{"Stopping to Stopped", StateStopping, StateStopping, StateStopped, nil, StateStopped},
 		{"Stopping to Shutdown", StateStopping, StateStopping, StateShutdown, nil, StateShutdown},
 		{"Stopped to Ready", StateStopped, StateStopped, StateReady, ErrInvalidStateTransition, StateStopped},
-		{"Starting to Stopped", StateStarting, StateStarting, StateStopped, ErrInvalidStateTransition, StateStarting},
 		{"Ready to Starting", StateReady, StateReady, StateStarting, ErrInvalidStateTransition, StateReady},
-		{"Ready to Failed", StateReady, StateReady, StateFailed, ErrInvalidStateTransition, StateReady},
 		{"Stopping to Ready", StateStopping, StateStopping, StateReady, ErrInvalidStateTransition, StateStopping},
-		{"Failed to Stopped", StateFailed, StateFailed, StateStopped, ErrInvalidStateTransition, StateFailed},
-		{"Failed to Starting", StateFailed, StateFailed, StateStarting, ErrInvalidStateTransition, StateFailed},
 		{"Shutdown to Stopped", StateShutdown, StateShutdown, StateStopped, ErrInvalidStateTransition, StateShutdown},
 		{"Shutdown to Starting", StateShutdown, StateShutdown, StateStarting, ErrInvalidStateTransition, StateShutdown},
 		{"Expected state mismatch", StateStopped, StateStarting, StateStarting, ErrExpectedStateMismatch, StateStopped},

proxy/proxymanager.go

@@ -348,8 +348,6 @@ func (pm *ProxyManager) upstreamIndex(c *gin.Context) {
 					stateStr = "Starting"
 				case StateStopping:
 					stateStr = "Stopping"
-				case StateFailed:
-					stateStr = "Failed"
 				case StateShutdown:
 					stateStr = "Shutdown"
 				case StateStopped: