chore: update status only when changes occur (#2473) (#208)

Co-authored-by: AlinsRan <[email protected]>

Author: ronething

internal/controller/consumer_controller.go

@@ -19,7 +19,6 @@ package controller
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
@@ -276,13 +275,9 @@ func (r *ConsumerReconciler) updateStatus(consumer *v1alpha1.Consumer, err error
 		NamespacedName: utils.NamespacedName(consumer),
 		Resource:       consumer.DeepCopy(),
 		Mutator: status.MutatorFunc(func(obj client.Object) client.Object {
-			t, ok := obj.(*v1alpha1.Consumer)
-			if !ok {
-				err := fmt.Errorf("unsupported object type %T", obj)
-				panic(err)
-			}
-			t.Status = consumer.Status
-			return t
+			cp := obj.(*v1alpha1.Consumer).DeepCopy()
+			cp.Status = consumer.Status
+			return cp
 		}),
 	})
 }

internal/controller/gatewayclass_controller.go

@@ -121,13 +121,9 @@ func (r *GatewayClassReconciler) Reconcile(ctx context.Context, req ctrl.Request
 			NamespacedName: utils.NamespacedName(gc),
 			Resource:       gc.DeepCopy(),
 			Mutator: status.MutatorFunc(func(obj client.Object) client.Object {
-				t, ok := obj.(*gatewayv1.GatewayClass)
-				if !ok {
-					err := fmt.Errorf("unsupported object type %T", obj)
-					panic(err)
-				}
-				t.Status = gc.Status
-				return t
+				cp := obj.(*gatewayv1.GatewayClass).DeepCopy()
+				cp.Status = gc.Status
+				return cp
 			}),
 		})
 	}

internal/controller/httproutepolicy.go

@@ -20,7 +20,6 @@ package controller
 import (
 	"cmp"
 	"context"
-	"fmt"
 	"slices"
 
 	networkingv1 "k8s.io/api/networking/v1"
@@ -92,13 +91,9 @@ func (r *HTTPRouteReconciler) processHTTPRoutePolicies(tctx *provider.TranslateC
 				NamespacedName: utils.NamespacedName(&policy),
 				Resource:       policy.DeepCopy(),
 				Mutator: status.MutatorFunc(func(obj client.Object) client.Object {
-					t, ok := obj.(*v1alpha1.HTTPRoutePolicy)
-					if !ok {
-						err := fmt.Errorf("unsupported object type %T", obj)
-						panic(err)
-					}
-					t.Status = policy.Status
-					return t
+					cp := obj.(*v1alpha1.HTTPRoutePolicy).DeepCopy()
+					cp.Status = policy.Status
+					return cp
 				}),
 			})
 		}
@@ -170,13 +165,9 @@ func (r *IngressReconciler) processHTTPRoutePolicies(tctx *provider.TranslateCon
 				NamespacedName: utils.NamespacedName(&policy),
 				Resource:       policy.DeepCopy(),
 				Mutator: status.MutatorFunc(func(obj client.Object) client.Object {
-					t, ok := obj.(*v1alpha1.HTTPRoutePolicy)
-					if !ok {
-						err := fmt.Errorf("unsupported object type %T", obj)
-						panic(err)
-					}
-					t.Status = policy.Status
-					return t
+					cp := obj.(*v1alpha1.HTTPRoutePolicy).DeepCopy()
+					cp.Status = policy.Status
+					return cp
 				}),
 			})
 		}
@@ -282,13 +273,9 @@ func updateDeleteAncestors(updater status.Updater, policy v1alpha1.HTTPRoutePoli
 			NamespacedName: utils.NamespacedName(&policy),
 			Resource:       policy.DeepCopy(),
 			Mutator: status.MutatorFunc(func(obj client.Object) client.Object {
-				t, ok := obj.(*v1alpha1.HTTPRoutePolicy)
-				if !ok {
-					err := fmt.Errorf("unsupported object type %T", obj)
-					panic(err)
-				}
-				t.Status = policy.Status
-				return t
+				cp := obj.(*v1alpha1.HTTPRoutePolicy).DeepCopy()
+				cp.Status = policy.Status
+				return cp
 			}),
 		})
 	}

internal/controller/ingress_controller.go

@@ -639,13 +639,9 @@ func (r *IngressReconciler) updateStatus(ctx context.Context, tctx *provider.Tra
 			NamespacedName: utils.NamespacedName(ingress),
 			Resource:       ingress.DeepCopy(),
 			Mutator: status.MutatorFunc(func(obj client.Object) client.Object {
-				t, ok := obj.(*networkingv1.Ingress)
-				if !ok {
-					err := fmt.Errorf("unsupported object type %T", obj)
-					panic(err)
-				}
-				t.Status = ingress.Status
-				return t
+				cp := obj.(*networkingv1.Ingress).DeepCopy()
+				cp.Status = ingress.Status
+				return cp
 			}),
 		})
 		return nil

