diff --git a/internal/gatewayapi/resource.go b/internal/gatewayapi/resource.go index a7a16f664b4..f855cc3bb11 100644 --- a/internal/gatewayapi/resource.go +++ b/internal/gatewayapi/resource.go @@ -6,6 +6,10 @@ package gatewayapi import ( + "cmp" + "reflect" + + "golang.org/x/exp/slices" v1 "k8s.io/api/core/v1" discoveryv1 "k8s.io/api/discovery/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -20,41 +24,6 @@ import ( type XdsIRMap map[string]*ir.Xds type InfraIRMap map[string]*ir.Infra -type GatewayClassResources map[string]*Resources - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -// This was generated by controller-gen and moved over from -// zz_generated.deepcopy.go to this file. -func (in GatewayClassResources) DeepCopyInto(out *GatewayClassResources) { - { - in := &in - *out = make(GatewayClassResources, len(*in)) - for key, val := range *in { - var outVal *Resources - if val == nil { - (*out)[key] = nil - } else { - inVal := (*in)[key] - in, out := &inVal, &outVal - *out = new(Resources) - (*in).DeepCopyInto(*out) - } - (*out)[key] = outVal - } - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GatewayClassResources. -// This was generated by controller-gen and moved over from -// zz_generated.deepcopy.go to this file. -func (in GatewayClassResources) DeepCopy() *GatewayClassResources { - if in == nil { - return nil - } - out := new(GatewayClassResources) - in.DeepCopyInto(out) - return out -} // Resources holds the Gateway API and related // resources that the translators needs as inputs. @@ -171,3 +140,33 @@ func (r *Resources) GetEndpointSlicesForBackend(svcNamespace, svcName string, ba } return endpointSlices } + +// ControllerResources holds all the GatewayAPI resources per GatewayClass +type ControllerResources []*Resources + +// DeepCopy creates a new ControllerResources. +// It is handwritten since the tooling was unable to copy into a new slice +func (c *ControllerResources) DeepCopy() *ControllerResources { + if c == nil { + return nil + } + out := make(ControllerResources, len(*c)) + copy(out, *c) + return &out +} + +// Equal implements the Comparable interface used by watchable.DeepEqual to skip unnecessary updates. +func (c *ControllerResources) Equal(y *ControllerResources) bool { + // Deep copy to avoid modifying the original ordering. + c = c.DeepCopy() + c.sort() + y = y.DeepCopy() + y.sort() + return reflect.DeepEqual(c, y) +} + +func (c *ControllerResources) sort() { + slices.SortFunc(*c, func(c1, c2 *Resources) int { + return cmp.Compare(c1.GatewayClass.Name, c2.GatewayClass.Name) + }) +} diff --git a/internal/gatewayapi/resource_test.go b/internal/gatewayapi/resource_test.go new file mode 100644 index 00000000000..8b9fddc0fcd --- /dev/null +++ b/internal/gatewayapi/resource_test.go @@ -0,0 +1,125 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +package gatewayapi + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +func TestEqualXds(t *testing.T) { + tests := []struct { + desc string + a *ControllerResources + b *ControllerResources + equal bool + }{ + { + desc: "different resources", + a: &ControllerResources{ + { + GatewayClass: &gwapiv1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + }, + }, + }, + b: &ControllerResources{ + { + GatewayClass: &gwapiv1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + }, + }, + }, + }, + equal: false, + }, + { + desc: "same order resources are equal", + a: &ControllerResources{ + { + GatewayClass: &gwapiv1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + }, + }, + { + GatewayClass: &gwapiv1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + }, + }, + }, + }, + b: &ControllerResources{ + { + GatewayClass: &gwapiv1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + }, + }, + { + GatewayClass: &gwapiv1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + }, + }, + }, + }, + equal: true, + }, + { + desc: "out of order resources are equal", + a: &ControllerResources{ + { + GatewayClass: &gwapiv1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + }, + }, + { + GatewayClass: &gwapiv1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + }, + }, + }, + }, + b: &ControllerResources{ + { + GatewayClass: &gwapiv1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + }, + }, + }, + { + GatewayClass: &gwapiv1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + }, + }, + }, + equal: true, + }, + } + + for _, tc := range tests { + t.Run(tc.desc, func(t *testing.T) { + require.Equal(t, tc.equal, cmp.Equal(tc.a, tc.b)) + }) + } +} diff --git a/internal/gatewayapi/runner/runner.go b/internal/gatewayapi/runner/runner.go index ebc5d612d25..a9cc6e555c6 100644 --- a/internal/gatewayapi/runner/runner.go +++ b/internal/gatewayapi/runner/runner.go @@ -49,7 +49,7 @@ func (r *Runner) Start(ctx context.Context) (err error) { func (r *Runner) subscribeAndTranslate(ctx context.Context) { message.HandleSubscription(message.Metadata{Runner: string(v1alpha1.LogComponentGatewayAPIRunner), Message: "provider-resources"}, r.ProviderResources.GatewayAPIResources.Subscribe(ctx), - func(update message.Update[string, *gatewayapi.GatewayClassResources], errChan chan error) { + func(update message.Update[string, *gatewayapi.ControllerResources], errChan chan error) { r.Logger.Info("received an update") val := update.Value // There is only 1 key which is the controller name @@ -65,11 +65,11 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) { curKeys = append(curKeys, key) } - for gc, resources := range *val { + for _, resources := range *val { // Translate and publish IRs. t := &gatewayapi.Translator{ GatewayControllerName: r.Server.EnvoyGateway.Gateway.ControllerName, - GatewayClassName: v1.ObjectName(gc), + GatewayClassName: v1.ObjectName(resources.GatewayClass.Name), GlobalRateLimitEnabled: r.EnvoyGateway.RateLimit != nil, EnvoyPatchPolicyEnabled: r.EnvoyGateway.ExtensionAPIs != nil && r.EnvoyGateway.ExtensionAPIs.EnableEnvoyPatchPolicy, } diff --git a/internal/message/types.go b/internal/message/types.go index 1328c2a853c..2d7fdfc2c6e 100644 --- a/internal/message/types.go +++ b/internal/message/types.go @@ -21,7 +21,7 @@ import ( type ProviderResources struct { // GatewayAPIResources is a map from a GatewayClass name to // a group of gateway API and other related resources. - GatewayAPIResources watchable.Map[string, *gatewayapi.GatewayClassResources] + GatewayAPIResources watchable.Map[string, *gatewayapi.ControllerResources] // GatewayAPIStatuses is a group of gateway api // resource statuses maps. @@ -31,13 +31,25 @@ type ProviderResources struct { PolicyStatuses } -func (p *ProviderResources) GetResources() *gatewayapi.GatewayClassResources { +func (p *ProviderResources) GetResources() []*gatewayapi.Resources { if p.GatewayAPIResources.Len() == 0 { return nil } + for _, v := range p.GatewayAPIResources.LoadAll() { - return v + return *v } + + return nil +} + +func (p *ProviderResources) GetResourcesByGatewayClass(name string) *gatewayapi.Resources { + for _, r := range p.GetResources() { + if r != nil && r.GatewayClass != nil && r.GatewayClass.Name == name { + return r + } + } + return nil } diff --git a/internal/provider/kubernetes/controller.go b/internal/provider/kubernetes/controller.go index b7d305caa83..1d700daf034 100644 --- a/internal/provider/kubernetes/controller.go +++ b/internal/provider/kubernetes/controller.go @@ -169,14 +169,16 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques return reconcile.Result{}, nil } - resourcesMap := make(gatewayapi.GatewayClassResources) + gwcResources := make(gatewayapi.ControllerResources, 0, len(acceptedGCs)) for _, acceptedGC := range acceptedGCs { // Initialize resource types. acceptedGC := acceptedGC - resourcesMap[acceptedGC.Name] = gatewayapi.NewResources() + gwcResource := gatewayapi.NewResources() + gwcResource.GatewayClass = acceptedGC + gwcResources = append(gwcResources, gwcResource) resourceMappings := newResourceMapping() - if err := r.processGateways(ctx, acceptedGC, resourceMappings, resourcesMap[acceptedGC.Name]); err != nil { + if err := r.processGateways(ctx, acceptedGC, resourceMappings, gwcResource); err != nil { return reconcile.Result{}, err } @@ -195,7 +197,7 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques "name", string(backendRef.Name)) } else { resourceMappings.allAssociatedNamespaces[service.Namespace] = struct{}{} - resourcesMap[acceptedGC.Name].Services = append(resourcesMap[acceptedGC.Name].Services, service) + gwcResource.Services = append(gwcResource.Services, service) r.log.Info("added Service to resource tree", "namespace", string(*backendRef.Namespace), "name", string(backendRef.Name)) } @@ -209,7 +211,7 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques "name", string(backendRef.Name)) } else { resourceMappings.allAssociatedNamespaces[serviceImport.Namespace] = struct{}{} - resourcesMap[acceptedGC.Name].ServiceImports = append(resourcesMap[acceptedGC.Name].ServiceImports, serviceImport) + gwcResource.ServiceImports = append(gwcResource.ServiceImports, serviceImport) r.log.Info("added ServiceImport to resource tree", "namespace", string(*backendRef.Namespace), "name", string(backendRef.Name)) } @@ -232,14 +234,14 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques endpointSlice := endpointSlice r.log.Info("added EndpointSlice to resource tree", "namespace", endpointSlice.Namespace, "name", endpointSlice.Name) - resourcesMap[acceptedGC.Name].EndpointSlices = append(resourcesMap[acceptedGC.Name].EndpointSlices, &endpointSlice) + gwcResource.EndpointSlices = append(gwcResource.EndpointSlices, &endpointSlice) } } } // Add all ReferenceGrants to the resourceTree for _, referenceGrant := range resourceMappings.allAssociatedRefGrants { - resourcesMap[acceptedGC.Name].ReferenceGrants = append(resourcesMap[acceptedGC.Name].ReferenceGrants, referenceGrant) + gwcResource.ReferenceGrants = append(gwcResource.ReferenceGrants, referenceGrant) } // Add all EnvoyPatchPolicies @@ -254,7 +256,7 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques // It will be recomputed by the gateway-api layer policy.Status = egv1a1.EnvoyPatchPolicyStatus{} - resourcesMap[acceptedGC.Name].EnvoyPatchPolicies = append(resourcesMap[acceptedGC.Name].EnvoyPatchPolicies, &policy) + gwcResource.EnvoyPatchPolicies = append(gwcResource.EnvoyPatchPolicies, &policy) } // Add all ClientTrafficPolicies @@ -268,12 +270,12 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques // Discard Status to reduce memory consumption in watchable // It will be recomputed by the gateway-api layer policy.Status = egv1a1.ClientTrafficPolicyStatus{} - resourcesMap[acceptedGC.Name].ClientTrafficPolicies = append(resourcesMap[acceptedGC.Name].ClientTrafficPolicies, &policy) + gwcResource.ClientTrafficPolicies = append(gwcResource.ClientTrafficPolicies, &policy) } // Add the referenced ConfigMaps in ClientTrafficPolicies to the resourceTree - r.processCtpConfigMapRefs(ctx, resourcesMap[acceptedGC.Name], resourceMappings) + r.processCtpConfigMapRefs(ctx, gwcResource, resourceMappings) // Add all BackendTrafficPolicies backendTrafficPolicies := egv1a1.BackendTrafficPolicyList{} @@ -286,7 +288,7 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques // Discard Status to reduce memory consumption in watchable // It will be recomputed by the gateway-api layer policy.Status = egv1a1.BackendTrafficPolicyStatus{} - resourcesMap[acceptedGC.Name].BackendTrafficPolicies = append(resourcesMap[acceptedGC.Name].BackendTrafficPolicies, &policy) + gwcResource.BackendTrafficPolicies = append(gwcResource.BackendTrafficPolicies, &policy) } // Add all SecurityPolicies @@ -300,11 +302,11 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques // Discard Status to reduce memory consumption in watchable // It will be recomputed by the gateway-api layer policy.Status = egv1a1.SecurityPolicyStatus{} - resourcesMap[acceptedGC.Name].SecurityPolicies = append(resourcesMap[acceptedGC.Name].SecurityPolicies, &policy) + gwcResource.SecurityPolicies = append(gwcResource.SecurityPolicies, &policy) } // Add the referenced Secrets in SecurityPolicies to the resourceTree - r.processSecurityPolicySecretRefs(ctx, resourcesMap[acceptedGC.Name], resourceMappings) + r.processSecurityPolicySecretRefs(ctx, gwcResource, resourceMappings) // For this particular Gateway, and all associated objects, check whether the // namespace exists. Add to the resourceTree. @@ -318,12 +320,12 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques return reconcile.Result{}, err } - resourcesMap[acceptedGC.Name].Namespaces = append(resourcesMap[acceptedGC.Name].Namespaces, namespace) + gwcResource.Namespaces = append(gwcResource.Namespaces, namespace) } // Process the parametersRef of the accepted GatewayClass. if acceptedGC.Spec.ParametersRef != nil && acceptedGC.DeletionTimestamp == nil { - if err := r.processParamsRef(ctx, acceptedGC, resourcesMap[acceptedGC.Name]); err != nil { + if err := r.processParamsRef(ctx, acceptedGC, gwcResource); err != nil { msg := fmt.Sprintf("%s: %v", status.MsgGatewayClassInvalidParams, err) if err := r.updateStatusForGatewayClass(ctx, acceptedGC, false, string(gwapiv1.GatewayClassReasonInvalidParameters), msg); err != nil { r.log.Error(err, "unable to update GatewayClass status") @@ -333,8 +335,8 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques } } - if resourcesMap[acceptedGC.Name].EnvoyProxy != nil && resourcesMap[acceptedGC.Name].EnvoyProxy.Spec.MergeGateways != nil { - r.mergeGateways[acceptedGC.Name] = *resourcesMap[acceptedGC.Name].EnvoyProxy.Spec.MergeGateways + if gwcResource.EnvoyProxy != nil && gwcResource.EnvoyProxy.Spec.MergeGateways != nil { + r.mergeGateways[acceptedGC.Name] = *gwcResource.EnvoyProxy.Spec.MergeGateways } if err := r.updateStatusForGatewayClass(ctx, acceptedGC, true, string(gwapiv1.GatewayClassReasonAccepted), status.MsgValidGatewayClass); err != nil { @@ -342,7 +344,7 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques return reconcile.Result{}, err } - if len(resourcesMap[acceptedGC.Name].Gateways) == 0 { + if len(gwcResource.Gateways) == 0 { r.log.Info("No gateways found for accepted gatewayclass") // If needed, remove the finalizer from the accepted GatewayClass. @@ -363,7 +365,8 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques // The Store is triggered even when there are no Gateways associated to the // GatewayClass. This would happen in case the last Gateway is removed and the // Store will be required to trigger a cleanup of envoy infra resources. - r.resources.GatewayAPIResources.Store(string(r.classController), resourcesMap.DeepCopy()) + + r.resources.GatewayAPIResources.Store(string(r.classController), &gwcResources) r.log.Info("reconciled gateways successfully") return reconcile.Result{}, nil diff --git a/internal/provider/kubernetes/kubernetes_test.go b/internal/provider/kubernetes/kubernetes_test.go index fc4cc7a39a0..350818cb182 100644 --- a/internal/provider/kubernetes/kubernetes_test.go +++ b/internal/provider/kubernetes/kubernetes_test.go @@ -366,10 +366,8 @@ func testGatewayScheduledStatus(ctx context.Context, t *testing.T, provider *Pro return cli.Get(ctx, key, gw) == nil }, defaultWait, defaultTick) - gatewayClassResources, _ := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) - assert.NotNil(t, gatewayClassResources) - - res := (*gatewayClassResources)[gc.Name] + res := resources.GetResourcesByGatewayClass(gc.Name) + assert.NotNil(t, res) // Only check if the spec is equal // The watchable map will not store a resource // with an updated status if the spec has not changed @@ -903,10 +901,8 @@ func testHTTPRoute(ctx context.Context, t *testing.T, provider *Provider, resour return ok && len(res.HTTPRoutes) != 0 }, defaultWait, defaultTick) - gatewayClassResources, _ := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) - assert.NotNil(t, gatewayClassResources) - - res := (*gatewayClassResources)[gc.Name] + res := resources.GetResourcesByGatewayClass(gc.Name) + assert.NotNil(t, res) assert.Equal(t, &testCase.route, res.HTTPRoutes[0]) // Ensure the HTTPRoute Namespace is in the Namespace resource map. @@ -1054,10 +1050,8 @@ func testTLSRoute(ctx context.Context, t *testing.T, provider *Provider, resourc return ok && len(res.TLSRoutes) != 0 }, defaultWait, defaultTick) - gatewayClassResources, _ := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) - assert.NotNil(t, gatewayClassResources) - - res, _ := (*gatewayClassResources)[gc.Name] + res := resources.GetResourcesByGatewayClass(gc.Name) + assert.NotNil(t, res) assert.Equal(t, &testCase.route, res.TLSRoutes[0]) // Ensure the HTTPRoute Namespace is in the Namespace resource map. @@ -1593,13 +1587,8 @@ func TestNamespaceSelectorProvider(t *testing.T) { } func waitUntilGatewayClassResourcesAreReady(resources *message.ProviderResources, gatewayClassName string) (*gatewayapi.Resources, bool) { - gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) - if !ok { - return nil, false - } - - res, ok := (*gatewayClassResources)[gatewayClassName] - if !ok { + res := resources.GetResourcesByGatewayClass(gatewayClassName) + if res == nil { return nil, false }