resource: optionally compare timestamps in `EqualStatus` (#17275)

This commit is contained in:
Dan Upton 2023-05-10 10:37:54 +01:00 committed by GitHub
parent 5ecab506a6
commit 6c24a66f73
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 34 additions and 11 deletions

View File

@ -111,7 +111,7 @@ func (r *artistReconciler) Reconcile(ctx context.Context, rt controller.Runtime,
Conditions: conditions,
}
if resource.EqualStatus(res.Status[statusKeyArtistController], newStatus) {
if resource.EqualStatus(res.Status[statusKeyArtistController], newStatus, false) {
return nil
}

View File

@ -52,7 +52,10 @@ func EqualID(a, b *pbresource.ID) bool {
}
// EqualStatus compares two statuses for equality without reflection.
func EqualStatus(a, b *pbresource.Status) bool {
//
// Pass true for compareUpdatedAt to compare the UpdatedAt timestamps, which you
// generally *don't* want when dirty checking the status in a controller.
func EqualStatus(a, b *pbresource.Status, compareUpdatedAt bool) bool {
if a == b {
return true
}
@ -65,6 +68,10 @@ func EqualStatus(a, b *pbresource.Status) bool {
return false
}
if compareUpdatedAt && !a.UpdatedAt.AsTime().Equal(b.UpdatedAt.AsTime()) {
return false
}
if len(a.Conditions) != len(b.Conditions) {
return false
}
@ -125,7 +132,7 @@ func EqualStatusMap(a, b map[string]*pbresource.Status) bool {
if !ok {
return false
}
if !EqualStatus(av, bv) {
if !EqualStatus(av, bv, true) {
return false
}
compared[k] = struct{}{}
@ -141,7 +148,7 @@ func EqualStatusMap(a, b map[string]*pbresource.Status) bool {
return false
}
if !EqualStatus(av, bv) {
if !EqualStatus(av, bv, true) {
return false
}
}

View File

@ -6,10 +6,12 @@ package resource_test
import (
"fmt"
"testing"
"time"
"github.com/oklog/ulid/v2"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/timestamppb"
"github.com/hashicorp/consul/internal/resource"
"github.com/hashicorp/consul/proto-public/pbresource"
@ -310,17 +312,17 @@ func TestEqualStatus(t *testing.T) {
// Equal cases.
t.Run("same pointer", func(t *testing.T) {
require.True(t, resource.EqualStatus(orig, orig))
require.True(t, resource.EqualStatus(orig, orig, true))
})
t.Run("equal", func(t *testing.T) {
require.True(t, resource.EqualStatus(orig, clone(orig)))
require.True(t, resource.EqualStatus(orig, clone(orig), true))
})
// Not equal cases.
t.Run("nil", func(t *testing.T) {
require.False(t, resource.EqualStatus(orig, nil))
require.False(t, resource.EqualStatus(nil, orig))
require.False(t, resource.EqualStatus(orig, nil, true))
require.False(t, resource.EqualStatus(nil, orig, true))
})
testCases := map[string]func(*pbresource.Status){
@ -366,10 +368,24 @@ func TestEqualStatus(t *testing.T) {
a, b := clone(orig), clone(orig)
modFn(b)
require.False(t, resource.EqualStatus(a, b))
require.False(t, resource.EqualStatus(b, a))
require.False(t, resource.EqualStatus(a, b, true))
require.False(t, resource.EqualStatus(b, a, true))
})
}
t.Run("compareUpdatedAt = true", func(t *testing.T) {
a, b := clone(orig), clone(orig)
b.UpdatedAt = timestamppb.New(b.UpdatedAt.AsTime().Add(1 * time.Minute))
require.False(t, resource.EqualStatus(a, b, true))
require.False(t, resource.EqualStatus(b, a, true))
})
t.Run("compareUpdatedAt = false", func(t *testing.T) {
a, b := clone(orig), clone(orig)
b.UpdatedAt = timestamppb.New(b.UpdatedAt.AsTime().Add(1 * time.Minute))
require.True(t, resource.EqualStatus(a, b, false))
require.True(t, resource.EqualStatus(b, a, false))
})
}
func TestEqualStatusMap(t *testing.T) {
@ -615,7 +631,7 @@ func BenchmarkEqualStatus(b *testing.B) {
b.Run("ours", func(b *testing.B) {
for i := 0; i < b.N; i++ {
_ = resource.EqualStatus(statusA, statusB)
_ = resource.EqualStatus(statusA, statusB, true)
}
})