internal/controller/policies.go

@@ -158,13 +158,9 @@ func ProcessBackendTrafficPolicy(
 				NamespacedName: utils.NamespacedName(policy),
 				Resource:       policy.DeepCopy(),
 				Mutator: status.MutatorFunc(func(obj client.Object) client.Object {
-					t, ok := obj.(*v1alpha1.BackendTrafficPolicy)
-					if !ok {
-						err := fmt.Errorf("unsupported object type %T", obj)
-						panic(err)
-					}
-					t.Status = policy.Status
-					return t
+					cp := obj.(*v1alpha1.BackendTrafficPolicy).DeepCopy()
+					cp.Status = policy.Status
+					return cp
 				}),
 			})
 		}

internal/controller/status/updater.go

@@ -22,18 +22,25 @@ import (
 	"sync"
 
 	"github.com/go-logr/logr"
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/types"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	k8stypes "k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/util/retry"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
 
+	"github.com/apache/apisix-ingress-controller/api/v1alpha1"
+	v2 "github.com/apache/apisix-ingress-controller/api/v2"
+	types "github.com/apache/apisix-ingress-controller/internal/types"
 	pkgmetrics "github.com/apache/apisix-ingress-controller/pkg/metrics"
 )
 
 const UpdateChannelBufferSize = 1000
 
 type Update struct {
-	NamespacedName types.NamespacedName
+	NamespacedName k8stypes.NamespacedName
 	Resource       client.Object
 	Mutator        Mutator
 }
@@ -52,6 +59,13 @@ func (m MutatorFunc) Mutate(obj client.Object) client.Object {
 	return m(obj)
 }
 
+var cmpIgnoreLastTT = cmp.Options{
+	cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime"),
+	cmpopts.IgnoreMapEntries(func(k string, _ any) bool {
+		return k == "lastTransitionTime"
+	}),
+}
+
 type UpdateHandler struct {
 	log           logr.Logger
 	client        client.Client
@@ -96,8 +110,20 @@ func (u *UpdateHandler) updateStatus(ctx context.Context, update Update) error {
 		return nil
 	}
 
+	if statusEqual(obj, newObj, cmpIgnoreLastTT) {
+		u.log.V(1).Info("status is equal, skipping update", "name", update.NamespacedName.Name,
+			"namespace", update.NamespacedName.Namespace,
+			"kind", types.KindOf(obj))
+		return nil
+	}
+
 	newObj.SetUID(obj.GetUID())
 
+	u.log.Info("updating status", "name", update.NamespacedName.Name,
+		"namespace", update.NamespacedName.Namespace,
+		"kind", types.KindOf(newObj),
+	)
+
 	return u.client.Status().Update(ctx, newObj)
 }
 
@@ -114,8 +140,10 @@ func (u *UpdateHandler) Start(ctx context.Context) error {
 		case update := <-u.updateChannel:
 			// Decrement queue length after removing item from queue
 			pkgmetrics.DecStatusQueueLength()
-			u.log.Info("received a status update", "namespace", update.NamespacedName.Namespace,
-				"name", update.NamespacedName.Name)
+			u.log.V(1).Info("received a status update", "namespace", update.NamespacedName.Namespace,
+				"name", update.NamespacedName.Name,
+				"kind", types.KindOf(update.Resource),
+			)
 
 			u.apply(ctx, update)
 		}
@@ -144,3 +172,82 @@ func (u *UpdateWriter) Update(update Update) {
 	// Increment queue length after adding new item
 	pkgmetrics.IncStatusQueueLength()
 }
+
+func statusEqual(a, b any, opts ...cmp.Option) bool {
+	var statusA, statusB any
+
+	switch a := a.(type) {
+	case *gatewayv1.GatewayClass:
+		b, ok := b.(*gatewayv1.GatewayClass)
+		if !ok {
+			return false
+		}
+		statusA, statusB = a.Status, b.Status
+
+	case *gatewayv1.Gateway:
+		b, ok := b.(*gatewayv1.Gateway)
+		if !ok {
+			return false
+		}
+		statusA, statusB = a.Status, b.Status
+
+	case *gatewayv1.HTTPRoute:
+		b, ok := b.(*gatewayv1.HTTPRoute)
+		if !ok {
+			return false
+		}
+		statusA, statusB = a.Status, b.Status
+	case *v2.ApisixRoute:
+		b, ok := b.(*v2.ApisixRoute)
+		if !ok {
+			return false
+		}
+		statusA, statusB = a.Status, b.Status
+	case *v2.ApisixGlobalRule:
+		b, ok := b.(*v2.ApisixGlobalRule)
+		if !ok {
+			return false
+		}
+		statusA, statusB = a.Status, b.Status
+	case *v2.ApisixPluginConfig:
+		b, ok := b.(*v2.ApisixPluginConfig)
+		if !ok {
+			return false
+		}
+		statusA, statusB = a.Status, b.Status
+	case *v2.ApisixTls:
+		b, ok := b.(*v2.ApisixTls)
+		if !ok {
+			return false
+		}
+		statusA, statusB = a.Status, b.Status
+	case *v2.ApisixConsumer:
+		b, ok := b.(*v2.ApisixConsumer)
+		if !ok {
+			return false
+		}
+		statusA, statusB = a.Status, b.Status
+	case *v1alpha1.HTTPRoutePolicy:
+		b, ok := b.(*v1alpha1.HTTPRoutePolicy)
+		if !ok {
+			return false
+		}
+		statusA, statusB = a.Status, b.Status
+	case *v1alpha1.BackendTrafficPolicy:
+		b, ok := b.(*v1alpha1.BackendTrafficPolicy)
+		if !ok {
+			return false
+		}
+		statusA, statusB = a.Status, b.Status
+	case *v1alpha1.Consumer:
+		b, ok := b.(*v1alpha1.Consumer)
+		if !ok {
+			return false
+		}
+		statusA, statusB = a.Status, b.Status
+	default:
+		return false
+	}
+
+	return cmp.Equal(statusA, statusB, opts...)
+}

