Merge pull request #637 from Darkren/feature/improve-app-config-update

Make app be restarted only once if some arg changed
skycoin · Dec 17, 2020 · e4ec693 · e4ec693
2 parents 5b0f027 + 1e7706a
commit e4ec693
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 24 deletions.
diff --git a/pkg/visor/api.go b/pkg/visor/api.go
@@ -34,6 +34,7 @@ type API interface {
 	Apps() ([]*launcher.AppState, error)
 	StartApp(appName string) error
 	StopApp(appName string) error
+	RestartApp(appName string) error
 	SetAutoStart(appName string, autostart bool) error
 	SetAppPassword(appName, password string) error
 	SetAppPK(appName string, pk cipher.PubKey) error
@@ -258,6 +259,16 @@ func (v *Visor) StopApp(appName string) error {
 	return err
 }
 
+// RestartApp implements API.
+func (v *Visor) RestartApp(appName string) error {
+	if _, ok := v.procM.ProcByName(appName); ok {
+		v.log.Infof("Updated %v password, restarting it", appName)
+		return v.appL.RestartApp(appName)
+	}
+
+	return nil
+}
+
 // SetAutoStart implements API.
 func (v *Visor) SetAutoStart(appName string, autoStart bool) error {
 	if _, ok := v.appL.AppState(appName); !ok {
@@ -295,11 +306,6 @@ func (v *Visor) SetAppPassword(appName, password string) error {
 		return err
 	}
 
-	if _, ok := v.procM.ProcByName(appName); ok {
-		v.log.Infof("Updated %v password, restarting it", appName)
-		return v.appL.RestartApp(appName)
-	}
-
 	v.log.Infof("Updated %v password", appName)
 
 	return nil
@@ -328,11 +334,6 @@ func (v *Visor) SetAppKillswitch(appName string, killswitch bool) error {
 		return err
 	}
 
-	if _, ok := v.procM.ProcByName(appName); ok {
-		v.log.Infof("Updated %v killswitch state, restarting it", appName)
-		return v.appL.RestartApp(appName)
-	}
-
 	v.log.Infof("Updated %v killswitch state", appName)
 
 	return nil
@@ -361,11 +362,6 @@ func (v *Visor) SetAppSecure(appName string, isSecure bool) error {
 		return err
 	}
 
-	if _, ok := v.procM.ProcByName(appName); ok {
-		v.log.Infof("Updated %v secure state, restarting it", appName)
-		return v.appL.RestartApp(appName)
-	}
-
 	v.log.Infof("Updated %v secure state", appName)
 
 	return nil
@@ -397,11 +393,6 @@ func (v *Visor) SetAppPK(appName string, pk cipher.PubKey) error {
 		return err
 	}
 
-	if _, ok := v.procM.ProcByName(appName); ok {
-		v.log.Infof("Updated %v PK, restarting it", appName)
-		return v.appL.RestartApp(appName)
-	}
-
 	v.log.Infof("Updated %v PK", appName)
 
 	return nil

diff --git a/pkg/visor/hypervisor.go b/pkg/visor/hypervisor.go
@@ -513,7 +513,7 @@ func (hv *Hypervisor) getApp() http.HandlerFunc {
 // nolint: funlen,gocognit,godox
 func (hv *Hypervisor) putApp() http.HandlerFunc {
 	return hv.withCtx(hv.appCtx, func(w http.ResponseWriter, r *http.Request, ctx *httpCtx) {
-		var reqBody struct {
+		type req struct {
 			AutoStart  *bool          `json:"autostart,omitempty"`
 			Killswitch *bool          `json:"killswitch,omitempty"`
 			Secure     *bool          `json:"secure,omitempty"`
@@ -522,6 +522,13 @@ func (hv *Hypervisor) putApp() http.HandlerFunc {
 			PK         *cipher.PubKey `json:"pk,omitempty"`
 		}
 
+		shouldRestartApp := func(r req) bool {
+			// we restart the app if one of these fields was changed
+			return r.Killswitch != nil || r.Secure != nil || r.Passcode != nil ||
+				r.PK != nil
+		}
+
+		var reqBody req
 		if err := httputil.ReadJSON(r, &reqBody); err != nil {
 			if err != io.EOF {
 				hv.log(r).Warnf("putApp request: %v", err)
@@ -555,9 +562,6 @@ func (hv *Hypervisor) putApp() http.HandlerFunc {
 			}
 		}
 
-		// todo: this was just a quick copypasting to mimic the style already present here
-		// possible issue: each cal to ctx.API.Set* triggers app restart, so if we change multiple
-		// values in a single request the app will be restarted multple times
 		if reqBody.Killswitch != nil {
 			if err := ctx.API.SetAppKillswitch(ctx.App.Name, *reqBody.Killswitch); err != nil {
 				httputil.WriteJSON(w, r, http.StatusInternalServerError, err)
@@ -572,6 +576,13 @@ func (hv *Hypervisor) putApp() http.HandlerFunc {
 			}
 		}
 
+		if shouldRestartApp(reqBody) {
+			if err := ctx.API.RestartApp(ctx.App.Name); err != nil {
+				httputil.WriteJSON(w, r, http.StatusInternalServerError, err)
+				return
+			}
+		}
+
 		if reqBody.Status != nil {
 			switch *reqBody.Status {
 			case statusStop:

diff --git a/pkg/visor/rpc.go b/pkg/visor/rpc.go
@@ -220,6 +220,13 @@ func (r *RPC) StopApp(name *string, _ *struct{}) (err error) {
 	return r.visor.StopApp(*name)
 }
 
+// RestartApp restarts App with provided name.
+func (r *RPC) RestartApp(name *string, _ *struct{}) (err error) {
+	defer rpcutil.LogCall(r.log, "RestartApp", name)(nil, &err)
+
+	return r.visor.RestartApp(*name)
+}
+
 // SetAutoStartIn is input for SetAutoStart.
 type SetAutoStartIn struct {
 	AppName   string

diff --git a/pkg/visor/rpc_client.go b/pkg/visor/rpc_client.go
@@ -137,6 +137,11 @@ func (rc *rpcClient) StopApp(appName string) error {
 	return rc.Call("StopApp", &appName, &struct{}{})
 }
 
+// RestartApp calls `RestartApp`.
+func (rc *rpcClient) RestartApp(appName string) error {
+	return rc.Call("RestartApp", &appName, &struct{}{})
+}
+
 // SetAutoStart calls SetAutoStart.
 func (rc *rpcClient) SetAutoStart(appName string, autostart bool) error {
 	return rc.Call("SetAutoStart", &SetAutoStartIn{
@@ -615,6 +620,11 @@ func (*mockRPCClient) StopApp(string) error {
 	return nil
 }
 
+// RestartApp implements API.
+func (*mockRPCClient) RestartApp(string) error {
+	return nil
+}
+
 // SetAutoStart implements API.
 func (mc *mockRPCClient) SetAutoStart(appName string, autostart bool) error {
 	return mc.do(true, func() error {