diff --git a/internal/nginx/runtime/manager.go b/internal/nginx/runtime/manager.go index 4ef3c76a7c..4c4cddda49 100644 --- a/internal/nginx/runtime/manager.go +++ b/internal/nginx/runtime/manager.go @@ -2,6 +2,7 @@ package runtime import ( "context" + "errors" "fmt" "io/fs" "os" @@ -9,6 +10,8 @@ import ( "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 } diff --git a/internal/nginx/runtime/manager_test.go b/internal/nginx/runtime/manager_test.go index 9e04d89196..c5ff89b041 100644 --- a/internal/nginx/runtime/manager_test.go +++ b/internal/nginx/runtime/manager_test.go @@ -1,6 +1,7 @@ package runtime import ( + "context" "errors" "io/fs" "testing" @@ -32,8 +33,12 @@ 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 @@ -41,6 +46,7 @@ func TestFindMainProcess(t *testing.T) { expectError bool }{ { + ctx: ctx, readFile: readFileFuncGen([]byte("1\n")), checkFile: checkFileFuncGen(testFileInfo), expected: 1, @@ -48,6 +54,7 @@ func TestFindMainProcess(t *testing.T) { msg: "normal case", }, { + ctx: ctx, readFile: readFileFuncGen([]byte("")), checkFile: checkFileFuncGen(testFileInfo), expected: 0, @@ -55,6 +62,7 @@ func TestFindMainProcess(t *testing.T) { msg: "empty file content", }, { + ctx: ctx, readFile: readFileFuncGen([]byte("not a number")), checkFile: checkFileFuncGen(testFileInfo), expected: 0, @@ -62,6 +70,7 @@ func TestFindMainProcess(t *testing.T) { msg: "bad file content", }, { + ctx: ctx, readFile: readFileError, checkFile: checkFileFuncGen(testFileInfo), expected: 0, @@ -69,16 +78,25 @@ func TestFindMainProcess(t *testing.T) { 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)