revert timestamp updates to object if non-managed fields do not change

add short-circuiting logic for long comaprison

replace timestamps rather than doing a full managed fields deepcopy

add guard

staging/src/k8s.io/apimachinery/pkg/conversion/deep_equal.go

@@ -34,3 +34,14 @@ func EqualitiesOrDie(funcs ...interface{}) Equalities {
 	}
 	return e
 }
+
+// Performs a shallow copy of the equalities map
+func (e Equalities) Copy() Equalities {
+	result := Equalities{reflect.Equalities{}}
+
+	for key, value := range e.Equalities {
+		result.Equalities[key] = value
+	}
+
+	return result
+}

staging/src/k8s.io/apimachinery/third_party/forked/golang/reflect/deep_equal.go

@@ -100,7 +100,8 @@ func makeUsefulPanic(v reflect.Value) {
 // Tests for deep equality using reflected types. The map argument tracks
 // comparisons that have already been seen, which allows short circuiting on
 // recursive types.
-func (e Equalities) deepValueEqual(v1, v2 reflect.Value, visited map[visit]bool, depth int) bool {
+// equateNilAndEmpty controls whether empty maps/slices are equivalent to nil
+func (e Equalities) deepValueEqual(v1, v2 reflect.Value, visited map[visit]bool, equateNilAndEmpty bool, depth int) bool {
 	defer makeUsefulPanic(v1)
 
 	if !v1.IsValid() || !v2.IsValid() {
@@ -150,17 +151,24 @@ func (e Equalities) deepValueEqual(v1, v2 reflect.Value, visited map[visit]bool,
 		// We don't need to check length here because length is part of
 		// an array's type, which has already been filtered for.
 		for i := 0; i < v1.Len(); i++ {
-			if !e.deepValueEqual(v1.Index(i), v2.Index(i), visited, depth+1) {
+			if !e.deepValueEqual(v1.Index(i), v2.Index(i), visited, equateNilAndEmpty, depth+1) {
 				return false
 			}
 		}
 		return true
 	case reflect.Slice:
-		if (v1.IsNil() || v1.Len() == 0) != (v2.IsNil() || v2.Len() == 0) {
+		if equateNilAndEmpty {
-			return false
+			if (v1.IsNil() || v1.Len() == 0) != (v2.IsNil() || v2.Len() == 0) {
-		}
+				return false
-		if v1.IsNil() || v1.Len() == 0 {
+			}
-			return true
+
+			if v1.IsNil() || v1.Len() == 0 {
+				return true
+			}
+		} else {
+			if v1.IsNil() != v2.IsNil() {
+				return false
+			}
 		}
 		if v1.Len() != v2.Len() {
 			return false
@@ -169,7 +177,7 @@ func (e Equalities) deepValueEqual(v1, v2 reflect.Value, visited map[visit]bool,
 			return true
 		}
 		for i := 0; i < v1.Len(); i++ {
-			if !e.deepValueEqual(v1.Index(i), v2.Index(i), visited, depth+1) {
+			if !e.deepValueEqual(v1.Index(i), v2.Index(i), visited, equateNilAndEmpty, depth+1) {
 				return false
 			}
 		}
@@ -178,22 +186,28 @@ func (e Equalities) deepValueEqual(v1, v2 reflect.Value, visited map[visit]bool,
 		if v1.IsNil() || v2.IsNil() {
 			return v1.IsNil() == v2.IsNil()
 		}
-		return e.deepValueEqual(v1.Elem(), v2.Elem(), visited, depth+1)
+		return e.deepValueEqual(v1.Elem(), v2.Elem(), visited, equateNilAndEmpty, depth+1)
-	case reflect.Pointer:
+	case reflect.Ptr:
-		return e.deepValueEqual(v1.Elem(), v2.Elem(), visited, depth+1)
+		return e.deepValueEqual(v1.Elem(), v2.Elem(), visited, equateNilAndEmpty, depth+1)
 	case reflect.Struct:
 		for i, n := 0, v1.NumField(); i < n; i++ {
-			if !e.deepValueEqual(v1.Field(i), v2.Field(i), visited, depth+1) {
+			if !e.deepValueEqual(v1.Field(i), v2.Field(i), visited, equateNilAndEmpty, depth+1) {
 				return false
 			}
 		}
 		return true
 	case reflect.Map:
-		if (v1.IsNil() || v1.Len() == 0) != (v2.IsNil() || v2.Len() == 0) {
+		if equateNilAndEmpty {
-			return false
+			if (v1.IsNil() || v1.Len() == 0) != (v2.IsNil() || v2.Len() == 0) {
-		}
+				return false
-		if v1.IsNil() || v1.Len() == 0 {
+			}
-			return true
+			if v1.IsNil() || v1.Len() == 0 {
+				return true
+			}
+		} else {
+			if v1.IsNil() != v2.IsNil() {
+				return false
+			}
 		}
 		if v1.Len() != v2.Len() {
 			return false
@@ -202,7 +216,7 @@ func (e Equalities) deepValueEqual(v1, v2 reflect.Value, visited map[visit]bool,
 			return true
 		}
 		for _, k := range v1.MapKeys() {
-			if !e.deepValueEqual(v1.MapIndex(k), v2.MapIndex(k), visited, depth+1) {
+			if !e.deepValueEqual(v1.MapIndex(k), v2.MapIndex(k), visited, equateNilAndEmpty, depth+1) {
 				return false
 			}
 		}
@@ -232,6 +246,14 @@ func (e Equalities) deepValueEqual(v1, v2 reflect.Value, visited map[visit]bool,
 // Unexported field members cannot be compared and will cause an informative panic; you must add an Equality
 // function for these types.
 func (e Equalities) DeepEqual(a1, a2 interface{}) bool {
+	return e.deepEqual(a1, a2, true)
+}
+
+func (e Equalities) DeepEqualWithNilDifferentFromEmpty(a1, a2 interface{}) bool {
+	return e.deepEqual(a1, a2, false)
+}
+
+func (e Equalities) deepEqual(a1, a2 interface{}, equateNilAndEmpty bool) bool {
 	if a1 == nil || a2 == nil {
 		return a1 == a2
 	}
@@ -240,7 +262,7 @@ func (e Equalities) DeepEqual(a1, a2 interface{}) bool {
 	if v1.Type() != v2.Type() {
 		return false
 	}
-	return e.deepValueEqual(v1, v2, make(map[visit]bool), 0)
+	return e.deepValueEqual(v1, v2, make(map[visit]bool), equateNilAndEmpty, 0)
 }
 
 func (e Equalities) deepValueDerive(v1, v2 reflect.Value, visited map[visit]bool, depth int) bool {

staging/src/k8s.io/apimachinery/third_party/forked/golang/reflect/deep_equal_test.go

@@ -16,6 +16,10 @@ func TestEqualities(t *testing.T) {
 	type Baz struct {
 		Y Bar
 	}
+	type Zap struct {
+		A []int
+		B map[string][]int
+	}
 	err := e.AddFuncs(
 		func(a, b int) bool {
 			return a+1 == b
@@ -32,10 +36,12 @@ func TestEqualities(t *testing.T) {
 		X int
 	}
 
-	table := []struct {
+	type Case struct {
 		a, b  interface{}
 		equal bool
-	}{
+	}
+
+	table := []Case{
 		{1, 2, true},
 		{2, 1, false},
 		{"foo", "fo", false},
@@ -70,6 +76,26 @@ func TestEqualities(t *testing.T) {
 			t.Errorf("Expected (%+v == %+v) == %v, but got %v", item.a, item.b, e, a)
 		}
 	}
+
+	// Cases which hinge upon implicit nil/empty map/slice equality
+	implicitTable := []Case{
+		{map[string][]int{}, map[string][]int(nil), true},
+		{[]int{}, []int(nil), true},
+		{map[string][]int{"foo": nil}, map[string][]int{"foo": {}}, true},
+		{Zap{A: nil, B: map[string][]int{"foo": nil}}, Zap{A: []int{}, B: map[string][]int{"foo": {}}}, true},
+	}
+
+	for _, item := range implicitTable {
+		if e, a := item.equal, e.DeepEqual(item.a, item.b); e != a {
+			t.Errorf("Expected (%+v == %+v) == %v, but got %v", item.a, item.b, e, a)
+		}
+	}
+
+	for _, item := range implicitTable {
+		if e, a := !item.equal, e.DeepEqualWithNilDifferentFromEmpty(item.a, item.b); e != a {
+			t.Errorf("Expected (%+v == %+v) == %v, but got %v", item.a, item.b, e, a)
+		}
+	}
 }
 
 func TestDerivatives(t *testing.T) {

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/equality.go

@@ -0,0 +1,109 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package fieldmanager
+
+import (
+	"context"
+	"fmt"
+	"reflect"
+
+	"k8s.io/apimachinery/pkg/api/equality"
+	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/conversion"
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+var ignoreTimestampEqualities = func() conversion.Equalities {
+	var eqs = equality.Semantic.Copy()
+
+	err := eqs.AddFunc(
+		func(a, b metav1.ManagedFieldsEntry) bool {
+			// Two objects' managed fields are equivalent if, ignoring timestamp,
+			//	the objects are deeply equal.
+			a.Time = nil
+			b.Time = nil
+			return reflect.DeepEqual(a, b)
+		},
+	)
+
+	if err != nil {
+		panic(err)
+	}
+
+	return eqs
+}()
+
+// IgnoreManagedFieldsTimestampsTransformer reverts timestamp updates
+// if the non-managed parts of the object are equivalent
+func IgnoreManagedFieldsTimestampsTransformer(
+	_ context.Context,
+	newObj runtime.Object,
+	oldObj runtime.Object,
+) (runtime.Object, error) {
+	// If managedFields modulo timestamps are unchanged
+	//		and
+	//	rest of object is unchanged
+	//		then
+	//	revert any changes to timestamps in managed fields
+	//		(to prevent spurious ResourceVersion bump)
+	//
+	// Procecure:
+	// Do a quicker check to see if just managed fields modulo timestamps are
+	//	unchanged. If so, then do the full, slower check.
+	//
+	// In most cases which actually update the object, the managed fields modulo
+	//	timestamp check will fail, and we will be able to return early.
+	//
+	// In other cases, the managed fields may be exactly the same,
+	// 	except for timestamp, but the objects are the different. This is the
+	//	slow path which checks the full object.
+	oldAccessor, err := meta.Accessor(oldObj)
+	if err != nil {
+		return nil, fmt.Errorf("failed to acquire accessor for oldObj: %v", err)
+	}
+
+	accessor, err := meta.Accessor(newObj)
+	if err != nil {
+		return nil, fmt.Errorf("failed to acquire accessor for newObj: %v", err)
+	}
+
+	oldManagedFields := oldAccessor.GetManagedFields()
+	newManagedFields := accessor.GetManagedFields()
+
+	// This condition ensures the managed fields are always compared first. If
+	//	this check fails, the if statement will short circuit. If the check
+	// 	succeeds the slow path is taken which compares entire objects.
+	if ignoreTimestampEqualities.DeepEqualWithNilDifferentFromEmpty(oldManagedFields, newManagedFields) &&
+		ignoreTimestampEqualities.DeepEqualWithNilDifferentFromEmpty(newObj, oldObj) {
+
+		// Remove any changed timestamps, so that timestamp is not the only
+		// change seen by etcd.
+		//
+		// newManagedFields is known to be exactly pairwise equal to
+		// oldManagedFields except for timestamps.
+		//
+		// Simply replace possibly changed new timestamps with their old values.
+		for idx := 0; idx < len(oldManagedFields); idx++ {
+			newManagedFields[idx].Time = oldManagedFields[idx].Time
+		}
+
+		accessor.SetManagedFields(newManagedFields)
+		return newObj, nil
+	}
+	return newObj, nil
+}

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go

@@ -660,7 +660,7 @@ func (p *patcher) patchResource(ctx context.Context, scope *RequestScope) (runti
 	}
 
 	wasCreated := false
-	p.updatedObjectInfo = rest.DefaultUpdatedObjectInfo(nil, p.applyPatch, p.applyAdmission, dedupOwnerReferencesTransformer)
+	p.updatedObjectInfo = rest.DefaultUpdatedObjectInfo(nil, p.applyPatch, p.applyAdmission, dedupOwnerReferencesTransformer, fieldmanager.IgnoreManagedFieldsTimestampsTransformer)
 	requestFunc := func() (runtime.Object, error) {
 		// Pass in UpdateOptions to override UpdateStrategy.AllowUpdateOnCreate
 		options := patchToUpdateOptions(p.options)

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go

@@ -191,6 +191,15 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa
 			})
 		}
 
+		// Ignore changes that only affect managed fields
+		// timestamps. FieldManager can't know about changes
+		// like normalized fields, defaulted fields and other
+		// mutations.
+		// Only makes sense when SSA field manager is being used
+		if scope.FieldManager != nil {
+			transformers = append(transformers, fieldmanager.IgnoreManagedFieldsTimestampsTransformer)
+		}
+
 		createAuthorizerAttributes := authorizer.AttributesRecord{
 			User:            userInfo,
 			ResourceRequest: true,

test/integration/apiserver/apply/apply_test.go

@@ -249,6 +249,185 @@ func TestNoOpUpdateSameResourceVersion(t *testing.T) {
 	}
 }
 
+func getRV(obj runtime.Object) (string, error) {
+	acc, err := meta.Accessor(obj)
+	if err != nil {
+		return "", err
+	}
+	return acc.GetResourceVersion(), nil
+}
+
+// TestNoSemanticUpdateAppleSameResourceVersion makes sure that APPLY requests which makes no semantic changes
+// will not change the resource version (no write to etcd is done)
+//
+// Some of the non-semantic changes are:
+// - Applying an atomic struct that removes a default
+// - Changing Quantity or other fields that are normalized
+func TestNoSemanticUpdateApplySameResourceVersion(t *testing.T) {
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ServerSideApply, true)()
+
+	client, closeFn := setup(t)
+	defer closeFn()
+
+	ssBytes := []byte(`{
+		"apiVersion": "apps/v1",
+		"kind": "StatefulSet",
+		"metadata": {
+			"name": "nginx",
+			"labels": {"app": "nginx"}
+		},
+		"spec": {
+			"serviceName": "nginx",
+			"selector": { "matchLabels": {"app": "nginx"}},
+			"template": {
+				"metadata": {
+					"labels": {"app": "nginx"}
+				},
+				"spec": {
+					"containers": [{
+						"name":  "nginx",
+						"image": "nginx",
+						"resources": {
+							"limits": {"memory": "2048Mi"}
+						}
+					}]
+				}
+			},
+			"volumeClaimTemplates": [{
+				"metadata": {"name": "nginx"},
+				"spec": {
+					"accessModes": ["ReadWriteOnce"],
+					"resources": {"requests": {"storage": "1Gi"}}
+				}
+			}]
+		}
+	}`)
+
+	obj, err := client.AppsV1().RESTClient().Patch(types.ApplyPatchType).
+		Namespace("default").
+		Param("fieldManager", "apply_test").
+		Resource("statefulsets").
+		Name("nginx").
+		Body(ssBytes).
+		Do(context.TODO()).
+		Get()
+	if err != nil {
+		t.Fatalf("Failed to create object: %v", err)
+	}
+
+	rvCreated, err := getRV(obj)
+	if err != nil {
+		t.Fatalf("Failed to get RV: %v", err)
+	}
+
+	// Sleep for one second to make sure that the times of each update operation is different.
+	time.Sleep(1200 * time.Millisecond)
+
+	obj, err = client.AppsV1().RESTClient().Patch(types.ApplyPatchType).
+		Namespace("default").
+		Param("fieldManager", "apply_test").
+		Resource("statefulsets").
+		Name("nginx").
+		Body(ssBytes).
+		Do(context.TODO()).
+		Get()
+	if err != nil {
+		t.Fatalf("Failed to create object: %v", err)
+	}
+	rvApplied, err := getRV(obj)
+	if err != nil {
+		t.Fatalf("Failed to get RV: %v", err)
+	}
+	if rvApplied != rvCreated {
+		t.Fatal("ResourceVersion changed after apply")
+	}
+}
+
+// TestNoSemanticUpdateAppleSameResourceVersion makes sure that PUT requests which makes no semantic changes
+// will not change the resource version (no write to etcd is done)
+//
+// Some of the non-semantic changes are:
+// - Applying an atomic struct that removes a default
+// - Changing Quantity or other fields that are normalized
+func TestNoSemanticUpdatePutSameResourceVersion(t *testing.T) {
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ServerSideApply, true)()
+
+	client, closeFn := setup(t)
+	defer closeFn()
+
+	ssBytes := []byte(`{
+		"apiVersion": "apps/v1",
+		"kind": "StatefulSet",
+		"metadata": {
+			"name": "nginx",
+			"labels": {"app": "nginx"}
+		},
+		"spec": {
+			"serviceName": "nginx",
+			"selector": { "matchLabels": {"app": "nginx"}},
+			"template": {
+				"metadata": {
+					"labels": {"app": "nginx"}
+				},
+				"spec": {
+					"containers": [{
+						"name":  "nginx",
+						"image": "nginx",
+						"resources": {
+							"limits": {"memory": "2048Mi"}
+						}
+					}]
+				}
+			},
+			"volumeClaimTemplates": [{
+				"metadata": {"name": "nginx"},
+				"spec": {
+					"accessModes": ["ReadWriteOnce"],
+					"resources": { "requests": { "storage": "1Gi"}}
+				}
+			}]
+		}
+	}`)
+
+	obj, err := client.AppsV1().RESTClient().Post().
+		Namespace("default").
+		Param("fieldManager", "apply_test").
+		Resource("statefulsets").
+		Body(ssBytes).
+		Do(context.TODO()).
+		Get()
+	if err != nil {
+		t.Fatalf("Failed to create object: %v", err)
+	}
+
+	rvCreated, err := getRV(obj)
+	if err != nil {
+		t.Fatalf("Failed to get RV: %v", err)
+	}
+
+	// Sleep for one second to make sure that the times of each update operation is different.
+	time.Sleep(1200 * time.Millisecond)
+
+	obj, err = client.AppsV1().RESTClient().Put().
+		Namespace("default").
+		Param("fieldManager", "apply_test").
+		Resource("statefulsets").
+		Name("nginx").
+		Body(ssBytes).
+		Do(context.TODO()).
+		Get()
+	if err != nil {
+		t.Fatalf("Failed to create object: %v", err)
+	}
+	rvApplied, err := getRV(obj)
+	if err != nil {
+		t.Fatalf("Failed to get RV: %v", err)
+	}
+	if rvApplied != rvCreated {
+		t.Fatal("ResourceVersion changed after similar PUT")
+	}
+}
+
 // TestCreateOnApplyFailsWithUID makes sure that PATCH requests with the apply content type
 // will not create the object if it doesn't already exist and it specifies a UID
 func TestCreateOnApplyFailsWithUID(t *testing.T) {