From 65f6bd30f429291a2cacba40bd4c387ff3969fb9 Mon Sep 17 00:00:00 2001 From: Mile Druzijanic Date: Sun, 7 Jul 2024 15:28:59 +0200 Subject: [PATCH 01/24] add tests for runtime manager --- .../mode/static/nginx/runtime/manager_test.go | 36 ++++++ .../nginx/runtime/runtime_manager_test.go | 107 ++++++++++++++++++ 2 files changed, 143 insertions(+) create mode 100644 internal/mode/static/nginx/runtime/runtime_manager_test.go diff --git a/internal/mode/static/nginx/runtime/manager_test.go b/internal/mode/static/nginx/runtime/manager_test.go index 323bb514a5..d6ff923c02 100644 --- a/internal/mode/static/nginx/runtime/manager_test.go +++ b/internal/mode/static/nginx/runtime/manager_test.go @@ -23,6 +23,42 @@ var _ = Describe("NGINX Runtime Manager", func() { }) }) +func TestEnsureNginxRunning(t *testing.T) { + ctx := context.Background() + cancellingCtx, cancel := context.WithCancel(ctx) + time.AfterFunc(1*time.Millisecond, cancel) + tests := []struct { + ctx context.Context + name string + expectError bool + }{ + { + ctx: ctx, + name: "context exceeded", + expectError: true, + }, + { + ctx: cancellingCtx, + name: "context cancelled", + expectError: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + err := EnsureNginxRunning(test.ctx) + + if test.expectError { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + }) + } +} + func TestFindMainProcess(t *testing.T) { readFileFuncGen := func(content []byte) readFileFunc { return func(name string) ([]byte, error) { diff --git a/internal/mode/static/nginx/runtime/runtime_manager_test.go b/internal/mode/static/nginx/runtime/runtime_manager_test.go new file mode 100644 index 0000000000..86129ef74d --- /dev/null +++ b/internal/mode/static/nginx/runtime/runtime_manager_test.go @@ -0,0 +1,107 @@ +package runtime_test + +import ( + "net/http" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/go-logr/logr" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime/runtimefakes" + "github.com/nginxinc/nginx-plus-go-client/client" + ngxclient "github.com/nginxinc/nginx-plus-go-client/client" +) + +var _ = Describe("ManagerImpl UpdateHTTPServers", func() { + + var ( + MaxConnsMock = 15 + MaxFailsMock = 2 + WeightMock = 1 + BackupMock = false + DownMock = false + ) + + BeforeEach(func() { + serversMock := ngxclient.UpstreamServer{ + ID: 1, + Server: "10.0.0.1:8080", + MaxConns: &MaxConnsMock, + MaxFails: &MaxFailsMock, + FailTimeout: "test", + SlowStart: "test", + Route: "test", + Backup: &BackupMock, + Down: &DownMock, + Drain: false, + Weight: &WeightMock, + Service: "server", + } + + serverMocks := []ngxclient.UpstreamServer{ + serversMock, + } + + nginxStatusSock := "http://10.0.0.1:8080" + httpClient := &http.Client{} + clientMock, _ := ngxclient.NewNginxClient(nginxStatusSock, client.WithHTTPClient(httpClient)) + logrMock := logr.New(GinkgoLogr.GetSink()) + + mockedManager := runtime.NewManagerImpl(clientMock, nil, logrMock) + err := mockedManager.UpdateHTTPServers("10.0.0.1:8080", serverMocks) + Expect(err).To(BeNil()) + + }) +}) + +var _ = Describe("ManagerImpl GetUpstreams", func() { + var ( + mockNginxClient *ngxclient.NginxClient + manager *runtimefakes.FakeManager + ) + + var ( + MaxConnsMock = 15 + MaxFailsMock = 2 + WeightMock = 1 + BackupMock = false + DownMock = false + ) + + BeforeEach(func() { + nginxStatusSock := "http://10.0.0.1:8080" + httpClient := &http.Client{} + mockNginxClient, _ = ngxclient.NewNginxClient(nginxStatusSock, client.WithHTTPClient(httpClient)) + mockNginxClient.AddHTTPServer("10.0.0.1:8080", ngxclient.UpstreamServer{ + ID: 1, + Server: "10.0.0.1:8080", + MaxConns: &MaxConnsMock, + MaxFails: &MaxFailsMock, + FailTimeout: "test", + SlowStart: "test", + Route: "test", + Backup: &BackupMock, + Down: &DownMock, + Drain: false, + Weight: &WeightMock, + Service: "server", + }) + manager = &runtimefakes.FakeManager{} + }) + + Describe("GetUpstreams", func() { + Context("when NGINX Plus is not enabled", func() { + BeforeEach(func() { + manager.IsPlusReturns(false) + }) + + It("returns no upstreams from NGINX Plus API", func() { + + _, err := manager.GetUpstreams() + Expect(err).ToNot(HaveOccurred()) + }) + + }) + }) +}) From b98942e5ba3df5cd4d4ead6c8014686d080f20a7 Mon Sep 17 00:00:00 2001 From: Mile Druzijanic Date: Sun, 7 Jul 2024 15:29:23 +0200 Subject: [PATCH 02/24] adding required tests update --- .../mode/static/nginx/runtime/manager_test.go | 68 +++++++++++ .../nginx/runtime/runtime_manager_test.go | 107 ------------------ 2 files changed, 68 insertions(+), 107 deletions(-) delete mode 100644 internal/mode/static/nginx/runtime/runtime_manager_test.go diff --git a/internal/mode/static/nginx/runtime/manager_test.go b/internal/mode/static/nginx/runtime/manager_test.go index d6ff923c02..4220c07b29 100644 --- a/internal/mode/static/nginx/runtime/manager_test.go +++ b/internal/mode/static/nginx/runtime/manager_test.go @@ -4,10 +4,12 @@ import ( "context" "errors" "io/fs" + "net/http" "testing" "time" "github.com/go-logr/logr" + "github.com/nginxinc/nginx-plus-go-client/client" ngxclient "github.com/nginxinc/nginx-plus-go-client/client" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -21,6 +23,72 @@ var _ = Describe("NGINX Runtime Manager", func() { mgr = NewManagerImpl(&ngxclient.NginxClient{}, nil, logr.New(GinkgoLogr.GetSink())) Expect(mgr.IsPlus()).To(BeTrue()) }) + + Describe("ManagerImpl", Ordered, func() { + var ( + mockedManager Manager + serverMocks []ngxclient.UpstreamServer + serversMock ngxclient.UpstreamServer + + clientMock *ngxclient.NginxClient + ) + + var ( + MaxConnsMock = 150 + MaxFailsMock = 20 + WeightMock = 10 + BackupMock = false + DownMock = false + ) + + BeforeAll(func() { + + serversMock = ngxclient.UpstreamServer{ + ID: 1, + Server: "unknown", + MaxConns: &MaxConnsMock, + MaxFails: &MaxFailsMock, + FailTimeout: "test", + SlowStart: "test", + Route: "test", + Backup: &BackupMock, + Down: &DownMock, + Drain: false, + Weight: &WeightMock, + Service: "", + } + + serverMocks = []ngxclient.UpstreamServer{ + serversMock, + } + + httpClient := &http.Client{ + Transport: http.DefaultTransport, + CheckRedirect: http.DefaultClient.CheckRedirect, + Jar: http.DefaultClient.Jar, + Timeout: time.Second * 4, + } + + clientMock, _ = ngxclient.NewNginxClient("test", client.WithHTTPClient(httpClient)) + logrMock := logr.New(GinkgoLogr.GetSink()) + mockedManager = NewManagerImpl(clientMock, nil, logrMock) + + }) + + It("UpdateHTTPServers fails upon unknown HTTP server upstream", func() { + err := mockedManager.UpdateHTTPServers("unknown", serverMocks) + Expect(err).ToNot(BeNil()) + }) + + Context("GetUpstreams returns empty map of upstreams", func() { + It("returns no upstreams from NGINX Plus API", func() { + + upstreams, err := mockedManager.GetUpstreams() + Expect(err).To(HaveOccurred()) + Expect(upstreams).To(BeEmpty()) + }) + }) + }) }) func TestEnsureNginxRunning(t *testing.T) { diff --git a/internal/mode/static/nginx/runtime/runtime_manager_test.go b/internal/mode/static/nginx/runtime/runtime_manager_test.go deleted file mode 100644 index 86129ef74d..0000000000 --- a/internal/mode/static/nginx/runtime/runtime_manager_test.go +++ /dev/null @@ -1,107 +0,0 @@ -package runtime_test - -import ( - "net/http" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - "github.com/go-logr/logr" - "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime" - "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime/runtimefakes" - "github.com/nginxinc/nginx-plus-go-client/client" - ngxclient "github.com/nginxinc/nginx-plus-go-client/client" -) - -var _ = Describe("ManagerImpl UpdateHTTPServers", func() { - - var ( - MaxConnsMock = 15 - MaxFailsMock = 2 - WeightMock = 1 - BackupMock = false - DownMock = false - ) - - BeforeEach(func() { - serversMock := ngxclient.UpstreamServer{ - ID: 1, - Server: "10.0.0.1:8080", - MaxConns: &MaxConnsMock, - MaxFails: &MaxFailsMock, - FailTimeout: "test", - SlowStart: "test", - Route: "test", - Backup: &BackupMock, - Down: &DownMock, - Drain: false, - Weight: &WeightMock, - Service: "server", - } - - serverMocks := []ngxclient.UpstreamServer{ - serversMock, - } - - nginxStatusSock := "http://10.0.0.1:8080" - httpClient := &http.Client{} - clientMock, _ := ngxclient.NewNginxClient(nginxStatusSock, client.WithHTTPClient(httpClient)) - logrMock := logr.New(GinkgoLogr.GetSink()) - - mockedManager := runtime.NewManagerImpl(clientMock, nil, logrMock) - err := mockedManager.UpdateHTTPServers("10.0.0.1:8080", serverMocks) - Expect(err).To(BeNil()) - - }) -}) - -var _ = Describe("ManagerImpl GetUpstreams", func() { - var ( - mockNginxClient *ngxclient.NginxClient - manager *runtimefakes.FakeManager - ) - - var ( - MaxConnsMock = 15 - MaxFailsMock = 2 - WeightMock = 1 - BackupMock = false - DownMock = false - ) - - BeforeEach(func() { - nginxStatusSock := "http://10.0.0.1:8080" - httpClient := &http.Client{} - mockNginxClient, _ = ngxclient.NewNginxClient(nginxStatusSock, client.WithHTTPClient(httpClient)) - mockNginxClient.AddHTTPServer("10.0.0.1:8080", ngxclient.UpstreamServer{ - ID: 1, - Server: "10.0.0.1:8080", - MaxConns: &MaxConnsMock, - MaxFails: &MaxFailsMock, - FailTimeout: "test", - SlowStart: "test", - Route: "test", - Backup: &BackupMock, - Down: &DownMock, - Drain: false, - Weight: &WeightMock, - Service: "server", - }) - manager = &runtimefakes.FakeManager{} - }) - - Describe("GetUpstreams", func() { - Context("when NGINX Plus is not enabled", func() { - BeforeEach(func() { - manager.IsPlusReturns(false) - }) - - It("returns no upstreams from NGINX Plus API", func() { - - _, err := manager.GetUpstreams() - Expect(err).ToNot(HaveOccurred()) - }) - - }) - }) -}) From 266fdc0fd307324320adfc1967ec093db1b21b93 Mon Sep 17 00:00:00 2001 From: Mile Druzijanic Date: Sun, 7 Jul 2024 15:29:34 +0200 Subject: [PATCH 03/24] resolving conflicts --- internal/mode/static/nginx/runtime/manager.go | 15 +- .../mode/static/nginx/runtime/manager_test.go | 68 ------ .../nginx/runtime/runtime_manager_test.go | 95 ++++++++ .../runtimefakes/fake_nginx_plus_client.go | 204 ++++++++++++++++++ 4 files changed, 312 insertions(+), 70 deletions(-) create mode 100644 internal/mode/static/nginx/runtime/runtime_manager_test.go create mode 100644 internal/mode/static/nginx/runtime/runtimefakes/fake_nginx_plus_client.go diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index a79c74b42c..96d29fadc7 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -31,7 +31,18 @@ type ( var childProcPathFmt = "/proc/%[1]v/task/%[1]v/children" +<<<<<<< HEAD //counterfeiter:generate . Manager +======= +//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . NginxPlusClient + +type NginxPlusClient interface { + UpdateHTTPServers(upstream string, servers []ngxclient.UpstreamServer) (added []ngxclient.UpstreamServer, deleted []ngxclient.UpstreamServer, updated []ngxclient.UpstreamServer, err error) + GetUpstreams() (*ngxclient.Upstreams, error) +} + +//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . Manager +>>>>>>> 0b42d66 (test update for ManagerImpl) // Manager manages the runtime of NGINX. type Manager interface { @@ -58,13 +69,13 @@ type MetricsCollector interface { type ManagerImpl struct { verifyClient *verifyClient metricsCollector MetricsCollector - ngxPlusClient *ngxclient.NginxClient + ngxPlusClient NginxPlusClient logger logr.Logger } // NewManagerImpl creates a new ManagerImpl. func NewManagerImpl( - ngxPlusClient *ngxclient.NginxClient, + ngxPlusClient NginxPlusClient, collector MetricsCollector, logger logr.Logger, ) *ManagerImpl { diff --git a/internal/mode/static/nginx/runtime/manager_test.go b/internal/mode/static/nginx/runtime/manager_test.go index 4220c07b29..d6ff923c02 100644 --- a/internal/mode/static/nginx/runtime/manager_test.go +++ b/internal/mode/static/nginx/runtime/manager_test.go @@ -4,12 +4,10 @@ import ( "context" "errors" "io/fs" - "net/http" "testing" "time" "github.com/go-logr/logr" - "github.com/nginxinc/nginx-plus-go-client/client" ngxclient "github.com/nginxinc/nginx-plus-go-client/client" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -23,72 +21,6 @@ var _ = Describe("NGINX Runtime Manager", func() { mgr = NewManagerImpl(&ngxclient.NginxClient{}, nil, logr.New(GinkgoLogr.GetSink())) Expect(mgr.IsPlus()).To(BeTrue()) }) - - Describe("ManagerImpl", Ordered, func() { - var ( - mockedManager Manager - serverMocks []ngxclient.UpstreamServer - serversMock ngxclient.UpstreamServer - - clientMock *ngxclient.NginxClient - ) - - var ( - MaxConnsMock = 150 - MaxFailsMock = 20 - WeightMock = 10 - BackupMock = false - DownMock = false - ) - - BeforeAll(func() { - - serversMock = ngxclient.UpstreamServer{ - ID: 1, - Server: "unknown", - MaxConns: &MaxConnsMock, - MaxFails: &MaxFailsMock, - FailTimeout: "test", - SlowStart: "test", - Route: "test", - Backup: &BackupMock, - Down: &DownMock, - Drain: false, - Weight: &WeightMock, - Service: "", - } - - serverMocks = []ngxclient.UpstreamServer{ - serversMock, - } - - httpClient := &http.Client{ - Transport: http.DefaultTransport, - CheckRedirect: http.DefaultClient.CheckRedirect, - Jar: http.DefaultClient.Jar, - Timeout: time.Second * 4, - } - - clientMock, _ = ngxclient.NewNginxClient("test", client.WithHTTPClient(httpClient)) - logrMock := logr.New(GinkgoLogr.GetSink()) - mockedManager = NewManagerImpl(clientMock, nil, logrMock) - - }) - - It("UpdateHTTPServers fails upon unknown HTTP server upstream", func() { - err := mockedManager.UpdateHTTPServers("unknown", serverMocks) - Expect(err).ToNot(BeNil()) - }) - - Context("GetUpstreams returns empty map of upstreams", func() { - It("returns no upstreams from NGINX Plus API", func() { - - upstreams, err := mockedManager.GetUpstreams() - Expect(err).To(HaveOccurred()) - Expect(upstreams).To(BeEmpty()) - }) - }) - }) }) func TestEnsureNginxRunning(t *testing.T) { diff --git a/internal/mode/static/nginx/runtime/runtime_manager_test.go b/internal/mode/static/nginx/runtime/runtime_manager_test.go new file mode 100644 index 0000000000..bfa91a6db6 --- /dev/null +++ b/internal/mode/static/nginx/runtime/runtime_manager_test.go @@ -0,0 +1,95 @@ +package runtime_test + +import ( + "github.com/go-logr/logr" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime/runtimefakes" + ngxclient "github.com/nginxinc/nginx-plus-go-client/client" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("NGINX Runtime Manager", Ordered, func() { + var ( + manager runtime.Manager + upstreamServers []ngxclient.UpstreamServer + upstreamServer ngxclient.UpstreamServer + logger logr.Logger + ngxPlusClient *runtimefakes.FakeNginxPlusClient + ) + + var ( + MaxConnsMock = 150 + MaxFailsMock = 20 + WeightMock = 10 + BackupMock = false + DownMock = false + ) + + BeforeAll(func() { + upstreamServer = ngxclient.UpstreamServer{ + ID: 1, + Server: "10.0.0.1:80", + MaxConns: &MaxConnsMock, + MaxFails: &MaxFailsMock, + FailTimeout: "test", + SlowStart: "test", + Route: "test", + Backup: &BackupMock, + Down: &DownMock, + Drain: false, + Weight: &WeightMock, + Service: "test", + } + + upstreamServers = []ngxclient.UpstreamServer{ + upstreamServer, + } + + logger = logr.New(GinkgoLogr.GetSink()) + }) + + When("running NGINX plus", func() { + BeforeAll(func() { + ngxPlusClient = &runtimefakes.FakeNginxPlusClient{} + manager = runtime.NewManagerImpl(ngxPlusClient, nil, logger) + }) + + It("sucessfully updates HTTP server upstream", func() { + err := manager.UpdateHTTPServers("test", upstreamServers) + + Expect(err).To(BeNil()) + }) + + It("returns no upstreams from NGINX Plus API", func() { + upstreams, err := manager.GetUpstreams() + + Expect(err).To(HaveOccurred()) + Expect(upstreams).To(BeEmpty()) + }) + }) + + When("not running NGINX plus", func() { + BeforeAll(func() { + ngxPlusClient = nil + manager = runtime.NewManagerImpl(ngxPlusClient, nil, logger) + }) + + It("should panic when updating HTTP upstream servers", func() { + updateServers := func() { + manager.UpdateHTTPServers("test", upstreamServers) + } + + Expect(updateServers).To(Panic()) + }) + + It("should panic when fetching HTTP upstream servers", func() { + upstreams := func() { + manager.GetUpstreams() + } + + Expect(upstreams).To(Panic()) + }) + }) +}) diff --git a/internal/mode/static/nginx/runtime/runtimefakes/fake_nginx_plus_client.go b/internal/mode/static/nginx/runtime/runtimefakes/fake_nginx_plus_client.go new file mode 100644 index 0000000000..3ea431d29b --- /dev/null +++ b/internal/mode/static/nginx/runtime/runtimefakes/fake_nginx_plus_client.go @@ -0,0 +1,204 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package runtimefakes + +import ( + "sync" + + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime" + "github.com/nginxinc/nginx-plus-go-client/client" +) + +type FakeNginxPlusClient struct { + GetUpstreamsStub func() (*client.Upstreams, error) + getUpstreamsMutex sync.RWMutex + getUpstreamsArgsForCall []struct { + } + getUpstreamsReturns struct { + result1 *client.Upstreams + result2 error + } + getUpstreamsReturnsOnCall map[int]struct { + result1 *client.Upstreams + result2 error + } + UpdateHTTPServersStub func(string, []client.UpstreamServer) ([]client.UpstreamServer, []client.UpstreamServer, []client.UpstreamServer, error) + updateHTTPServersMutex sync.RWMutex + updateHTTPServersArgsForCall []struct { + arg1 string + arg2 []client.UpstreamServer + } + updateHTTPServersReturns struct { + result1 []client.UpstreamServer + result2 []client.UpstreamServer + result3 []client.UpstreamServer + result4 error + } + updateHTTPServersReturnsOnCall map[int]struct { + result1 []client.UpstreamServer + result2 []client.UpstreamServer + result3 []client.UpstreamServer + result4 error + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *FakeNginxPlusClient) GetUpstreams() (*client.Upstreams, error) { + fake.getUpstreamsMutex.Lock() + ret, specificReturn := fake.getUpstreamsReturnsOnCall[len(fake.getUpstreamsArgsForCall)] + fake.getUpstreamsArgsForCall = append(fake.getUpstreamsArgsForCall, struct { + }{}) + stub := fake.GetUpstreamsStub + fakeReturns := fake.getUpstreamsReturns + fake.recordInvocation("GetUpstreams", []interface{}{}) + fake.getUpstreamsMutex.Unlock() + if stub != nil { + return stub() + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeNginxPlusClient) GetUpstreamsCallCount() int { + fake.getUpstreamsMutex.RLock() + defer fake.getUpstreamsMutex.RUnlock() + return len(fake.getUpstreamsArgsForCall) +} + +func (fake *FakeNginxPlusClient) GetUpstreamsCalls(stub func() (*client.Upstreams, error)) { + fake.getUpstreamsMutex.Lock() + defer fake.getUpstreamsMutex.Unlock() + fake.GetUpstreamsStub = stub +} + +func (fake *FakeNginxPlusClient) GetUpstreamsReturns(result1 *client.Upstreams, result2 error) { + fake.getUpstreamsMutex.Lock() + defer fake.getUpstreamsMutex.Unlock() + fake.GetUpstreamsStub = nil + fake.getUpstreamsReturns = struct { + result1 *client.Upstreams + result2 error + }{result1, result2} +} + +func (fake *FakeNginxPlusClient) GetUpstreamsReturnsOnCall(i int, result1 *client.Upstreams, result2 error) { + fake.getUpstreamsMutex.Lock() + defer fake.getUpstreamsMutex.Unlock() + fake.GetUpstreamsStub = nil + if fake.getUpstreamsReturnsOnCall == nil { + fake.getUpstreamsReturnsOnCall = make(map[int]struct { + result1 *client.Upstreams + result2 error + }) + } + fake.getUpstreamsReturnsOnCall[i] = struct { + result1 *client.Upstreams + result2 error + }{result1, result2} +} + +func (fake *FakeNginxPlusClient) UpdateHTTPServers(arg1 string, arg2 []client.UpstreamServer) ([]client.UpstreamServer, []client.UpstreamServer, []client.UpstreamServer, error) { + var arg2Copy []client.UpstreamServer + if arg2 != nil { + arg2Copy = make([]client.UpstreamServer, len(arg2)) + copy(arg2Copy, arg2) + } + fake.updateHTTPServersMutex.Lock() + ret, specificReturn := fake.updateHTTPServersReturnsOnCall[len(fake.updateHTTPServersArgsForCall)] + fake.updateHTTPServersArgsForCall = append(fake.updateHTTPServersArgsForCall, struct { + arg1 string + arg2 []client.UpstreamServer + }{arg1, arg2Copy}) + stub := fake.UpdateHTTPServersStub + fakeReturns := fake.updateHTTPServersReturns + fake.recordInvocation("UpdateHTTPServers", []interface{}{arg1, arg2Copy}) + fake.updateHTTPServersMutex.Unlock() + if stub != nil { + return stub(arg1, arg2) + } + if specificReturn { + return ret.result1, ret.result2, ret.result3, ret.result4 + } + return fakeReturns.result1, fakeReturns.result2, fakeReturns.result3, fakeReturns.result4 +} + +func (fake *FakeNginxPlusClient) UpdateHTTPServersCallCount() int { + fake.updateHTTPServersMutex.RLock() + defer fake.updateHTTPServersMutex.RUnlock() + return len(fake.updateHTTPServersArgsForCall) +} + +func (fake *FakeNginxPlusClient) UpdateHTTPServersCalls(stub func(string, []client.UpstreamServer) ([]client.UpstreamServer, []client.UpstreamServer, []client.UpstreamServer, error)) { + fake.updateHTTPServersMutex.Lock() + defer fake.updateHTTPServersMutex.Unlock() + fake.UpdateHTTPServersStub = stub +} + +func (fake *FakeNginxPlusClient) UpdateHTTPServersArgsForCall(i int) (string, []client.UpstreamServer) { + fake.updateHTTPServersMutex.RLock() + defer fake.updateHTTPServersMutex.RUnlock() + argsForCall := fake.updateHTTPServersArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *FakeNginxPlusClient) UpdateHTTPServersReturns(result1 []client.UpstreamServer, result2 []client.UpstreamServer, result3 []client.UpstreamServer, result4 error) { + fake.updateHTTPServersMutex.Lock() + defer fake.updateHTTPServersMutex.Unlock() + fake.UpdateHTTPServersStub = nil + fake.updateHTTPServersReturns = struct { + result1 []client.UpstreamServer + result2 []client.UpstreamServer + result3 []client.UpstreamServer + result4 error + }{result1, result2, result3, result4} +} + +func (fake *FakeNginxPlusClient) UpdateHTTPServersReturnsOnCall(i int, result1 []client.UpstreamServer, result2 []client.UpstreamServer, result3 []client.UpstreamServer, result4 error) { + fake.updateHTTPServersMutex.Lock() + defer fake.updateHTTPServersMutex.Unlock() + fake.UpdateHTTPServersStub = nil + if fake.updateHTTPServersReturnsOnCall == nil { + fake.updateHTTPServersReturnsOnCall = make(map[int]struct { + result1 []client.UpstreamServer + result2 []client.UpstreamServer + result3 []client.UpstreamServer + result4 error + }) + } + fake.updateHTTPServersReturnsOnCall[i] = struct { + result1 []client.UpstreamServer + result2 []client.UpstreamServer + result3 []client.UpstreamServer + result4 error + }{result1, result2, result3, result4} +} + +func (fake *FakeNginxPlusClient) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.getUpstreamsMutex.RLock() + defer fake.getUpstreamsMutex.RUnlock() + fake.updateHTTPServersMutex.RLock() + defer fake.updateHTTPServersMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *FakeNginxPlusClient) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} + +var _ runtime.NginxPlusClient = new(FakeNginxPlusClient) From 676548c38d60cf491e9669cc2f898ecc1bba7187 Mon Sep 17 00:00:00 2001 From: Mile Druzijanic Date: Sun, 7 Jul 2024 15:29:41 +0200 Subject: [PATCH 04/24] adding resolving conflicts --- internal/mode/static/nginx/runtime/manager.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index 96d29fadc7..d0c3cc8131 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -31,9 +31,6 @@ type ( var childProcPathFmt = "/proc/%[1]v/task/%[1]v/children" -<<<<<<< HEAD -//counterfeiter:generate . Manager -======= //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . NginxPlusClient type NginxPlusClient interface { @@ -42,7 +39,6 @@ type NginxPlusClient interface { } //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . Manager ->>>>>>> 0b42d66 (test update for ManagerImpl) // Manager manages the runtime of NGINX. type Manager interface { From 88f656f5b7d66a6bce5ad10901c8da403aae2c11 Mon Sep 17 00:00:00 2001 From: Mile Druzijanic Date: Mon, 26 Feb 2024 11:30:57 +0100 Subject: [PATCH 05/24] adding additional updates with Reload fnction tests --- internal/mode/static/manager.go | 2 + internal/mode/static/nginx/runtime/manager.go | 70 +++-- .../mode/static/nginx/runtime/manager_test.go | 176 ++++++++--- .../nginx/runtime/runtime_manager_test.go | 95 ------ .../runtimefakes/fake_metrics_collector.go | 137 +++++++++ .../runtimefakes/fake_nginx_plus_client.go | 3 - .../runtimefakes/fake_process_handler.go | 278 ++++++++++++++++++ .../runtimefakes/fake_verify_client.go | 269 +++++++++++++++++ internal/mode/static/nginx/runtime/verify.go | 34 ++- .../mode/static/nginx/runtime/verify_test.go | 8 +- 10 files changed, 895 insertions(+), 177 deletions(-) delete mode 100644 internal/mode/static/nginx/runtime/runtime_manager_test.go create mode 100644 internal/mode/static/nginx/runtime/runtimefakes/fake_metrics_collector.go create mode 100644 internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go create mode 100644 internal/mode/static/nginx/runtime/runtimefakes/fake_verify_client.go diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index f8545aa627..a66ba1f972 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -221,6 +221,8 @@ func StartManager(cfg config.Config) error { ngxPlusClient, ngxruntimeCollector, cfg.Logger.WithName("nginxRuntimeManager"), + nil, + nil, ), statusUpdater: groupStatusUpdater, eventRecorder: recorder, diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index d0c3cc8131..66ad6f955e 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -19,25 +19,33 @@ import ( //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate const ( - pidFile = "/var/run/nginx/nginx.pid" + PidFile = "/var/run/nginx/nginx.pid" pidFileTimeout = 10000 * time.Millisecond nginxReloadTimeout = 60000 * time.Millisecond ) type ( - readFileFunc func(string) ([]byte, error) - checkFileFunc func(string) (fs.FileInfo, error) + ReadFileFunc func(string) ([]byte, error) + CheckFileFunc func(string) (fs.FileInfo, error) ) var childProcPathFmt = "/proc/%[1]v/task/%[1]v/children" -//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . NginxPlusClient +//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . nginxPlusClient -type NginxPlusClient interface { +type nginxPlusClient interface { UpdateHTTPServers(upstream string, servers []ngxclient.UpstreamServer) (added []ngxclient.UpstreamServer, deleted []ngxclient.UpstreamServer, updated []ngxclient.UpstreamServer, err error) GetUpstreams() (*ngxclient.Upstreams, error) } +//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . processHandler + +type processHandler interface { + FindMainProcess(ctx context.Context, checkFile CheckFileFunc, readFile ReadFileFunc, timeout time.Duration) (int, error) + ReadFile(file string) ([]byte, error) + SysCallKill(pid int, signum syscall.Signal) error +} + //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . Manager // Manager manages the runtime of NGINX. @@ -54,6 +62,8 @@ type Manager interface { GetUpstreams() (ngxclient.Upstreams, error) } +//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . MetricsCollector + // MetricsCollector is an interface for the metrics of the NGINX runtime manager. type MetricsCollector interface { IncReloadCount() @@ -63,23 +73,27 @@ type MetricsCollector interface { // ManagerImpl implements Manager. type ManagerImpl struct { - verifyClient *verifyClient + verifyClient verifyClient metricsCollector MetricsCollector - ngxPlusClient NginxPlusClient + ngxPlusClient nginxPlusClient logger logr.Logger + process processHandler } // NewManagerImpl creates a new ManagerImpl. func NewManagerImpl( - ngxPlusClient NginxPlusClient, + ngxPlusClient nginxPlusClient, collector MetricsCollector, logger logr.Logger, + process processHandler, + verifyClient verifyClient, ) *ManagerImpl { return &ManagerImpl{ - verifyClient: newVerifyClient(nginxReloadTimeout), + verifyClient: verifyClient, metricsCollector: collector, ngxPlusClient: ngxPlusClient, logger: logger, + process: process, } } @@ -91,25 +105,25 @@ func (m *ManagerImpl) IsPlus() bool { func (m *ManagerImpl) Reload(ctx context.Context, configVersion int) error { start := time.Now() // We find the main NGINX PID on every reload because it will change if the NGINX container is restarted. - pid, err := findMainProcess(ctx, os.Stat, os.ReadFile, pidFileTimeout) + pid, err := m.process.FindMainProcess(ctx, os.Stat, os.ReadFile, pidFileTimeout) if err != nil { return fmt.Errorf("failed to find NGINX main process: %w", err) } childProcFile := fmt.Sprintf(childProcPathFmt, pid) - previousChildProcesses, err := os.ReadFile(childProcFile) + previousChildProcesses, err := m.process.ReadFile(childProcFile) if err != nil { return err } // send HUP signal to the NGINX main process reload configuration // See https://nginx.org/en/docs/control.html - if err := syscall.Kill(pid, syscall.SIGHUP); err != nil { + if err := m.process.SysCallKill(pid, syscall.SIGHUP); err != nil { m.metricsCollector.IncReloadErrors() return fmt.Errorf("failed to send the HUP signal to NGINX main: %w", err) } - if err = m.verifyClient.waitForCorrectVersion( + if err = m.verifyClient.WaitForCorrectVersion( ctx, configVersion, childProcFile, @@ -162,16 +176,16 @@ func (m *ManagerImpl) GetUpstreams() (ngxclient.Upstreams, error) { // EnsureNginxRunning ensures NGINX is running by locating the main process. func EnsureNginxRunning(ctx context.Context) error { - if _, err := findMainProcess(ctx, os.Stat, os.ReadFile, pidFileTimeout); err != nil { + if _, err := FindMainProcess(ctx, os.Stat, os.ReadFile, pidFileTimeout); err != nil { return fmt.Errorf("failed to find NGINX main process: %w", err) } return nil } -func findMainProcess( +func FindMainProcess( ctx context.Context, - checkFile checkFileFunc, - readFile readFileFunc, + checkFile CheckFileFunc, + readFile ReadFileFunc, timeout time.Duration, ) (int, error) { ctx, cancel := context.WithTimeout(ctx, timeout) @@ -182,7 +196,7 @@ func findMainProcess( 500*time.Millisecond, true, /* poll immediately */ func(_ context.Context) (bool, error) { - _, err := checkFile(pidFile) + _, err := checkFile(PidFile) if err == nil { return true, nil } @@ -195,7 +209,7 @@ func findMainProcess( return 0, err } - content, err := readFile(pidFile) + content, err := readFile(PidFile) if err != nil { return 0, err } @@ -207,3 +221,21 @@ func findMainProcess( return pid, nil } + +func ReadFile(file string) ([]byte, error) { + content, err := os.ReadFile(file) + if err != nil { + return nil, err + } + + return content, nil +} + +func SysCallKill(pid int, signum syscall.Signal) error { + err := syscall.Kill(pid, syscall.SIGHUP) + if err != nil { + return err + } + + return nil +} diff --git a/internal/mode/static/nginx/runtime/manager_test.go b/internal/mode/static/nginx/runtime/manager_test.go index d6ff923c02..ef69c1599f 100644 --- a/internal/mode/static/nginx/runtime/manager_test.go +++ b/internal/mode/static/nginx/runtime/manager_test.go @@ -1,4 +1,4 @@ -package runtime +package runtime_test import ( "context" @@ -7,62 +7,152 @@ import ( "testing" "time" - "github.com/go-logr/logr" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime/runtimefakes" ngxclient "github.com/nginxinc/nginx-plus-go-client/client" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "sigs.k8s.io/controller-runtime/pkg/log/zap" ) -var _ = Describe("NGINX Runtime Manager", func() { +var _ = Describe("NGINX Runtime Manager", Ordered, func() { It("returns whether or not we're using NGINX Plus", func() { - mgr := NewManagerImpl(nil, nil, logr.New(GinkgoLogr.GetSink())) + mgr := runtime.NewManagerImpl(nil, nil, zap.New(), nil, nil) Expect(mgr.IsPlus()).To(BeFalse()) - mgr = NewManagerImpl(&ngxclient.NginxClient{}, nil, logr.New(GinkgoLogr.GetSink())) + mgr = runtime.NewManagerImpl(&ngxclient.NginxClient{}, nil, zap.New(), nil, nil) Expect(mgr.IsPlus()).To(BeTrue()) }) -}) -func TestEnsureNginxRunning(t *testing.T) { - ctx := context.Background() - cancellingCtx, cancel := context.WithCancel(ctx) - time.AfterFunc(1*time.Millisecond, cancel) - tests := []struct { - ctx context.Context - name string - expectError bool - }{ - { - ctx: ctx, - name: "context exceeded", - expectError: true, - }, - { - ctx: cancellingCtx, - name: "context cancelled", - expectError: true, - }, - } + var ( + manager runtime.Manager + upstreamServers []ngxclient.UpstreamServer + upstreamServer ngxclient.UpstreamServer + ngxPlusClient *runtimefakes.FakeNginxPlusClient + process *runtimefakes.FakeProcessHandler - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - g := NewWithT(t) + metrics *runtimefakes.FakeMetricsCollector + verifyClient *runtimefakes.FakeVerifyClient + ) - err := EnsureNginxRunning(test.ctx) + var ( + MaxConnsMock = 150 + MaxFailsMock = 20 + WeightMock = 10 + BackupMock = false + DownMock = false + ) - if test.expectError { - g.Expect(err).To(HaveOccurred()) - } else { - g.Expect(err).ToNot(HaveOccurred()) + BeforeAll(func() { + upstreamServer = ngxclient.UpstreamServer{ + ID: 1, + Server: "10.0.0.1:80", + MaxConns: &MaxConnsMock, + MaxFails: &MaxFailsMock, + FailTimeout: "test", + SlowStart: "test", + Route: "test", + Backup: &BackupMock, + Down: &DownMock, + Drain: false, + Weight: &WeightMock, + Service: "test", + } + + upstreamServers = []ngxclient.UpstreamServer{ + upstreamServer, + } + + }) + + Context("Reload", func() { + BeforeAll(func() { + ngxPlusClient = &runtimefakes.FakeNginxPlusClient{} + process = &runtimefakes.FakeProcessHandler{} + metrics = &runtimefakes.FakeMetricsCollector{} + verifyClient = &runtimefakes.FakeVerifyClient{} + manager = runtime.NewManagerImpl(ngxPlusClient, metrics, zap.New(), process, verifyClient) + }) + + It("sucessfully reloads NGINX configuration", func() { + err := manager.Reload(context.Background(), 1) + Expect(err).To(BeNil()) + }) + + When("not sucessfully reloads NGINX configuration", func() { + It("should panic if MetricsCollector not enabled", func() { + metrics = nil + manager = runtime.NewManagerImpl(ngxPlusClient, metrics, zap.New(), process, verifyClient) + + reload := func() { + manager.Reload(context.Background(), 0) + } + + Expect(reload).To(Panic()) + }) + + It("should panic if VerifyClient not enabled", func() { + metrics = &runtimefakes.FakeMetricsCollector{} + verifyClient = nil + manager = runtime.NewManagerImpl(ngxPlusClient, metrics, zap.New(), process, verifyClient) + + reload := func() { + manager.Reload(context.Background(), 0) + } + + Expect(reload).To(Panic()) + }) + }) + }) + + When("running NGINX plus", func() { + BeforeAll(func() { + ngxPlusClient = &runtimefakes.FakeNginxPlusClient{} + manager = runtime.NewManagerImpl(ngxPlusClient, nil, zap.New(), nil, nil) + }) + + It("sucessfully updates HTTP server upstream", func() { + err := manager.UpdateHTTPServers("test", upstreamServers) + + Expect(err).To(BeNil()) + }) + + It("returns no upstreams from NGINX Plus API", func() { + upstreams, err := manager.GetUpstreams() + + Expect(err).To(HaveOccurred()) + Expect(upstreams).To(BeEmpty()) + }) + }) + + When("not running NGINX plus", func() { + BeforeAll(func() { + ngxPlusClient = nil + manager = runtime.NewManagerImpl(ngxPlusClient, nil, zap.New(), nil, nil) + }) + + It("should panic when updating HTTP upstream servers", func() { + updateServers := func() { + manager.UpdateHTTPServers("test", upstreamServers) } + + Expect(updateServers).To(Panic()) }) - } -} + + It("should panic when fetching HTTP upstream servers", func() { + upstreams := func() { + manager.GetUpstreams() + } + + Expect(upstreams).To(Panic()) + }) + }) +}) func TestFindMainProcess(t *testing.T) { - readFileFuncGen := func(content []byte) readFileFunc { + readFileFuncGen := func(content []byte) runtime.ReadFileFunc { return func(name string) ([]byte, error) { - if name != pidFile { + if name != runtime.PidFile { return nil, errors.New("error") } return content, nil @@ -72,9 +162,9 @@ func TestFindMainProcess(t *testing.T) { return nil, errors.New("error") } - checkFileFuncGen := func(content fs.FileInfo) checkFileFunc { + checkFileFuncGen := func(content fs.FileInfo) runtime.CheckFileFunc { return func(name string) (fs.FileInfo, error) { - if name != pidFile { + if name != runtime.PidFile { return nil, errors.New("error") } return content, nil @@ -90,8 +180,8 @@ func TestFindMainProcess(t *testing.T) { tests := []struct { ctx context.Context - readFile readFileFunc - checkFile checkFileFunc + readFile runtime.ReadFileFunc + checkFile runtime.CheckFileFunc name string expected int expectError bool @@ -150,7 +240,7 @@ func TestFindMainProcess(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - result, err := findMainProcess(test.ctx, test.checkFile, test.readFile, 2*time.Millisecond) + result, err := runtime.FindMainProcess(test.ctx, test.checkFile, test.readFile, 2*time.Millisecond) if test.expectError { g.Expect(err).To(HaveOccurred()) diff --git a/internal/mode/static/nginx/runtime/runtime_manager_test.go b/internal/mode/static/nginx/runtime/runtime_manager_test.go deleted file mode 100644 index bfa91a6db6..0000000000 --- a/internal/mode/static/nginx/runtime/runtime_manager_test.go +++ /dev/null @@ -1,95 +0,0 @@ -package runtime_test - -import ( - "github.com/go-logr/logr" - "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime" - "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime/runtimefakes" - ngxclient "github.com/nginxinc/nginx-plus-go-client/client" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -var _ = Describe("NGINX Runtime Manager", Ordered, func() { - var ( - manager runtime.Manager - upstreamServers []ngxclient.UpstreamServer - upstreamServer ngxclient.UpstreamServer - logger logr.Logger - ngxPlusClient *runtimefakes.FakeNginxPlusClient - ) - - var ( - MaxConnsMock = 150 - MaxFailsMock = 20 - WeightMock = 10 - BackupMock = false - DownMock = false - ) - - BeforeAll(func() { - upstreamServer = ngxclient.UpstreamServer{ - ID: 1, - Server: "10.0.0.1:80", - MaxConns: &MaxConnsMock, - MaxFails: &MaxFailsMock, - FailTimeout: "test", - SlowStart: "test", - Route: "test", - Backup: &BackupMock, - Down: &DownMock, - Drain: false, - Weight: &WeightMock, - Service: "test", - } - - upstreamServers = []ngxclient.UpstreamServer{ - upstreamServer, - } - - logger = logr.New(GinkgoLogr.GetSink()) - }) - - When("running NGINX plus", func() { - BeforeAll(func() { - ngxPlusClient = &runtimefakes.FakeNginxPlusClient{} - manager = runtime.NewManagerImpl(ngxPlusClient, nil, logger) - }) - - It("sucessfully updates HTTP server upstream", func() { - err := manager.UpdateHTTPServers("test", upstreamServers) - - Expect(err).To(BeNil()) - }) - - It("returns no upstreams from NGINX Plus API", func() { - upstreams, err := manager.GetUpstreams() - - Expect(err).To(HaveOccurred()) - Expect(upstreams).To(BeEmpty()) - }) - }) - - When("not running NGINX plus", func() { - BeforeAll(func() { - ngxPlusClient = nil - manager = runtime.NewManagerImpl(ngxPlusClient, nil, logger) - }) - - It("should panic when updating HTTP upstream servers", func() { - updateServers := func() { - manager.UpdateHTTPServers("test", upstreamServers) - } - - Expect(updateServers).To(Panic()) - }) - - It("should panic when fetching HTTP upstream servers", func() { - upstreams := func() { - manager.GetUpstreams() - } - - Expect(upstreams).To(Panic()) - }) - }) -}) diff --git a/internal/mode/static/nginx/runtime/runtimefakes/fake_metrics_collector.go b/internal/mode/static/nginx/runtime/runtimefakes/fake_metrics_collector.go new file mode 100644 index 0000000000..4562c2c5c2 --- /dev/null +++ b/internal/mode/static/nginx/runtime/runtimefakes/fake_metrics_collector.go @@ -0,0 +1,137 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package runtimefakes + +import ( + "sync" + "time" + + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime" +) + +type FakeMetricsCollector struct { + IncReloadCountStub func() + incReloadCountMutex sync.RWMutex + incReloadCountArgsForCall []struct { + } + IncReloadErrorsStub func() + incReloadErrorsMutex sync.RWMutex + incReloadErrorsArgsForCall []struct { + } + ObserveLastReloadTimeStub func(time.Duration) + observeLastReloadTimeMutex sync.RWMutex + observeLastReloadTimeArgsForCall []struct { + arg1 time.Duration + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *FakeMetricsCollector) IncReloadCount() { + fake.incReloadCountMutex.Lock() + fake.incReloadCountArgsForCall = append(fake.incReloadCountArgsForCall, struct { + }{}) + stub := fake.IncReloadCountStub + fake.recordInvocation("IncReloadCount", []interface{}{}) + fake.incReloadCountMutex.Unlock() + if stub != nil { + fake.IncReloadCountStub() + } +} + +func (fake *FakeMetricsCollector) IncReloadCountCallCount() int { + fake.incReloadCountMutex.RLock() + defer fake.incReloadCountMutex.RUnlock() + return len(fake.incReloadCountArgsForCall) +} + +func (fake *FakeMetricsCollector) IncReloadCountCalls(stub func()) { + fake.incReloadCountMutex.Lock() + defer fake.incReloadCountMutex.Unlock() + fake.IncReloadCountStub = stub +} + +func (fake *FakeMetricsCollector) IncReloadErrors() { + fake.incReloadErrorsMutex.Lock() + fake.incReloadErrorsArgsForCall = append(fake.incReloadErrorsArgsForCall, struct { + }{}) + stub := fake.IncReloadErrorsStub + fake.recordInvocation("IncReloadErrors", []interface{}{}) + fake.incReloadErrorsMutex.Unlock() + if stub != nil { + fake.IncReloadErrorsStub() + } +} + +func (fake *FakeMetricsCollector) IncReloadErrorsCallCount() int { + fake.incReloadErrorsMutex.RLock() + defer fake.incReloadErrorsMutex.RUnlock() + return len(fake.incReloadErrorsArgsForCall) +} + +func (fake *FakeMetricsCollector) IncReloadErrorsCalls(stub func()) { + fake.incReloadErrorsMutex.Lock() + defer fake.incReloadErrorsMutex.Unlock() + fake.IncReloadErrorsStub = stub +} + +func (fake *FakeMetricsCollector) ObserveLastReloadTime(arg1 time.Duration) { + fake.observeLastReloadTimeMutex.Lock() + fake.observeLastReloadTimeArgsForCall = append(fake.observeLastReloadTimeArgsForCall, struct { + arg1 time.Duration + }{arg1}) + stub := fake.ObserveLastReloadTimeStub + fake.recordInvocation("ObserveLastReloadTime", []interface{}{arg1}) + fake.observeLastReloadTimeMutex.Unlock() + if stub != nil { + fake.ObserveLastReloadTimeStub(arg1) + } +} + +func (fake *FakeMetricsCollector) ObserveLastReloadTimeCallCount() int { + fake.observeLastReloadTimeMutex.RLock() + defer fake.observeLastReloadTimeMutex.RUnlock() + return len(fake.observeLastReloadTimeArgsForCall) +} + +func (fake *FakeMetricsCollector) ObserveLastReloadTimeCalls(stub func(time.Duration)) { + fake.observeLastReloadTimeMutex.Lock() + defer fake.observeLastReloadTimeMutex.Unlock() + fake.ObserveLastReloadTimeStub = stub +} + +func (fake *FakeMetricsCollector) ObserveLastReloadTimeArgsForCall(i int) time.Duration { + fake.observeLastReloadTimeMutex.RLock() + defer fake.observeLastReloadTimeMutex.RUnlock() + argsForCall := fake.observeLastReloadTimeArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeMetricsCollector) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.incReloadCountMutex.RLock() + defer fake.incReloadCountMutex.RUnlock() + fake.incReloadErrorsMutex.RLock() + defer fake.incReloadErrorsMutex.RUnlock() + fake.observeLastReloadTimeMutex.RLock() + defer fake.observeLastReloadTimeMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *FakeMetricsCollector) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} + +var _ runtime.MetricsCollector = new(FakeMetricsCollector) diff --git a/internal/mode/static/nginx/runtime/runtimefakes/fake_nginx_plus_client.go b/internal/mode/static/nginx/runtime/runtimefakes/fake_nginx_plus_client.go index 3ea431d29b..c6291816d8 100644 --- a/internal/mode/static/nginx/runtime/runtimefakes/fake_nginx_plus_client.go +++ b/internal/mode/static/nginx/runtime/runtimefakes/fake_nginx_plus_client.go @@ -4,7 +4,6 @@ package runtimefakes import ( "sync" - "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime" "github.com/nginxinc/nginx-plus-go-client/client" ) @@ -200,5 +199,3 @@ func (fake *FakeNginxPlusClient) recordInvocation(key string, args []interface{} } fake.invocations[key] = append(fake.invocations[key], args) } - -var _ runtime.NginxPlusClient = new(FakeNginxPlusClient) diff --git a/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go b/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go new file mode 100644 index 0000000000..4495c19dc3 --- /dev/null +++ b/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go @@ -0,0 +1,278 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package runtimefakes + +import ( + "context" + "sync" + "syscall" + "time" + + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime" +) + +type FakeProcessHandler struct { + FindMainProcessStub func(context.Context, runtime.CheckFileFunc, runtime.ReadFileFunc, time.Duration) (int, error) + findMainProcessMutex sync.RWMutex + findMainProcessArgsForCall []struct { + arg1 context.Context + arg2 runtime.CheckFileFunc + arg3 runtime.ReadFileFunc + arg4 time.Duration + } + findMainProcessReturns struct { + result1 int + result2 error + } + findMainProcessReturnsOnCall map[int]struct { + result1 int + result2 error + } + ReadFileStub func(string) ([]byte, error) + readFileMutex sync.RWMutex + readFileArgsForCall []struct { + arg1 string + } + readFileReturns struct { + result1 []byte + result2 error + } + readFileReturnsOnCall map[int]struct { + result1 []byte + result2 error + } + SysCallKillStub func(int, syscall.Signal) error + sysCallKillMutex sync.RWMutex + sysCallKillArgsForCall []struct { + arg1 int + arg2 syscall.Signal + } + sysCallKillReturns struct { + result1 error + } + sysCallKillReturnsOnCall map[int]struct { + result1 error + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *FakeProcessHandler) FindMainProcess(arg1 context.Context, arg2 runtime.CheckFileFunc, arg3 runtime.ReadFileFunc, arg4 time.Duration) (int, error) { + fake.findMainProcessMutex.Lock() + ret, specificReturn := fake.findMainProcessReturnsOnCall[len(fake.findMainProcessArgsForCall)] + fake.findMainProcessArgsForCall = append(fake.findMainProcessArgsForCall, struct { + arg1 context.Context + arg2 runtime.CheckFileFunc + arg3 runtime.ReadFileFunc + arg4 time.Duration + }{arg1, arg2, arg3, arg4}) + stub := fake.FindMainProcessStub + fakeReturns := fake.findMainProcessReturns + fake.recordInvocation("FindMainProcess", []interface{}{arg1, arg2, arg3, arg4}) + fake.findMainProcessMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3, arg4) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeProcessHandler) FindMainProcessCallCount() int { + fake.findMainProcessMutex.RLock() + defer fake.findMainProcessMutex.RUnlock() + return len(fake.findMainProcessArgsForCall) +} + +func (fake *FakeProcessHandler) FindMainProcessCalls(stub func(context.Context, runtime.CheckFileFunc, runtime.ReadFileFunc, time.Duration) (int, error)) { + fake.findMainProcessMutex.Lock() + defer fake.findMainProcessMutex.Unlock() + fake.FindMainProcessStub = stub +} + +func (fake *FakeProcessHandler) FindMainProcessArgsForCall(i int) (context.Context, runtime.CheckFileFunc, runtime.ReadFileFunc, time.Duration) { + fake.findMainProcessMutex.RLock() + defer fake.findMainProcessMutex.RUnlock() + argsForCall := fake.findMainProcessArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4 +} + +func (fake *FakeProcessHandler) FindMainProcessReturns(result1 int, result2 error) { + fake.findMainProcessMutex.Lock() + defer fake.findMainProcessMutex.Unlock() + fake.FindMainProcessStub = nil + fake.findMainProcessReturns = struct { + result1 int + result2 error + }{result1, result2} +} + +func (fake *FakeProcessHandler) FindMainProcessReturnsOnCall(i int, result1 int, result2 error) { + fake.findMainProcessMutex.Lock() + defer fake.findMainProcessMutex.Unlock() + fake.FindMainProcessStub = nil + if fake.findMainProcessReturnsOnCall == nil { + fake.findMainProcessReturnsOnCall = make(map[int]struct { + result1 int + result2 error + }) + } + fake.findMainProcessReturnsOnCall[i] = struct { + result1 int + result2 error + }{result1, result2} +} + +func (fake *FakeProcessHandler) ReadFile(arg1 string) ([]byte, error) { + fake.readFileMutex.Lock() + ret, specificReturn := fake.readFileReturnsOnCall[len(fake.readFileArgsForCall)] + fake.readFileArgsForCall = append(fake.readFileArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.ReadFileStub + fakeReturns := fake.readFileReturns + fake.recordInvocation("ReadFile", []interface{}{arg1}) + fake.readFileMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeProcessHandler) ReadFileCallCount() int { + fake.readFileMutex.RLock() + defer fake.readFileMutex.RUnlock() + return len(fake.readFileArgsForCall) +} + +func (fake *FakeProcessHandler) ReadFileCalls(stub func(string) ([]byte, error)) { + fake.readFileMutex.Lock() + defer fake.readFileMutex.Unlock() + fake.ReadFileStub = stub +} + +func (fake *FakeProcessHandler) ReadFileArgsForCall(i int) string { + fake.readFileMutex.RLock() + defer fake.readFileMutex.RUnlock() + argsForCall := fake.readFileArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeProcessHandler) ReadFileReturns(result1 []byte, result2 error) { + fake.readFileMutex.Lock() + defer fake.readFileMutex.Unlock() + fake.ReadFileStub = nil + fake.readFileReturns = struct { + result1 []byte + result2 error + }{result1, result2} +} + +func (fake *FakeProcessHandler) ReadFileReturnsOnCall(i int, result1 []byte, result2 error) { + fake.readFileMutex.Lock() + defer fake.readFileMutex.Unlock() + fake.ReadFileStub = nil + if fake.readFileReturnsOnCall == nil { + fake.readFileReturnsOnCall = make(map[int]struct { + result1 []byte + result2 error + }) + } + fake.readFileReturnsOnCall[i] = struct { + result1 []byte + result2 error + }{result1, result2} +} + +func (fake *FakeProcessHandler) SysCallKill(arg1 int, arg2 syscall.Signal) error { + fake.sysCallKillMutex.Lock() + ret, specificReturn := fake.sysCallKillReturnsOnCall[len(fake.sysCallKillArgsForCall)] + fake.sysCallKillArgsForCall = append(fake.sysCallKillArgsForCall, struct { + arg1 int + arg2 syscall.Signal + }{arg1, arg2}) + stub := fake.SysCallKillStub + fakeReturns := fake.sysCallKillReturns + fake.recordInvocation("SysCallKill", []interface{}{arg1, arg2}) + fake.sysCallKillMutex.Unlock() + if stub != nil { + return stub(arg1, arg2) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeProcessHandler) SysCallKillCallCount() int { + fake.sysCallKillMutex.RLock() + defer fake.sysCallKillMutex.RUnlock() + return len(fake.sysCallKillArgsForCall) +} + +func (fake *FakeProcessHandler) SysCallKillCalls(stub func(int, syscall.Signal) error) { + fake.sysCallKillMutex.Lock() + defer fake.sysCallKillMutex.Unlock() + fake.SysCallKillStub = stub +} + +func (fake *FakeProcessHandler) SysCallKillArgsForCall(i int) (int, syscall.Signal) { + fake.sysCallKillMutex.RLock() + defer fake.sysCallKillMutex.RUnlock() + argsForCall := fake.sysCallKillArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *FakeProcessHandler) SysCallKillReturns(result1 error) { + fake.sysCallKillMutex.Lock() + defer fake.sysCallKillMutex.Unlock() + fake.SysCallKillStub = nil + fake.sysCallKillReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeProcessHandler) SysCallKillReturnsOnCall(i int, result1 error) { + fake.sysCallKillMutex.Lock() + defer fake.sysCallKillMutex.Unlock() + fake.SysCallKillStub = nil + if fake.sysCallKillReturnsOnCall == nil { + fake.sysCallKillReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.sysCallKillReturnsOnCall[i] = struct { + result1 error + }{result1} +} + +func (fake *FakeProcessHandler) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.findMainProcessMutex.RLock() + defer fake.findMainProcessMutex.RUnlock() + fake.readFileMutex.RLock() + defer fake.readFileMutex.RUnlock() + fake.sysCallKillMutex.RLock() + defer fake.sysCallKillMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *FakeProcessHandler) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} diff --git a/internal/mode/static/nginx/runtime/runtimefakes/fake_verify_client.go b/internal/mode/static/nginx/runtime/runtimefakes/fake_verify_client.go new file mode 100644 index 0000000000..13c5f78224 --- /dev/null +++ b/internal/mode/static/nginx/runtime/runtimefakes/fake_verify_client.go @@ -0,0 +1,269 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package runtimefakes + +import ( + "context" + "sync" + + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime" +) + +type FakeVerifyClient struct { + EnsureConfigVersionStub func(context.Context, int) error + ensureConfigVersionMutex sync.RWMutex + ensureConfigVersionArgsForCall []struct { + arg1 context.Context + arg2 int + } + ensureConfigVersionReturns struct { + result1 error + } + ensureConfigVersionReturnsOnCall map[int]struct { + result1 error + } + GetConfigVersionStub func() (int, error) + getConfigVersionMutex sync.RWMutex + getConfigVersionArgsForCall []struct { + } + getConfigVersionReturns struct { + result1 int + result2 error + } + getConfigVersionReturnsOnCall map[int]struct { + result1 int + result2 error + } + WaitForCorrectVersionStub func(context.Context, int, string, []byte, runtime.ReadFileFunc) error + waitForCorrectVersionMutex sync.RWMutex + waitForCorrectVersionArgsForCall []struct { + arg1 context.Context + arg2 int + arg3 string + arg4 []byte + arg5 runtime.ReadFileFunc + } + waitForCorrectVersionReturns struct { + result1 error + } + waitForCorrectVersionReturnsOnCall map[int]struct { + result1 error + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *FakeVerifyClient) EnsureConfigVersion(arg1 context.Context, arg2 int) error { + fake.ensureConfigVersionMutex.Lock() + ret, specificReturn := fake.ensureConfigVersionReturnsOnCall[len(fake.ensureConfigVersionArgsForCall)] + fake.ensureConfigVersionArgsForCall = append(fake.ensureConfigVersionArgsForCall, struct { + arg1 context.Context + arg2 int + }{arg1, arg2}) + stub := fake.EnsureConfigVersionStub + fakeReturns := fake.ensureConfigVersionReturns + fake.recordInvocation("EnsureConfigVersion", []interface{}{arg1, arg2}) + fake.ensureConfigVersionMutex.Unlock() + if stub != nil { + return stub(arg1, arg2) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeVerifyClient) EnsureConfigVersionCallCount() int { + fake.ensureConfigVersionMutex.RLock() + defer fake.ensureConfigVersionMutex.RUnlock() + return len(fake.ensureConfigVersionArgsForCall) +} + +func (fake *FakeVerifyClient) EnsureConfigVersionCalls(stub func(context.Context, int) error) { + fake.ensureConfigVersionMutex.Lock() + defer fake.ensureConfigVersionMutex.Unlock() + fake.EnsureConfigVersionStub = stub +} + +func (fake *FakeVerifyClient) EnsureConfigVersionArgsForCall(i int) (context.Context, int) { + fake.ensureConfigVersionMutex.RLock() + defer fake.ensureConfigVersionMutex.RUnlock() + argsForCall := fake.ensureConfigVersionArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *FakeVerifyClient) EnsureConfigVersionReturns(result1 error) { + fake.ensureConfigVersionMutex.Lock() + defer fake.ensureConfigVersionMutex.Unlock() + fake.EnsureConfigVersionStub = nil + fake.ensureConfigVersionReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeVerifyClient) EnsureConfigVersionReturnsOnCall(i int, result1 error) { + fake.ensureConfigVersionMutex.Lock() + defer fake.ensureConfigVersionMutex.Unlock() + fake.EnsureConfigVersionStub = nil + if fake.ensureConfigVersionReturnsOnCall == nil { + fake.ensureConfigVersionReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.ensureConfigVersionReturnsOnCall[i] = struct { + result1 error + }{result1} +} + +func (fake *FakeVerifyClient) GetConfigVersion() (int, error) { + fake.getConfigVersionMutex.Lock() + ret, specificReturn := fake.getConfigVersionReturnsOnCall[len(fake.getConfigVersionArgsForCall)] + fake.getConfigVersionArgsForCall = append(fake.getConfigVersionArgsForCall, struct { + }{}) + stub := fake.GetConfigVersionStub + fakeReturns := fake.getConfigVersionReturns + fake.recordInvocation("GetConfigVersion", []interface{}{}) + fake.getConfigVersionMutex.Unlock() + if stub != nil { + return stub() + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeVerifyClient) GetConfigVersionCallCount() int { + fake.getConfigVersionMutex.RLock() + defer fake.getConfigVersionMutex.RUnlock() + return len(fake.getConfigVersionArgsForCall) +} + +func (fake *FakeVerifyClient) GetConfigVersionCalls(stub func() (int, error)) { + fake.getConfigVersionMutex.Lock() + defer fake.getConfigVersionMutex.Unlock() + fake.GetConfigVersionStub = stub +} + +func (fake *FakeVerifyClient) GetConfigVersionReturns(result1 int, result2 error) { + fake.getConfigVersionMutex.Lock() + defer fake.getConfigVersionMutex.Unlock() + fake.GetConfigVersionStub = nil + fake.getConfigVersionReturns = struct { + result1 int + result2 error + }{result1, result2} +} + +func (fake *FakeVerifyClient) GetConfigVersionReturnsOnCall(i int, result1 int, result2 error) { + fake.getConfigVersionMutex.Lock() + defer fake.getConfigVersionMutex.Unlock() + fake.GetConfigVersionStub = nil + if fake.getConfigVersionReturnsOnCall == nil { + fake.getConfigVersionReturnsOnCall = make(map[int]struct { + result1 int + result2 error + }) + } + fake.getConfigVersionReturnsOnCall[i] = struct { + result1 int + result2 error + }{result1, result2} +} + +func (fake *FakeVerifyClient) WaitForCorrectVersion(arg1 context.Context, arg2 int, arg3 string, arg4 []byte, arg5 runtime.ReadFileFunc) error { + var arg4Copy []byte + if arg4 != nil { + arg4Copy = make([]byte, len(arg4)) + copy(arg4Copy, arg4) + } + fake.waitForCorrectVersionMutex.Lock() + ret, specificReturn := fake.waitForCorrectVersionReturnsOnCall[len(fake.waitForCorrectVersionArgsForCall)] + fake.waitForCorrectVersionArgsForCall = append(fake.waitForCorrectVersionArgsForCall, struct { + arg1 context.Context + arg2 int + arg3 string + arg4 []byte + arg5 runtime.ReadFileFunc + }{arg1, arg2, arg3, arg4Copy, arg5}) + stub := fake.WaitForCorrectVersionStub + fakeReturns := fake.waitForCorrectVersionReturns + fake.recordInvocation("WaitForCorrectVersion", []interface{}{arg1, arg2, arg3, arg4Copy, arg5}) + fake.waitForCorrectVersionMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3, arg4, arg5) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeVerifyClient) WaitForCorrectVersionCallCount() int { + fake.waitForCorrectVersionMutex.RLock() + defer fake.waitForCorrectVersionMutex.RUnlock() + return len(fake.waitForCorrectVersionArgsForCall) +} + +func (fake *FakeVerifyClient) WaitForCorrectVersionCalls(stub func(context.Context, int, string, []byte, runtime.ReadFileFunc) error) { + fake.waitForCorrectVersionMutex.Lock() + defer fake.waitForCorrectVersionMutex.Unlock() + fake.WaitForCorrectVersionStub = stub +} + +func (fake *FakeVerifyClient) WaitForCorrectVersionArgsForCall(i int) (context.Context, int, string, []byte, runtime.ReadFileFunc) { + fake.waitForCorrectVersionMutex.RLock() + defer fake.waitForCorrectVersionMutex.RUnlock() + argsForCall := fake.waitForCorrectVersionArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4, argsForCall.arg5 +} + +func (fake *FakeVerifyClient) WaitForCorrectVersionReturns(result1 error) { + fake.waitForCorrectVersionMutex.Lock() + defer fake.waitForCorrectVersionMutex.Unlock() + fake.WaitForCorrectVersionStub = nil + fake.waitForCorrectVersionReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeVerifyClient) WaitForCorrectVersionReturnsOnCall(i int, result1 error) { + fake.waitForCorrectVersionMutex.Lock() + defer fake.waitForCorrectVersionMutex.Unlock() + fake.WaitForCorrectVersionStub = nil + if fake.waitForCorrectVersionReturnsOnCall == nil { + fake.waitForCorrectVersionReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.waitForCorrectVersionReturnsOnCall[i] = struct { + result1 error + }{result1} +} + +func (fake *FakeVerifyClient) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.ensureConfigVersionMutex.RLock() + defer fake.ensureConfigVersionMutex.RUnlock() + fake.getConfigVersionMutex.RLock() + defer fake.getConfigVersionMutex.RUnlock() + fake.waitForCorrectVersionMutex.RLock() + defer fake.waitForCorrectVersionMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *FakeVerifyClient) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} diff --git a/internal/mode/static/nginx/runtime/verify.go b/internal/mode/static/nginx/runtime/verify.go index 4e3ff68732..af688cc04d 100644 --- a/internal/mode/static/nginx/runtime/verify.go +++ b/internal/mode/static/nginx/runtime/verify.go @@ -19,15 +19,23 @@ const configVersionURI = "/var/run/nginx/nginx-config-version.sock" var noNewWorkersErrFmt = "reload unsuccessful: no new NGINX worker processes started for config version %d." + " Please check the NGINX container logs for possible configuration issues: %w" -// verifyClient is a client for verifying the config version. -type verifyClient struct { +//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . verifyClient + +type verifyClient interface { + GetConfigVersion() (int, error) + WaitForCorrectVersion(ctx context.Context, expectedVersion int, childProcFile string, previousChildProcesses []byte, readFile ReadFileFunc) error + EnsureConfigVersion(ctx context.Context, expectedVersion int) error +} + +// verifyClientImpl is a client for verifying the config version. +type verifyClientImpl struct { client *http.Client timeout time.Duration } // newVerifyClient returns a new client pointed at the config version socket. -func newVerifyClient(timeout time.Duration) *verifyClient { - return &verifyClient{ +func newVerifyClient(timeout time.Duration) *verifyClientImpl { + return &verifyClientImpl{ client: &http.Client{ Transport: &http.Transport{ DialContext: func(_ context.Context, _, _ string) (net.Conn, error) { @@ -39,9 +47,9 @@ func newVerifyClient(timeout time.Duration) *verifyClient { } } -// getConfigVersion gets the version number that we put in the nginx config to verify that we're using +// GetConfigVersion gets the version number that we put in the nginx config to verify that we're using // the correct config. -func (c *verifyClient) getConfigVersion() (int, error) { +func (c *verifyClientImpl) GetConfigVersion() (int, error) { ctx, cancel := context.WithTimeout(context.Background(), c.timeout) defer cancel() @@ -71,15 +79,15 @@ func (c *verifyClient) getConfigVersion() (int, error) { return v, nil } -// waitForCorrectVersion first ensures any new worker processes have been started, and then calls the config version +// WaitForCorrectVersion first ensures any new worker processes have been started, and then calls the config version // endpoint until it gets the expectedVersion, which ensures that a new worker process has been started for that config // version. -func (c *verifyClient) waitForCorrectVersion( +func (c *verifyClientImpl) WaitForCorrectVersion( ctx context.Context, expectedVersion int, childProcFile string, previousChildProcesses []byte, - readFile readFileFunc, + readFile ReadFileFunc, ) error { ctx, cancel := context.WithTimeout(ctx, c.timeout) defer cancel() @@ -93,7 +101,7 @@ func (c *verifyClient) waitForCorrectVersion( return fmt.Errorf(noNewWorkersErrFmt, expectedVersion, err) } - if err := c.ensureConfigVersion(ctx, expectedVersion); err != nil { + if err := c.EnsureConfigVersion(ctx, expectedVersion); err != nil { if errors.Is(err, context.DeadlineExceeded) { err = fmt.Errorf( "config version check didn't return expected version %d within the deadline", @@ -105,13 +113,13 @@ func (c *verifyClient) waitForCorrectVersion( return nil } -func (c *verifyClient) ensureConfigVersion(ctx context.Context, expectedVersion int) error { +func (c *verifyClientImpl) EnsureConfigVersion(ctx context.Context, expectedVersion int) error { return wait.PollUntilContextCancel( ctx, 25*time.Millisecond, true, /* poll immediately */ func(_ context.Context) (bool, error) { - version, err := c.getConfigVersion() + version, err := c.GetConfigVersion() return version == expectedVersion, err }, ) @@ -121,7 +129,7 @@ func ensureNewNginxWorkers( ctx context.Context, childProcFile string, previousContents []byte, - readFile readFileFunc, + readFile ReadFileFunc, ) error { return wait.PollUntilContextCancel( ctx, diff --git a/internal/mode/static/nginx/runtime/verify_test.go b/internal/mode/static/nginx/runtime/verify_test.go index fda58b56f3..696489127b 100644 --- a/internal/mode/static/nginx/runtime/verify_test.go +++ b/internal/mode/static/nginx/runtime/verify_test.go @@ -30,7 +30,7 @@ func getTestHTTPClient() *http.Client { } func TestVerifyClient(t *testing.T) { - c := verifyClient{ + c := verifyClientImpl{ client: getTestHTTPClient(), timeout: 25 * time.Millisecond, } @@ -50,7 +50,7 @@ func TestVerifyClient(t *testing.T) { tests := []struct { ctx context.Context - readFile readFileFunc + readFile ReadFileFunc name string expectedVersion int expectError bool @@ -89,7 +89,7 @@ func TestVerifyClient(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - err := c.waitForCorrectVersion(test.ctx, test.expectedVersion, "/childfile", []byte("1 2 3"), test.readFile) + err := c.WaitForCorrectVersion(test.ctx, test.expectedVersion, "/childfile", []byte("1 2 3"), test.readFile) if test.expectError { g.Expect(err).To(HaveOccurred()) @@ -126,7 +126,7 @@ func TestEnsureNewNginxWorkers(t *testing.T) { tests := []struct { ctx context.Context - readFile readFileFunc + readFile ReadFileFunc name string previousContents []byte expectError bool From 93157b9617c78238ab75eb45dc9086acff0d9791 Mon Sep 17 00:00:00 2001 From: Mile Druzijanic Date: Tue, 12 Mar 2024 19:44:49 +0100 Subject: [PATCH 06/24] adding required updates --- internal/mode/static/manager.go | 7 +- internal/mode/static/nginx/runtime/manager.go | 29 ++-- .../mode/static/nginx/runtime/manager_test.go | 60 +++---- .../runtimefakes/fake_process_handler.go | 152 +++++++++--------- internal/mode/static/nginx/runtime/verify.go | 16 +- .../mode/static/nginx/runtime/verify_test.go | 2 +- 6 files changed, 128 insertions(+), 138 deletions(-) diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index a66ba1f972..3c2dff88b6 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -145,7 +145,7 @@ func StartManager(cfg config.Config) error { } // Ensure NGINX is running before registering metrics & starting the manager. - if err := ngxruntime.EnsureNginxRunning(ctx); err != nil { + if err := ngxruntime.EnsureNginxRunning(ctx, &ngxruntime.ProcessHandlerImpl{}); err != nil { return fmt.Errorf("NGINX is not running: %w", err) } @@ -156,6 +156,7 @@ func StartManager(cfg config.Config) error { var ngxPlusClient *ngxclient.NginxClient var usageSecret *usage.Secret + //var processHandler *ngxruntime.ProcessHandler if cfg.Plus { ngxPlusClient, err = ngxruntime.CreatePlusClient() if err != nil { @@ -221,8 +222,8 @@ func StartManager(cfg config.Config) error { ngxPlusClient, ngxruntimeCollector, cfg.Logger.WithName("nginxRuntimeManager"), - nil, - nil, + &ngxruntime.ProcessHandlerImpl{}, + ngxruntime.NewVerifyClient(1*time.Second), ), statusUpdater: groupStatusUpdater, eventRecorder: recorder, diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index 66ad6f955e..ef42bf1244 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -40,10 +40,13 @@ type nginxPlusClient interface { //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . processHandler -type processHandler interface { +type ProcessHandler interface { FindMainProcess(ctx context.Context, checkFile CheckFileFunc, readFile ReadFileFunc, timeout time.Duration) (int, error) ReadFile(file string) ([]byte, error) - SysCallKill(pid int, signum syscall.Signal) error + Kill(pid int, signum syscall.Signal) error +} + +type ProcessHandlerImpl struct { } //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . Manager @@ -77,7 +80,7 @@ type ManagerImpl struct { metricsCollector MetricsCollector ngxPlusClient nginxPlusClient logger logr.Logger - process processHandler + processHandler ProcessHandler } // NewManagerImpl creates a new ManagerImpl. @@ -85,7 +88,7 @@ func NewManagerImpl( ngxPlusClient nginxPlusClient, collector MetricsCollector, logger logr.Logger, - process processHandler, + processHandler ProcessHandler, verifyClient verifyClient, ) *ManagerImpl { return &ManagerImpl{ @@ -93,7 +96,7 @@ func NewManagerImpl( metricsCollector: collector, ngxPlusClient: ngxPlusClient, logger: logger, - process: process, + processHandler: processHandler, } } @@ -105,20 +108,20 @@ func (m *ManagerImpl) IsPlus() bool { func (m *ManagerImpl) Reload(ctx context.Context, configVersion int) error { start := time.Now() // We find the main NGINX PID on every reload because it will change if the NGINX container is restarted. - pid, err := m.process.FindMainProcess(ctx, os.Stat, os.ReadFile, pidFileTimeout) + pid, err := m.processHandler.FindMainProcess(ctx, os.Stat, os.ReadFile, pidFileTimeout) if err != nil { return fmt.Errorf("failed to find NGINX main process: %w", err) } childProcFile := fmt.Sprintf(childProcPathFmt, pid) - previousChildProcesses, err := m.process.ReadFile(childProcFile) + previousChildProcesses, err := m.processHandler.ReadFile(childProcFile) if err != nil { return err } // send HUP signal to the NGINX main process reload configuration // See https://nginx.org/en/docs/control.html - if err := m.process.SysCallKill(pid, syscall.SIGHUP); err != nil { + if err := m.processHandler.Kill(pid, syscall.SIGHUP); err != nil { m.metricsCollector.IncReloadErrors() return fmt.Errorf("failed to send the HUP signal to NGINX main: %w", err) } @@ -175,14 +178,14 @@ func (m *ManagerImpl) GetUpstreams() (ngxclient.Upstreams, error) { } // EnsureNginxRunning ensures NGINX is running by locating the main process. -func EnsureNginxRunning(ctx context.Context) error { - if _, err := FindMainProcess(ctx, os.Stat, os.ReadFile, pidFileTimeout); err != nil { +func EnsureNginxRunning(ctx context.Context, processHandler ProcessHandler) error { + if _, err := processHandler.FindMainProcess(ctx, os.Stat, os.ReadFile, pidFileTimeout); err != nil { return fmt.Errorf("failed to find NGINX main process: %w", err) } return nil } -func FindMainProcess( +func (p *ProcessHandlerImpl) FindMainProcess( ctx context.Context, checkFile CheckFileFunc, readFile ReadFileFunc, @@ -222,7 +225,7 @@ func FindMainProcess( return pid, nil } -func ReadFile(file string) ([]byte, error) { +func (p *ProcessHandlerImpl) ReadFile(file string) ([]byte, error) { content, err := os.ReadFile(file) if err != nil { return nil, err @@ -231,7 +234,7 @@ func ReadFile(file string) ([]byte, error) { return content, nil } -func SysCallKill(pid int, signum syscall.Signal) error { +func (p *ProcessHandlerImpl) Kill(pid int, signum syscall.Signal) error { err := syscall.Kill(pid, syscall.SIGHUP) if err != nil { return err diff --git a/internal/mode/static/nginx/runtime/manager_test.go b/internal/mode/static/nginx/runtime/manager_test.go index ef69c1599f..4c69090ca9 100644 --- a/internal/mode/static/nginx/runtime/manager_test.go +++ b/internal/mode/static/nginx/runtime/manager_test.go @@ -7,15 +7,16 @@ import ( "testing" "time" - "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime" - "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime/runtimefakes" ngxclient "github.com/nginxinc/nginx-plus-go-client/client" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "sigs.k8s.io/controller-runtime/pkg/log/zap" + + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime/runtimefakes" ) -var _ = Describe("NGINX Runtime Manager", Ordered, func() { +var _ = Describe("NGINX Runtime Manager", func() { It("returns whether or not we're using NGINX Plus", func() { mgr := runtime.NewManagerImpl(nil, nil, zap.New(), nil, nil) Expect(mgr.IsPlus()).To(BeFalse()) @@ -35,29 +36,8 @@ var _ = Describe("NGINX Runtime Manager", Ordered, func() { verifyClient *runtimefakes.FakeVerifyClient ) - var ( - MaxConnsMock = 150 - MaxFailsMock = 20 - WeightMock = 10 - BackupMock = false - DownMock = false - ) - - BeforeAll(func() { - upstreamServer = ngxclient.UpstreamServer{ - ID: 1, - Server: "10.0.0.1:80", - MaxConns: &MaxConnsMock, - MaxFails: &MaxFailsMock, - FailTimeout: "test", - SlowStart: "test", - Route: "test", - Backup: &BackupMock, - Down: &DownMock, - Drain: false, - Weight: &WeightMock, - Service: "test", - } + BeforeEach(func() { + upstreamServer = ngxclient.UpstreamServer{} upstreamServers = []ngxclient.UpstreamServer{ upstreamServer, @@ -66,7 +46,7 @@ var _ = Describe("NGINX Runtime Manager", Ordered, func() { }) Context("Reload", func() { - BeforeAll(func() { + BeforeEach(func() { ngxPlusClient = &runtimefakes.FakeNginxPlusClient{} process = &runtimefakes.FakeProcessHandler{} metrics = &runtimefakes.FakeMetricsCollector{} @@ -74,12 +54,18 @@ var _ = Describe("NGINX Runtime Manager", Ordered, func() { manager = runtime.NewManagerImpl(ngxPlusClient, metrics, zap.New(), process, verifyClient) }) - It("sucessfully reloads NGINX configuration", func() { - err := manager.Reload(context.Background(), 1) - Expect(err).To(BeNil()) + It("NGINX configuration reload is successful", func() { + Expect(manager.Reload(context.Background(), 1)).To(Succeed()) + + Expect(process.FindMainProcessCallCount()).To(Equal(1)) + Expect(process.ReadFileCallCount()).To(Equal(1)) + Expect(process.KillCallCount()).To(Equal(1)) + Expect(metrics.IncReloadCountCallCount()).To(Equal(1)) + Expect(verifyClient.WaitForCorrectVersionCallCount()).To(Equal(1)) + Expect(metrics.ObserveLastReloadTimeCallCount()).To(Equal(1)) }) - When("not sucessfully reloads NGINX configuration", func() { + When("NGINX configuration reload is not successful", func() { It("should panic if MetricsCollector not enabled", func() { metrics = nil manager = runtime.NewManagerImpl(ngxPlusClient, metrics, zap.New(), process, verifyClient) @@ -106,15 +92,14 @@ var _ = Describe("NGINX Runtime Manager", Ordered, func() { }) When("running NGINX plus", func() { - BeforeAll(func() { + BeforeEach(func() { ngxPlusClient = &runtimefakes.FakeNginxPlusClient{} manager = runtime.NewManagerImpl(ngxPlusClient, nil, zap.New(), nil, nil) }) It("sucessfully updates HTTP server upstream", func() { - err := manager.UpdateHTTPServers("test", upstreamServers) - Expect(err).To(BeNil()) + Expect(manager.UpdateHTTPServers("test", upstreamServers)).To(Succeed()) }) It("returns no upstreams from NGINX Plus API", func() { @@ -126,7 +111,7 @@ var _ = Describe("NGINX Runtime Manager", Ordered, func() { }) When("not running NGINX plus", func() { - BeforeAll(func() { + BeforeEach(func() { ngxPlusClient = nil manager = runtime.NewManagerImpl(ngxPlusClient, nil, zap.New(), nil, nil) }) @@ -239,8 +224,9 @@ func TestFindMainProcess(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - - result, err := runtime.FindMainProcess(test.ctx, test.checkFile, test.readFile, 2*time.Millisecond) + p := runtime.ProcessHandlerImpl{} + result, err := p.FindMainProcess(test.ctx, test.checkFile, test.readFile, 2*time.Millisecond) + //result, err := runtime.FindMainProcess(test.ctx, test.checkFile, test.readFile, 2*time.Millisecond) if test.expectError { g.Expect(err).To(HaveOccurred()) diff --git a/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go b/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go index 4495c19dc3..b0b8e3ca74 100644 --- a/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go +++ b/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go @@ -27,6 +27,18 @@ type FakeProcessHandler struct { result1 int result2 error } + KillStub func(int, syscall.Signal) error + killMutex sync.RWMutex + killArgsForCall []struct { + arg1 int + arg2 syscall.Signal + } + killReturns struct { + result1 error + } + killReturnsOnCall map[int]struct { + result1 error + } ReadFileStub func(string) ([]byte, error) readFileMutex sync.RWMutex readFileArgsForCall []struct { @@ -40,18 +52,6 @@ type FakeProcessHandler struct { result1 []byte result2 error } - SysCallKillStub func(int, syscall.Signal) error - sysCallKillMutex sync.RWMutex - sysCallKillArgsForCall []struct { - arg1 int - arg2 syscall.Signal - } - sysCallKillReturns struct { - result1 error - } - sysCallKillReturnsOnCall map[int]struct { - result1 error - } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } @@ -123,6 +123,68 @@ func (fake *FakeProcessHandler) FindMainProcessReturnsOnCall(i int, result1 int, }{result1, result2} } +func (fake *FakeProcessHandler) Kill(arg1 int, arg2 syscall.Signal) error { + fake.killMutex.Lock() + ret, specificReturn := fake.killReturnsOnCall[len(fake.killArgsForCall)] + fake.killArgsForCall = append(fake.killArgsForCall, struct { + arg1 int + arg2 syscall.Signal + }{arg1, arg2}) + stub := fake.KillStub + fakeReturns := fake.killReturns + fake.recordInvocation("Kill", []interface{}{arg1, arg2}) + fake.killMutex.Unlock() + if stub != nil { + return stub(arg1, arg2) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeProcessHandler) KillCallCount() int { + fake.killMutex.RLock() + defer fake.killMutex.RUnlock() + return len(fake.killArgsForCall) +} + +func (fake *FakeProcessHandler) KillCalls(stub func(int, syscall.Signal) error) { + fake.killMutex.Lock() + defer fake.killMutex.Unlock() + fake.KillStub = stub +} + +func (fake *FakeProcessHandler) KillArgsForCall(i int) (int, syscall.Signal) { + fake.killMutex.RLock() + defer fake.killMutex.RUnlock() + argsForCall := fake.killArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *FakeProcessHandler) KillReturns(result1 error) { + fake.killMutex.Lock() + defer fake.killMutex.Unlock() + fake.KillStub = nil + fake.killReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeProcessHandler) KillReturnsOnCall(i int, result1 error) { + fake.killMutex.Lock() + defer fake.killMutex.Unlock() + fake.KillStub = nil + if fake.killReturnsOnCall == nil { + fake.killReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.killReturnsOnCall[i] = struct { + result1 error + }{result1} +} + func (fake *FakeProcessHandler) ReadFile(arg1 string) ([]byte, error) { fake.readFileMutex.Lock() ret, specificReturn := fake.readFileReturnsOnCall[len(fake.readFileArgsForCall)] @@ -187,77 +249,15 @@ func (fake *FakeProcessHandler) ReadFileReturnsOnCall(i int, result1 []byte, res }{result1, result2} } -func (fake *FakeProcessHandler) SysCallKill(arg1 int, arg2 syscall.Signal) error { - fake.sysCallKillMutex.Lock() - ret, specificReturn := fake.sysCallKillReturnsOnCall[len(fake.sysCallKillArgsForCall)] - fake.sysCallKillArgsForCall = append(fake.sysCallKillArgsForCall, struct { - arg1 int - arg2 syscall.Signal - }{arg1, arg2}) - stub := fake.SysCallKillStub - fakeReturns := fake.sysCallKillReturns - fake.recordInvocation("SysCallKill", []interface{}{arg1, arg2}) - fake.sysCallKillMutex.Unlock() - if stub != nil { - return stub(arg1, arg2) - } - if specificReturn { - return ret.result1 - } - return fakeReturns.result1 -} - -func (fake *FakeProcessHandler) SysCallKillCallCount() int { - fake.sysCallKillMutex.RLock() - defer fake.sysCallKillMutex.RUnlock() - return len(fake.sysCallKillArgsForCall) -} - -func (fake *FakeProcessHandler) SysCallKillCalls(stub func(int, syscall.Signal) error) { - fake.sysCallKillMutex.Lock() - defer fake.sysCallKillMutex.Unlock() - fake.SysCallKillStub = stub -} - -func (fake *FakeProcessHandler) SysCallKillArgsForCall(i int) (int, syscall.Signal) { - fake.sysCallKillMutex.RLock() - defer fake.sysCallKillMutex.RUnlock() - argsForCall := fake.sysCallKillArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2 -} - -func (fake *FakeProcessHandler) SysCallKillReturns(result1 error) { - fake.sysCallKillMutex.Lock() - defer fake.sysCallKillMutex.Unlock() - fake.SysCallKillStub = nil - fake.sysCallKillReturns = struct { - result1 error - }{result1} -} - -func (fake *FakeProcessHandler) SysCallKillReturnsOnCall(i int, result1 error) { - fake.sysCallKillMutex.Lock() - defer fake.sysCallKillMutex.Unlock() - fake.SysCallKillStub = nil - if fake.sysCallKillReturnsOnCall == nil { - fake.sysCallKillReturnsOnCall = make(map[int]struct { - result1 error - }) - } - fake.sysCallKillReturnsOnCall[i] = struct { - result1 error - }{result1} -} - func (fake *FakeProcessHandler) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() fake.findMainProcessMutex.RLock() defer fake.findMainProcessMutex.RUnlock() + fake.killMutex.RLock() + defer fake.killMutex.RUnlock() fake.readFileMutex.RLock() defer fake.readFileMutex.RUnlock() - fake.sysCallKillMutex.RLock() - defer fake.sysCallKillMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value diff --git a/internal/mode/static/nginx/runtime/verify.go b/internal/mode/static/nginx/runtime/verify.go index af688cc04d..ebcf9dd94b 100644 --- a/internal/mode/static/nginx/runtime/verify.go +++ b/internal/mode/static/nginx/runtime/verify.go @@ -27,15 +27,15 @@ type verifyClient interface { EnsureConfigVersion(ctx context.Context, expectedVersion int) error } -// verifyClientImpl is a client for verifying the config version. -type verifyClientImpl struct { +// VerifyClientImpl is a client for verifying the config version. +type VerifyClientImpl struct { client *http.Client timeout time.Duration } -// newVerifyClient returns a new client pointed at the config version socket. -func newVerifyClient(timeout time.Duration) *verifyClientImpl { - return &verifyClientImpl{ +// NewVerifyClient returns a new client pointed at the config version socket. +func NewVerifyClient(timeout time.Duration) *VerifyClientImpl { + return &VerifyClientImpl{ client: &http.Client{ Transport: &http.Transport{ DialContext: func(_ context.Context, _, _ string) (net.Conn, error) { @@ -49,7 +49,7 @@ func newVerifyClient(timeout time.Duration) *verifyClientImpl { // GetConfigVersion gets the version number that we put in the nginx config to verify that we're using // the correct config. -func (c *verifyClientImpl) GetConfigVersion() (int, error) { +func (c *VerifyClientImpl) GetConfigVersion() (int, error) { ctx, cancel := context.WithTimeout(context.Background(), c.timeout) defer cancel() @@ -82,7 +82,7 @@ func (c *verifyClientImpl) GetConfigVersion() (int, error) { // WaitForCorrectVersion first ensures any new worker processes have been started, and then calls the config version // endpoint until it gets the expectedVersion, which ensures that a new worker process has been started for that config // version. -func (c *verifyClientImpl) WaitForCorrectVersion( +func (c *VerifyClientImpl) WaitForCorrectVersion( ctx context.Context, expectedVersion int, childProcFile string, @@ -113,7 +113,7 @@ func (c *verifyClientImpl) WaitForCorrectVersion( return nil } -func (c *verifyClientImpl) EnsureConfigVersion(ctx context.Context, expectedVersion int) error { +func (c *VerifyClientImpl) EnsureConfigVersion(ctx context.Context, expectedVersion int) error { return wait.PollUntilContextCancel( ctx, 25*time.Millisecond, diff --git a/internal/mode/static/nginx/runtime/verify_test.go b/internal/mode/static/nginx/runtime/verify_test.go index 696489127b..9be83c5f53 100644 --- a/internal/mode/static/nginx/runtime/verify_test.go +++ b/internal/mode/static/nginx/runtime/verify_test.go @@ -30,7 +30,7 @@ func getTestHTTPClient() *http.Client { } func TestVerifyClient(t *testing.T) { - c := verifyClientImpl{ + c := VerifyClientImpl{ client: getTestHTTPClient(), timeout: 25 * time.Millisecond, } From df5ccbe3da32665538b0fd60172e21f05925b380 Mon Sep 17 00:00:00 2001 From: Mile Druzijanic Date: Mon, 6 May 2024 17:54:15 +0200 Subject: [PATCH 07/24] adding fixes --- internal/mode/static/manager.go | 4 ++-- internal/mode/static/nginx/runtime/manager.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 3c2dff88b6..10260101cf 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -156,7 +156,7 @@ func StartManager(cfg config.Config) error { var ngxPlusClient *ngxclient.NginxClient var usageSecret *usage.Secret - //var processHandler *ngxruntime.ProcessHandler + if cfg.Plus { ngxPlusClient, err = ngxruntime.CreatePlusClient() if err != nil { @@ -223,7 +223,7 @@ func StartManager(cfg config.Config) error { ngxruntimeCollector, cfg.Logger.WithName("nginxRuntimeManager"), &ngxruntime.ProcessHandlerImpl{}, - ngxruntime.NewVerifyClient(1*time.Second), + ngxruntime.NewVerifyClient(ngxruntime.NginxReloadTimeout), ), statusUpdater: groupStatusUpdater, eventRecorder: recorder, diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index ef42bf1244..9affe729db 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -21,7 +21,7 @@ import ( const ( PidFile = "/var/run/nginx/nginx.pid" pidFileTimeout = 10000 * time.Millisecond - nginxReloadTimeout = 60000 * time.Millisecond + NginxReloadTimeout = 60000 * time.Millisecond ) type ( From 3dc727ab3d674cd4d540e795c572e4f2bd379e44 Mon Sep 17 00:00:00 2001 From: Mile Druzijanic Date: Mon, 20 May 2024 16:38:02 +0200 Subject: [PATCH 08/24] adding processHandler struct update --- internal/mode/static/manager.go | 5 +- internal/mode/static/nginx/runtime/manager.go | 7 +- .../runtimefakes/fake_process_handler.go | 76 +++++++++++++++++++ 3 files changed, 83 insertions(+), 5 deletions(-) diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 10260101cf..fc56577f25 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -68,6 +68,7 @@ const ( ) var scheme = runtime.NewScheme() +var proccesHandler = &ngxruntime.ProcessHandlerImpl{} func init() { utilruntime.Must(gatewayv1beta1.Install(scheme)) @@ -145,7 +146,7 @@ func StartManager(cfg config.Config) error { } // Ensure NGINX is running before registering metrics & starting the manager. - if err := ngxruntime.EnsureNginxRunning(ctx, &ngxruntime.ProcessHandlerImpl{}); err != nil { + if err := proccesHandler.EnsureNginxRunning(ctx); err != nil { return fmt.Errorf("NGINX is not running: %w", err) } @@ -222,7 +223,7 @@ func StartManager(cfg config.Config) error { ngxPlusClient, ngxruntimeCollector, cfg.Logger.WithName("nginxRuntimeManager"), - &ngxruntime.ProcessHandlerImpl{}, + proccesHandler, ngxruntime.NewVerifyClient(ngxruntime.NginxReloadTimeout), ), statusUpdater: groupStatusUpdater, diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index 9affe729db..95d42322b1 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -38,12 +38,13 @@ type nginxPlusClient interface { GetUpstreams() (*ngxclient.Upstreams, error) } -//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . processHandler +//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . ProcessHandler type ProcessHandler interface { FindMainProcess(ctx context.Context, checkFile CheckFileFunc, readFile ReadFileFunc, timeout time.Duration) (int, error) ReadFile(file string) ([]byte, error) Kill(pid int, signum syscall.Signal) error + EnsureNginxRunning(ctx context.Context) error } type ProcessHandlerImpl struct { @@ -178,8 +179,8 @@ func (m *ManagerImpl) GetUpstreams() (ngxclient.Upstreams, error) { } // EnsureNginxRunning ensures NGINX is running by locating the main process. -func EnsureNginxRunning(ctx context.Context, processHandler ProcessHandler) error { - if _, err := processHandler.FindMainProcess(ctx, os.Stat, os.ReadFile, pidFileTimeout); err != nil { +func (p *ProcessHandlerImpl) EnsureNginxRunning(ctx context.Context) error { + if _, err := p.FindMainProcess(ctx, os.Stat, os.ReadFile, pidFileTimeout); err != nil { return fmt.Errorf("failed to find NGINX main process: %w", err) } return nil diff --git a/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go b/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go index b0b8e3ca74..c5d63ff2c7 100644 --- a/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go +++ b/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go @@ -11,6 +11,17 @@ import ( ) type FakeProcessHandler struct { + EnsureNginxRunningStub func(context.Context) error + ensureNginxRunningMutex sync.RWMutex + ensureNginxRunningArgsForCall []struct { + arg1 context.Context + } + ensureNginxRunningReturns struct { + result1 error + } + ensureNginxRunningReturnsOnCall map[int]struct { + result1 error + } FindMainProcessStub func(context.Context, runtime.CheckFileFunc, runtime.ReadFileFunc, time.Duration) (int, error) findMainProcessMutex sync.RWMutex findMainProcessArgsForCall []struct { @@ -56,6 +67,67 @@ type FakeProcessHandler struct { invocationsMutex sync.RWMutex } +func (fake *FakeProcessHandler) EnsureNginxRunning(arg1 context.Context) error { + fake.ensureNginxRunningMutex.Lock() + ret, specificReturn := fake.ensureNginxRunningReturnsOnCall[len(fake.ensureNginxRunningArgsForCall)] + fake.ensureNginxRunningArgsForCall = append(fake.ensureNginxRunningArgsForCall, struct { + arg1 context.Context + }{arg1}) + stub := fake.EnsureNginxRunningStub + fakeReturns := fake.ensureNginxRunningReturns + fake.recordInvocation("EnsureNginxRunning", []interface{}{arg1}) + fake.ensureNginxRunningMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeProcessHandler) EnsureNginxRunningCallCount() int { + fake.ensureNginxRunningMutex.RLock() + defer fake.ensureNginxRunningMutex.RUnlock() + return len(fake.ensureNginxRunningArgsForCall) +} + +func (fake *FakeProcessHandler) EnsureNginxRunningCalls(stub func(context.Context) error) { + fake.ensureNginxRunningMutex.Lock() + defer fake.ensureNginxRunningMutex.Unlock() + fake.EnsureNginxRunningStub = stub +} + +func (fake *FakeProcessHandler) EnsureNginxRunningArgsForCall(i int) context.Context { + fake.ensureNginxRunningMutex.RLock() + defer fake.ensureNginxRunningMutex.RUnlock() + argsForCall := fake.ensureNginxRunningArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeProcessHandler) EnsureNginxRunningReturns(result1 error) { + fake.ensureNginxRunningMutex.Lock() + defer fake.ensureNginxRunningMutex.Unlock() + fake.EnsureNginxRunningStub = nil + fake.ensureNginxRunningReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeProcessHandler) EnsureNginxRunningReturnsOnCall(i int, result1 error) { + fake.ensureNginxRunningMutex.Lock() + defer fake.ensureNginxRunningMutex.Unlock() + fake.EnsureNginxRunningStub = nil + if fake.ensureNginxRunningReturnsOnCall == nil { + fake.ensureNginxRunningReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.ensureNginxRunningReturnsOnCall[i] = struct { + result1 error + }{result1} +} + func (fake *FakeProcessHandler) FindMainProcess(arg1 context.Context, arg2 runtime.CheckFileFunc, arg3 runtime.ReadFileFunc, arg4 time.Duration) (int, error) { fake.findMainProcessMutex.Lock() ret, specificReturn := fake.findMainProcessReturnsOnCall[len(fake.findMainProcessArgsForCall)] @@ -252,6 +324,8 @@ func (fake *FakeProcessHandler) ReadFileReturnsOnCall(i int, result1 []byte, res func (fake *FakeProcessHandler) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() + fake.ensureNginxRunningMutex.RLock() + defer fake.ensureNginxRunningMutex.RUnlock() fake.findMainProcessMutex.RLock() defer fake.findMainProcessMutex.RUnlock() fake.killMutex.RLock() @@ -276,3 +350,5 @@ func (fake *FakeProcessHandler) recordInvocation(key string, args []interface{}) } fake.invocations[key] = append(fake.invocations[key], args) } + +var _ runtime.ProcessHandler = new(FakeProcessHandler) From a0c0228816772229081cdb8aa1914eca377ed47b Mon Sep 17 00:00:00 2001 From: Mile Druzijanic Date: Mon, 20 May 2024 17:20:52 +0200 Subject: [PATCH 09/24] adding cleanup fix update --- internal/mode/static/manager.go | 6 +++--- internal/mode/static/nginx/runtime/manager.go | 14 ++------------ internal/mode/static/nginx/runtime/manager_test.go | 8 +------- 3 files changed, 6 insertions(+), 22 deletions(-) diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index fc56577f25..d7a02dc500 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -68,7 +68,7 @@ const ( ) var scheme = runtime.NewScheme() -var proccesHandler = &ngxruntime.ProcessHandlerImpl{} +var processHandler = &ngxruntime.ProcessHandlerImpl{} func init() { utilruntime.Must(gatewayv1beta1.Install(scheme)) @@ -146,7 +146,7 @@ func StartManager(cfg config.Config) error { } // Ensure NGINX is running before registering metrics & starting the manager. - if err := proccesHandler.EnsureNginxRunning(ctx); err != nil { + if err := processHandler.EnsureNginxRunning(ctx); err != nil { return fmt.Errorf("NGINX is not running: %w", err) } @@ -223,7 +223,7 @@ func StartManager(cfg config.Config) error { ngxPlusClient, ngxruntimeCollector, cfg.Logger.WithName("nginxRuntimeManager"), - proccesHandler, + processHandler, ngxruntime.NewVerifyClient(ngxruntime.NginxReloadTimeout), ), statusUpdater: groupStatusUpdater, diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index 95d42322b1..b699630bc9 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -227,19 +227,9 @@ func (p *ProcessHandlerImpl) FindMainProcess( } func (p *ProcessHandlerImpl) ReadFile(file string) ([]byte, error) { - content, err := os.ReadFile(file) - if err != nil { - return nil, err - } - - return content, nil + return os.ReadFile(file) } func (p *ProcessHandlerImpl) Kill(pid int, signum syscall.Signal) error { - err := syscall.Kill(pid, syscall.SIGHUP) - if err != nil { - return err - } - - return nil + return syscall.Kill(pid, syscall.SIGHUP) } diff --git a/internal/mode/static/nginx/runtime/manager_test.go b/internal/mode/static/nginx/runtime/manager_test.go index 4c69090ca9..fbbd8f7d94 100644 --- a/internal/mode/static/nginx/runtime/manager_test.go +++ b/internal/mode/static/nginx/runtime/manager_test.go @@ -28,7 +28,6 @@ var _ = Describe("NGINX Runtime Manager", func() { var ( manager runtime.Manager upstreamServers []ngxclient.UpstreamServer - upstreamServer ngxclient.UpstreamServer ngxPlusClient *runtimefakes.FakeNginxPlusClient process *runtimefakes.FakeProcessHandler @@ -37,12 +36,9 @@ var _ = Describe("NGINX Runtime Manager", func() { ) BeforeEach(func() { - upstreamServer = ngxclient.UpstreamServer{} - upstreamServers = []ngxclient.UpstreamServer{ - upstreamServer, + ngxclient.UpstreamServer{}, } - }) Context("Reload", func() { @@ -98,7 +94,6 @@ var _ = Describe("NGINX Runtime Manager", func() { }) It("sucessfully updates HTTP server upstream", func() { - Expect(manager.UpdateHTTPServers("test", upstreamServers)).To(Succeed()) }) @@ -226,7 +221,6 @@ func TestFindMainProcess(t *testing.T) { g := NewWithT(t) p := runtime.ProcessHandlerImpl{} result, err := p.FindMainProcess(test.ctx, test.checkFile, test.readFile, 2*time.Millisecond) - //result, err := runtime.FindMainProcess(test.ctx, test.checkFile, test.readFile, 2*time.Millisecond) if test.expectError { g.Expect(err).To(HaveOccurred()) From 596efd3cf01e4dee550e763341d942ba48527d32 Mon Sep 17 00:00:00 2001 From: Mile Druzijanic Date: Mon, 20 May 2024 17:58:30 +0200 Subject: [PATCH 10/24] adding gofumpt fix update --- internal/mode/static/manager.go | 6 ++++-- internal/mode/static/nginx/runtime/manager.go | 3 +-- internal/mode/static/nginx/runtime/manager_test.go | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index d7a02dc500..37fd977ebc 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -67,8 +67,10 @@ const ( clusterTimeout = 10 * time.Second ) -var scheme = runtime.NewScheme() -var processHandler = &ngxruntime.ProcessHandlerImpl{} +var ( + scheme = runtime.NewScheme() + processHandler = &ngxruntime.ProcessHandlerImpl{} +) func init() { utilruntime.Must(gatewayv1beta1.Install(scheme)) diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index b699630bc9..dafef5fd8c 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -47,8 +47,7 @@ type ProcessHandler interface { EnsureNginxRunning(ctx context.Context) error } -type ProcessHandlerImpl struct { -} +type ProcessHandlerImpl struct{} //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . Manager diff --git a/internal/mode/static/nginx/runtime/manager_test.go b/internal/mode/static/nginx/runtime/manager_test.go index fbbd8f7d94..dfb2680ae2 100644 --- a/internal/mode/static/nginx/runtime/manager_test.go +++ b/internal/mode/static/nginx/runtime/manager_test.go @@ -37,7 +37,7 @@ var _ = Describe("NGINX Runtime Manager", func() { BeforeEach(func() { upstreamServers = []ngxclient.UpstreamServer{ - ngxclient.UpstreamServer{}, + {}, } }) From 54e1efee4069c33c1a3f6b67ea945dbcdcb4280c Mon Sep 17 00:00:00 2001 From: Mile Druzijanic Date: Mon, 20 May 2024 20:28:48 +0200 Subject: [PATCH 11/24] adding additional fix updates --- internal/mode/static/nginx/runtime/manager.go | 31 +++++++++++++------ .../mode/static/nginx/runtime/manager_test.go | 15 ++++++--- .../runtimefakes/fake_process_handler.go | 19 +++++------- internal/mode/static/nginx/runtime/verify.go | 8 ++++- 4 files changed, 47 insertions(+), 26 deletions(-) diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index dafef5fd8c..6921a2ec9a 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -34,16 +34,29 @@ var childProcPathFmt = "/proc/%[1]v/task/%[1]v/children" //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . nginxPlusClient type nginxPlusClient interface { - UpdateHTTPServers(upstream string, servers []ngxclient.UpstreamServer) (added []ngxclient.UpstreamServer, deleted []ngxclient.UpstreamServer, updated []ngxclient.UpstreamServer, err error) + UpdateHTTPServers( + upstream string, + servers []ngxclient.UpstreamServer, + ) ( + added []ngxclient.UpstreamServer, + deleted []ngxclient.UpstreamServer, + updated []ngxclient.UpstreamServer, + err error, + ) GetUpstreams() (*ngxclient.Upstreams, error) } //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . ProcessHandler type ProcessHandler interface { - FindMainProcess(ctx context.Context, checkFile CheckFileFunc, readFile ReadFileFunc, timeout time.Duration) (int, error) + FindMainProcess( + ctx context.Context, + checkFile CheckFileFunc, + readFile ReadFileFunc, + timeout time.Duration, + ) (int, error) ReadFile(file string) ([]byte, error) - Kill(pid int, signum syscall.Signal) error + Kill(pid int) error EnsureNginxRunning(ctx context.Context) error } @@ -76,11 +89,11 @@ type MetricsCollector interface { // ManagerImpl implements Manager. type ManagerImpl struct { - verifyClient verifyClient + processHandler ProcessHandler metricsCollector MetricsCollector + verifyClient verifyClient ngxPlusClient nginxPlusClient logger logr.Logger - processHandler ProcessHandler } // NewManagerImpl creates a new ManagerImpl. @@ -92,11 +105,11 @@ func NewManagerImpl( verifyClient verifyClient, ) *ManagerImpl { return &ManagerImpl{ - verifyClient: verifyClient, + processHandler: processHandler, metricsCollector: collector, + verifyClient: verifyClient, ngxPlusClient: ngxPlusClient, logger: logger, - processHandler: processHandler, } } @@ -121,7 +134,7 @@ func (m *ManagerImpl) Reload(ctx context.Context, configVersion int) error { // send HUP signal to the NGINX main process reload configuration // See https://nginx.org/en/docs/control.html - if err := m.processHandler.Kill(pid, syscall.SIGHUP); err != nil { + if err := m.processHandler.Kill(pid); err != nil { m.metricsCollector.IncReloadErrors() return fmt.Errorf("failed to send the HUP signal to NGINX main: %w", err) } @@ -229,6 +242,6 @@ func (p *ProcessHandlerImpl) ReadFile(file string) ([]byte, error) { return os.ReadFile(file) } -func (p *ProcessHandlerImpl) Kill(pid int, signum syscall.Signal) error { +func (p *ProcessHandlerImpl) Kill(pid int) error { return syscall.Kill(pid, syscall.SIGHUP) } diff --git a/internal/mode/static/nginx/runtime/manager_test.go b/internal/mode/static/nginx/runtime/manager_test.go index dfb2680ae2..7a4b510684 100644 --- a/internal/mode/static/nginx/runtime/manager_test.go +++ b/internal/mode/static/nginx/runtime/manager_test.go @@ -26,6 +26,7 @@ var _ = Describe("NGINX Runtime Manager", func() { }) var ( + err error manager runtime.Manager upstreamServers []ngxclient.UpstreamServer ngxPlusClient *runtimefakes.FakeNginxPlusClient @@ -67,10 +68,11 @@ var _ = Describe("NGINX Runtime Manager", func() { manager = runtime.NewManagerImpl(ngxPlusClient, metrics, zap.New(), process, verifyClient) reload := func() { - manager.Reload(context.Background(), 0) + err = manager.Reload(context.Background(), 0) } Expect(reload).To(Panic()) + Expect(err).ToNot(HaveOccurred()) }) It("should panic if VerifyClient not enabled", func() { @@ -79,10 +81,11 @@ var _ = Describe("NGINX Runtime Manager", func() { manager = runtime.NewManagerImpl(ngxPlusClient, metrics, zap.New(), process, verifyClient) reload := func() { - manager.Reload(context.Background(), 0) + err = manager.Reload(context.Background(), 0) } Expect(reload).To(Panic()) + Expect(err).ToNot(HaveOccurred()) }) }) }) @@ -93,7 +96,7 @@ var _ = Describe("NGINX Runtime Manager", func() { manager = runtime.NewManagerImpl(ngxPlusClient, nil, zap.New(), nil, nil) }) - It("sucessfully updates HTTP server upstream", func() { + It("successfully updates HTTP server upstream", func() { Expect(manager.UpdateHTTPServers("test", upstreamServers)).To(Succeed()) }) @@ -113,18 +116,20 @@ var _ = Describe("NGINX Runtime Manager", func() { It("should panic when updating HTTP upstream servers", func() { updateServers := func() { - manager.UpdateHTTPServers("test", upstreamServers) + err = manager.UpdateHTTPServers("test", upstreamServers) } Expect(updateServers).To(Panic()) + Expect(err).ToNot(HaveOccurred()) }) It("should panic when fetching HTTP upstream servers", func() { upstreams := func() { - manager.GetUpstreams() + _, err = manager.GetUpstreams() } Expect(upstreams).To(Panic()) + Expect(err).ToNot(HaveOccurred()) }) }) }) diff --git a/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go b/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go index c5d63ff2c7..de908c65d4 100644 --- a/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go +++ b/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go @@ -4,7 +4,6 @@ package runtimefakes import ( "context" "sync" - "syscall" "time" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime" @@ -38,11 +37,10 @@ type FakeProcessHandler struct { result1 int result2 error } - KillStub func(int, syscall.Signal) error + KillStub func(int) error killMutex sync.RWMutex killArgsForCall []struct { arg1 int - arg2 syscall.Signal } killReturns struct { result1 error @@ -195,19 +193,18 @@ func (fake *FakeProcessHandler) FindMainProcessReturnsOnCall(i int, result1 int, }{result1, result2} } -func (fake *FakeProcessHandler) Kill(arg1 int, arg2 syscall.Signal) error { +func (fake *FakeProcessHandler) Kill(arg1 int) error { fake.killMutex.Lock() ret, specificReturn := fake.killReturnsOnCall[len(fake.killArgsForCall)] fake.killArgsForCall = append(fake.killArgsForCall, struct { arg1 int - arg2 syscall.Signal - }{arg1, arg2}) + }{arg1}) stub := fake.KillStub fakeReturns := fake.killReturns - fake.recordInvocation("Kill", []interface{}{arg1, arg2}) + fake.recordInvocation("Kill", []interface{}{arg1}) fake.killMutex.Unlock() if stub != nil { - return stub(arg1, arg2) + return stub(arg1) } if specificReturn { return ret.result1 @@ -221,17 +218,17 @@ func (fake *FakeProcessHandler) KillCallCount() int { return len(fake.killArgsForCall) } -func (fake *FakeProcessHandler) KillCalls(stub func(int, syscall.Signal) error) { +func (fake *FakeProcessHandler) KillCalls(stub func(int) error) { fake.killMutex.Lock() defer fake.killMutex.Unlock() fake.KillStub = stub } -func (fake *FakeProcessHandler) KillArgsForCall(i int) (int, syscall.Signal) { +func (fake *FakeProcessHandler) KillArgsForCall(i int) int { fake.killMutex.RLock() defer fake.killMutex.RUnlock() argsForCall := fake.killArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2 + return argsForCall.arg1 } func (fake *FakeProcessHandler) KillReturns(result1 error) { diff --git a/internal/mode/static/nginx/runtime/verify.go b/internal/mode/static/nginx/runtime/verify.go index ebcf9dd94b..18ce52c1fc 100644 --- a/internal/mode/static/nginx/runtime/verify.go +++ b/internal/mode/static/nginx/runtime/verify.go @@ -23,7 +23,13 @@ var noNewWorkersErrFmt = "reload unsuccessful: no new NGINX worker processes sta type verifyClient interface { GetConfigVersion() (int, error) - WaitForCorrectVersion(ctx context.Context, expectedVersion int, childProcFile string, previousChildProcesses []byte, readFile ReadFileFunc) error + WaitForCorrectVersion( + ctx context.Context, + expectedVersion int, + childProcFile string, + previousChildProcesses []byte, + readFile ReadFileFunc, + ) error EnsureConfigVersion(ctx context.Context, expectedVersion int) error } From 39cc3d76720725d387ac7f70606b8a596ab7cff3 Mon Sep 17 00:00:00 2001 From: Mile Druzijanic Date: Thu, 27 Jun 2024 14:33:33 +0200 Subject: [PATCH 12/24] adding tests for runtime manager --- internal/mode/static/nginx/runtime/manager.go | 15 +++++---- .../mode/static/nginx/runtime/manager_test.go | 33 +++++++++++++------ internal/mode/static/nginx/runtime/verify.go | 2 +- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index 6921a2ec9a..f539060e4f 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -19,8 +19,11 @@ import ( //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate const ( - PidFile = "/var/run/nginx/nginx.pid" - pidFileTimeout = 10000 * time.Millisecond + // PidFile specifies the location of the PID file for the Nginx process + PidFile = "/var/run/nginx/nginx.pid" + // pidFileTimeout defines the timeout duration for accessing the PID file + pidFileTimeout = 10000 * time.Millisecond + /// NginxReloadTimeout sets the timeout duration for reloading the Nginx configuration NginxReloadTimeout = 60000 * time.Millisecond ) @@ -60,8 +63,6 @@ type ProcessHandler interface { EnsureNginxRunning(ctx context.Context) error } -type ProcessHandlerImpl struct{} - //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . Manager // Manager manages the runtime of NGINX. @@ -91,7 +92,7 @@ type MetricsCollector interface { type ManagerImpl struct { processHandler ProcessHandler metricsCollector MetricsCollector - verifyClient verifyClient + verifyClient nginxConfigVerifier ngxPlusClient nginxPlusClient logger logr.Logger } @@ -102,7 +103,7 @@ func NewManagerImpl( collector MetricsCollector, logger logr.Logger, processHandler ProcessHandler, - verifyClient verifyClient, + verifyClient nginxConfigVerifier, ) *ManagerImpl { return &ManagerImpl{ processHandler: processHandler, @@ -190,6 +191,8 @@ func (m *ManagerImpl) GetUpstreams() (ngxclient.Upstreams, error) { return *upstreams, nil } +type ProcessHandlerImpl struct{} + // EnsureNginxRunning ensures NGINX is running by locating the main process. func (p *ProcessHandlerImpl) EnsureNginxRunning(ctx context.Context) error { if _, err := p.FindMainProcess(ctx, os.Stat, os.ReadFile, pidFileTimeout); err != nil { diff --git a/internal/mode/static/nginx/runtime/manager_test.go b/internal/mode/static/nginx/runtime/manager_test.go index 7a4b510684..014fc0f374 100644 --- a/internal/mode/static/nginx/runtime/manager_test.go +++ b/internal/mode/static/nginx/runtime/manager_test.go @@ -51,15 +51,18 @@ var _ = Describe("NGINX Runtime Manager", func() { manager = runtime.NewManagerImpl(ngxPlusClient, metrics, zap.New(), process, verifyClient) }) - It("NGINX configuration reload is successful", func() { - Expect(manager.Reload(context.Background(), 1)).To(Succeed()) - - Expect(process.FindMainProcessCallCount()).To(Equal(1)) - Expect(process.ReadFileCallCount()).To(Equal(1)) - Expect(process.KillCallCount()).To(Equal(1)) - Expect(metrics.IncReloadCountCallCount()).To(Equal(1)) - Expect(verifyClient.WaitForCorrectVersionCallCount()).To(Equal(1)) - Expect(metrics.ObserveLastReloadTimeCallCount()).To(Equal(1)) + When("MetricsCollector is nil", func() { + It("NGINX configuration reload is successful", func() { + Expect(manager.Reload(context.Background(), 1)).To(Succeed()) + + Expect(process.FindMainProcessCallCount()).To(Equal(1)) + Expect(process.ReadFileCallCount()).To(Equal(1)) + Expect(process.KillCallCount()).To(Equal(1)) + Expect(metrics.IncReloadCountCallCount()).To(Equal(1)) + Expect(verifyClient.WaitForCorrectVersionCallCount()).To(Equal(1)) + Expect(metrics.ObserveLastReloadTimeCallCount()).To(Equal(1)) + Expect(metrics.IncReloadErrorsCallCount()).To(Equal(0)) + }) }) When("NGINX configuration reload is not successful", func() { @@ -100,12 +103,22 @@ var _ = Describe("NGINX Runtime Manager", func() { Expect(manager.UpdateHTTPServers("test", upstreamServers)).To(Succeed()) }) - It("returns no upstreams from NGINX Plus API", func() { + It("returns no upstreams from NGINX Plus API when upstreams are nil", func() { upstreams, err := manager.GetUpstreams() Expect(err).To(HaveOccurred()) Expect(upstreams).To(BeEmpty()) }) + + It("returns an error when GetUpstreams fails", func() { + ngxPlusClient.GetUpstreamsReturns(nil, errors.New("failed to get upstreams")) + + upstreams, err := manager.GetUpstreams() + + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError("failed to get upstreams")) + Expect(upstreams).To(BeNil()) + }) }) When("not running NGINX plus", func() { diff --git a/internal/mode/static/nginx/runtime/verify.go b/internal/mode/static/nginx/runtime/verify.go index 18ce52c1fc..e728ca122e 100644 --- a/internal/mode/static/nginx/runtime/verify.go +++ b/internal/mode/static/nginx/runtime/verify.go @@ -21,7 +21,7 @@ var noNewWorkersErrFmt = "reload unsuccessful: no new NGINX worker processes sta //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . verifyClient -type verifyClient interface { +type nginxConfigVerifier interface { GetConfigVersion() (int, error) WaitForCorrectVersion( ctx context.Context, From 7f6d1bf4f4f29542576347d9404bb9d17fa0ac27 Mon Sep 17 00:00:00 2001 From: Mile Druzijanic Date: Sat, 29 Jun 2024 14:32:33 +0200 Subject: [PATCH 13/24] resolving partionally comments in pr --- internal/mode/static/manager.go | 7 +++---- internal/mode/static/nginx/runtime/manager_test.go | 10 ++++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 37fd977ebc..57222d0887 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -67,10 +67,7 @@ const ( clusterTimeout = 10 * time.Second ) -var ( - scheme = runtime.NewScheme() - processHandler = &ngxruntime.ProcessHandlerImpl{} -) +var scheme = runtime.NewScheme() func init() { utilruntime.Must(gatewayv1beta1.Install(scheme)) @@ -147,6 +144,8 @@ func StartManager(cfg config.Config) error { return fmt.Errorf("cannot clear NGINX configuration folders: %w", err) } + var processHandler = &ngxruntime.ProcessHandlerImpl{} + // Ensure NGINX is running before registering metrics & starting the manager. if err := processHandler.EnsureNginxRunning(ctx); err != nil { return fmt.Errorf("NGINX is not running: %w", err) diff --git a/internal/mode/static/nginx/runtime/manager_test.go b/internal/mode/static/nginx/runtime/manager_test.go index 014fc0f374..781b6ddf8c 100644 --- a/internal/mode/static/nginx/runtime/manager_test.go +++ b/internal/mode/static/nginx/runtime/manager_test.go @@ -52,7 +52,7 @@ var _ = Describe("NGINX Runtime Manager", func() { }) When("MetricsCollector is nil", func() { - It("NGINX configuration reload is successful", func() { + It("Is successful", func() { Expect(manager.Reload(context.Background(), 1)).To(Succeed()) Expect(process.FindMainProcessCallCount()).To(Equal(1)) @@ -65,8 +65,8 @@ var _ = Describe("NGINX Runtime Manager", func() { }) }) - When("NGINX configuration reload is not successful", func() { - It("should panic if MetricsCollector not enabled", func() { + When("MetricsCollector is nil", func() { + It("panics", func() { metrics = nil manager = runtime.NewManagerImpl(ngxPlusClient, metrics, zap.New(), process, verifyClient) @@ -77,8 +77,10 @@ var _ = Describe("NGINX Runtime Manager", func() { Expect(reload).To(Panic()) Expect(err).ToNot(HaveOccurred()) }) + }) - It("should panic if VerifyClient not enabled", func() { + When("VerifyClient is nil", func() { + It("panics", func() { metrics = &runtimefakes.FakeMetricsCollector{} verifyClient = nil manager = runtime.NewManagerImpl(ngxPlusClient, metrics, zap.New(), process, verifyClient) From 0ecf5d14e6ce45b0e1ae7770218b2dfb5789fa8e Mon Sep 17 00:00:00 2001 From: Mile Druzijanic Date: Sun, 7 Jul 2024 16:33:36 +0200 Subject: [PATCH 14/24] adding first part of resolving final comments --- internal/mode/static/manager.go | 2 +- internal/mode/static/nginx/runtime/manager.go | 38 +++++++++---------- .../mode/static/nginx/runtime/manager_test.go | 4 +- .../runtimefakes/fake_process_handler.go | 38 +++++++++---------- internal/mode/static/nginx/runtime/verify.go | 14 +++---- 5 files changed, 44 insertions(+), 52 deletions(-) diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 57222d0887..045d167fd1 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -144,7 +144,7 @@ func StartManager(cfg config.Config) error { return fmt.Errorf("cannot clear NGINX configuration folders: %w", err) } - var processHandler = &ngxruntime.ProcessHandlerImpl{} + processHandler := &ngxruntime.NewProcessHandlerImpl{} // Ensure NGINX is running before registering metrics & starting the manager. if err := processHandler.EnsureNginxRunning(ctx); err != nil { diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index f539060e4f..700afca1ee 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -34,7 +34,7 @@ type ( var childProcPathFmt = "/proc/%[1]v/task/%[1]v/children" -//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . nginxPlusClient +//counterfeiter:generate . nginxPlusClient type nginxPlusClient interface { UpdateHTTPServers( @@ -49,21 +49,19 @@ type nginxPlusClient interface { GetUpstreams() (*ngxclient.Upstreams, error) } -//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . ProcessHandler +//counterfeiter:generate . ProcessHandler type ProcessHandler interface { FindMainProcess( ctx context.Context, - checkFile CheckFileFunc, - readFile ReadFileFunc, timeout time.Duration, ) (int, error) - ReadFile(file string) ([]byte, error) + readFile(file string) ([]byte, error) Kill(pid int) error EnsureNginxRunning(ctx context.Context) error } -//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . Manager +//counterfeiter:generate . Manager // Manager manages the runtime of NGINX. type Manager interface { @@ -79,9 +77,9 @@ type Manager interface { GetUpstreams() (ngxclient.Upstreams, error) } -//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . MetricsCollector - // MetricsCollector is an interface for the metrics of the NGINX runtime manager. +// +//counterfeiter:generate . MetricsCollector type MetricsCollector interface { IncReloadCount() IncReloadErrors() @@ -122,13 +120,13 @@ func (m *ManagerImpl) IsPlus() bool { func (m *ManagerImpl) Reload(ctx context.Context, configVersion int) error { start := time.Now() // We find the main NGINX PID on every reload because it will change if the NGINX container is restarted. - pid, err := m.processHandler.FindMainProcess(ctx, os.Stat, os.ReadFile, pidFileTimeout) + pid, err := m.processHandler.FindMainProcess(ctx, pidFileTimeout) if err != nil { return fmt.Errorf("failed to find NGINX main process: %w", err) } childProcFile := fmt.Sprintf(childProcPathFmt, pid) - previousChildProcesses, err := m.processHandler.ReadFile(childProcFile) + previousChildProcesses, err := m.processHandler.readFile(childProcFile) if err != nil { return err } @@ -191,20 +189,18 @@ func (m *ManagerImpl) GetUpstreams() (ngxclient.Upstreams, error) { return *upstreams, nil } -type ProcessHandlerImpl struct{} +type NewProcessHandlerImpl struct{} // EnsureNginxRunning ensures NGINX is running by locating the main process. -func (p *ProcessHandlerImpl) EnsureNginxRunning(ctx context.Context) error { - if _, err := p.FindMainProcess(ctx, os.Stat, os.ReadFile, pidFileTimeout); err != nil { +func (p *NewProcessHandlerImpl) EnsureNginxRunning(ctx context.Context) error { + if _, err := p.FindMainProcess(ctx, pidFileTimeout); err != nil { return fmt.Errorf("failed to find NGINX main process: %w", err) } return nil } -func (p *ProcessHandlerImpl) FindMainProcess( +func (p *NewProcessHandlerImpl) FindMainProcess( ctx context.Context, - checkFile CheckFileFunc, - readFile ReadFileFunc, timeout time.Duration, ) (int, error) { ctx, cancel := context.WithTimeout(ctx, timeout) @@ -215,7 +211,7 @@ func (p *ProcessHandlerImpl) FindMainProcess( 500*time.Millisecond, true, /* poll immediately */ func(_ context.Context) (bool, error) { - _, err := checkFile(PidFile) + _, err := p.checkFile(PidFile) if err == nil { return true, nil } @@ -228,7 +224,7 @@ func (p *ProcessHandlerImpl) FindMainProcess( return 0, err } - content, err := readFile(PidFile) + content, err := p.readFile(PidFile) if err != nil { return 0, err } @@ -241,10 +237,10 @@ func (p *ProcessHandlerImpl) FindMainProcess( return pid, nil } -func (p *ProcessHandlerImpl) ReadFile(file string) ([]byte, error) { - return os.ReadFile(file) +func (p *NewProcessHandlerImpl) readFile(file string) ([]byte, error) { + return p.readFile(file) } -func (p *ProcessHandlerImpl) Kill(pid int) error { +func (p *NewProcessHandlerImpl) Kill(pid int) error { return syscall.Kill(pid, syscall.SIGHUP) } diff --git a/internal/mode/static/nginx/runtime/manager_test.go b/internal/mode/static/nginx/runtime/manager_test.go index 781b6ddf8c..4b85f00a3f 100644 --- a/internal/mode/static/nginx/runtime/manager_test.go +++ b/internal/mode/static/nginx/runtime/manager_test.go @@ -239,8 +239,8 @@ func TestFindMainProcess(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - p := runtime.ProcessHandlerImpl{} - result, err := p.FindMainProcess(test.ctx, test.checkFile, test.readFile, 2*time.Millisecond) + p := runtime.NewProcessHandlerImpl{} + result, err := p.FindMainProcess(test.ctx, 2*time.Millisecond) if test.expectError { g.Expect(err).To(HaveOccurred()) diff --git a/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go b/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go index de908c65d4..970c0aeecf 100644 --- a/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go +++ b/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go @@ -21,13 +21,11 @@ type FakeProcessHandler struct { ensureNginxRunningReturnsOnCall map[int]struct { result1 error } - FindMainProcessStub func(context.Context, runtime.CheckFileFunc, runtime.ReadFileFunc, time.Duration) (int, error) + FindMainProcessStub func(context.Context, time.Duration) (int, error) findMainProcessMutex sync.RWMutex findMainProcessArgsForCall []struct { arg1 context.Context - arg2 runtime.CheckFileFunc - arg3 runtime.ReadFileFunc - arg4 time.Duration + arg2 time.Duration } findMainProcessReturns struct { result1 int @@ -48,7 +46,7 @@ type FakeProcessHandler struct { killReturnsOnCall map[int]struct { result1 error } - ReadFileStub func(string) ([]byte, error) + readFileStub func(string) ([]byte, error) readFileMutex sync.RWMutex readFileArgsForCall []struct { arg1 string @@ -126,21 +124,19 @@ func (fake *FakeProcessHandler) EnsureNginxRunningReturnsOnCall(i int, result1 e }{result1} } -func (fake *FakeProcessHandler) FindMainProcess(arg1 context.Context, arg2 runtime.CheckFileFunc, arg3 runtime.ReadFileFunc, arg4 time.Duration) (int, error) { +func (fake *FakeProcessHandler) FindMainProcess(arg1 context.Context, arg2 time.Duration) (int, error) { fake.findMainProcessMutex.Lock() ret, specificReturn := fake.findMainProcessReturnsOnCall[len(fake.findMainProcessArgsForCall)] fake.findMainProcessArgsForCall = append(fake.findMainProcessArgsForCall, struct { arg1 context.Context - arg2 runtime.CheckFileFunc - arg3 runtime.ReadFileFunc - arg4 time.Duration - }{arg1, arg2, arg3, arg4}) + arg2 time.Duration + }{arg1, arg2}) stub := fake.FindMainProcessStub fakeReturns := fake.findMainProcessReturns - fake.recordInvocation("FindMainProcess", []interface{}{arg1, arg2, arg3, arg4}) + fake.recordInvocation("FindMainProcess", []interface{}{arg1, arg2}) fake.findMainProcessMutex.Unlock() if stub != nil { - return stub(arg1, arg2, arg3, arg4) + return stub(arg1, arg2) } if specificReturn { return ret.result1, ret.result2 @@ -154,17 +150,17 @@ func (fake *FakeProcessHandler) FindMainProcessCallCount() int { return len(fake.findMainProcessArgsForCall) } -func (fake *FakeProcessHandler) FindMainProcessCalls(stub func(context.Context, runtime.CheckFileFunc, runtime.ReadFileFunc, time.Duration) (int, error)) { +func (fake *FakeProcessHandler) FindMainProcessCalls(stub func(context.Context, time.Duration) (int, error)) { fake.findMainProcessMutex.Lock() defer fake.findMainProcessMutex.Unlock() fake.FindMainProcessStub = stub } -func (fake *FakeProcessHandler) FindMainProcessArgsForCall(i int) (context.Context, runtime.CheckFileFunc, runtime.ReadFileFunc, time.Duration) { +func (fake *FakeProcessHandler) FindMainProcessArgsForCall(i int) (context.Context, time.Duration) { fake.findMainProcessMutex.RLock() defer fake.findMainProcessMutex.RUnlock() argsForCall := fake.findMainProcessArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4 + return argsForCall.arg1, argsForCall.arg2 } func (fake *FakeProcessHandler) FindMainProcessReturns(result1 int, result2 error) { @@ -254,15 +250,15 @@ func (fake *FakeProcessHandler) KillReturnsOnCall(i int, result1 error) { }{result1} } -func (fake *FakeProcessHandler) ReadFile(arg1 string) ([]byte, error) { +func (fake *FakeProcessHandler) readFile(arg1 string) ([]byte, error) { fake.readFileMutex.Lock() ret, specificReturn := fake.readFileReturnsOnCall[len(fake.readFileArgsForCall)] fake.readFileArgsForCall = append(fake.readFileArgsForCall, struct { arg1 string }{arg1}) - stub := fake.ReadFileStub + stub := fake.readFileStub fakeReturns := fake.readFileReturns - fake.recordInvocation("ReadFile", []interface{}{arg1}) + fake.recordInvocation("readFile", []interface{}{arg1}) fake.readFileMutex.Unlock() if stub != nil { return stub(arg1) @@ -282,7 +278,7 @@ func (fake *FakeProcessHandler) ReadFileCallCount() int { func (fake *FakeProcessHandler) ReadFileCalls(stub func(string) ([]byte, error)) { fake.readFileMutex.Lock() defer fake.readFileMutex.Unlock() - fake.ReadFileStub = stub + fake.readFileStub = stub } func (fake *FakeProcessHandler) ReadFileArgsForCall(i int) string { @@ -295,7 +291,7 @@ func (fake *FakeProcessHandler) ReadFileArgsForCall(i int) string { func (fake *FakeProcessHandler) ReadFileReturns(result1 []byte, result2 error) { fake.readFileMutex.Lock() defer fake.readFileMutex.Unlock() - fake.ReadFileStub = nil + fake.readFileStub = nil fake.readFileReturns = struct { result1 []byte result2 error @@ -305,7 +301,7 @@ func (fake *FakeProcessHandler) ReadFileReturns(result1 []byte, result2 error) { func (fake *FakeProcessHandler) ReadFileReturnsOnCall(i int, result1 []byte, result2 error) { fake.readFileMutex.Lock() defer fake.readFileMutex.Unlock() - fake.ReadFileStub = nil + fake.readFileStub = nil if fake.readFileReturnsOnCall == nil { fake.readFileReturnsOnCall = make(map[int]struct { result1 []byte diff --git a/internal/mode/static/nginx/runtime/verify.go b/internal/mode/static/nginx/runtime/verify.go index e728ca122e..0b69df1152 100644 --- a/internal/mode/static/nginx/runtime/verify.go +++ b/internal/mode/static/nginx/runtime/verify.go @@ -33,15 +33,15 @@ type nginxConfigVerifier interface { EnsureConfigVersion(ctx context.Context, expectedVersion int) error } -// VerifyClientImpl is a client for verifying the config version. -type VerifyClientImpl struct { +// VerifyClient is a client for verifying the config version. +type VerifyClient struct { client *http.Client timeout time.Duration } // NewVerifyClient returns a new client pointed at the config version socket. -func NewVerifyClient(timeout time.Duration) *VerifyClientImpl { - return &VerifyClientImpl{ +func NewVerifyClient(timeout time.Duration) *VerifyClient { + return &VerifyClient{ client: &http.Client{ Transport: &http.Transport{ DialContext: func(_ context.Context, _, _ string) (net.Conn, error) { @@ -55,7 +55,7 @@ func NewVerifyClient(timeout time.Duration) *VerifyClientImpl { // GetConfigVersion gets the version number that we put in the nginx config to verify that we're using // the correct config. -func (c *VerifyClientImpl) GetConfigVersion() (int, error) { +func (c *VerifyClient) GetConfigVersion() (int, error) { ctx, cancel := context.WithTimeout(context.Background(), c.timeout) defer cancel() @@ -88,7 +88,7 @@ func (c *VerifyClientImpl) GetConfigVersion() (int, error) { // WaitForCorrectVersion first ensures any new worker processes have been started, and then calls the config version // endpoint until it gets the expectedVersion, which ensures that a new worker process has been started for that config // version. -func (c *VerifyClientImpl) WaitForCorrectVersion( +func (c *VerifyClient) WaitForCorrectVersion( ctx context.Context, expectedVersion int, childProcFile string, @@ -119,7 +119,7 @@ func (c *VerifyClientImpl) WaitForCorrectVersion( return nil } -func (c *VerifyClientImpl) EnsureConfigVersion(ctx context.Context, expectedVersion int) error { +func (c *VerifyClient) EnsureConfigVersion(ctx context.Context, expectedVersion int) error { return wait.PollUntilContextCancel( ctx, 25*time.Millisecond, From ecc39c58e6772214bf1d0024e5f1b1831fd37369 Mon Sep 17 00:00:00 2001 From: Mile Druzijanic Date: Sun, 7 Jul 2024 19:28:21 +0200 Subject: [PATCH 15/24] experimental commit for NewProcessHandlerImpl --- internal/mode/static/manager.go | 2 +- internal/mode/static/nginx/runtime/manager.go | 30 +++---- .../runtimefakes/fake_process_handler.go | 88 ++----------------- .../mode/static/nginx/runtime/verify_test.go | 2 +- 4 files changed, 23 insertions(+), 99 deletions(-) diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 045d167fd1..c889ddbd3e 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -147,7 +147,7 @@ func StartManager(cfg config.Config) error { processHandler := &ngxruntime.NewProcessHandlerImpl{} // Ensure NGINX is running before registering metrics & starting the manager. - if err := processHandler.EnsureNginxRunning(ctx); err != nil { + if _, err := processHandler.FindMainProcess(ctx, ngxruntime.PidFileTimeout); err != nil { return fmt.Errorf("NGINX is not running: %w", err) } diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index 700afca1ee..4a3bbf5c50 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -22,7 +22,7 @@ const ( // PidFile specifies the location of the PID file for the Nginx process PidFile = "/var/run/nginx/nginx.pid" // pidFileTimeout defines the timeout duration for accessing the PID file - pidFileTimeout = 10000 * time.Millisecond + PidFileTimeout = 10000 * time.Millisecond /// NginxReloadTimeout sets the timeout duration for reloading the Nginx configuration NginxReloadTimeout = 60000 * time.Millisecond ) @@ -56,9 +56,13 @@ type ProcessHandler interface { ctx context.Context, timeout time.Duration, ) (int, error) - readFile(file string) ([]byte, error) + ReadFile(file string) ([]byte, error) Kill(pid int) error - EnsureNginxRunning(ctx context.Context) error +} + +type ProcessHandlerImpl struct { + ReadFile ReadFileFunc + checkFile CheckFileFunc } //counterfeiter:generate . Manager @@ -120,13 +124,13 @@ func (m *ManagerImpl) IsPlus() bool { func (m *ManagerImpl) Reload(ctx context.Context, configVersion int) error { start := time.Now() // We find the main NGINX PID on every reload because it will change if the NGINX container is restarted. - pid, err := m.processHandler.FindMainProcess(ctx, pidFileTimeout) + pid, err := m.processHandler.FindMainProcess(ctx, PidFileTimeout) if err != nil { return fmt.Errorf("failed to find NGINX main process: %w", err) } childProcFile := fmt.Sprintf(childProcPathFmt, pid) - previousChildProcesses, err := m.processHandler.readFile(childProcFile) + previousChildProcesses, err := m.processHandler.ReadFile(childProcFile) if err != nil { return err } @@ -189,14 +193,8 @@ func (m *ManagerImpl) GetUpstreams() (ngxclient.Upstreams, error) { return *upstreams, nil } -type NewProcessHandlerImpl struct{} - -// EnsureNginxRunning ensures NGINX is running by locating the main process. -func (p *NewProcessHandlerImpl) EnsureNginxRunning(ctx context.Context) error { - if _, err := p.FindMainProcess(ctx, pidFileTimeout); err != nil { - return fmt.Errorf("failed to find NGINX main process: %w", err) - } - return nil +type NewProcessHandlerImpl struct { + checkFile func(file string) (string, error) } func (p *NewProcessHandlerImpl) FindMainProcess( @@ -224,7 +222,7 @@ func (p *NewProcessHandlerImpl) FindMainProcess( return 0, err } - content, err := p.readFile(PidFile) + content, err := p.ReadFile(PidFile) if err != nil { return 0, err } @@ -237,8 +235,8 @@ func (p *NewProcessHandlerImpl) FindMainProcess( return pid, nil } -func (p *NewProcessHandlerImpl) readFile(file string) ([]byte, error) { - return p.readFile(file) +func (p *NewProcessHandlerImpl) ReadFile(file string) ([]byte, error) { + return p.ReadFile(file) } func (p *NewProcessHandlerImpl) Kill(pid int) error { diff --git a/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go b/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go index 970c0aeecf..bae52e2fa7 100644 --- a/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go +++ b/internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go @@ -10,17 +10,6 @@ import ( ) type FakeProcessHandler struct { - EnsureNginxRunningStub func(context.Context) error - ensureNginxRunningMutex sync.RWMutex - ensureNginxRunningArgsForCall []struct { - arg1 context.Context - } - ensureNginxRunningReturns struct { - result1 error - } - ensureNginxRunningReturnsOnCall map[int]struct { - result1 error - } FindMainProcessStub func(context.Context, time.Duration) (int, error) findMainProcessMutex sync.RWMutex findMainProcessArgsForCall []struct { @@ -46,7 +35,7 @@ type FakeProcessHandler struct { killReturnsOnCall map[int]struct { result1 error } - readFileStub func(string) ([]byte, error) + ReadFileStub func(string) ([]byte, error) readFileMutex sync.RWMutex readFileArgsForCall []struct { arg1 string @@ -63,67 +52,6 @@ type FakeProcessHandler struct { invocationsMutex sync.RWMutex } -func (fake *FakeProcessHandler) EnsureNginxRunning(arg1 context.Context) error { - fake.ensureNginxRunningMutex.Lock() - ret, specificReturn := fake.ensureNginxRunningReturnsOnCall[len(fake.ensureNginxRunningArgsForCall)] - fake.ensureNginxRunningArgsForCall = append(fake.ensureNginxRunningArgsForCall, struct { - arg1 context.Context - }{arg1}) - stub := fake.EnsureNginxRunningStub - fakeReturns := fake.ensureNginxRunningReturns - fake.recordInvocation("EnsureNginxRunning", []interface{}{arg1}) - fake.ensureNginxRunningMutex.Unlock() - if stub != nil { - return stub(arg1) - } - if specificReturn { - return ret.result1 - } - return fakeReturns.result1 -} - -func (fake *FakeProcessHandler) EnsureNginxRunningCallCount() int { - fake.ensureNginxRunningMutex.RLock() - defer fake.ensureNginxRunningMutex.RUnlock() - return len(fake.ensureNginxRunningArgsForCall) -} - -func (fake *FakeProcessHandler) EnsureNginxRunningCalls(stub func(context.Context) error) { - fake.ensureNginxRunningMutex.Lock() - defer fake.ensureNginxRunningMutex.Unlock() - fake.EnsureNginxRunningStub = stub -} - -func (fake *FakeProcessHandler) EnsureNginxRunningArgsForCall(i int) context.Context { - fake.ensureNginxRunningMutex.RLock() - defer fake.ensureNginxRunningMutex.RUnlock() - argsForCall := fake.ensureNginxRunningArgsForCall[i] - return argsForCall.arg1 -} - -func (fake *FakeProcessHandler) EnsureNginxRunningReturns(result1 error) { - fake.ensureNginxRunningMutex.Lock() - defer fake.ensureNginxRunningMutex.Unlock() - fake.EnsureNginxRunningStub = nil - fake.ensureNginxRunningReturns = struct { - result1 error - }{result1} -} - -func (fake *FakeProcessHandler) EnsureNginxRunningReturnsOnCall(i int, result1 error) { - fake.ensureNginxRunningMutex.Lock() - defer fake.ensureNginxRunningMutex.Unlock() - fake.EnsureNginxRunningStub = nil - if fake.ensureNginxRunningReturnsOnCall == nil { - fake.ensureNginxRunningReturnsOnCall = make(map[int]struct { - result1 error - }) - } - fake.ensureNginxRunningReturnsOnCall[i] = struct { - result1 error - }{result1} -} - func (fake *FakeProcessHandler) FindMainProcess(arg1 context.Context, arg2 time.Duration) (int, error) { fake.findMainProcessMutex.Lock() ret, specificReturn := fake.findMainProcessReturnsOnCall[len(fake.findMainProcessArgsForCall)] @@ -250,15 +178,15 @@ func (fake *FakeProcessHandler) KillReturnsOnCall(i int, result1 error) { }{result1} } -func (fake *FakeProcessHandler) readFile(arg1 string) ([]byte, error) { +func (fake *FakeProcessHandler) ReadFile(arg1 string) ([]byte, error) { fake.readFileMutex.Lock() ret, specificReturn := fake.readFileReturnsOnCall[len(fake.readFileArgsForCall)] fake.readFileArgsForCall = append(fake.readFileArgsForCall, struct { arg1 string }{arg1}) - stub := fake.readFileStub + stub := fake.ReadFileStub fakeReturns := fake.readFileReturns - fake.recordInvocation("readFile", []interface{}{arg1}) + fake.recordInvocation("ReadFile", []interface{}{arg1}) fake.readFileMutex.Unlock() if stub != nil { return stub(arg1) @@ -278,7 +206,7 @@ func (fake *FakeProcessHandler) ReadFileCallCount() int { func (fake *FakeProcessHandler) ReadFileCalls(stub func(string) ([]byte, error)) { fake.readFileMutex.Lock() defer fake.readFileMutex.Unlock() - fake.readFileStub = stub + fake.ReadFileStub = stub } func (fake *FakeProcessHandler) ReadFileArgsForCall(i int) string { @@ -291,7 +219,7 @@ func (fake *FakeProcessHandler) ReadFileArgsForCall(i int) string { func (fake *FakeProcessHandler) ReadFileReturns(result1 []byte, result2 error) { fake.readFileMutex.Lock() defer fake.readFileMutex.Unlock() - fake.readFileStub = nil + fake.ReadFileStub = nil fake.readFileReturns = struct { result1 []byte result2 error @@ -301,7 +229,7 @@ func (fake *FakeProcessHandler) ReadFileReturns(result1 []byte, result2 error) { func (fake *FakeProcessHandler) ReadFileReturnsOnCall(i int, result1 []byte, result2 error) { fake.readFileMutex.Lock() defer fake.readFileMutex.Unlock() - fake.readFileStub = nil + fake.ReadFileStub = nil if fake.readFileReturnsOnCall == nil { fake.readFileReturnsOnCall = make(map[int]struct { result1 []byte @@ -317,8 +245,6 @@ func (fake *FakeProcessHandler) ReadFileReturnsOnCall(i int, result1 []byte, res func (fake *FakeProcessHandler) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() - fake.ensureNginxRunningMutex.RLock() - defer fake.ensureNginxRunningMutex.RUnlock() fake.findMainProcessMutex.RLock() defer fake.findMainProcessMutex.RUnlock() fake.killMutex.RLock() diff --git a/internal/mode/static/nginx/runtime/verify_test.go b/internal/mode/static/nginx/runtime/verify_test.go index 9be83c5f53..34982f48be 100644 --- a/internal/mode/static/nginx/runtime/verify_test.go +++ b/internal/mode/static/nginx/runtime/verify_test.go @@ -30,7 +30,7 @@ func getTestHTTPClient() *http.Client { } func TestVerifyClient(t *testing.T) { - c := VerifyClientImpl{ + c := VerifyClient{ client: getTestHTTPClient(), timeout: 25 * time.Millisecond, } From f146435857fc43e09af6fa14f12edd6ef65e29a5 Mon Sep 17 00:00:00 2001 From: Mile Date: Sat, 13 Jul 2024 16:48:37 +0200 Subject: [PATCH 16/24] updating test desc --- .../mode/static/nginx/runtime/manager_test.go | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/internal/mode/static/nginx/runtime/manager_test.go b/internal/mode/static/nginx/runtime/manager_test.go index 4b85f00a3f..b7acfb52b1 100644 --- a/internal/mode/static/nginx/runtime/manager_test.go +++ b/internal/mode/static/nginx/runtime/manager_test.go @@ -51,18 +51,16 @@ var _ = Describe("NGINX Runtime Manager", func() { manager = runtime.NewManagerImpl(ngxPlusClient, metrics, zap.New(), process, verifyClient) }) - When("MetricsCollector is nil", func() { - It("Is successful", func() { - Expect(manager.Reload(context.Background(), 1)).To(Succeed()) - - Expect(process.FindMainProcessCallCount()).To(Equal(1)) - Expect(process.ReadFileCallCount()).To(Equal(1)) - Expect(process.KillCallCount()).To(Equal(1)) - Expect(metrics.IncReloadCountCallCount()).To(Equal(1)) - Expect(verifyClient.WaitForCorrectVersionCallCount()).To(Equal(1)) - Expect(metrics.ObserveLastReloadTimeCallCount()).To(Equal(1)) - Expect(metrics.IncReloadErrorsCallCount()).To(Equal(0)) - }) + It("Is successful", func() { + Expect(manager.Reload(context.Background(), 1)).To(Succeed()) + + Expect(process.FindMainProcessCallCount()).To(Equal(1)) + Expect(process.ReadFileCallCount()).To(Equal(1)) + Expect(process.KillCallCount()).To(Equal(1)) + Expect(metrics.IncReloadCountCallCount()).To(Equal(1)) + Expect(verifyClient.WaitForCorrectVersionCallCount()).To(Equal(1)) + Expect(metrics.ObserveLastReloadTimeCallCount()).To(Equal(1)) + Expect(metrics.IncReloadErrorsCallCount()).To(Equal(0)) }) When("MetricsCollector is nil", func() { From b7884d0e8280bdc88901b009fde7390d1b037cc2 Mon Sep 17 00:00:00 2001 From: Mile Date: Sat, 13 Jul 2024 16:54:49 +0200 Subject: [PATCH 17/24] adding required updates regarding readFile --- internal/mode/static/nginx/runtime/manager.go | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index 4a3bbf5c50..80edf5bd69 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -56,12 +56,12 @@ type ProcessHandler interface { ctx context.Context, timeout time.Duration, ) (int, error) - ReadFile(file string) ([]byte, error) + readFile(file string) ([]byte, error) Kill(pid int) error } type ProcessHandlerImpl struct { - ReadFile ReadFileFunc + readFile ReadFileFunc checkFile CheckFileFunc } @@ -130,7 +130,7 @@ func (m *ManagerImpl) Reload(ctx context.Context, configVersion int) error { } childProcFile := fmt.Sprintf(childProcPathFmt, pid) - previousChildProcesses, err := m.processHandler.ReadFile(childProcFile) + previousChildProcesses, err := m.processHandler.readFile(childProcFile) if err != nil { return err } @@ -193,11 +193,16 @@ func (m *ManagerImpl) GetUpstreams() (ngxclient.Upstreams, error) { return *upstreams, nil } -type NewProcessHandlerImpl struct { - checkFile func(file string) (string, error) +func NewProcessHandlerImpl(readFile ReadFileFunc, checkFile CheckFileFunc) *ProcessHandlerImpl { + ph := &ProcessHandlerImpl{ + readFile: readFile, + checkFile: checkFile, + } + + return ph } -func (p *NewProcessHandlerImpl) FindMainProcess( +func (p *ProcessHandlerImpl) FindMainProcess( ctx context.Context, timeout time.Duration, ) (int, error) { @@ -222,7 +227,7 @@ func (p *NewProcessHandlerImpl) FindMainProcess( return 0, err } - content, err := p.ReadFile(PidFile) + content, err := p.readFile(PidFile) if err != nil { return 0, err } @@ -235,10 +240,10 @@ func (p *NewProcessHandlerImpl) FindMainProcess( return pid, nil } -func (p *NewProcessHandlerImpl) ReadFile(file string) ([]byte, error) { +func (p *ProcessHandlerImpl) ReadFile(file string) ([]byte, error) { return p.ReadFile(file) } -func (p *NewProcessHandlerImpl) Kill(pid int) error { +func (p *ProcessHandlerImpl) Kill(pid int) error { return syscall.Kill(pid, syscall.SIGHUP) } From 04f6e306065c2c078bebb338eb5a2092b76daae3 Mon Sep 17 00:00:00 2001 From: Mile Date: Sat, 13 Jul 2024 23:08:45 +0200 Subject: [PATCH 18/24] adding additional experimental ReadFile update --- internal/mode/static/nginx/runtime/manager.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index 80edf5bd69..1a5e9a24a1 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -56,7 +56,7 @@ type ProcessHandler interface { ctx context.Context, timeout time.Duration, ) (int, error) - readFile(file string) ([]byte, error) + ReadFile(file string) ([]byte, error) Kill(pid int) error } @@ -130,7 +130,7 @@ func (m *ManagerImpl) Reload(ctx context.Context, configVersion int) error { } childProcFile := fmt.Sprintf(childProcPathFmt, pid) - previousChildProcesses, err := m.processHandler.readFile(childProcFile) + previousChildProcesses, err := m.processHandler.ReadFile(childProcFile) if err != nil { return err } From 6963fd4ad53d0f30b971b7c016d8904400a406a6 Mon Sep 17 00:00:00 2001 From: Mile Date: Sun, 14 Jul 2024 17:51:17 +0200 Subject: [PATCH 19/24] adding general fix for manager --- internal/mode/static/nginx/runtime/manager.go | 4 +- .../mode/static/nginx/runtime/manager_test.go | 4 +- .../fake_nginx_config_verifier.go | 269 ++++++++++++++++++ internal/mode/static/nginx/runtime/verify.go | 2 +- 4 files changed, 274 insertions(+), 5 deletions(-) create mode 100644 internal/mode/static/nginx/runtime/runtimefakes/fake_nginx_config_verifier.go diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index 1a5e9a24a1..aa6201ee8b 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -194,12 +194,10 @@ func (m *ManagerImpl) GetUpstreams() (ngxclient.Upstreams, error) { } func NewProcessHandlerImpl(readFile ReadFileFunc, checkFile CheckFileFunc) *ProcessHandlerImpl { - ph := &ProcessHandlerImpl{ + return &ProcessHandlerImpl{ readFile: readFile, checkFile: checkFile, } - - return ph } func (p *ProcessHandlerImpl) FindMainProcess( diff --git a/internal/mode/static/nginx/runtime/manager_test.go b/internal/mode/static/nginx/runtime/manager_test.go index b7acfb52b1..b5b28a5080 100644 --- a/internal/mode/static/nginx/runtime/manager_test.go +++ b/internal/mode/static/nginx/runtime/manager_test.go @@ -237,7 +237,9 @@ func TestFindMainProcess(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - p := runtime.NewProcessHandlerImpl{} + p := runtime.NewProcessHandlerImpl( + test.readFile, + test.checkFile) result, err := p.FindMainProcess(test.ctx, 2*time.Millisecond) if test.expectError { diff --git a/internal/mode/static/nginx/runtime/runtimefakes/fake_nginx_config_verifier.go b/internal/mode/static/nginx/runtime/runtimefakes/fake_nginx_config_verifier.go new file mode 100644 index 0000000000..23c5c67348 --- /dev/null +++ b/internal/mode/static/nginx/runtime/runtimefakes/fake_nginx_config_verifier.go @@ -0,0 +1,269 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package runtimefakes + +import ( + "context" + "sync" + + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime" +) + +type FakeNginxConfigVerifier struct { + EnsureConfigVersionStub func(context.Context, int) error + ensureConfigVersionMutex sync.RWMutex + ensureConfigVersionArgsForCall []struct { + arg1 context.Context + arg2 int + } + ensureConfigVersionReturns struct { + result1 error + } + ensureConfigVersionReturnsOnCall map[int]struct { + result1 error + } + GetConfigVersionStub func() (int, error) + getConfigVersionMutex sync.RWMutex + getConfigVersionArgsForCall []struct { + } + getConfigVersionReturns struct { + result1 int + result2 error + } + getConfigVersionReturnsOnCall map[int]struct { + result1 int + result2 error + } + WaitForCorrectVersionStub func(context.Context, int, string, []byte, runtime.ReadFileFunc) error + waitForCorrectVersionMutex sync.RWMutex + waitForCorrectVersionArgsForCall []struct { + arg1 context.Context + arg2 int + arg3 string + arg4 []byte + arg5 runtime.ReadFileFunc + } + waitForCorrectVersionReturns struct { + result1 error + } + waitForCorrectVersionReturnsOnCall map[int]struct { + result1 error + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *FakeNginxConfigVerifier) EnsureConfigVersion(arg1 context.Context, arg2 int) error { + fake.ensureConfigVersionMutex.Lock() + ret, specificReturn := fake.ensureConfigVersionReturnsOnCall[len(fake.ensureConfigVersionArgsForCall)] + fake.ensureConfigVersionArgsForCall = append(fake.ensureConfigVersionArgsForCall, struct { + arg1 context.Context + arg2 int + }{arg1, arg2}) + stub := fake.EnsureConfigVersionStub + fakeReturns := fake.ensureConfigVersionReturns + fake.recordInvocation("EnsureConfigVersion", []interface{}{arg1, arg2}) + fake.ensureConfigVersionMutex.Unlock() + if stub != nil { + return stub(arg1, arg2) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeNginxConfigVerifier) EnsureConfigVersionCallCount() int { + fake.ensureConfigVersionMutex.RLock() + defer fake.ensureConfigVersionMutex.RUnlock() + return len(fake.ensureConfigVersionArgsForCall) +} + +func (fake *FakeNginxConfigVerifier) EnsureConfigVersionCalls(stub func(context.Context, int) error) { + fake.ensureConfigVersionMutex.Lock() + defer fake.ensureConfigVersionMutex.Unlock() + fake.EnsureConfigVersionStub = stub +} + +func (fake *FakeNginxConfigVerifier) EnsureConfigVersionArgsForCall(i int) (context.Context, int) { + fake.ensureConfigVersionMutex.RLock() + defer fake.ensureConfigVersionMutex.RUnlock() + argsForCall := fake.ensureConfigVersionArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *FakeNginxConfigVerifier) EnsureConfigVersionReturns(result1 error) { + fake.ensureConfigVersionMutex.Lock() + defer fake.ensureConfigVersionMutex.Unlock() + fake.EnsureConfigVersionStub = nil + fake.ensureConfigVersionReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeNginxConfigVerifier) EnsureConfigVersionReturnsOnCall(i int, result1 error) { + fake.ensureConfigVersionMutex.Lock() + defer fake.ensureConfigVersionMutex.Unlock() + fake.EnsureConfigVersionStub = nil + if fake.ensureConfigVersionReturnsOnCall == nil { + fake.ensureConfigVersionReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.ensureConfigVersionReturnsOnCall[i] = struct { + result1 error + }{result1} +} + +func (fake *FakeNginxConfigVerifier) GetConfigVersion() (int, error) { + fake.getConfigVersionMutex.Lock() + ret, specificReturn := fake.getConfigVersionReturnsOnCall[len(fake.getConfigVersionArgsForCall)] + fake.getConfigVersionArgsForCall = append(fake.getConfigVersionArgsForCall, struct { + }{}) + stub := fake.GetConfigVersionStub + fakeReturns := fake.getConfigVersionReturns + fake.recordInvocation("GetConfigVersion", []interface{}{}) + fake.getConfigVersionMutex.Unlock() + if stub != nil { + return stub() + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeNginxConfigVerifier) GetConfigVersionCallCount() int { + fake.getConfigVersionMutex.RLock() + defer fake.getConfigVersionMutex.RUnlock() + return len(fake.getConfigVersionArgsForCall) +} + +func (fake *FakeNginxConfigVerifier) GetConfigVersionCalls(stub func() (int, error)) { + fake.getConfigVersionMutex.Lock() + defer fake.getConfigVersionMutex.Unlock() + fake.GetConfigVersionStub = stub +} + +func (fake *FakeNginxConfigVerifier) GetConfigVersionReturns(result1 int, result2 error) { + fake.getConfigVersionMutex.Lock() + defer fake.getConfigVersionMutex.Unlock() + fake.GetConfigVersionStub = nil + fake.getConfigVersionReturns = struct { + result1 int + result2 error + }{result1, result2} +} + +func (fake *FakeNginxConfigVerifier) GetConfigVersionReturnsOnCall(i int, result1 int, result2 error) { + fake.getConfigVersionMutex.Lock() + defer fake.getConfigVersionMutex.Unlock() + fake.GetConfigVersionStub = nil + if fake.getConfigVersionReturnsOnCall == nil { + fake.getConfigVersionReturnsOnCall = make(map[int]struct { + result1 int + result2 error + }) + } + fake.getConfigVersionReturnsOnCall[i] = struct { + result1 int + result2 error + }{result1, result2} +} + +func (fake *FakeNginxConfigVerifier) WaitForCorrectVersion(arg1 context.Context, arg2 int, arg3 string, arg4 []byte, arg5 runtime.ReadFileFunc) error { + var arg4Copy []byte + if arg4 != nil { + arg4Copy = make([]byte, len(arg4)) + copy(arg4Copy, arg4) + } + fake.waitForCorrectVersionMutex.Lock() + ret, specificReturn := fake.waitForCorrectVersionReturnsOnCall[len(fake.waitForCorrectVersionArgsForCall)] + fake.waitForCorrectVersionArgsForCall = append(fake.waitForCorrectVersionArgsForCall, struct { + arg1 context.Context + arg2 int + arg3 string + arg4 []byte + arg5 runtime.ReadFileFunc + }{arg1, arg2, arg3, arg4Copy, arg5}) + stub := fake.WaitForCorrectVersionStub + fakeReturns := fake.waitForCorrectVersionReturns + fake.recordInvocation("WaitForCorrectVersion", []interface{}{arg1, arg2, arg3, arg4Copy, arg5}) + fake.waitForCorrectVersionMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3, arg4, arg5) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeNginxConfigVerifier) WaitForCorrectVersionCallCount() int { + fake.waitForCorrectVersionMutex.RLock() + defer fake.waitForCorrectVersionMutex.RUnlock() + return len(fake.waitForCorrectVersionArgsForCall) +} + +func (fake *FakeNginxConfigVerifier) WaitForCorrectVersionCalls(stub func(context.Context, int, string, []byte, runtime.ReadFileFunc) error) { + fake.waitForCorrectVersionMutex.Lock() + defer fake.waitForCorrectVersionMutex.Unlock() + fake.WaitForCorrectVersionStub = stub +} + +func (fake *FakeNginxConfigVerifier) WaitForCorrectVersionArgsForCall(i int) (context.Context, int, string, []byte, runtime.ReadFileFunc) { + fake.waitForCorrectVersionMutex.RLock() + defer fake.waitForCorrectVersionMutex.RUnlock() + argsForCall := fake.waitForCorrectVersionArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4, argsForCall.arg5 +} + +func (fake *FakeNginxConfigVerifier) WaitForCorrectVersionReturns(result1 error) { + fake.waitForCorrectVersionMutex.Lock() + defer fake.waitForCorrectVersionMutex.Unlock() + fake.WaitForCorrectVersionStub = nil + fake.waitForCorrectVersionReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeNginxConfigVerifier) WaitForCorrectVersionReturnsOnCall(i int, result1 error) { + fake.waitForCorrectVersionMutex.Lock() + defer fake.waitForCorrectVersionMutex.Unlock() + fake.WaitForCorrectVersionStub = nil + if fake.waitForCorrectVersionReturnsOnCall == nil { + fake.waitForCorrectVersionReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.waitForCorrectVersionReturnsOnCall[i] = struct { + result1 error + }{result1} +} + +func (fake *FakeNginxConfigVerifier) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.ensureConfigVersionMutex.RLock() + defer fake.ensureConfigVersionMutex.RUnlock() + fake.getConfigVersionMutex.RLock() + defer fake.getConfigVersionMutex.RUnlock() + fake.waitForCorrectVersionMutex.RLock() + defer fake.waitForCorrectVersionMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *FakeNginxConfigVerifier) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} diff --git a/internal/mode/static/nginx/runtime/verify.go b/internal/mode/static/nginx/runtime/verify.go index 0b69df1152..9dc64d4e34 100644 --- a/internal/mode/static/nginx/runtime/verify.go +++ b/internal/mode/static/nginx/runtime/verify.go @@ -19,7 +19,7 @@ const configVersionURI = "/var/run/nginx/nginx-config-version.sock" var noNewWorkersErrFmt = "reload unsuccessful: no new NGINX worker processes started for config version %d." + " Please check the NGINX container logs for possible configuration issues: %w" -//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . verifyClient +//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . nginxConfigVerifier type nginxConfigVerifier interface { GetConfigVersion() (int, error) From 547e903ca11c799d8be2d236c21a687037e4dddb Mon Sep 17 00:00:00 2001 From: Mile Druzijanic Date: Sat, 10 Aug 2024 22:54:49 +0200 Subject: [PATCH 20/24] adding small fix update --- internal/mode/static/manager.go | 3 +- internal/mode/static/nginx/runtime/manager.go | 4 +- .../mode/static/nginx/runtime/manager_test.go | 77 +++++++++++++++++++ 3 files changed, 81 insertions(+), 3 deletions(-) diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index c889ddbd3e..9ea205ad49 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "os" "time" "github.com/go-logr/logr" @@ -144,7 +145,7 @@ func StartManager(cfg config.Config) error { return fmt.Errorf("cannot clear NGINX configuration folders: %w", err) } - processHandler := &ngxruntime.NewProcessHandlerImpl{} + processHandler := ngxruntime.NewProcessHandlerImpl(os.ReadFile, os.Stat) // Ensure NGINX is running before registering metrics & starting the manager. if _, err := processHandler.FindMainProcess(ctx, ngxruntime.PidFileTimeout); err != nil { diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index aa6201ee8b..529e2006f2 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -21,9 +21,9 @@ import ( const ( // PidFile specifies the location of the PID file for the Nginx process PidFile = "/var/run/nginx/nginx.pid" - // pidFileTimeout defines the timeout duration for accessing the PID file + // PidFileTimeout defines the timeout duration for accessing the PID file PidFileTimeout = 10000 * time.Millisecond - /// NginxReloadTimeout sets the timeout duration for reloading the Nginx configuration + // NginxReloadTimeout sets the timeout duration for reloading the Nginx configuration NginxReloadTimeout = 60000 * time.Millisecond ) diff --git a/internal/mode/static/nginx/runtime/manager_test.go b/internal/mode/static/nginx/runtime/manager_test.go index b5b28a5080..8af97ef816 100644 --- a/internal/mode/static/nginx/runtime/manager_test.go +++ b/internal/mode/static/nginx/runtime/manager_test.go @@ -3,6 +3,7 @@ package runtime_test import ( "context" "errors" + "fmt" "io/fs" "testing" "time" @@ -63,6 +64,52 @@ var _ = Describe("NGINX Runtime Manager", func() { Expect(metrics.IncReloadErrorsCallCount()).To(Equal(0)) }) + It("Fails to find the main process", func() { + process.FindMainProcessReturns(0, fmt.Errorf("failed to find process")) + + err := manager.Reload(context.Background(), 1) + + Expect(err).To(MatchError("failed to find NGINX main process: failed to find process")) + Expect(process.ReadFileCallCount()).To(Equal(0)) + Expect(process.KillCallCount()).To(Equal(0)) + Expect(verifyClient.WaitForCorrectVersionCallCount()).To(Equal(0)) + }) + + It("Fails to read file", func() { + process.FindMainProcessReturns(1234, nil) + process.ReadFileReturns(nil, fmt.Errorf("failed to read file")) + + err := manager.Reload(context.Background(), 1) + + Expect(err).To(MatchError("failed to read file")) + Expect(process.KillCallCount()).To(Equal(0)) + Expect(verifyClient.WaitForCorrectVersionCallCount()).To(Equal(0)) + }) + + It("Fails to send kill signal", func() { + process.FindMainProcessReturns(1234, nil) + process.ReadFileReturns([]byte("child1\nchild2"), nil) + process.KillReturns(fmt.Errorf("failed to send kill signal")) + + err := manager.Reload(context.Background(), 1) + + Expect(err).To(MatchError("failed to send the HUP signal to NGINX main: failed to send kill signal")) + Expect(metrics.IncReloadErrorsCallCount()).To(Equal(1)) + Expect(verifyClient.WaitForCorrectVersionCallCount()).To(Equal(0)) + }) + + It("times out waiting for correct version", func() { + process.FindMainProcessReturns(1234, nil) + process.ReadFileReturns([]byte("child1\nchild2"), nil) + process.KillReturns(nil) + verifyClient.WaitForCorrectVersionReturns(fmt.Errorf("timeout waiting for correct version")) + + err := manager.Reload(context.Background(), 1) + + Expect(err).To(MatchError("timeout waiting for correct version")) + Expect(metrics.IncReloadErrorsCallCount()).To(Equal(1)) + }) + When("MetricsCollector is nil", func() { It("panics", func() { metrics = nil @@ -110,6 +157,36 @@ var _ = Describe("NGINX Runtime Manager", func() { Expect(upstreams).To(BeEmpty()) }) + It("successfully returns server upstreams", func() { + upstreams := ngxclient.Upstreams{ + "upstream1": { + Zone: "zone1", + Peers: []ngxclient.Peer{ + {ID: 1, Name: "peer1-name"}, + }, + Queue: ngxclient.Queue{Size: 10}, + Keepalives: 5, + Zombies: 2, + }, + "upstream2": { + Zone: "zone2", + Peers: []ngxclient.Peer{ + {ID: 2, Name: "peer2-name"}, + }, + Queue: ngxclient.Queue{Size: 20}, + Keepalives: 3, + Zombies: 1, + }, + } + + ngxPlusClient.GetUpstreamsReturns(&upstreams, nil) + + upstreams, err := manager.GetUpstreams() + + Expect(err).NotTo(HaveOccurred()) + Expect(upstreams).To(Equal(upstreams)) + }) + It("returns an error when GetUpstreams fails", func() { ngxPlusClient.GetUpstreamsReturns(nil, errors.New("failed to get upstreams")) From 27fed56f18b5089fb378dd5e856f895fc2235415 Mon Sep 17 00:00:00 2001 From: miledxz Date: Tue, 20 Aug 2024 18:42:32 +0200 Subject: [PATCH 21/24] adding manager fix update --- internal/mode/static/nginx/runtime/manager.go | 32 +++++++++---------- .../mode/static/nginx/runtime/manager_test.go | 6 ++-- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index 529e2006f2..e190905a53 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -49,22 +49,6 @@ type nginxPlusClient interface { GetUpstreams() (*ngxclient.Upstreams, error) } -//counterfeiter:generate . ProcessHandler - -type ProcessHandler interface { - FindMainProcess( - ctx context.Context, - timeout time.Duration, - ) (int, error) - ReadFile(file string) ([]byte, error) - Kill(pid int) error -} - -type ProcessHandlerImpl struct { - readFile ReadFileFunc - checkFile CheckFileFunc -} - //counterfeiter:generate . Manager // Manager manages the runtime of NGINX. @@ -193,6 +177,22 @@ func (m *ManagerImpl) GetUpstreams() (ngxclient.Upstreams, error) { return *upstreams, nil } +//counterfeiter:generate . ProcessHandler + +type ProcessHandler interface { + FindMainProcess( + ctx context.Context, + timeout time.Duration, + ) (int, error) + ReadFile(file string) ([]byte, error) + Kill(pid int) error +} + +type ProcessHandlerImpl struct { + readFile ReadFileFunc + checkFile CheckFileFunc +} + func NewProcessHandlerImpl(readFile ReadFileFunc, checkFile CheckFileFunc) *ProcessHandlerImpl { return &ProcessHandlerImpl{ readFile: readFile, diff --git a/internal/mode/static/nginx/runtime/manager_test.go b/internal/mode/static/nginx/runtime/manager_test.go index 8af97ef816..bc8e14530c 100644 --- a/internal/mode/static/nginx/runtime/manager_test.go +++ b/internal/mode/static/nginx/runtime/manager_test.go @@ -158,7 +158,7 @@ var _ = Describe("NGINX Runtime Manager", func() { }) It("successfully returns server upstreams", func() { - upstreams := ngxclient.Upstreams{ + expUpstreams := ngxclient.Upstreams{ "upstream1": { Zone: "zone1", Peers: []ngxclient.Peer{ @@ -179,12 +179,12 @@ var _ = Describe("NGINX Runtime Manager", func() { }, } - ngxPlusClient.GetUpstreamsReturns(&upstreams, nil) + ngxPlusClient.GetUpstreamsReturns(&expUpstreams, nil) upstreams, err := manager.GetUpstreams() Expect(err).NotTo(HaveOccurred()) - Expect(upstreams).To(Equal(upstreams)) + Expect(upstreams).To(Equal(expUpstreams)) }) It("returns an error when GetUpstreams fails", func() { From 8b94a0567a3fbe153cd00ac0c27a5146626e2365 Mon Sep 17 00:00:00 2001 From: miledxz Date: Tue, 20 Aug 2024 18:44:48 +0200 Subject: [PATCH 22/24] quick upstream test update --- internal/mode/static/nginx/runtime/manager.go | 8 ++++---- internal/mode/static/nginx/runtime/manager_test.go | 12 +++++------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index e190905a53..0d111e0eb3 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -19,11 +19,11 @@ import ( //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate const ( - // PidFile specifies the location of the PID file for the Nginx process + // PidFile specifies the location of the PID file for the Nginx process. PidFile = "/var/run/nginx/nginx.pid" - // PidFileTimeout defines the timeout duration for accessing the PID file + // PidFileTimeout defines the timeout duration for accessing the PID file. PidFileTimeout = 10000 * time.Millisecond - // NginxReloadTimeout sets the timeout duration for reloading the Nginx configuration + // NginxReloadTimeout sets the timeout duration for reloading the Nginx configuration. NginxReloadTimeout = 60000 * time.Millisecond ) @@ -239,7 +239,7 @@ func (p *ProcessHandlerImpl) FindMainProcess( } func (p *ProcessHandlerImpl) ReadFile(file string) ([]byte, error) { - return p.ReadFile(file) + return p.readFile(file) } func (p *ProcessHandlerImpl) Kill(pid int) error { diff --git a/internal/mode/static/nginx/runtime/manager_test.go b/internal/mode/static/nginx/runtime/manager_test.go index bc8e14530c..7b3c30af15 100644 --- a/internal/mode/static/nginx/runtime/manager_test.go +++ b/internal/mode/static/nginx/runtime/manager_test.go @@ -164,18 +164,16 @@ var _ = Describe("NGINX Runtime Manager", func() { Peers: []ngxclient.Peer{ {ID: 1, Name: "peer1-name"}, }, - Queue: ngxclient.Queue{Size: 10}, - Keepalives: 5, - Zombies: 2, + Queue: ngxclient.Queue{Size: 10}, + Zombies: 2, }, "upstream2": { Zone: "zone2", Peers: []ngxclient.Peer{ {ID: 2, Name: "peer2-name"}, }, - Queue: ngxclient.Queue{Size: 20}, - Keepalives: 3, - Zombies: 1, + Queue: ngxclient.Queue{Size: 20}, + Zombies: 1, }, } @@ -184,7 +182,7 @@ var _ = Describe("NGINX Runtime Manager", func() { upstreams, err := manager.GetUpstreams() Expect(err).NotTo(HaveOccurred()) - Expect(upstreams).To(Equal(expUpstreams)) + Expect(expUpstreams).To(Equal(upstreams)) }) It("returns an error when GetUpstreams fails", func() { From 7be17ee37e430947252a47e0ebdbf9e49491eb88 Mon Sep 17 00:00:00 2001 From: miledxz Date: Wed, 21 Aug 2024 21:29:51 +0200 Subject: [PATCH 23/24] experimental fix --- internal/mode/static/manager.go | 3 +-- internal/mode/static/metrics/collectors/nginx.go | 3 +-- internal/mode/static/nginx/runtime/manager.go | 6 +++--- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 72fbd651bb..0b277ca056 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -8,7 +8,6 @@ import ( "time" "github.com/go-logr/logr" - ngxclient "github.com/nginxinc/nginx-plus-go-client/client" tel "github.com/nginxinc/telemetry-exporter/pkg/telemetry" "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" @@ -159,7 +158,7 @@ func StartManager(cfg config.Config) error { handlerCollector handlerMetricsCollector = collectors.NewControllerNoopCollector() ) - var ngxPlusClient *ngxclient.NginxClient + var ngxPlusClient ngxruntime.NginxPlusClient var usageSecret *usage.Secret if cfg.Plus { diff --git a/internal/mode/static/metrics/collectors/nginx.go b/internal/mode/static/metrics/collectors/nginx.go index 28cbc635d3..09400a5230 100644 --- a/internal/mode/static/metrics/collectors/nginx.go +++ b/internal/mode/static/metrics/collectors/nginx.go @@ -2,7 +2,6 @@ package collectors import ( "github.com/go-kit/log" - "github.com/nginxinc/nginx-plus-go-client/client" prometheusClient "github.com/nginxinc/nginx-prometheus-exporter/client" nginxCollector "github.com/nginxinc/nginx-prometheus-exporter/collector" "github.com/prometheus/client_golang/prometheus" @@ -26,7 +25,7 @@ func NewNginxMetricsCollector(constLabels map[string]string, logger log.Logger) // NewNginxPlusMetricsCollector creates an NginxCollector which fetches stats from NGINX Plus API over a unix socket. func NewNginxPlusMetricsCollector( - plusClient *client.NginxClient, + plusClient runtime.NginxPlusClient, constLabels map[string]string, logger log.Logger, ) (prometheus.Collector, error) { diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index 0d111e0eb3..a5c26a7765 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -36,7 +36,7 @@ var childProcPathFmt = "/proc/%[1]v/task/%[1]v/children" //counterfeiter:generate . nginxPlusClient -type nginxPlusClient interface { +type NginxPlusClient interface { UpdateHTTPServers( upstream string, servers []ngxclient.UpstreamServer, @@ -79,13 +79,13 @@ type ManagerImpl struct { processHandler ProcessHandler metricsCollector MetricsCollector verifyClient nginxConfigVerifier - ngxPlusClient nginxPlusClient + ngxPlusClient NginxPlusClient logger logr.Logger } // NewManagerImpl creates a new ManagerImpl. func NewManagerImpl( - ngxPlusClient nginxPlusClient, + ngxPlusClient NginxPlusClient, collector MetricsCollector, logger logr.Logger, processHandler ProcessHandler, From 1a564411edf8e4749659af961a6cf2e8760c7fa8 Mon Sep 17 00:00:00 2001 From: miledxz Date: Thu, 29 Aug 2024 19:51:53 +0200 Subject: [PATCH 24/24] adding lint fix update --- internal/mode/static/metrics/collectors/nginx.go | 9 ++++++++- internal/mode/static/nginx/runtime/manager.go | 2 +- .../nginx/runtime/runtimefakes/fake_nginx_plus_client.go | 3 +++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/internal/mode/static/metrics/collectors/nginx.go b/internal/mode/static/metrics/collectors/nginx.go index 09400a5230..f8ce60b26a 100644 --- a/internal/mode/static/metrics/collectors/nginx.go +++ b/internal/mode/static/metrics/collectors/nginx.go @@ -1,7 +1,10 @@ package collectors import ( + "fmt" + "github.com/go-kit/log" + "github.com/nginxinc/nginx-plus-go-client/client" prometheusClient "github.com/nginxinc/nginx-prometheus-exporter/client" nginxCollector "github.com/nginxinc/nginx-prometheus-exporter/collector" "github.com/prometheus/client_golang/prometheus" @@ -29,8 +32,12 @@ func NewNginxPlusMetricsCollector( constLabels map[string]string, logger log.Logger, ) (prometheus.Collector, error) { + nc, ok := plusClient.(*client.NginxClient) + if !ok { + panic(fmt.Sprintf("expected *client.NginxClient, got %T", plusClient)) + } collector := nginxCollector.NewNginxPlusCollector( - plusClient, + nc, metrics.Namespace, nginxCollector.VariableLabelNames{}, constLabels, diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index a5c26a7765..fd7bf9e3fc 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -34,7 +34,7 @@ type ( var childProcPathFmt = "/proc/%[1]v/task/%[1]v/children" -//counterfeiter:generate . nginxPlusClient +//counterfeiter:generate . NginxPlusClient type NginxPlusClient interface { UpdateHTTPServers( diff --git a/internal/mode/static/nginx/runtime/runtimefakes/fake_nginx_plus_client.go b/internal/mode/static/nginx/runtime/runtimefakes/fake_nginx_plus_client.go index c6291816d8..3ea431d29b 100644 --- a/internal/mode/static/nginx/runtime/runtimefakes/fake_nginx_plus_client.go +++ b/internal/mode/static/nginx/runtime/runtimefakes/fake_nginx_plus_client.go @@ -4,6 +4,7 @@ package runtimefakes import ( "sync" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime" "github.com/nginxinc/nginx-plus-go-client/client" ) @@ -199,3 +200,5 @@ func (fake *FakeNginxPlusClient) recordInvocation(key string, args []interface{} } fake.invocations[key] = append(fake.invocations[key], args) } + +var _ runtime.NginxPlusClient = new(FakeNginxPlusClient)