Merge pull request #106 from Kifen/backport-appconfig

Backport master fix to milestone2

go.mod

@@ -12,7 +12,6 @@ require (
 	github.com/google/uuid v1.1.1
 	github.com/gorilla/handlers v1.4.2
 	github.com/gorilla/securecookie v1.1.1
-	github.com/mitchellh/go-homedir v1.1.0
 	github.com/pkg/profile v1.3.0
 	github.com/prometheus/client_golang v1.2.1
 	github.com/prometheus/common v0.7.0

pkg/visor/config.go

@@ -134,13 +134,13 @@ func (c *Config) RoutingTable() (routing.Table, error) {
 }
 
 // AppsConfig decodes AppsConfig from a local json config file.
-func (c *Config) AppsConfig() ([]AppConfig, error) {
-	apps := make([]AppConfig, 0)
+func (c *Config) AppsConfig() (map[string]AppConfig, error) {
+	apps := make(map[string]AppConfig)
 	for _, app := range c.Apps {
 		if app.Version == "" {
 			app.Version = c.Version
 		}
-		apps = append(apps, app)
+		apps[app.App] = app
 	}
 
 	return apps, nil

pkg/visor/config_test.go

@@ -94,13 +94,13 @@ func TestAppsConfig(t *testing.T) {
 	appsConf, err := conf.AppsConfig()
 	require.NoError(t, err)
 
-	app1 := appsConf[0]
+	app1 := appsConf["foo"]
 	assert.Equal(t, "foo", app1.App)
 	assert.Equal(t, "1.1", app1.Version)
 	assert.Equal(t, routing.Port(1), app1.Port)
 	assert.False(t, app1.AutoStart)
 
-	app2 := appsConf[1]
+	app2 := appsConf["bar"]
 	assert.Equal(t, "bar", app2.App)
 	assert.Equal(t, "1.0", app2.Version)
 	assert.Equal(t, routing.Port(2), app2.Port)

pkg/visor/rpc_test.go

@@ -68,7 +68,8 @@ func TestUptime(t *testing.T) {
 
 // TODO (Darkren): fix tests
 func TestListApps(t *testing.T) {
-	apps := []AppConfig{
+	apps := make(map[string]AppConfig)
+	appCfg := []AppConfig{
 		{
 			App:       "foo",
 			AutoStart: false,
@@ -81,9 +82,13 @@ func TestListApps(t *testing.T) {
 		},
 	}
 
+	for _, app := range appCfg {
+		apps[app.App] = app
+	}
+
 	pm := &appserver.MockProcManager{}
-	pm.On("Exists", apps[0].App).Return(false)
-	pm.On("Exists", apps[1].App).Return(true)
+	pm.On("Exists", apps["foo"].App).Return(false)
+	pm.On("Exists", apps["bar"].App).Return(true)
 
 	n := Node{
 		appsConf:    apps,
@@ -119,17 +124,20 @@ func TestStartStopApp(t *testing.T) {
 		require.NoError(t, os.RemoveAll("skychat"))
 	}()
 
-	apps := []AppConfig{
+	appCfg := []AppConfig{
 		{
 			App:       "foo",
 			Version:   "1.0",
 			AutoStart: false,
 			Port:      10,
 		},
 	}
+	apps := map[string]AppConfig{
+		"foo": appCfg[0],
+	}
 
 	unknownApp := "bar"
-	app := apps[0].App
+	app := apps["foo"].App
 
 	nodeCfg := Config{}
 	nodeCfg.Node.StaticPubKey = pk
@@ -148,12 +156,12 @@ func TestStartStopApp(t *testing.T) {
 	pm := &appserver.MockProcManager{}
 	appCfg1 := appcommon.Config{
 		Name:         app,
-		Version:      apps[0].Version,
+		Version:      apps["foo"].Version,
 		SockFilePath: nodeCfg.AppServerSockFile,
 		VisorPK:      nodeCfg.Node.StaticPubKey.Hex(),
-		WorkDir:      filepath.Join("", app, fmt.Sprintf("v%s", apps[0].Version)),
+		WorkDir:      filepath.Join("", app, fmt.Sprintf("v%s", apps["foo"].Version)),
 	}
-	appArgs1 := append([]string{filepath.Join(node.dir(), app)}, apps[0].Args...)
+	appArgs1 := append([]string{filepath.Join(node.dir(), app)}, apps["foo"].Args...)
 	appPID1 := appcommon.ProcID(10)
 	pm.On("Run", mock.Anything, appCfg1, appArgs1, mock.Anything, mock.Anything).
 		Return(appPID1, testhelpers.NoErr)

pkg/visor/visor.go

@@ -81,7 +81,7 @@ type Node struct {
 
 	appsPath  string
 	localPath string
-	appsConf  []AppConfig
+	appsConf  map[string]AppConfig
 
 	startedAt  time.Time
 	restartCtx *restart.Context
@@ -487,11 +487,12 @@ func (node *Node) StopApp(appName string) error {
 
 // SetAutoStart sets an app to auto start or not.
 func (node *Node) SetAutoStart(appName string, autoStart bool) error {
-	for i, ac := range node.appsConf {
-		if ac.App == appName {
-			node.appsConf[i].AutoStart = autoStart
-			return nil
-		}
+	appConf, ok := node.appsConf[appName]
+	if !ok {
+		return ErrUnknownApp
 	}
-	return ErrUnknownApp
+
+	appConf.AutoStart = autoStart
+	node.appsConf[appName] = appConf
+	return nil
 }

pkg/visor/visor_test.go

@@ -88,7 +88,8 @@ func TestNodeStartClose(t *testing.T) {
 	r.On("Serve", mock.Anything /* context */).Return(testhelpers.NoErr)
 	r.On("Close").Return(testhelpers.NoErr)
 
-	apps := []AppConfig{
+	apps := make(map[string]AppConfig)
+	appCfg := []AppConfig{
 		{
 			App:       "skychat",
 			Version:   "1.0",
@@ -102,6 +103,10 @@ func TestNodeStartClose(t *testing.T) {
 		},
 	}
 
+	for _, app := range appCfg {
+		apps[app.App] = app
+	}
+
 	defer func() {
 		require.NoError(t, os.RemoveAll("skychat"))
 	}()
@@ -117,17 +122,17 @@ func TestNodeStartClose(t *testing.T) {
 
 	pm := &appserver.MockProcManager{}
 	appCfg1 := appcommon.Config{
-		Name:         apps[0].App,
-		Version:      apps[0].Version,
+		Name:         apps["skychat"].App,
+		Version:      apps["skychat"].Version,
 		SockFilePath: nodeCfg.AppServerSockFile,
 		VisorPK:      nodeCfg.Node.StaticPubKey.Hex(),
-		WorkDir:      filepath.Join("", apps[0].App, fmt.Sprintf("v%s", apps[0].Version)),
+		WorkDir:      filepath.Join("", apps["skychat"].App, fmt.Sprintf("v%s", apps["skychat"].Version)),
 	}
-	appArgs1 := append([]string{filepath.Join(node.dir(), apps[0].App)}, apps[0].Args...)
+	appArgs1 := append([]string{filepath.Join(node.dir(), apps["skychat"].App)}, apps["skychat"].Args...)
 	appPID1 := appcommon.ProcID(10)
 	pm.On("Run", mock.Anything, appCfg1, appArgs1, mock.Anything, mock.Anything).
 		Return(appPID1, testhelpers.NoErr)
-	pm.On("Wait", apps[0].App).Return(testhelpers.NoErr)
+	pm.On("Wait", apps["skychat"].App).Return(testhelpers.NoErr)
 
 	pm.On("StopAll").Return()
 
@@ -180,12 +185,15 @@ func TestNodeSpawnApp(t *testing.T) {
 		Args:      []string{"foo"},
 	}
 
+	apps := make(map[string]AppConfig)
+	apps["skychat"] = app
+
 	nodeCfg := Config{}
 	nodeCfg.Node.StaticPubKey = pk
 
 	node := &Node{
 		router:   r,
-		appsConf: []AppConfig{app},
+		appsConf: apps,
 		logger:   logging.MustGetLogger("test"),
 		conf:     &nodeCfg,
 	}