Add timeout and retry logic for the NGINX pid file

Author: ciarams87

internal/nginx/runtime/manager.go

@@ -2,13 +2,16 @@ package runtime
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io/fs"
 	"os"
 	"strconv"
 	"strings"
 	"syscall"
 	"time"
+
+	"k8s.io/apimachinery/pkg/util/wait"
 )
 
 const (
@@ -39,7 +42,7 @@ func NewManagerImpl() *ManagerImpl {
 
 func (m *ManagerImpl) Reload(ctx context.Context) error {
 	// We find the main NGINX PID on every reload because it will change if the NGINX container is restarted.
-	pid, err := findMainProcess(os.Stat, os.ReadFile, pidFileTimeout)
+	pid, err := findMainProcess(ctx, os.Stat, os.ReadFile, pidFileTimeout)
 	if err != nil {
 		return fmt.Errorf("failed to find NGINX main process: %w", err)
 	}
@@ -67,22 +70,34 @@ func (m *ManagerImpl) Reload(ctx context.Context) error {
 	return nil
 }
 
-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) {
+func findMainProcess(
+	ctx context.Context,
+	checkFile checkFileFunc,
+	readFile readFileFunc,
+	timeout time.Duration,
+) (int, error) {
+	ctx, cancel := context.WithTimeout(ctx, timeout)
+	defer cancel()
+
+	err := wait.PollUntilContextCancel(
+		ctx,
+		1*time.Second,
+		true, /* poll immediately */
+		func(ctx context.Context) (bool, error) {
 			_, err := checkFile(pidFile)
 			if err == nil {
-				return readFile(pidFile)
+				return true, nil
+			}
+			if !errors.Is(err, fs.ErrNotExist) {
+				return false, err
 			}
-			time.Sleep(1 * time.Second)
-		}
-		return nil, fmt.Errorf("timeout waiting for pid file to appear")
+			return false, nil
+		})
+	if err != nil {
+		return 0, err
 	}
 
-	content, err := fileCheck()
+	content, err := readFile(pidFile)
 	if err != nil {
 		return 0, err
 	}

internal/nginx/runtime/manager_test.go

@@ -1,6 +1,7 @@
 package runtime
 
 import (
+	"context"
 	"errors"
 	"io/fs"
 	"testing"
@@ -32,53 +33,70 @@ func TestFindMainProcess(t *testing.T) {
 		return nil, errors.New("error")
 	}
 	var testFileInfo fs.FileInfo
+	ctx := context.Background()
+	cancellingCtx, cancel := context.WithCancel(ctx)
+	time.AfterFunc(1*time.Millisecond, cancel)
 
 	tests := []struct {
+		ctx         context.Context
 		readFile    readFileFunc
 		checkFile   checkFileFunc
 		msg         string
 		expected    int
 		expectError bool
 	}{
 		{
+			ctx:         ctx,
 			readFile:    readFileFuncGen([]byte("1\n")),
 			checkFile:   checkFileFuncGen(testFileInfo),
 			expected:    1,
 			expectError: false,
 			msg:         "normal case",
 		},
 		{
+			ctx:         ctx,
 			readFile:    readFileFuncGen([]byte("")),
 			checkFile:   checkFileFuncGen(testFileInfo),
 			expected:    0,
 			expectError: true,
 			msg:         "empty file content",
 		},
 		{
+			ctx:         ctx,
 			readFile:    readFileFuncGen([]byte("not a number")),
 			checkFile:   checkFileFuncGen(testFileInfo),
 			expected:    0,
 			expectError: true,
 			msg:         "bad file content",
 		},
 		{
+			ctx:         ctx,
 			readFile:    readFileError,
 			checkFile:   checkFileFuncGen(testFileInfo),
 			expected:    0,
 			expectError: true,
 			msg:         "cannot read file",
 		},
 		{
+			ctx:         ctx,
 			readFile:    readFileFuncGen([]byte("1\n")),
 			checkFile:   checkFileError,
 			expected:    0,
 			expectError: true,
-			msg:         "cannot file pid file",
+			msg:         "cannot find pid file",
+		},
+		{
+			ctx:         cancellingCtx,
+			readFile:    readFileFuncGen([]byte("1\n")),
+			checkFile:   checkFileError,
+			expected:    0,
+			expectError: true,
+			msg:         "context canceled",
 		},
 	}
 
 	for _, test := range tests {
-		result, err := findMainProcess(test.checkFile, test.readFile, 1*time.Microsecond)
+		result, err := findMainProcess(test.ctx, test.checkFile, test.readFile, 2*time.Millisecond)
 
 		if result != test.expected {
 			t.Errorf("findMainProcess() returned %d but expected %d for case %q", result, test.expected, test.msg)