Make tags for states, use to ensure thread safety

Tags are used as identifiers for the state, to ensure no other thread
has changed the thread in global history when setting a new state

Log an error when a supposedly impossible thing occurs. To be modified
in the future

src/controllers/deploy.go

@@ -48,7 +48,7 @@ func internaldeploy(a *history.ActionInstance) ([]byte, error) {
 	branch := defaultBranch
 
 	// This is a value, and thus modifying it does not change the original state in the history map
-	state := history.GetState(a.RepoURL)
+	state, tag := history.GetState(a.RepoURL)
 
 	var output []byte
 	var err error
@@ -72,20 +72,26 @@ func internaldeploy(a *history.ActionInstance) ([]byte, error) {
 		state.Access = a.Access
 		state.Server = a.Server
 		state.Status = "deploying"
-		if err1 := history.SetState(a.RepoURL, state); err1 != nil {
+		tag, err1 := history.SetState(a.RepoURL, tag, state)
+		if err1 != nil {
 			log.Infof("setting state to deploying failed - %v", err1)
 			output = []byte("InternalDeployError: cannot set state to deploying - " + err1.Error())
 			return output, err1
 		}
 		output, err = exec.Command(deployScriptName, "-n", "-u", a.RepoURL, "-b", branch, "-m", a.Server, "-s", a.Subdomain, "-a", a.Access).CombinedOutput()
 		if err != nil {
 			state.Status = "stopped"
-			history.SetState(a.RepoURL, state)
 		} else {
 			state.Status = "running"
-			history.SetState(a.RepoURL, state)
 		}
 
+		// There should be no error here, ever. Checking it to make sure
+		// TODO: On error, set state to an "error" state which only stop should be able to modify
+		tag, err1 = history.SetState(a.RepoURL, tag, state)
+		for ; err1 != nil; tag, err1 = history.SetState(a.RepoURL, tag, state) {
+			log.Errorf("setting state to %v failed - %v. Retrying...", state.Status, err1)
+		}
+		log.Infof("setting state to %v successful", state.Status)
 	}
 	return output, err
 }

src/controllers/logs.go

@@ -50,7 +50,7 @@ func logs(callbackID string, data map[string]interface{}) {
 func internalLogs(data map[string]interface{}) ([]byte, error) {
 	gitRepoURL := data["git_repo"].(string)
 	tailCount := data["tail_count"].(string)
-	current := history.GetState(gitRepoURL)
+	current, _ := history.GetState(gitRepoURL)
 	serverName := current.Server
 	if current.Status != "running" {
 		log.Infof("service %s is not running, cannot fetch logs", gitRepoURL)

src/controllers/redeploy.go

@@ -6,7 +6,7 @@ import (
 
 func redeploy(callbackID string, data map[string]interface{}) {
 	stop(callbackID, data)
-	state := history.GetState(data["git_repo"].(string))
+	state, _ := history.GetState(data["git_repo"].(string))
 	if state.Status == "stopped" {
 		data["subdomain"] = state.Subdomain
 		data["access"] = state.Access

src/controllers/stop.go

@@ -43,7 +43,7 @@ func stop(callbackID string, data map[string]interface{}) {
 
 // internalStop actually runs the script to stop the given app.
 func internalStop(a *history.ActionInstance) ([]byte, error) {
-	state := history.GetState(a.RepoURL)
+	state, tag := history.GetState(a.RepoURL)
 
 	var output []byte
 	var err error
@@ -60,14 +60,26 @@ func internalStop(a *history.ActionInstance) ([]byte, error) {
 	case "running":
 		log.Infof("calling %s to stop service(%s)", stopScriptName, a.RepoURL)
 		state.Status = "stopping"
-		history.SetState(a.RepoURL, state)
+		tag, err1 := history.SetState(a.RepoURL, tag, state)
+		if err1 != nil {
+			log.Infof("setting state to stopping failed - %v", err1)
+			output = []byte("InternalStopError: cannot set state to stopping - " + err1.Error())
+			return output, err1
+		}
+
 		if output, err = exec.Command(stopScriptName, state.Subdomain, a.RepoURL, state.Server).CombinedOutput(); err != nil {
 			state.Status = "running"
-			history.SetState(a.RepoURL, state)
 		} else {
 			state.Status = "stopped"
-			history.SetState(a.RepoURL, state)
 		}
+
+		// There should be no error here, ever. Checking it to make sure
+		// TODO: On error, set state to an "error" state which only stop should be able to modify
+		tag, err1 = history.SetState(a.RepoURL, tag, state)
+		for ; err1 != nil; tag, err1 = history.SetState(a.RepoURL, tag, state) {
+			log.Errorf("setting state to %v failed - %v. Retrying...", state.Status, err1)
+		}
+		log.Infof("setting state to %v successful", state.Status)
 	default:
 		log.Infof("service(%s) is already stopped", a.RepoURL)
 		output = []byte("Service is already stopped!")

src/history/config.go

@@ -2,6 +2,7 @@ package history
 
 import (
 	"bytes"
+	"crypto/sha256"
 	"fmt"
 	"html/template"
 	"path"
@@ -191,6 +192,7 @@ type Service struct {
 	Actions      []*ActionInstance `json:"actions"`
 	HealthChecks []*HealthCheck    `json:"health_checks"`
 	Current      *State            `json:"current"`
+	StateTag     string            `json:"state_tag"`
 }
 
 // NewService returns a blank service, with the state as stopped
@@ -202,8 +204,16 @@ func NewService() *Service {
 	}
 }
 
-var history = make(map[string]*Service)
-var mux sync.Mutex
+var (
+	history = make(map[string]*Service)
+	mux     sync.Mutex
+	tagHash = sha256.Sum256
+)
+
+func makeTag(repoURL string, state State) string {
+	data := []byte(fmt.Sprintf("%v-%v-%v-%v-%v", repoURL, state.Status, state.Subdomain, state.Access, state.Server))
+	return fmt.Sprintf("%x", tagHash(data))
+}
 
 // newZapLogger returns a sugared logger with output to a given file, in a format we need
 func newZapLogger(outfile string) (*zap.SugaredLogger, error) {

src/history/history.go

@@ -2,12 +2,12 @@ package history
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net/http"
 	"strings"
 	"time"
-	"errors"
 
 	"github.com/devclub-iitd/DeployBot/src/helper"
 	log "github.com/sirupsen/logrus"
@@ -28,6 +28,15 @@ func initState() error {
 	if err := json.Unmarshal(bytes, &history); err != nil {
 		return fmt.Errorf("cannot unmarshal json to history - %v", err)
 	}
+
+	for repoURL, service := range history {
+		actualTag := makeTag(repoURL, *service.Current)
+		if service.StateTag != actualTag {
+			log.Warnf("State tag for %s in history file is \"%s\", expected \"%s\"", repoURL, service.StateTag, actualTag)
+			service.StateTag = actualTag
+		}
+	}
+
 	return nil
 }
 
@@ -94,32 +103,39 @@ func StoreHealth(hc *HealthCheck) {
 	go writeHealth(hc)
 }
 
-// GetState returns the current state of the service
-func GetState(repoURL string) State {
+// GetState returns the current state of the service and a tag identifying it
+func GetState(repoURL string) (State, string) {
 	mux.Lock()
 	defer mux.Unlock()
 	if _, ok := history[repoURL]; !ok {
 		history[repoURL] = NewService()
 	}
-	return *history[repoURL].Current
+	return *history[repoURL].Current, history[repoURL].StateTag
 }
 
 // SetState sets the current state of service
-func SetState(repoURL string, cur State) error {
+// Compares the current state of repoURL to provided tag, and if it matches,
+// sets it to reqState. Returns a tag for the new state and an error.
+func SetState(repoURL string, tag string, reqState State) (string, error) {
 	mux.Lock()
 	defer mux.Unlock()
 	if _, ok := history[repoURL]; !ok {
 		history[repoURL] = NewService()
 	}
-	cur.Timestamp = time.Now()
+	reqState.Timestamp = time.Now()
 	var err error
-	if cur.Status != "deploying" || checkSubdomain(cur.Subdomain) {
-		history[repoURL].Current = &cur
-	} else {
+	if reqState.Status == "deploying" && checkSubdomain(reqState.Subdomain) {
 		err = errors.New("subdomain in use")
+	} else if history[repoURL].StateTag != tag {
+		err = fmt.Errorf("old tag provided %s, does not match the original tag %s", tag, history[repoURL].StateTag)
+		tag = history[repoURL].StateTag
+	} else {
+		history[repoURL].Current = &reqState
+		tag = makeTag(repoURL, reqState)
+		history[repoURL].StateTag = tag
 	}
 	go BackupState()
-	return err
+	return tag, err
 }
 
 func serviceBytes(subdomain string) []byte {