Merge pull request #95240 from apelisse/no-field-update-on-nop

Do not update managedFields timestamp when they don't change

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/BUILD

@@ -56,6 +56,7 @@ go_test(
         "fieldmanager_test.go",
         "lastappliedmanager_test.go",
         "lastappliedupdater_test.go",
+        "managedfieldsupdater_test.go",
         "skipnonapplied_test.go",
     ],
     data = [

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

@@ -44,6 +44,7 @@ func NewManagedFieldsUpdater(fieldManager Manager) Manager {
 // Update implements Manager.
 func (f *managedFieldsUpdater) Update(liveObj, newObj runtime.Object, managed Managed, manager string) (runtime.Object, Managed, error) {
 	self := "current-operation"
+	formerSet := managed.Fields()[manager]
 	object, managed, err := f.fieldManager.Update(liveObj, newObj, managed, self)
 	if err != nil {
 		return object, managed, err
@@ -54,12 +55,15 @@ func (f *managedFieldsUpdater) Update(liveObj, newObj runtime.Object, managed Ma
 	if vs, ok := managed.Fields()[self]; ok {
 		delete(managed.Fields(), self)
 
-		managed.Times()[manager] = &metav1.Time{Time: time.Now().UTC()}
 		if previous, ok := managed.Fields()[manager]; ok {
 			managed.Fields()[manager] = fieldpath.NewVersionedSet(vs.Set().Union(previous.Set()), vs.APIVersion(), vs.Applied())
 		} else {
 			managed.Fields()[manager] = vs
 		}
+		// Update the time only if the manager's fieldSet has changed.
+		if formerSet == nil || !managed.Fields()[manager].Set().Equals(formerSet.Set()) {
+			managed.Times()[manager] = &metav1.Time{Time: time.Now().UTC()}
+		}
 	}
 
 	return object, managed, nil

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

@@ -0,0 +1,66 @@
+/*
+Copyright 2020 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_test
+
+import (
+	"reflect"
+	"testing"
+	"time"
+
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"sigs.k8s.io/yaml"
+)
+
+func TestNoManagedFieldsUpdateDoesntUpdateTime(t *testing.T) {
+	f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"), false, nil)
+
+	obj := &unstructured.Unstructured{Object: map[string]interface{}{}}
+	if err := yaml.Unmarshal([]byte(`{
+		"apiVersion": "v1",
+		"kind": "Pod",
+		"metadata": {
+			"name": "pod",
+			"labels": {"app": "nginx"}
+		},
+	}`), &obj.Object); err != nil {
+		t.Fatalf("error decoding YAML: %v", err)
+	}
+
+	if err := f.Update(obj, "fieldmanager_test"); err != nil {
+		t.Fatalf("failed to update object: %v", err)
+	}
+	managed := f.ManagedFields()
+	obj2 := &unstructured.Unstructured{Object: map[string]interface{}{}}
+	if err := yaml.Unmarshal([]byte(`{
+		"apiVersion": "v1",
+		"kind": "Pod",
+		"metadata": {
+			"name": "pod",
+			"labels": {"app": "nginx2"}
+		},
+	}`), &obj2.Object); err != nil {
+		t.Fatalf("error decoding YAML: %v", err)
+	}
+	time.Sleep(time.Second)
+	if err := f.Update(obj2, "fieldmanager_test"); err != nil {
+		t.Fatalf("failed to update object: %v", err)
+	}
+	if !reflect.DeepEqual(managed, f.ManagedFields()) {
+		t.Errorf("ManagedFields changed:\nBefore:\n%v\nAfter:\n%v", managed, f.ManagedFields())
+	}
+}