From e75a8f8ae2ddb1f0335d7cf5a619b1f3e29159e2 Mon Sep 17 00:00:00 2001 From: Arpit Saxena Date: Sat, 16 May 2020 13:23:59 +0530 Subject: [PATCH] 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 | 14 ++++++++++---- src/controllers/logs.go | 2 +- src/controllers/redeploy.go | 2 +- src/controllers/stop.go | 20 ++++++++++++++++---- src/history/config.go | 14 ++++++++++++-- src/history/history.go | 36 ++++++++++++++++++++++++++---------- 6 files changed, 66 insertions(+), 22 deletions(-) diff --git a/src/controllers/deploy.go b/src/controllers/deploy.go index 3af797f..353063a 100644 --- a/src/controllers/deploy.go +++ b/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,7 +72,8 @@ 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 @@ -80,12 +81,17 @@ func internaldeploy(a *history.ActionInstance) ([]byte, error) { 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 } diff --git a/src/controllers/logs.go b/src/controllers/logs.go index e50fae0..c85a5b8 100644 --- a/src/controllers/logs.go +++ b/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) diff --git a/src/controllers/redeploy.go b/src/controllers/redeploy.go index 486fc62..9f245df 100644 --- a/src/controllers/redeploy.go +++ b/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 diff --git a/src/controllers/stop.go b/src/controllers/stop.go index 85078bb..a4ccac5 100644 --- a/src/controllers/stop.go +++ b/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!") diff --git a/src/history/config.go b/src/history/config.go index 75e3d2d..67e50dd 100644 --- a/src/history/config.go +++ b/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) { diff --git a/src/history/history.go b/src/history/history.go index 574aabc..b1b238b 100644 --- a/src/history/history.go +++ b/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 {