Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
40 changes: 32 additions & 8 deletions pkg/reconciler/internal/updater/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func (u *Updater) Apply(ctx context.Context, baseObj *unstructured.Unstructured)
updateErr := u.client.Status().Update(ctx, obj)
if errors.IsConflict(updateErr) && u.enableAggressiveConflictResolution {
u.logger.V(1).Info("Status update conflict detected")
resolved, resolveErr := u.tryRefresh(ctx, baseObj, isSafeForStatusUpdate)
resolved, resolveErr := u.tryRefresh(ctx, baseObj)
if resolveErr != nil {
return resolveErr
}
Expand Down Expand Up @@ -177,7 +177,7 @@ func (u *Updater) Apply(ctx context.Context, baseObj *unstructured.Unstructured)
updateErr := u.client.Update(ctx, obj)
if errors.IsConflict(updateErr) && u.enableAggressiveConflictResolution {
u.logger.V(1).Info("Update conflict detected")
resolved, resolveErr := u.tryRefresh(ctx, baseObj, isSafeForUpdate)
resolved, resolveErr := u.tryRefresh(ctx, baseObj)
if resolveErr != nil {
return resolveErr
}
Expand All @@ -195,11 +195,20 @@ func (u *Updater) Apply(ctx context.Context, baseObj *unstructured.Unstructured)
return err
}

func isSafeForStatusUpdate(_ logr.Logger, _ *unstructured.Unstructured, _ *unstructured.Unstructured) bool {
return true
}

func isSafeForUpdate(logger logr.Logger, inMemory *unstructured.Unstructured, onCluster *unstructured.Unstructured) bool {
// Compare metadata (excluding resourceVersion).
inMemoryMetadata := metadataWithoutResourceVersion(inMemory)
onClusterMetadata := metadataWithoutResourceVersion(onCluster)
if !reflect.DeepEqual(inMemoryMetadata, onClusterMetadata) {
// Diff in generation. Nothing we can do about it -> Fail.
logger.V(1).Info("Not refreshing object due to generation mismatch",
"namespace", inMemory.GetNamespace(),
"name", inMemory.GetName(),
"gkv", inMemory.GroupVersionKind(),
)
return false
}
// Extra verification to make sure that the spec has not changed.
if !reflect.DeepEqual(inMemory.Object["spec"], onCluster.Object["spec"]) {
// Diff in object spec. Nothing we can do about it -> Fail.
logger.V(1).Info("Not refreshing object due to spec mismatch",
Expand All @@ -212,7 +221,22 @@ func isSafeForUpdate(logger logr.Logger, inMemory *unstructured.Unstructured, on
return true
}

func (u *Updater) tryRefresh(ctx context.Context, obj *unstructured.Unstructured, isSafe func(logger logr.Logger, inMemory *unstructured.Unstructured, onCluster *unstructured.Unstructured) bool) (bool, error) {
func metadataWithoutResourceVersion(u *unstructured.Unstructured) map[string]interface{} {
metadata, ok := u.Object["metadata"].(map[string]interface{})
if !ok {
return nil
}
modifiedMetadata := make(map[string]interface{}, len(metadata))
for k, v := range metadata {
if k == "resourceVersion" {
continue
}
modifiedMetadata[k] = v
}
return modifiedMetadata
}

func (u *Updater) tryRefresh(ctx context.Context, obj *unstructured.Unstructured) (bool, error) {
// Re-fetch object with client.
current := &unstructured.Unstructured{}
current.SetGroupVersionKind(obj.GroupVersionKind())
Expand All @@ -222,7 +246,7 @@ func (u *Updater) tryRefresh(ctx context.Context, obj *unstructured.Unstructured
return false, err
}

if !isSafe(u.logger, obj, current) {
if !isSafeForUpdate(u.logger, obj, current) {
return false, nil
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/reconciler/internal/updater/updater_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,9 @@ var _ = Describe("Updater", func() {
})
Context("when in-cluster object has been updated", func() {
JustBeforeEach(func() {
// Refresh obj first with what is on the cluster, otherwise the following Apply() calls will fail due to
// subtle changes in obj's spec and its refreshed counterpart, both of which are empty, but not in the same way...
Expect(cl.Get(context.TODO(), types.NamespacedName{Namespace: "testNamespace", Name: "testDeployment"}, obj)).To(Succeed())
// Add external status condition on cluster.
clusterObj := obj.DeepCopy()
unknownCondition := map[string]interface{}{
Expand Down
Loading