Skip to content

perf: do not set last transition time for status in watcher layer #7268

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

perf: do not set last transition time for status in watcher layer #7268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions internal/gatewayapi/status/backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
package status

import (
"time"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
Expand All @@ -33,10 +31,10 @@ func computeBackendAcceptedCondition(be *egv1a1.Backend, accepted bool, msg stri
case true:
return newCondition(string(egv1a1.BackendReasonAccepted), metav1.ConditionTrue,
string(egv1a1.BackendConditionAccepted),
"The Backend was accepted", time.Now(), be.Generation)
"The Backend was accepted", be.Generation)
default:
return newCondition(string(egv1a1.BackendReasonInvalid), metav1.ConditionFalse,
string(egv1a1.BackendConditionAccepted),
msg, time.Now(), be.Generation)
msg, be.Generation)
}
}
5 changes: 1 addition & 4 deletions internal/gatewayapi/status/conditions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
package status

import (
"time"
"unicode"

"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -42,7 +41,6 @@ func MergeConditions(conditions []metav1.Condition, updates ...metav1.Condition)
conditions[j].Reason = updates[i].Reason
conditions[j].Message = updates[i].Message
conditions[j].ObservedGeneration = updates[i].ObservedGeneration
conditions[j].LastTransitionTime = updates[i].LastTransitionTime
break
}
}
Expand All @@ -55,13 +53,12 @@ func MergeConditions(conditions []metav1.Condition, updates ...metav1.Condition)
return conditions
}

