From a5125bfac07d56893d2c8a0c05b7df08d1b911a5 Mon Sep 17 00:00:00 2001 From: Karol Szwaj Date: Sat, 17 Feb 2024 03:24:11 +0100 Subject: [PATCH] feat: support multiple GatewayClass per controller (#2298) * reconcile multiple gatewayclasses per controller Signed-off-by: Karol Szwaj * filter gateway-api infra layer by gc label Signed-off-by: Karol Szwaj * wip: modify watchable message for gatewayapi translate Signed-off-by: Karol Szwaj * gen deepcopy for gatewayclassresources Signed-off-by: Karol Szwaj * add comments to deepcopy gen Signed-off-by: Karol Szwaj * fix store order Signed-off-by: Karol Szwaj --------- Signed-off-by: Karol Szwaj Co-authored-by: zirain --- internal/gatewayapi/resource.go | 35 ++ internal/gatewayapi/runner/runner.go | 177 +++++----- internal/message/types.go | 4 +- internal/provider/kubernetes/controller.go | 334 +++++++++--------- internal/provider/kubernetes/helpers.go | 51 --- internal/provider/kubernetes/helpers_test.go | 95 ----- .../provider/kubernetes/kubernetes_test.go | 97 +++-- internal/provider/kubernetes/predicates.go | 37 +- test/e2e/testdata/multiple-gc.yaml | 191 ++++++++++ test/e2e/tests/multiple-gc.go | 69 ++++ 10 files changed, 643 insertions(+), 447 deletions(-) create mode 100644 test/e2e/testdata/multiple-gc.yaml create mode 100644 test/e2e/tests/multiple-gc.go diff --git a/internal/gatewayapi/resource.go b/internal/gatewayapi/resource.go index 0cea818a637..a7a16f664b4 100644 --- a/internal/gatewayapi/resource.go +++ b/internal/gatewayapi/resource.go @@ -20,6 +20,41 @@ 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 b930b887a62..a40cdaedef5 100644 --- a/internal/gatewayapi/runner/runner.go +++ b/internal/gatewayapi/runner/runner.go @@ -49,63 +49,110 @@ 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.Resources], errChan chan error) { + func(update message.Update[string, *gatewayapi.GatewayClassResources], errChan chan error) { r.Logger.Info("received an update") - val := update.Value - if update.Delete || val == nil { return } - // Translate and publish IRs. - t := &gatewayapi.Translator{ - GatewayControllerName: r.Server.EnvoyGateway.Gateway.ControllerName, - GatewayClassName: v1.ObjectName(update.Key), - GlobalRateLimitEnabled: r.EnvoyGateway.RateLimit != nil, - EnvoyPatchPolicyEnabled: r.EnvoyGateway.ExtensionAPIs != nil && r.EnvoyGateway.ExtensionAPIs.EnableEnvoyPatchPolicy, - } - - // If an extension is loaded, pass its supported groups/kinds to the translator - if r.EnvoyGateway.ExtensionManager != nil { - var extGKs []schema.GroupKind - for _, gvk := range r.EnvoyGateway.ExtensionManager.Resources { - extGKs = append(extGKs, schema.GroupKind{Group: gvk.Group, Kind: gvk.Kind}) - } - t.ExtensionGroupKinds = extGKs - } - // Translate to IR - result := t.Translate(val) - var curKeys, newKeys []string // Get current IR keys for key := range r.InfraIR.LoadAll() { curKeys = append(curKeys, key) } - // Publish the IRs. - // Also validate the ir before sending it. - for key, val := range result.InfraIR { - r.Logger.WithValues("infra-ir", key).Info(val.YAMLString()) - if err := val.Validate(); err != nil { - r.Logger.Error(err, "unable to validate infra ir, skipped sending it") - errChan <- err - } else { - r.InfraIR.Store(key, val) - newKeys = append(newKeys, key) + for gc, resources := range *val { + // Translate and publish IRs. + t := &gatewayapi.Translator{ + GatewayControllerName: r.Server.EnvoyGateway.Gateway.ControllerName, + GatewayClassName: v1.ObjectName(gc), + GlobalRateLimitEnabled: r.EnvoyGateway.RateLimit != nil, + EnvoyPatchPolicyEnabled: r.EnvoyGateway.ExtensionAPIs != nil && r.EnvoyGateway.ExtensionAPIs.EnableEnvoyPatchPolicy, } - } - for key, val := range result.XdsIR { - r.Logger.WithValues("xds-ir", key).Info(val.YAMLString()) - if err := val.Validate(); err != nil { - r.Logger.Error(err, "unable to validate xds ir, skipped sending it") - errChan <- err - } else { - r.XdsIR.Store(key, val) + // If an extension is loaded, pass its supported groups/kinds to the translator + if r.EnvoyGateway.ExtensionManager != nil { + var extGKs []schema.GroupKind + for _, gvk := range r.EnvoyGateway.ExtensionManager.Resources { + extGKs = append(extGKs, schema.GroupKind{Group: gvk.Group, Kind: gvk.Kind}) + } + t.ExtensionGroupKinds = extGKs + } + // Translate to IR + result := t.Translate(resources) + + // Publish the IRs. + // Also validate the ir before sending it. + for key, val := range result.InfraIR { + r.Logger.WithValues("infra-ir", key).Info(val.YAMLString()) + if err := val.Validate(); err != nil { + r.Logger.Error(err, "unable to validate infra ir, skipped sending it") + errChan <- err + } else { + r.InfraIR.Store(key, val) + newKeys = append(newKeys, key) + } + } + + for key, val := range result.XdsIR { + r.Logger.WithValues("xds-ir", key).Info(val.YAMLString()) + if err := val.Validate(); err != nil { + r.Logger.Error(err, "unable to validate xds ir, skipped sending it") + errChan <- err + } else { + r.XdsIR.Store(key, val) + } + } + + // Update Status + for _, gateway := range result.Gateways { + gateway := gateway + key := utils.NamespacedName(gateway) + r.ProviderResources.GatewayStatuses.Store(key, &gateway.Status) + } + for _, httpRoute := range result.HTTPRoutes { + httpRoute := httpRoute + key := utils.NamespacedName(httpRoute) + r.ProviderResources.HTTPRouteStatuses.Store(key, &httpRoute.Status) + } + for _, grpcRoute := range result.GRPCRoutes { + grpcRoute := grpcRoute + key := utils.NamespacedName(grpcRoute) + r.ProviderResources.GRPCRouteStatuses.Store(key, &grpcRoute.Status) } - } + for _, tlsRoute := range result.TLSRoutes { + tlsRoute := tlsRoute + key := utils.NamespacedName(tlsRoute) + r.ProviderResources.TLSRouteStatuses.Store(key, &tlsRoute.Status) + } + for _, tcpRoute := range result.TCPRoutes { + tcpRoute := tcpRoute + key := utils.NamespacedName(tcpRoute) + r.ProviderResources.TCPRouteStatuses.Store(key, &tcpRoute.Status) + } + for _, udpRoute := range result.UDPRoutes { + udpRoute := udpRoute + key := utils.NamespacedName(udpRoute) + r.ProviderResources.UDPRouteStatuses.Store(key, &udpRoute.Status) + } + for _, clientTrafficPolicy := range result.ClientTrafficPolicies { + clientTrafficPolicy := clientTrafficPolicy + key := utils.NamespacedName(clientTrafficPolicy) + r.ProviderResources.ClientTrafficPolicyStatuses.Store(key, &clientTrafficPolicy.Status) + } + for _, backendTrafficPolicy := range result.BackendTrafficPolicies { + backendTrafficPolicy := backendTrafficPolicy + key := utils.NamespacedName(backendTrafficPolicy) + r.ProviderResources.BackendTrafficPolicyStatuses.Store(key, &backendTrafficPolicy.Status) + } + for _, securityPolicy := range result.SecurityPolicies { + securityPolicy := securityPolicy + key := utils.NamespacedName(securityPolicy) + r.ProviderResources.SecurityPolicyStatuses.Store(key, &securityPolicy.Status) + } + } // Delete keys // There is a 1:1 mapping between infra and xds IR keys delKeys := getIRKeysToDelete(curKeys, newKeys) @@ -113,54 +160,6 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) { r.InfraIR.Delete(key) r.XdsIR.Delete(key) } - - // Update Status - for _, gateway := range result.Gateways { - gateway := gateway - key := utils.NamespacedName(gateway) - r.ProviderResources.GatewayStatuses.Store(key, &gateway.Status) - } - for _, httpRoute := range result.HTTPRoutes { - httpRoute := httpRoute - key := utils.NamespacedName(httpRoute) - r.ProviderResources.HTTPRouteStatuses.Store(key, &httpRoute.Status) - } - for _, grpcRoute := range result.GRPCRoutes { - grpcRoute := grpcRoute - key := utils.NamespacedName(grpcRoute) - r.ProviderResources.GRPCRouteStatuses.Store(key, &grpcRoute.Status) - } - - for _, tlsRoute := range result.TLSRoutes { - tlsRoute := tlsRoute - key := utils.NamespacedName(tlsRoute) - r.ProviderResources.TLSRouteStatuses.Store(key, &tlsRoute.Status) - } - for _, tcpRoute := range result.TCPRoutes { - tcpRoute := tcpRoute - key := utils.NamespacedName(tcpRoute) - r.ProviderResources.TCPRouteStatuses.Store(key, &tcpRoute.Status) - } - for _, udpRoute := range result.UDPRoutes { - udpRoute := udpRoute - key := utils.NamespacedName(udpRoute) - r.ProviderResources.UDPRouteStatuses.Store(key, &udpRoute.Status) - } - for _, clientTrafficPolicy := range result.ClientTrafficPolicies { - clientTrafficPolicy := clientTrafficPolicy - key := utils.NamespacedName(clientTrafficPolicy) - r.ProviderResources.ClientTrafficPolicyStatuses.Store(key, &clientTrafficPolicy.Status) - } - for _, backendTrafficPolicy := range result.BackendTrafficPolicies { - backendTrafficPolicy := backendTrafficPolicy - key := utils.NamespacedName(backendTrafficPolicy) - r.ProviderResources.BackendTrafficPolicyStatuses.Store(key, &backendTrafficPolicy.Status) - } - for _, securityPolicy := range result.SecurityPolicies { - securityPolicy := securityPolicy - key := utils.NamespacedName(securityPolicy) - r.ProviderResources.SecurityPolicyStatuses.Store(key, &securityPolicy.Status) - } }, ) r.Logger.Info("shutting down") diff --git a/internal/message/types.go b/internal/message/types.go index 1825a8f033e..1328c2a853c 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.Resources] + GatewayAPIResources watchable.Map[string, *gatewayapi.GatewayClassResources] // GatewayAPIStatuses is a group of gateway api // resource statuses maps. @@ -31,7 +31,7 @@ type ProviderResources struct { PolicyStatuses } -func (p *ProviderResources) GetResources() *gatewayapi.Resources { +func (p *ProviderResources) GetResources() *gatewayapi.GatewayClassResources { if p.GatewayAPIResources.Len() == 0 { return nil } diff --git a/internal/provider/kubernetes/controller.go b/internal/provider/kubernetes/controller.go index 3240d7b90b7..8d53b7f071c 100644 --- a/internal/provider/kubernetes/controller.go +++ b/internal/provider/kubernetes/controller.go @@ -51,10 +51,9 @@ type gatewayAPIReconciler struct { namespace string namespaceLabel *metav1.LabelSelector envoyGateway *egv1a1.EnvoyGateway - mergeGateways bool - - resources *message.ProviderResources - extGVKs []schema.GroupVersionKind + mergeGateways map[string]bool + resources *message.ProviderResources + extGVKs []schema.GroupVersionKind } // newGatewayAPIController @@ -88,6 +87,7 @@ func newGatewayAPIController(mgr manager.Manager, cfg *config.Server, su status. extGVKs: extGVKs, store: newProviderStore(), envoyGateway: cfg.EnvoyGateway, + mergeGateways: map[string]bool{}, } if byNamespaceSelector { @@ -165,214 +165,207 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques } // The gatewayclass was already deleted/finalized and there are stale queue entries. - acceptedGC := cc.acceptedClass() - if acceptedGC == nil { + acceptedGCs := cc.matchedClasses + if acceptedGCs == nil { r.log.Info("no accepted gatewayclass") return reconcile.Result{}, nil } - // Update status for all gateway classes - for _, gc := range cc.notAcceptedClasses() { - if err := r.updateStatusForGatewayClass(ctx, gc, false, string(status.ReasonOlderGatewayClassExists), - status.MsgOlderGatewayClassExists); err != nil { - r.resources.GatewayAPIResources.Delete(acceptedGC.Name) + resourcesMap := make(gatewayapi.GatewayClassResources) + for _, acceptedGC := range acceptedGCs { + // Initialize resource types. + acceptedGC := acceptedGC + resourcesMap[acceptedGC.Name] = gatewayapi.NewResources() + resourceMappings := newResourceMapping() + + if err := r.processGateways(ctx, acceptedGC, resourceMappings, resourcesMap[acceptedGC.Name]); err != nil { return reconcile.Result{}, err } - } - - // Initialize resource types. - resourceTree := gatewayapi.NewResources() - resourceMap := newResourceMapping() - if err := r.processGateways(ctx, acceptedGC, resourceMap, resourceTree); err != nil { - return reconcile.Result{}, err - } - - for backendRef := range resourceMap.allAssociatedBackendRefs { - backendRefKind := gatewayapi.KindDerefOr(backendRef.Kind, gatewayapi.KindService) - r.log.Info("processing Backend", "kind", backendRefKind, "namespace", string(*backendRef.Namespace), - "name", string(backendRef.Name)) - - var endpointSliceLabelKey string - switch backendRefKind { - case gatewayapi.KindService: - service := new(corev1.Service) - err := r.client.Get(ctx, types.NamespacedName{Namespace: string(*backendRef.Namespace), Name: string(backendRef.Name)}, service) - if err != nil { - r.log.Error(err, "failed to get Service", "namespace", string(*backendRef.Namespace), - "name", string(backendRef.Name)) - } else { - resourceMap.allAssociatedNamespaces[service.Namespace] = struct{}{} - resourceTree.Services = append(resourceTree.Services, service) - r.log.Info("added Service to resource tree", "namespace", string(*backendRef.Namespace), - "name", string(backendRef.Name)) + for backendRef := range resourceMappings.allAssociatedBackendRefs { + backendRefKind := gatewayapi.KindDerefOr(backendRef.Kind, gatewayapi.KindService) + r.log.Info("processing Backend", "kind", backendRefKind, "namespace", string(*backendRef.Namespace), + "name", string(backendRef.Name)) + + var endpointSliceLabelKey string + switch backendRefKind { + case gatewayapi.KindService: + service := new(corev1.Service) + err := r.client.Get(ctx, types.NamespacedName{Namespace: string(*backendRef.Namespace), Name: string(backendRef.Name)}, service) + if err != nil { + r.log.Error(err, "failed to get Service", "namespace", string(*backendRef.Namespace), + "name", string(backendRef.Name)) + } else { + resourceMappings.allAssociatedNamespaces[service.Namespace] = struct{}{} + resourcesMap[acceptedGC.Name].Services = append(resourcesMap[acceptedGC.Name].Services, service) + r.log.Info("added Service to resource tree", "namespace", string(*backendRef.Namespace), + "name", string(backendRef.Name)) + } + endpointSliceLabelKey = discoveryv1.LabelServiceName + + case gatewayapi.KindServiceImport: + serviceImport := new(mcsapi.ServiceImport) + err := r.client.Get(ctx, types.NamespacedName{Namespace: string(*backendRef.Namespace), Name: string(backendRef.Name)}, serviceImport) + if err != nil { + r.log.Error(err, "failed to get ServiceImport", "namespace", string(*backendRef.Namespace), + "name", string(backendRef.Name)) + } else { + resourceMappings.allAssociatedNamespaces[serviceImport.Namespace] = struct{}{} + resourcesMap[acceptedGC.Name].ServiceImports = append(resourcesMap[acceptedGC.Name].ServiceImports, serviceImport) + r.log.Info("added ServiceImport to resource tree", "namespace", string(*backendRef.Namespace), + "name", string(backendRef.Name)) + } + endpointSliceLabelKey = mcsapi.LabelServiceName } - endpointSliceLabelKey = discoveryv1.LabelServiceName - case gatewayapi.KindServiceImport: - serviceImport := new(mcsapi.ServiceImport) - err := r.client.Get(ctx, types.NamespacedName{Namespace: string(*backendRef.Namespace), Name: string(backendRef.Name)}, serviceImport) - if err != nil { - r.log.Error(err, "failed to get ServiceImport", "namespace", string(*backendRef.Namespace), - "name", string(backendRef.Name)) + // Retrieve the EndpointSlices associated with the service + endpointSliceList := new(discoveryv1.EndpointSliceList) + opts := []client.ListOption{ + client.MatchingLabels(map[string]string{ + endpointSliceLabelKey: string(backendRef.Name), + }), + client.InNamespace(string(*backendRef.Namespace)), + } + if err := r.client.List(ctx, endpointSliceList, opts...); err != nil { + r.log.Error(err, "failed to get EndpointSlices", "namespace", string(*backendRef.Namespace), + backendRefKind, string(backendRef.Name)) } else { - resourceMap.allAssociatedNamespaces[serviceImport.Namespace] = struct{}{} - resourceTree.ServiceImports = append(resourceTree.ServiceImports, serviceImport) - r.log.Info("added ServiceImport to resource tree", "namespace", string(*backendRef.Namespace), - "name", string(backendRef.Name)) + for _, endpointSlice := range endpointSliceList.Items { + 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) + } } - endpointSliceLabelKey = mcsapi.LabelServiceName } - // Retrieve the EndpointSlices associated with the service - endpointSliceList := new(discoveryv1.EndpointSliceList) - opts := []client.ListOption{ - client.MatchingLabels(map[string]string{ - endpointSliceLabelKey: string(backendRef.Name), - }), - client.InNamespace(string(*backendRef.Namespace)), + // Add all ReferenceGrants to the resourceTree + for _, referenceGrant := range resourceMappings.allAssociatedRefGrants { + resourcesMap[acceptedGC.Name].ReferenceGrants = append(resourcesMap[acceptedGC.Name].ReferenceGrants, referenceGrant) } - if err := r.client.List(ctx, endpointSliceList, opts...); err != nil { - r.log.Error(err, "failed to get EndpointSlices", "namespace", string(*backendRef.Namespace), - backendRefKind, string(backendRef.Name)) - } else { - for _, endpointSlice := range endpointSliceList.Items { - endpointSlice := endpointSlice - r.log.Info("added EndpointSlice to resource tree", "namespace", endpointSlice.Namespace, - "name", endpointSlice.Name) - resourceTree.EndpointSlices = append(resourceTree.EndpointSlices, &endpointSlice) - } + + // Add all EnvoyPatchPolicies + envoyPatchPolicies := egv1a1.EnvoyPatchPolicyList{} + if err := r.client.List(ctx, &envoyPatchPolicies); err != nil { + return reconcile.Result{}, fmt.Errorf("error listing EnvoyPatchPolicies: %w", err) } - } - // Add all ReferenceGrants to the resourceTree - for _, referenceGrant := range resourceMap.allAssociatedRefGrants { - resourceTree.ReferenceGrants = append(resourceTree.ReferenceGrants, referenceGrant) - } + for _, policy := range envoyPatchPolicies.Items { + policy := policy + // Discard Status to reduce memory consumption in watchable + // It will be recomputed by the gateway-api layer + policy.Status = egv1a1.EnvoyPatchPolicyStatus{} - // Add all EnvoyPatchPolicies - envoyPatchPolicies := egv1a1.EnvoyPatchPolicyList{} - if err := r.client.List(ctx, &envoyPatchPolicies); err != nil { - return reconcile.Result{}, fmt.Errorf("error listing EnvoyPatchPolicies: %w", err) - } + resourcesMap[acceptedGC.Name].EnvoyPatchPolicies = append(resourcesMap[acceptedGC.Name].EnvoyPatchPolicies, &policy) + } - for _, policy := range envoyPatchPolicies.Items { - policy := policy - // Discard Status to reduce memory consumption in watchable - // It will be recomputed by the gateway-api layer - policy.Status = egv1a1.EnvoyPatchPolicyStatus{} + // Add all ClientTrafficPolicies + clientTrafficPolicies := egv1a1.ClientTrafficPolicyList{} + if err := r.client.List(ctx, &clientTrafficPolicies); err != nil { + return reconcile.Result{}, fmt.Errorf("error listing ClientTrafficPolicies: %w", err) + } - resourceTree.EnvoyPatchPolicies = append(resourceTree.EnvoyPatchPolicies, &policy) - } + for _, policy := range clientTrafficPolicies.Items { + policy := policy + // 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) - // Add all ClientTrafficPolicies - clientTrafficPolicies := egv1a1.ClientTrafficPolicyList{} - if err := r.client.List(ctx, &clientTrafficPolicies); err != nil { - return reconcile.Result{}, fmt.Errorf("error listing ClientTrafficPolicies: %w", err) - } + } - for _, policy := range clientTrafficPolicies.Items { - policy := policy - // Discard Status to reduce memory consumption in watchable - // It will be recomputed by the gateway-api layer - policy.Status = egv1a1.ClientTrafficPolicyStatus{} - resourceTree.ClientTrafficPolicies = append(resourceTree.ClientTrafficPolicies, &policy) + // Add the referenced ConfigMaps in ClientTrafficPolicies to the resourceTree + r.processCtpConfigMapRefs(ctx, resourcesMap[acceptedGC.Name], resourceMappings) - } + // Add all BackendTrafficPolicies + backendTrafficPolicies := egv1a1.BackendTrafficPolicyList{} + if err := r.client.List(ctx, &backendTrafficPolicies); err != nil { + return reconcile.Result{}, fmt.Errorf("error listing BackendTrafficPolicies: %w", err) + } - // Add the referenced ConfigMaps in ClientTrafficPolicies to the resourceTree - r.processCtpConfigMapRefs(ctx, resourceTree, resourceMap) + for _, policy := range backendTrafficPolicies.Items { + policy := policy + // 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) + } - // Add all BackendTrafficPolicies - backendTrafficPolicies := egv1a1.BackendTrafficPolicyList{} - if err := r.client.List(ctx, &backendTrafficPolicies); err != nil { - return reconcile.Result{}, fmt.Errorf("error listing BackendTrafficPolicies: %w", err) - } + // Add all SecurityPolicies + securityPolicies := egv1a1.SecurityPolicyList{} + if err := r.client.List(ctx, &securityPolicies); err != nil { + return reconcile.Result{}, fmt.Errorf("error listing SecurityPolicies: %w", err) + } - for _, policy := range backendTrafficPolicies.Items { - policy := policy - // Discard Status to reduce memory consumption in watchable - // It will be recomputed by the gateway-api layer - policy.Status = egv1a1.BackendTrafficPolicyStatus{} - resourceTree.BackendTrafficPolicies = append(resourceTree.BackendTrafficPolicies, &policy) - } + for _, policy := range securityPolicies.Items { + policy := policy + // 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) + } - // Add all SecurityPolicies - securityPolicies := egv1a1.SecurityPolicyList{} - if err := r.client.List(ctx, &securityPolicies); err != nil { - return reconcile.Result{}, fmt.Errorf("error listing SecurityPolicies: %w", err) - } + // Add the referenced Secrets in SecurityPolicies to the resourceTree + r.processSecurityPolicySecretRefs(ctx, resourcesMap[acceptedGC.Name], resourceMappings) - for _, policy := range securityPolicies.Items { - policy := policy - // Discard Status to reduce memory consumption in watchable - // It will be recomputed by the gateway-api layer - policy.Status = egv1a1.SecurityPolicyStatus{} - resourceTree.SecurityPolicies = append(resourceTree.SecurityPolicies, &policy) - } + // For this particular Gateway, and all associated objects, check whether the + // namespace exists. Add to the resourceTree. + for ns := range resourceMappings.allAssociatedNamespaces { + namespace, err := r.getNamespace(ctx, ns) + if err != nil { + r.log.Error(err, "unable to find the namespace") + if kerrors.IsNotFound(err) { + return reconcile.Result{}, nil + } + return reconcile.Result{}, err + } - // Add the referenced Secrets in SecurityPolicies to the resourceTree - r.processSecurityPolicySecretRefs(ctx, resourceTree, resourceMap) + resourcesMap[acceptedGC.Name].Namespaces = append(resourcesMap[acceptedGC.Name].Namespaces, namespace) + } - // For this particular Gateway, and all associated objects, check whether the - // namespace exists. Add to the resourceTree. - for ns := range resourceMap.allAssociatedNamespaces { - namespace, err := r.getNamespace(ctx, ns) - if err != nil { - r.log.Error(err, "unable to find the namespace") - if kerrors.IsNotFound(err) { - return reconcile.Result{}, nil + // 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 { + 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") + } + r.log.Error(err, "failed to process parametersRef for gatewayclass", "name", acceptedGC.Name) + return reconcile.Result{}, err } - return reconcile.Result{}, err } - resourceTree.Namespaces = append(resourceTree.Namespaces, namespace) - } + if resourcesMap[acceptedGC.Name].EnvoyProxy != nil && resourcesMap[acceptedGC.Name].EnvoyProxy.Spec.MergeGateways != nil { + r.mergeGateways[acceptedGC.Name] = *resourcesMap[acceptedGC.Name].EnvoyProxy.Spec.MergeGateways + } - // Process the parametersRef of the accepted GatewayClass. - if acceptedGC.Spec.ParametersRef != nil && acceptedGC.DeletionTimestamp == nil { - if err := r.processParamsRef(ctx, acceptedGC, resourceTree); 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") - } - r.log.Error(err, "failed to process parametersRef for gatewayclass", "name", acceptedGC.Name) + if err := r.updateStatusForGatewayClass(ctx, acceptedGC, true, string(gwapiv1.GatewayClassReasonAccepted), status.MsgValidGatewayClass); err != nil { + r.log.Error(err, "unable to update GatewayClass status") return reconcile.Result{}, err } - } - - if resourceTree.EnvoyProxy != nil && resourceTree.EnvoyProxy.Spec.MergeGateways != nil { - r.mergeGateways = *resourceTree.EnvoyProxy.Spec.MergeGateways - } - - if err := r.updateStatusForGatewayClass(ctx, acceptedGC, true, string(gwapiv1.GatewayClassReasonAccepted), status.MsgValidGatewayClass); err != nil { - r.log.Error(err, "unable to update GatewayClass status") - return reconcile.Result{}, err - } - // Update finalizer on the gateway class based on the resource tree. - if len(resourceTree.Gateways) == 0 { - r.log.Info("No gateways found for accepted gatewayclass") + if len(resourcesMap[acceptedGC.Name].Gateways) == 0 { + r.log.Info("No gateways found for accepted gatewayclass") - // If needed, remove the finalizer from the accepted GatewayClass. - if err := r.removeFinalizer(ctx, acceptedGC); err != nil { - r.log.Error(err, fmt.Sprintf("failed to remove finalizer from gatewayclass %s", - acceptedGC.Name)) - return reconcile.Result{}, err - } - } else { - // finalize the accepted GatewayClass. - if err := r.addFinalizer(ctx, acceptedGC); err != nil { - r.log.Error(err, fmt.Sprintf("failed adding finalizer to gatewayclass %s", - acceptedGC.Name)) - return reconcile.Result{}, err + // If needed, remove the finalizer from the accepted GatewayClass. + if err := r.removeFinalizer(ctx, acceptedGC); err != nil { + r.log.Error(err, fmt.Sprintf("failed to remove finalizer from gatewayclass %s", + acceptedGC.Name)) + return reconcile.Result{}, err + } else { + // finalize the accepted GatewayClass. + if err := r.addFinalizer(ctx, acceptedGC); err != nil { + r.log.Error(err, fmt.Sprintf("failed adding finalizer to gatewayclass %s", + acceptedGC.Name)) + return reconcile.Result{}, err + } + } } } - // 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(acceptedGC.Name, resourceTree) + r.resources.GatewayAPIResources.Store(string(r.classController), resourcesMap.DeepCopy()) r.log.Info("reconciled gateways successfully") return reconcile.Result{}, nil @@ -705,6 +698,7 @@ func (r *gatewayAPIReconciler) processGateways(ctx context.Context, acceptedGC * gtw.Status = gwapiv1.GatewayStatus{} resourceTree.Gateways = append(resourceTree.Gateways, >w) } + return nil } diff --git a/internal/provider/kubernetes/helpers.go b/internal/provider/kubernetes/helpers.go index d8d6a74b29f..00029f1cb92 100644 --- a/internal/provider/kubernetes/helpers.go +++ b/internal/provider/kubernetes/helpers.go @@ -80,25 +80,10 @@ func validateParentRefs(ctx context.Context, client client.Client, namespace str type controlledClasses struct { // matchedClasses holds all GatewayClass objects with matching controllerName. matchedClasses []*gwapiv1.GatewayClass - - // oldestClass stores the first GatewayClass encountered with matching - // controllerName. This is maintained so that the oldestClass does not change - // during reboots. - oldestClass *gwapiv1.GatewayClass } func (cc *controlledClasses) addMatch(gc *gwapiv1.GatewayClass) { cc.matchedClasses = append(cc.matchedClasses, gc) - - switch { - case cc.oldestClass == nil: - cc.oldestClass = gc - case gc.CreationTimestamp.Time.Before(cc.oldestClass.CreationTimestamp.Time): - cc.oldestClass = gc - case gc.CreationTimestamp.Time.Equal(cc.oldestClass.CreationTimestamp.Time) && gc.Name < cc.oldestClass.Name: - // tie-breaker: first one in alphabetical order is considered oldest/accepted - cc.oldestClass = gc - } } func (cc *controlledClasses) removeMatch(gc *gwapiv1.GatewayClass) { @@ -110,42 +95,6 @@ func (cc *controlledClasses) removeMatch(gc *gwapiv1.GatewayClass) { break } } - - // If the oldestClass is removed, find the new oldestClass candidate - // from matchedClasses. - if cc.oldestClass != nil && cc.oldestClass.Name == gc.Name { - if len(cc.matchedClasses) == 0 { - cc.oldestClass = nil - return - } - - cc.oldestClass = cc.matchedClasses[0] - for i := 1; i < len(cc.matchedClasses); i++ { - current := cc.matchedClasses[i] - if current.CreationTimestamp.Time.Before(cc.oldestClass.CreationTimestamp.Time) || - (current.CreationTimestamp.Time.Equal(cc.oldestClass.CreationTimestamp.Time) && - current.Name < cc.oldestClass.Name) { - cc.oldestClass = current - return - } - } - } -} - -func (cc *controlledClasses) acceptedClass() *gwapiv1.GatewayClass { - return cc.oldestClass -} - -func (cc *controlledClasses) notAcceptedClasses() []*gwapiv1.GatewayClass { - var res []*gwapiv1.GatewayClass - for _, gc := range cc.matchedClasses { - // skip the oldest one since it will be accepted. - if gc.Name != cc.oldestClass.Name { - res = append(res, gc) - } - } - - return res } // isAccepted returns true if the provided gatewayclass contains the Accepted=true diff --git a/internal/provider/kubernetes/helpers_test.go b/internal/provider/kubernetes/helpers_test.go index ea4e31d4bb3..0d5deb0d5d3 100644 --- a/internal/provider/kubernetes/helpers_test.go +++ b/internal/provider/kubernetes/helpers_test.go @@ -7,7 +7,6 @@ package kubernetes import ( "testing" - "time" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -194,100 +193,6 @@ func TestIsGatewayClassAccepted(t *testing.T) { } } -func TestGatewayOldestClass(t *testing.T) { - createGatewayClass := func(name string, creationTime time.Time) *gwapiv1.GatewayClass { - return &gwapiv1.GatewayClass{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - CreationTimestamp: metav1.NewTime(creationTime), - }, - Spec: gwapiv1.GatewayClassSpec{ - ControllerName: egv1a1.GatewayControllerName, - }, - } - } - - currentTime := metav1.Now() - addDuration := time.Duration(10) - testCases := []struct { - name string - classes map[string]time.Time - remove map[string]time.Time - oldest string - }{ - { - name: "normal", - classes: map[string]time.Time{ - "class-b": currentTime.Time, - "class-a": currentTime.Add(1 * addDuration), - }, - remove: nil, - oldest: "class-b", - }, - { - name: "tie breaker", - classes: map[string]time.Time{ - "class-aa": currentTime.Time, - "class-ab": currentTime.Time, - }, - remove: nil, - oldest: "class-aa", - }, - { - name: "remove from matched", - classes: map[string]time.Time{ - "class-a": currentTime.Time, - "class-b": currentTime.Add(1 * addDuration), - "class-c": currentTime.Add(2 * addDuration), - }, - remove: map[string]time.Time{ - "class-b": currentTime.Add(1 * addDuration), - }, - oldest: "class-a", - }, - { - name: "remove oldest", - classes: map[string]time.Time{ - "class-a": currentTime.Time, - "class-b": currentTime.Add(1 * addDuration), - "class-c": currentTime.Add(2 * addDuration), - }, - remove: map[string]time.Time{ - "class-a": currentTime.Time, - }, - oldest: "class-b", - }, - { - name: "remove oldest last", - classes: map[string]time.Time{ - "class-a": currentTime.Time, - }, - remove: map[string]time.Time{ - "class-a": currentTime.Time, - }, - oldest: "", - }, - } - - for _, tc := range testCases { - var cc controlledClasses - for name, timestamp := range tc.classes { - cc.addMatch(createGatewayClass(name, timestamp)) - } - - for name, timestamp := range tc.remove { - cc.removeMatch(createGatewayClass(name, timestamp)) - } - - if tc.oldest == "" { - require.Nil(t, cc.oldestClass) - return - } - - require.Equal(t, tc.oldest, cc.oldestClass.Name) - } -} - func TestRefsEnvoyProxy(t *testing.T) { testCases := []struct { name string diff --git a/internal/provider/kubernetes/kubernetes_test.go b/internal/provider/kubernetes/kubernetes_test.go index 7381e774e0a..2c470f12d02 100644 --- a/internal/provider/kubernetes/kubernetes_test.go +++ b/internal/provider/kubernetes/kubernetes_test.go @@ -141,7 +141,11 @@ func testGatewayClassAcceptedStatus(ctx context.Context, t *testing.T, provider // Even though no gateways exist, the controller loads the empty resource map // to support gateway deletions. require.Eventually(t, func() bool { - _, ok := resources.GatewayAPIResources.Load(gc.Name) + gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + if !ok { + return false + } + _, ok = (*gatewayClassResources)[gc.Name] return ok }, defaultWait, defaultTick) } @@ -195,9 +199,12 @@ func testGatewayClassWithParamRef(ctx context.Context, t *testing.T, provider *P return false }, defaultWait, defaultTick) - // Ensure the resource map contains the EnvoyProxy. require.Eventually(t, func() bool { - res, ok := resources.GatewayAPIResources.Load(gc.Name) + gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + if !ok { + return false + } + res, ok := (*gatewayClassResources)[gc.Name] if !ok { return false } @@ -335,7 +342,8 @@ func testGatewayScheduledStatus(ctx context.Context, t *testing.T, provider *Pro // Ensure the number of Gateways in the Gateway resource table is as expected. require.Eventually(t, func() bool { - res, _ := resources.GatewayAPIResources.Load("gc-scheduled-status-test") + gatewayClassResources, _ := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + res := (*gatewayClassResources)[gc.Name] return res != nil && len(res.Gateways) == 1 }, defaultWait, defaultTick) @@ -354,7 +362,8 @@ func testGatewayScheduledStatus(ctx context.Context, t *testing.T, provider *Pro return cli.Get(ctx, key, gw) == nil }, defaultWait, defaultTick) - res, _ := resources.GatewayAPIResources.Load("gc-scheduled-status-test") + gatewayClassResources, _ := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + res := (*gatewayClassResources)[gc.Name] // 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 @@ -884,15 +893,18 @@ func testHTTPRoute(ctx context.Context, t *testing.T, provider *Provider, resour }, defaultWait, defaultTick) require.Eventually(t, func() bool { - res, ok := resources.GatewayAPIResources.Load("httproute-test") + gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + res := (*gatewayClassResources)[gc.Name] return ok && len(res.HTTPRoutes) != 0 }, defaultWait, defaultTick) - res, _ := resources.GatewayAPIResources.Load("httproute-test") + gatewayClassResources, _ := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + res := (*gatewayClassResources)[gc.Name] assert.Equal(t, &testCase.route, res.HTTPRoutes[0]) // Ensure the HTTPRoute Namespace is in the Namespace resource map. require.Eventually(t, func() bool { - res, ok := resources.GatewayAPIResources.Load(testCase.route.Namespace) + gatewayClassResources, _ := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + res, ok := (*gatewayClassResources)[testCase.route.Namespace] if !ok { return false } @@ -911,7 +923,8 @@ func testHTTPRoute(ctx context.Context, t *testing.T, provider *Provider, resour return true } - res, ok := resources.GatewayAPIResources.Load("httproute-test") + gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + res, ok := (*gatewayClassResources)[gc.Name] if !ok { return false } @@ -1031,15 +1044,21 @@ func testTLSRoute(ctx context.Context, t *testing.T, provider *Provider, resourc }, defaultWait, defaultTick) require.Eventually(t, func() bool { - res, ok := resources.GatewayAPIResources.Load("tlsroute-test") + gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + res, ok := (*gatewayClassResources)[gc.Name] + if !ok { + return false + } return ok && len(res.TLSRoutes) != 0 }, defaultWait, defaultTick) - res, _ := resources.GatewayAPIResources.Load("tlsroute-test") + gatewayClassResources, _ := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + res, _ := (*gatewayClassResources)[gc.Name] assert.Equal(t, &testCase.route, res.TLSRoutes[0]) // Ensure the HTTPRoute Namespace is in the Namespace resource map. require.Eventually(t, func() bool { - res, ok := resources.GatewayAPIResources.Load(testCase.route.Namespace) + gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + res, ok := (*gatewayClassResources)[gc.Name] if !ok { return false } @@ -1053,7 +1072,8 @@ func testTLSRoute(ctx context.Context, t *testing.T, provider *Provider, resourc // Ensure the Service is in the resource map. require.Eventually(t, func() bool { - res, ok := resources.GatewayAPIResources.Load("tlsroute-test") + gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + res, ok := (*gatewayClassResources)[gc.Name] if !ok { return false } @@ -1182,7 +1202,8 @@ func testServiceCleanupForMultipleRoutes(ctx context.Context, t *testing.T, prov // Check that the Service is present in the resource map require.Eventually(t, func() bool { - res, ok := resources.GatewayAPIResources.Load("service-cleanup-test") + gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + res, ok := (*gatewayClassResources)[gc.Name] if !ok { return false } @@ -1197,7 +1218,8 @@ func testServiceCleanupForMultipleRoutes(ctx context.Context, t *testing.T, prov // Delete the TLSRoute, and check if the Service is still present require.NoError(t, cli.Delete(ctx, &tlsRoute)) require.Eventually(t, func() bool { - res, ok := resources.GatewayAPIResources.Load("service-cleanup-test") + gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + res, ok := (*gatewayClassResources)[gc.Name] if !ok { return false } @@ -1212,7 +1234,8 @@ func testServiceCleanupForMultipleRoutes(ctx context.Context, t *testing.T, prov // Delete the HTTPRoute, and check if the Service is also removed require.NoError(t, cli.Delete(ctx, &httpRoute)) require.Eventually(t, func() bool { - res, ok := resources.GatewayAPIResources.Load("service-cleanup-test") + gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + res, ok := (*gatewayClassResources)[gc.Name] if !ok { return false } @@ -1359,7 +1382,11 @@ func TestNamespaceSelectorProvider(t *testing.T) { }() require.Eventually(t, func() bool { - res, _ := resources.GatewayAPIResources.Load(gc.Name) + gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + res, ok := (*gatewayClassResources)[gc.Name] + if !ok { + return false + } return res != nil && len(res.Gateways) == 1 }, defaultWait, defaultTick) @@ -1508,33 +1535,57 @@ func TestNamespaceSelectorProvider(t *testing.T) { }() require.Eventually(t, func() bool { - res, _ := resources.GatewayAPIResources.Load(gc.Name) + gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + res, ok := (*gatewayClassResources)[gc.Name] + if !ok { + return false + } // The service number dependes on the service created and the backendRef return res != nil && len(res.Services) == 5 }, defaultWait, defaultTick) require.Eventually(t, func() bool { - res, _ := resources.GatewayAPIResources.Load(gc.Name) + gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + res, ok := (*gatewayClassResources)[gc.Name] + if !ok { + return false + } return res != nil && len(res.HTTPRoutes) == 1 }, defaultWait, defaultTick) require.Eventually(t, func() bool { - res, _ := resources.GatewayAPIResources.Load(gc.Name) + gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + res, ok := (*gatewayClassResources)[gc.Name] + if !ok { + return false + } return res != nil && len(res.TCPRoutes) == 1 }, defaultWait, defaultTick) require.Eventually(t, func() bool { - res, _ := resources.GatewayAPIResources.Load(gc.Name) + gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + res, ok := (*gatewayClassResources)[gc.Name] + if !ok { + return false + } return res != nil && len(res.TLSRoutes) == 1 }, defaultWait, defaultTick) require.Eventually(t, func() bool { - res, _ := resources.GatewayAPIResources.Load(gc.Name) + gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + res, ok := (*gatewayClassResources)[gc.Name] + if !ok { + return false + } return res != nil && len(res.UDPRoutes) == 1 }, defaultWait, defaultTick) require.Eventually(t, func() bool { - res, _ := resources.GatewayAPIResources.Load(gc.Name) + gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName) + res, ok := (*gatewayClassResources)[gc.Name] + if !ok { + return false + } return res != nil && len(res.GRPCRoutes) == 1 }, defaultWait, defaultTick) diff --git a/internal/provider/kubernetes/predicates.go b/internal/provider/kubernetes/predicates.go index a4145bac29d..5a042d1cacf 100644 --- a/internal/provider/kubernetes/predicates.go +++ b/internal/provider/kubernetes/predicates.go @@ -220,17 +220,18 @@ func (r *gatewayAPIReconciler) validateServiceForReconcile(obj client.Object) bo return false } - // Only merged gateways will have this label, update status of all Gateways under found GatewayClass. + // Merged gateways will have only this label, update status of all Gateways under found GatewayClass. gclass, ok := labels[gatewayapi.OwningGatewayClassLabel] - if ok { - res, _ := r.resources.GatewayAPIResources.Load(gclass) - if res != nil && len(res.Gateways) > 0 { - for _, gw := range res.Gateways { - gw := gw - r.updateStatusForGateway(ctx, gw) + if ok && r.mergeGateways[gclass] { + res, _ := r.resources.GatewayAPIResources.Load(string(r.classController)) + if res != nil { + if (*res)[gclass] != nil && len((*res)[gclass].Gateways) > 0 { + for _, gw := range (*res)[gclass].Gateways { + gw := gw + r.updateStatusForGateway(ctx, gw) + } } } - return false } @@ -376,14 +377,16 @@ func (r *gatewayAPIReconciler) validateDeploymentForReconcile(obj client.Object) } } - // Only merged gateways will have this label, update status of all Gateways under found GatewayClass. + // Merged gateways will have only this label, update status of all Gateways under found GatewayClass. gclass, ok := labels[gatewayapi.OwningGatewayClassLabel] - if ok { - res, _ := r.resources.GatewayAPIResources.Load(gclass) - if res != nil && len(res.Gateways) > 0 { - for _, gw := range res.Gateways { - gw := gw - r.updateStatusForGateway(ctx, gw) + if ok && r.mergeGateways[gclass] { + res, _ := r.resources.GatewayAPIResources.Load(string(r.classController)) + if res != nil { + if (*res)[gclass] != nil && len((*res)[gclass].Gateways) > 0 { + for _, gw := range (*res)[gclass].Gateways { + gw := gw + r.updateStatusForGateway(ctx, gw) + } } } return false @@ -397,7 +400,7 @@ func (r *gatewayAPIReconciler) validateDeploymentForReconcile(obj client.Object) func (r *gatewayAPIReconciler) envoyDeploymentForGateway(ctx context.Context, gateway *gwapiv1.Gateway) (*appsv1.Deployment, error) { key := types.NamespacedName{ Namespace: r.namespace, - Name: infraName(gateway, r.mergeGateways), + Name: infraName(gateway, r.mergeGateways[string(gateway.Spec.GatewayClassName)]), } deployment := new(appsv1.Deployment) if err := r.client.Get(ctx, key, deployment); err != nil { @@ -413,7 +416,7 @@ func (r *gatewayAPIReconciler) envoyDeploymentForGateway(ctx context.Context, ga func (r *gatewayAPIReconciler) envoyServiceForGateway(ctx context.Context, gateway *gwapiv1.Gateway) (*corev1.Service, error) { key := types.NamespacedName{ Namespace: r.namespace, - Name: infraName(gateway, r.mergeGateways), + Name: infraName(gateway, r.mergeGateways[string(gateway.Spec.GatewayClassName)]), } svc := new(corev1.Service) if err := r.client.Get(ctx, key, svc); err != nil { diff --git a/test/e2e/testdata/multiple-gc.yaml b/test/e2e/testdata/multiple-gc.yaml new file mode 100644 index 00000000000..94682535573 --- /dev/null +++ b/test/e2e/testdata/multiple-gc.yaml @@ -0,0 +1,191 @@ +kind: GatewayClass +apiVersion: gateway.networking.k8s.io/v1 +metadata: + name: internet +spec: + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parametersRef: + name: internet-config + namespace: envoy-gateway-system + group: gateway.envoyproxy.io + kind: EnvoyProxy +--- +apiVersion: gateway.envoyproxy.io/v1alpha1 +kind: EnvoyProxy +metadata: + name: internet-config + namespace: envoy-gateway-system +spec: + mergeGateways: true +--- +kind: GatewayClass +apiVersion: gateway.networking.k8s.io/v1 +metadata: + name: private +spec: + controllerName: gateway.envoyproxy.io/gatewayclass-controller +--- +apiVersion: v1 +kind: Namespace +metadata: + name: internet + labels: + gateway-conformance: internet +--- +apiVersion: v1 +kind: Namespace +metadata: + name: private + labels: + gateway-conformance: private +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: private-gateway + namespace: private +spec: + gatewayClassName: private + listeners: + - name: http + port: 80 + protocol: HTTP + allowedRoutes: + namespaces: + from: Same +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: internet-gateway + namespace: internet +spec: + gatewayClassName: internet + listeners: + - name: http + port: 80 + protocol: HTTP + allowedRoutes: + namespaces: + from: Same +--- +apiVersion: v1 +kind: Service +metadata: + name: private-backend + namespace: private +spec: + selector: + app: private-backend + ports: + - protocol: TCP + port: 8080 + targetPort: 3000 +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: private-backend + namespace: private + labels: + app: private-backend +spec: + replicas: 2 + selector: + matchLabels: + app: private-backend + template: + metadata: + labels: + app: private-backend + spec: + containers: + - name: private-backend + # From https://github.com/kubernetes-sigs/ingress-controller-conformance/tree/master/images/echoserver + image: gcr.io/k8s-staging-ingressconformance/echoserver:v20221109-7ee2f3e + env: + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name + - name: NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + resources: + requests: + cpu: 10m +--- +apiVersion: v1 +kind: Service +metadata: + name: internet-backend + namespace: internet +spec: + selector: + app: internet-backend + ports: + - protocol: TCP + port: 8080 + targetPort: 3000 +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: internet-backend + namespace: internet + labels: + app: internet-backend +spec: + replicas: 2 + selector: + matchLabels: + app: internet-backend + template: + metadata: + labels: + app: internet-backend + spec: + containers: + - name: internet-backend + image: gcr.io/k8s-staging-ingressconformance/echoserver:v20221109-7ee2f3e + env: + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name + - name: NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + resources: + requests: + cpu: 10m +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: internet-route + namespace: internet +spec: + parentRefs: + - name: internet-gateway + sectionName: http + rules: + - backendRefs: + - name: internet-backend + port: 8080 +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: private-route + namespace: private +spec: + parentRefs: + - name: private-gateway + sectionName: http + rules: + - backendRefs: + - name: private-backend + port: 8080 diff --git a/test/e2e/tests/multiple-gc.go b/test/e2e/tests/multiple-gc.go new file mode 100644 index 00000000000..0977b6f1f47 --- /dev/null +++ b/test/e2e/tests/multiple-gc.go @@ -0,0 +1,69 @@ +// 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. + +// This file contains code derived from upstream gateway-api, it will be moved to upstream. + +//go:build e2e +// +build e2e + +package tests + +import ( + "testing" + + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/gateway-api/conformance/utils/http" + "sigs.k8s.io/gateway-api/conformance/utils/kubernetes" + "sigs.k8s.io/gateway-api/conformance/utils/suite" +) + +func init() { + ConformanceTests = append(ConformanceTests, MultipleGCTest) +} + +var MultipleGCTest = suite.ConformanceTest{ + ShortName: "MultipleGC", + Description: "Testing multiple GatewayClass with the same controller", + Manifests: []string{"testdata/multiple-gc.yaml"}, + Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { + t.Run("gc-1", func(t *testing.T) { + ns := "private" + routeNN := types.NamespacedName{Name: "private-route", Namespace: ns} + gwNN := types.NamespacedName{Name: "private-gateway", Namespace: ns} + gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) + OkResp := http.ExpectedResponse{ + Request: http.Request{ + Path: "/", + }, + Response: http.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + + // Send a request to an valid path and expect a successful response + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, OkResp) + }) + t.Run("gc-2", func(t *testing.T) { + ns := "internet" + routeNN := types.NamespacedName{Name: "internet-route", Namespace: ns} + gwNN := types.NamespacedName{Name: "internet-gateway", Namespace: ns} + gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) + OkResp := http.ExpectedResponse{ + Request: http.Request{ + Path: "/", + }, + Response: http.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + + // Send a request to an valid path and expect a successful response + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, OkResp) + }) + + }, +}