diff --git a/changelogs/unreleased/4912-fangfpeng-small.md b/changelogs/unreleased/4912-fangfpeng-small.md new file mode 100644 index 00000000000..f8f85e67657 --- /dev/null +++ b/changelogs/unreleased/4912-fangfpeng-small.md @@ -0,0 +1 @@ +Don't trigger DAG rebuilds/xDS configuration updates for irrelevant HTTPRoutes and TLSRoutes. \ No newline at end of file diff --git a/internal/dag/cache.go b/internal/dag/cache.go index 543a7b017a0..861882a0635 100644 --- a/internal/dag/cache.go +++ b/internal/dag/cache.go @@ -22,6 +22,7 @@ import ( contour_api_v1 "github.com/projectcontour/contour/apis/projectcontour/v1" contour_api_v1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1" "github.com/projectcontour/contour/internal/annotation" + "github.com/projectcontour/contour/internal/gatewayapi" "github.com/projectcontour/contour/internal/ingressclass" "github.com/projectcontour/contour/internal/k8s" "github.com/projectcontour/contour/internal/ref" @@ -186,10 +187,10 @@ func (kc *KubernetesCache) Insert(obj interface{}) bool { } case *gatewayapi_v1beta1.HTTPRoute: kc.httproutes[k8s.NamespacedNameOf(obj)] = obj - return true + return kc.routeTriggersRebuild(obj.Spec.ParentRefs) case *gatewayapi_v1alpha2.TLSRoute: kc.tlsroutes[k8s.NamespacedNameOf(obj)] = obj - return true + return kc.routeTriggersRebuild(obj.Spec.ParentRefs) case *gatewayapi_v1beta1.ReferenceGrant: kc.referencegrants[k8s.NamespacedNameOf(obj)] = obj return true @@ -304,14 +305,12 @@ func (kc *KubernetesCache) remove(obj interface{}) bool { } case *gatewayapi_v1beta1.HTTPRoute: m := k8s.NamespacedNameOf(obj) - _, ok := kc.httproutes[m] delete(kc.httproutes, m) - return ok + return kc.routeTriggersRebuild(obj.Spec.ParentRefs) case *gatewayapi_v1alpha2.TLSRoute: m := k8s.NamespacedNameOf(obj) - _, ok := kc.tlsroutes[m] delete(kc.tlsroutes, m) - return ok + return kc.routeTriggersRebuild(obj.Spec.ParentRefs) case *gatewayapi_v1beta1.ReferenceGrant: m := k8s.NamespacedNameOf(obj) _, ok := kc.referencegrants[m] @@ -514,6 +513,21 @@ func isRefToSecret(ref gatewayapi_v1beta1.SecretObjectReference, secret *v1.Secr string(ref.Name) == secret.Name } +// routesTriggersRebuild returns true if this route references gateway in this cache. +func (kc *KubernetesCache) routeTriggersRebuild(parentRefs []gatewayapi_v1beta1.ParentReference) bool { + if kc.gateway == nil { + return false + } + + for _, parentRef := range parentRefs { + if gatewayapi.IsRefToGateway(parentRef, k8s.NamespacedNameOf(kc.gateway)) { + return true + } + } + + return false +} + func (kc *KubernetesCache) LookupTLSSecret(name types.NamespacedName) (*Secret, error) { sec, ok := kc.secrets[name] if !ok { diff --git a/internal/dag/cache_test.go b/internal/dag/cache_test.go index 714b944bdc1..287ebca2726 100644 --- a/internal/dag/cache_test.go +++ b/internal/dag/cache_test.go @@ -868,21 +868,69 @@ func TestKubernetesCacheInsert(t *testing.T) { }, want: true, }, - "insert gateway-api HTTPRoute": { + "insert gateway-api HTTPRoute, no reference to Gateway": { obj: &gatewayapi_v1beta1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ Name: "httproute", Namespace: "default", }, }, + want: false, + }, + "insert gateway-api HTTPRoute, has reference to Gateway": { + pre: []interface{}{ + &gatewayapi_v1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "gateway-namespace", + Name: "gateway-name", + }, + }, + }, + obj: &gatewayapi_v1beta1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "httproute", + Namespace: "default", + }, + Spec: gatewayapi_v1beta1.HTTPRouteSpec{ + CommonRouteSpec: gatewayapi_v1beta1.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1beta1.ParentReference{ + gatewayapi.GatewayParentRef("gateway-namespace", "gateway-name"), + }, + }, + }, + }, want: true, }, - "insert gateway-api TLSRoute": { + "insert gateway-api TLSRoute, no reference to Gateway": { + obj: &gatewayapi_v1alpha2.TLSRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tlsroute", + Namespace: "default", + }, + }, + want: false, + }, + "insert gateway-api TLSRoute, has reference to Gateway": { + pre: []interface{}{ + &gatewayapi_v1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "gateway-namespace", + Name: "gateway-name", + }, + }, + }, obj: &gatewayapi_v1alpha2.TLSRoute{ ObjectMeta: metav1.ObjectMeta{ Name: "tlsroute", Namespace: "default", }, + Spec: gatewayapi_v1alpha2.TLSRouteSpec{ + CommonRouteSpec: gatewayapi_v1alpha2.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1alpha2.ParentReference{ + gatewayapi.GatewayParentRef("gateway-namespace", "gateway-name"), + }, + }, + }, }, want: true, }, @@ -1285,33 +1333,114 @@ func TestKubernetesCacheRemove(t *testing.T) { }, want: true, }, - "remove gateway-api HTTPRoute": { - cache: cache(&gatewayapi_v1beta1.HTTPRoute{ + "remove gateway-api HTTPRoute with no parentRef": { + cache: cache(&gatewayapi_v1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "Gateway", + Namespace: "default", + }}, + &gatewayapi_v1beta1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "httproute", + Namespace: "default", + }, + }, + ), + obj: &gatewayapi_v1beta1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ Name: "httproute", Namespace: "default", }, - }), + }, + want: false, + }, + "remove gateway-api HTTPRoute with parentRef": { + cache: cache(&gatewayapi_v1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway", + Namespace: "default", + }}, + &gatewayapi_v1beta1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "httproute", + Namespace: "default", + }, + Spec: gatewayapi_v1beta1.HTTPRouteSpec{ + CommonRouteSpec: gatewayapi_v1beta1.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1beta1.ParentReference{ + gatewayapi.GatewayParentRef("default", "gateway"), + }, + }, + }, + }, + ), obj: &gatewayapi_v1beta1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ Name: "httproute", Namespace: "default", }, + Spec: gatewayapi_v1beta1.HTTPRouteSpec{ + CommonRouteSpec: gatewayapi_v1beta1.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1beta1.ParentReference{ + gatewayapi.GatewayParentRef("default", "gateway"), + }, + }, + }, }, want: true, }, - "remove gateway-api TLSRoute": { - cache: cache(&gatewayapi_v1alpha2.TLSRoute{ + "remove gateway-api TLSRoute with no parentRef": { + cache: cache(&gatewayapi_v1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "Gateway", + Namespace: "default", + }}, + &gatewayapi_v1alpha2.TLSRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tlsroute", + Namespace: "default", + }, + }), + obj: &gatewayapi_v1alpha2.TLSRoute{ ObjectMeta: metav1.ObjectMeta{ Name: "tlsroute", Namespace: "default", }, - }), + }, + want: false, + }, + "remove gateway-api TLSRoute with parentRef": { + cache: cache(&gatewayapi_v1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway", + Namespace: "default", + }}, + &gatewayapi_v1alpha2.TLSRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tlsroute", + Namespace: "default", + }, + Spec: gatewayapi_v1alpha2.TLSRouteSpec{ + CommonRouteSpec: gatewayapi_v1alpha2.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1alpha2.ParentReference{ + gatewayapi.GatewayParentRef("default", "gateway"), + }, + }, + }, + }, + ), obj: &gatewayapi_v1alpha2.TLSRoute{ ObjectMeta: metav1.ObjectMeta{ Name: "tlsroute", Namespace: "default", }, + Spec: gatewayapi_v1alpha2.TLSRouteSpec{ + CommonRouteSpec: gatewayapi_v1alpha2.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1alpha2.ParentReference{ + gatewayapi.GatewayParentRef("default", "gateway"), + }, + }, + }, }, want: true, }, @@ -2074,3 +2203,168 @@ func TestSecretTriggersRebuild(t *testing.T) { }) } } + +func TestRouteTriggersRebuild(t *testing.T) { + + cache := func(objs ...interface{}) *KubernetesCache { + cache := KubernetesCache{ + FieldLogger: fixture.NewTestLogger(t), + } + for _, o := range objs { + cache.Insert(o) + } + return &cache + } + + httpRoute := func(namespace, name, parentRefNamespace, parentRefName string) *gatewayapi_v1beta1.HTTPRoute { + return &gatewayapi_v1beta1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: gatewayapi_v1beta1.HTTPRouteSpec{ + CommonRouteSpec: gatewayapi_v1beta1.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1beta1.ParentReference{ + gatewayapi.GatewayParentRef(parentRefNamespace, parentRefName), + }, + }, + Rules: []gatewayapi_v1beta1.HTTPRouteRule{{ + BackendRefs: gatewayapi.HTTPBackendRef(name, 80, 1), + }}, + }, + } + } + + tlsRoute := func(namespace, name, parentRefNamespace, parentRefName string) *gatewayapi_v1alpha2.TLSRoute { + return &gatewayapi_v1alpha2.TLSRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: gatewayapi_v1alpha2.TLSRouteSpec{ + CommonRouteSpec: gatewayapi_v1alpha2.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1alpha2.ParentReference{ + gatewayapi.GatewayParentRef(parentRefNamespace, parentRefName), + }, + }, + Rules: []gatewayapi_v1alpha2.TLSRouteRule{{ + BackendRefs: gatewayapi.TLSRouteBackendRef(name, 80, nil), + }}, + }, + } + } + + gateway := func(namespace, name string) *gatewayapi_v1beta1.Gateway { + return &gatewayapi_v1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + } + } + + tests := map[string]struct { + cache *KubernetesCache + httproute *gatewayapi_v1beta1.HTTPRoute + tlsroute *gatewayapi_v1alpha2.TLSRoute + want bool + }{ + "httproute empty cache does not trigger rebuild": { + cache: cache(), + httproute: httpRoute("default", "httproute", "default", "gateway"), + want: false, + }, + "httproute with empty parentRef does not trigger rebuild": { + cache: cache( + gateway("default", "gateway"), + ), + httproute: &gatewayapi_v1beta1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "httproute", + Namespace: "default", + }, + Spec: gatewayapi_v1beta1.HTTPRouteSpec{ + Rules: []gatewayapi_v1beta1.HTTPRouteRule{{ + BackendRefs: gatewayapi.HTTPBackendRef("httproute", 80, 1), + }}, + }, + }, + want: false, + }, + "httproute with unmatching gateway namespace in parentref does not trigger rebuild": { + cache: cache( + gateway("default", "gateway"), + ), + httproute: httpRoute("default", "httproute", "gateway-unmatching-namespace", "gateway"), + want: false, + }, + "httproute with unmatching gateway name in parentref does not trigger rebuild": { + cache: cache( + gateway("default", "gateway"), + ), + httproute: httpRoute("default", "httproute", "default", "gateway-unmatching-name"), + want: false, + }, + "httproute with matching gateway parentref triggers rebuild": { + cache: cache( + gateway("default", "gateway"), + ), + httproute: httpRoute("default", "httproute", "default", "gateway"), + want: true, + }, + "tlsroute empty cache does not trigger rebuild": { + cache: cache(), + tlsroute: tlsRoute("default", "tlsroute", "default", "gateway"), + want: false, + }, + "tlsroute with empty parentRef does not trigger rebuild": { + cache: cache( + gateway("default", "gateway"), + ), + tlsroute: &gatewayapi_v1alpha2.TLSRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tlsroute", + Namespace: "default", + }, + Spec: gatewayapi_v1alpha2.TLSRouteSpec{ + Rules: []gatewayapi_v1alpha2.TLSRouteRule{{ + BackendRefs: gatewayapi.TLSRouteBackendRef("tlsroute", 80, nil), + }}, + }, + }, + want: false, + }, + "tlsroute with unmatching gateway namespace parentref does not trigger rebuild": { + cache: cache( + gateway("default", "gateway"), + ), + tlsroute: tlsRoute("default", "tlsroute", "gateway-unmatching-namespace", "gateway"), + want: false, + }, + "tlsroute with unmatching gateway name parentref does not trigger rebuild": { + cache: cache( + gateway("default", "gateway"), + ), + tlsroute: tlsRoute("default", "tlsroute", "default", "gateway-unmatching-name"), + want: false, + }, + "tlsroute with matching gateway parentref triggers rebuild": { + cache: cache( + gateway("default", "gateway"), + ), + tlsroute: tlsRoute("default", "tlsroute", "default", "gateway"), + want: true, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + if tc.httproute != nil { + assert.Equal(t, tc.want, tc.cache.routeTriggersRebuild(tc.httproute.Spec.ParentRefs)) + } + if tc.tlsroute != nil { + assert.Equal(t, tc.want, tc.cache.routeTriggersRebuild(tc.tlsroute.Spec.ParentRefs)) + } + }) + } +}