internal/provider/adc/config.go

@@ -207,7 +207,6 @@ func (d *adcClient) updateConfigForGatewayProxy(tctx *provider.TranslateContext,
 	for _, ref := range referrers {
 		d.configs[ref] = *config
 	}
-
 	return nil
 }
 

internal/types/k8s.go

@@ -17,21 +17,76 @@
 
 package types
 
+import (
+	"github.com/apache/apisix-ingress-controller/api/v1alpha1"
+	v2 "github.com/apache/apisix-ingress-controller/api/v2"
+	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/api/networking/v1"
+	gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
+)
+
 const DefaultIngressClassAnnotation = "ingressclass.kubernetes.io/is-default-class"
 
 const (
-	KindGateway            = "Gateway"
-	KindHTTPRoute          = "HTTPRoute"
-	KindGatewayClass       = "GatewayClass"
-	KindIngress            = "Ingress"
-	KindIngressClass       = "IngressClass"
-	KindGatewayProxy       = "GatewayProxy"
-	KindSecret             = "Secret"
-	KindService            = "Service"
-	KindApisixRoute        = "ApisixRoute"
-	KindApisixGlobalRule   = "ApisixGlobalRule"
-	KindApisixPluginConfig = "ApisixPluginConfig"
-	KindPod                = "Pod"
-	KindApisixTls          = "ApisixTls"
-	KindApisixConsumer     = "ApisixConsumer"
+	KindGateway              = "Gateway"
+	KindHTTPRoute            = "HTTPRoute"
+	KindGatewayClass         = "GatewayClass"
+	KindIngress              = "Ingress"
+	KindIngressClass         = "IngressClass"
+	KindGatewayProxy         = "GatewayProxy"
+	KindSecret               = "Secret"
+	KindService              = "Service"
+	KindApisixRoute          = "ApisixRoute"
+	KindApisixGlobalRule     = "ApisixGlobalRule"
+	KindApisixPluginConfig   = "ApisixPluginConfig"
+	KindPod                  = "Pod"
+	KindApisixTls            = "ApisixTls"
+	KindApisixConsumer       = "ApisixConsumer"
+	KindHTTPRoutePolicy      = "HTTPRoutePolicy"
+	KindBackendTrafficPolicy = "BackendTrafficPolicy"
+	KindConsumer             = "Consumer"
+	KindPluginConfig         = "PluginConfig"
 )
+
+func KindOf(obj any) string {
+	switch obj.(type) {
+	case *gatewayv1.Gateway:
+		return KindGateway
+	case *gatewayv1.HTTPRoute:
+		return KindHTTPRoute
+	case *gatewayv1.GatewayClass:
+		return KindGatewayClass
+	case *v1.Ingress:
+		return KindIngress
+	case *v1.IngressClass:
+		return KindIngressClass
+	case *corev1.Secret:
+		return KindSecret
+	case *corev1.Service:
+		return KindService
+	case *v2.ApisixRoute:
+		return KindApisixRoute
+	case *v2.ApisixGlobalRule:
+		return KindApisixGlobalRule
+	case *v2.ApisixPluginConfig:
+		return KindApisixPluginConfig
+	case *corev1.Pod:
+		return KindPod
+	case *v2.ApisixTls:
+		return KindApisixTls
+	case *v2.ApisixConsumer:
+		return KindApisixConsumer
+	case *v1alpha1.HTTPRoutePolicy:
+		return KindHTTPRoutePolicy
+	case *v1alpha1.BackendTrafficPolicy:
+		return KindBackendTrafficPolicy
+	case *v1alpha1.GatewayProxy:
+		return KindGatewayProxy
+	case *v1alpha1.Consumer:
+		return KindConsumer
+	case *v1alpha1.PluginConfig:
+		return KindPluginConfig
+	default:
+		return "Unknown"
+	}
+}