Merge branch 'handle-malformed-pid' into 'master'

Handle malformed PID file in gitaly-wrapper

See merge request gitlab-org/gitaly!1825

3 files changed, 48 insertions, 2 deletions

diff --git a/changelogs/unreleased/handle-malformed-pid.yml b/changelogs/unreleased/handle-malformed-pid.yml
new file mode 100644
index 000000000..60ccec633
--- /dev/null
+++ b/changelogs/unreleased/handle-malformed-pid.yml
@@ -0,0 +1,5 @@
+---
+title: Handle malformed PID file
+merge_request: 1825
+author: maxmati
+type: fixed
diff --git a/cmd/gitaly-wrapper/main.go b/cmd/gitaly-wrapper/main.go
index ec0d2c522..20d491058 100644
--- a/cmd/gitaly-wrapper/main.go
+++ b/cmd/gitaly-wrapper/main.go
@@ -40,8 +40,10 @@ func main() {
 
 	log.WithField("pid_file", pidFile()).Info("finding gitaly")
 	gitaly, err := findGitaly()
-	if err != nil {
+	if err != nil && !isRecoverable(err) {
 		log.WithError(err).Fatal("find gitaly")
+	} else if err != nil {
+		log.WithError(err).Error("find gitaly")
 	}
 
 	if gitaly != nil && isGitaly(gitaly, gitalyBin) {
@@ -70,9 +72,14 @@ func main() {
 	log.Error("wrapper for gitaly shutting down")
 }
 
+func isRecoverable(err error) bool {
+	_, isNumError := err.(*strconv.NumError)
+	return os.IsNotExist(err) || isNumError
+}
+
 func findGitaly() (*os.Process, error) {
 	pid, err := getPid()
-	if err != nil && !os.IsNotExist(err) {
+	if err != nil {
 		return nil, err
 	}
 
diff --git a/cmd/gitaly-wrapper/main_test.go b/cmd/gitaly-wrapper/main_test.go
index 2b5778142..59963da52 100644
--- a/cmd/gitaly-wrapper/main_test.go
+++ b/cmd/gitaly-wrapper/main_test.go
@@ -1,6 +1,7 @@
 package main
 
 import (
+	"errors"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -44,3 +45,36 @@ func TestStolenPid(t *testing.T) {
 		require.True(t, isGitaly(tail, "/path/to/tail"))
 	})
 }
+
+func TestIsRecoverable(t *testing.T) {
+	_, numericError := strconv.Atoi("")
+
+	tests := []struct {
+		name string
+		err  error
+		want bool
+	}{
+		{
+			name: "file doesn't exist",
+			err:  os.ErrNotExist,
+			want: true,
+		},
+		{
+			name: "numeric error",
+			err:  numericError,
+			want: true,
+		},
+		{
+			name: "generic error",
+			err:  errors.New("generic error"),
+			want: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := isRecoverable(tt.err); got != tt.want {
+				t.Errorf("isRecoverable() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}