func newCondition(t string, status metav1.ConditionStatus, reason, msg string, lt time.Time, og int64) metav1.Condition {
func newCondition(t string, status metav1.ConditionStatus, reason, msg string, og int64) metav1.Condition {
return metav1.Condition{
Type: t,
Status: status,
Reason: reason,
Message: truncateConditionMessage(msg),
LastTransitionTime: metav1.NewTime(lt),
ObservedGeneration: og,
}
}
Expand Down
52 changes: 16 additions & 36 deletions internal/gatewayapi/status/conditions_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,12 @@ import (
"errors"
"strings"
"testing"
"time"

"github.com/stretchr/testify/assert"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilclock "k8s.io/utils/clock"
fakeclock "k8s.io/utils/clock/testing"
gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
)

var clock utilclock.Clock = utilclock.RealClock{}

func TestConditionChanged(t *testing.T) {
testCases := []struct {
name string
Expand Down Expand Up @@ -95,17 +90,6 @@ func TestConditionChanged(t *testing.T) {
}

func TestMergeConditions(t *testing.T) {
// Inject a fake clock and don't forget to reset it
fakeClock := fakeclock.NewFakeClock(time.Time{})
clock = fakeClock
defer func() {
clock = utilclock.RealClock{}
}()

start := fakeClock.Now()
middle := start.Add(1 * time.Minute)
later := start.Add(2 * time.Minute)

gen := int64(1)

testCases := []struct {
Expand All @@ -117,69 +101,65 @@ func TestMergeConditions(t *testing.T) {
{
name: "status updated",
current: []metav1.Condition{
newCondition("available", "false", "Reason", "Message", start, gen),
newCondition("available", "false", "Reason", "Message", gen),
},
updates: []metav1.Condition{
newCondition("available", "true", "Reason", "Message", middle, gen),
newCondition("available", "true", "Reason", "Message", gen),
},
expected: []metav1.Condition{
newCondition("available", "true", "Reason", "Message", middle, gen),
newCondition("available", "true", "Reason", "Message", gen),
},
},
{
name: "reason updated",
current: []metav1.Condition{
newCondition("available", "false", "Reason", "Message", start, gen),
newCondition("available", "false", "Reason", "Message", gen),
},
updates: []metav1.Condition{
newCondition("available", "false", "New Reason", "Message", middle, gen),
newCondition("available", "false", "New Reason", "Message", gen),
},
expected: []metav1.Condition{
newCondition("available", "false", "New Reason", "Message", middle, gen),
newCondition("available", "false", "New Reason", "Message", gen),
},
},
{
name: "message updated",
current: []metav1.Condition{
newCondition("available", "false", "Reason", "Message", start, gen),
newCondition("available", "false", "Reason", "Message", gen),
},
updates: []metav1.Condition{
newCondition("available", "false", "Reason", "New Message", middle, gen),
newCondition("available", "false", "Reason", "New Message", gen),
},
expected: []metav1.Condition{
newCondition("available", "false", "Reason", "New Message", middle, gen),
newCondition("available", "false", "Reason", "New Message", gen),
},
},
{
name: "observed generation updated",
current: []metav1.Condition{
newCondition("available", "false", "Reason", "Message", start, gen),
newCondition("available", "false", "Reason", "Message", gen),
},
updates: []metav1.Condition{
newCondition("available", "false", "Reason", "Message", middle, gen+1),
newCondition("available", "false", "Reason", "Message", gen+1),
},
expected: []metav1.Condition{
newCondition("available", "false", "Reason", "Message", middle, gen+1),
newCondition("available", "false", "Reason", "Message", gen+1),
},
},
{
name: "status unchanged",
current: []metav1.Condition{
newCondition("available", "false", "Reason", "Message", start, gen),
newCondition("available", "false", "Reason", "Message", gen),
},
updates: []metav1.Condition{
newCondition("available", "false", "Reason", "Message", middle, gen),
newCondition("available", "false", "Reason", "Message", gen),
},
expected: []metav1.Condition{
newCondition("available", "false", "Reason", "Message", start, gen),
newCondition("available", "false", "Reason", "Message", gen),
},
},
}

// Simulate the passage of time between original condition creation
// and update processing
fakeClock.SetTime(later)

for _, tc := range testCases {
got := MergeConditions(tc.current, tc.updates...)
assert.ElementsMatch(t, tc.expected, got, tc.name)
Expand All @@ -188,7 +168,7 @@ func TestMergeConditions(t *testing.T) {

func TestMergeConditionsTruncatesMessages(t *testing.T) {
longMsg := strings.Repeat("x", conditionMessageMaxLength+5)
cond := newCondition("available", metav1.ConditionTrue, "Reason", longMsg, time.Now(), 1)
cond := newCondition("available", metav1.ConditionTrue, "Reason", longMsg, 1)
conditions := MergeConditions(nil, cond)

if assert.Len(t, conditions, 1) {
Expand Down
7 changes: 3 additions & 4 deletions internal/gatewayapi/status/envoypatchpolicy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package status

import (
"strings"
"time"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
Expand All @@ -30,22 +29,22 @@ func SetProgrammedForEnvoyPatchPolicy(s *gwapiv1.PolicyStatus) {
}

message := "Patches have been successfully applied."
cond := newCondition(string(egv1a1.PolicyConditionProgrammed), metav1.ConditionTrue, string(egv1a1.PolicyReasonProgrammed), message, time.Now(), 0)
cond := newCondition(string(egv1a1.PolicyConditionProgrammed), metav1.ConditionTrue, string(egv1a1.PolicyReasonProgrammed), message, 0)
for i := range s.Ancestors {
s.Ancestors[i].Conditions = MergeConditions(s.Ancestors[i].Conditions, cond)
}
}

func SetTranslationErrorForEnvoyPatchPolicy(s *gwapiv1.PolicyStatus, errMsg string) {
cond := newCondition(string(egv1a1.PolicyConditionProgrammed), metav1.ConditionFalse, string(egv1a1.PolicyReasonInvalid), errMsg, time.Now(), 0)
cond := newCondition(string(egv1a1.PolicyConditionProgrammed), metav1.ConditionFalse, string(egv1a1.PolicyReasonInvalid), errMsg, 0)
for i := range s.Ancestors {
s.Ancestors[i].Conditions = MergeConditions(s.Ancestors[i].Conditions, cond)
}
}

func SetResourceNotFoundErrorForEnvoyPatchPolicy(s *gwapiv1.PolicyStatus, notFoundResources []string) {
message := "Unable to find xds resources: " + strings.Join(notFoundResources, ",")
cond := newCondition(string(egv1a1.PolicyConditionProgrammed), metav1.ConditionFalse, string(egv1a1.PolicyReasonResourceNotFound), message, time.Now(), 0)
cond := newCondition(string(egv1a1.PolicyConditionProgrammed), metav1.ConditionFalse, string(egv1a1.PolicyReasonResourceNotFound), message, 0)
for i := range s.Ancestors {
s.Ancestors[i].Conditions = MergeConditions(s.Ancestors[i].Conditions, cond)
}
Expand Down
16 changes: 7 additions & 9 deletions internal/gatewayapi/status/gateway.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package status
import (
"fmt"
"slices"
"time"

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
Expand All @@ -19,14 +18,14 @@ import (
)

func UpdateGatewayStatusNotAccepted(gw *gwapiv1.Gateway, reason gwapiv1.GatewayConditionReason, msg string) *gwapiv1.Gateway {
cond := newCondition(string(gwapiv1.GatewayConditionAccepted), metav1.ConditionFalse, string(reason), msg, time.Now(), gw.Generation)
cond := newCondition(string(gwapiv1.GatewayConditionAccepted), metav1.ConditionFalse, string(reason), msg, gw.Generation)
gw.Status.Conditions = MergeConditions(gw.Status.Conditions, cond)
return gw
}

func UpdateGatewayStatusAccepted(gw *gwapiv1.Gateway) *gwapiv1.Gateway {
cond := newCondition(string(gwapiv1.GatewayConditionAccepted), metav1.ConditionTrue,
string(gwapiv1.GatewayReasonAccepted), "The Gateway has been scheduled by Envoy Gateway", time.Now(), gw.Generation)
string(gwapiv1.GatewayReasonAccepted), "The Gateway has been scheduled by Envoy Gateway", gw.Generation)
gw.Status.Conditions = MergeConditions(gw.Status.Conditions, cond)
return gw
}
Expand Down Expand Up @@ -144,7 +143,6 @@ func SetGatewayListenerStatusCondition(gateway *gwapiv1.Gateway, listenerStatusI
Reason: string(reason),
Message: message,
ObservedGeneration: gateway.Generation,
LastTransitionTime: metav1.NewTime(time.Now()),
}
gateway.Status.Listeners[listenerStatusIdx].Conditions = MergeConditions(gateway.Status.Listeners[listenerStatusIdx].Conditions, cond)
}
Expand All @@ -162,14 +160,14 @@ func updateGatewayProgrammedCondition(gw *gwapiv1.Gateway, envoyObj client.Objec
if len(gw.Status.Addresses) == 0 {
gw.Status.Conditions = MergeConditions(gw.Status.Conditions,
newCondition(string(gwapiv1.GatewayConditionProgrammed), metav1.ConditionFalse, string(gwapiv1.GatewayReasonAddressNotAssigned),
messageAddressNotAssigned, time.Now(), gw.Generation))
messageAddressNotAssigned, gw.Generation))
return
}

if len(gw.Status.Addresses) > 16 {
gw.Status.Conditions = MergeConditions(gw.Status.Conditions,
newCondition(string(gwapiv1.GatewayConditionProgrammed), metav1.ConditionFalse, string(gwapiv1.GatewayReasonInvalid),
fmt.Sprintf(messageFmtTooManyAddresses, len(gw.Status.Addresses)), time.Now(), gw.Generation))
fmt.Sprintf(messageFmtTooManyAddresses, len(gw.Status.Addresses)), gw.Generation))

// Truncate the addresses to 16
// so that the status can be updated successfully.
Expand All @@ -183,14 +181,14 @@ func updateGatewayProgrammedCondition(gw *gwapiv1.Gateway, envoyObj client.Objec
if obj != nil && obj.Status.AvailableReplicas > 0 {
gw.Status.Conditions = MergeConditions(gw.Status.Conditions,
newCondition(string(gwapiv1.GatewayConditionProgrammed), metav1.ConditionTrue, string(gwapiv1.GatewayConditionProgrammed),
fmt.Sprintf(messageFmtProgrammed, obj.Status.AvailableReplicas, obj.Status.Replicas), time.Now(), gw.Generation))
fmt.Sprintf(messageFmtProgrammed, obj.Status.AvailableReplicas, obj.Status.Replicas), gw.Generation))
return
}
case *appsv1.DaemonSet:
if obj != nil && obj.Status.NumberAvailable > 0 {
gw.Status.Conditions = MergeConditions(gw.Status.Conditions,
newCondition(string(gwapiv1.GatewayConditionProgrammed), metav1.ConditionTrue, string(gwapiv1.GatewayConditionProgrammed),
fmt.Sprintf(messageFmtProgrammed, obj.Status.NumberAvailable, obj.Status.CurrentNumberScheduled), time.Now(), gw.Generation))
fmt.Sprintf(messageFmtProgrammed, obj.Status.NumberAvailable, obj.Status.CurrentNumberScheduled), gw.Generation))
return
}
}
Expand All @@ -199,7 +197,7 @@ func updateGatewayProgrammedCondition(gw *gwapiv1.Gateway, envoyObj client.Objec
// Envoy DaemonSet, don't mark the Gateway as ready yet.
gw.Status.Conditions = MergeConditions(gw.Status.Conditions,
newCondition(string(gwapiv1.GatewayConditionProgrammed), metav1.ConditionFalse, string(gwapiv1.GatewayReasonNoResources),
messageNoResources, time.Now(), gw.Generation))
messageNoResources, gw.Generation))
}

// GetGatewayListenerStatusConditions returns the status conditions for a specific listener in the gateway status.
Expand Down
4 changes: 0 additions & 4 deletions internal/gatewayapi/status/gatewayclass.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
package status

import (
"time"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"
gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
Expand Down Expand Up @@ -57,7 +55,6 @@ func computeGatewayClassAcceptedCondition(gatewayClass *gwapiv1.GatewayClass,
Reason: reason,
Message: msg,
ObservedGeneration: gatewayClass.Generation,
LastTransitionTime: metav1.NewTime(time.Now()),
}
default:
return metav1.Condition{
Expand All @@ -66,7 +63,6 @@ func computeGatewayClassAcceptedCondition(gatewayClass *gwapiv1.GatewayClass,
Reason: reason,
Message: msg,
ObservedGeneration: gatewayClass.Generation,
LastTransitionTime: metav1.NewTime(time.Now()),
}
}
}
Expand Down
5 changes: 2 additions & 3 deletions internal/gatewayapi/status/policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package status

import (
"sort"
"time"

"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -104,14 +103,14 @@ func SetConditionForPolicyAncestor(policyStatus *gwapiv1.PolicyStatus, ancestorR
}

// Only create condition and merge if needed
cond := newCondition(string(conditionType), status, string(reason), sanitizedMessage, time.Now(), generation)
cond := newCondition(string(conditionType), status, string(reason), sanitizedMessage, generation)
policyStatus.Ancestors[i].Conditions = MergeConditions(policyStatus.Ancestors[i].Conditions, cond)
return
}
}

// Add condition for new PolicyAncestorStatus
cond := newCondition(string(conditionType), status, string(reason), sanitizedMessage, time.Now(), generation)
cond := newCondition(string(conditionType), status, string(reason), sanitizedMessage, generation)
policyStatus.Ancestors = append(policyStatus.Ancestors, gwapiv1.PolicyAncestorStatus{
AncestorRef: *ancestorRef,
ControllerName: gwapiv1a2.GatewayController(controllerName),
Expand Down
3 changes: 0 additions & 3 deletions internal/gatewayapi/status/route.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
package status

import (
"time"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
)
Expand All @@ -21,7 +19,6 @@ func SetRouteStatusCondition(route *gwapiv1.RouteStatus, routeParentStatusIdx in
Reason: string(reason),
Message: message,
ObservedGeneration: routeGeneration,
LastTransitionTime: metav1.NewTime(time.Now()),
}

route.Parents[routeParentStatusIdx].Conditions = MergeConditions(route.Parents[routeParentStatusIdx].Conditions, cond)
Expand Down
Loading
Loading