Skip to content

Commit

Permalink
Add timeout and retry logic for the NGINX pid file
Browse files Browse the repository at this point in the history
  • Loading branch information
ciarams87 committed Jun 12, 2023
1 parent ec53c9d commit a864dc1
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 14 deletions.
39 changes: 27 additions & 12 deletions internal/nginx/runtime/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@ package runtime

import (
"context"
"errors"
"fmt"
"io/fs"
"os"
"strconv"
"strings"
"syscall"
"time"

"k8s.io/apimachinery/pkg/util/wait"
)

const (
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}
Expand Down
22 changes: 20 additions & 2 deletions internal/nginx/runtime/manager_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package runtime

import (
"context"
"errors"
"io/fs"
"testing"
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit a864dc1

Please sign in to comment.