diff --git a/internal/framework/status2/updater.go b/internal/framework/status2/updater.go new file mode 100644 index 0000000000..d83b0fa010 --- /dev/null +++ b/internal/framework/status2/updater.go @@ -0,0 +1,239 @@ +package status2 + +import ( + "context" + "errors" + "slices" + "sync" + "time" + + "github.com/go-logr/logr" + apierrors "k8s.io/apimachinery/pkg/api/errors" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/wait" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller" +) + +// K8sUpdater updates a resource from the k8s API. +// It allows us to mock the client.Reader.Status.Update method. +type K8sUpdater interface { + // Update is from client.StatusClient.SubResourceWriter. + Update(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error +} + +type UpdateRequest struct { + NsName types.NamespacedName + ResourceType client.Object + Setter Setter +} + +type Setter func(client.Object) bool + +type Updater struct { + client client.Client + logger logr.Logger +} + +func NewUpdater(client client.Client, logger logr.Logger) *Updater { + return &Updater{ + client: client, + logger: logger, + } +} + +func (u *Updater) Update(ctx context.Context, reqs ...UpdateRequest) { + for _, r := range reqs { + u.writeStatuses(ctx, r.NsName, r.ResourceType, r.Setter) + } +} + +func (u *Updater) writeStatuses( + ctx context.Context, + nsname types.NamespacedName, + obj client.Object, + statusSetter Setter, +) { + err := wait.ExponentialBackoffWithContext( + ctx, + wait.Backoff{ + Duration: time.Millisecond * 200, + Factor: 2, + Jitter: 0.5, + Steps: 4, + Cap: time.Millisecond * 3000, + }, + // Function returns true if the condition is satisfied, or an error if the loop should be aborted. + NewRetryUpdateFunc(u.client, u.client.Status(), nsname, obj, u.logger, statusSetter), + ) + if err != nil && !errors.Is(err, context.Canceled) { + u.logger.Error( + err, + "Failed to update status", + "namespace", nsname.Namespace, + "name", nsname.Name, + "kind", obj.GetObjectKind().GroupVersionKind().Kind) + } +} + +// NewRetryUpdateFunc returns a function which will be used in wait.ExponentialBackoffWithContext. +// The function will attempt to Update a kubernetes resource and will be retried in +// wait.ExponentialBackoffWithContext if an error occurs. Exported for testing purposes. +// +// wait.ExponentialBackoffWithContext will retry if this function returns nil as its error, +// which is what we want if we encounter an error from the functions we call. However, +// the linter will complain if we return nil if an error was found. +// +//nolint:nilerr +func NewRetryUpdateFunc( + getter controller.Getter, + updater K8sUpdater, + nsname types.NamespacedName, + obj client.Object, + logger logr.Logger, + statusSetter func(client.Object) bool, +) func(ctx context.Context) (bool, error) { + return func(ctx context.Context) (bool, error) { + // The function handles errors by reporting them in the logs. + // We need to get the latest version of the resource. + // Otherwise, the Update status API call can fail. + // Note: the default client uses a cache for reads, so we're not making an unnecessary API call here. + // the default is configurable in the Manager options. + if err := getter.Get(ctx, nsname, obj); err != nil { + // apierrors.IsNotFound(err) can happen when the resource is deleted, + // so no need to retry or return an error. + if apierrors.IsNotFound(err) { + return true, nil + } + + logger.V(1).Info( + "Encountered error when getting resource to update status", + "error", err, + "namespace", nsname.Namespace, + "name", nsname.Name, + "kind", obj.GetObjectKind().GroupVersionKind().Kind, + ) + + return false, nil + } + + if !statusSetter(obj) { + logger.V(1).Info( + "Skipping status update because there's no change", + "namespace", nsname.Namespace, + "name", nsname.Name, + "kind", obj.GetObjectKind().GroupVersionKind().Kind, + ) + + return true, nil + } + + if err := updater.Update(ctx, obj); err != nil { + logger.V(1).Info( + "Encountered error updating status", + "error", err, + "namespace", nsname.Namespace, + "name", nsname.Name, + "kind", obj.GetObjectKind().GroupVersionKind().Kind, + ) + + return false, nil + } + + return true, nil + } +} + +type GroupUpdateRequest struct { + Name string + Request []UpdateRequest +} + +type CachingGroupUpdater struct { + updater *Updater + lock *sync.Mutex + groups map[string]GroupUpdateRequest + enabled bool +} + +func NewCachingGroupUpdater(updater *Updater) *CachingGroupUpdater { + return &CachingGroupUpdater{ + updater: updater, + lock: &sync.Mutex{}, + groups: make(map[string]GroupUpdateRequest), + } +} + +func (u *CachingGroupUpdater) Update(ctx context.Context, update GroupUpdateRequest) { + u.lock.Lock() + defer u.lock.Unlock() + + if len(update.Request) == 0 { + delete(u.groups, update.Name) + } + + u.groups[update.Name] = update + + if !u.enabled { + return + } + + u.updater.Update(ctx, update.Request...) +} + +func (u *CachingGroupUpdater) Enable(ctx context.Context) { + u.lock.Lock() + defer u.lock.Unlock() + + u.enabled = true + + for _, update := range u.groups { + u.updater.Update(ctx, update.Request...) + } +} + +func ConditionsEqual(prev, cur []v1.Condition) bool { + return slices.EqualFunc(prev, cur, func(c1, c2 v1.Condition) bool { + if c1.ObservedGeneration != c2.ObservedGeneration { + return false + } + + if c1.Type != c2.Type { + return false + } + + if c1.Status != c2.Status { + return false + } + + if c1.Message != c2.Message { + return false + } + + return c1.Reason == c2.Reason + }) +} + +func ConvertConditions( + conds []conditions.Condition, + observedGeneration int64, + transitionTime v1.Time, +) []v1.Condition { + apiConds := make([]v1.Condition, len(conds)) + + for i := range conds { + apiConds[i] = v1.Condition{ + Type: conds[i].Type, + Status: conds[i].Status, + ObservedGeneration: observedGeneration, + LastTransitionTime: transitionTime, + Reason: conds[i].Reason, + Message: conds[i].Message, + } + } + + return apiConds +} diff --git a/internal/mode/provisioner/handler.go b/internal/mode/provisioner/handler.go index 853e814f1b..e6b4bbf9f7 100644 --- a/internal/mode/provisioner/handler.go +++ b/internal/mode/provisioner/handler.go @@ -6,13 +6,15 @@ import ( "github.com/go-logr/logr" v1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/events" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/gatewayclass" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status2" ) // eventHandler ensures each Gateway for the specific GatewayClass has a corresponding Deployment @@ -26,7 +28,7 @@ type eventHandler struct { // provisions maps NamespacedName of Gateway to its corresponding Deployment provisions map[types.NamespacedName]*v1.Deployment - statusUpdater status.Updater + statusUpdater *status2.Updater k8sClient client.Client staticModeDeploymentYAML []byte @@ -36,7 +38,7 @@ type eventHandler struct { func newEventHandler( gcName string, - statusUpdater status.Updater, + statusUpdater *status2.Updater, k8sClient client.Client, staticModeDeploymentYAML []byte, ) *eventHandler { @@ -52,9 +54,7 @@ func newEventHandler( } func (h *eventHandler) setGatewayClassStatuses(ctx context.Context) { - statuses := status.GatewayAPIStatuses{ - GatewayClassStatuses: make(status.GatewayClassStatuses), - } + var reqs []status2.UpdateRequest var gcExists bool @@ -74,17 +74,30 @@ func (h *eventHandler) setGatewayClassStatuses(ctx context.Context) { supportedVersionConds, _ := gatewayclass.ValidateCRDVersions(h.store.crdMetadata) conds = append(conds, supportedVersionConds...) - statuses.GatewayClassStatuses[nsname] = status.GatewayClassStatus{ - Conditions: conditions.DeduplicateConditions(conds), - ObservedGeneration: gc.Generation, - } + reqs = append(reqs, status2.UpdateRequest{ + NsName: nsname, + ResourceType: &gatewayv1.GatewayClass{}, + Setter: func(object client.Object) bool { + gcs := gatewayv1.GatewayClassStatus{ + Conditions: status2.ConvertConditions(conditions.DeduplicateConditions(conds), gc.Generation, metav1.Now()), + } + + if status2.ConditionsEqual(gc.Status.Conditions, gcs.Conditions) { + return false + } + + gc.Status = gcs + + return true + }, + }) } if !gcExists { panic(fmt.Errorf("GatewayClass %s must exist", h.gcName)) } - h.statusUpdater.Update(ctx, statuses) + h.statusUpdater.Update(ctx, reqs...) } func (h *eventHandler) ensureDeploymentsMatchGateways(ctx context.Context, logger logr.Logger) { diff --git a/internal/mode/provisioner/manager.go b/internal/mode/provisioner/manager.go index 1bca844bac..167be73c8b 100644 --- a/internal/mode/provisioner/manager.go +++ b/internal/mode/provisioner/manager.go @@ -20,7 +20,7 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller/predicate" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/events" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/gatewayclass" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status2" ) // Config is configuration for the provisioner mode. @@ -121,14 +121,9 @@ func StartManager(cfg Config) error { }, ) - statusUpdater := status.NewUpdater( - status.UpdaterConfig{ - Client: mgr.GetClient(), - Clock: status.NewRealClock(), - Logger: cfg.Logger.WithName("statusUpdater"), - GatewayClassName: cfg.GatewayClassName, - UpdateGatewayClassStatus: true, - }, + statusUpdater := status2.NewUpdater( + mgr.GetClient(), + cfg.Logger.WithName("statusUpdater"), ) handler := newEventHandler( diff --git a/internal/mode/static/build_statuses.go b/internal/mode/static/build_statuses.go index 6055d9fdd1..430bdf3911 100644 --- a/internal/mode/static/build_statuses.go +++ b/internal/mode/static/build_statuses.go @@ -1,12 +1,14 @@ package static import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" v1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status2" staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph" ) @@ -15,24 +17,14 @@ type nginxReloadResult struct { error error } -// buildGatewayAPIStatuses builds status.Statuses from a Graph. -func buildGatewayAPIStatuses( - graph *graph.Graph, - gwAddresses []v1.GatewayStatusAddress, +func buildRouteStatuses( + routes map[types.NamespacedName]*graph.Route, nginxReloadRes nginxReloadResult, -) status.GatewayAPIStatuses { - statuses := status.GatewayAPIStatuses{ - HTTPRouteStatuses: make(status.HTTPRouteStatuses), - } - - statuses.GatewayClassStatuses = buildGatewayClassStatuses(graph.GatewayClass, graph.IgnoredGatewayClasses) +) []status2.UpdateRequest { + reqs := make([]status2.UpdateRequest, 0, len(routes)) - statuses.GatewayStatuses = buildGatewayStatuses(graph.Gateway, graph.IgnoredGateways, gwAddresses, nginxReloadRes) - - statuses.BackendTLSPolicyStatuses = buildBackendTLSPolicyStatuses(graph.BackendTLSPolicies) - - for nsname, r := range graph.Routes { - parentStatuses := make([]status.ParentStatus, 0, len(r.ParentRefs)) + for nsname, r := range routes { + parents := make([]v1.RouteParentStatus, 0, len(r.ParentRefs)) defaultConds := staticConds.NewDefaultRouteConditions() @@ -60,27 +52,46 @@ func buildGatewayAPIStatuses( routeRef := r.Source.Spec.ParentRefs[ref.Idx] - parentStatuses = append(parentStatuses, status.ParentStatus{ - GatewayNsName: ref.Gateway, - SectionName: routeRef.SectionName, - Conditions: conditions.DeduplicateConditions(allConds), - }) + conds := conditions.DeduplicateConditions(allConds) + + apiConds := status2.ConvertConditions(conds, r.Source.Generation, metav1.Now()) + + ps := v1.RouteParentStatus{ + ParentRef: v1.ParentReference{ + Namespace: (*v1.Namespace)(&ref.Gateway.Namespace), + Name: v1.ObjectName(ref.Gateway.Name), + SectionName: routeRef.SectionName, + }, + ControllerName: v1.GatewayController("todo.controller/path"), + Conditions: apiConds, + } + + parents = append(parents, ps) + } + + status := v1.HTTPRouteStatus{ + RouteStatus: v1.RouteStatus{ + Parents: parents, + }, } - statuses.HTTPRouteStatuses[nsname] = status.HTTPRouteStatus{ - ObservedGeneration: r.Source.Generation, - ParentStatuses: parentStatuses, + req := status2.UpdateRequest{ + NsName: nsname, + ResourceType: &v1.HTTPRoute{}, + Setter: newHTTPRouteStatusSetter(status, "todo.controller/path"), } + + reqs = append(reqs, req) } - return statuses + return reqs } func buildGatewayClassStatuses( gc *graph.GatewayClass, ignoredGwClasses map[types.NamespacedName]*v1.GatewayClass, -) status.GatewayClassStatuses { - statuses := make(status.GatewayClassStatuses) +) []status2.UpdateRequest { + var reqs []status2.UpdateRequest if gc != nil { defaultConds := conditions.NewDefaultGatewayClassConditions() @@ -92,20 +103,28 @@ func buildGatewayClassStatuses( conds = append(conds, defaultConds...) conds = append(conds, gc.Conditions...) - statuses[client.ObjectKeyFromObject(gc.Source)] = status.GatewayClassStatus{ - Conditions: conditions.DeduplicateConditions(conds), - ObservedGeneration: gc.Source.Generation, + conds = conditions.DeduplicateConditions(conds) + + req := status2.UpdateRequest{ + NsName: client.ObjectKeyFromObject(gc.Source), + ResourceType: &v1.GatewayClass{}, + Setter: newGatewayClassStatusSetter(gc.Source.Generation, conds), } + + reqs = append(reqs, req) } for nsname, gwClass := range ignoredGwClasses { - statuses[nsname] = status.GatewayClassStatus{ - Conditions: []conditions.Condition{conditions.NewGatewayClassConflict()}, - ObservedGeneration: gwClass.Generation, + req := status2.UpdateRequest{ + NsName: nsname, + ResourceType: &v1.GatewayClass{}, + Setter: newGatewayClassStatusSetter(gwClass.Generation, []conditions.Condition{conditions.NewGatewayClassConflict()}), } + + reqs = append(reqs, req) } - return statuses + return reqs } func buildGatewayStatuses( @@ -113,37 +132,44 @@ func buildGatewayStatuses( ignoredGateways map[types.NamespacedName]*v1.Gateway, gwAddresses []v1.GatewayStatusAddress, nginxReloadRes nginxReloadResult, -) status.GatewayStatuses { - statuses := make(status.GatewayStatuses) +) []status2.UpdateRequest { + reqs := make([]status2.UpdateRequest, 0, 1+len(ignoredGateways)) if gateway != nil { - statuses[client.ObjectKeyFromObject(gateway.Source)] = buildGatewayStatus(gateway, gwAddresses, nginxReloadRes) + reqs = append(reqs, buildGatewayStatus(gateway, gwAddresses, nginxReloadRes)) } for nsname, gw := range ignoredGateways { - statuses[nsname] = status.GatewayStatus{ - Conditions: staticConds.NewGatewayConflict(), - ObservedGeneration: gw.Generation, - Ignored: true, - } + apiConds := status2.ConvertConditions(staticConds.NewGatewayConflict(), gw.Generation, metav1.Now()) + reqs = append(reqs, status2.UpdateRequest{ + NsName: nsname, + ResourceType: &v1.Gateway{}, + Setter: newGatewayStatusSetter(v1.GatewayStatus{ + Conditions: apiConds, + }), + }) } - return statuses + return reqs } func buildGatewayStatus( gateway *graph.Gateway, gwAddresses []v1.GatewayStatusAddress, nginxReloadRes nginxReloadResult, -) status.GatewayStatus { +) status2.UpdateRequest { if !gateway.Valid { - return status.GatewayStatus{ - Conditions: conditions.DeduplicateConditions(gateway.Conditions), - ObservedGeneration: gateway.Source.Generation, + conds := status2.ConvertConditions(conditions.DeduplicateConditions(gateway.Conditions), gateway.Source.Generation, metav1.Now()) + return status2.UpdateRequest{ + NsName: client.ObjectKeyFromObject(gateway.Source), + ResourceType: &v1.Gateway{}, + Setter: newGatewayStatusSetter(v1.GatewayStatus{ + Conditions: conds, + }), } } - listenerStatuses := make([]status.ListenerStatus, 0, len(gateway.Listeners)) + listenerStatuses := make([]v1.ListenerStatus, 0, len(gateway.Listeners)) validListenerCount := 0 for _, l := range gateway.Listeners { @@ -163,11 +189,13 @@ func buildGatewayStatus( ) } - listenerStatuses = append(listenerStatuses, status.ListenerStatus{ + apiConds := status2.ConvertConditions(conditions.DeduplicateConditions(conds), gateway.Source.Generation, metav1.Now()) + + listenerStatuses = append(listenerStatuses, v1.ListenerStatus{ Name: v1.SectionName(l.Name), - AttachedRoutes: int32(len(l.Routes)), - Conditions: conditions.DeduplicateConditions(conds), SupportedKinds: l.SupportedKinds, + AttachedRoutes: int32(len(l.Routes)), + Conditions: apiConds, }) } @@ -185,31 +213,33 @@ func buildGatewayStatus( ) } - return status.GatewayStatus{ - Conditions: conditions.DeduplicateConditions(gwConds), - ListenerStatuses: listenerStatuses, - Addresses: gwAddresses, - ObservedGeneration: gateway.Source.Generation, + apiGwConds := status2.ConvertConditions(gwConds, gateway.Source.Generation, metav1.Now()) + + return status2.UpdateRequest{ + NsName: client.ObjectKeyFromObject(gateway.Source), + ResourceType: &v1.Gateway{}, + Setter: newGatewayStatusSetter(v1.GatewayStatus{ + Listeners: listenerStatuses, + Conditions: apiGwConds, + Addresses: gwAddresses, + }), } } func buildBackendTLSPolicyStatuses(backendTLSPolicies map[types.NamespacedName]*graph.BackendTLSPolicy, -) status.BackendTLSPolicyStatuses { - statuses := make(status.BackendTLSPolicyStatuses, len(backendTLSPolicies)) +) []status2.UpdateRequest { + reqs := make([]status2.UpdateRequest, 0, len(backendTLSPolicies)) for nsname, backendTLSPolicy := range backendTLSPolicies { if backendTLSPolicy.IsReferenced { if !backendTLSPolicy.Ignored { - statuses[nsname] = status.BackendTLSPolicyStatus{ - AncestorStatuses: []status.AncestorStatus{ - { - GatewayNsName: backendTLSPolicy.Gateway, - Conditions: conditions.DeduplicateConditions(backendTLSPolicy.Conditions), - }, - }, - } + reqs = append(reqs, status2.UpdateRequest{ + NsName: nsname, + ResourceType: &v1alpha2.BackendTLSPolicy{}, + Setter: newBackendTLSPolicyStatusSetter("todo.controller/path", backendTLSPolicy), + }) } } } - return statuses + return reqs } diff --git a/internal/mode/static/handler.go b/internal/mode/static/handler.go index 430b30474f..e9ca911e82 100644 --- a/internal/mode/static/handler.go +++ b/internal/mode/static/handler.go @@ -10,6 +10,7 @@ import ( ngxclient "github.com/nginxinc/nginx-plus-go-client/client" apiv1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" @@ -19,7 +20,7 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/events" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status2" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" ngfConfig "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" ngxConfig "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config" @@ -66,7 +67,7 @@ type eventHandlerConfig struct { // nginxRuntimeMgr manages nginx runtime. nginxRuntimeMgr runtime.Manager // statusUpdater updates statuses on Kubernetes resources. - statusUpdater status.Updater + statusUpdater *status2.CachingGroupUpdater // eventRecorder records events for Kubernetes resources. eventRecorder record.EventRecorder // logLevelSetter is used to update the logging level. @@ -107,7 +108,8 @@ type eventHandlerImpl struct { lock sync.Mutex // version is the current version number of the nginx config. - version int + version int + latestReloadResult nginxReloadResult } // newEventHandlerImpl creates a new eventHandlerImpl. @@ -214,12 +216,36 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, logger logr.Log } } + h.latestReloadResult = nginxReloadRes + gwAddresses, err := getGatewayAddresses(ctx, h.cfg.k8sClient, nil, h.cfg.gatewayPodConfig) if err != nil { logger.Error(err, "Setting GatewayStatusAddress to Pod IP Address") } - h.cfg.statusUpdater.Update(ctx, buildGatewayAPIStatuses(graph, gwAddresses, nginxReloadRes)) + var statuses []status2.UpdateRequest + + statuses = append(statuses, buildGatewayClassStatuses(graph.GatewayClass, graph.IgnoredGatewayClasses)...) + statuses = append(statuses, buildRouteStatuses(graph.Routes, nginxReloadRes)...) + statuses = append(statuses, buildBackendTLSPolicyStatuses(graph.BackendTLSPolicies)...) + + groupReq := status2.GroupUpdateRequest{ + Name: "all-graphs-expect-gateway", + Request: statuses, + } + + h.cfg.statusUpdater.Update(ctx, groupReq) + + // We put Gateway status updates separately from the rest of the statuses because we want to be able + // to update them separately from the rest of the graph whenever the public IP of NGF changes. + + gatewayStatuses := buildGatewayStatuses(graph.Gateway, graph.IgnoredGateways, gwAddresses, nginxReloadRes) + gatewaysGroup := status2.GroupUpdateRequest{ + Name: "gateways", + Request: gatewayStatuses, + } + + h.cfg.statusUpdater.Update(ctx, gatewaysGroup) } func (h *eventHandlerImpl) parseAndCaptureEvent(ctx context.Context, logger logr.Logger, event interface{}) { @@ -382,13 +408,23 @@ func (h *eventHandlerImpl) updateControlPlaneAndSetStatus( } if cfg != nil { - nginxGatewayStatus := &status.NginxGatewayStatus{ - NsName: client.ObjectKeyFromObject(cfg), - Conditions: cond, - ObservedGeneration: cfg.Generation, + nginxGwsStatus := ngfAPI.NginxGatewayStatus{ + Conditions: status2.ConvertConditions(cond, cfg.Generation, metav1.Now()), + } + + groupReq := status2.GroupUpdateRequest{ + Name: "control-plane", + Request: []status2.UpdateRequest{ + { + NsName: client.ObjectKeyFromObject(cfg), + ResourceType: &ngfAPI.NginxGateway{}, + Setter: newNginxGatewayStatusSetter(nginxGwsStatus), + }, + }, } - h.cfg.statusUpdater.Update(ctx, nginxGatewayStatus) + h.cfg.statusUpdater.Update(ctx, groupReq) + logger.Info("Reconfigured control plane.") } } @@ -506,7 +542,18 @@ func (h *eventHandlerImpl) nginxGatewayServiceUpsert(ctx context.Context, logger logger.Error(err, "Setting GatewayStatusAddress to Pod IP Address") } - h.cfg.statusUpdater.UpdateAddresses(ctx, gwAddresses) + graph := h.cfg.processor.GetLatestGraph() + if graph == nil { + return + } + + gatewayStatuses := buildGatewayStatuses(graph.Gateway, graph.IgnoredGateways, gwAddresses, h.latestReloadResult) + gatewaysGroup := status2.GroupUpdateRequest{ + Name: "gateways", + Request: gatewayStatuses, + } + + h.cfg.statusUpdater.Update(ctx, gatewaysGroup) } func (h *eventHandlerImpl) nginxGatewayServiceDelete( @@ -519,7 +566,18 @@ func (h *eventHandlerImpl) nginxGatewayServiceDelete( logger.Error(err, "Setting GatewayStatusAddress to Pod IP Address") } - h.cfg.statusUpdater.UpdateAddresses(ctx, gwAddresses) + graph := h.cfg.processor.GetLatestGraph() + if graph == nil { + return + } + + gatewayStatuses := buildGatewayStatuses(graph.Gateway, graph.IgnoredGateways, gwAddresses, h.latestReloadResult) + gatewaysGroup := status2.GroupUpdateRequest{ + Name: "gateways", + Request: gatewayStatuses, + } + + h.cfg.statusUpdater.Update(ctx, gatewaysGroup) } func (h *eventHandlerImpl) nginxPlusUsageSecretUpsert(_ context.Context, _ logr.Logger, obj client.Object) { diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 21c123ba99..1380b63da4 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -43,7 +43,7 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/framework/gatewayclass" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/runnables" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status2" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/metrics/collectors" ngxcfg "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config" @@ -188,15 +188,12 @@ func StartManager(cfg config.Config) error { ) } - statusUpdater := status.NewUpdater(status.UpdaterConfig{ - GatewayCtlrName: cfg.GatewayCtlrName, - GatewayClassName: cfg.GatewayClassName, - Client: mgr.GetClient(), - Logger: cfg.Logger.WithName("statusUpdater"), - Clock: status.NewRealClock(), - UpdateGatewayClassStatus: cfg.UpdateGatewayClassStatus, - LeaderElectionEnabled: cfg.LeaderElection.Enabled, - }) + statusUpdater := status2.NewUpdater( + mgr.GetClient(), + cfg.Logger.WithName("statusUpdater"), + ) + + cachingGroupStatusUpdater := status2.NewCachingGroupUpdater(statusUpdater) eventHandler := newEventHandlerImpl(eventHandlerConfig{ k8sClient: mgr.GetClient(), @@ -213,7 +210,7 @@ func StartManager(cfg config.Config) error { ngxruntimeCollector, cfg.Logger.WithName("nginxRuntimeManager"), ), - statusUpdater: statusUpdater, + statusUpdater: cachingGroupStatusUpdater, eventRecorder: recorder, nginxConfiguredOnStartChecker: nginxChecker, controlConfigNSName: controlConfigNSName, @@ -240,7 +237,7 @@ func StartManager(cfg config.Config) error { return fmt.Errorf("cannot register event loop: %w", err) } - if err = mgr.Add(runnables.NewEnableAfterBecameLeader(statusUpdater.Enable)); err != nil { + if err = mgr.Add(runnables.NewEnableAfterBecameLeader(cachingGroupStatusUpdater.Enable)); err != nil { return fmt.Errorf("cannot register status updater: %w", err) } diff --git a/internal/mode/static/status_setters.go b/internal/mode/static/status_setters.go new file mode 100644 index 0000000000..de5a1e5d2f --- /dev/null +++ b/internal/mode/static/status_setters.go @@ -0,0 +1,315 @@ +package static + +import ( + "slices" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status2" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph" +) + +func newNginxGatewayStatusSetter(status ngfAPI.NginxGatewayStatus) func(client.Object) bool { + return func(object client.Object) bool { + ng := object.(*ngfAPI.NginxGateway) + + if status2.ConditionsEqual(ng.Status.Conditions, status.Conditions) { + return false + } + + ng.Status = status + + return true + } +} + +func newGatewayStatusSetter(gwStatus gatewayv1.GatewayStatus) func(client.Object) bool { + return func(object client.Object) bool { + gw := object.(*gatewayv1.Gateway) + + if gwStatusEqual(gw.Status, gwStatus) { + return false + } + + gw.Status = gwStatus + + return true + } +} + +func gwStatusEqual(prev, cur gatewayv1.GatewayStatus) bool { + addressesEqual := slices.EqualFunc(prev.Addresses, cur.Addresses, func(a1, a2 gatewayv1.GatewayStatusAddress) bool { + if !equalPointers[gatewayv1.AddressType](a1.Type, a2.Type) { + return false + } + + return a1.Value == a2.Value + }) + + if !addressesEqual { + return false + } + + if !status2.ConditionsEqual(prev.Conditions, cur.Conditions) { + return false + } + + return slices.EqualFunc(prev.Listeners, cur.Listeners, func(s1, s2 gatewayv1.ListenerStatus) bool { + if s1.Name != s2.Name { + return false + } + + if s1.AttachedRoutes != s2.AttachedRoutes { + return false + } + + if !status2.ConditionsEqual(s1.Conditions, s2.Conditions) { + return false + } + + return slices.EqualFunc(s1.SupportedKinds, s2.SupportedKinds, func(k1, k2 gatewayv1.RouteGroupKind) bool { + if k1.Kind != k2.Kind { + return false + } + + return equalPointers(k1.Group, k2.Group) + }) + }) +} + +func newHTTPRouteStatusSetter(hrStatus gatewayv1.HTTPRouteStatus, gatewayCtlrName string) func(client.Object) bool { + return func(object client.Object) bool { + hr := object.(*gatewayv1.HTTPRoute) + + // keep all the parent statuses that belong to other controllers + for _, os := range hr.Status.Parents { + if string(os.ControllerName) != gatewayCtlrName { + hrStatus.Parents = append(hrStatus.Parents, os) + } + } + + if hrStatusEqual(gatewayCtlrName, hr.Status, hrStatus) { + return false + } + + hr.Status = hrStatus + + return true + } +} + +func hrStatusEqual(gatewayCtlrName string, prev, cur gatewayv1.HTTPRouteStatus) bool { + // Since other controllers may update HTTPRoute status we can't assume anything about the order of the statuses, + // and we have to ignore statuses written by other controllers when checking for equality. + // Therefore, we can't use slices.EqualFunc here because it cares about the order. + + // First, we check if the prev status has any RouteParentStatuses that are no longer present in the cur status. + for _, prevParent := range prev.Parents { + if prevParent.ControllerName != gatewayv1.GatewayController(gatewayCtlrName) { + continue + } + + exists := slices.ContainsFunc(cur.Parents, func(curParent gatewayv1.RouteParentStatus) bool { + return routeParentStatusEqual(prevParent, curParent) + }) + + if !exists { + return false + } + } + + // Then, we check if the cur status has any RouteParentStatuses that are no longer present in the prev status. + for _, curParent := range cur.Parents { + exists := slices.ContainsFunc(prev.Parents, func(prevParent gatewayv1.RouteParentStatus) bool { + return routeParentStatusEqual(curParent, prevParent) + }) + + if !exists { + return false + } + } + + return true +} + +func routeParentStatusEqual(p1, p2 gatewayv1.RouteParentStatus) bool { + if p1.ControllerName != p2.ControllerName { + return false + } + + if p1.ParentRef.Name != p2.ParentRef.Name { + return false + } + + if !equalPointers(p1.ParentRef.Namespace, p2.ParentRef.Namespace) { + return false + } + + if !equalPointers(p1.ParentRef.SectionName, p2.ParentRef.SectionName) { + return false + } + + // we ignore the rest of the ParentRef fields because we do not set them + + return status2.ConditionsEqual(p1.Conditions, p2.Conditions) +} + +func newGatewayClassStatusSetter(generation int64, conds []conditions.Condition) func(client.Object) bool { + return func(object client.Object) bool { + gc := object.(*gatewayv1.GatewayClass) + + apiConds := status2.ConvertConditions(conds, generation, metav1.Now()) + + if status2.ConditionsEqual(gc.Status.Conditions, apiConds) { + return false + } + + gc.Status = gatewayv1.GatewayClassStatus{ + Conditions: apiConds, + } + + return true + } +} + +func newBackendTLSPolicyStatusSetter( + gatewayCtlrName string, + policy *graph.BackendTLSPolicy, +) func(client.Object) bool { + return func(object client.Object) bool { + btp := object.(*gatewayv1alpha2.BackendTLSPolicy) + status := prepareBackendTLSPolicyStatus( + btp.Status, + policy.Source.Generation, + policy.Conditions, + policy.Gateway, + gatewayCtlrName, + metav1.Now(), + ) + + if btpStatusEqual(gatewayCtlrName, btp.Status, status) { + return false + } + + btp.Status = status + + return true + } +} + +// prepareBackendTLSPolicyStatus prepares the status for a BackendTLSPolicy resource. +func prepareBackendTLSPolicyStatus( + oldStatus gatewayv1alpha2.PolicyStatus, + observedGeneration int64, + conds []conditions.Condition, + gatewayNsName types.NamespacedName, + gatewayCtlrName string, + transitionTime metav1.Time, +) gatewayv1alpha2.PolicyStatus { + // maxAncestors is the max number of ancestor statuses which is the sum of all new ancestor statuses and all old + // ancestor statuses. + maxAncestors := 1 + len(oldStatus.Ancestors) + ancestors := make([]gatewayv1alpha2.PolicyAncestorStatus, 0, maxAncestors) + + // keep all the ancestor statuses that belong to other controllers + for _, os := range oldStatus.Ancestors { + if string(os.ControllerName) != gatewayCtlrName { + ancestors = append(ancestors, os) + } + } + + a := gatewayv1alpha2.PolicyAncestorStatus{ + AncestorRef: gatewayv1.ParentReference{ + Namespace: (*gatewayv1.Namespace)(&gatewayNsName.Namespace), + Name: gatewayv1alpha2.ObjectName(gatewayNsName.Name), + }, + ControllerName: gatewayv1alpha2.GatewayController(gatewayCtlrName), + Conditions: status2.ConvertConditions(conds, observedGeneration, transitionTime), + } + ancestors = append(ancestors, a) + + return gatewayv1alpha2.PolicyStatus{ + Ancestors: ancestors, + } +} + +func btpStatusEqual(gatewayCtlrName string, prev, cur gatewayv1alpha2.PolicyStatus) bool { + // Since other controllers may update BackendTLSPolicy status we can't assume anything about the order of the + // statuses, and we have to ignore statuses written by other controllers when checking for equality. + // Therefore, we can't use slices.EqualFunc here because it cares about the order. + + // First, we check if the prev status has any PolicyAncestorStatuses that are no longer present in the cur status. + for _, prevAncestor := range prev.Ancestors { + if prevAncestor.ControllerName != gatewayv1.GatewayController(gatewayCtlrName) { + continue + } + + exists := slices.ContainsFunc(cur.Ancestors, func(curAncestor gatewayv1alpha2.PolicyAncestorStatus) bool { + return btpAncestorStatusEqual(prevAncestor, curAncestor) + }) + + if !exists { + return false + } + } + + // Then, we check if the cur status has any PolicyAncestorStatuses that are no longer present in the prev status. + for _, curParent := range cur.Ancestors { + exists := slices.ContainsFunc(prev.Ancestors, func(prevAncestor gatewayv1alpha2.PolicyAncestorStatus) bool { + return btpAncestorStatusEqual(curParent, prevAncestor) + }) + + if !exists { + return false + } + } + + return true +} + +func btpAncestorStatusEqual(p1, p2 gatewayv1alpha2.PolicyAncestorStatus) bool { + if p1.ControllerName != p2.ControllerName { + return false + } + + if p1.AncestorRef.Name != p2.AncestorRef.Name { + return false + } + + if !equalPointers(p1.AncestorRef.Namespace, p2.AncestorRef.Namespace) { + return false + } + + // we ignore the rest of the AncestorRef fields because we do not set them + + return status2.ConditionsEqual(p1.Conditions, p2.Conditions) +} + +// equalPointers returns whether two pointers are equal. +// Pointers are considered equal if one of the following is true: +// - They are both nil. +// - One is nil and the other is empty (e.g. nil string and ""). +// - They are both non-nil, and their values are the same. +func equalPointers[T comparable](p1, p2 *T) bool { + if p1 == nil && p2 == nil { + return true + } + + var p1Val, p2Val T + + if p1 != nil { + p1Val = *p1 + } + + if p2 != nil { + p2Val = *p2 + } + + return p1Val == p2Val +}