From dc8d8ff39e84625fda4326efec62afcfb119a93d Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Mon, 8 Sep 2025 16:54:25 +0100 Subject: [PATCH 1/3] wait for one worker to be reloaded --- internal/resource/nginx_instance_operator.go | 18 ++++----- .../resource/nginx_instance_operator_test.go | 38 ++++++++++++++++++- 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/internal/resource/nginx_instance_operator.go b/internal/resource/nginx_instance_operator.go index ee5063742..2fa4c226c 100644 --- a/internal/resource/nginx_instance_operator.go +++ b/internal/resource/nginx_instance_operator.go @@ -62,7 +62,7 @@ func (i *NginxInstanceOperator) Validate(ctx context.Context, instance *mpi.Inst } func (i *NginxInstanceOperator) Reload(ctx context.Context, instance *mpi.Instance) error { - var reloadTime time.Time + var createdTime time.Time var errorsFound error pid := instance.GetInstanceRuntime().GetProcessId() @@ -72,7 +72,7 @@ func (i *NginxInstanceOperator) Reload(ctx context.Context, instance *mpi.Instan workers := i.nginxProcessOperator.NginxWorkerProcesses(ctx, pid) if len(workers) > 0 { - reloadTime = workers[0].Created + createdTime = workers[0].Created } errorLogs := i.errorLogs(instance) @@ -92,7 +92,7 @@ func (i *NginxInstanceOperator) Reload(ctx context.Context, instance *mpi.Instan slog.WarnContext(ctx, "Error finding parent process ID, unable to check if NGINX worker "+ "processes have reloaded", "error", procErr) } else { - i.checkWorkers(ctx, instance.GetInstanceMeta().GetInstanceId(), reloadTime, processes) + i.checkWorkers(ctx, instance.GetInstanceMeta().GetInstanceId(), createdTime, processes) } slog.InfoContext(ctx, "NGINX reloaded", "process_id", pid) @@ -117,7 +117,7 @@ func (i *NginxInstanceOperator) Reload(ctx context.Context, instance *mpi.Instan return nil } -func (i *NginxInstanceOperator) checkWorkers(ctx context.Context, instanceID string, reloadTime time.Time, +func (i *NginxInstanceOperator) checkWorkers(ctx context.Context, instanceID string, createdTime time.Time, processes []*nginxprocess.Process, ) { backoffSettings := &config.BackOff{ @@ -148,13 +148,13 @@ func (i *NginxInstanceOperator) checkWorkers(ctx context.Context, instanceID str } for _, worker := range currentWorkers { - if !worker.Created.After(reloadTime) { - return fmt.Errorf("waiting for all NGINX workers to be newer "+ - "than %v, found worker with time %v", reloadTime, worker.Created) + if worker.Created.After(createdTime) { + return nil } } - return nil + return fmt.Errorf("waiting for NGINX worker to be newer "+ + "than %v", createdTime) }) if err != nil { slog.WarnContext(ctx, "Failed to check if NGINX worker processes have successfully reloaded, "+ @@ -163,7 +163,7 @@ func (i *NginxInstanceOperator) checkWorkers(ctx context.Context, instanceID str return } - slog.InfoContext(ctx, "All NGINX workers have been reloaded") + slog.InfoContext(ctx, "NGINX workers have been reloaded") } func (i *NginxInstanceOperator) validateConfigCheckResponse(out []byte) error { diff --git a/internal/resource/nginx_instance_operator_test.go b/internal/resource/nginx_instance_operator_test.go index 8947a849b..d558229cc 100644 --- a/internal/resource/nginx_instance_operator_test.go +++ b/internal/resource/nginx_instance_operator_test.go @@ -315,7 +315,41 @@ func TestInstanceOperator_checkWorkers(t *testing.T) { }, }, { - name: "Test 2: Unsuccessful reload", + name: "Test 2: Successful reload - one workers is reloaded", + expectedLog: "All NGINX workers have been reloaded", + reloadTime: time.Date(2025, 8, 13, 8, 0, 0, 0, time.Local), + instanceID: "e1374cb1-462d-3b6c-9f3b-f28332b5f10c", + masterProcess: []*nginxprocess.Process{ + { + PID: 1234, + Created: time.Date(2025, 8, 13, 8, 1, 0, 0, time.Local), + PPID: 1, + Name: "nginx", + Cmd: "nginx: master process /usr/local/opt/nginx/bin/nginx -g daemon off;", + Exe: exePath, + }, + }, + workers: []*nginxprocess.Process{ + { + PID: 567, + Created: time.Date(2025, 8, 13, 8, 1, 0, 0, time.Local), + PPID: 1234, + Name: "nginx", + Cmd: "nginx: worker process", + Exe: exePath, + }, + { + PID: 789, + PPID: 1234, + Created: time.Date(2025, 8, 13, 7, 1, 0, 0, time.Local), + Name: "nginx", + Cmd: "nginx: worker process", + Exe: exePath, + }, + }, + }, + { + name: "Test 3: Unsuccessful reload", expectedLog: "\"Failed to check if NGINX worker processes have successfully reloaded, timed out " + "waiting\" error=\"waiting for NGINX worker processes\"", reloadTime: time.Date(2025, 8, 13, 8, 0, 0, 0, time.Local), @@ -333,7 +367,7 @@ func TestInstanceOperator_checkWorkers(t *testing.T) { workers: []*nginxprocess.Process{ { PID: 567, - Created: time.Date(2025, 8, 13, 8, 1, 0, 0, time.Local), + Created: time.Date(2025, 8, 13, 7, 1, 0, 0, time.Local), PPID: 1234, Name: "nginx", Cmd: "nginx: worker process", From fb76265f9acdc4d91543f50bfb9977623a1e3135 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Tue, 9 Sep 2025 10:33:05 +0100 Subject: [PATCH 2/3] wait for one worker to be reloaded --- internal/resource/nginx_instance_operator_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/resource/nginx_instance_operator_test.go b/internal/resource/nginx_instance_operator_test.go index d558229cc..3e757cfc9 100644 --- a/internal/resource/nginx_instance_operator_test.go +++ b/internal/resource/nginx_instance_operator_test.go @@ -282,7 +282,7 @@ func TestInstanceOperator_checkWorkers(t *testing.T) { }{ { name: "Test 1: Successful reload", - expectedLog: "All NGINX workers have been reloaded", + expectedLog: "NGINX workers have been reloaded", reloadTime: time.Date(2025, 8, 13, 8, 0, 0, 0, time.Local), instanceID: "e1374cb1-462d-3b6c-9f3b-f28332b5f10c", workers: []*nginxprocess.Process{ @@ -316,7 +316,7 @@ func TestInstanceOperator_checkWorkers(t *testing.T) { }, { name: "Test 2: Successful reload - one workers is reloaded", - expectedLog: "All NGINX workers have been reloaded", + expectedLog: "NGINX workers have been reloaded", reloadTime: time.Date(2025, 8, 13, 8, 0, 0, 0, time.Local), instanceID: "e1374cb1-462d-3b6c-9f3b-f28332b5f10c", masterProcess: []*nginxprocess.Process{ From 8da828e35708b7db5e393889f676e8f391879eb5 Mon Sep 17 00:00:00 2001 From: Aphral Griffin Date: Tue, 9 Sep 2025 14:00:55 +0100 Subject: [PATCH 3/3] update defaults --- internal/config/defaults.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/config/defaults.go b/internal/config/defaults.go index 6a464d54e..cb456bfb0 100644 --- a/internal/config/defaults.go +++ b/internal/config/defaults.go @@ -17,11 +17,11 @@ const ( DefNginxApiTlsCa = "" // Nginx Reload Backoff defaults - DefNginxReloadBackoffInitialInterval = 1 * time.Second + DefNginxReloadBackoffInitialInterval = 500 * time.Millisecond DefNginxReloadBackoffRandomizationFactor = 0.5 // the value is 0 <= and < 1 - DefNginxReloadBackoffMultiplier = 5 - DefNginxReloadBackoffMaxInterval = 10 * time.Second - DefNginxReloadBackoffMaxElapsedTime = 30 * time.Second + DefNginxReloadBackoffMultiplier = 2 + DefNginxReloadBackoffMaxInterval = 3 * time.Second + DefNginxReloadBackoffMaxElapsedTime = 10 * time.Second DefCommandServerHostKey = "" DefCommandServerPortKey = 0