diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go index d6d21d450d1..466593dd4d1 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go @@ -98,7 +98,7 @@ func (f *FieldManager) Update(liveObj, newObj runtime.Object, manager string) (r // If the managed field is empty or we failed to decode it, // let's try the live object. This is to prevent clients who // don't understand managedFields from deleting it accidentally. - if err != nil || len(managed) == 0 { + if err != nil || len(managed.Fields) == 0 { managed, err = internal.DecodeObjectManagedFields(liveObj) if err != nil { return nil, fmt.Errorf("failed to decode managed fields: %v", err) @@ -133,11 +133,14 @@ func (f *FieldManager) Update(liveObj, newObj runtime.Object, manager string) (r } // TODO(apelisse) use the first return value when unions are implemented - _, managed, err = f.updater.Update(liveObjTyped, newObjTyped, apiVersion, managed, manager) + _, managed.Fields, err = f.updater.Update(liveObjTyped, newObjTyped, apiVersion, managed.Fields, manager) if err != nil { return nil, fmt.Errorf("failed to update ManagedFields: %v", err) } - managed = f.stripFields(managed, manager) + managed.Fields = f.stripFields(managed.Fields, manager) + + // Update the time in the managedFieldsEntry for this operation + managed.Times[manager] = &metav1.Time{Time: time.Now().UTC()} if err := internal.EncodeObjectManagedFields(newObj, managed); err != nil { return nil, fmt.Errorf("failed to encode managed fields: %v", err) @@ -192,14 +195,17 @@ func (f *FieldManager) Apply(liveObj runtime.Object, patch []byte, fieldManager } apiVersion := fieldpath.APIVersion(f.groupVersion.String()) - newObjTyped, managed, err := f.updater.Apply(liveObjTyped, patchObjTyped, apiVersion, managed, manager, force) + newObjTyped, managedFields, err := f.updater.Apply(liveObjTyped, patchObjTyped, apiVersion, managed.Fields, manager, force) if err != nil { if conflicts, ok := err.(merge.Conflicts); ok { return nil, internal.NewConflictError(conflicts) } return nil, err } - managed = f.stripFields(managed, manager) + managed.Fields = f.stripFields(managedFields, manager) + + // Update the time in the managedFieldsEntry for this operation + managed.Times[manager] = &metav1.Time{Time: time.Now().UTC()} newObj, err := f.typeConverter.TypedToObject(newObjTyped) if err != nil { @@ -236,7 +242,6 @@ func (f *FieldManager) buildManagerInfo(prefix string, operation metav1.ManagedF Manager: prefix, Operation: operation, APIVersion: f.groupVersion.String(), - Time: &metav1.Time{Time: time.Now().UTC()}, } if managerInfo.Manager == "" { managerInfo.Manager = "unknown" diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields.go index 00a4481f06d..c71a47b0005 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields.go @@ -28,6 +28,12 @@ import ( "sigs.k8s.io/structured-merge-diff/fieldpath" ) +// Managed groups a fieldpath.ManagedFields together with the timestamps associated with each operation. +type Managed struct { + Fields fieldpath.ManagedFields + Times map[string]*metav1.Time +} + // RemoveObjectManagedFields removes the ManagedFields from the object // before we merge so that it doesn't appear in the ManagedFields // recursively. @@ -40,9 +46,9 @@ func RemoveObjectManagedFields(obj runtime.Object) { } // DecodeObjectManagedFields extracts and converts the objects ManagedFields into a fieldpath.ManagedFields. -func DecodeObjectManagedFields(from runtime.Object) (fieldpath.ManagedFields, error) { +func DecodeObjectManagedFields(from runtime.Object) (Managed, error) { if from == nil { - return fieldpath.ManagedFields{}, nil + return Managed{}, nil } accessor, err := meta.Accessor(from) if err != nil { @@ -51,42 +57,44 @@ func DecodeObjectManagedFields(from runtime.Object) (fieldpath.ManagedFields, er managed, err := decodeManagedFields(accessor.GetManagedFields()) if err != nil { - return nil, fmt.Errorf("failed to convert managed fields from API: %v", err) + return Managed{}, fmt.Errorf("failed to convert managed fields from API: %v", err) } return managed, err } // EncodeObjectManagedFields converts and stores the fieldpathManagedFields into the objects ManagedFields -func EncodeObjectManagedFields(obj runtime.Object, fields fieldpath.ManagedFields) error { +func EncodeObjectManagedFields(obj runtime.Object, managed Managed) error { accessor, err := meta.Accessor(obj) if err != nil { panic(fmt.Sprintf("couldn't get accessor: %v", err)) } - managed, err := encodeManagedFields(fields) + encodedManagedFields, err := encodeManagedFields(managed) if err != nil { return fmt.Errorf("failed to convert back managed fields to API: %v", err) } - accessor.SetManagedFields(managed) + accessor.SetManagedFields(encodedManagedFields) return nil } // decodeManagedFields converts ManagedFields from the wire format (api format) // to the format used by sigs.k8s.io/structured-merge-diff -func decodeManagedFields(encodedManagedFields []metav1.ManagedFieldsEntry) (managedFields fieldpath.ManagedFields, err error) { - managedFields = make(fieldpath.ManagedFields, len(encodedManagedFields)) +func decodeManagedFields(encodedManagedFields []metav1.ManagedFieldsEntry) (managed Managed, err error) { + managed.Fields = make(fieldpath.ManagedFields, len(encodedManagedFields)) + managed.Times = make(map[string]*metav1.Time, len(encodedManagedFields)) for _, encodedVersionedSet := range encodedManagedFields { manager, err := BuildManagerIdentifier(&encodedVersionedSet) if err != nil { - return nil, fmt.Errorf("error decoding manager from %v: %v", encodedVersionedSet, err) + return Managed{}, fmt.Errorf("error decoding manager from %v: %v", encodedVersionedSet, err) } - managedFields[manager], err = decodeVersionedSet(&encodedVersionedSet) + managed.Fields[manager], err = decodeVersionedSet(&encodedVersionedSet) if err != nil { - return nil, fmt.Errorf("error decoding versioned set from %v: %v", encodedVersionedSet, err) + return Managed{}, fmt.Errorf("error decoding versioned set from %v: %v", encodedVersionedSet, err) } + managed.Times[manager] = encodedVersionedSet.Time } - return managedFields, nil + return managed, nil } // BuildManagerIdentifier creates a manager identifier string from a ManagedFieldsEntry @@ -96,11 +104,13 @@ func BuildManagerIdentifier(encodedManager *metav1.ManagedFieldsEntry) (manager // Never include the fields in the manager identifier encodedManagerCopy.Fields = nil - // For appliers, don't include the APIVersion or Time in the manager identifier, + // Never include the time in the manager identifier + encodedManagerCopy.Time = nil + + // For appliers, don't include the APIVersion in the manager identifier, // so it will always have the same manager identifier each time it applied. if encodedManager.Operation == metav1.ManagedFieldsOperationApply { encodedManagerCopy.APIVersion = "" - encodedManagerCopy.Time = nil } // Use the remaining fields to build the manager identifier @@ -126,21 +136,17 @@ func decodeVersionedSet(encodedVersionedSet *metav1.ManagedFieldsEntry) (version // encodeManagedFields converts ManagedFields from the format used by // sigs.k8s.io/structured-merge-diff to the wire format (api format) -func encodeManagedFields(managedFields fieldpath.ManagedFields) (encodedManagedFields []metav1.ManagedFieldsEntry, err error) { - // Sort the keys so a predictable order will be used. - managers := []string{} - for manager := range managedFields { - managers = append(managers, manager) - } - sort.Strings(managers) - +func encodeManagedFields(managed Managed) (encodedManagedFields []metav1.ManagedFieldsEntry, err error) { encodedManagedFields = []metav1.ManagedFieldsEntry{} - for _, manager := range managers { - versionedSet := managedFields[manager] + for manager := range managed.Fields { + versionedSet := managed.Fields[manager] v, err := encodeManagerVersionedSet(manager, versionedSet) if err != nil { return nil, fmt.Errorf("error encoding versioned set for %v: %v", manager, err) } + if t, ok := managed.Times[manager]; ok { + v.Time = t + } encodedManagedFields = append(encodedManagedFields, *v) } return sortEncodedManagedFields(encodedManagedFields) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields_test.go index 61aa135d0ac..703a10ab816 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields_test.go @@ -169,7 +169,7 @@ manager: foo operation: Update time: "2001-02-03T04:05:06Z" `, - expected: "{\"manager\":\"foo\",\"operation\":\"Update\",\"apiVersion\":\"v1\",\"time\":\"2001-02-03T04:05:06Z\"}", + expected: "{\"manager\":\"foo\",\"operation\":\"Update\",\"apiVersion\":\"v1\"}", }, { managedFieldsEntry: ` diff --git a/test/integration/apiserver/apply/apply_test.go b/test/integration/apiserver/apply/apply_test.go index 803888c1f8a..a4b6efef872 100644 --- a/test/integration/apiserver/apply/apply_test.go +++ b/test/integration/apiserver/apply/apply_test.go @@ -291,6 +291,21 @@ func TestApplyManagedFields(t *testing.T) { t.Fatalf("Failed to create object using Apply patch: %v", err) } + _, err = client.CoreV1().RESTClient().Patch(types.MergePatchType). + Namespace("default"). + Resource("configmaps"). + Name("test-cm"). + Param("fieldManager", "updater"). + Body([]byte(`{"data":{"new-key": "value"}}`)).Do().Get() + if err != nil { + t.Fatalf("Failed to patch object: %v", err) + } + + // Sleep for one second to make sure that the times of each update operation is different. + // This will let us check that update entries with the same manager name are grouped together, + // and that the most recent update time is recorded in the grouped entry. + time.Sleep(1 * time.Second) + _, err = client.CoreV1().RESTClient().Patch(types.MergePatchType). Namespace("default"). Resource("configmaps"). @@ -332,6 +347,7 @@ func TestApplyManagedFields(t *testing.T) { "manager": "apply_test", "operation": "Apply", "apiVersion": "v1", + "time": "` + accessor.GetManagedFields()[0].Time.UTC().Format(time.RFC3339) + `", "fields": { "f:metadata": { "f:labels": { @@ -347,20 +363,26 @@ func TestApplyManagedFields(t *testing.T) { "time": "` + accessor.GetManagedFields()[1].Time.UTC().Format(time.RFC3339) + `", "fields": { "f:data": { - "f:key": {} + "f:key": {}, + "f:new-key": {} } } } ] }, "data": { - "key": "new value" + "key": "new value", + "new-key": "value" } }`) if string(expected) != string(actual) { t.Fatalf("Expected:\n%v\nGot:\n%v", string(expected), string(actual)) } + + if accessor.GetManagedFields()[0].Time.UTC().Format(time.RFC3339) == accessor.GetManagedFields()[1].Time.UTC().Format(time.RFC3339) { + t.Fatalf("Expected times to be different but got:\n%v", string(actual)) + } } // TestApplyRemovesEmptyManagedFields there are no empty managers in managedFields