diff --git a/docs/architecture.md b/docs/architecture.md index 3eabd49f65..026135587e 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -106,7 +106,7 @@ API to update the handled resources' statuses and emit events. - Read: *NKG* reads the PID file `nginx.pid` from the `nginx-run` volume, located at `/var/run/nginx`. *NKG* extracts the PID of the nginx process from this file in order to send reload signals to *NGINX master*. 4. (File I/O) *NKG* writes logs to its *stdout* and *stderr*, which are collected by the container runtime. -5. (HTTP) *NKG* fetches the NGINX metrics via the unix:/var/lib/nginx/nginx-status.sock UNIX socket and converts it to +5. (HTTP) *NKG* fetches the NGINX metrics via the unix:/var/run/nginx/nginx-status.sock UNIX socket and converts it to *Prometheus* format used in #2. 6. (Signal) To reload NGINX, *NKG* sends the [reload signal][reload] to the **NGINX master**. 7. (File I/O) @@ -124,9 +124,12 @@ API to update the handled resources' statuses and emit events. 9. (File I/O) The *NGINX master* sends logs to its *stdout* and *stderr*, which are collected by the container runtime. 10. (File I/O) An *NGINX worker* writes logs to its *stdout* and *stderr*, which are collected by the container runtime. 11. (Signal) The *NGINX master* controls the [lifecycle of *NGINX workers*][lifecycle] it creates workers with the new -configuration and shutdowns workers with the old configuration. -12. (HTTP,HTTPS) A *client* sends traffic to and receives traffic from any of the *NGINX workers* on ports 80 and 443. -13. (HTTP,HTTPS) An *NGINX worker* sends traffic to and receives traffic from the *backends*. + configuration and shutdowns workers with the old configuration. +12. (HTTP) To consider a configuration reload a success, *NKG* ensures that at least one NGINX worker has the new + configuration. To do that, *NKG* checks a particular endpoint via the unix:/var/run/nginx/nginx-config-version.sock + UNIX socket. +13. (HTTP,HTTPS) A *client* sends traffic to and receives traffic from any of the *NGINX workers* on ports 80 and 443. +14. (HTTP,HTTPS) An *NGINX worker* sends traffic to and receives traffic from the *backends*. [controller]: https://kubernetes.io/docs/concepts/architecture/controller/ diff --git a/docs/images/nkg-pod.png b/docs/images/nkg-pod.png index 7af069f201..48e2e60a35 100644 Binary files a/docs/images/nkg-pod.png and b/docs/images/nkg-pod.png differ diff --git a/internal/mode/static/handler.go b/internal/mode/static/handler.go index 17427964db..144cc0adc2 100644 --- a/internal/mode/static/handler.go +++ b/internal/mode/static/handler.go @@ -45,6 +45,8 @@ type eventHandlerConfig struct { logger logr.Logger // controlConfigNSName is the NamespacedName of the NginxGateway config for this controller. controlConfigNSName types.NamespacedName + // version is the current version number of the nginx config. + version int } // eventHandlerImpl implements EventHandler. @@ -90,7 +92,11 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, batch events.Ev } var nginxReloadRes nginxReloadResult - if err := h.updateNginx(ctx, dataplane.BuildConfiguration(ctx, graph, h.cfg.serviceResolver)); err != nil { + h.cfg.version++ + if err := h.updateNginx( + ctx, + dataplane.BuildConfiguration(ctx, graph, h.cfg.serviceResolver, h.cfg.version), + ); err != nil { h.cfg.logger.Error(err, "Failed to update NGINX configuration") nginxReloadRes.error = err } else { @@ -107,7 +113,7 @@ func (h *eventHandlerImpl) updateNginx(ctx context.Context, conf dataplane.Confi return fmt.Errorf("failed to replace NGINX configuration files: %w", err) } - if err := h.cfg.nginxRuntimeMgr.Reload(ctx); err != nil { + if err := h.cfg.nginxRuntimeMgr.Reload(ctx, conf.Version); err != nil { return fmt.Errorf("failed to reload NGINX: %w", err) } diff --git a/internal/mode/static/handler_test.go b/internal/mode/static/handler_test.go index d3299e8ebf..a6b0fa4a42 100644 --- a/internal/mode/static/handler_test.go +++ b/internal/mode/static/handler_test.go @@ -111,7 +111,7 @@ var _ = Describe("eventHandler", func() { handler.HandleEventBatch(context.Background(), batch) checkUpsertEventExpectations(e) - expectReconfig(dataplane.Configuration{}, fakeCfgFiles) + expectReconfig(dataplane.Configuration{Version: 1}, fakeCfgFiles) }) It("should process Delete", func() { @@ -124,7 +124,7 @@ var _ = Describe("eventHandler", func() { handler.HandleEventBatch(context.Background(), batch) checkDeleteEventExpectations(e) - expectReconfig(dataplane.Configuration{}, fakeCfgFiles) + expectReconfig(dataplane.Configuration{Version: 1}, fakeCfgFiles) }) }) diff --git a/internal/mode/static/nginx/config/generator.go b/internal/mode/static/nginx/config/generator.go index 1956a03664..851d8147f8 100644 --- a/internal/mode/static/nginx/config/generator.go +++ b/internal/mode/static/nginx/config/generator.go @@ -20,6 +20,9 @@ const ( // httpConfigFile is the path to the configuration file with HTTP configuration. httpConfigFile = httpFolder + "/http.conf" + + // configVersionFile is the path to the config version configuration file. + configVersionFile = httpFolder + "/config-version.conf" ) // ConfigFolders is a list of folders where NGINX configuration files are stored. @@ -63,6 +66,8 @@ func (g GeneratorImpl) Generate(conf dataplane.Configuration) []file.File { files = append(files, generateHTTPConfig(conf)) + files = append(files, generateConfigVersion(conf.Version)) + return files } @@ -104,3 +109,14 @@ func getExecuteFuncs() []executeFunc { executeMaps, } } + +// generateConfigVersion writes the config version file. +func generateConfigVersion(configVersion int) file.File { + c := executeVersion(configVersion) + + return file.File{ + Content: c, + Path: configVersionFile, + Type: file.TypeRegular, + } +} diff --git a/internal/mode/static/nginx/config/generator_test.go b/internal/mode/static/nginx/config/generator_test.go index 3d64174dad..983185d2c0 100644 --- a/internal/mode/static/nginx/config/generator_test.go +++ b/internal/mode/static/nginx/config/generator_test.go @@ -1,6 +1,7 @@ package config_test import ( + "fmt" "testing" . "github.com/onsi/gomega" @@ -65,7 +66,7 @@ func TestGenerate(t *testing.T) { files := generator.Generate(conf) - g.Expect(files).To(HaveLen(2)) + g.Expect(files).To(HaveLen(3)) g.Expect(files[0]).To(Equal(file.File{ Type: file.TypeSecret, @@ -82,4 +83,9 @@ func TestGenerate(t *testing.T) { g.Expect(httpCfg).To(ContainSubstring("listen 443")) g.Expect(httpCfg).To(ContainSubstring("upstream")) g.Expect(httpCfg).To(ContainSubstring("split_clients")) + + g.Expect(files[2].Type).To(Equal(file.TypeRegular)) + g.Expect(files[2].Path).To(Equal("/etc/nginx/conf.d/config-version.conf")) + configVersion := string(files[2].Content) + g.Expect(configVersion).To(ContainSubstring(fmt.Sprintf("return 200 %d", conf.Version))) } diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index bf0be01065..84d10ebe60 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -79,9 +79,9 @@ func TestExecuteServers(t *testing.T) { func TestExecuteForDefaultServers(t *testing.T) { testcases := []struct { msg string - conf dataplane.Configuration httpPorts []int sslPorts []int + conf dataplane.Configuration }{ { conf: dataplane.Configuration{}, diff --git a/internal/mode/static/nginx/config/version.go b/internal/mode/static/nginx/config/version.go new file mode 100644 index 0000000000..494a3f7d31 --- /dev/null +++ b/internal/mode/static/nginx/config/version.go @@ -0,0 +1,11 @@ +package config + +import ( + gotemplate "text/template" +) + +var versionTemplate = gotemplate.Must(gotemplate.New("version").Parse(versionTemplateText)) + +func executeVersion(version int) []byte { + return execute(versionTemplate, version) +} diff --git a/internal/mode/static/nginx/config/version_template.go b/internal/mode/static/nginx/config/version_template.go new file mode 100644 index 0000000000..3f0d3ea544 --- /dev/null +++ b/internal/mode/static/nginx/config/version_template.go @@ -0,0 +1,12 @@ +package config + +var versionTemplateText = ` +server { + listen unix:/var/run/nginx/nginx-config-version.sock; + access_log off; + + location /version { + return 200 {{.}}; + } +} +` diff --git a/internal/mode/static/nginx/config/version_test.go b/internal/mode/static/nginx/config/version_test.go new file mode 100644 index 0000000000..4219844c39 --- /dev/null +++ b/internal/mode/static/nginx/config/version_test.go @@ -0,0 +1,20 @@ +package config + +import ( + "strings" + "testing" + + . "github.com/onsi/gomega" +) + +func TestExecuteVersion(t *testing.T) { + g := NewWithT(t) + expSubStrings := map[string]int{ + "return 200 42;": 1, + } + + maps := string(executeVersion(42)) + for expSubStr, expCount := range expSubStrings { + g.Expect(expCount).To(Equal(strings.Count(maps, expSubStr))) + } +} diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index 957d7f491e..eb7f48c208 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -1,6 +1,7 @@ package runtime import ( + "bytes" "context" "errors" "fmt" @@ -15,8 +16,10 @@ import ( ) const ( - pidFile = "/var/run/nginx/nginx.pid" - pidFileTimeout = 10 * time.Second + pidFile = "/var/run/nginx/nginx.pid" + pidFileTimeout = 10000 * time.Millisecond + childProcsTimeout = 1000 * time.Millisecond + nginxReloadTimeout = 60000 * time.Millisecond ) type ( @@ -24,49 +27,62 @@ type ( checkFileFunc func(string) (fs.FileInfo, error) ) +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" + childProcPathFmt = "/proc/%[1]v/task/%[1]v/children" +) + //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . Manager // Manager manages the runtime of NGINX. type Manager interface { // Reload reloads NGINX configuration. It is a blocking operation. - Reload(ctx context.Context) error + Reload(ctx context.Context, configVersion int) error } // ManagerImpl implements Manager. -type ManagerImpl struct{} +type ManagerImpl struct { + verifyClient *verifyClient +} // NewManagerImpl creates a new ManagerImpl. func NewManagerImpl() *ManagerImpl { - return &ManagerImpl{} + return &ManagerImpl{ + verifyClient: newVerifyClient(nginxReloadTimeout), + } } -func (m *ManagerImpl) Reload(ctx context.Context) error { +func (m *ManagerImpl) Reload(ctx context.Context, configVersion int) error { // 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) if err != nil { return fmt.Errorf("failed to find NGINX main process: %w", err) } + childProcFile := fmt.Sprintf(childProcPathFmt, pid) + previousChildProcesses, err := os.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 { return fmt.Errorf("failed to send the HUP signal to NGINX main: %w", err) } - // FIXME(pleshakov) - // (1) ensure the reload actually happens. - // https://github.com/nginxinc/nginx-kubernetes-gateway/issues/664 - - // for now, to prevent a subsequent reload starting before the in-flight reload finishes, we simply sleep. - // Fixing (1) will make the sleep unnecessary. - - select { - case <-ctx.Done(): - return nil - case <-time.After(1 * time.Second): + if err := ensureNewNginxWorkers( + ctx, + childProcFile, + previousChildProcesses, + os.ReadFile, + childProcsTimeout, + ); err != nil { + return fmt.Errorf(noNewWorkersErrFmt, configVersion, err) } - return nil + return m.verifyClient.waitForCorrectVersion(ctx, configVersion) } // EnsureNginxRunning ensures NGINX is running by locating the main process. @@ -116,3 +132,30 @@ func findMainProcess( return pid, nil } + +func ensureNewNginxWorkers( + ctx context.Context, + childProcFile string, + previousContents []byte, + readFile readFileFunc, + timeout time.Duration, +) error { + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + return wait.PollUntilContextCancel( + ctx, + 25*time.Millisecond, + true, /* poll immediately */ + func(ctx context.Context) (bool, error) { + content, err := readFile(childProcFile) + if err != nil { + return false, err + } + if !bytes.Equal(previousContents, content) { + return true, nil + } + return false, nil + }, + ) +} diff --git a/internal/mode/static/nginx/runtime/manager_test.go b/internal/mode/static/nginx/runtime/manager_test.go index c5ff89b041..1b31408580 100644 --- a/internal/mode/static/nginx/runtime/manager_test.go +++ b/internal/mode/static/nginx/runtime/manager_test.go @@ -6,6 +6,8 @@ import ( "io/fs" "testing" "time" + + . "github.com/onsi/gomega" ) func TestFindMainProcess(t *testing.T) { @@ -41,7 +43,7 @@ func TestFindMainProcess(t *testing.T) { ctx context.Context readFile readFileFunc checkFile checkFileFunc - msg string + name string expected int expectError bool }{ @@ -51,7 +53,7 @@ func TestFindMainProcess(t *testing.T) { checkFile: checkFileFuncGen(testFileInfo), expected: 1, expectError: false, - msg: "normal case", + name: "normal case", }, { ctx: ctx, @@ -59,7 +61,7 @@ func TestFindMainProcess(t *testing.T) { checkFile: checkFileFuncGen(testFileInfo), expected: 0, expectError: true, - msg: "empty file content", + name: "empty file content", }, { ctx: ctx, @@ -67,7 +69,7 @@ func TestFindMainProcess(t *testing.T) { checkFile: checkFileFuncGen(testFileInfo), expected: 0, expectError: true, - msg: "bad file content", + name: "bad file content", }, { ctx: ctx, @@ -75,7 +77,7 @@ func TestFindMainProcess(t *testing.T) { checkFile: checkFileFuncGen(testFileInfo), expected: 0, expectError: true, - msg: "cannot read file", + name: "cannot read file", }, { ctx: ctx, @@ -83,7 +85,7 @@ func TestFindMainProcess(t *testing.T) { checkFile: checkFileError, expected: 0, expectError: true, - msg: "cannot find pid file", + name: "cannot find pid file", }, { ctx: cancellingCtx, @@ -91,25 +93,100 @@ func TestFindMainProcess(t *testing.T) { checkFile: checkFileError, expected: 0, expectError: true, - msg: "context canceled", + name: "context canceled", }, } for _, test := range tests { - result, err := findMainProcess(test.ctx, test.checkFile, test.readFile, 2*time.Millisecond) + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) - if result != test.expected { - t.Errorf("findMainProcess() returned %d but expected %d for case %q", result, test.expected, test.msg) - } + result, err := findMainProcess(test.ctx, test.checkFile, test.readFile, 2*time.Millisecond) - if test.expectError { - if err == nil { - t.Errorf("findMainProcess() didn't return error for case %q", test.msg) + if test.expectError { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result).To(Equal(test.expected)) } - } else { - if err != nil { - t.Errorf("findMainProcess() returned unexpected error %v for case %q", err, test.msg) + }) + } +} + +func TestEnsureNewNginxWorkers(t *testing.T) { + previousContents := []byte("1 2 3") + newContents := []byte("4 5 6") + + readFileError := func(string) ([]byte, error) { + return nil, errors.New("error") + } + + readFilePrevious := func(string) ([]byte, error) { + return previousContents, nil + } + + readFileNew := func(string) ([]byte, error) { + return newContents, nil + } + + ctx := context.Background() + cancellingCtx, cancel := context.WithCancel(ctx) + time.AfterFunc(1*time.Millisecond, cancel) + + tests := []struct { + ctx context.Context + readFile readFileFunc + name string + previousContents []byte + expectError bool + }{ + { + ctx: ctx, + readFile: readFileNew, + previousContents: previousContents, + expectError: false, + name: "normal case", + }, + { + ctx: ctx, + readFile: readFileError, + previousContents: previousContents, + expectError: true, + name: "cannot read file", + }, + { + ctx: ctx, + readFile: readFilePrevious, + previousContents: previousContents, + expectError: true, + name: "no new workers", + }, + { + ctx: cancellingCtx, + readFile: readFilePrevious, + previousContents: previousContents, + expectError: true, + name: "context canceled", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + err := ensureNewNginxWorkers( + test.ctx, + "/childfile", + test.previousContents, + test.readFile, + 2*time.Millisecond, + ) + + if test.expectError { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) } - } + }) } } diff --git a/internal/mode/static/nginx/runtime/runtimefakes/fake_manager.go b/internal/mode/static/nginx/runtime/runtimefakes/fake_manager.go index 93b65360a3..1fd6724c7d 100644 --- a/internal/mode/static/nginx/runtime/runtimefakes/fake_manager.go +++ b/internal/mode/static/nginx/runtime/runtimefakes/fake_manager.go @@ -9,10 +9,11 @@ import ( ) type FakeManager struct { - ReloadStub func(context.Context) error + ReloadStub func(context.Context, int) error reloadMutex sync.RWMutex reloadArgsForCall []struct { arg1 context.Context + arg2 int } reloadReturns struct { result1 error @@ -24,18 +25,19 @@ type FakeManager struct { invocationsMutex sync.RWMutex } -func (fake *FakeManager) Reload(arg1 context.Context) error { +func (fake *FakeManager) Reload(arg1 context.Context, arg2 int) error { fake.reloadMutex.Lock() ret, specificReturn := fake.reloadReturnsOnCall[len(fake.reloadArgsForCall)] fake.reloadArgsForCall = append(fake.reloadArgsForCall, struct { arg1 context.Context - }{arg1}) + arg2 int + }{arg1, arg2}) stub := fake.ReloadStub fakeReturns := fake.reloadReturns - fake.recordInvocation("Reload", []interface{}{arg1}) + fake.recordInvocation("Reload", []interface{}{arg1, arg2}) fake.reloadMutex.Unlock() if stub != nil { - return stub(arg1) + return stub(arg1, arg2) } if specificReturn { return ret.result1 @@ -49,17 +51,17 @@ func (fake *FakeManager) ReloadCallCount() int { return len(fake.reloadArgsForCall) } -func (fake *FakeManager) ReloadCalls(stub func(context.Context) error) { +func (fake *FakeManager) ReloadCalls(stub func(context.Context, int) error) { fake.reloadMutex.Lock() defer fake.reloadMutex.Unlock() fake.ReloadStub = stub } -func (fake *FakeManager) ReloadArgsForCall(i int) context.Context { +func (fake *FakeManager) ReloadArgsForCall(i int) (context.Context, int) { fake.reloadMutex.RLock() defer fake.reloadMutex.RUnlock() argsForCall := fake.reloadArgsForCall[i] - return argsForCall.arg1 + return argsForCall.arg1, argsForCall.arg2 } func (fake *FakeManager) ReloadReturns(result1 error) { diff --git a/internal/mode/static/nginx/runtime/verify.go b/internal/mode/static/nginx/runtime/verify.go new file mode 100644 index 0000000000..1b5dff93d1 --- /dev/null +++ b/internal/mode/static/nginx/runtime/verify.go @@ -0,0 +1,93 @@ +package runtime + +import ( + "context" + "errors" + "fmt" + "io" + "net" + "net/http" + "strconv" + "time" + + "k8s.io/apimachinery/pkg/util/wait" +) + +const configVersionURI = "/var/run/nginx/nginx-config-version.sock" + +// 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) *verifyClient { + return &verifyClient{ + client: &http.Client{ + Transport: &http.Transport{ + DialContext: func(_ context.Context, _, _ string) (net.Conn, error) { + return net.Dial("unix", configVersionURI) + }, + }, + }, + timeout: timeout, + } +} + +// 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) { + ctx, cancel := context.WithTimeout(context.Background(), c.timeout) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, "GET", "http://config-version/version", nil) + if err != nil { + return 0, fmt.Errorf("error creating request: %w", err) + } + + resp, err := c.client.Do(req) + if err != nil { + return 0, fmt.Errorf("error getting client: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return 0, fmt.Errorf("non-200 response: %v", resp.StatusCode) + } + + body, err := io.ReadAll(resp.Body) + if err != nil { + return 0, fmt.Errorf("failed to read the response body: %w", err) + } + v, err := strconv.Atoi(string(body)) + if err != nil { + return 0, fmt.Errorf("error converting string to int: %w", err) + } + return v, nil +} + +// waitForCorrectVersion 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(ctx context.Context, expectedVersion int) error { + ctx, cancel := context.WithTimeout(ctx, c.timeout) + defer cancel() + + if err := wait.PollUntilContextCancel( + ctx, + 25*time.Millisecond, + true, /* poll immediately */ + func(ctx context.Context) (bool, error) { + version, err := c.getConfigVersion() + return version == expectedVersion, err + }); err != nil { + if errors.Is(err, context.DeadlineExceeded) { + err = fmt.Errorf( + "config version check didn't return expected version %d within the deadline", + expectedVersion, + ) + } + return fmt.Errorf("could not get expected config version %d: %w", expectedVersion, err) + } + return nil +} diff --git a/internal/mode/static/nginx/runtime/verify_test.go b/internal/mode/static/nginx/runtime/verify_test.go new file mode 100644 index 0000000000..9cd35f0bca --- /dev/null +++ b/internal/mode/static/nginx/runtime/verify_test.go @@ -0,0 +1,80 @@ +package runtime + +import ( + "bytes" + "context" + "io" + "net/http" + "testing" + "time" + + . "github.com/onsi/gomega" +) + +type transport struct{} + +func (c transport) RoundTrip(_ *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewBufferString("42")), + Header: make(http.Header), + }, nil +} + +func getTestHTTPClient() *http.Client { + ts := transport{} + return &http.Client{ + Transport: ts, + } +} + +func TestVerifyClient(t *testing.T) { + c := verifyClient{ + client: getTestHTTPClient(), + timeout: 25 * time.Millisecond, + } + + ctx := context.Background() + cancellingCtx, cancel := context.WithCancel(ctx) + time.AfterFunc(1*time.Millisecond, cancel) + + tests := []struct { + ctx context.Context + name string + expectedVersion int + expectError bool + }{ + { + ctx: ctx, + expectedVersion: 42, + expectError: false, + name: "normal case", + }, + { + ctx: ctx, + expectedVersion: 43, + expectError: true, + name: "wrong version", + }, + { + ctx: cancellingCtx, + expectedVersion: 0, + expectError: true, + name: "context canceled", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + err := c.waitForCorrectVersion(test.ctx, test.expectedVersion) + + if test.expectError { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + }) + } +} diff --git a/internal/mode/static/state/conditions/conditions.go b/internal/mode/static/state/conditions/conditions.go index 3d27571afa..9ad84eb35b 100644 --- a/internal/mode/static/state/conditions/conditions.go +++ b/internal/mode/static/state/conditions/conditions.go @@ -18,7 +18,7 @@ const ( // ListenerMessageFailedNginxReload is a message used with ListenerConditionProgrammed (false) // when nginx fails to reload. ListenerMessageFailedNginxReload = "The Listener is not programmed due to a failure to " + - "reload nginx with the configuration" + "reload nginx with the configuration. Please see the nginx container logs for any possible configuration issues." // RouteReasonBackendRefUnsupportedValue is used with the "ResolvedRefs" condition when one of the // Route rules has a backendRef with an unsupported value. @@ -50,7 +50,7 @@ const ( // GatewayMessageFailedNginxReload is a message used with GatewayConditionProgrammed (false) // when nginx fails to reload. GatewayMessageFailedNginxReload = "The Gateway is not programmed due to a failure to " + - "reload nginx with the configuration" + "reload nginx with the configuration. Please see the nginx container logs for any possible configuration issues" // RouteMessageFailedNginxReload is a message used with RouteReasonGatewayNotProgrammed // when nginx fails to reload. diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index cb41b52bd4..d438c1ec66 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -16,13 +16,18 @@ import ( const wildcardHostname = "~^" // BuildConfiguration builds the Configuration from the Graph. -func BuildConfiguration(ctx context.Context, g *graph.Graph, resolver resolver.ServiceResolver) Configuration { +func BuildConfiguration( + ctx context.Context, + g *graph.Graph, + resolver resolver.ServiceResolver, + configVersion int, +) Configuration { if g.GatewayClass == nil || !g.GatewayClass.Valid { - return Configuration{} + return Configuration{Version: configVersion} } if g.Gateway == nil { - return Configuration{} + return Configuration{Version: configVersion} } upstreams := buildUpstreams(ctx, g.Gateway.Listeners, resolver) @@ -36,6 +41,7 @@ func BuildConfiguration(ctx context.Context, g *graph.Graph, resolver resolver.S Upstreams: upstreams, BackendGroups: backendGroups, SSLKeyPairs: keyPairs, + Version: configVersion, } return config diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index 9e722d0e01..cff5bc8592 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -1488,13 +1488,14 @@ func TestBuildConfiguration(t *testing.T) { t.Run(test.msg, func(t *testing.T) { g := NewGomegaWithT(t) - result := BuildConfiguration(context.TODO(), test.graph, fakeResolver) + result := BuildConfiguration(context.TODO(), test.graph, fakeResolver, 1) g.Expect(result.BackendGroups).To(ConsistOf(test.expConf.BackendGroups)) g.Expect(result.Upstreams).To(ConsistOf(test.expConf.Upstreams)) g.Expect(result.HTTPServers).To(ConsistOf(test.expConf.HTTPServers)) g.Expect(result.SSLServers).To(ConsistOf(test.expConf.SSLServers)) g.Expect(result.SSLKeyPairs).To(Equal(test.expConf.SSLKeyPairs)) + g.Expect(result.Version).To(Equal(1)) }) } } diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go index e6ee3f0d5f..9964227829 100644 --- a/internal/mode/static/state/dataplane/types.go +++ b/internal/mode/static/state/dataplane/types.go @@ -31,6 +31,8 @@ type Configuration struct { Upstreams []Upstream // BackendGroups holds all unique BackendGroups. BackendGroups []BackendGroup + // Version represents the version of the generated configuration. + Version int } // SSLKeyPairID is a unique identifier for a SSLKeyPair.