Lock app restart, cleanup and better testing

Makefile

@@ -38,9 +38,9 @@ run-supervisor: supervisor-dind stop-supervisor
 stop-supervisor:
 	# Stop docker and remove volumes to prevent us from running out of loopback devices,
 	# as per https://github.com/jpetazzo/dind/issues/19
-	-docker exec resin_supervisor_1 bash -c "systemctl stop docker"
-	-docker stop resin_supervisor_1 > /dev/null
-	-docker rm -f --volumes resin_supervisor_1 > /dev/null
+	-docker exec resin_supervisor_1 bash -c "systemctl stop docker" || true
+	-docker stop resin_supervisor_1 > /dev/null || true
+	-docker rm -f --volumes resin_supervisor_1 > /dev/null || true
 
 supervisor: gosuper
 	cp Dockerfile.$(ARCH) Dockerfile
@@ -59,22 +59,22 @@ go-builder:
 
 gosuper: go-builder
 	-mkdir -p gosuper/bin
-	-docker rm --volumes -f resin_build_gosuper_$(JOB_NAME)
+	-docker rm --volumes -f resin_build_gosuper_$(JOB_NAME) || true
 	docker run --name resin_build_gosuper_$(JOB_NAME) -v $(shell pwd)/bin:/usr/src/app/bin -e USER_ID=$(shell id -u) -e GROUP_ID=$(shell id -g) -e GOARCH=$(GOARCH) resin/go-supervisor-builder:$(SUPERVISOR_VERSION)
 	docker rm --volumes -f resin_build_gosuper_$(JOB_NAME)
 
 test-gosuper: go-builder
-	-docker rm --volumes -f resin_test_gosuper_$(JOB_NAME)
+	-docker rm --volumes -f resin_test_gosuper_$(JOB_NAME) || true
 	docker run --name resin_test_gosuper_$(JOB_NAME) -v $(shell pwd)/bin:/usr/src/app/bin resin/go-supervisor-builder:$(SUPERVISOR_VERSION) bash -c "cd src/resin-supervisor/gosuper && ./test_formatting.sh && go test -v ./gosuper"
 	docker rm --volumes -f resin_test_gosuper_$(JOB_NAME)
 
 format-gosuper: go-builder
-	-docker rm --volumes -f resin_test_gosuper_$(JOB_NAME)
+	-docker rm --volumes -f resin_test_gosuper_$(JOB_NAME) || true
 	docker run --name resin_test_gosuper_$(JOB_NAME) -v $(shell pwd)/bin:/usr/src/app/bin -v $(shell pwd)/gosuper:/usr/src/app/src/resin-supervisor/gosuper resin/go-supervisor-builder:$(SUPERVISOR_VERSION) bash -c "cd src/resin-supervisor/gosuper && go fmt ./..."
 	docker rm --volumes -f resin_test_gosuper_$(JOB_NAME)
 
 test-integration: go-builder
-	-docker rm --volumes -f resin_test_integration_$(JOB_NAME)
+	-docker rm --volumes -f resin_test_integration_$(JOB_NAME) || true
 	docker run --name resin_test_integration_$(JOB_NAME) --net=host -e SUPERVISOR_IP="$(shell docker inspect --format '{{ .NetworkSettings.IPAddress }}' resin_supervisor_1)" -v $(shell pwd)/bin:/usr/src/app/bin --volumes-from resin_supervisor_1 resin/go-supervisor-builder:$(SUPERVISOR_VERSION) bash -c "cd src/resin-supervisor/gosuper && go test -v ./supertest"
 	docker rm --volumes -f resin_test_integration_$(JOB_NAME)
 

gosuper/build_gosuper.sh

@@ -3,9 +3,10 @@
 go install -a -v ./gosuper
 RETURN_VALUE=$?
 
+HOSTARCH=$(uname -m)
 # For consistency, always keep the binary within a linux_$GOARCH folder
