From 6f463bfafd7f2bfdd1e5a3570c64f1c6c6ca41a3 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Mon, 22 May 2023 15:08:48 -0600 Subject: [PATCH 1/4] Set Gateway Programmed condition When the Gateway configuration has been successfully uploaded to nginx, we will set the status Programmed to true. If there's any error with updating nginx, then we set Programmed to false. We'll also set a negative status on the HTTPRoutes in the system to explain that the Gateway is not programmed. Moved the buildStatuses logic into the status package and removed redundant tests. --- docs/gateway-api-compatibility.md | 74 +- internal/events/handler.go | 11 +- internal/events/handler_test.go | 18 +- internal/manager/manager.go | 2 +- internal/nginx/runtime/manager.go | 1 - internal/state/change_processor.go | 26 +- internal/state/change_processor_test.go | 1183 +---------------- internal/state/conditions/conditions.go | 119 +- internal/state/graph/gateway.go | 6 +- internal/state/graph/gateway_test.go | 24 +- .../state/statefakes/fake_change_processor.go | 51 +- internal/status/gateway.go | 4 +- internal/status/gateway_test.go | 5 +- internal/status/gatewayclass.go | 4 +- internal/status/gatewayclass_test.go | 3 +- internal/status/httproute.go | 4 +- internal/status/httproute_test.go | 5 +- internal/{state => status}/statuses.go | 29 +- internal/{state => status}/statuses_test.go | 144 +- internal/status/statusfakes/fake_updater.go | 13 +- internal/status/updater.go | 6 +- internal/status/updater_test.go | 29 +- 22 files changed, 451 insertions(+), 1310 deletions(-) rename internal/{state => status}/statuses.go (88%) rename internal/{state => status}/statuses_test.go (70%) diff --git a/docs/gateway-api-compatibility.md b/docs/gateway-api-compatibility.md index ecc62ff5a4..51f12f5066 100644 --- a/docs/gateway-api-compatibility.md +++ b/docs/gateway-api-compatibility.md @@ -39,11 +39,11 @@ of the [static-mode](./cli-help.md#static-mode) command. Fields: * `spec` - * `controllerName` - supported. - * `parametersRef` - not supported. - * `description` - supported. + * `controllerName` - supported. + * `parametersRef` - not supported. + * `description` - supported. * `status` - * `conditions` - partially supported. + * `conditions` - partially supported. ### Gateway @@ -54,18 +54,18 @@ See [static-mode](./cli-help.md#static-mode) command for more info. Fields: * `spec` - * `gatewayClassName` - supported. - * `listeners` - * `name` - supported. - * `hostname` - partially supported. Wildcard hostnames like `*.example.com` are not yet supported. - * `port` - partially supported. Allowed values: `80` for HTTP listeners and `443` for HTTPS listeners. - * `protocol` - partially supported. Allowed values: `HTTP`, `HTTPS`. - * `tls` - * `mode` - partially supported. Allowed value: `Terminate`. - * `certificateRefs` - partially supported. The TLS certificate and key must be stored in a Secret resource of type `kubernetes.io/tls` in the same namespace as the Gateway resource. Only a single reference is supported. You must deploy the Secret before the Gateway resource. Secret rotation (watching for updates) is not supported. - * `options` - not supported. - * `allowedRoutes` - not supported. - * `addresses` - not supported. + * `gatewayClassName` - supported. + * `listeners` + * `name` - supported. + * `hostname` - partially supported. Wildcard hostnames like `*.example.com` are not yet supported. + * `port` - partially supported. Allowed values: `80` for HTTP listeners and `443` for HTTPS listeners. + * `protocol` - partially supported. Allowed values: `HTTP`, `HTTPS`. + * `tls` + * `mode` - partially supported. Allowed value: `Terminate`. + * `certificateRefs` - partially supported. The TLS certificate and key must be stored in a Secret resource of type `kubernetes.io/tls` in the same namespace as the Gateway resource. Only a single reference is supported. You must deploy the Secret before the Gateway resource. Secret rotation (watching for updates) is not supported. + * `options` - not supported. + * `allowedRoutes` - not supported. + * `addresses` - not supported. * `status` * `addresses` - Pod IPAddress supported. * `conditions` - Supported (Condition/Status/Reason): @@ -75,11 +75,14 @@ Fields: * `Accepted/False/Invalid` * `Accepted/False/UnsupportedValue`: Custom reason for when a value of a field in a Gateway is invalid or not supported. * `Accepted/False/GatewayConflict`: Custom reason for when the Gateway is ignored due to a conflicting Gateway. NKG only supports a single Gateway. + * `Programmed/True/Programmed` + * `Programmed/False/Invalid` + * `Programmed/False/GatewayConflict`: Custom reason for when the Gateway is ignored due to a conflicting Gateway. NKG only supports a single Gateway. * `listeners` - * `name` - supported. - * `supportedKinds` - not supported. - * `attachedRoutes` - supported. - * `conditions` - Supported (Condition/Status/Reason): + * `name` - supported. + * `supportedKinds` - not supported. + * `attachedRoutes` - supported. + * `conditions` - Supported (Condition/Status/Reason): * `Accepted/True/Accepted` * `Accepted/False/UnsupportedProtocol` * `Accepted/False/InvalidCertificateRef` @@ -101,26 +104,27 @@ Fields: * `parentRefs` - partially supported. Port not supported. * `hostnames` - partially supported. Wildcard binding is not supported: a hostname like `example.com` will not bind to a listener with the hostname `*.example.com`. However, `example.com` will bind to a listener with the empty hostname. * `rules` - * `matches` - * `path` - partially supported. Only `PathPrefix` and `Exact` types. - * `headers` - partially supported. Only `Exact` type. - * `queryParams` - partially supported. Only `Exact` type. - * `method` - supported. - * `filters` - * `type` - supported. - * `requestRedirect` - supported except for the experimental `path` field. If multiple filters with `requestRedirect` are configured, NGINX Kubernetes Gateway will choose the first one and ignore the rest. - * `requestHeaderModifier`, `requestMirror`, `urlRewrite`, `extensionRef` - not supported. - * `backendRefs` - partially supported. Backend ref `filters` are not supported. + * `matches` + * `path` - partially supported. Only `PathPrefix` and `Exact` types. + * `headers` - partially supported. Only `Exact` type. + * `queryParams` - partially supported. Only `Exact` type. + * `method` - supported. + * `filters` + * `type` - supported. + * `requestRedirect` - supported except for the experimental `path` field. If multiple filters with `requestRedirect` are configured, NGINX Kubernetes Gateway will choose the first one and ignore the rest. + * `requestHeaderModifier`, `requestMirror`, `urlRewrite`, `extensionRef` - not supported. + * `backendRefs` - partially supported. Backend ref `filters` are not supported. * `status` * `parents` - * `parentRef` - supported. - * `controllerName` - supported. - * `conditions` - partially supported. Supported (Condition/Status/Reason): - * `Accepted/True/Accepted` - * `Accepted/False/NoMatchingListenerHostname` + * `parentRef` - supported. + * `controllerName` - supported. + * `conditions` - partially supported. Supported (Condition/Status/Reason): + * `Accepted/True/Accepted` + * `Accepted/False/NoMatchingListenerHostname` * `Accepted/False/NoMatchingParent` * `Accepted/False/UnsupportedValue`: Custom reason for when the HTTPRoute includes an invalid or unsupported value. * `Accepted/False/InvalidListener`: Custom reason for when the HTTPRoute references an invalid listener. + * `Accepted/False/GatewayNotProgrammed`: Custom reason for when the Gateway is not Programmed. HTTPRoute may be valid and configured, but will maintain this status as long as the Gateway is not Programmed. * `ResolvedRefs/True/ResolvedRefs` * `ResolvedRefs/False/InvalidKind` * `ResolvedRefs/False/RefNotPermitted` diff --git a/internal/events/handler.go b/internal/events/handler.go index 3326b569e3..3477a3adec 100644 --- a/internal/events/handler.go +++ b/internal/events/handler.go @@ -14,6 +14,7 @@ import ( "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/runtime" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/dataplane" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets" "github.com/nginxinc/nginx-kubernetes-gateway/internal/status" ) @@ -35,6 +36,8 @@ type EventHandlerConfig struct { SecretStore secrets.SecretStore // SecretMemoryManager is the state SecretMemoryManager. SecretMemoryManager secrets.SecretDiskMemoryManager + // ServiceResolver resolves Services to Endpoints. + ServiceResolver resolver.ServiceResolver // Generator is the nginx config Generator. Generator config.Generator // NginxFileMgr is the file Manager for nginx. @@ -74,20 +77,22 @@ func (h *EventHandlerImpl) HandleEventBatch(ctx context.Context, batch EventBatc } } - changed, conf, statuses := h.cfg.Processor.Process(ctx) + changed, graphCfg := h.cfg.Processor.Process() if !changed { h.cfg.Logger.Info("Handling events didn't result into NGINX configuration changes") return } - err := h.updateNginx(ctx, conf) + var nginxReloadRes status.NginxReloadResult + err := h.updateNginx(ctx, dataplane.BuildConfiguration(ctx, graphCfg, h.cfg.ServiceResolver)) if err != nil { h.cfg.Logger.Error(err, "Failed to update NGINX configuration") + nginxReloadRes.Error = err } else { h.cfg.Logger.Info("NGINX configuration was successfully updated") } - h.cfg.StatusUpdater.Update(ctx, statuses) + h.cfg.StatusUpdater.Update(ctx, status.BuildStatuses(graphCfg, nginxReloadRes)) } func (h *EventHandlerImpl) updateNginx(ctx context.Context, conf dataplane.Configuration) error { diff --git a/internal/events/handler_test.go b/internal/events/handler_test.go index 2f7d49620d..c0218b09b0 100644 --- a/internal/events/handler_test.go +++ b/internal/events/handler_test.go @@ -19,8 +19,8 @@ import ( "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config/configfakes" "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/file/filefakes" "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/runtime/runtimefakes" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/dataplane" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/graph" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets/secretsfakes" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/statefakes" "github.com/nginxinc/nginx-kubernetes-gateway/internal/status/statusfakes" @@ -50,7 +50,7 @@ var _ = Describe("EventHandler", func() { fakeStatusUpdater *statusfakes.FakeUpdater ) - expectReconfig := func(expectedConf dataplane.Configuration, expectedCfg []byte, expectedStatuses state.Statuses) { + expectReconfig := func(expectedConf dataplane.Configuration, expectedCfg []byte) { Expect(fakeProcessor.ProcessCallCount()).Should(Equal(1)) Expect(fakeGenerator.GenerateCallCount()).Should(Equal(1)) @@ -64,8 +64,6 @@ var _ = Describe("EventHandler", func() { Expect(fakeNginxRuntimeMgr.ReloadCallCount()).Should(Equal(1)) Expect(fakeStatusUpdater.UpdateCallCount()).Should(Equal(1)) - _, statuses := fakeStatusUpdater.UpdateArgsForCall(0) - Expect(statuses).Should(Equal(expectedStatuses)) } BeforeEach(func() { @@ -93,10 +91,8 @@ var _ = Describe("EventHandler", func() { DescribeTable( "A batch with one event", func(e interface{}) { - fakeConf := dataplane.Configuration{} - fakeStatuses := state.Statuses{} changed := true - fakeProcessor.ProcessReturns(changed, fakeConf, fakeStatuses) + fakeProcessor.ProcessReturns(changed, &graph.Graph{}) fakeCfg := []byte("fake") fakeGenerator.GenerateReturns(fakeCfg) @@ -120,7 +116,7 @@ var _ = Describe("EventHandler", func() { } // Check that a reconfig happened - expectReconfig(fakeConf, fakeCfg, fakeStatuses) + expectReconfig(dataplane.Configuration{}, fakeCfg) }, Entry( "HTTPRoute upsert", @@ -258,10 +254,8 @@ var _ = Describe("EventHandler", func() { batch = append(batch, upserts...) batch = append(batch, deletes...) - fakeConf := dataplane.Configuration{} changed := true - fakeStatuses := state.Statuses{} - fakeProcessor.ProcessReturns(changed, fakeConf, fakeStatuses) + fakeProcessor.ProcessReturns(changed, &graph.Graph{}) fakeCfg := []byte("fake") fakeGenerator.GenerateReturns(fakeCfg) @@ -294,7 +288,7 @@ var _ = Describe("EventHandler", func() { Expect(fakeSecretStore.DeleteArgsForCall(0)).Should(Equal(secretNsName)) // Check that a reconfig happened - expectReconfig(fakeConf, fakeCfg, fakeStatuses) + expectReconfig(dataplane.Configuration{}, fakeCfg) }) Describe("Edge cases", func() { diff --git a/internal/manager/manager.go b/internal/manager/manager.go index 520d932c89..b67fc565e0 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -133,7 +133,6 @@ func Start(cfg config.Config) error { GatewayCtlrName: cfg.GatewayCtlrName, GatewayClassName: cfg.GatewayClassName, SecretMemoryManager: secretMemoryMgr, - ServiceResolver: resolver.NewServiceResolverImpl(mgr.GetClient()), RelationshipCapturer: relationship.NewCapturerImpl(), Logger: cfg.Logger.WithName("changeProcessor"), Validators: validation.Validators{ @@ -160,6 +159,7 @@ func Start(cfg config.Config) error { Processor: processor, SecretStore: secretStore, SecretMemoryManager: secretMemoryMgr, + ServiceResolver: resolver.NewServiceResolverImpl(mgr.GetClient()), Generator: configGenerator, Logger: cfg.Logger.WithName("eventHandler"), NginxFileMgr: nginxFileMgr, diff --git a/internal/nginx/runtime/manager.go b/internal/nginx/runtime/manager.go index ee8892c139..54fcc64ba5 100644 --- a/internal/nginx/runtime/manager.go +++ b/internal/nginx/runtime/manager.go @@ -51,7 +51,6 @@ func (m *ManagerImpl) Reload(ctx context.Context) error { // FIXME(pleshakov) // (1) ensure the reload actually happens. - // (2) ensure that in case of an error, the error message can be seen by the admins. // 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. diff --git a/internal/state/change_processor.go b/internal/state/change_processor.go index 0536e84138..f6fda13434 100644 --- a/internal/state/change_processor.go +++ b/internal/state/change_processor.go @@ -1,7 +1,6 @@ package state import ( - "context" "fmt" "sync" @@ -18,10 +17,8 @@ import ( gwapivalidation "sigs.k8s.io/gateway-api/apis/v1beta1/validation" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/dataplane" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/graph" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/relationship" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/validation" ) @@ -46,19 +43,15 @@ type ChangeProcessor interface { // The method panics if the resource is of unsupported type or if the passed Gateway is different from the one // this ChangeProcessor was created for. CaptureDeleteChange(resourceType client.Object, nsname types.NamespacedName) - // Process processes any captured changes and produces an internal representation of the Gateway configuration and - // the status information about the processed resources. - // If no changes were captured, the changed return argument will be false and both the configuration and statuses - // will be empty. - Process(ctx context.Context) (changed bool, conf dataplane.Configuration, statuses Statuses) + // Process produces an internal representation of the Gateway configuration. + // If no changes were captured, the changed return argument will be false and graph will be empty. + Process() (changed bool, graphCfg *graph.Graph) } // ChangeProcessorConfig holds configuration parameters for ChangeProcessorImpl. type ChangeProcessorConfig struct { // SecretMemoryManager is the secret memory manager. SecretMemoryManager secrets.SecretDiskMemoryManager - // ServiceResolver resolves Services to Endpoints. - ServiceResolver resolver.ServiceResolver // RelationshipCapturer captures relationships between Kubernetes API resources and Gateway API resources. RelationshipCapturer relationship.Capturer // Validators validate resources according to data-plane specific rules. @@ -197,17 +190,15 @@ func (c *ChangeProcessorImpl) CaptureDeleteChange(resourceType client.Object, ns c.updater.Delete(resourceType, nsname) } -func (c *ChangeProcessorImpl) Process( - ctx context.Context, -) (changed bool, conf dataplane.Configuration, statuses Statuses) { +func (c *ChangeProcessorImpl) Process() (bool, *graph.Graph) { c.lock.Lock() defer c.lock.Unlock() if !c.getAndResetClusterStateChanged() { - return false, conf, statuses + return false, nil } - g := graph.BuildGraph( + graphCfg := graph.BuildGraph( c.clusterState, c.cfg.GatewayCtlrName, c.cfg.GatewayClassName, @@ -215,8 +206,5 @@ func (c *ChangeProcessorImpl) Process( c.cfg.Validators, ) - conf = dataplane.BuildConfiguration(ctx, g, c.cfg.ServiceResolver) - statuses = buildStatuses(g) - - return true, conf, statuses + return true, graphCfg } diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go index e679f3031d..c8fee94da6 100644 --- a/internal/state/change_processor_test.go +++ b/internal/state/change_processor_test.go @@ -1,9 +1,6 @@ package state_test import ( - "context" - "sort" - . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" apiv1 "k8s.io/api/core/v1" @@ -21,8 +18,7 @@ import ( "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" "github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/index" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/dataplane" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/graph" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/relationship" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/relationship/relationshipfakes" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets/secretsfakes" @@ -191,24 +187,6 @@ func createScheme() *runtime.Scheme { return scheme } -func assertStatuses(expected, result state.Statuses) { - sortConditions := func(statuses state.HTTPRouteStatuses) { - for _, status := range statuses { - for _, ps := range status.ParentStatuses { - sort.Slice(ps.Conditions, func(i, j int) bool { - return ps.Conditions[i].Type < ps.Conditions[j].Type - }) - } - } - } - - sortConditions(expected.HTTPRouteStatuses) - sortConditions(result.HTTPRouteStatuses) - - ExpectWithOffset(1, helpers.Diff(expected, result)).To(BeEmpty()) -} - -// FIXME(kate-osborn): Consider refactoring these tests to reduce code duplication. var _ = Describe("ChangeProcessor", func() { Describe("Normal cases of processing changes", func() { var ( @@ -243,10 +221,9 @@ var _ = Describe("ChangeProcessor", func() { Describe("Process gateway resources", Ordered, func() { var ( - gcUpdated *v1beta1.GatewayClass - hr1, hr1Updated, hr2 *v1beta1.HTTPRoute - hr1Group, hr2Group dataplane.BackendGroup - gw1, gw1Updated, gw2 *v1beta1.Gateway + gcUpdated *v1beta1.GatewayClass + hr1, hr1Updated *v1beta1.HTTPRoute + gw1, gw1Updated *v1beta1.Gateway ) BeforeAll(func() { gcUpdated = gc.DeepCopy() @@ -254,1012 +231,122 @@ var _ = Describe("ChangeProcessor", func() { hr1 = createRoute("hr-1", "gateway-1", "foo.example.com") - hr1Group = dataplane.BackendGroup{ - Source: types.NamespacedName{Namespace: hr1.Namespace, Name: hr1.Name}, - RuleIdx: 0, - } - hr1Updated = hr1.DeepCopy() hr1Updated.Generation++ - hr2 = createRoute("hr-2", "gateway-2", "bar.example.com") - - hr2Group = dataplane.BackendGroup{ - Source: types.NamespacedName{Namespace: hr2.Namespace, Name: hr2.Name}, - RuleIdx: 0, - } - gw1 = createGatewayWithTLSListener("gateway-1") gw1Updated = gw1.DeepCopy() gw1Updated.Generation++ - - gw2 = createGatewayWithTLSListener("gateway-2") }) When("no upsert has occurred", func() { - It("returns empty configuration and statuses", func() { - changed, conf, statuses := processor.Process(context.TODO()) + It("returns empty graph", func() { + changed, graphCfg := processor.Process() Expect(changed).To(BeFalse()) - Expect(conf).To(BeZero()) - Expect(statuses).To(BeZero()) - }) - }) - When("GatewayClass doesn't exist", func() { - When("Gateways don't exist", func() { - When("the first HTTPRoute is upserted", func() { - It("returns empty configuration and statuses", func() { - processor.CaptureUpsertChange(hr1) - - expectedConf := dataplane.Configuration{} - expectedStatuses := state.Statuses{ - GatewayStatuses: state.GatewayStatuses{}, - HTTPRouteStatuses: state.HTTPRouteStatuses{}, - } - - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) - }) - }) - }) - When("the first Gateway is upserted", func() { - It("returns empty configuration and updated statuses", func() { - processor.CaptureUpsertChange(gw1) - - expectedConf := dataplane.Configuration{} - expectedStatuses := state.Statuses{ - GatewayStatuses: state.GatewayStatuses{ - {Namespace: "test", Name: "gateway-1"}: { - Conditions: []conditions.Condition{ - conditions.NewGatewayInvalid("GatewayClass doesn't exist"), - }, - ObservedGeneration: gw1.Generation, - }, - }, - HTTPRouteStatuses: state.HTTPRouteStatuses{ - {Namespace: "test", Name: "hr-1"}: { - ObservedGeneration: hr1.Generation, - ParentStatuses: []state.ParentStatus{ - { - GatewayNsName: client.ObjectKeyFromObject(gw1), - SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-80-1"), - Conditions: []conditions.Condition{ - conditions.NewRouteResolvedRefs(), - conditions.NewRouteInvalidGateway(), - }, - }, - { - GatewayNsName: client.ObjectKeyFromObject(gw1), - SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-443-1"), - Conditions: []conditions.Condition{ - conditions.NewRouteResolvedRefs(), - conditions.NewRouteInvalidGateway(), - }, - }, - }, - }, - }, - } - - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - assertStatuses(expectedStatuses, statuses) - }) + Expect(graphCfg).To(BeNil()) }) }) - When("the GatewayClass is upserted", func() { - It("returns updated configuration and statuses", func() { + When("upsert has occurred", func() { + It("returns populated graph", func() { + processor.CaptureUpsertChange(hr1) + processor.CaptureUpsertChange(gw1) processor.CaptureUpsertChange(gc) - expectedConf := dataplane.Configuration{ - HTTPServers: []dataplane.VirtualServer{ - { - IsDefault: true, - }, - { - Hostname: "foo.example.com", - PathRules: []dataplane.PathRule{ - { - Path: "/", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1, - }, - }, - }, - }, - }, - }, - SSLServers: []dataplane.VirtualServer{ - { - IsDefault: true, - }, - { - Hostname: "foo.example.com", - SSL: &dataplane.SSL{CertificatePath: certificatePath}, - PathRules: []dataplane.PathRule{ - { - Path: "/", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1, - }, - }, - }, - }, - }, - { - Hostname: "~^", - SSL: &dataplane.SSL{CertificatePath: certificatePath}, - }, - }, - BackendGroups: []dataplane.BackendGroup{hr1Group}, - } - - expectedStatuses := state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ - ObservedGeneration: gc.Generation, - Conditions: conditions.NewDefaultGatewayClassConditions(), - }, - GatewayStatuses: state.GatewayStatuses{ - {Namespace: "test", Name: "gateway-1"}: { - Conditions: conditions.NewDefaultGatewayConditions(), - ObservedGeneration: gw1.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - "listener-443-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - }, - }, - }, - HTTPRouteStatuses: state.HTTPRouteStatuses{ - {Namespace: "test", Name: "hr-1"}: { - ObservedGeneration: hr1.Generation, - ParentStatuses: []state.ParentStatus{ - { - GatewayNsName: client.ObjectKeyFromObject(gw1), - SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-80-1"), - Conditions: conditions.NewDefaultRouteConditions(), - }, - { - GatewayNsName: client.ObjectKeyFromObject(gw1), - SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-443-1"), - Conditions: conditions.NewDefaultRouteConditions(), - }, - }, - }, - }, - } - - changed, conf, statuses := processor.Process(context.TODO()) + changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - assertStatuses(expectedStatuses, statuses) + Expect(graphCfg).ToNot(BeNil()) }) }) When("the first HTTPRoute without a generation changed is processed", func() { - It("returns empty configuration and statuses", func() { + It("returns empty graph", func() { hr1UpdatedSameGen := hr1.DeepCopy() // hr1UpdatedSameGen.Generation has not been changed processor.CaptureUpsertChange(hr1UpdatedSameGen) - changed, conf, statuses := processor.Process(context.TODO()) + changed, graphCfg := processor.Process() Expect(changed).To(BeFalse()) - Expect(conf).To(BeZero()) - Expect(statuses).To(BeZero()) + Expect(graphCfg).To(BeNil()) }) }) When("the first HTTPRoute update with a generation changed is processed", func() { - It("returns updated configuration and statuses", func() { + It("returns populated graph", func() { processor.CaptureUpsertChange(hr1Updated) - expectedConf := dataplane.Configuration{ - HTTPServers: []dataplane.VirtualServer{ - { - IsDefault: true, - }, - { - Hostname: "foo.example.com", - PathRules: []dataplane.PathRule{ - { - Path: "/", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1Updated, - }, - }, - }, - }, - }, - }, - SSLServers: []dataplane.VirtualServer{ - { - IsDefault: true, - }, - { - Hostname: "foo.example.com", - SSL: &dataplane.SSL{CertificatePath: certificatePath}, - PathRules: []dataplane.PathRule{ - { - Path: "/", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1Updated, - }, - }, - }, - }, - }, - { - Hostname: "~^", - SSL: &dataplane.SSL{CertificatePath: certificatePath}, - }, - }, - BackendGroups: []dataplane.BackendGroup{hr1Group}, - } - expectedStatuses := state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ - ObservedGeneration: gc.Generation, - Conditions: conditions.NewDefaultGatewayClassConditions(), - }, - GatewayStatuses: state.GatewayStatuses{ - {Namespace: "test", Name: "gateway-1"}: { - Conditions: conditions.NewDefaultGatewayConditions(), - ObservedGeneration: gw1.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - "listener-443-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - }, - }, - }, - HTTPRouteStatuses: state.HTTPRouteStatuses{ - {Namespace: "test", Name: "hr-1"}: { - ObservedGeneration: hr1Updated.Generation, - ParentStatuses: []state.ParentStatus{ - { - GatewayNsName: client.ObjectKeyFromObject(gw1), - SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-80-1"), - Conditions: conditions.NewDefaultRouteConditions(), - }, - { - GatewayNsName: client.ObjectKeyFromObject(gw1), - SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-443-1"), - Conditions: conditions.NewDefaultRouteConditions(), - }, - }, - }, - }, - } - - changed, conf, statuses := processor.Process(context.TODO()) + changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - assertStatuses(expectedStatuses, statuses) + Expect(graphCfg).ToNot(BeNil()) }, ) }) When("the first Gateway update without generation changed is processed", func() { - It("returns empty configuration and statuses", func() { + It("returns empty graph", func() { gwUpdatedSameGen := gw1.DeepCopy() // gwUpdatedSameGen.Generation has not been changed processor.CaptureUpsertChange(gwUpdatedSameGen) - changed, conf, statuses := processor.Process(context.TODO()) + changed, graphCfg := processor.Process() Expect(changed).To(BeFalse()) - Expect(conf).To(BeZero()) - Expect(statuses).To(BeZero()) + Expect(graphCfg).To(BeNil()) }) }) When("the first Gateway update with a generation changed is processed", func() { - It("returns updated configuration and statuses", func() { + It("returns populated graph", func() { processor.CaptureUpsertChange(gw1Updated) - expectedConf := dataplane.Configuration{ - HTTPServers: []dataplane.VirtualServer{ - { - IsDefault: true, - }, - { - Hostname: "foo.example.com", - PathRules: []dataplane.PathRule{ - { - Path: "/", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1Updated, - }, - }, - }, - }, - }, - }, - SSLServers: []dataplane.VirtualServer{ - { - IsDefault: true, - }, - { - Hostname: "foo.example.com", - SSL: &dataplane.SSL{CertificatePath: certificatePath}, - PathRules: []dataplane.PathRule{ - { - Path: "/", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1Updated, - }, - }, - }, - }, - }, - { - Hostname: "~^", - SSL: &dataplane.SSL{CertificatePath: certificatePath}, - }, - }, - BackendGroups: []dataplane.BackendGroup{hr1Group}, - } - expectedStatuses := state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ - ObservedGeneration: gc.Generation, - Conditions: conditions.NewDefaultGatewayClassConditions(), - }, - GatewayStatuses: state.GatewayStatuses{ - {Namespace: "test", Name: "gateway-1"}: { - Conditions: conditions.NewDefaultGatewayConditions(), - ObservedGeneration: gw1Updated.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - "listener-443-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - }, - }, - }, - HTTPRouteStatuses: state.HTTPRouteStatuses{ - {Namespace: "test", Name: "hr-1"}: { - ObservedGeneration: hr1Updated.Generation, - ParentStatuses: []state.ParentStatus{ - { - GatewayNsName: client.ObjectKeyFromObject(gw1Updated), - SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-80-1"), - Conditions: conditions.NewDefaultRouteConditions(), - }, - { - GatewayNsName: client.ObjectKeyFromObject(gw1Updated), - SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-443-1"), - Conditions: conditions.NewDefaultRouteConditions(), - }, - }, - }, - }, - } - - changed, conf, statuses := processor.Process(context.TODO()) + changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - assertStatuses(expectedStatuses, statuses) + Expect(graphCfg).ToNot(BeNil()) }) }) When("the GatewayClass update without generation change is processed", func() { - It("returns empty configuration and statuses", func() { + It("returns empty graph", func() { gcUpdatedSameGen := gc.DeepCopy() // gcUpdatedSameGen.Generation has not been changed processor.CaptureUpsertChange(gcUpdatedSameGen) - changed, conf, statuses := processor.Process(context.TODO()) + changed, graphCfg := processor.Process() Expect(changed).To(BeFalse()) - Expect(conf).To(BeZero()) - Expect(statuses).To(BeZero()) + Expect(graphCfg).To(BeNil()) }) }) When("the GatewayClass update with generation change is processed", func() { - It("returns updated configuration and statuses", func() { + It("returns populated graph", func() { processor.CaptureUpsertChange(gcUpdated) - expectedConf := dataplane.Configuration{ - HTTPServers: []dataplane.VirtualServer{ - { - IsDefault: true, - }, - { - Hostname: "foo.example.com", - PathRules: []dataplane.PathRule{ - { - Path: "/", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1Updated, - }, - }, - }, - }, - }, - }, - SSLServers: []dataplane.VirtualServer{ - { - IsDefault: true, - }, - { - Hostname: "foo.example.com", - SSL: &dataplane.SSL{CertificatePath: certificatePath}, - PathRules: []dataplane.PathRule{ - { - Path: "/", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1Updated, - }, - }, - }, - }, - }, - { - Hostname: "~^", - SSL: &dataplane.SSL{CertificatePath: certificatePath}, - }, - }, - BackendGroups: []dataplane.BackendGroup{hr1Group}, - } - expectedStatuses := state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ - ObservedGeneration: gcUpdated.Generation, - Conditions: conditions.NewDefaultGatewayClassConditions(), - }, - GatewayStatuses: state.GatewayStatuses{ - {Namespace: "test", Name: "gateway-1"}: { - Conditions: conditions.NewDefaultGatewayConditions(), - ObservedGeneration: gw1Updated.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - "listener-443-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - }, - }, - }, - HTTPRouteStatuses: state.HTTPRouteStatuses{ - {Namespace: "test", Name: "hr-1"}: { - ObservedGeneration: hr1Updated.Generation, - ParentStatuses: []state.ParentStatus{ - { - GatewayNsName: client.ObjectKeyFromObject(gw1Updated), - SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-80-1"), - Conditions: conditions.NewDefaultRouteConditions(), - }, - { - GatewayNsName: client.ObjectKeyFromObject(gw1Updated), - SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-443-1"), - Conditions: conditions.NewDefaultRouteConditions(), - }, - }, - }, - }, - } - - changed, conf, statuses := processor.Process(context.TODO()) + changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - assertStatuses(expectedStatuses, statuses) + Expect(graphCfg).ToNot(BeNil()) }) }) When("no changes are captured", func() { - It("returns empty configuration and statuses", func() { - changed, conf, statuses := processor.Process(context.TODO()) + It("returns empty graph", func() { + changed, graphCfg := processor.Process() Expect(changed).To(BeFalse()) - Expect(conf).To(BeZero()) - Expect(statuses).To(BeZero()) + Expect(graphCfg).To(BeNil()) }) }) - When("the second Gateway is upserted", func() { - It("returns updated configuration and statuses", func() { - processor.CaptureUpsertChange(gw2) - - expectedConf := dataplane.Configuration{ - HTTPServers: []dataplane.VirtualServer{ - { - IsDefault: true, - }, - { - Hostname: "foo.example.com", - PathRules: []dataplane.PathRule{ - { - Path: "/", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1Updated, - }, - }, - }, - }, - }, - }, - SSLServers: []dataplane.VirtualServer{ - { - IsDefault: true, - }, - { - Hostname: "foo.example.com", - PathRules: []dataplane.PathRule{ - { - Path: "/", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1Updated, - }, - }, - }, - }, - SSL: &dataplane.SSL{ - CertificatePath: certificatePath, - }, - }, - { - Hostname: "~^", - SSL: &dataplane.SSL{CertificatePath: certificatePath}, - }, - }, - BackendGroups: []dataplane.BackendGroup{hr1Group}, - } - expectedStatuses := state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ - ObservedGeneration: gcUpdated.Generation, - Conditions: conditions.NewDefaultGatewayClassConditions(), - }, - GatewayStatuses: state.GatewayStatuses{ - {Namespace: "test", Name: "gateway-1"}: { - Conditions: conditions.NewDefaultGatewayConditions(), - ObservedGeneration: gw1Updated.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - "listener-443-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - }, - }, - {Namespace: "test", Name: "gateway-2"}: { - Conditions: []conditions.Condition{conditions.NewGatewayConflict()}, - ObservedGeneration: gw2.Generation, - }, - }, - HTTPRouteStatuses: state.HTTPRouteStatuses{ - {Namespace: "test", Name: "hr-1"}: { - ObservedGeneration: hr1Updated.Generation, - ParentStatuses: []state.ParentStatus{ - { - GatewayNsName: client.ObjectKeyFromObject(gw1Updated), - SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-80-1"), - Conditions: conditions.NewDefaultRouteConditions(), - }, - { - GatewayNsName: client.ObjectKeyFromObject(gw1Updated), - SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-443-1"), - Conditions: conditions.NewDefaultRouteConditions(), - }, - }, - }, - }, - } - - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - assertStatuses(expectedStatuses, statuses) - }) - }) - When("the second HTTPRoute is upserted", func() { - It("returns same configuration and updated statuses", func() { - processor.CaptureUpsertChange(hr2) - - expectedConf := dataplane.Configuration{ - HTTPServers: []dataplane.VirtualServer{ - { - IsDefault: true, - }, - { - Hostname: "foo.example.com", - PathRules: []dataplane.PathRule{ - { - Path: "/", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1Updated, - }, - }, - }, - }, - }, - }, - SSLServers: []dataplane.VirtualServer{ - { - IsDefault: true, - }, - { - Hostname: "foo.example.com", - SSL: &dataplane.SSL{CertificatePath: certificatePath}, - PathRules: []dataplane.PathRule{ - { - Path: "/", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1Updated, - }, - }, - }, - }, - }, - { - Hostname: "~^", - SSL: &dataplane.SSL{CertificatePath: certificatePath}, - }, - }, - BackendGroups: []dataplane.BackendGroup{hr1Group}, - } - expectedStatuses := state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ - ObservedGeneration: gcUpdated.Generation, - Conditions: conditions.NewDefaultGatewayClassConditions(), - }, - GatewayStatuses: state.GatewayStatuses{ - {Namespace: "test", Name: "gateway-1"}: { - Conditions: conditions.NewDefaultGatewayConditions(), - ObservedGeneration: gw1Updated.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - "listener-443-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - }, - }, - {Namespace: "test", Name: "gateway-2"}: { - ObservedGeneration: gw2.Generation, - Conditions: []conditions.Condition{conditions.NewGatewayConflict()}, - }, - }, - HTTPRouteStatuses: state.HTTPRouteStatuses{ - {Namespace: "test", Name: "hr-1"}: { - ObservedGeneration: hr1Updated.Generation, - ParentStatuses: []state.ParentStatus{ - { - GatewayNsName: client.ObjectKeyFromObject(gw1Updated), - SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-80-1"), - Conditions: conditions.NewDefaultRouteConditions(), - }, - { - GatewayNsName: client.ObjectKeyFromObject(gw1Updated), - SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-443-1"), - Conditions: conditions.NewDefaultRouteConditions(), - }, - }, - }, - {Namespace: "test", Name: "hr-2"}: { - ObservedGeneration: hr2.Generation, - ParentStatuses: []state.ParentStatus{ - { - GatewayNsName: client.ObjectKeyFromObject(gw2), - SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-80-1"), - Conditions: append( - conditions.NewDefaultRouteConditions(), - conditions.NewTODO("Gateway is ignored"), - ), - }, - { - GatewayNsName: client.ObjectKeyFromObject(gw2), - SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-443-1"), - Conditions: append( - conditions.NewDefaultRouteConditions(), - conditions.NewTODO("Gateway is ignored"), - ), - }, - }, - }, - }, - } - - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - assertStatuses(expectedStatuses, statuses) - }) - }) - When("the first Gateway is deleted", func() { - It("returns updated configuration and statuses", func() { - processor.CaptureDeleteChange( - &v1beta1.Gateway{}, - types.NamespacedName{Namespace: "test", Name: "gateway-1"}, - ) - - expectedConf := dataplane.Configuration{ - HTTPServers: []dataplane.VirtualServer{ - { - IsDefault: true, - }, - { - Hostname: "bar.example.com", - PathRules: []dataplane.PathRule{ - { - Path: "/", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr2Group, - Source: hr2, - }, - }, - }, - }, - }, - }, - SSLServers: []dataplane.VirtualServer{ - { - IsDefault: true, - }, - { - Hostname: "bar.example.com", - SSL: &dataplane.SSL{CertificatePath: certificatePath}, - PathRules: []dataplane.PathRule{ - { - Path: "/", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr2Group, - Source: hr2, - }, - }, - }, - }, - }, - { - Hostname: "~^", - SSL: &dataplane.SSL{CertificatePath: certificatePath}, - }, - }, - BackendGroups: []dataplane.BackendGroup{hr2Group}, - } - expectedStatuses := state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ - ObservedGeneration: gcUpdated.Generation, - Conditions: conditions.NewDefaultGatewayClassConditions(), - }, - GatewayStatuses: state.GatewayStatuses{ - {Namespace: "test", Name: "gateway-2"}: { - Conditions: conditions.NewDefaultGatewayConditions(), - ObservedGeneration: gw2.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - "listener-443-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - }, - }, - }, - HTTPRouteStatuses: state.HTTPRouteStatuses{ - {Namespace: "test", Name: "hr-2"}: { - ObservedGeneration: hr2.Generation, - ParentStatuses: []state.ParentStatus{ - { - GatewayNsName: client.ObjectKeyFromObject(gw2), - SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-80-1"), - Conditions: conditions.NewDefaultRouteConditions(), - }, - { - GatewayNsName: client.ObjectKeyFromObject(gw2), - SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-443-1"), - Conditions: conditions.NewDefaultRouteConditions(), - }, - }, - }, - }, - } - - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - assertStatuses(expectedStatuses, statuses) - }) - }) - When("the second HTTPRoute is deleted", func() { - It("returns configuration with default ssl server and updated statuses", func() { - processor.CaptureDeleteChange( - &v1beta1.HTTPRoute{}, - types.NamespacedName{Namespace: "test", Name: "hr-2"}, - ) - - expectedConf := dataplane.Configuration{ - HTTPServers: []dataplane.VirtualServer{ - { - IsDefault: true, - }, - }, - SSLServers: []dataplane.VirtualServer{ - { - IsDefault: true, - }, - { - Hostname: "~^", - SSL: &dataplane.SSL{CertificatePath: certificatePath}, - }, - }, - } - expectedStatuses := state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ - ObservedGeneration: gcUpdated.Generation, - Conditions: conditions.NewDefaultGatewayClassConditions(), - }, - GatewayStatuses: state.GatewayStatuses{ - {Namespace: "test", Name: "gateway-2"}: { - Conditions: conditions.NewDefaultGatewayConditions(), - ObservedGeneration: gw2.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 0, - Conditions: conditions.NewDefaultListenerConditions(), - }, - "listener-443-1": { - AttachedRoutes: 0, - Conditions: conditions.NewDefaultListenerConditions(), - }, - }, - }, - }, - HTTPRouteStatuses: state.HTTPRouteStatuses{}, - } - - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - assertStatuses(expectedStatuses, statuses) - }) - }) - When("the GatewayClass is deleted", func() { - It("returns empty configuration and updated statuses", func() { + When("resources are deleted", func() { + It("returns empty graph", func() { processor.CaptureDeleteChange( &v1beta1.GatewayClass{}, types.NamespacedName{Name: gcName}, ) - - expectedConf := dataplane.Configuration{} - expectedStatuses := state.Statuses{ - GatewayStatuses: state.GatewayStatuses{ - {Namespace: "test", Name: "gateway-2"}: { - Conditions: []conditions.Condition{ - conditions.NewGatewayInvalid("GatewayClass doesn't exist"), - }, - ObservedGeneration: gw2.Generation, - }, - }, - HTTPRouteStatuses: state.HTTPRouteStatuses{}, - } - - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - assertStatuses(expectedStatuses, statuses) - }) - }) - When("the second Gateway is deleted", func() { - It("returns empty configuration and empty statuses", func() { processor.CaptureDeleteChange( &v1beta1.Gateway{}, - types.NamespacedName{Namespace: "test", Name: "gateway-2"}, + types.NamespacedName{Namespace: "test", Name: "gateway-1"}, ) - - expectedConf := dataplane.Configuration{} - expectedStatuses := state.Statuses{ - GatewayStatuses: state.GatewayStatuses{}, - HTTPRouteStatuses: state.HTTPRouteStatuses{}, - } - - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - assertStatuses(expectedStatuses, statuses) - }) - }) - When("the first HTTPRoute is deleted", func() { - It("returns empty configuration and empty statuses", func() { processor.CaptureDeleteChange( &v1beta1.HTTPRoute{}, types.NamespacedName{Namespace: "test", Name: "hr-1"}, ) - expectedConf := dataplane.Configuration{} - expectedStatuses := state.Statuses{ - GatewayStatuses: state.GatewayStatuses{}, - HTTPRouteStatuses: state.HTTPRouteStatuses{}, - } + emptyGraph := &graph.Graph{} - changed, conf, statuses := processor.Process(context.TODO()) + changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - assertStatuses(expectedStatuses, statuses) + Expect(helpers.Diff(emptyGraph, graphCfg)).To(BeEmpty()) }) }) }) @@ -1336,7 +423,7 @@ var _ = Describe("ChangeProcessor", func() { }) testProcessChangedVal := func(expChanged bool) { - changed, _, _ := processor.Process(context.TODO()) + changed, _ := processor.Process() Expect(changed).To(Equal(expChanged)) } @@ -1703,7 +790,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(gw1) processor.CaptureUpsertChange(hr1) - changed, _, _ := processor.Process(context.TODO()) + changed, _ := processor.Process() Expect(changed).To(BeTrue()) }) It("should report not changed after multiple Upserts of the resource with same generation", func() { @@ -1711,7 +798,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(gw1) processor.CaptureUpsertChange(hr1) - changed, _, _ := processor.Process(context.TODO()) + changed, _ := processor.Process() Expect(changed).To(BeFalse()) }) When("a upsert of updated resources is followed by an upsert of the same generation", func() { @@ -1726,7 +813,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(gw1Updated) processor.CaptureUpsertChange(hr1Updated) - changed, _, _ := processor.Process(context.TODO()) + changed, _ := processor.Process() Expect(changed).To(BeTrue()) }) }) @@ -1735,7 +822,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(gw2) processor.CaptureUpsertChange(hr2) - changed, _, _ := processor.Process(context.TODO()) + changed, _ := processor.Process() Expect(changed).To(BeTrue()) }) When("resources are deleted followed by upserts with the same generations", func() { @@ -1749,14 +836,14 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(gw2) processor.CaptureUpsertChange(hr2) - changed, _, _ := processor.Process(context.TODO()) + changed, _ := processor.Process() Expect(changed).To(BeTrue()) }) }) It("should report changed after deleting resources", func() { processor.CaptureDeleteChange(&v1beta1.HTTPRoute{}, hr2NsName) - changed, _, _ := processor.Process(context.TODO()) + changed, _ := processor.Process() Expect(changed).To(BeTrue()) }) }) @@ -1767,7 +854,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureDeleteChange(&v1beta1.HTTPRoute{}, hrNsName) processor.CaptureDeleteChange(&v1beta1.HTTPRoute{}, hr2NsName) - changed, _, _ := processor.Process(context.TODO()) + changed, _ := processor.Process() Expect(changed).To(BeFalse()) }) }) @@ -1777,7 +864,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(svc) processor.CaptureUpsertChange(slice) - changed, _, _ := processor.Process(context.TODO()) + changed, _ := processor.Process() Expect(changed).To(BeTrue()) }) @@ -1786,7 +873,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(svc) processor.CaptureUpsertChange(slice) - changed, _, _ := processor.Process(context.TODO()) + changed, _ := processor.Process() Expect(changed).To(BeFalse()) }) When("upserts of related resources are followed by upserts of unrelated resources", func() { @@ -1801,7 +888,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(svc) processor.CaptureUpsertChange(slice) - changed, _, _ := processor.Process(context.TODO()) + changed, _ := processor.Process() Expect(changed).To(BeTrue()) }) }) @@ -1817,7 +904,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(svc) processor.CaptureUpsertChange(slice) - changed, _, _ := processor.Process(context.TODO()) + changed, _ := processor.Process() Expect(changed).To(BeTrue()) }) }) @@ -1835,7 +922,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(svc) processor.CaptureUpsertChange(slice) - changed, _, _ := processor.Process(context.TODO()) + changed, _ := processor.Process() Expect(changed).To(BeTrue()) }) @@ -1851,7 +938,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(svc) processor.CaptureUpsertChange(slice) - changed, _, _ := processor.Process(context.TODO()) + changed, _ := processor.Process() Expect(changed).To(BeFalse()) }) @@ -1868,7 +955,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(gw1) processor.CaptureUpsertChange(hr1) - changed, _, _ := processor.Process(context.TODO()) + changed, _ := processor.Process() Expect(changed).To(BeTrue()) }, ) @@ -1885,7 +972,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(svc) processor.CaptureUpsertChange(slice) - changed, _, _ := processor.Process(context.TODO()) + changed, _ := processor.Process() Expect(changed).To(BeTrue()) }, ) @@ -1903,7 +990,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(gw1Updated) processor.CaptureUpsertChange(hr1Updated) - changed, _, _ := processor.Process(context.TODO()) + changed, _ := processor.Process() Expect(changed).To(BeTrue()) }, ) @@ -2028,20 +1115,9 @@ var _ = Describe("ChangeProcessor", func() { It("should process GatewayClass", func() { processor.CaptureUpsertChange(gc) - expectedConf := dataplane.Configuration{} - expectedStatuses := state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ - ObservedGeneration: gc.Generation, - Conditions: conditions.NewDefaultGatewayClassConditions(), - }, - GatewayStatuses: state.GatewayStatuses{}, - HTTPRouteStatuses: state.HTTPRouteStatuses{}, - } - - changed, conf, statuses := processor.Process(context.TODO()) + changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - assertStatuses(expectedStatuses, statuses) + Expect(graphCfg.GatewayClass).ToNot(BeNil()) Expect(fakeEventRecorder.Events).To(HaveLen(0)) }) @@ -2050,14 +1126,10 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(gwInvalid) processor.CaptureUpsertChange(hrInvalid) - expectedConf := dataplane.Configuration{} - expectedStatuses := state.Statuses{} - - changed, conf, statuses := processor.Process(context.TODO()) + changed, graphCfg := processor.Process() Expect(changed).To(BeFalse()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - assertStatuses(expectedStatuses, statuses) + Expect(graphCfg).To(BeNil()) Expect(fakeEventRecorder.Events).To(HaveLen(2)) assertGwEvent() @@ -2070,73 +1142,12 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(gw) processor.CaptureUpsertChange(hr) - bg := dataplane.BackendGroup{ - Source: types.NamespacedName{Namespace: hr.Namespace, Name: hr.Name}, - RuleIdx: 0, - } - - expectedConf := dataplane.Configuration{ - HTTPServers: []dataplane.VirtualServer{ - { - IsDefault: true, - }, - { - Hostname: "foo.example.com", - PathRules: []dataplane.PathRule{ - { - Path: "/", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: bg, - Source: hr, - }, - }, - }, - }, - }, - }, - SSLServers: []dataplane.VirtualServer{}, - BackendGroups: []dataplane.BackendGroup{bg}, - } - expectedStatuses := state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ - ObservedGeneration: gc.Generation, - Conditions: conditions.NewDefaultGatewayClassConditions(), - }, - GatewayStatuses: state.GatewayStatuses{ - gwNsName: { - Conditions: conditions.NewDefaultGatewayConditions(), - ObservedGeneration: gw.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - }, - }, - }, - HTTPRouteStatuses: state.HTTPRouteStatuses{ - hrNsName: { - ObservedGeneration: hr.Generation, - ParentStatuses: []state.ParentStatus{ - { - GatewayNsName: gwNsName, - SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-80-1"), - Conditions: conditions.NewDefaultRouteConditions(), - }, - }, - }, - }, - } - - changed, conf, statuses := processor.Process(context.TODO()) + changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - assertStatuses(expectedStatuses, statuses) + Expect(graphCfg).ToNot(BeNil()) + Expect(graphCfg.Gateway).ToNot(BeNil()) + Expect(graphCfg.Routes).To(HaveLen(1)) Expect(fakeEventRecorder.Events).To(HaveLen(0)) }) @@ -2146,39 +1157,10 @@ var _ = Describe("ChangeProcessor", func() { It("it should delete the configuration for the old one and not process the new one", func() { processor.CaptureUpsertChange(hrInvalid) - expectedConf := dataplane.Configuration{ - HTTPServers: []dataplane.VirtualServer{ - { - IsDefault: true, - }, - }, - SSLServers: []dataplane.VirtualServer{}, - } - expectedStatuses := state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ - ObservedGeneration: gc.Generation, - Conditions: conditions.NewDefaultGatewayClassConditions(), - }, - GatewayStatuses: state.GatewayStatuses{ - gwNsName: { - Conditions: conditions.NewDefaultGatewayConditions(), - ObservedGeneration: gw.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 0, - Conditions: conditions.NewDefaultListenerConditions(), - }, - }, - }, - }, - HTTPRouteStatuses: state.HTTPRouteStatuses{}, - } - - changed, conf, statuses := processor.Process(context.TODO()) + changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - assertStatuses(expectedStatuses, statuses) + Expect(graphCfg.Routes).To(HaveLen(0)) Expect(fakeEventRecorder.Events).To(HaveLen(1)) assertHREvent() @@ -2189,21 +1171,10 @@ var _ = Describe("ChangeProcessor", func() { It("it should delete the configuration for the old one and not process the new one", func() { processor.CaptureUpsertChange(gwInvalid) - expectedConf := dataplane.Configuration{} - expectedStatuses := state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ - ObservedGeneration: gc.Generation, - Conditions: conditions.NewDefaultGatewayClassConditions(), - }, - GatewayStatuses: state.GatewayStatuses{}, - HTTPRouteStatuses: state.HTTPRouteStatuses{}, - } - - changed, conf, statuses := processor.Process(context.TODO()) + changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - assertStatuses(expectedStatuses, statuses) + Expect(graphCfg.Gateway).To(BeNil()) Expect(fakeEventRecorder.Events).To(HaveLen(1)) assertGwEvent() @@ -2258,14 +1229,10 @@ var _ = Describe("ChangeProcessor", func() { func(hr *v1beta1.HTTPRoute) { processor.CaptureUpsertChange(hr) - expectedConf := dataplane.Configuration{} - expectedStatuses := state.Statuses{} - - changed, conf, statuses := processor.Process(context.Background()) + changed, graphCfg := processor.Process() Expect(changed).To(BeFalse()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - assertStatuses(expectedStatuses, statuses) + Expect(graphCfg).To(BeNil()) assertRejectedEvent() }, @@ -2306,14 +1273,10 @@ var _ = Describe("ChangeProcessor", func() { func(gw *v1beta1.Gateway) { processor.CaptureUpsertChange(gw) - expectedConf := dataplane.Configuration{} - expectedStatuses := state.Statuses{} - - changed, conf, statuses := processor.Process(context.Background()) + changed, graphCfg := processor.Process() Expect(changed).To(BeFalse()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - assertStatuses(expectedStatuses, statuses) + Expect(graphCfg).To(BeNil()) assertRejectedEvent() }, diff --git a/internal/state/conditions/conditions.go b/internal/state/conditions/conditions.go index 73e81370f7..ab11ccfda2 100644 --- a/internal/state/conditions/conditions.go +++ b/internal/state/conditions/conditions.go @@ -23,17 +23,32 @@ const ( // RouteReasonInvalidListener is used with the "Accepted" condition when the Route references an invalid listener. RouteReasonInvalidListener v1beta1.RouteConditionReason = "InvalidListener" + // RouteReasonGatewayNotProgrammed is used when the associated Gateway is not programmed. + // Used with Accepted (false). + RouteReasonGatewayNotProgrammed v1beta1.RouteConditionReason = "GatewayNotProgrammed" + // GatewayReasonGatewayConflict indicates there are multiple Gateway resources to choose from, // and we ignored the resource in question and picked another Gateway as the winner. // This reason is used with GatewayConditionAccepted (false). GatewayReasonGatewayConflict v1beta1.GatewayConditionReason = "GatewayConflict" - // GatewayMessageGatewayConflict is message that describes GatewayReasonGatewayConflict. + // GatewayMessageGatewayConflict is a message that describes GatewayReasonGatewayConflict. GatewayMessageGatewayConflict = "The resource is ignored due to a conflicting Gateway resource" // GatewayReasonUnsupportedValue is used with GatewayConditionAccepted (false) when a value of a field in a Gateway // is invalid or not supported. GatewayReasonUnsupportedValue v1beta1.GatewayConditionReason = "UnsupportedValue" + + // 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" + + // RouteMessageFailedNginxReload is a message used with RouteReasonGatewayNotProgrammed + // when nginx fails to reload. + RouteMessageFailedNginxReload = GatewayMessageFailedNginxReload + ". NGINX may still be configured " + + "for this HTTPRoute. However, future updates to this resource will not be configured until the Gateway " + + "is programmed again" ) // Condition defines a condition to be reported in the status of resources. @@ -212,6 +227,17 @@ func NewRouteNoMatchingParent() Condition { } } +// NewRouteGatewayNotProgrammed returns a Condition that indicates that the Gateway it references is not programmed, +// which does not guarantee that the HTTPRoute has been configured. +func NewRouteGatewayNotProgrammed(msg string) Condition { + return Condition{ + Type: string(v1beta1.RouteConditionAccepted), + Status: metav1.ConditionFalse, + Reason: string(RouteReasonGatewayNotProgrammed), + Message: msg, + } +} + // NewDefaultListenerConditions returns the default Conditions that must be present in the status of a Listener. func NewDefaultListenerConditions() []Condition { return []Condition{ @@ -344,6 +370,7 @@ func NewGatewayClassInvalidParameters(msg string) Condition { func NewDefaultGatewayConditions() []Condition { return []Condition{ NewGatewayAccepted(), + NewGatewayProgrammed(), } } @@ -358,12 +385,15 @@ func NewGatewayAccepted() Condition { } // NewGatewayConflict returns a Condition that indicates the Gateway has a conflict with another Gateway. -func NewGatewayConflict() Condition { - return Condition{ - Type: string(v1beta1.GatewayConditionAccepted), - Status: metav1.ConditionFalse, - Reason: string(GatewayReasonGatewayConflict), - Message: GatewayMessageGatewayConflict, +func NewGatewayConflict() []Condition { + return []Condition{ + { + Type: string(v1beta1.GatewayConditionAccepted), + Status: metav1.ConditionFalse, + Reason: string(GatewayReasonGatewayConflict), + Message: GatewayMessageGatewayConflict, + }, + NewGatewayConflictNotProgrammed(), } } @@ -380,33 +410,80 @@ func NewGatewayAcceptedListenersNotValid() Condition { // NewGatewayNotAcceptedListenersNotValid returns a Condition that indicates the Gateway is not accepted, // because all listeners are invalid. -func NewGatewayNotAcceptedListenersNotValid() Condition { +func NewGatewayNotAcceptedListenersNotValid() []Condition { + msg := "Gateway has no valid listeners" + return []Condition{ + { + Type: string(v1beta1.GatewayConditionAccepted), + Status: metav1.ConditionFalse, + Reason: string(v1beta1.GatewayReasonListenersNotValid), + Message: msg, + }, + NewGatewayNotProgrammedInvalid(msg), + } +} + +// NewGatewayInvalid returns a Condition that indicates the Gateway is not accepted and programmed because it is +// semantically or syntactically invalid. The provided message contains the details of why the Gateway is invalid. +func NewGatewayInvalid(msg string) []Condition { + return []Condition{ + { + Type: string(v1beta1.GatewayConditionAccepted), + Status: metav1.ConditionFalse, + Reason: string(v1beta1.GatewayReasonInvalid), + Message: msg, + }, + NewGatewayNotProgrammedInvalid(msg), + } +} + +// NewGatewayUnsupportedValue returns a Condition that indicates that a field of the Gateway has an unsupported value. +// Unsupported means that the value is not supported by the implementation or invalid. +func NewGatewayUnsupportedValue(msg string) []Condition { + return []Condition{ + { + Type: string(v1beta1.GatewayConditionAccepted), + Status: metav1.ConditionFalse, + Reason: string(GatewayReasonUnsupportedValue), + Message: msg, + }, + { + Type: string(v1beta1.GatewayConditionProgrammed), + Status: metav1.ConditionFalse, + Reason: string(GatewayReasonUnsupportedValue), + Message: msg, + }, + } +} + +// NewGatewayProgrammed returns a Condition that indicates the Gateway is programmed. +func NewGatewayProgrammed() Condition { return Condition{ - Type: string(v1beta1.GatewayConditionAccepted), - Status: metav1.ConditionFalse, - Reason: string(v1beta1.GatewayReasonListenersNotValid), - Message: "Gateway has no valid listeners", + Type: string(v1beta1.GatewayConditionProgrammed), + Status: metav1.ConditionTrue, + Reason: string(v1beta1.GatewayReasonProgrammed), + Message: "Gateway is programmed", } } -// NewGatewayInvalid returns a Condition that indicates the Gateway is not accepted because it is +// NewGatewayInvalid returns a Condition that indicates the Gateway is not programmed because it is // semantically or syntactically invalid. The provided message contains the details of why the Gateway is invalid. -func NewGatewayInvalid(msg string) Condition { +func NewGatewayNotProgrammedInvalid(msg string) Condition { return Condition{ - Type: string(v1beta1.GatewayConditionAccepted), + Type: string(v1beta1.GatewayConditionProgrammed), Status: metav1.ConditionFalse, Reason: string(v1beta1.GatewayReasonInvalid), Message: msg, } } -// NewGatewayUnsupportedValue returns a Condition that indicates that a field of the Gateway has an unsupported value. -// Unsupported means that the value is not supported by the implementation or invalid. -func NewGatewayUnsupportedValue(msg string) Condition { +// NewGatewayConflictNotProgrammed returns a custom Programmed Condition that indicates the Gateway has a +// conflict with another Gateway. +func NewGatewayConflictNotProgrammed() Condition { return Condition{ - Type: string(v1beta1.GatewayConditionAccepted), + Type: string(v1beta1.GatewayConditionProgrammed), Status: metav1.ConditionFalse, - Reason: string(GatewayReasonUnsupportedValue), - Message: msg, + Reason: string(GatewayReasonGatewayConflict), + Message: GatewayMessageGatewayConflict, } } diff --git a/internal/state/graph/gateway.go b/internal/state/graph/gateway.go index e83cc0c693..07675b1605 100644 --- a/internal/state/graph/gateway.go +++ b/internal/state/graph/gateway.go @@ -116,16 +116,16 @@ func validateGateway(gw *v1beta1.Gateway, gc *GatewayClass) []conditions.Conditi var conds []conditions.Condition if gc == nil { - conds = append(conds, conditions.NewGatewayInvalid("GatewayClass doesn't exist")) + conds = append(conds, conditions.NewGatewayInvalid("GatewayClass doesn't exist")...) } else if !gc.Valid { - conds = append(conds, conditions.NewGatewayInvalid("GatewayClass is invalid")) + conds = append(conds, conditions.NewGatewayInvalid("GatewayClass is invalid")...) } if len(gw.Spec.Addresses) > 0 { path := field.NewPath("spec", "addresses") valErr := field.Forbidden(path, "addresses are not supported") - conds = append(conds, conditions.NewGatewayUnsupportedValue(valErr.Error())) + conds = append(conds, conditions.NewGatewayUnsupportedValue(valErr.Error())...) } return conds diff --git a/internal/state/graph/gateway_test.go b/internal/state/graph/gateway_test.go index c1cc34a6c0..727ff4627f 100644 --- a/internal/state/graph/gateway_test.go +++ b/internal/state/graph/gateway_test.go @@ -511,11 +511,9 @@ func TestBuildGateway(t *testing.T) { }), gatewayClass: validGC, expected: &Gateway{ - Source: getLastCreatedGetaway(), - Valid: false, - Conditions: []conditions.Condition{ - conditions.NewGatewayUnsupportedValue("spec.addresses: Forbidden: addresses are not supported"), - }, + Source: getLastCreatedGetaway(), + Valid: false, + Conditions: conditions.NewGatewayUnsupportedValue("spec.addresses: Forbidden: addresses are not supported"), }, name: "gateway addresses are not supported", }, @@ -528,11 +526,9 @@ func TestBuildGateway(t *testing.T) { gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener801, listener802}}), gatewayClass: invalidGC, expected: &Gateway{ - Source: getLastCreatedGetaway(), - Valid: false, - Conditions: []conditions.Condition{ - conditions.NewGatewayInvalid("GatewayClass is invalid"), - }, + Source: getLastCreatedGetaway(), + Valid: false, + Conditions: conditions.NewGatewayInvalid("GatewayClass is invalid"), }, name: "invalid gatewayclass", }, @@ -540,11 +536,9 @@ func TestBuildGateway(t *testing.T) { gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener801, listener802}}), gatewayClass: nil, expected: &Gateway{ - Source: getLastCreatedGetaway(), - Valid: false, - Conditions: []conditions.Condition{ - conditions.NewGatewayInvalid("GatewayClass doesn't exist"), - }, + Source: getLastCreatedGetaway(), + Valid: false, + Conditions: conditions.NewGatewayInvalid("GatewayClass doesn't exist"), }, name: "nil gatewayclass", }, diff --git a/internal/state/statefakes/fake_change_processor.go b/internal/state/statefakes/fake_change_processor.go index af7a55b62f..43f7aa3a6e 100644 --- a/internal/state/statefakes/fake_change_processor.go +++ b/internal/state/statefakes/fake_change_processor.go @@ -2,11 +2,10 @@ package statefakes import ( - "context" "sync" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/dataplane" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/graph" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -23,20 +22,17 @@ type FakeChangeProcessor struct { captureUpsertChangeArgsForCall []struct { arg1 client.Object } - ProcessStub func(context.Context) (bool, dataplane.Configuration, state.Statuses) + ProcessStub func() (bool, *graph.Graph) processMutex sync.RWMutex processArgsForCall []struct { - arg1 context.Context } processReturns struct { result1 bool - result2 dataplane.Configuration - result3 state.Statuses + result2 *graph.Graph } processReturnsOnCall map[int]struct { result1 bool - result2 dataplane.Configuration - result3 state.Statuses + result2 *graph.Graph } invocations map[string][][]interface{} invocationsMutex sync.RWMutex @@ -107,23 +103,22 @@ func (fake *FakeChangeProcessor) CaptureUpsertChangeArgsForCall(i int) client.Ob return argsForCall.arg1 } -func (fake *FakeChangeProcessor) Process(arg1 context.Context) (bool, dataplane.Configuration, state.Statuses) { +func (fake *FakeChangeProcessor) Process() (bool, *graph.Graph) { fake.processMutex.Lock() ret, specificReturn := fake.processReturnsOnCall[len(fake.processArgsForCall)] fake.processArgsForCall = append(fake.processArgsForCall, struct { - arg1 context.Context - }{arg1}) + }{}) stub := fake.ProcessStub fakeReturns := fake.processReturns - fake.recordInvocation("Process", []interface{}{arg1}) + fake.recordInvocation("Process", []interface{}{}) fake.processMutex.Unlock() if stub != nil { - return stub(arg1) + return stub() } if specificReturn { - return ret.result1, ret.result2, ret.result3 + return ret.result1, ret.result2 } - return fakeReturns.result1, fakeReturns.result2, fakeReturns.result3 + return fakeReturns.result1, fakeReturns.result2 } func (fake *FakeChangeProcessor) ProcessCallCount() int { @@ -132,46 +127,36 @@ func (fake *FakeChangeProcessor) ProcessCallCount() int { return len(fake.processArgsForCall) } -func (fake *FakeChangeProcessor) ProcessCalls(stub func(context.Context) (bool, dataplane.Configuration, state.Statuses)) { +func (fake *FakeChangeProcessor) ProcessCalls(stub func() (bool, *graph.Graph)) { fake.processMutex.Lock() defer fake.processMutex.Unlock() fake.ProcessStub = stub } -func (fake *FakeChangeProcessor) ProcessArgsForCall(i int) context.Context { - fake.processMutex.RLock() - defer fake.processMutex.RUnlock() - argsForCall := fake.processArgsForCall[i] - return argsForCall.arg1 -} - -func (fake *FakeChangeProcessor) ProcessReturns(result1 bool, result2 dataplane.Configuration, result3 state.Statuses) { +func (fake *FakeChangeProcessor) ProcessReturns(result1 bool, result2 *graph.Graph) { fake.processMutex.Lock() defer fake.processMutex.Unlock() fake.ProcessStub = nil fake.processReturns = struct { result1 bool - result2 dataplane.Configuration - result3 state.Statuses - }{result1, result2, result3} + result2 *graph.Graph + }{result1, result2} } -func (fake *FakeChangeProcessor) ProcessReturnsOnCall(i int, result1 bool, result2 dataplane.Configuration, result3 state.Statuses) { +func (fake *FakeChangeProcessor) ProcessReturnsOnCall(i int, result1 bool, result2 *graph.Graph) { fake.processMutex.Lock() defer fake.processMutex.Unlock() fake.ProcessStub = nil if fake.processReturnsOnCall == nil { fake.processReturnsOnCall = make(map[int]struct { result1 bool - result2 dataplane.Configuration - result3 state.Statuses + result2 *graph.Graph }) } fake.processReturnsOnCall[i] = struct { result1 bool - result2 dataplane.Configuration - result3 state.Statuses - }{result1, result2, result3} + result2 *graph.Graph + }{result1, result2} } func (fake *FakeChangeProcessor) Invocations() map[string][][]interface{} { diff --git a/internal/status/gateway.go b/internal/status/gateway.go index abb37e3098..aeaf13576e 100644 --- a/internal/status/gateway.go +++ b/internal/status/gateway.go @@ -5,13 +5,11 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/gateway-api/apis/v1beta1" - - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" ) // prepareGatewayStatus prepares the status for a Gateway resource. func prepareGatewayStatus( - gatewayStatus state.GatewayStatus, + gatewayStatus GatewayStatus, podIP string, transitionTime metav1.Time, ) v1beta1.GatewayStatus { diff --git a/internal/status/gateway_test.go b/internal/status/gateway_test.go index 5be4f53f5e..96df92a89f 100644 --- a/internal/status/gateway_test.go +++ b/internal/status/gateway_test.go @@ -9,7 +9,6 @@ import ( "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" ) func TestPrepareGatewayStatus(t *testing.T) { @@ -19,9 +18,9 @@ func TestPrepareGatewayStatus(t *testing.T) { Value: "1.2.3.4", } - status := state.GatewayStatus{ + status := GatewayStatus{ Conditions: CreateTestConditions("GatewayTest"), - ListenerStatuses: state.ListenerStatuses{ + ListenerStatuses: ListenerStatuses{ "listener": { AttachedRoutes: 3, Conditions: CreateTestConditions("ListenerTest"), diff --git a/internal/status/gatewayclass.go b/internal/status/gatewayclass.go index ce2f0a1e83..3cccf3414b 100644 --- a/internal/status/gatewayclass.go +++ b/internal/status/gatewayclass.go @@ -3,12 +3,10 @@ package status import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/gateway-api/apis/v1beta1" - - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" ) // prepareGatewayClassStatus prepares the status for the GatewayClass resource. -func prepareGatewayClassStatus(status state.GatewayClassStatus, transitionTime metav1.Time) v1beta1.GatewayClassStatus { +func prepareGatewayClassStatus(status GatewayClassStatus, transitionTime metav1.Time) v1beta1.GatewayClassStatus { return v1beta1.GatewayClassStatus{ Conditions: convertConditions(status.Conditions, status.ObservedGeneration, transitionTime), } diff --git a/internal/status/gatewayclass_test.go b/internal/status/gatewayclass_test.go index d208f22351..c49ce2d747 100644 --- a/internal/status/gatewayclass_test.go +++ b/internal/status/gatewayclass_test.go @@ -9,13 +9,12 @@ import ( "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" ) func TestPrepareGatewayClassStatus(t *testing.T) { transitionTime := metav1.NewTime(time.Now()) - status := state.GatewayClassStatus{ + status := GatewayClassStatus{ ObservedGeneration: 1, Conditions: CreateTestConditions("Test"), } diff --git a/internal/status/httproute.go b/internal/status/httproute.go index f0e6a63c52..2d235a25b0 100644 --- a/internal/status/httproute.go +++ b/internal/status/httproute.go @@ -3,13 +3,11 @@ package status import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/gateway-api/apis/v1beta1" - - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" ) // prepareHTTPRouteStatus prepares the status for an HTTPRoute resource. func prepareHTTPRouteStatus( - status state.HTTPRouteStatus, + status HTTPRouteStatus, gatewayCtlrName string, transitionTime metav1.Time, ) v1beta1.HTTPRouteStatus { diff --git a/internal/status/httproute_test.go b/internal/status/httproute_test.go index a56451c87f..e3bcf8b5d2 100644 --- a/internal/status/httproute_test.go +++ b/internal/status/httproute_test.go @@ -10,16 +10,15 @@ import ( "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" ) func TestPrepareHTTPRouteStatus(t *testing.T) { gwNsName1 := types.NamespacedName{Namespace: "test", Name: "gateway-1"} gwNsName2 := types.NamespacedName{Namespace: "test", Name: "gateway-2"} - status := state.HTTPRouteStatus{ + status := HTTPRouteStatus{ ObservedGeneration: 1, - ParentStatuses: []state.ParentStatus{ + ParentStatuses: []ParentStatus{ { GatewayNsName: gwNsName1, SectionName: helpers.GetPointer[v1beta1.SectionName]("http"), diff --git a/internal/state/statuses.go b/internal/status/statuses.go similarity index 88% rename from internal/state/statuses.go rename to internal/status/statuses.go index e1f9b78d88..e624367c9b 100644 --- a/internal/state/statuses.go +++ b/internal/status/statuses.go @@ -1,4 +1,4 @@ -package state +package status import ( "k8s.io/apimachinery/pkg/types" @@ -67,8 +67,12 @@ type GatewayClassStatus struct { ObservedGeneration int64 } -// buildStatuses builds statuses from a Graph. -func buildStatuses(graph *graph.Graph) Statuses { +type NginxReloadResult struct { + Error error +} + +// BuildStatuses builds statuses from a Graph. +func BuildStatuses(graph *graph.Graph, nginxReloadRes NginxReloadResult) Statuses { statuses := Statuses{ HTTPRouteStatuses: make(HTTPRouteStatuses), } @@ -89,7 +93,7 @@ func buildStatuses(graph *graph.Graph) Statuses { } } - statuses.GatewayStatuses = buildGatewayStatuses(graph.Gateway, graph.IgnoredGateways) + statuses.GatewayStatuses = buildGatewayStatuses(graph.Gateway, graph.IgnoredGateways, nginxReloadRes) for nsname, r := range graph.Routes { parentStatuses := make([]ParentStatus, 0, len(r.ParentRefs)) @@ -111,6 +115,10 @@ func buildStatuses(graph *graph.Graph) Statuses { allConds = append(allConds, ref.Attachment.FailedCondition) } + if nginxReloadRes.Error != nil { + allConds = append(allConds, conditions.NewRouteGatewayNotProgrammed(conditions.RouteMessageFailedNginxReload)) + } + routeRef := r.Source.Spec.ParentRefs[ref.Idx] parentStatuses = append(parentStatuses, ParentStatus{ @@ -132,16 +140,17 @@ func buildStatuses(graph *graph.Graph) Statuses { func buildGatewayStatuses( gateway *graph.Gateway, ignoredGateways map[types.NamespacedName]*v1beta1.Gateway, + nginxReloadRes NginxReloadResult, ) GatewayStatuses { statuses := make(GatewayStatuses) if gateway != nil { - statuses[client.ObjectKeyFromObject(gateway.Source)] = buildGatewayStatus(gateway) + statuses[client.ObjectKeyFromObject(gateway.Source)] = buildGatewayStatus(gateway, nginxReloadRes) } for nsname, gw := range ignoredGateways { statuses[nsname] = GatewayStatus{ - Conditions: []conditions.Condition{conditions.NewGatewayConflict()}, + Conditions: conditions.NewGatewayConflict(), ObservedGeneration: gw.Generation, } } @@ -149,7 +158,7 @@ func buildGatewayStatuses( return statuses } -func buildGatewayStatus(gateway *graph.Gateway) GatewayStatus { +func buildGatewayStatus(gateway *graph.Gateway, nginxReloadRes NginxReloadResult) GatewayStatus { if !gateway.Valid { return GatewayStatus{ Conditions: conditions.DeduplicateConditions(gateway.Conditions), @@ -178,11 +187,15 @@ func buildGatewayStatus(gateway *graph.Gateway) GatewayStatus { gwConds := conditions.NewDefaultGatewayConditions() if validListenerCount == 0 { - gwConds = append(gwConds, conditions.NewGatewayNotAcceptedListenersNotValid()) + gwConds = append(gwConds, conditions.NewGatewayNotAcceptedListenersNotValid()...) } else if validListenerCount < len(gateway.Listeners) { gwConds = append(gwConds, conditions.NewGatewayAcceptedListenersNotValid()) } + if nginxReloadRes.Error != nil { + gwConds = append(gwConds, conditions.NewGatewayNotProgrammedInvalid(conditions.GatewayMessageFailedNginxReload)) + } + return GatewayStatus{ Conditions: conditions.DeduplicateConditions(gwConds), ListenerStatuses: listenerStatuses, diff --git a/internal/state/statuses_test.go b/internal/status/statuses_test.go similarity index 70% rename from internal/state/statuses_test.go rename to internal/status/statuses_test.go index aff1dd70eb..f15edbe582 100644 --- a/internal/state/statuses_test.go +++ b/internal/status/statuses_test.go @@ -1,6 +1,7 @@ -package state +package status import ( + "errors" "testing" . "github.com/onsi/gomega" @@ -149,7 +150,7 @@ func TestBuildStatuses(t *testing.T) { ObservedGeneration: 2, }, {Namespace: "test", Name: "ignored-gateway"}: { - Conditions: []conditions.Condition{conditions.NewGatewayConflict()}, + Conditions: conditions.NewGatewayConflict(), ObservedGeneration: 1, }, }, @@ -190,12 +191,100 @@ func TestBuildStatuses(t *testing.T) { g := NewGomegaWithT(t) - result := buildStatuses(graph) + var nginxReloadRes NginxReloadResult + result := BuildStatuses(graph, nginxReloadRes) + g.Expect(helpers.Diff(expected, result)).To(BeEmpty()) +} + +func TestBuildStatusesNginxErr(t *testing.T) { + routes := map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "hr-valid"}: { + Valid: true, + Source: &v1beta1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 3, + }, + Spec: v1beta1.HTTPRouteSpec{ + CommonRouteSpec: v1beta1.CommonRouteSpec{ + ParentRefs: []v1beta1.ParentReference{ + { + SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-80-1"), + }, + }, + }, + }, + }, + ParentRefs: []graph.ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + Attachment: &graph.ParentRefAttachmentStatus{ + Attached: true, + }, + }, + }, + }, + } + + graph := &graph.Graph{ + Gateway: &graph.Gateway{ + Source: gw, + Listeners: map[string]*graph.Listener{ + "listener-80-1": { + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "hr-1"}: {}, + }, + }, + }, + Valid: true, + }, + Routes: routes, + } + + expected := Statuses{ + GatewayStatuses: GatewayStatuses{ + {Namespace: "test", Name: "gateway"}: { + Conditions: []conditions.Condition{ + conditions.NewGatewayAccepted(), + conditions.NewGatewayNotProgrammedInvalid(conditions.GatewayMessageFailedNginxReload), + }, + ListenerStatuses: map[string]ListenerStatus{ + "listener-80-1": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, + }, + ObservedGeneration: 2, + }, + }, + HTTPRouteStatuses: HTTPRouteStatuses{ + {Namespace: "test", Name: "hr-valid"}: { + ObservedGeneration: 3, + ParentStatuses: []ParentStatus{ + { + GatewayNsName: client.ObjectKeyFromObject(gw), + SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-80-1"), + Conditions: []conditions.Condition{ + conditions.NewRouteResolvedRefs(), + conditions.NewRouteGatewayNotProgrammed(conditions.RouteMessageFailedNginxReload), + }, + }, + }, + }, + }, + } + + g := NewGomegaWithT(t) + + nginxReloadRes := NginxReloadResult{Error: errors.New("test error")} + result := BuildStatuses(graph, nginxReloadRes) g.Expect(helpers.Diff(expected, result)).To(BeEmpty()) } func TestBuildGatewayStatuses(t *testing.T) { tests := []struct { + nginxReloadRes NginxReloadResult gateway *graph.Gateway ignoredGateways map[types.NamespacedName]*v1beta1.Gateway expected GatewayStatuses @@ -221,11 +310,11 @@ func TestBuildGatewayStatuses(t *testing.T) { }, expected: GatewayStatuses{ {Namespace: "test", Name: "ignored-1"}: { - Conditions: []conditions.Condition{conditions.NewGatewayConflict()}, + Conditions: conditions.NewGatewayConflict(), ObservedGeneration: 1, }, {Namespace: "test", Name: "ignored-2"}: { - Conditions: []conditions.Condition{conditions.NewGatewayConflict()}, + Conditions: conditions.NewGatewayConflict(), ObservedGeneration: 2, }, }, @@ -289,7 +378,10 @@ func TestBuildGatewayStatuses(t *testing.T) { }, expected: GatewayStatuses{ {Namespace: "test", Name: "gateway"}: { - Conditions: []conditions.Condition{conditions.NewGatewayAcceptedListenersNotValid()}, + Conditions: []conditions.Condition{ + conditions.NewGatewayProgrammed(), + conditions.NewGatewayAcceptedListenersNotValid(), + }, ListenerStatuses: map[string]ListenerStatus{ "listener-valid": { AttachedRoutes: 1, @@ -327,7 +419,7 @@ func TestBuildGatewayStatuses(t *testing.T) { }, expected: GatewayStatuses{ {Namespace: "test", Name: "gateway"}: { - Conditions: []conditions.Condition{conditions.NewGatewayNotAcceptedListenersNotValid()}, + Conditions: conditions.NewGatewayNotAcceptedListenersNotValid(), ListenerStatuses: map[string]ListenerStatus{ "listener-invalid-1": { Conditions: []conditions.Condition{ @@ -349,14 +441,46 @@ func TestBuildGatewayStatuses(t *testing.T) { gateway: &graph.Gateway{ Source: gw, Valid: false, - Conditions: []conditions.Condition{conditions.NewGatewayInvalid("no gateway class")}, + Conditions: conditions.NewGatewayInvalid("no gateway class"), }, expected: GatewayStatuses{ {Namespace: "test", Name: "gateway"}: { - Conditions: []conditions.Condition{conditions.NewGatewayInvalid("no gateway class")}, + Conditions: conditions.NewGatewayInvalid("no gateway class"), + ObservedGeneration: 2, + }, + }, + }, + { + name: "error reloading nginx; gateway not programmed", + gateway: &graph.Gateway{ + Source: gw, + Valid: true, + Conditions: conditions.NewDefaultGatewayConditions(), + Listeners: map[string]*graph.Listener{ + "listener-valid": { + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "hr-1"}: {}, + }, + }, + }, + }, + expected: GatewayStatuses{ + {Namespace: "test", Name: "gateway"}: { + Conditions: []conditions.Condition{ + conditions.NewGatewayAccepted(), + conditions.NewGatewayNotProgrammedInvalid(conditions.GatewayMessageFailedNginxReload), + }, + ListenerStatuses: map[string]ListenerStatus{ + "listener-valid": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, + }, ObservedGeneration: 2, }, }, + nginxReloadRes: NginxReloadResult{Error: errors.New("test error")}, }, } @@ -364,7 +488,7 @@ func TestBuildGatewayStatuses(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewGomegaWithT(t) - result := buildGatewayStatuses(test.gateway, test.ignoredGateways) + result := buildGatewayStatuses(test.gateway, test.ignoredGateways, test.nginxReloadRes) g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) }) } diff --git a/internal/status/statusfakes/fake_updater.go b/internal/status/statusfakes/fake_updater.go index 6f611eb919..206db177db 100644 --- a/internal/status/statusfakes/fake_updater.go +++ b/internal/status/statusfakes/fake_updater.go @@ -5,26 +5,25 @@ import ( "context" "sync" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" "github.com/nginxinc/nginx-kubernetes-gateway/internal/status" ) type FakeUpdater struct { - UpdateStub func(context.Context, state.Statuses) + UpdateStub func(context.Context, status.Statuses) updateMutex sync.RWMutex updateArgsForCall []struct { arg1 context.Context - arg2 state.Statuses + arg2 status.Statuses } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } -func (fake *FakeUpdater) Update(arg1 context.Context, arg2 state.Statuses) { +func (fake *FakeUpdater) Update(arg1 context.Context, arg2 status.Statuses) { fake.updateMutex.Lock() fake.updateArgsForCall = append(fake.updateArgsForCall, struct { arg1 context.Context - arg2 state.Statuses + arg2 status.Statuses }{arg1, arg2}) stub := fake.UpdateStub fake.recordInvocation("Update", []interface{}{arg1, arg2}) @@ -40,13 +39,13 @@ func (fake *FakeUpdater) UpdateCallCount() int { return len(fake.updateArgsForCall) } -func (fake *FakeUpdater) UpdateCalls(stub func(context.Context, state.Statuses)) { +func (fake *FakeUpdater) UpdateCalls(stub func(context.Context, status.Statuses)) { fake.updateMutex.Lock() defer fake.updateMutex.Unlock() fake.UpdateStub = stub } -func (fake *FakeUpdater) UpdateArgsForCall(i int) (context.Context, state.Statuses) { +func (fake *FakeUpdater) UpdateArgsForCall(i int) (context.Context, status.Statuses) { fake.updateMutex.RLock() defer fake.updateMutex.RUnlock() argsForCall := fake.updateArgsForCall[i] diff --git a/internal/status/updater.go b/internal/status/updater.go index a5323df070..16aed554ca 100644 --- a/internal/status/updater.go +++ b/internal/status/updater.go @@ -8,8 +8,6 @@ import ( "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" - - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" ) //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . Updater @@ -17,7 +15,7 @@ import ( // Updater updates statuses of the Gateway API resources. type Updater interface { // Update updates the statuses of the resources. - Update(context.Context, state.Statuses) + Update(context.Context, Statuses) } // UpdaterConfig holds configuration parameters for Updater. @@ -81,7 +79,7 @@ func NewUpdater(cfg UpdaterConfig) Updater { } } -func (upd *updaterImpl) Update(ctx context.Context, statuses state.Statuses) { +func (upd *updaterImpl) Update(ctx context.Context, statuses Statuses) { // FIXME(pleshakov) Merge the new Conditions in the status with the existing Conditions // https://github.com/nginxinc/nginx-kubernetes-gateway/issues/558 diff --git a/internal/status/updater_test.go b/internal/status/updater_test.go index 419123a570..43e48e28bc 100644 --- a/internal/status/updater_test.go +++ b/internal/status/updater_test.go @@ -16,7 +16,6 @@ import ( gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions" "github.com/nginxinc/nginx-kubernetes-gateway/internal/status" "github.com/nginxinc/nginx-kubernetes-gateway/internal/status/statusfakes" @@ -73,16 +72,16 @@ var _ = Describe("Updater", func() { Value: "1.2.3.4", } - createStatuses = func(gens generations) state.Statuses { - return state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ + createStatuses = func(gens generations) status.Statuses { + return status.Statuses{ + GatewayClassStatus: &status.GatewayClassStatus{ ObservedGeneration: gens.gatewayClass, Conditions: status.CreateTestConditions("Test"), }, - GatewayStatuses: state.GatewayStatuses{ + GatewayStatuses: status.GatewayStatuses{ {Namespace: "test", Name: "gateway"}: { Conditions: status.CreateTestConditions("Test"), - ListenerStatuses: map[string]state.ListenerStatus{ + ListenerStatuses: map[string]status.ListenerStatus{ "http": { AttachedRoutes: 1, Conditions: status.CreateTestConditions("Test"), @@ -91,14 +90,14 @@ var _ = Describe("Updater", func() { ObservedGeneration: gens.gateways, }, {Namespace: "test", Name: "ignored-gateway"}: { - Conditions: []conditions.Condition{conditions.NewGatewayConflict()}, + Conditions: conditions.NewGatewayConflict(), ObservedGeneration: 1, }, }, - HTTPRouteStatuses: state.HTTPRouteStatuses{ + HTTPRouteStatuses: status.HTTPRouteStatuses{ {Namespace: "test", Name: "route1"}: { ObservedGeneration: 5, - ParentStatuses: []state.ParentStatus{ + ParentStatuses: []status.ParentStatus{ { GatewayNsName: types.NamespacedName{Namespace: "test", Name: "gateway"}, SectionName: helpers.GetPointer[v1beta1.SectionName]("http"), @@ -174,6 +173,14 @@ var _ = Describe("Updater", func() { Reason: string(conditions.GatewayReasonGatewayConflict), Message: conditions.GatewayMessageGatewayConflict, }, + { + Type: string(v1beta1.GatewayConditionProgrammed), + Status: metav1.ConditionFalse, + ObservedGeneration: 1, + LastTransitionTime: fakeClockTime, + Reason: string(conditions.GatewayReasonGatewayConflict), + Message: conditions.GatewayMessageGatewayConflict, + }, }, Addresses: []v1beta1.GatewayAddress{addr}, }, @@ -436,8 +443,8 @@ var _ = Describe("Updater", func() { It("should not update GatewayClass status", func() { updater.Update( context.Background(), - state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ + status.Statuses{ + GatewayClassStatus: &status.GatewayClassStatus{ ObservedGeneration: 1, Conditions: status.CreateTestConditions("Test"), }, From 5934655c5b1e6077e59e2b040567228b2397dc6d Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Wed, 31 May 2023 08:58:54 -0600 Subject: [PATCH 2/4] Update tests; code review comments --- internal/events/handler.go | 6 +- internal/state/change_processor.go | 2 +- internal/state/change_processor_test.go | 120 +++++++++++++++++++----- 3 files changed, 103 insertions(+), 25 deletions(-) diff --git a/internal/events/handler.go b/internal/events/handler.go index 3477a3adec..56eb1e25fd 100644 --- a/internal/events/handler.go +++ b/internal/events/handler.go @@ -77,14 +77,14 @@ func (h *EventHandlerImpl) HandleEventBatch(ctx context.Context, batch EventBatc } } - changed, graphCfg := h.cfg.Processor.Process() + changed, graph := h.cfg.Processor.Process() if !changed { h.cfg.Logger.Info("Handling events didn't result into NGINX configuration changes") return } var nginxReloadRes status.NginxReloadResult - err := h.updateNginx(ctx, dataplane.BuildConfiguration(ctx, graphCfg, h.cfg.ServiceResolver)) + err := h.updateNginx(ctx, dataplane.BuildConfiguration(ctx, graph, h.cfg.ServiceResolver)) if err != nil { h.cfg.Logger.Error(err, "Failed to update NGINX configuration") nginxReloadRes.Error = err @@ -92,7 +92,7 @@ func (h *EventHandlerImpl) HandleEventBatch(ctx context.Context, batch EventBatc h.cfg.Logger.Info("NGINX configuration was successfully updated") } - h.cfg.StatusUpdater.Update(ctx, status.BuildStatuses(graphCfg, nginxReloadRes)) + h.cfg.StatusUpdater.Update(ctx, status.BuildStatuses(graph, nginxReloadRes)) } func (h *EventHandlerImpl) updateNginx(ctx context.Context, conf dataplane.Configuration) error { diff --git a/internal/state/change_processor.go b/internal/state/change_processor.go index f6fda13434..f334e1ecaa 100644 --- a/internal/state/change_processor.go +++ b/internal/state/change_processor.go @@ -43,7 +43,7 @@ type ChangeProcessor interface { // The method panics if the resource is of unsupported type or if the passed Gateway is different from the one // this ChangeProcessor was created for. CaptureDeleteChange(resourceType client.Object, nsname types.NamespacedName) - // Process produces an internal representation of the Gateway configuration. + // Process produces a Graph-like representation of GatewayAPI resources. // If no changes were captured, the changed return argument will be false and graph will be empty. Process() (changed bool, graphCfg *graph.Graph) } diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go index c8fee94da6..a20019c0a9 100644 --- a/internal/state/change_processor_test.go +++ b/internal/state/change_processor_test.go @@ -18,7 +18,6 @@ import ( "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" "github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/index" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/graph" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/relationship" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/relationship/relationshipfakes" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets/secretsfakes" @@ -221,9 +220,9 @@ var _ = Describe("ChangeProcessor", func() { Describe("Process gateway resources", Ordered, func() { var ( - gcUpdated *v1beta1.GatewayClass - hr1, hr1Updated *v1beta1.HTTPRoute - gw1, gw1Updated *v1beta1.Gateway + gcUpdated *v1beta1.GatewayClass + hr1, hr1Updated, hr2 *v1beta1.HTTPRoute + gw1, gw1Updated, gw2 *v1beta1.Gateway ) BeforeAll(func() { gcUpdated = gc.DeepCopy() @@ -234,10 +233,14 @@ var _ = Describe("ChangeProcessor", func() { hr1Updated = hr1.DeepCopy() hr1Updated.Generation++ + hr2 = createRoute("hr-2", "gateway-2", "bar.example.com") + gw1 = createGatewayWithTLSListener("gateway-1") gw1Updated = gw1.DeepCopy() gw1Updated.Generation++ + + gw2 = createGatewayWithTLSListener("gateway-2") }) When("no upsert has occurred", func() { @@ -247,15 +250,36 @@ var _ = Describe("ChangeProcessor", func() { Expect(graphCfg).To(BeNil()) }) }) - When("upsert has occurred", func() { - It("returns populated graph", func() { - processor.CaptureUpsertChange(hr1) - processor.CaptureUpsertChange(gw1) + When("GatewayClass doesn't exist", func() { + When("Gateways don't exist", func() { + When("the first HTTPRoute is upserted", func() { + It("returns empty graph", func() { + processor.CaptureUpsertChange(hr1) + + changed, graphCfg := processor.Process() + Expect(changed).To(BeTrue()) + Expect(graphCfg.Routes).To(BeNil()) + }) + }) + When("the first Gateway is upserted", func() { + It("returns populated graph", func() { + processor.CaptureUpsertChange(gw1) + + changed, graphCfg := processor.Process() + Expect(changed).To(BeTrue()) + Expect(graphCfg.Gateway).ToNot(BeNil()) + Expect(graphCfg.Routes).To(HaveLen(1)) + }) + }) + }) + }) + When("the GatewayClass is upserted", func() { + It("returns updated graph", func() { processor.CaptureUpsertChange(gc) changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(graphCfg).ToNot(BeNil()) + Expect(graphCfg.GatewayClass).ToNot(BeNil()) }) }) When("the first HTTPRoute without a generation changed is processed", func() { @@ -275,7 +299,7 @@ var _ = Describe("ChangeProcessor", func() { changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(graphCfg).ToNot(BeNil()) + Expect(graphCfg.Routes).To(HaveLen(1)) }, ) }) @@ -296,7 +320,7 @@ var _ = Describe("ChangeProcessor", func() { changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(graphCfg).ToNot(BeNil()) + Expect(graphCfg.Gateway).ToNot(BeNil()) }) }) When("the GatewayClass update without generation change is processed", func() { @@ -316,7 +340,7 @@ var _ = Describe("ChangeProcessor", func() { changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(graphCfg).ToNot(BeNil()) + Expect(graphCfg.GatewayClass).ToNot(BeNil()) }) }) When("no changes are captured", func() { @@ -327,26 +351,83 @@ var _ = Describe("ChangeProcessor", func() { Expect(graphCfg).To(BeNil()) }) }) - When("resources are deleted", func() { - It("returns empty graph", func() { + When("the second Gateway is upserted", func() { + It("returns populated graph using first gateway", func() { + processor.CaptureUpsertChange(gw2) + + changed, graphCfg := processor.Process() + Expect(changed).To(BeTrue()) + Expect(graphCfg.Gateway.Source.Name).To(Equal(gw1.Name)) + }) + }) + When("the second HTTPRoute is upserted", func() { + It("returns populated graph", func() { + processor.CaptureUpsertChange(hr2) + + changed, graphCfg := processor.Process() + Expect(changed).To(BeTrue()) + Expect(graphCfg.Routes).To(HaveLen(2)) + }) + }) + When("the first Gateway is deleted", func() { + It("returns updated graph", func() { + processor.CaptureDeleteChange( + &v1beta1.Gateway{}, + types.NamespacedName{Namespace: "test", Name: "gateway-1"}, + ) + + changed, graphCfg := processor.Process() + Expect(changed).To(BeTrue()) + Expect(graphCfg.Gateway.Source.Name).To(Equal(gw2.Name)) + Expect(graphCfg.Routes).To(HaveLen(1)) + }) + }) + When("the second HTTPRoute is deleted", func() { + It("returns updated graph", func() { + processor.CaptureDeleteChange( + &v1beta1.HTTPRoute{}, + types.NamespacedName{Namespace: "test", Name: "hr-2"}, + ) + + changed, graphCfg := processor.Process() + Expect(changed).To(BeTrue()) + Expect(graphCfg.Routes).To(HaveLen(0)) + }) + }) + When("the GatewayClass is deleted", func() { + It("returns updated graph", func() { processor.CaptureDeleteChange( &v1beta1.GatewayClass{}, types.NamespacedName{Name: gcName}, ) + + changed, graphCfg := processor.Process() + Expect(changed).To(BeTrue()) + Expect(graphCfg.GatewayClass).To(BeNil()) + }) + }) + When("the second Gateway is deleted", func() { + It("returns updated graph", func() { processor.CaptureDeleteChange( &v1beta1.Gateway{}, - types.NamespacedName{Namespace: "test", Name: "gateway-1"}, + types.NamespacedName{Namespace: "test", Name: "gateway-2"}, ) + + changed, graphCfg := processor.Process() + Expect(changed).To(BeTrue()) + Expect(graphCfg.Gateway).To(BeNil()) + }) + }) + When("the first HTTPRoute is deleted", func() { + It("returns updated graph", func() { processor.CaptureDeleteChange( &v1beta1.HTTPRoute{}, types.NamespacedName{Namespace: "test", Name: "hr-1"}, ) - emptyGraph := &graph.Graph{} - changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(helpers.Diff(emptyGraph, graphCfg)).To(BeEmpty()) + Expect(graphCfg.Routes).To(BeNil()) }) }) }) @@ -685,9 +766,6 @@ var _ = Describe("ChangeProcessor", func() { }) Describe("Ensuring non-changing changes don't override previously changing changes", func() { - // Note: in these tests, we deliberately don't fully inspect the returned configuration and statuses - // -- this is done in 'Normal cases of processing changes' - var ( processor *state.ChangeProcessorImpl fakeRelationshipCapturer *relationshipfakes.FakeCapturer From 85657cc62d4dd25fa831a601229bdf8778714c34 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Thu, 1 Jun 2023 11:19:18 -0600 Subject: [PATCH 3/4] Add graph check back to tests --- internal/state/change_processor_test.go | 199 +++++++++++++++++++++--- 1 file changed, 181 insertions(+), 18 deletions(-) diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go index a20019c0a9..19f2dc827b 100644 --- a/internal/state/change_processor_test.go +++ b/internal/state/change_processor_test.go @@ -3,6 +3,7 @@ package state_test import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/onsi/gomega/format" apiv1 "k8s.io/api/core/v1" discoveryV1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -18,6 +19,8 @@ import ( "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" "github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/index" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/graph" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/relationship" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/relationship/relationshipfakes" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets/secretsfakes" @@ -187,6 +190,7 @@ func createScheme() *runtime.Scheme { } var _ = Describe("ChangeProcessor", func() { + format.MaxLength = 0 Describe("Normal cases of processing changes", func() { var ( gc = &v1beta1.GatewayClass{ @@ -220,9 +224,11 @@ var _ = Describe("ChangeProcessor", func() { Describe("Process gateway resources", Ordered, func() { var ( - gcUpdated *v1beta1.GatewayClass - hr1, hr1Updated, hr2 *v1beta1.HTTPRoute - gw1, gw1Updated, gw2 *v1beta1.Gateway + gcUpdated *v1beta1.GatewayClass + hr1, hr1Updated, hr2 *v1beta1.HTTPRoute + gw1, gw1Updated, gw2 *v1beta1.Gateway + expGraph *graph.Graph + expRouteHR1, expRouteHR2 *graph.Route ) BeforeAll(func() { gcUpdated = gc.DeepCopy() @@ -242,6 +248,87 @@ var _ = Describe("ChangeProcessor", func() { gw2 = createGatewayWithTLSListener("gateway-2") }) + BeforeEach(func() { + expRouteHR1 = &graph.Route{ + Source: hr1, + ParentRefs: []graph.ParentRef{ + { + Attachment: &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{"listener-80-1": {"foo.example.com"}}, + Attached: true, + }, + Gateway: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, + }, + { + Attachment: &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{"listener-443-1": {"foo.example.com"}}, + Attached: true, + }, + Gateway: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, + Idx: 1, + }, + }, + Rules: []graph.Rule{{ValidMatches: true, ValidFilters: true}}, + Valid: true, + } + + expRouteHR2 = &graph.Route{ + Source: hr2, + ParentRefs: []graph.ParentRef{ + { + Attachment: &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{"listener-80-1": {"bar.example.com"}}, + Attached: true, + }, + Gateway: types.NamespacedName{Namespace: "test", Name: "gateway-2"}, + }, + { + Attachment: &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{"listener-443-1": {"bar.example.com"}}, + Attached: true, + }, + Gateway: types.NamespacedName{Namespace: "test", Name: "gateway-2"}, + Idx: 1, + }, + }, + Rules: []graph.Rule{{ValidMatches: true, ValidFilters: true}}, + Valid: true, + } + + // This is the base case expected graph. Tests will manipulate this to add or remove elements + // to fit the expected output of the input under test. + expGraph = &graph.Graph{ + GatewayClass: &graph.GatewayClass{ + Source: gc, + Valid: true, + }, + Gateway: &graph.Gateway{ + Source: gw1, + Listeners: map[string]*graph.Listener{ + "listener-80-1": { + Source: gw1.Spec.Listeners[0], + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "hr-1"}: expRouteHR1, + }, + }, + "listener-443-1": { + Source: gw1.Spec.Listeners[1], + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "hr-1"}: expRouteHR1, + }, + SecretPath: "path/to/cert", + }, + }, + Valid: true, + }, + IgnoredGateways: map[types.NamespacedName]*v1beta1.Gateway{}, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "hr-1"}: expRouteHR1, + }, + } + }) When("no upsert has occurred", func() { It("returns empty graph", func() { @@ -258,17 +345,32 @@ var _ = Describe("ChangeProcessor", func() { changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(graphCfg.Routes).To(BeNil()) + Expect(graphCfg).To(Equal(&graph.Graph{})) }) }) When("the first Gateway is upserted", func() { It("returns populated graph", func() { processor.CaptureUpsertChange(gw1) + expGraph.GatewayClass = nil + + expGraph.Gateway.Conditions = conditions.NewGatewayInvalid("GatewayClass doesn't exist") + expGraph.Gateway.Valid = false + expGraph.Gateway.Listeners = nil + + hrName := types.NamespacedName{Namespace: "test", Name: "hr-1"} + expGraph.Routes[hrName].ParentRefs[0].Attachment = &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{}, + FailedCondition: conditions.NewRouteInvalidGateway(), + } + expGraph.Routes[hrName].ParentRefs[1].Attachment = &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{}, + FailedCondition: conditions.NewRouteInvalidGateway(), + } + changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(graphCfg.Gateway).ToNot(BeNil()) - Expect(graphCfg.Routes).To(HaveLen(1)) + Expect(helpers.Diff(expGraph, graphCfg)).To(BeEmpty()) }) }) }) @@ -279,7 +381,7 @@ var _ = Describe("ChangeProcessor", func() { changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(graphCfg.GatewayClass).ToNot(BeNil()) + Expect(helpers.Diff(expGraph, graphCfg)).To(BeEmpty()) }) }) When("the first HTTPRoute without a generation changed is processed", func() { @@ -297,9 +399,13 @@ var _ = Describe("ChangeProcessor", func() { It("returns populated graph", func() { processor.CaptureUpsertChange(hr1Updated) + hrName := types.NamespacedName{Namespace: "test", Name: "hr-1"} + expGraph.Gateway.Listeners["listener-443-1"].Routes[hrName].Source.Generation = hr1Updated.Generation + expGraph.Gateway.Listeners["listener-80-1"].Routes[hrName].Source.Generation = hr1Updated.Generation + changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(graphCfg.Routes).To(HaveLen(1)) + Expect(helpers.Diff(expGraph, graphCfg)).To(BeEmpty()) }, ) }) @@ -318,9 +424,11 @@ var _ = Describe("ChangeProcessor", func() { It("returns populated graph", func() { processor.CaptureUpsertChange(gw1Updated) + expGraph.Gateway.Source.Generation = gw1Updated.Generation + changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(graphCfg.Gateway).ToNot(BeNil()) + Expect(helpers.Diff(expGraph, graphCfg)).To(BeEmpty()) }) }) When("the GatewayClass update without generation change is processed", func() { @@ -338,9 +446,11 @@ var _ = Describe("ChangeProcessor", func() { It("returns populated graph", func() { processor.CaptureUpsertChange(gcUpdated) + expGraph.GatewayClass.Source.Generation = gcUpdated.Generation + changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(graphCfg.GatewayClass).ToNot(BeNil()) + Expect(helpers.Diff(expGraph, graphCfg)).To(BeEmpty()) }) }) When("no changes are captured", func() { @@ -355,18 +465,37 @@ var _ = Describe("ChangeProcessor", func() { It("returns populated graph using first gateway", func() { processor.CaptureUpsertChange(gw2) + expGraph.IgnoredGateways = map[types.NamespacedName]*v1beta1.Gateway{ + {Namespace: "test", Name: "gateway-2"}: gw2, + } + changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(graphCfg.Gateway.Source.Name).To(Equal(gw1.Name)) + Expect(helpers.Diff(expGraph, graphCfg)).To(BeEmpty()) }) }) When("the second HTTPRoute is upserted", func() { It("returns populated graph", func() { processor.CaptureUpsertChange(hr2) + hrName := types.NamespacedName{Namespace: "test", Name: "hr-2"} + + expGraph.IgnoredGateways = map[types.NamespacedName]*v1beta1.Gateway{ + {Namespace: "test", Name: "gateway-2"}: gw2, + } + expGraph.Routes[hrName] = expRouteHR2 + expGraph.Routes[hrName].ParentRefs[0].Attachment = &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{}, + FailedCondition: conditions.NewTODO("Gateway is ignored"), + } + expGraph.Routes[hrName].ParentRefs[1].Attachment = &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{}, + FailedCondition: conditions.NewTODO("Gateway is ignored"), + } + changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(graphCfg.Routes).To(HaveLen(2)) + Expect(helpers.Diff(expGraph, graphCfg)).To(BeEmpty()) }) }) When("the first Gateway is deleted", func() { @@ -376,10 +505,22 @@ var _ = Describe("ChangeProcessor", func() { types.NamespacedName{Namespace: "test", Name: "gateway-1"}, ) + // gateway 2 takes over; + // route 1 has been replaced by route 2 + hr1Name := types.NamespacedName{Namespace: "test", Name: "hr-1"} + hr2Name := types.NamespacedName{Namespace: "test", Name: "hr-2"} + expGraph.Gateway.Source = gw2 + delete(expGraph.Gateway.Listeners["listener-80-1"].Routes, hr1Name) + delete(expGraph.Gateway.Listeners["listener-443-1"].Routes, hr1Name) + expGraph.Gateway.Listeners["listener-80-1"].Routes[hr2Name] = expRouteHR2 + expGraph.Gateway.Listeners["listener-443-1"].Routes[hr2Name] = expRouteHR2 + + delete(expGraph.Routes, hr1Name) + expGraph.Routes[hr2Name] = expRouteHR2 + changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(graphCfg.Gateway.Source.Name).To(Equal(gw2.Name)) - Expect(graphCfg.Routes).To(HaveLen(1)) + Expect(helpers.Diff(expGraph, graphCfg)).To(BeEmpty()) }) }) When("the second HTTPRoute is deleted", func() { @@ -389,9 +530,17 @@ var _ = Describe("ChangeProcessor", func() { types.NamespacedName{Namespace: "test", Name: "hr-2"}, ) + // gateway 2 still in charge; + // no routes remain + hr1Name := types.NamespacedName{Namespace: "test", Name: "hr-1"} + expGraph.Gateway.Source = gw2 + delete(expGraph.Gateway.Listeners["listener-80-1"].Routes, hr1Name) + delete(expGraph.Gateway.Listeners["listener-443-1"].Routes, hr1Name) + expGraph.Routes = map[types.NamespacedName]*graph.Route{} + changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(graphCfg.Routes).To(HaveLen(0)) + Expect(helpers.Diff(expGraph, graphCfg)).To(BeEmpty()) }) }) When("the GatewayClass is deleted", func() { @@ -401,9 +550,16 @@ var _ = Describe("ChangeProcessor", func() { types.NamespacedName{Name: gcName}, ) + expGraph.GatewayClass = nil + expGraph.Gateway = &graph.Gateway{ + Source: gw2, + Conditions: conditions.NewGatewayInvalid("GatewayClass doesn't exist"), + } + expGraph.Routes = map[types.NamespacedName]*graph.Route{} + changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(graphCfg.GatewayClass).To(BeNil()) + Expect(helpers.Diff(expGraph, graphCfg)).To(BeEmpty()) }) }) When("the second Gateway is deleted", func() { @@ -413,9 +569,11 @@ var _ = Describe("ChangeProcessor", func() { types.NamespacedName{Namespace: "test", Name: "gateway-2"}, ) + expGraph := &graph.Graph{} + changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(graphCfg.Gateway).To(BeNil()) + Expect(helpers.Diff(expGraph, graphCfg)).To(BeEmpty()) }) }) When("the first HTTPRoute is deleted", func() { @@ -425,9 +583,11 @@ var _ = Describe("ChangeProcessor", func() { types.NamespacedName{Namespace: "test", Name: "hr-1"}, ) + expGraph := &graph.Graph{} + changed, graphCfg := processor.Process() Expect(changed).To(BeTrue()) - Expect(graphCfg.Routes).To(BeNil()) + Expect(helpers.Diff(expGraph, graphCfg)).To(BeEmpty()) }) }) }) @@ -766,6 +926,9 @@ var _ = Describe("ChangeProcessor", func() { }) Describe("Ensuring non-changing changes don't override previously changing changes", func() { + // Note: in these tests, we deliberately don't fully inspect the returned configuration and statuses + // -- this is done in 'Normal cases of processing changes' + var ( processor *state.ChangeProcessorImpl fakeRelationshipCapturer *relationshipfakes.FakeCapturer From 4dac3f29d05e59e8a562e0e884eeabefd4e62495 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Thu, 1 Jun 2023 14:34:42 -0600 Subject: [PATCH 4/4] Update comments --- internal/state/change_processor.go | 4 ++-- internal/state/change_processor_test.go | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/state/change_processor.go b/internal/state/change_processor.go index f334e1ecaa..07050889cb 100644 --- a/internal/state/change_processor.go +++ b/internal/state/change_processor.go @@ -32,7 +32,7 @@ const ( type extractGVKFunc func(obj client.Object) schema.GroupVersionKind -// ChangeProcessor processes the changes to resources producing the internal representation +// ChangeProcessor processes the changes to resources and produces a graph-like representation // of the Gateway configuration. It only supports one GatewayClass resource. type ChangeProcessor interface { // CaptureUpsertChange captures an upsert change to a resource. @@ -43,7 +43,7 @@ type ChangeProcessor interface { // The method panics if the resource is of unsupported type or if the passed Gateway is different from the one // this ChangeProcessor was created for. CaptureDeleteChange(resourceType client.Object, nsname types.NamespacedName) - // Process produces a Graph-like representation of GatewayAPI resources. + // Process produces a graph-like representation of GatewayAPI resources. // If no changes were captured, the changed return argument will be false and graph will be empty. Process() (changed bool, graphCfg *graph.Graph) } diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go index 19f2dc827b..f8f97b0cf2 100644 --- a/internal/state/change_processor_test.go +++ b/internal/state/change_processor_test.go @@ -190,6 +190,7 @@ func createScheme() *runtime.Scheme { } var _ = Describe("ChangeProcessor", func() { + // graph outputs are large, so allow gomega to print everything on test failure format.MaxLength = 0 Describe("Normal cases of processing changes", func() { var (