Add timeout and retry logic for the NGINX pid file

Author: ciarams87

internal/nginx/runtime/manager.go

@@ -3,16 +3,23 @@ package runtime
 import (
 	"context"
 	"fmt"
+	"io/fs"
 	"os"
 	"strconv"
 	"strings"
 	"syscall"
 	"time"
 )
 
-const pidFile = "/etc/nginx/nginx.pid"
+const (
+	pidFile        = "/etc/nginx/nginx.pid"
+	pidFileTimeout = 5 * time.Second
+)
 
-type readFileFunc func(string) ([]byte, error)
+type (
+	readFileFunc  func(string) ([]byte, error)
+	checkFileFunc func(string) (fs.FileInfo, error)
+)
 
 //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . Manager
 
@@ -31,13 +38,8 @@ func NewManagerImpl() *ManagerImpl {
 }
 
 func (m *ManagerImpl) Reload(ctx context.Context) error {
-	// FIXME(pleshakov): Before reload attempt, make sure NGINX is running.
-	// If the gateway container starts before NGINX container (which is possible),
-	// then it is possible that a reload can be attempted when NGINX is not running yet.
-	// Make sure to prevent this case, so we don't get an error.
-
 	// We find the main NGINX PID on every reload because it will change if the NGINX container is restarted.
-	pid, err := findMainProcess(os.ReadFile)
+	pid, err := findMainProcess(os.Stat, os.ReadFile, pidFileTimeout)
 	if err != nil {
 		return fmt.Errorf("failed to find NGINX main process: %w", err)
 	}
@@ -65,8 +67,22 @@ func (m *ManagerImpl) Reload(ctx context.Context) error {
 	return nil
 }
 
-func findMainProcess(readFile readFileFunc) (int, error) {
-	content, err := readFile(pidFile)
+func findMainProcess(checkFile checkFileFunc, readFile readFileFunc, timeout time.Duration) (int, error) {
+	startTime := time.Now()
+	deadline := startTime.Add(timeout)
+
+	fileCheck := func() (content []byte, err error) {
+		for time.Now().Before(deadline) {
+			_, err := checkFile(pidFile)
+			if err == nil {
+				return readFile(pidFile)
+			}
+			time.Sleep(1 * time.Second)
+		}
+		return nil, fmt.Errorf("timeout waiting for pid file to appear")
+	}
+
+	content, err := fileCheck()
 	if err != nil {
 		return 0, err
 	}

internal/nginx/runtime/manager_test.go

@@ -2,7 +2,9 @@ package runtime
 
 import (
 	"errors"
+	"io/fs"
 	"testing"
+	"time"
 )
 
 func TestFindMainProcess(t *testing.T) {
@@ -18,40 +20,65 @@ func TestFindMainProcess(t *testing.T) {
 		return nil, errors.New("error")
 	}
 
+	checkFileFuncGen := func(content fs.FileInfo) checkFileFunc {
+		return func(name string) (fs.FileInfo, error) {
+			if name != pidFile {
+				return nil, errors.New("error")
+			}
+			return content, nil
+		}
+	}
+	checkFileError := func(string) (fs.FileInfo, error) {
+		return nil, errors.New("error")
+	}
+	var testFileInfo fs.FileInfo
+
 	tests := []struct {
 		readFile    readFileFunc
+		checkFile   checkFileFunc
 		msg         string
 		expected    int
 		expectError bool
 	}{
 		{
 			readFile:    readFileFuncGen([]byte("1\n")),
+			checkFile:   checkFileFuncGen(testFileInfo),
 			expected:    1,
 			expectError: false,
 			msg:         "normal case",
 		},
 		{
 			readFile:    readFileFuncGen([]byte("")),
+			checkFile:   checkFileFuncGen(testFileInfo),
 			expected:    0,
 			expectError: true,
 			msg:         "empty file content",
 		},
 		{
 			readFile:    readFileFuncGen([]byte("not a number")),
+			checkFile:   checkFileFuncGen(testFileInfo),
 			expected:    0,
 			expectError: true,
 			msg:         "bad file content",
 		},
 		{
 			readFile:    readFileError,
+			checkFile:   checkFileFuncGen(testFileInfo),
 			expected:    0,
 			expectError: true,
 			msg:         "cannot read file",
 		},
+		{
+			readFile:    readFileFuncGen([]byte("1\n")),
+			checkFile:   checkFileError,
+			expected:    0,
+			expectError: true,
+			msg:         "cannot file pid file",
+		},
 	}
 
 	for _, test := range tests {
-		result, err := findMainProcess(test.readFile)
+		result, err := findMainProcess(test.checkFile, test.readFile, 1*time.Microsecond)
 
 		if result != test.expected {
 			t.Errorf("findMainProcess() returned %d but expected %d for case %q", result, test.expected, test.msg)