From 6e91b7025ba7be3e812f91d703f10479d91cc6ea Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Mon, 19 Feb 2024 17:29:48 -0800 Subject: [PATCH] use list instead of map for Provider Resources * list adds order stability Signed-off-by: Arko Dasgupta --- internal/gatewayapi/resource.go | 35 ------------------ internal/gatewayapi/runner/runner.go | 6 ++-- internal/message/types.go | 8 +++-- internal/provider/kubernetes/controller.go | 41 ++++++++++++---------- internal/provider/kubernetes/predicates.go | 26 ++++++++------ 5 files changed, 46 insertions(+), 70 deletions(-) diff --git a/internal/gatewayapi/resource.go b/internal/gatewayapi/resource.go index a7a16f664b41..0cea818a6376 100644 --- a/internal/gatewayapi/resource.go +++ b/internal/gatewayapi/resource.go @@ -20,41 +20,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. diff --git a/internal/gatewayapi/runner/runner.go b/internal/gatewayapi/runner/runner.go index a40cdaedef5e..1365d775a998 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.Resources], errChan chan error) { r.Logger.Info("received an update") val := update.Value if update.Delete || val == nil { @@ -62,11 +62,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 1328c2a853c5..5034cad1f896 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.Resources] // GatewayAPIStatuses is a group of gateway api // resource statuses maps. @@ -31,13 +31,15 @@ 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 } diff --git a/internal/provider/kubernetes/controller.go b/internal/provider/kubernetes/controller.go index 8d53b7f071cb..a8ddf3dcc854 100644 --- a/internal/provider/kubernetes/controller.go +++ b/internal/provider/kubernetes/controller.go @@ -171,14 +171,16 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques return reconcile.Result{}, nil } - resourcesMap := make(gatewayapi.GatewayClassResources) + gwcResources := make([]*gatewayapi.Resources, 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 } @@ -197,7 +199,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)) } @@ -211,7 +213,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)) } @@ -234,14 +236,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 @@ -256,7 +258,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 @@ -270,12 +272,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{} @@ -288,7 +290,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 @@ -302,11 +304,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. @@ -320,12 +322,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") @@ -335,8 +337,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 { @@ -344,7 +346,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. @@ -365,7 +367,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/predicates.go b/internal/provider/kubernetes/predicates.go index 5a042d1cacfc..417dcce150dd 100644 --- a/internal/provider/kubernetes/predicates.go +++ b/internal/provider/kubernetes/predicates.go @@ -223,12 +223,15 @@ func (r *gatewayAPIReconciler) validateServiceForReconcile(obj client.Object) bo // Merged gateways will have only this label, update status of all Gateways under found GatewayClass. gclass, ok := labels[gatewayapi.OwningGatewayClassLabel] if ok && r.mergeGateways[gclass] { - res, _ := r.resources.GatewayAPIResources.Load(string(r.classController)) + res := r.resources.GetResources() if res != nil { - if (*res)[gclass] != nil && len((*res)[gclass].Gateways) > 0 { - for _, gw := range (*res)[gclass].Gateways { - gw := gw - r.updateStatusForGateway(ctx, gw) + for _, gwcRes := range res { + if (gwcRes != nil) && (gwcRes.GatewayClass.Name == gclass) && len(gwcRes.Gateways) > 0 { + for _, gw := range gwcRes.Gateways { + gw := gw + r.updateStatusForGateway(ctx, gw) + } + break } } } @@ -380,12 +383,15 @@ func (r *gatewayAPIReconciler) validateDeploymentForReconcile(obj client.Object) // Merged gateways will have only this label, update status of all Gateways under found GatewayClass. gclass, ok := labels[gatewayapi.OwningGatewayClassLabel] if ok && r.mergeGateways[gclass] { - res, _ := r.resources.GatewayAPIResources.Load(string(r.classController)) + res := r.resources.GetResources() if res != nil { - if (*res)[gclass] != nil && len((*res)[gclass].Gateways) > 0 { - for _, gw := range (*res)[gclass].Gateways { - gw := gw - r.updateStatusForGateway(ctx, gw) + for _, gwcRes := range res { + if (gwcRes != nil) && (gwcRes.GatewayClass.Name == gclass) && len(gwcRes.Gateways) > 0 { + for _, gw := range gwcRes.Gateways { + gw := gw + r.updateStatusForGateway(ctx, gw) + } + break } } }