-if [ $GOARCH == "amd64" ]; then
-	mkdir $GOPATH/bin/linux_$GOARCH || true
+if [[ ( $GOARCH == "amd64" && $HOSTARCH == "x86_64" ) || ( $GOARCH == "arm" && $HOSTARCH == "armv7l" ) ]]; then
+	mkdir -p $GOPATH/bin/linux_$GOARCH
 	cp $GOPATH/bin/gosuper $GOPATH/bin/linux_$GOARCH/
 fi
 chown -R $USER_ID:$GROUP_ID $GOPATH/bin

gosuper/gosuper/config.go

@@ -16,17 +16,10 @@ type UserConfig struct {
 	DeviceId      float64
 }
 
-func ReadConfig(path string) (UserConfig, error) {
-
-	var config UserConfig
-
-	data, err := ioutil.ReadFile(path)
-
-	if err != nil {
-		return config, err
+func ReadConfig(path string) (config UserConfig, err error) {
+	if data, err := ioutil.ReadFile(path); err == nil {
+		err = json.Unmarshal(data, &config)
 	}
 
-	err = json.Unmarshal(data, &config)
-
-	return config, err
+	return
 }

gosuper/gosuper/main.go

@@ -10,40 +10,34 @@ import (
 	"resin-supervisor/gosuper/Godeps/_workspace/src/github.com/gorilla/mux"
 )
 
-func pingHandler(w http.ResponseWriter, r *http.Request) {
-	fmt.Fprintln(w, "OK")
+var ResinDataPath string = "/mnt/root/resin-data/"
+
+func setupApi(router *mux.Router) {
+	router.HandleFunc("/ping", func(writer http.ResponseWriter, request *http.Request) {
+		fmt.Fprintln(writer, "OK")
+	})
+
+	apiv1 := router.PathPrefix("/v1").Subrouter()
+	apiv1.HandleFunc("/purge", PurgeHandler).Methods("POST")
 }
 
-//var Config UserConfig // Disabled until we use Config
-var ResinDataPath string
+func startApi(listenAddress string, router *mux.Router) {
+	if listener, err := net.Listen("unix", listenAddress); err != nil {
+		log.Fatalf("Could not listen on %s: %v", listenAddress, err)
+	} else {
+		log.Printf("Starting HTTP server on %s\n", listenAddress)
+		if err = http.Serve(listener, router); err != nil {
+			log.Fatalf("Could not start HTTP server: %v", err)
+		}
+	}
+}
 
 func main() {
-	fmt.Println("Resin Go Supervisor starting")
-	/* Disabled until we use Config
-	var err error
-	Config, err = ReadConfig("/boot/config.json")
-	if err != nil {
-		log.Fatalf("Could not read configuration file: %v", err)
-	}
-	*/
+	log.SetFlags(log.Lshortfile | log.LstdFlags)
+	log.Println("Resin Go Supervisor starting")
 
-	ResinDataPath = "/mnt/root/resin-data/"
 	listenAddress := os.Getenv("GOSUPER_SOCKET")
-
-	r := mux.NewRouter()
-	r.HandleFunc("/ping", pingHandler)
-	apiv1 := r.PathPrefix("/v1").Subrouter()
-
-	apiv1.HandleFunc("/purge", PurgeHandler).Methods("POST")
-
-	fmt.Println("Going to listen on " + listenAddress)
-	listener, err := net.Listen("unix", listenAddress)
-	if err != nil {
-		log.Fatalf("Could not listen on "+listenAddress+": %v", err)
-	}
-	fmt.Println("Starting HTTP server")
-	err = http.Serve(listener, r)
-	if err != nil {
-		log.Fatalf("Could not start HTTP server: %v", err)
-	}
+	router := mux.NewRouter()
+	setupApi(router)
+	startApi(listenAddress, router)
 }

gosuper/gosuper/main_test.go

@@ -11,50 +11,41 @@ import (
 
 func TestPurge(t *testing.T) {
 	appId := "1"
-	req, _ := http.NewRequest("POST", "/v1/purge", strings.NewReader(`{"applicationId": "`+appId+`"}`))
-	req.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value")
-
-	w := httptest.NewRecorder()
-
+	request, err := http.NewRequest("POST", "/v1/purge", strings.NewReader(`{"applicationId": "`+appId+`"}`))
+	if err != nil {
+		t.Fatal(err)
+	}
+	request.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value")
+	writer := httptest.NewRecorder()
 	ResinDataPath = "test-data/"
-
 	dataPath := ResinDataPath + appId
 
-	err := os.MkdirAll(dataPath, 0755)
-	if err != nil {
-		t.Error("Could not create test directory for purge")
+	if err = os.MkdirAll(dataPath, 0755); err != nil {
+		t.Fatal("Could not create test directory for purge")
+	} else if err = ioutil.WriteFile(dataPath+"/test", []byte("test"), 777); err != nil {
+		t.Fatal("Could not create test file for purge")
 	}
 
-	err = ioutil.WriteFile(dataPath+"/test", []byte("test"), 777)
-	if err != nil {
-		t.Error("Could not create test file for purge")
-	}
+	PurgeHandler(writer, request)
 
-	PurgeHandler(w, req)
-
-	if w.Code != http.StatusOK {
+	if writer.Code != http.StatusOK {
 		t.Errorf("Purge didn't return %v", http.StatusOK)
 	}
-	if !strings.EqualFold(w.Body.String(), `{"Status":"OK","Error":""}`) {
-		t.Errorf("Purge response didn't match the expected JSON, got: %s", w.Body.String())
+	if !strings.EqualFold(writer.Body.String(), `{"Status":"OK","Error":""}`) {
+		t.Errorf("Purge response didn't match the expected JSON, got: %s", writer.Body.String())
 	}
 
-	dirContents, err := ioutil.ReadDir(dataPath)
-	if err != nil {
+	if dirContents, err := ioutil.ReadDir(dataPath); err != nil {
 		t.Errorf("Could not read the data path after purge: %s", err)
-	}
-	if len(dirContents) > 0 {
+	} else if len(dirContents) > 0 {
 		t.Error("Data directory not empty after purge")
 	}
 }
 
 func TestReadConfig(t *testing.T) {
-	config, err := ReadConfig("config_for_test.json")
-	if err != nil {
+	if config, err := ReadConfig("config_for_test.json"); err != nil {
 		t.Error(err)
-	}
-
-	if !strings.EqualFold(config.ApplicationId, "1939") || !strings.EqualFold(config.ApiKey, "SuperSecretAPIKey") {
+	} else if !strings.EqualFold(config.ApplicationId, "1939") || !strings.EqualFold(config.ApiKey, "SuperSecretAPIKey") {
 		t.Error("Config not parsed correctly")
 	}
 }

gosuper/gosuper/purge.go

@@ -3,9 +3,9 @@ package main
 import (
 	"encoding/json"
 	"fmt"
+	"log"
 	"net/http"
 	"os"
-	"strconv"
 )
 
 type ApiResponse struct {
@@ -17,56 +17,52 @@ type PurgeBody struct {
 	ApplicationId string
 }
 
-func jsonResponse(w http.ResponseWriter, response interface{}, status int) {
-	j, _ := json.Marshal(response)
-	w.Header().Set("Content-Type", "application/json")
-	w.WriteHeader(status)
-	w.Write(j)
+func jsonResponse(writer http.ResponseWriter, response interface{}, status int) {
+	jsonBody, err := json.Marshal(response)
+	if err != nil {
+		log.Printf("Could not marshal JSON for %+v\n", response)
+	}
+	writer.Header().Set("Content-Type", "application/json")
+	writer.WriteHeader(status)
+	writer.Write(jsonBody)
 }
 
-func parseJsonBody(r *http.Request, dest interface{}) error {
-	decoder := json.NewDecoder(r.Body)
-	err := decoder.Decode(&dest)
-	return err
+func parseJsonBody(destination interface{}, request *http.Request) error {
+	decoder := json.NewDecoder(request.Body)
+	return decoder.Decode(&destination)
 }
 
-func PurgeHandler(w http.ResponseWriter, r *http.Request) {
-	fmt.Println("Purging /data")
+func PurgeHandler(writer http.ResponseWriter, request *http.Request) {
+	log.Println("Purging /data")
 	var body PurgeBody
-	err := parseJsonBody(r, &body)
-	if err != nil {
-		jsonResponse(w, ApiResponse{"Error", "Invalid request"}, 422)
-		return
+
+	sendResponse := func(statusMsg, errorMsg string, statusCode int) {
+		jsonResponse(writer, ApiResponse{statusMsg, errorMsg}, statusCode)
+	}
+	sendError := func(err error) {
+		sendResponse("Error", err.Error(), http.StatusInternalServerError)
+	}
+	sendBadRequest := func(errorMsg string) {
+		sendResponse("Error", errorMsg, http.StatusBadRequest)
 	}
 
-	appId := body.ApplicationId
-	if appId == "" {
-		jsonResponse(w, ApiResponse{"Error", "applicationId is required"}, 422)
-		return
+	if err := parseJsonBody(&body, request); err != nil {
+		sendBadRequest("Invalid request")
+	} else if appId := body.ApplicationId; appId == "" {
+		sendBadRequest("applicationId is required")
+	} else if !IsValidAppId(appId) {
+		sendBadRequest(fmt.Sprintf("Invalid applicationId '%s'", appId))
+	} else if _, err = os.Stat(ResinDataPath + appId); err != nil {
+		if os.IsNotExist(err) {
+			sendBadRequest(fmt.Sprintf("Invalid applicationId '%s': Directory does not exist", appId))
+		} else {
+			sendError(err)
+		}
+	} else if err = os.RemoveAll(ResinDataPath + appId); err != nil {
+		sendError(err)
+	} else if err = os.Mkdir(ResinDataPath+appId, 0755); err != nil {
+		sendError(err)
+	} else {
+		sendResponse("OK", "", http.StatusOK)
 	}
-
-	// Validate that the appId is an integer
-	_, err = strconv.ParseInt(appId, 10, 0)
-	if err != nil {
-		jsonResponse(w, ApiResponse{"Error", "Invalid applicationId"}, 422)
-		return
-	}
-
-	directory := ResinDataPath + appId
-
-	err = os.RemoveAll(directory)
-
-	if err != nil {
-		jsonResponse(w, ApiResponse{"Error", err.Error()}, 500)
-		return
-	}
-
-	err = os.Mkdir(directory, 0755)
-	if err != nil {
-		jsonResponse(w, ApiResponse{"Error", err.Error()}, 500)
-		return
-	}
-
-	jsonResponse(w, ApiResponse{"OK", ""}, 200)
-
 }

gosuper/gosuper/validation.go

@@ -0,0 +1,11 @@
+package main
+
+import (
+	"strconv"
+)
+
+func IsValidAppId(appId string) (valid bool) {
+	_, err := strconv.ParseUint(appId, 10, 0)
+	valid = err == nil
+	return
+}

gosuper/supertest/super_test.go

@@ -15,76 +15,63 @@ var supervisorAddress string
 var config gosuper.UserConfig
 
 func TestMain(m *testing.M) {
-	supervisorIP := os.Getenv("SUPERVISOR_IP")
-	if supervisorIP == "" {
-		log.Fatal("Supervisor IP not set - is it running?")
-	}
-
-	supervisorAddress = "http://" + supervisorIP + ":48484"
-
-	gopath := os.Getenv("GOPATH")
-
 	var err error
-	config, err = gosuper.ReadConfig(gopath + "/src/resin-supervisor/gosuper/config.json")
-	if err != nil {
+	if gopath := os.Getenv("GOPATH"); gopath == "" {
+		log.Fatal("GOPATH is not set - where are you running this?")
+	} else if supervisorIP := os.Getenv("SUPERVISOR_IP"); supervisorIP == "" {
+		log.Fatal("Supervisor IP not set - is it running?")
+	} else if config, err = gosuper.ReadConfig(gopath + "/src/resin-supervisor/gosuper/config.json"); err != nil {
 		log.Fatal(err)
+	} else {
+		supervisorAddress = "http://" + supervisorIP + ":48484"
+		os.Exit(m.Run())
 	}
-
-	os.Exit(m.Run())
 }
 
 func TestPing(t *testing.T) {
-	request, err := http.NewRequest("GET", supervisorAddress+"/ping?apikey=bananas", nil)
-	if err != nil {
+	if request, err := http.NewRequest("GET", supervisorAddress+"/ping?apikey=bananas", nil); err != nil {
 		t.Fatal(err)
-	}
-
-	res, err := http.DefaultClient.Do(request)
-	if err != nil {
+	} else if response, err := http.DefaultClient.Do(request); err != nil {
 		t.Fatal(err)
-	}
-
-	if res.StatusCode != 200 {
-		t.Fatalf("Expected 200, got %d", res.StatusCode)
+	} else if response.StatusCode != http.StatusOK {
+		t.Fatalf("Expected 200, got %d", response.StatusCode)
 	}
 }
 
 func TestPurge(t *testing.T) {
 	appId := config.ApplicationId
 	dataPath := "/resin-data/" + appId
-	err := ioutil.WriteFile(dataPath+"/test", []byte("test"), 777)
-	if err != nil {
-		t.Error("Could not create test file for purge")
-	}
 
-	request, err := http.NewRequest("POST", supervisorAddress+"/v1/purge?apikey=bananas", strings.NewReader(`{"appId": "`+appId+`"}`))
-	request.Header.Set("Content-Type", "application/json")
-	if err != nil {
+	if err := ioutil.WriteFile(dataPath+"/test", []byte("test"), 777); err != nil {
+		t.Fatal("Could not create test file for purge")
+	} else if request, err := http.NewRequest("POST", supervisorAddress+"/v1/purge?apikey=bananas", strings.NewReader(`{"appId": "`+appId+`"}`)); err != nil {
 		t.Fatal(err)
-	}
+	} else {
+		request.Header.Set("Content-Type", "application/json")
 
-	res, err := http.DefaultClient.Do(request)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if res.StatusCode != 200 {
-		t.Fatalf("Expected 200, got %d", res.StatusCode)
-	}
+		if response, err := http.DefaultClient.Do(request); err != nil {
+			t.Fatal(err)
+		} else if response.StatusCode != http.StatusOK {
+			t.Errorf("Expected 200, got %d", response.StatusCode)
+			defer response.Body.Close()
+			if contents, err := ioutil.ReadAll(response.Body); err != nil {
+				t.Fatal(err)
+			} else {
+				t.Fatalf("Response: %s", contents)
+			}
+		} else {
+			defer response.Body.Close()
+			if contents, err := ioutil.ReadAll(response.Body); err != nil {
+				t.Fatal(err)
+			} else if !strings.EqualFold(string(contents), `{"Status":"OK","Error":""}`) {
+				t.Errorf("Purge response didn't match the expected JSON, got: %s", contents)
+			}
 
-	defer res.Body.Close()
-	contents, err := ioutil.ReadAll(res.Body)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if !strings.EqualFold(string(contents), `{"Status":"OK","Error":""}`) {
-		t.Errorf("Purge response didn't match the expected JSON, got: %s", contents)
-	}
-
-	dirContents, err := ioutil.ReadDir(dataPath)
-	if err != nil {
-		t.Errorf("Could not read the data path after purge: %s", err)
-	}
-	if len(dirContents) > 0 {
-		t.Error("Data directory not empty after purge")
+			if dirContents, err := ioutil.ReadDir(dataPath); err != nil {
+				t.Errorf("Could not read the data path after purge: %s", err)
+			} else if len(dirContents) > 0 {
+				t.Error("Data directory not empty after purge")
+			}
+		}
 	}
 }

package.json

@@ -24,6 +24,7 @@
     "randomstring": "~1.0.3",
     "request": "^2.51.0",
     "resin-register-device": "^1.0.1",
+    "rwlock": "^5.0.0",
     "server-destroy": "^1.0.0",
     "sqlite3": "~3.0.4",
     "tty.js": "0.2.14-1",

src/api.coffee

@@ -37,7 +37,7 @@ module.exports = (secret) ->
 		utils.mixpanelTrack('Spawn tty', appId)
 		if !appId?
 			return res.status(400).send('Missing app id')
-		knex('app').select().where({appId})
+		knex('app').select().where({ appId })
 		.then ([ app ]) ->
 			if !app?
 				throw new Error('App not found')
@@ -52,7 +52,7 @@ module.exports = (secret) ->
 		utils.mixpanelTrack('Despawn tty', appId)
 		if !appId?
 			return res.status(400).send('Missing app id')
-		knex('app').select().where({appId})
+		knex('app').select().where({ appId })
 		.then ([ app ]) ->
 			if !app?
 				throw new Error('App not found')
@@ -67,14 +67,20 @@ module.exports = (secret) ->
 		utils.mixpanelTrack('Purge /data', appId)
 		if !appId?
 			return res.status(400).send('Missing app id')
-		knex('app').select().where({appId})
-		.then ([ app ]) ->
-			if !app?
+		app = null
+		knex('app').select().where({ appId })
+		.then ([ appFromDB ]) ->
+			if !appFromDB?
 				throw new Error('App not found')
+			app = appFromDB
+			application.lockUpdatesAsync()
+		.tap ->
 			application.kill(app)
-		.then ->
-			request.post config.gosuperAddress + '/v1/purge', {json: true, body: applicationId: appId}, ->
+		.then (release) ->
+			request.post config.gosuperAddress + '/v1/purge', { json: true, body: applicationId: appId }, ->
 				application.start(app)
+				.then ->
+					release()
 			.pipe(res)
 		.catch (err) ->
 			res.status(503).send(err?.message or err or 'Unknown error')

src/application.coffee

@@ -1,5 +1,6 @@
 _ = require 'lodash'
 url = require 'url'
+Lock = require 'rwlock'
 knex = require './db'
 path = require 'path'
 config = require './config'
@@ -13,7 +14,6 @@ device = require './device'
 
 { docker } = dockerUtils
 
-
 knex('config').select('value').where(key: 'uuid').then ([ uuid ]) ->
 	logger.init(
 		dockerSocket: config.dockerSocket
@@ -242,6 +242,10 @@ getEnvironment = do ->
 			console.error("Failed to get environment for device #{deviceId}, app #{appId}. #{err}")
 			throw err
 
+lock = new Lock()
+exports.lockUpdates = lockUpdates = lock.async.writeLock
+exports.lockUpdatesAsync = lockUpdatesAsync = Promise.promisify(lockUpdates)
+
 # 0 - Idle
 # 1 - Updating
 # 2 - Update required
@@ -321,10 +325,12 @@ exports.update = update = ->
 				app = remoteApps[imageId]
 				fetch(app)
 			.then ->
+				lockUpdatesAsync()
+			.tap ->
 				# Then delete all the ones to remove in one go
 				Promise.map toBeRemoved, (imageId) ->
 					kill(apps[imageId])
-			.then ->
+			.tap ->
 				# Then install the apps and add each to the db as they succeed
 				installingPromises = toBeInstalled.map (imageId) ->
 					app = remoteApps[imageId]
@@ -338,6 +344,8 @@ exports.update = update = ->
 					.then ->
 						start(app)
 				Promise.all(installingPromises.concat(updatingPromises))
+			.then (release) ->
+				release()
 	.then ->
 		failedUpdates = 0
 		# We cleanup here as we want a point when we have a consistent apps/images state, rather than potentially at a

tools/dind/config/env

@@ -5,6 +5,7 @@ PUBNUB_SUBSCRIBE_KEY=sub-c-bananas
 PUBNUB_PUBLISH_KEY=pub-c-bananas
 MIXPANEL_TOKEN=bananasbananas
 LISTEN_PORT=48484
+RESIN_SUPERVISOR_SECRET=bananas
 
 SUPERVISOR_IMAGE=
 LED_FILE=/dev/null