Do not update route status when nothing changed

This commit is contained in:
Kevin Pollet 2024-07-29 15:48:05 +02:00 committed by GitHub
parent 386c2ffb20
commit 7dbd3f88f6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 116 additions and 63 deletions

View file

@ -5,6 +5,7 @@ import (
"errors" "errors"
"fmt" "fmt"
"os" "os"
"reflect"
"slices" "slices"
"time" "time"
@ -53,7 +54,7 @@ func (reh *resourceEventHandler) OnDelete(obj interface{}) {
type Client interface { type Client interface {
WatchAll(namespaces []string, stopCh <-chan struct{}) (<-chan interface{}, error) WatchAll(namespaces []string, stopCh <-chan struct{}) (<-chan interface{}, error)
UpdateGatewayStatus(ctx context.Context, gateway ktypes.NamespacedName, status gatev1.GatewayStatus) error UpdateGatewayStatus(ctx context.Context, gateway ktypes.NamespacedName, status gatev1.GatewayStatus) error
UpdateGatewayClassStatus(ctx context.Context, name string, condition metav1.Condition) error UpdateGatewayClassStatus(ctx context.Context, name string, status gatev1.GatewayClassStatus) error
UpdateHTTPRouteStatus(ctx context.Context, route ktypes.NamespacedName, status gatev1.HTTPRouteStatus) error UpdateHTTPRouteStatus(ctx context.Context, route ktypes.NamespacedName, status gatev1.HTTPRouteStatus) error
UpdateTCPRouteStatus(ctx context.Context, route ktypes.NamespacedName, status gatev1alpha2.TCPRouteStatus) error UpdateTCPRouteStatus(ctx context.Context, route ktypes.NamespacedName, status gatev1alpha2.TCPRouteStatus) error
UpdateTLSRouteStatus(ctx context.Context, route ktypes.NamespacedName, status gatev1alpha2.TLSRouteStatus) error UpdateTLSRouteStatus(ctx context.Context, route ktypes.NamespacedName, status gatev1alpha2.TLSRouteStatus) error
@ -378,7 +379,7 @@ func (c *clientWrapper) ListGatewayClasses() ([]*gatev1.GatewayClass, error) {
return c.factoryGatewayClass.Gateway().V1().GatewayClasses().Lister().List(labels.Everything()) return c.factoryGatewayClass.Gateway().V1().GatewayClasses().Lister().List(labels.Everything())
} }
func (c *clientWrapper) UpdateGatewayClassStatus(ctx context.Context, name string, condition metav1.Condition) error { func (c *clientWrapper) UpdateGatewayClassStatus(ctx context.Context, name string, status gatev1.GatewayClassStatus) error {
err := retry.RetryOnConflict(retry.DefaultRetry, func() error { err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
currentGatewayClass, err := c.factoryGatewayClass.Gateway().V1().GatewayClasses().Lister().Get(name) currentGatewayClass, err := c.factoryGatewayClass.Gateway().V1().GatewayClasses().Lister().Get(name)
if err != nil { if err != nil {
@ -387,23 +388,12 @@ func (c *clientWrapper) UpdateGatewayClassStatus(ctx context.Context, name strin
return err return err
} }
currentGatewayClass = currentGatewayClass.DeepCopy() if conditionsEqual(currentGatewayClass.Status.Conditions, status.Conditions) {
var newConditions []metav1.Condition
for _, cond := range currentGatewayClass.Status.Conditions {
// No update for identical condition.
if cond.Type == condition.Type && cond.Status == condition.Status && cond.ObservedGeneration == condition.ObservedGeneration {
return nil return nil
} }
// Keep other condition types. currentGatewayClass = currentGatewayClass.DeepCopy()
if cond.Type != condition.Type { currentGatewayClass.Status = status
newConditions = append(newConditions, cond)
}
}
// Append the condition to update.
newConditions = append(newConditions, condition)
currentGatewayClass.Status.Conditions = newConditions
if _, err = c.csGateway.GatewayV1().GatewayClasses().UpdateStatus(ctx, currentGatewayClass, metav1.UpdateOptions{}); err != nil { if _, err = c.csGateway.GatewayV1().GatewayClasses().UpdateStatus(ctx, currentGatewayClass, metav1.UpdateOptions{}); err != nil {
// We have to return err itself here (not wrapped inside another error) // We have to return err itself here (not wrapped inside another error)
@ -433,7 +423,7 @@ func (c *clientWrapper) UpdateGatewayStatus(ctx context.Context, gateway ktypes.
return err return err
} }
if gatewayStatusEquals(currentGateway.Status, status) { if gatewayStatusEqual(currentGateway.Status, status) {
return nil return nil
} }
@ -468,16 +458,22 @@ func (c *clientWrapper) UpdateHTTPRouteStatus(ctx context.Context, route ktypes.
return err return err
} }
// TODO: keep statuses for gateways managed by other Traefik instances. parentStatuses := make([]gatev1.RouteParentStatus, len(status.Parents))
var parentStatuses []gatev1.RouteParentStatus copy(parentStatuses, status.Parents)
for _, currentParentStatus := range currentRoute.Status.Parents {
if currentParentStatus.ControllerName != controllerName { // keep statuses added by other gateway controllers.
parentStatuses = append(parentStatuses, currentParentStatus) // TODO: we should also keep statuses for gateways managed by other Traefik instances.
for _, parentStatus := range currentRoute.Status.Parents {
if parentStatus.ControllerName != controllerName {
parentStatuses = append(parentStatuses, parentStatus)
continue continue
} }
} }
parentStatuses = append(parentStatuses, status.Parents...) // do not update status when nothing has changed.
if routeParentStatusesEqual(currentRoute.Status.Parents, parentStatuses) {
return nil
}
currentRoute = currentRoute.DeepCopy() currentRoute = currentRoute.DeepCopy()
currentRoute.Status = gatev1.HTTPRouteStatus{ currentRoute.Status = gatev1.HTTPRouteStatus{
@ -514,16 +510,22 @@ func (c *clientWrapper) UpdateTCPRouteStatus(ctx context.Context, route ktypes.N
return err return err
} }
// TODO: keep statuses for gateways managed by other Traefik instances. parentStatuses := make([]gatev1.RouteParentStatus, len(status.Parents))
var parentStatuses []gatev1alpha2.RouteParentStatus copy(parentStatuses, status.Parents)
for _, currentParentStatus := range currentRoute.Status.Parents {
if currentParentStatus.ControllerName != controllerName { // keep statuses added by other gateway controllers.
parentStatuses = append(parentStatuses, currentParentStatus) // TODO: we should also keep statuses for gateways managed by other Traefik instances.
for _, parentStatus := range currentRoute.Status.Parents {
if parentStatus.ControllerName != controllerName {
parentStatuses = append(parentStatuses, parentStatus)
continue continue
} }
} }
parentStatuses = append(parentStatuses, status.Parents...) // do not update status when nothing has changed.
if routeParentStatusesEqual(currentRoute.Status.Parents, parentStatuses) {
return nil
}
currentRoute = currentRoute.DeepCopy() currentRoute = currentRoute.DeepCopy()
currentRoute.Status = gatev1alpha2.TCPRouteStatus{ currentRoute.Status = gatev1alpha2.TCPRouteStatus{
@ -560,16 +562,22 @@ func (c *clientWrapper) UpdateTLSRouteStatus(ctx context.Context, route ktypes.N
return err return err
} }
// TODO: keep statuses for gateways managed by other Traefik instances. parentStatuses := make([]gatev1.RouteParentStatus, len(status.Parents))
var parentStatuses []gatev1alpha2.RouteParentStatus copy(parentStatuses, status.Parents)
for _, currentParentStatus := range currentRoute.Status.Parents {
if currentParentStatus.ControllerName != controllerName { // keep statuses added by other gateway controllers.
parentStatuses = append(parentStatuses, currentParentStatus) // TODO: we should also keep statuses for gateways managed by other Traefik instances.
for _, parentStatus := range currentRoute.Status.Parents {
if parentStatus.ControllerName != controllerName {
parentStatuses = append(parentStatuses, parentStatus)
continue continue
} }
} }
parentStatuses = append(parentStatuses, status.Parents...) // do not update status when nothing has changed.
if routeParentStatusesEqual(currentRoute.Status.Parents, parentStatuses) {
return nil
}
currentRoute = currentRoute.DeepCopy() currentRoute = currentRoute.DeepCopy()
currentRoute.Status = gatev1alpha2.TLSRouteStatus{ currentRoute.Status = gatev1alpha2.TLSRouteStatus{
@ -675,12 +683,12 @@ func translateNotFoundError(err error) (bool, error) {
return err == nil, err return err == nil, err
} }
func gatewayStatusEquals(statusA, statusB gatev1.GatewayStatus) bool { func gatewayStatusEqual(statusA, statusB gatev1.GatewayStatus) bool {
if len(statusA.Listeners) != len(statusB.Listeners) { if len(statusA.Listeners) != len(statusB.Listeners) {
return false return false
} }
if !conditionsEquals(statusA.Conditions, statusB.Conditions) { if !conditionsEqual(statusA.Conditions, statusB.Conditions) {
return false return false
} }
@ -688,7 +696,7 @@ func gatewayStatusEquals(statusA, statusB gatev1.GatewayStatus) bool {
for _, newListener := range statusB.Listeners { for _, newListener := range statusB.Listeners {
for _, oldListener := range statusA.Listeners { for _, oldListener := range statusA.Listeners {
if newListener.Name == oldListener.Name { if newListener.Name == oldListener.Name {
if !conditionsEquals(newListener.Conditions, oldListener.Conditions) { if !conditionsEqual(newListener.Conditions, oldListener.Conditions) {
return false return false
} }
@ -704,22 +712,48 @@ func gatewayStatusEquals(statusA, statusB gatev1.GatewayStatus) bool {
return listenerMatches == len(statusA.Listeners) return listenerMatches == len(statusA.Listeners)
} }
func conditionsEquals(conditionsA, conditionsB []metav1.Condition) bool { func routeParentStatusesEqual(routeParentStatusesA, routeParentStatusesB []gatev1alpha2.RouteParentStatus) bool {
if len(conditionsA) != len(conditionsB) { if len(routeParentStatusesA) != len(routeParentStatusesB) {
return false return false
} }
conditionMatches := 0 for _, sA := range routeParentStatusesA {
for _, conditionA := range conditionsA { if !slices.ContainsFunc(routeParentStatusesB, func(sB gatev1alpha2.RouteParentStatus) bool {
for _, conditionB := range conditionsB { return routeParentStatusEqual(sB, sA)
if conditionA.Type == conditionB.Type { }) {
if conditionA.Reason != conditionB.Reason || conditionA.Status != conditionB.Status || conditionA.Message != conditionB.Message || conditionA.ObservedGeneration != conditionB.ObservedGeneration {
return false return false
} }
conditionMatches++
}
}
} }
return conditionMatches == len(conditionsA) for _, sB := range routeParentStatusesB {
if !slices.ContainsFunc(routeParentStatusesA, func(sA gatev1alpha2.RouteParentStatus) bool {
return routeParentStatusEqual(sA, sB)
}) {
return false
}
}
return true
}
func routeParentStatusEqual(sA, sB gatev1alpha2.RouteParentStatus) bool {
if !reflect.DeepEqual(sA.ParentRef, sB.ParentRef) {
return false
}
if sA.ControllerName != sB.ControllerName {
return false
}
return conditionsEqual(sA.Conditions, sB.Conditions)
}
func conditionsEqual(conditionsA, conditionsB []metav1.Condition) bool {
return slices.EqualFunc(conditionsA, conditionsB, func(cA metav1.Condition, cB metav1.Condition) bool {
return cA.Type == cB.Type &&
cA.Reason == cB.Reason &&
cA.Status == cB.Status &&
cA.Message == cB.Message &&
cA.ObservedGeneration == cB.ObservedGeneration
})
} }

View file

@ -268,7 +268,7 @@ func Test_gatewayStatusEquals(t *testing.T) {
t.Run(test.desc, func(t *testing.T) { t.Run(test.desc, func(t *testing.T) {
t.Parallel() t.Parallel()
result := gatewayStatusEquals(test.statusA, test.statusB) result := gatewayStatusEqual(test.statusA, test.statusB)
assert.Equal(t, test.expected, result) assert.Equal(t, test.expected, result)
}) })

View file

@ -318,15 +318,18 @@ func (p *Provider) loadConfigurationFromGateways(ctx context.Context) *dynamic.C
gatewayClassNames[gatewayClass.Name] = struct{}{} gatewayClassNames[gatewayClass.Name] = struct{}{}
err := p.client.UpdateGatewayClassStatus(ctx, gatewayClass.Name, metav1.Condition{ status := gatev1.GatewayClassStatus{
Conditions: upsertGatewayClassConditionAccepted(gatewayClass.Status.Conditions, metav1.Condition{
Type: string(gatev1.GatewayClassConditionStatusAccepted), Type: string(gatev1.GatewayClassConditionStatusAccepted),
Status: metav1.ConditionTrue, Status: metav1.ConditionTrue,
ObservedGeneration: gatewayClass.Generation, ObservedGeneration: gatewayClass.Generation,
Reason: "Handled", Reason: "Handled",
Message: "Handled by Traefik controller", Message: "Handled by Traefik controller",
LastTransitionTime: metav1.Now(), LastTransitionTime: metav1.Now(),
}) }),
if err != nil { }
if err := p.client.UpdateGatewayClassStatus(ctx, gatewayClass.Name, status); err != nil {
log.Ctx(ctx). log.Ctx(ctx).
Warn(). Warn().
Err(err). Err(err).
@ -1228,3 +1231,14 @@ func upsertRouteConditionResolvedRefs(conditions []metav1.Condition, condition m
} }
return append(conds, condition) return append(conds, condition)
} }
func upsertGatewayClassConditionAccepted(conditions []metav1.Condition, condition metav1.Condition) []metav1.Condition {
var conds []metav1.Condition
for _, c := range conditions {
if c.Type == string(gatev1.GatewayClassConditionStatusAccepted) {
continue
}
conds = append(conds, c)
}
return append(conds, condition)
}

View file

@ -27,7 +27,10 @@ func (p *Provider) loadTCPRoutes(ctx context.Context, gatewayListeners []gateway
} }
for _, route := range routes { for _, route := range routes {
logger := log.Ctx(ctx).With().Str("tcproute", route.Name).Str("namespace", route.Namespace).Logger() logger := log.Ctx(ctx).With().
Str("tcp_route", route.Name).
Str("namespace", route.Namespace).
Logger()
var parentStatuses []gatev1alpha2.RouteParentStatus var parentStatuses []gatev1alpha2.RouteParentStatus
for _, parentRef := range route.Spec.ParentRefs { for _, parentRef := range route.Spec.ParentRefs {

View file

@ -24,7 +24,9 @@ func (p *Provider) loadTLSRoutes(ctx context.Context, gatewayListeners []gateway
} }
for _, route := range routes { for _, route := range routes {
logger := log.Ctx(ctx).With().Str("tlsroute", route.Name).Str("namespace", route.Namespace).Logger() logger := log.Ctx(ctx).With().
Str("tls_route", route.Name).
Str("namespace", route.Namespace).Logger()
var parentStatuses []gatev1alpha2.RouteParentStatus var parentStatuses []gatev1alpha2.RouteParentStatus
for _, parentRef := range route.Spec.ParentRefs { for _, parentRef := range route.Spec.ParentRefs {