Split fieldmanager with interface

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go

@@ -815,7 +815,7 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd
 		}
 		if utilfeature.DefaultFeatureGate.Enabled(features.ServerSideApply) {
 			reqScope := *requestScopes[v.Name]
-			reqScope.FieldManager, err = fieldmanager.NewCRDFieldManager(
+			fm, err := fieldmanager.NewCRDFieldManager(
 				openAPIModels,
 				reqScope.Convertor,
 				reqScope.Defaulter,
@@ -826,6 +826,7 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd
 			if err != nil {
 				return nil, err
 			}
+			reqScope.FieldManager = fieldmanager.NewSkipNonAppliedManager(fm, reqScope.Creater, reqScope.Kind)
 			requestScopes[v.Name] = &reqScope
 		}
 

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

@@ -2,7 +2,10 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "go_default_library",
-    srcs = ["fieldmanager.go"],
+    srcs = [
+        "fieldmanager.go",
+        "skipnonapplied.go",
+    ],
     importmap = "k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager",
     importpath = "k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager",
     visibility = ["//visibility:public"],
@@ -41,7 +44,10 @@ filegroup(
 
 go_test(
     name = "go_default_test",
-    srcs = ["fieldmanager_test.go"],
+    srcs = [
+        "fieldmanager_test.go",
+        "skipnonapplied_test.go",
+    ],
     embed = [":go_default_library"],
     deps = [
         "//staging/src/k8s.io/api/core/v1:go_default_library",

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

@@ -36,7 +36,18 @@ import (
 
 // FieldManager updates the managed fields and merge applied
 // configurations.
-type FieldManager struct {
+type FieldManager interface {
+	// Update is used when the object has already been merged (non-apply
+	// use-case), and simply updates the managed fields in the output
+	// object.
+	Update(liveObj, newObj runtime.Object, manager string) (runtime.Object, error)
+
+	// Apply is used when server-side apply is called, as it merges the
+	// object and update the managed fields.
+	Apply(liveObj runtime.Object, patch []byte, fieldManager string, force bool) (runtime.Object, error)
+}
+
+type fieldManager struct {
 	typeConverter   internal.TypeConverter
 	objectConverter runtime.ObjectConvertor
 	objectDefaulter runtime.ObjectDefaulter
@@ -45,15 +56,17 @@ type FieldManager struct {
 	updater         merge.Updater
 }
 
+var _ FieldManager = &fieldManager{}
+
 // NewFieldManager creates a new FieldManager that merges apply requests
 // and update managed fields for other types of requests.
-func NewFieldManager(models openapiproto.Models, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, gv schema.GroupVersion, hub schema.GroupVersion) (*FieldManager, error) {
+func NewFieldManager(models openapiproto.Models, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, gv schema.GroupVersion, hub schema.GroupVersion) (FieldManager, error) {
 	typeConverter, err := internal.NewTypeConverter(models, false)
 	if err != nil {
 		return nil, err
 	}
 
-	return &FieldManager{
+	return &fieldManager{
 		typeConverter:   typeConverter,
 		objectConverter: objectConverter,
 		objectDefaulter: objectDefaulter,
@@ -68,7 +81,7 @@ func NewFieldManager(models openapiproto.Models, objectConverter runtime.ObjectC
 // NewCRDFieldManager creates a new FieldManager specifically for
 // CRDs. This allows for the possibility of fields which are not defined
 // in models, as well as having no models defined at all.
-func NewCRDFieldManager(models openapiproto.Models, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, gv schema.GroupVersion, hub schema.GroupVersion, preserveUnknownFields bool) (_ *FieldManager, err error) {
+func NewCRDFieldManager(models openapiproto.Models, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, gv schema.GroupVersion, hub schema.GroupVersion, preserveUnknownFields bool) (_ FieldManager, err error) {
 	var typeConverter internal.TypeConverter = internal.DeducedTypeConverter{}
 	if models != nil {
 		typeConverter, err = internal.NewTypeConverter(models, preserveUnknownFields)
@@ -76,7 +89,7 @@ func NewCRDFieldManager(models openapiproto.Models, objectConverter runtime.Obje
 			return nil, err
 		}
 	}
-	return &FieldManager{
+	return &fieldManager{
 		typeConverter:   typeConverter,
 		objectConverter: objectConverter,
 		objectDefaulter: objectDefaulter,
@@ -88,10 +101,8 @@ func NewCRDFieldManager(models openapiproto.Models, objectConverter runtime.Obje
 	}, nil
 }
 
-// Update is used when the object has already been merged (non-apply
+// Update implements FieldManager.
-// use-case), and simply updates the managed fields in the output
+func (f *fieldManager) Update(liveObj, newObj runtime.Object, manager string) (runtime.Object, error) {
-// object.
-func (f *FieldManager) Update(liveObj, newObj runtime.Object, manager string) (runtime.Object, error) {
 	// If the object doesn't have metadata, we should just return without trying to
 	// set the managedFields at all, so creates/updates/patches will work normally.
 	if _, err := meta.Accessor(newObj); err != nil {
@@ -111,10 +122,6 @@ func (f *FieldManager) Update(liveObj, newObj runtime.Object, manager string) (r
 			return nil, fmt.Errorf("failed to decode managed fields: %v", err)
 		}
 	}
-	// if managed field is still empty, skip updating managed fields altogether
-	if len(managed.Fields) == 0 {
-		return newObj, nil
-	}
 	newObjVersioned, err := f.toVersioned(newObj)
 	if err != nil {
 		return nil, fmt.Errorf("failed to convert new object to proper version: %v", err)
@@ -172,15 +179,13 @@ func (f *FieldManager) Update(liveObj, newObj runtime.Object, manager string) (r
 	return newObj, nil
 }
 
-// Apply is used when server-side apply is called, as it merges the
+// Apply implements FieldManager.
-// object and update the managed fields.
+func (f *fieldManager) Apply(liveObj runtime.Object, patch []byte, fieldManager string, force bool) (runtime.Object, error) {
-func (f *FieldManager) Apply(liveObj runtime.Object, patch []byte, fieldManager string, force bool) (runtime.Object, error) {
 	// If the object doesn't have metadata, apply isn't allowed.
-	accessor, err := meta.Accessor(liveObj)
+	_, err := meta.Accessor(liveObj)
 	if err != nil {
 		return nil, fmt.Errorf("couldn't get accessor: %v", err)
 	}
-	missingManagedFields := (len(accessor.GetManagedFields()) == 0)
 
 	managed, err := internal.DecodeObjectManagedFields(liveObj)
 	if err != nil {
@@ -225,23 +230,6 @@ func (f *FieldManager) Apply(liveObj runtime.Object, patch []byte, fieldManager
 	}
 
 	apiVersion := fieldpath.APIVersion(f.groupVersion.String())
-	// if managed field is missing, create a single entry for all the fields
-	if missingManagedFields {
-		unknownManager, err := internal.BuildManagerIdentifier(&metav1.ManagedFieldsEntry{
-			Manager:    "before-first-apply",
-			Operation:  metav1.ManagedFieldsOperationUpdate,
-			APIVersion: f.groupVersion.String(),
-		})
-		if err != nil {
-			return nil, fmt.Errorf("failed to create manager for existing fields: %v", err)
-		}
-		unknownFieldSet, err := liveObjTyped.ToFieldSet()
-		if err != nil {
-			return nil, fmt.Errorf("failed to create fieldset for existing fields: %v", err)
-		}
-		managed.Fields[unknownManager] = fieldpath.NewVersionedSet(unknownFieldSet, apiVersion, false)
-		f.stripFields(managed.Fields, unknownManager)
-	}
 	newObjTyped, managedFields, err := f.updater.Apply(liveObjTyped, patchObjTyped, apiVersion, managed.Fields, manager, force)
 	if err != nil {
 		if conflicts, ok := err.(merge.Conflicts); ok {
@@ -276,15 +264,15 @@ func (f *FieldManager) Apply(liveObj runtime.Object, patch []byte, fieldManager
 	return newObjUnversioned, nil
 }
 
-func (f *FieldManager) toVersioned(obj runtime.Object) (runtime.Object, error) {
+func (f *fieldManager) toVersioned(obj runtime.Object) (runtime.Object, error) {
 	return f.objectConverter.ConvertToVersion(obj, f.groupVersion)
 }
 
-func (f *FieldManager) toUnversioned(obj runtime.Object) (runtime.Object, error) {
+func (f *fieldManager) toUnversioned(obj runtime.Object) (runtime.Object, error) {
 	return f.objectConverter.ConvertToVersion(obj, f.hubVersion)
 }
 
-func (f *FieldManager) buildManagerInfo(prefix string, operation metav1.ManagedFieldsOperationType) (string, error) {
+func (f *fieldManager) buildManagerInfo(prefix string, operation metav1.ManagedFieldsOperationType) (string, error) {
 	managerInfo := metav1.ManagedFieldsEntry{
 		Manager:    prefix,
 		Operation:  operation,
@@ -313,7 +301,7 @@ var stripSet = fieldpath.NewSet(
 )
 
 // stripFields removes a predefined set of paths found in typed from managed and returns the updated ManagedFields
-func (f *FieldManager) stripFields(managed fieldpath.ManagedFields, manager string) fieldpath.ManagedFields {
+func (f *fieldManager) stripFields(managed fieldpath.ManagedFields, manager string) fieldpath.ManagedFields {
 	vs, ok := managed[manager]
 	if ok {
 		if vs == nil {

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

@@ -23,7 +23,6 @@ import (
 	"testing"
 	"time"
 
-	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -54,7 +53,7 @@ type fakeObjectDefaulter struct{}
 func (d *fakeObjectDefaulter) Default(in runtime.Object) {}
 
 type TestFieldManager struct {
-	fieldManager *fieldmanager.FieldManager
+	fieldManager fieldmanager.FieldManager
 	liveObj      runtime.Object
 }
 
@@ -86,14 +85,18 @@ func (f *TestFieldManager) Reset() {
 }
 
 func (f *TestFieldManager) Apply(obj []byte, manager string, force bool) error {
-	var err error
+	out, err := f.fieldManager.Apply(f.liveObj, obj, manager, force)
-	f.liveObj, err = f.fieldManager.Apply(f.liveObj, obj, manager, force)
+	if err == nil {
+		f.liveObj = out
+	}
 	return err
 }
 
 func (f *TestFieldManager) Update(obj runtime.Object, manager string) error {
-	var err error
+	out, err := f.fieldManager.Update(f.liveObj, obj, manager)
-	f.liveObj, err = f.fieldManager.Update(f.liveObj, obj, manager)
+	if err == nil {
+		f.liveObj = out
+	}
 	return err
 }
 
@@ -106,21 +109,6 @@ func (f *TestFieldManager) ManagedFields() []metav1.ManagedFieldsEntry {
 	return accessor.GetManagedFields()
 }
 
-func TestUpdateOnlyDoesNotTrackManagedFields(t *testing.T) {
-	f := NewTestFieldManager()
-
-	updatedObj := &corev1.Pod{}
-	updatedObj.ObjectMeta.Labels = map[string]string{"k": "v"}
-
-	if err := f.Update(updatedObj, "fieldmanager_test"); err != nil {
-		t.Fatalf("failed to update object: %v", err)
-	}
-
-	if m := f.ManagedFields(); len(m) != 0 {
-		t.Fatalf("managedFields were tracked on update only: %v", m)
-	}
-}
-
 // TestUpdateApplyConflict tests that applying to an object, which wasn't created by apply, will give conflicts
 func TestUpdateApplyConflict(t *testing.T) {
 	f := NewTestFieldManager()

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

@@ -0,0 +1,79 @@
+/*
+Copyright 2019 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 (
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+type skipNonAppliedManager struct {
+	fieldManager  FieldManager
+	objectCreater runtime.ObjectCreater
+	gvk           schema.GroupVersionKind
+}
+
+var _ FieldManager = &skipNonAppliedManager{}
+
+// NewSkipNonAppliedManager creates a new wrapped FieldManager that only starts tracking managers after the first apply
+func NewSkipNonAppliedManager(fieldManager FieldManager, objectCreater runtime.ObjectCreater, gvk schema.GroupVersionKind) FieldManager {
+	return &skipNonAppliedManager{
+		fieldManager:  fieldManager,
+		objectCreater: objectCreater,
+		gvk:           gvk,
+	}
+}
+
+// Update implements FieldManager.
+func (f *skipNonAppliedManager) Update(liveObj, newObj runtime.Object, manager string) (runtime.Object, error) {
+	liveObjAccessor, err := meta.Accessor(liveObj)
+	if err != nil {
+		return newObj, nil
+	}
+	newObjAccessor, err := meta.Accessor(newObj)
+	if err != nil {
+		return newObj, nil
+	}
+	if len(liveObjAccessor.GetManagedFields()) == 0 && len(newObjAccessor.GetManagedFields()) == 0 {
+		return newObj, nil
+	}
+
+	return f.fieldManager.Update(liveObj, newObj, manager)
+}
+
+// Apply implements FieldManager.
+func (f *skipNonAppliedManager) Apply(liveObj runtime.Object, patch []byte, fieldManager string, force bool) (runtime.Object, error) {
+	liveObjAccessor, err := meta.Accessor(liveObj)
+	if err != nil {
+		return nil, fmt.Errorf("couldn't get accessor: %v", err)
+	}
+	if len(liveObjAccessor.GetManagedFields()) == 0 {
+		emptyObj, err := f.objectCreater.New(f.gvk)
+		if err != nil {
+			return nil, fmt.Errorf("failed to create empty object of type %v: %v", f.gvk, err)
+		}
+		liveObj, err = f.fieldManager.Update(emptyObj, liveObj, "before-first-apply")
+		if err != nil {
+			return nil, fmt.Errorf("failed to create manager for existing fields: %v", err)
+		}
+	}
+
+	return f.fieldManager.Apply(liveObj, patch, fieldManager, force)
+}

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

@@ -0,0 +1,128 @@
+/*
+Copyright 2019 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 (
+	"strings"
+	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager"
+)
+
+type fakeObjectCreater struct{}
+
+var _ runtime.ObjectCreater = &fakeObjectCreater{}
+
+func (*fakeObjectCreater) New(_ schema.GroupVersionKind) (runtime.Object, error) {
+	return &unstructured.Unstructured{Object: map[string]interface{}{}}, nil
+}
+
+func TestNoUpdateBeforeFirstApply(t *testing.T) {
+	f := NewTestFieldManager()
+	f.fieldManager = fieldmanager.NewSkipNonAppliedManager(f.fieldManager, &fakeObjectCreater{}, schema.GroupVersionKind{})
+
+	if err := f.Apply([]byte(`{
+		"apiVersion": "apps/v1",
+		"kind": "Pod",
+		"metadata": {
+			"name": "pod",
+			"labels": {"app": "nginx"}
+		},
+		"spec": {
+			"containers": [{
+				"name":  "nginx",
+				"image": "nginx:latest"
+			}]
+        }
+	}`), "fieldmanager_test_apply", false); err != nil {
+		t.Fatalf("failed to update object: %v", err)
+	}
+
+	if e, a := 1, len(f.ManagedFields()); e != a {
+		t.Fatalf("exected %v entries in managedFields, but got %v: %#v", e, a, f.ManagedFields())
+	}
+
+	if e, a := "fieldmanager_test_apply", f.ManagedFields()[0].Manager; e != a {
+		t.Fatalf("exected manager name to be %v, but got %v: %#v", e, a, f.ManagedFields())
+	}
+}
+
+func TestUpateBeforeFirstApply(t *testing.T) {
+	f := NewTestFieldManager()
+	f.fieldManager = fieldmanager.NewSkipNonAppliedManager(f.fieldManager, &fakeObjectCreater{}, schema.GroupVersionKind{})
+
+	updatedObj := &corev1.Pod{}
+	updatedObj.ObjectMeta.Labels = map[string]string{"app": "nginx"}
+
+	if err := f.Update(updatedObj, "fieldmanager_test_update"); err != nil {
+		t.Fatalf("failed to update object: %v", err)
+	}
+
+	if m := f.ManagedFields(); len(m) != 0 {
+		t.Fatalf("managedFields were tracked on update only: %v", m)
+	}
+
+	appliedBytes := []byte(`{
+		"apiVersion": "apps/v1",
+		"kind": "Pod",
+		"metadata": {
+			"name": "pod",
+			"labels": {"app": "nginx"}
+		},
+		"spec": {
+			"containers": [{
+				"name":  "nginx",
+				"image": "nginx:latest"
+			}]
+        }
+	}`)
+
+	err := f.Apply(appliedBytes, "fieldmanager_test_apply", false)
+	apiStatus, _ := err.(apierrors.APIStatus)
+	if err == nil || !apierrors.IsConflict(err) || len(apiStatus.Status().Details.Causes) != 1 {
+		t.Fatalf("Expecting to get one conflict but got %v", err)
+	}
+
+	if e, a := ".spec.containers", apiStatus.Status().Details.Causes[0].Field; e != a {
+		t.Fatalf("Expecting to conflict on field %q but conflicted on field %q: %v", e, a, err)
+	}
+
+	if e, a := "before-first-apply", apiStatus.Status().Details.Causes[0].Message; !strings.Contains(a, e) {
+		t.Fatalf("Expecting conflict message to contain %q but got %q: %v", e, a, err)
+	}
+
+	if err := f.Apply(appliedBytes, "fieldmanager_test_apply", true); err != nil {
+		t.Fatalf("failed to update object: %v", err)
+	}
+
+	if e, a := 2, len(f.ManagedFields()); e != a {
+		t.Fatalf("exected %v entries in managedFields, but got %v: %#v", e, a, f.ManagedFields())
+	}
+
+	if e, a := "fieldmanager_test_apply", f.ManagedFields()[0].Manager; e != a {
+		t.Fatalf("exected first manager name to be %v, but got %v: %#v", e, a, f.ManagedFields())
+	}
+
+	if e, a := "before-first-apply", f.ManagedFields()[1].Manager; e != a {
+		t.Fatalf("exected second manager name to be %v, but got %v: %#v", e, a, f.ManagedFields())
+	}
+}

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

@@ -296,7 +296,7 @@ type patchMechanism interface {
 type jsonPatcher struct {
 	*patcher
 
-	fieldManager *fieldmanager.FieldManager
+	fieldManager fieldmanager.FieldManager
 }
 
 func (p *jsonPatcher) applyPatchToCurrentObject(currentObject runtime.Object) (runtime.Object, error) {
@@ -364,7 +364,7 @@ type smpPatcher struct {
 
 	// Schema
 	schemaReferenceObj runtime.Object
-	fieldManager       *fieldmanager.FieldManager
+	fieldManager       fieldmanager.FieldManager
 }
 
 func (p *smpPatcher) applyPatchToCurrentObject(currentObject runtime.Object) (runtime.Object, error) {
@@ -404,7 +404,7 @@ type applyPatcher struct {
 	options      *metav1.PatchOptions
 	creater      runtime.ObjectCreater
 	kind         schema.GroupVersionKind
-	fieldManager *fieldmanager.FieldManager
+	fieldManager fieldmanager.FieldManager
 }
 
 func (p *applyPatcher) applyPatchToCurrentObject(obj runtime.Object) (runtime.Object, error) {

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

@@ -67,7 +67,7 @@ type RequestScope struct {
 	EquivalentResourceMapper runtime.EquivalentResourceMapper
 
 	TableConvertor rest.TableConvertor
-	FieldManager   *fieldmanager.FieldManager
+	FieldManager   fieldmanager.FieldManager
 
 	Resource    schema.GroupVersionResource
 	Kind        schema.GroupVersionKind

staging/src/k8s.io/apiserver/pkg/endpoints/installer.go

@@ -560,6 +560,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 		if err != nil {
 			return nil, fmt.Errorf("failed to create field manager: %v", err)
 		}
+		fm = fieldmanager.NewSkipNonAppliedManager(fm, a.group.Creater, fqKindToRegister)
 		reqScope.FieldManager = fm
 	}
 	for _, action := range actions {