diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/managedfieldsupdater.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/managedfieldsupdater.go index 83f92b8ccec..412443a6c4e 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/managedfieldsupdater.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/managedfieldsupdater.go @@ -45,7 +45,6 @@ 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 @@ -61,10 +60,8 @@ func (f *managedFieldsUpdater) Update(liveObj, newObj runtime.Object, managed Ma } 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()} - } + + managed.Times()[manager] = &metav1.Time{Time: time.Now().UTC()} } return object, managed, nil @@ -72,15 +69,13 @@ func (f *managedFieldsUpdater) Update(liveObj, newObj runtime.Object, managed Ma // Apply implements Manager. func (f *managedFieldsUpdater) Apply(liveObj, appliedObj runtime.Object, managed Managed, fieldManager string, force bool) (runtime.Object, Managed, error) { - formerManaged := managed.Fields().Copy() object, managed, err := f.fieldManager.Apply(liveObj, appliedObj, managed, fieldManager, force) if err != nil { return object, managed, err } - if object != nil || !managed.Fields().Equals(formerManaged) { + if object != nil { managed.Times()[fieldManager] = &metav1.Time{Time: time.Now().UTC()} - } - if object == nil { + } else { object = liveObj.DeepCopyObject() internal.RemoveObjectManagedFields(object) } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/managedfieldsupdater_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/managedfieldsupdater_test.go index b54bbaed782..b8ff34d40cf 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/managedfieldsupdater_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/managedfieldsupdater_test.go @@ -17,6 +17,8 @@ limitations under the License. package fieldmanager import ( + "fmt" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "reflect" "testing" "time" @@ -29,42 +31,395 @@ import ( "sigs.k8s.io/yaml" ) -func TestNoManagedFieldsUpdateDoesntUpdateTime(t *testing.T) { - f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"), "", nil) +func TestManagedFieldsUpdateDoesModifyTime(t *testing.T) { + var err error + f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "ConfigMap"), "", nil) - obj := &unstructured.Unstructured{Object: map[string]interface{}{}} - if err := yaml.Unmarshal([]byte(`{ + err = updateObject(&f, "fieldmanager_test", []byte(`{ "apiVersion": "v1", - "kind": "Pod", + "kind": "ConfigMap", "metadata": { - "name": "pod", - "labels": {"app": "nginx"} + "name": "configmap" }, - }`), &obj.Object); err != nil { - t.Fatalf("error decoding YAML: %v", err) + "data": { + "key": "value" + } + }`)) + if err != nil { + t.Fatal(err) } + previousManagedFields := f.ManagedFields() - 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) + + err = updateObject(&f, "fieldmanager_test", []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "configmap" + }, + "data": { + "key": "new-value" + } + }`)) + if err != nil { + t.Fatal(err) } - if !reflect.DeepEqual(managed, f.ManagedFields()) { - t.Errorf("ManagedFields changed:\nBefore:\n%v\nAfter:\n%v", managed, f.ManagedFields()) + newManagedFields := f.ManagedFields() + + if previousManagedFields[0].Time.Equal(newManagedFields[0].Time) { + t.Errorf("ManagedFields time has not been updated:\n%v", newManagedFields) + } +} + +func TestManagedFieldsApplyDoesModifyTime(t *testing.T) { + var err error + f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "ConfigMap"), "", nil) + + err = applyObject(&f, "fieldmanager_test", []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "configmap" + }, + "data": { + "key": "value" + } + }`)) + if err != nil { + t.Fatal(err) + } + previousManagedFields := f.ManagedFields() + + time.Sleep(time.Second) + + err = applyObject(&f, "fieldmanager_test", []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "configmap" + }, + "data": { + "key": "new-value" + } + }`)) + if err != nil { + t.Fatal(err) + } + newManagedFields := f.ManagedFields() + + if previousManagedFields[0].Time.Equal(newManagedFields[0].Time) { + t.Errorf("ManagedFields time has not been updated:\n%v", newManagedFields) + } +} + +func TestManagedFieldsUpdateWithoutChangesDoesNotModifyTime(t *testing.T) { + var err error + f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "ConfigMap"), "", nil) + + err = updateObject(&f, "fieldmanager_test", []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "configmap" + }, + "data": { + "key": "value" + } + }`)) + if err != nil { + t.Fatal(err) + } + previousManagedFields := f.ManagedFields() + + time.Sleep(time.Second) + + err = updateObject(&f, "fieldmanager_test", []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "configmap" + }, + "data": { + "key": "value" + } + }`)) + if err != nil { + t.Fatal(err) + } + newManagedFields := f.ManagedFields() + + if !previousManagedFields[0].Time.Equal(newManagedFields[0].Time) { + t.Errorf("ManagedFields time has changed:\nBefore:\n%v\nAfter:\n%v", previousManagedFields, newManagedFields) + } +} + +func TestManagedFieldsApplyWithoutChangesDoesNotModifyTime(t *testing.T) { + var err error + f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "ConfigMap"), "", nil) + + err = applyObject(&f, "fieldmanager_test", []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "configmap" + }, + "data": { + "key": "value" + } + }`)) + if err != nil { + t.Fatal(err) + } + previousManagedFields := f.ManagedFields() + + time.Sleep(time.Second) + + err = applyObject(&f, "fieldmanager_test", []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "configmap" + }, + "data": { + "key": "value" + } + }`)) + if err != nil { + t.Fatal(err) + } + newManagedFields := f.ManagedFields() + + if !previousManagedFields[0].Time.Equal(newManagedFields[0].Time) { + t.Errorf("ManagedFields time has changed:\nBefore:\n%v\nAfter:\n%v", previousManagedFields, newManagedFields) + } +} + +func TestNonManagedFieldsUpdateDoesNotModifyTime(t *testing.T) { + var err error + f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "ConfigMap"), "", nil) + + err = updateObject(&f, "fieldmanager_a_test", []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "configmap" + }, + "data": { + "key_a": "value" + } + }`)) + if err != nil { + t.Fatal(err) + } + err = updateObject(&f, "fieldmanager_b_test", []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "configmap" + }, + "data": { + "key_b": "value" + } + }`)) + if err != nil { + t.Fatal(err) + } + previousManagedFields := f.ManagedFields() + previousEntries := map[string]v1.ManagedFieldsEntry{} + for _, entry := range previousManagedFields { + previousEntries[entry.Manager] = entry + } + + time.Sleep(time.Second) + + err = updateObject(&f, "fieldmanager_a_test", []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "configmap" + }, + "data": { + "key_a": "value", + "key_b": "new-value" + } + }`)) + if err != nil { + t.Fatal(err) + } + newManagedFields := f.ManagedFields() + newEntries := map[string]v1.ManagedFieldsEntry{} + for _, entry := range newManagedFields { + newEntries[entry.Manager] = entry + } + + if _, ok := newEntries["fieldmanager_b_test"]; ok { + t.Errorf("FieldManager B ManagedFields has changed:\n%v", newEntries["fieldmanager_b_test"]) + } +} + +func TestNonManagedFieldsApplyDoesNotModifyTime(t *testing.T) { + var err error + f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "ConfigMap"), "", nil) + + err = applyObject(&f, "fieldmanager_a_test", []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "configmap" + }, + "data": { + "key_a": "value" + } + }`)) + if err != nil { + t.Fatal(err) + } + err = applyObject(&f, "fieldmanager_b_test", []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "configmap" + }, + "data": { + "key_b": "value" + } + }`)) + if err != nil { + t.Fatal(err) + } + previousManagedFields := f.ManagedFields() + previousEntries := map[string]v1.ManagedFieldsEntry{} + for _, entry := range previousManagedFields { + previousEntries[entry.Manager] = entry + } + + time.Sleep(time.Second) + + err = applyObject(&f, "fieldmanager_a_test", []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "configmap" + }, + "data": { + "key_a": "new-value" + } + }`)) + if err != nil { + t.Fatal(err) + } + newManagedFields := f.ManagedFields() + newEntries := map[string]v1.ManagedFieldsEntry{} + for _, entry := range newManagedFields { + newEntries[entry.Manager] = entry + } + + if !previousEntries["fieldmanager_b_test"].Time.Equal(newEntries["fieldmanager_b_test"].Time) { + t.Errorf("FieldManager B ManagedFields time changed:\nBefore:\n%v\nAfter:\n%v", + previousEntries["fieldmanager_b_test"], newEntries["fieldmanager_b_test"]) + } +} + +func TestTakingOverManagedFieldsDuringUpdateDoesNotModifyPreviousManagerTime(t *testing.T) { + var err error + f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "ConfigMap"), "", nil) + + err = updateObject(&f, "fieldmanager_a_test", []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "configmap" + }, + "data": { + "key_a": "value", + "key_b": value" + } + }`)) + if err != nil { + t.Fatal(err) + } + previousManagedFields := f.ManagedFields() + previousEntries := map[string]v1.ManagedFieldsEntry{} + for _, entry := range previousManagedFields { + previousEntries[entry.Manager] = entry + } + + time.Sleep(time.Second) + + err = updateObject(&f, "fieldmanager_b_test", []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "configmap" + }, + "data": { + "key_b": "new-value" + } + }`)) + if err != nil { + t.Fatal(err) + } + newManagedFields := f.ManagedFields() + newEntries := map[string]v1.ManagedFieldsEntry{} + for _, entry := range newManagedFields { + newEntries[entry.Manager] = entry + } + + if !previousEntries["fieldmanager_a_test"].Time.Equal(newEntries["fieldmanager_a_test"].Time) { + t.Errorf("FieldManager A ManagedFields time has been updated:\nBefore:\n%v\nAfter:\n%v", + previousEntries["fieldmanager_a_test"], newEntries["fieldmanager_a_test"]) + } +} + +func TestTakingOverManagedFieldsDuringApplyDoesNotModifyPreviousManagerTime(t *testing.T) { + var err error + f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "ConfigMap"), "", nil) + + err = applyObject(&f, "fieldmanager_a_test", []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "configmap" + }, + "data": { + "key_a": "value", + "key_b": value" + } + }`)) + if err != nil { + t.Fatal(err) + } + previousManagedFields := f.ManagedFields() + previousEntries := map[string]v1.ManagedFieldsEntry{} + for _, entry := range previousManagedFields { + previousEntries[entry.Manager] = entry + } + + time.Sleep(time.Second) + + err = applyObject(&f, "fieldmanager_b_test", []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "configmap" + }, + "data": { + "key_b": "new-value" + } + }`)) + if err != nil { + t.Fatal(err) + } + newManagedFields := f.ManagedFields() + newEntries := map[string]v1.ManagedFieldsEntry{} + for _, entry := range newManagedFields { + newEntries[entry.Manager] = entry + } + + if !previousEntries["fieldmanager_a_test"].Time.Equal(newEntries["fieldmanager_a_test"].Time) { + t.Errorf("FieldManager A ManagedFields time has been updated:\nBefore:\n%v\nAfter:\n%v", + previousEntries["fieldmanager_a_test"], newEntries["fieldmanager_a_test"]) } } @@ -78,6 +433,28 @@ func (NoopManager) Update(liveObj, newObj runtime.Object, managed Managed, manag return nil, nil, nil } +func updateObject(f *TestFieldManager, fieldManagerName string, object []byte) error { + obj := &unstructured.Unstructured{Object: map[string]interface{}{}} + if err := yaml.Unmarshal(object, &obj.Object); err != nil { + return fmt.Errorf("error decoding YAML: %v", err) + } + if err := f.Update(obj, fieldManagerName); err != nil { + return fmt.Errorf("failed to update object: %v", err) + } + return nil +} + +func applyObject(f *TestFieldManager, fieldManagerName string, object []byte) error { + obj := &unstructured.Unstructured{Object: map[string]interface{}{}} + if err := yaml.Unmarshal(object, &obj.Object); err != nil { + return fmt.Errorf("error decoding YAML: %v", err) + } + if err := f.Apply(obj, fieldManagerName, true); err != nil { + return fmt.Errorf("failed to apply object: %v", err) + } + return nil +} + // Ensures that if ManagedFieldsUpdater gets a nil value from its nested manager // chain (meaning the operation was a no-op), then the ManagedFieldsUpdater // itself will return a copy of the input live object, with its managed fields