From 73172f5748de1a4e119a0b54be25016efbe201b6 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Thu, 29 Jun 2023 13:41:41 -0600 Subject: [PATCH 1/5] Set GatewayClass status for ignored GatewayClasses Problem: It is expected that any GatewayClass that references our controller should have its status set properly, even if not used by our controller. Solution: Both the provisioner and controller will now add Accepted status False and ObservedGeneration to any GatewayClass that references our controller but is not configured to be used by our controller. --- cmd/gateway/commands.go | 1 + conformance/Makefile | 2 +- docs/gateway-api-compatibility.md | 5 +- internal/manager/manager.go | 4 +- internal/manager/predicate/gatewayclass.go | 42 +++++ .../manager/predicate/gatewayclass_test.go | 33 ++++ internal/provisioner/handler.go | 29 +++- internal/provisioner/handler_test.go | 65 ++++++-- internal/provisioner/manager.go | 8 +- internal/state/conditions/conditions.go | 20 +++ internal/state/graph/gatewayclass.go | 39 ++++- internal/state/graph/gatewayclass_test.go | 155 ++++++++++++------ internal/state/graph/graph.go | 23 +-- internal/status/statuses.go | 57 +++++-- internal/status/statuses_test.go | 74 ++++++++- internal/status/updater.go | 14 +- internal/status/updater_test.go | 16 +- 17 files changed, 456 insertions(+), 131 deletions(-) create mode 100644 internal/manager/predicate/gatewayclass.go create mode 100644 internal/manager/predicate/gatewayclass_test.go diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go index 00f573934..ff6a2c055 100644 --- a/cmd/gateway/commands.go +++ b/cmd/gateway/commands.go @@ -193,6 +193,7 @@ func createProvisionerModeCommand() *cobra.Command { return provisioner.StartManager(provisioner.Config{ Logger: logger, GatewayClassName: gatewayClassName.value, + GatewayCtlrName: gatewayCtlrName.value, }) }, } diff --git a/conformance/Makefile b/conformance/Makefile index c111c8289..c797171d7 100644 --- a/conformance/Makefile +++ b/conformance/Makefile @@ -1,7 +1,7 @@ NKG_TAG = edge NKG_PREFIX = nginx-kubernetes-gateway GATEWAY_CLASS = nginx -SUPPORTED_FEATURES = HTTPRoute,HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect +SUPPORTED_FEATURES = HTTPRoute,HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,GatewayClassObservedGenerationBump KIND_KUBE_CONFIG=$${HOME}/.kube/kind/config TAG = latest PREFIX = conformance-test-runner diff --git a/docs/gateway-api-compatibility.md b/docs/gateway-api-compatibility.md index 4b1b4ef2a..62cd7f2b2 100644 --- a/docs/gateway-api-compatibility.md +++ b/docs/gateway-api-compatibility.md @@ -43,7 +43,10 @@ Fields: * `parametersRef` - not supported. * `description` - supported. * `status` - * `conditions` - partially supported. + * `conditions` - supported (Condition/Status/Reason): + * `Accepted/True/Accepted` + * `Accepted/False/InvalidParameters` + * `Accepted/False/GatewayClassConflict`: Custom reason for when the GatewayClass references this controller, but a different GatewayClass name is provided to the controller via the command-line argument. ### Gateway diff --git a/internal/manager/manager.go b/internal/manager/manager.go index 864dcfe36..994e0e40a 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -80,9 +80,7 @@ func Start(cfg config.Config) error { { objectType: &gatewayv1beta1.GatewayClass{}, options: []controller.Option{ - controller.WithNamespacedNameFilter(filter.CreateSingleResourceFilter( - types.NamespacedName{Name: cfg.GatewayClassName}, - )), + controller.WithK8sPredicate(predicate.GatewayClassPredicate{ControllerName: cfg.GatewayCtlrName}), }, }, { diff --git a/internal/manager/predicate/gatewayclass.go b/internal/manager/predicate/gatewayclass.go new file mode 100644 index 000000000..13f07e27c --- /dev/null +++ b/internal/manager/predicate/gatewayclass.go @@ -0,0 +1,42 @@ +package predicate + +import ( + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/gateway-api/apis/v1beta1" +) + +// GatewayClassPredicate implements a predicate function based on the controllerName of a GatewayClass. +// This predicate will skip events for GatewayClasses that don't reference this controller. +type GatewayClassPredicate struct { + predicate.Funcs + ControllerName string +} + +// Create implements default CreateEvent filter for validating a GatewayClass controllerName. +func (gcp GatewayClassPredicate) Create(e event.CreateEvent) bool { + if e.Object == nil { + return false + } + + gc, ok := e.Object.(*v1beta1.GatewayClass) + if !ok { + return false + } + + return string(gc.Spec.ControllerName) == gcp.ControllerName +} + +// Update implements default UpdateEvent filter for validating a GatewayClass controllerName. +func (gcp GatewayClassPredicate) Update(e event.UpdateEvent) bool { + if e.ObjectNew == nil { + return false + } + + gc, ok := e.ObjectNew.(*v1beta1.GatewayClass) + if !ok { + return false + } + + return string(gc.Spec.ControllerName) == gcp.ControllerName +} diff --git a/internal/manager/predicate/gatewayclass_test.go b/internal/manager/predicate/gatewayclass_test.go new file mode 100644 index 000000000..edcc8f9c7 --- /dev/null +++ b/internal/manager/predicate/gatewayclass_test.go @@ -0,0 +1,33 @@ +package predicate + +import ( + "testing" + + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/gateway-api/apis/v1beta1" +) + +func TestGatewayClassPredicate(t *testing.T) { + p := GatewayClassPredicate{ControllerName: "nginx-ctlr"} + + gc := &v1beta1.GatewayClass{ + Spec: v1beta1.GatewayClassSpec{ + ControllerName: "nginx-ctlr", + }, + } + + if !p.Create(event.CreateEvent{Object: gc}) { + t.Error("GatewayClassPredicate.Create() returned false; expected true") + } + if !p.Update(event.UpdateEvent{ObjectNew: gc}) { + t.Error("GatewayClassPredicate.Update() returned false; expected true") + } + + gc.Spec.ControllerName = "unknown" + if p.Create(event.CreateEvent{Object: gc}) { + t.Error("GatewayClassPredicate.Create() returned true; expected false") + } + if p.Update(event.UpdateEvent{ObjectNew: gc}) { + t.Error("GatewayClassPredicate.Update() returned true; expected false") + } +} diff --git a/internal/provisioner/handler.go b/internal/provisioner/handler.go index 93092c872..16f7c8a8d 100644 --- a/internal/provisioner/handler.go +++ b/internal/provisioner/handler.go @@ -53,17 +53,28 @@ func newEventHandler( } } -func (h *eventHandler) ensureGatewayClassAccepted(ctx context.Context) { - gc, exist := h.store.gatewayClasses[types.NamespacedName{Name: h.gcName}] - if !exist { - panic(fmt.Errorf("GatewayClass %s must exist", h.gcName)) +func (h *eventHandler) setGatewayClassStatuses(ctx context.Context) { + statuses := status.Statuses{ + GatewayClassStatuses: make(status.GatewayClassStatuses), } - statuses := status.Statuses{ - GatewayClassStatus: &status.GatewayClassStatus{ - Conditions: conditions.NewDefaultGatewayClassConditions(), + var gcExists bool + for nsname, gc := range h.store.gatewayClasses { + var conds []conditions.Condition + if gc.Name == h.gcName { + gcExists = true + conds = conditions.NewDefaultGatewayClassConditions() + } else { + conds = []conditions.Condition{conditions.NewGatewayClassConflict()} + } + + statuses.GatewayClassStatuses[nsname] = status.GatewayClassStatus{ + Conditions: conds, ObservedGeneration: gc.Generation, - }, + } + } + if !gcExists { + panic(fmt.Errorf("GatewayClass %s must exist", h.gcName)) } h.statusUpdater.Update(ctx, statuses) @@ -133,7 +144,7 @@ func (h *eventHandler) ensureDeploymentsMatchGateways(ctx context.Context) { func (h *eventHandler) HandleEventBatch(ctx context.Context, batch events.EventBatch) { h.store.update(batch) - h.ensureGatewayClassAccepted(ctx) + h.setGatewayClassStatuses(ctx) h.ensureDeploymentsMatchGateways(ctx) } diff --git a/internal/provisioner/handler_test.go b/internal/provisioner/handler_test.go index 96da6bcf9..ad47f3873 100644 --- a/internal/provisioner/handler_test.go +++ b/internal/provisioner/handler_test.go @@ -19,6 +19,7 @@ import ( embeddedfiles "github.com/nginxinc/nginx-kubernetes-gateway" "github.com/nginxinc/nginx-kubernetes-gateway/internal/events" "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" + "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" ) @@ -272,6 +273,46 @@ var _ = Describe("handler", func() { Expect(deps.Items).To(HaveLen(0)) }) }) + + When("upserting GatewayClass that is not set in command-line argument", func() { + It("should set the proper status if this controller is referenced", func() { + gc := &v1beta1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "unknown-gc", + }, + Spec: v1beta1.GatewayClassSpec{ + ControllerName: "test.example.com", + }, + } + err := k8sclient.Create(context.Background(), gc) + Expect(err).ShouldNot(HaveOccurred()) + + batch := []interface{}{ + &events.UpsertEvent{ + Resource: gc, + }, + } + + handler.HandleEventBatch(context.Background(), batch) + + unknownGC := &v1beta1.GatewayClass{} + err = k8sclient.Get(context.Background(), client.ObjectKeyFromObject(gc), unknownGC) + Expect(err).ShouldNot(HaveOccurred()) + + expectedConditions := []metav1.Condition{ + { + Type: string(v1beta1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionFalse, + ObservedGeneration: 0, + LastTransitionTime: fakeClockTime, + Reason: string(conditions.GatewayClassReasonGatewayClassConflict), + Message: string(conditions.GatewayClassMessageGatewayClassConflict), + }, + } + + Expect(unknownGC.Status.Conditions).To(Equal(expectedConditions)) + }) + }) }) Describe("Edge cases", func() { @@ -392,20 +433,20 @@ var _ = Describe("handler", func() { Expect(handle).Should(Panic()) }) }) - }) - When("upserting Gateway with broken static Deployment YAML", func() { - It("it should panic", func() { - handler = newEventHandler( - gcName, - statusUpdater, - k8sclient, - zap.New(), - []byte("broken YAML"), - ) + When("upserting Gateway with broken static Deployment YAML", func() { + It("it should panic", func() { + handler = newEventHandler( + gcName, + statusUpdater, + k8sclient, + zap.New(), + []byte("broken YAML"), + ) - itShouldUpsertGatewayClass() - itShouldPanicWhenUpsertingGateway(types.NamespacedName{Namespace: "test-ns", Name: "test-gw"}) + itShouldUpsertGatewayClass() + itShouldPanicWhenUpsertingGateway(types.NamespacedName{Namespace: "test-ns", Name: "test-gw"}) + }) }) }) }) diff --git a/internal/provisioner/manager.go b/internal/provisioner/manager.go index d394ad05c..0337c7b49 100644 --- a/internal/provisioner/manager.go +++ b/internal/provisioner/manager.go @@ -7,7 +7,6 @@ import ( v1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" ctlr "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -17,7 +16,7 @@ import ( embeddedfiles "github.com/nginxinc/nginx-kubernetes-gateway" "github.com/nginxinc/nginx-kubernetes-gateway/internal/controller" "github.com/nginxinc/nginx-kubernetes-gateway/internal/events" - "github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/filter" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/predicate" "github.com/nginxinc/nginx-kubernetes-gateway/internal/status" ) @@ -25,6 +24,7 @@ import ( type Config struct { Logger logr.Logger GatewayClassName string + GatewayCtlrName string } // StartManager starts a Manager for the provisioner mode, which provisions @@ -60,9 +60,7 @@ func StartManager(cfg Config) error { { objectType: &gatewayv1beta1.GatewayClass{}, options: []controller.Option{ - controller.WithNamespacedNameFilter( - filter.CreateSingleResourceFilter(types.NamespacedName{Name: cfg.GatewayClassName}), - ), + controller.WithK8sPredicate(predicate.GatewayClassPredicate{ControllerName: cfg.GatewayCtlrName}), }, }, { diff --git a/internal/state/conditions/conditions.go b/internal/state/conditions/conditions.go index b1e64ef23..929731a8b 100644 --- a/internal/state/conditions/conditions.go +++ b/internal/state/conditions/conditions.go @@ -8,6 +8,15 @@ import ( ) const ( + // GatewayClassReasonGatewayClassConflict indicates there are multiple GatewayClass resources + // that reference this controller, and we ignored the resource in question and picked the + // GatewayClass that is referenced in the command-line argument. + // This reason is used with GatewayClassConditionAccepted (false). + GatewayClassReasonGatewayClassConflict v1beta1.GatewayClassConditionReason = "GatewayClassConflict" + + // GatewayClassMessageGatewayClassConflict is a message that describes GatewayClassReasonGatewayClassConflict. + GatewayClassMessageGatewayClassConflict = "The resource is ignored due to a conflicting GatewayClass resource" + // ListenerReasonUnsupportedValue is used with the "Accepted" condition when a value of a field in a Listener // is invalid or not supported. ListenerReasonUnsupportedValue v1beta1.ListenerConditionReason = "UnsupportedValue" @@ -427,6 +436,17 @@ func NewDefaultGatewayClassConditions() []Condition { } } +// NewGatewayClassConflict returns a Condition that indicates that the GatewayClass is not accepted +// due to a conflict with another GatewayClass. +func NewGatewayClassConflict() Condition { + return Condition{ + Type: string(v1beta1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionFalse, + Reason: string(GatewayClassReasonGatewayClassConflict), + Message: GatewayClassMessageGatewayClassConflict, + } +} + // NewGatewayClassInvalidParameters returns a Condition that indicates that the GatewayClass has invalid parameters. func NewGatewayClassInvalidParameters(msg string) Condition { return Condition{ diff --git a/internal/state/graph/gatewayclass.go b/internal/state/graph/gatewayclass.go index 8413f3ae3..15c002a3a 100644 --- a/internal/state/graph/gatewayclass.go +++ b/internal/state/graph/gatewayclass.go @@ -1,7 +1,9 @@ package graph import ( + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions" @@ -17,13 +19,40 @@ type GatewayClass struct { Valid bool } -func gatewayClassBelongsToController(gc *v1beta1.GatewayClass, controllerName string) bool { - // if GatewayClass doesn't exist, we assume it belongs to the controller - if gc == nil { - return true +// processedGatewayClasses holds the resources that belong to NKG. +type processedGatewayClasses struct { + Winner *v1beta1.GatewayClass + Ignored map[types.NamespacedName]*v1beta1.GatewayClass +} + +// processGatewayClasses returns the "Winner" GatewayClass, which is defined in +// the command-line argument and references this controller, and a list of "Ignored" GatewayClasses +// that reference this controller, but are not named in the command-line argument. +// Also returns a boolean that says whether or not the GatewayClasa defined +// in the command-line argument exists, regardless of which controller it references. +func processGatewayClasses( + gcs map[types.NamespacedName]*v1beta1.GatewayClass, + gcName string, + controllerName string, +) (processedGatewayClasses, bool) { + processedGwClasses := processedGatewayClasses{} + + var gcExists bool + for _, gc := range gcs { + if gc.Name == gcName { + gcExists = true + if string(gc.Spec.ControllerName) == controllerName { + processedGwClasses.Winner = gc + } + } else if string(gc.Spec.ControllerName) == controllerName { + if processedGwClasses.Ignored == nil { + processedGwClasses.Ignored = make(map[types.NamespacedName]*v1beta1.GatewayClass) + } + processedGwClasses.Ignored[client.ObjectKeyFromObject(gc)] = gc + } } - return string(gc.Spec.ControllerName) == controllerName + return processedGwClasses, gcExists } func buildGatewayClass(gc *v1beta1.GatewayClass) *GatewayClass { diff --git a/internal/state/graph/gatewayclass_test.go b/internal/state/graph/gatewayclass_test.go index 0fa8dedee..178f2d04b 100644 --- a/internal/state/graph/gatewayclass_test.go +++ b/internal/state/graph/gatewayclass_test.go @@ -4,101 +4,156 @@ import ( "testing" . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "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/helpers" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions" ) -func TestBuildGatewayClass(t *testing.T) { - validGC := &v1beta1.GatewayClass{} - - invalidGC := &v1beta1.GatewayClass{ +func TestProcessGatewayClasses(t *testing.T) { + gcName := "test-gc" + ctlrName := "test-ctlr" + winner := &v1beta1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: gcName, + }, Spec: v1beta1.GatewayClassSpec{ - ParametersRef: &v1beta1.ParametersReference{}, + ControllerName: v1beta1.GatewayController(ctlrName), + }, + } + ignored := &v1beta1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-gc-ignored", + }, + Spec: v1beta1.GatewayClassSpec{ + ControllerName: v1beta1.GatewayController(ctlrName), }, } tests := []struct { - gc *v1beta1.GatewayClass - expected *GatewayClass + expected processedGatewayClasses + gcs map[types.NamespacedName]*v1beta1.GatewayClass name string + exists bool }{ { - gc: validGC, - expected: &GatewayClass{ - Source: validGC, - Valid: true, + gcs: nil, + expected: processedGatewayClasses{}, + name: "no gatewayclasses", + }, + { + gcs: map[types.NamespacedName]*v1beta1.GatewayClass{ + {Name: gcName}: winner, }, - name: "valid gatewayclass", + expected: processedGatewayClasses{ + Winner: winner, + }, + exists: true, + name: "one valid gatewayclass", }, { - gc: nil, - expected: nil, - name: "no gatewayclass", + gcs: map[types.NamespacedName]*v1beta1.GatewayClass{ + {Name: gcName}: { + ObjectMeta: metav1.ObjectMeta{ + Name: gcName, + }, + Spec: v1beta1.GatewayClassSpec{ + ControllerName: v1beta1.GatewayController("not ours"), + }, + }, + }, + expected: processedGatewayClasses{}, + exists: true, + name: "one valid gatewayclass, but references wrong controller", }, { - gc: invalidGC, - expected: &GatewayClass{ - Source: invalidGC, - Valid: false, - Conditions: []conditions.Condition{ - conditions.NewGatewayClassInvalidParameters("spec.parametersRef: Forbidden: parametersRef is not supported"), + gcs: map[types.NamespacedName]*v1beta1.GatewayClass{ + {Name: ignored.Name}: ignored, + }, + expected: processedGatewayClasses{ + Ignored: map[types.NamespacedName]*v1beta1.GatewayClass{ + client.ObjectKeyFromObject(ignored): ignored, }, }, - name: "invalid gatewayclass", + name: "one non-referenced gatewayclass with our controller", + }, + { + gcs: map[types.NamespacedName]*v1beta1.GatewayClass{ + {Name: "completely ignored"}: { + Spec: v1beta1.GatewayClassSpec{ + ControllerName: v1beta1.GatewayController("not ours"), + }, + }, + }, + expected: processedGatewayClasses{}, + name: "one non-referenced gatewayclass without our controller", + }, + { + gcs: map[types.NamespacedName]*v1beta1.GatewayClass{ + {Name: gcName}: winner, + {Name: ignored.Name}: ignored, + }, + expected: processedGatewayClasses{ + Winner: winner, + Ignored: map[types.NamespacedName]*v1beta1.GatewayClass{ + client.ObjectKeyFromObject(ignored): ignored, + }, + }, + exists: true, + name: "one valid gateway class and non-referenced gatewayclass", }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { g := NewGomegaWithT(t) - - result := buildGatewayClass(test.gc) + result, exists := processGatewayClasses(test.gcs, gcName, ctlrName) g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) + g.Expect(exists).To(Equal(test.exists)) }) } } -func TestGatewayClassBelongsToController(t *testing.T) { - const controllerName = "my.controller" +func TestBuildGatewayClass(t *testing.T) { + validGC := &v1beta1.GatewayClass{} + + invalidGC := &v1beta1.GatewayClass{ + Spec: v1beta1.GatewayClassSpec{ + ParametersRef: &v1beta1.ParametersReference{}, + }, + } tests := []struct { gc *v1beta1.GatewayClass + expected *GatewayClass name string - expected bool }{ { - gc: &v1beta1.GatewayClass{ - Spec: v1beta1.GatewayClassSpec{ - ControllerName: controllerName, - }, + gc: validGC, + expected: &GatewayClass{ + Source: validGC, + Valid: true, }, - expected: true, - name: "normal gatewayclass", + name: "valid gatewayclass", }, { gc: nil, - expected: true, + expected: nil, name: "no gatewayclass", }, { - gc: &v1beta1.GatewayClass{ - Spec: v1beta1.GatewayClassSpec{ - ControllerName: "some.controller", - }, - }, - expected: false, - name: "wrong controller name", - }, - { - gc: &v1beta1.GatewayClass{ - Spec: v1beta1.GatewayClassSpec{ - ControllerName: "", + gc: invalidGC, + expected: &GatewayClass{ + Source: invalidGC, + Valid: false, + Conditions: []conditions.Condition{ + conditions.NewGatewayClassInvalidParameters("spec.parametersRef: Forbidden: parametersRef is not supported"), }, }, - expected: false, - name: "empty controller name", + name: "invalid gatewayclass", }, } @@ -106,8 +161,8 @@ func TestGatewayClassBelongsToController(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewGomegaWithT(t) - result := gatewayClassBelongsToController(test.gc, controllerName) - g.Expect(result).To(Equal(test.expected)) + result := buildGatewayClass(test.gc) + g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) }) } } diff --git a/internal/state/graph/graph.go b/internal/state/graph/graph.go index 031752526..c73c2a345 100644 --- a/internal/state/graph/graph.go +++ b/internal/state/graph/graph.go @@ -25,6 +25,10 @@ type Graph struct { GatewayClass *GatewayClass // Gateway holds the winning Gateway resource. Gateway *Gateway + // IgnoredGatewayClasses holds the ignored GatewayClass resources, which reference NGINX Gateway in the + // controllerName, but are not configured via the NGINX Gateway CLI argument. It doesn't hold the GatewayClass + // resources that do not belong to the NGINX Gateway. + IgnoredGatewayClasses map[types.NamespacedName]*v1beta1.GatewayClass // IgnoredGateways holds the ignored Gateway resources, which belong to the NGINX Gateway (based on the // GatewayClassName field of the resource) but ignored. It doesn't hold the Gateway resources that do not belong to // the NGINX Gateway. @@ -41,16 +45,14 @@ func BuildGraph( secretMemoryMgr secrets.SecretDiskMemoryManager, validators validation.Validators, ) *Graph { - gatewayClass := state.GatewayClasses[types.NamespacedName{Name: gcName}] - - if !gatewayClassBelongsToController(gatewayClass, controllerName) { + processedGwClasses, gcExists := processGatewayClasses(state.GatewayClasses, gcName, controllerName) + if gcExists && processedGwClasses.Winner == nil { + // configured GatewayClass does not reference this controller return &Graph{} } - - gc := buildGatewayClass(gatewayClass) + gc := buildGatewayClass(processedGwClasses.Winner) processedGws := processGateways(state.Gateways, gcName) - gw := buildGateway(processedGws.Winner, secretMemoryMgr, gc, state.ReferenceGrants) routes := buildRoutesForGateways(validators.HTTPFieldsValidator, state.HTTPRoutes, processedGws.GetAllNsNames()) @@ -58,10 +60,11 @@ func BuildGraph( addBackendRefsToRouteRules(routes, state.Services) g := &Graph{ - GatewayClass: gc, - Gateway: gw, - Routes: routes, - IgnoredGateways: processedGws.Ignored, + GatewayClass: gc, + Gateway: gw, + Routes: routes, + IgnoredGatewayClasses: processedGwClasses.Ignored, + IgnoredGateways: processedGws.Ignored, } return g diff --git a/internal/status/statuses.go b/internal/status/statuses.go index 1e917c168..a38b3bac8 100644 --- a/internal/status/statuses.go +++ b/internal/status/statuses.go @@ -18,11 +18,14 @@ type HTTPRouteStatuses map[types.NamespacedName]HTTPRouteStatus // GatewayStatuses holds the statuses of Gateways where the key is the namespaced name of a Gateway. type GatewayStatuses map[types.NamespacedName]GatewayStatus +// GatewayClassStatuses holds the statuses of GatewayClasses where the key is the namespaced name of a GatewayClass. +type GatewayClassStatuses map[types.NamespacedName]GatewayClassStatus + // Statuses holds the status-related information about Gateway API resources. type Statuses struct { - GatewayClassStatus *GatewayClassStatus - GatewayStatuses GatewayStatuses - HTTPRouteStatuses HTTPRouteStatuses + GatewayClassStatuses GatewayClassStatuses + GatewayStatuses GatewayStatuses + HTTPRouteStatuses HTTPRouteStatuses } // GatewayStatus holds the status of the winning Gateway resource. @@ -77,21 +80,7 @@ func BuildStatuses(graph *graph.Graph, nginxReloadRes NginxReloadResult) Statuse HTTPRouteStatuses: make(HTTPRouteStatuses), } - if graph.GatewayClass != nil { - defaultConds := conditions.NewDefaultGatewayClassConditions() - - conds := make([]conditions.Condition, 0, len(graph.GatewayClass.Conditions)+len(defaultConds)) - - // We add default conds first, so that any additional conditions will override them, which is - // ensured by DeduplicateConditions. - conds = append(conds, defaultConds...) - conds = append(conds, graph.GatewayClass.Conditions...) - - statuses.GatewayClassStatus = &GatewayClassStatus{ - Conditions: conditions.DeduplicateConditions(conds), - ObservedGeneration: graph.GatewayClass.Source.Generation, - } - } + statuses.GatewayClassStatuses = buildGatewayClassStatuses(graph.GatewayClass, graph.IgnoredGatewayClasses) statuses.GatewayStatuses = buildGatewayStatuses(graph.Gateway, graph.IgnoredGateways, nginxReloadRes) @@ -137,6 +126,38 @@ func BuildStatuses(graph *graph.Graph, nginxReloadRes NginxReloadResult) Statuse return statuses } +func buildGatewayClassStatuses( + gc *graph.GatewayClass, + ignoredGwClasses map[types.NamespacedName]*v1beta1.GatewayClass, +) GatewayClassStatuses { + statuses := make(GatewayClassStatuses) + + if gc != nil { + defaultConds := conditions.NewDefaultGatewayClassConditions() + + conds := make([]conditions.Condition, 0, len(gc.Conditions)+len(defaultConds)) + + // We add default conds first, so that any additional conditions will override them, which is + // ensured by DeduplicateConditions. + conds = append(conds, defaultConds...) + conds = append(conds, gc.Conditions...) + + statuses[client.ObjectKeyFromObject(gc.Source)] = GatewayClassStatus{ + Conditions: conditions.DeduplicateConditions(conds), + ObservedGeneration: gc.Source.Generation, + } + } + + for nsname, gwClass := range ignoredGwClasses { + statuses[nsname] = GatewayClassStatus{ + Conditions: []conditions.Condition{conditions.NewGatewayClassConflict()}, + ObservedGeneration: gwClass.Generation, + } + } + + return statuses +} + func buildGatewayStatuses( gateway *graph.Gateway, ignoredGateways map[types.NamespacedName]*v1beta1.Gateway, diff --git a/internal/status/statuses_test.go b/internal/status/statuses_test.go index 26779498f..b151658bd 100644 --- a/internal/status/statuses_test.go +++ b/internal/status/statuses_test.go @@ -134,9 +134,11 @@ func TestBuildStatuses(t *testing.T) { } expected := Statuses{ - GatewayClassStatus: &GatewayClassStatus{ - ObservedGeneration: 1, - Conditions: conditions.NewDefaultGatewayClassConditions(), + GatewayClassStatuses: GatewayClassStatuses{ + {Name: ""}: { + ObservedGeneration: 1, + Conditions: conditions.NewDefaultGatewayClassConditions(), + }, }, GatewayStatuses: GatewayStatuses{ {Namespace: "test", Name: "gateway"}: { @@ -243,6 +245,7 @@ func TestBuildStatusesNginxErr(t *testing.T) { } expected := Statuses{ + GatewayClassStatuses: GatewayClassStatuses{}, GatewayStatuses: GatewayStatuses{ {Namespace: "test", Name: "gateway"}: { Conditions: []conditions.Condition{ @@ -287,6 +290,71 @@ func TestBuildStatusesNginxErr(t *testing.T) { g.Expect(helpers.Diff(expected, result)).To(BeEmpty()) } +func TestBuildGatewayClassStatuses(t *testing.T) { + tests := []struct { + gc *graph.GatewayClass + ignoredClasses map[types.NamespacedName]*v1beta1.GatewayClass + expected GatewayClassStatuses + name string + }{ + { + name: "nil gatewayclass and no ignored gatewayclasses", + expected: GatewayClassStatuses{}, + }, + { + name: "nil gatewayclass and ignored gatewayclasses", + ignoredClasses: map[types.NamespacedName]*v1beta1.GatewayClass{ + {Name: "ignored-1"}: { + ObjectMeta: metav1.ObjectMeta{ + Generation: 1, + }, + }, + {Name: "ignored-2"}: { + ObjectMeta: metav1.ObjectMeta{ + Generation: 2, + }, + }, + }, + expected: GatewayClassStatuses{ + {Name: "ignored-1"}: { + Conditions: []conditions.Condition{conditions.NewGatewayClassConflict()}, + ObservedGeneration: 1, + }, + {Name: "ignored-2"}: { + Conditions: []conditions.Condition{conditions.NewGatewayClassConflict()}, + ObservedGeneration: 2, + }, + }, + }, + { + name: "valid gatewayclass", + gc: &graph.GatewayClass{ + Source: &v1beta1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid-gc", + Generation: 1, + }, + }, + }, + expected: GatewayClassStatuses{ + {Name: "valid-gc"}: { + Conditions: conditions.NewDefaultGatewayClassConditions(), + ObservedGeneration: 1, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + result := buildGatewayClassStatuses(test.gc, test.ignoredClasses) + g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) + }) + } +} + func TestBuildGatewayStatuses(t *testing.T) { tests := []struct { nginxReloadRes NginxReloadResult diff --git a/internal/status/updater.go b/internal/status/updater.go index 16aed554c..6b615ec45 100644 --- a/internal/status/updater.go +++ b/internal/status/updater.go @@ -83,16 +83,14 @@ 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 - if upd.cfg.UpdateGatewayClassStatus && statuses.GatewayClassStatus != nil { - upd.update( - ctx, - types.NamespacedName{Name: upd.cfg.GatewayClassName}, - &v1beta1.GatewayClass{}, - func(object client.Object) { + if upd.cfg.UpdateGatewayClassStatus { + for nsname, gcs := range statuses.GatewayClassStatuses { + upd.update(ctx, nsname, &v1beta1.GatewayClass{}, func(object client.Object) { gc := object.(*v1beta1.GatewayClass) - gc.Status = prepareGatewayClassStatus(*statuses.GatewayClassStatus, upd.cfg.Clock.Now()) + gc.Status = prepareGatewayClassStatus(gcs, upd.cfg.Clock.Now()) }, - ) + ) + } } for nsname, gs := range statuses.GatewayStatuses { diff --git a/internal/status/updater_test.go b/internal/status/updater_test.go index f1020c196..43d401a51 100644 --- a/internal/status/updater_test.go +++ b/internal/status/updater_test.go @@ -71,9 +71,11 @@ var _ = Describe("Updater", func() { createStatuses = func(gens generations) status.Statuses { return status.Statuses{ - GatewayClassStatus: &status.GatewayClassStatus{ - ObservedGeneration: gens.gatewayClass, - Conditions: status.CreateTestConditions("Test"), + GatewayClassStatuses: status.GatewayClassStatuses{ + {Name: gcName}: { + ObservedGeneration: gens.gatewayClass, + Conditions: status.CreateTestConditions("Test"), + }, }, GatewayStatuses: status.GatewayStatuses{ {Namespace: "test", Name: "gateway"}: { @@ -441,9 +443,11 @@ var _ = Describe("Updater", func() { updater.Update( context.Background(), status.Statuses{ - GatewayClassStatus: &status.GatewayClassStatus{ - ObservedGeneration: 1, - Conditions: status.CreateTestConditions("Test"), + GatewayClassStatuses: status.GatewayClassStatuses{ + {Name: gcName}: { + ObservedGeneration: 1, + Conditions: status.CreateTestConditions("Test"), + }, }, }, ) From acf90b4218bb42c075c5ca7e07dff8d09face926 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Thu, 29 Jun 2023 16:00:47 -0600 Subject: [PATCH 2/5] Use gomega in predicate tests --- .../manager/predicate/gatewayclass_test.go | 19 ++++------ internal/manager/predicate/service_test.go | 37 +++++++------------ 2 files changed, 21 insertions(+), 35 deletions(-) diff --git a/internal/manager/predicate/gatewayclass_test.go b/internal/manager/predicate/gatewayclass_test.go index edcc8f9c7..3413baaa3 100644 --- a/internal/manager/predicate/gatewayclass_test.go +++ b/internal/manager/predicate/gatewayclass_test.go @@ -3,11 +3,14 @@ package predicate import ( "testing" + . "github.com/onsi/gomega" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/gateway-api/apis/v1beta1" ) func TestGatewayClassPredicate(t *testing.T) { + g := NewGomegaWithT(t) + p := GatewayClassPredicate{ControllerName: "nginx-ctlr"} gc := &v1beta1.GatewayClass{ @@ -16,18 +19,10 @@ func TestGatewayClassPredicate(t *testing.T) { }, } - if !p.Create(event.CreateEvent{Object: gc}) { - t.Error("GatewayClassPredicate.Create() returned false; expected true") - } - if !p.Update(event.UpdateEvent{ObjectNew: gc}) { - t.Error("GatewayClassPredicate.Update() returned false; expected true") - } + g.Expect(p.Create(event.CreateEvent{Object: gc})).To(BeTrue()) + g.Expect(p.Update(event.UpdateEvent{ObjectNew: gc})).To(BeTrue()) gc.Spec.ControllerName = "unknown" - if p.Create(event.CreateEvent{Object: gc}) { - t.Error("GatewayClassPredicate.Create() returned true; expected false") - } - if p.Update(event.UpdateEvent{ObjectNew: gc}) { - t.Error("GatewayClassPredicate.Update() returned true; expected false") - } + g.Expect(p.Create(event.CreateEvent{Object: gc})).To(BeFalse()) + g.Expect(p.Update(event.UpdateEvent{ObjectNew: gc})).To(BeFalse()) } diff --git a/internal/manager/predicate/service_test.go b/internal/manager/predicate/service_test.go index b7b80dd65..09526ad40 100644 --- a/internal/manager/predicate/service_test.go +++ b/internal/manager/predicate/service_test.go @@ -3,6 +3,7 @@ package predicate import ( "testing" + . "github.com/onsi/gomega" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/intstr" "sigs.k8s.io/controller-runtime/pkg/client" @@ -224,34 +225,24 @@ func TestServicePortsChangedPredicate_Update(t *testing.T) { p := ServicePortsChangedPredicate{} for _, tc := range testcases { - update := p.Update(event.UpdateEvent{ - ObjectOld: tc.objectOld, - ObjectNew: tc.objectNew, - }) + t.Run(tc.msg, func(t *testing.T) { + g := NewGomegaWithT(t) + update := p.Update(event.UpdateEvent{ + ObjectOld: tc.objectOld, + ObjectNew: tc.objectNew, + }) - if update != tc.expUpdate { - t.Errorf( - "ServicePortsChangedPredicate.Update() mismatch for %q; got %t, expected %t", - tc.msg, - update, - tc.expUpdate, - ) - } + g.Expect(update).To(Equal(tc.expUpdate)) + }) } } func TestServicePortsChangedPredicate(t *testing.T) { - p := ServicePortsChangedPredicate{} - - if !p.Delete(event.DeleteEvent{Object: &v1.Service{}}) { - t.Errorf("ServicePortsChangedPredicate.Delete() returned false; expected true") - } + g := NewGomegaWithT(t) - if !p.Create(event.CreateEvent{Object: &v1.Service{}}) { - t.Errorf("ServicePortsChangedPredicate.Create() returned false; expected true") - } + p := ServicePortsChangedPredicate{} - if !p.Generic(event.GenericEvent{Object: &v1.Service{}}) { - t.Errorf("ServicePortsChangedPredicate.Generic() returned false; expected true") - } + g.Expect(p.Delete(event.DeleteEvent{Object: &v1.Service{}})).To(BeTrue()) + g.Expect(p.Create(event.CreateEvent{Object: &v1.Service{}})).To(BeTrue()) + g.Expect(p.Generic(event.GenericEvent{Object: &v1.Service{}})).To(BeTrue()) } From 9c677a9b802267aee9b66d5919292f94f7a86ed2 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Fri, 30 Jun 2023 10:56:31 -0600 Subject: [PATCH 3/5] Handle updated GatewayClass --- internal/events/handler.go | 9 ++++++++- internal/manager/manager.go | 1 + internal/manager/predicate/gatewayclass.go | 17 +++++++++++------ internal/manager/predicate/gatewayclass_test.go | 10 +++++++--- internal/state/graph/gatewayclass.go | 2 +- 5 files changed, 28 insertions(+), 11 deletions(-) diff --git a/internal/events/handler.go b/internal/events/handler.go index f324d0965..6bbfc90f7 100644 --- a/internal/events/handler.go +++ b/internal/events/handler.go @@ -7,6 +7,7 @@ import ( "github.com/go-logr/logr" apiv1 "k8s.io/api/core/v1" discoveryV1 "k8s.io/api/discovery/v1" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config" @@ -48,6 +49,8 @@ type EventHandlerConfig struct { StatusUpdater status.Updater // Logger is the logger to be used by the EventHandler. Logger logr.Logger + // ControllerName is the name of this controller. + ControllerName string } // EventHandlerImpl implements EventHandler. @@ -121,7 +124,11 @@ func (h *EventHandlerImpl) updateNginx(ctx context.Context, conf dataplane.Confi func (h *EventHandlerImpl) propagateUpsert(e *UpsertEvent) { switch r := e.Resource.(type) { case *v1beta1.GatewayClass: - h.cfg.Processor.CaptureUpsertChange(r) + if string(r.Spec.ControllerName) != h.cfg.ControllerName { + h.cfg.Processor.CaptureDeleteChange(r, client.ObjectKeyFromObject(r)) + } else { + h.cfg.Processor.CaptureUpsertChange(r) + } case *v1beta1.Gateway: h.cfg.Processor.CaptureUpsertChange(r) case *v1beta1.HTTPRoute: diff --git a/internal/manager/manager.go b/internal/manager/manager.go index 994e0e40a..34ba05a60 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -175,6 +175,7 @@ func Start(cfg config.Config) error { NginxFileMgr: nginxFileMgr, NginxRuntimeMgr: nginxRuntimeMgr, StatusUpdater: statusUpdater, + ControllerName: cfg.GatewayCtlrName, }) objects, objectLists := prepareFirstEventBatchPreparerArgs(cfg.GatewayClassName, cfg.GatewayNsName) diff --git a/internal/manager/predicate/gatewayclass.go b/internal/manager/predicate/gatewayclass.go index 13f07e27c..56a792d25 100644 --- a/internal/manager/predicate/gatewayclass.go +++ b/internal/manager/predicate/gatewayclass.go @@ -29,14 +29,19 @@ func (gcp GatewayClassPredicate) Create(e event.CreateEvent) bool { // Update implements default UpdateEvent filter for validating a GatewayClass controllerName. func (gcp GatewayClassPredicate) Update(e event.UpdateEvent) bool { - if e.ObjectNew == nil { - return false + if e.ObjectOld != nil { + gcOld, ok := e.ObjectOld.(*v1beta1.GatewayClass) + if ok { + return string(gcOld.Spec.ControllerName) == gcp.ControllerName + } } - gc, ok := e.ObjectNew.(*v1beta1.GatewayClass) - if !ok { - return false + if e.ObjectNew != nil { + gcNew, ok := e.ObjectNew.(*v1beta1.GatewayClass) + if ok { + return string(gcNew.Spec.ControllerName) == gcp.ControllerName + } } - return string(gc.Spec.ControllerName) == gcp.ControllerName + return false } diff --git a/internal/manager/predicate/gatewayclass_test.go b/internal/manager/predicate/gatewayclass_test.go index 3413baaa3..febcdc008 100644 --- a/internal/manager/predicate/gatewayclass_test.go +++ b/internal/manager/predicate/gatewayclass_test.go @@ -22,7 +22,11 @@ func TestGatewayClassPredicate(t *testing.T) { g.Expect(p.Create(event.CreateEvent{Object: gc})).To(BeTrue()) g.Expect(p.Update(event.UpdateEvent{ObjectNew: gc})).To(BeTrue()) - gc.Spec.ControllerName = "unknown" - g.Expect(p.Create(event.CreateEvent{Object: gc})).To(BeFalse()) - g.Expect(p.Update(event.UpdateEvent{ObjectNew: gc})).To(BeFalse()) + gc2 := &v1beta1.GatewayClass{ + Spec: v1beta1.GatewayClassSpec{ + ControllerName: "unknown", + }, + } + g.Expect(p.Create(event.CreateEvent{Object: gc2})).To(BeFalse()) + g.Expect(p.Update(event.UpdateEvent{ObjectOld: gc, ObjectNew: gc2})).To(BeTrue()) } diff --git a/internal/state/graph/gatewayclass.go b/internal/state/graph/gatewayclass.go index 15c002a3a..8c932939c 100644 --- a/internal/state/graph/gatewayclass.go +++ b/internal/state/graph/gatewayclass.go @@ -28,7 +28,7 @@ type processedGatewayClasses struct { // processGatewayClasses returns the "Winner" GatewayClass, which is defined in // the command-line argument and references this controller, and a list of "Ignored" GatewayClasses // that reference this controller, but are not named in the command-line argument. -// Also returns a boolean that says whether or not the GatewayClasa defined +// Also returns a boolean that says whether or not the GatewayClass defined // in the command-line argument exists, regardless of which controller it references. func processGatewayClasses( gcs map[types.NamespacedName]*v1beta1.GatewayClass, From e8d94632f94be1506a5167596d51ecf59ad05291 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Fri, 30 Jun 2023 11:58:40 -0600 Subject: [PATCH 4/5] Code review --- internal/events/handler.go | 9 +-------- internal/manager/manager.go | 1 - internal/manager/predicate/gatewayclass.go | 8 ++++---- internal/manager/predicate/gatewayclass_test.go | 1 + 4 files changed, 6 insertions(+), 13 deletions(-) diff --git a/internal/events/handler.go b/internal/events/handler.go index 6bbfc90f7..f324d0965 100644 --- a/internal/events/handler.go +++ b/internal/events/handler.go @@ -7,7 +7,6 @@ import ( "github.com/go-logr/logr" apiv1 "k8s.io/api/core/v1" discoveryV1 "k8s.io/api/discovery/v1" - "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config" @@ -49,8 +48,6 @@ type EventHandlerConfig struct { StatusUpdater status.Updater // Logger is the logger to be used by the EventHandler. Logger logr.Logger - // ControllerName is the name of this controller. - ControllerName string } // EventHandlerImpl implements EventHandler. @@ -124,11 +121,7 @@ func (h *EventHandlerImpl) updateNginx(ctx context.Context, conf dataplane.Confi func (h *EventHandlerImpl) propagateUpsert(e *UpsertEvent) { switch r := e.Resource.(type) { case *v1beta1.GatewayClass: - if string(r.Spec.ControllerName) != h.cfg.ControllerName { - h.cfg.Processor.CaptureDeleteChange(r, client.ObjectKeyFromObject(r)) - } else { - h.cfg.Processor.CaptureUpsertChange(r) - } + h.cfg.Processor.CaptureUpsertChange(r) case *v1beta1.Gateway: h.cfg.Processor.CaptureUpsertChange(r) case *v1beta1.HTTPRoute: diff --git a/internal/manager/manager.go b/internal/manager/manager.go index 34ba05a60..994e0e40a 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -175,7 +175,6 @@ func Start(cfg config.Config) error { NginxFileMgr: nginxFileMgr, NginxRuntimeMgr: nginxRuntimeMgr, StatusUpdater: statusUpdater, - ControllerName: cfg.GatewayCtlrName, }) objects, objectLists := prepareFirstEventBatchPreparerArgs(cfg.GatewayClassName, cfg.GatewayNsName) diff --git a/internal/manager/predicate/gatewayclass.go b/internal/manager/predicate/gatewayclass.go index 56a792d25..1a69d86d2 100644 --- a/internal/manager/predicate/gatewayclass.go +++ b/internal/manager/predicate/gatewayclass.go @@ -31,15 +31,15 @@ func (gcp GatewayClassPredicate) Create(e event.CreateEvent) bool { func (gcp GatewayClassPredicate) Update(e event.UpdateEvent) bool { if e.ObjectOld != nil { gcOld, ok := e.ObjectOld.(*v1beta1.GatewayClass) - if ok { - return string(gcOld.Spec.ControllerName) == gcp.ControllerName + if ok && string(gcOld.Spec.ControllerName) == gcp.ControllerName { + return true } } if e.ObjectNew != nil { gcNew, ok := e.ObjectNew.(*v1beta1.GatewayClass) - if ok { - return string(gcNew.Spec.ControllerName) == gcp.ControllerName + if ok && string(gcNew.Spec.ControllerName) == gcp.ControllerName { + return true } } diff --git a/internal/manager/predicate/gatewayclass_test.go b/internal/manager/predicate/gatewayclass_test.go index febcdc008..b56f530af 100644 --- a/internal/manager/predicate/gatewayclass_test.go +++ b/internal/manager/predicate/gatewayclass_test.go @@ -29,4 +29,5 @@ func TestGatewayClassPredicate(t *testing.T) { } g.Expect(p.Create(event.CreateEvent{Object: gc2})).To(BeFalse()) g.Expect(p.Update(event.UpdateEvent{ObjectOld: gc, ObjectNew: gc2})).To(BeTrue()) + g.Expect(p.Update(event.UpdateEvent{ObjectOld: gc2, ObjectNew: gc})).To(BeTrue()) } From e2fa8bebc10943084a861a0d5fe6450bb8c69206 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Fri, 30 Jun 2023 13:41:24 -0600 Subject: [PATCH 5/5] test case --- internal/manager/predicate/gatewayclass_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/manager/predicate/gatewayclass_test.go b/internal/manager/predicate/gatewayclass_test.go index b56f530af..b5d6ada80 100644 --- a/internal/manager/predicate/gatewayclass_test.go +++ b/internal/manager/predicate/gatewayclass_test.go @@ -30,4 +30,5 @@ func TestGatewayClassPredicate(t *testing.T) { g.Expect(p.Create(event.CreateEvent{Object: gc2})).To(BeFalse()) g.Expect(p.Update(event.UpdateEvent{ObjectOld: gc, ObjectNew: gc2})).To(BeTrue()) g.Expect(p.Update(event.UpdateEvent{ObjectOld: gc2, ObjectNew: gc})).To(BeTrue()) + g.Expect(p.Update(event.UpdateEvent{ObjectOld: gc2, ObjectNew: gc2})).To(BeFalse()) }