diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go index bff4962b924..4abbb67277e 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go +++ b/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 } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/BUILD b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/BUILD index fc70f5759f9..03c1a9c34d6 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/BUILD +++ b/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", 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 05d3123f41d..cd7d51ac298 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 @@ -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 -// use-case), and simply updates the managed fields in the output -// object. -func (f *FieldManager) Update(liveObj, newObj runtime.Object, manager string) (runtime.Object, error) { +// Update implements FieldManager. +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 -// object and update the managed fields. -func (f *FieldManager) Apply(liveObj runtime.Object, patch []byte, fieldManager string, force bool) (runtime.Object, error) { +// Apply implements FieldManager. +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 { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go index 45da6460a45..82ff3d65d15 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go +++ b/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 - f.liveObj, err = f.fieldManager.Apply(f.liveObj, obj, manager, force) + out, 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 - f.liveObj, err = f.fieldManager.Update(f.liveObj, obj, manager) + out, 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() diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/skipnonapplied.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/skipnonapplied.go new file mode 100644 index 00000000000..4b8f26a050f --- /dev/null +++ b/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) +} diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/skipnonapplied_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/skipnonapplied_test.go new file mode 100644 index 00000000000..102167577a8 --- /dev/null +++ b/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()) + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go index 10d5f032dce..6f5f422e051 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/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) { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go index bc47a72ee72..03393bcfaf7 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go +++ b/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 diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go index 144fd44e2e1..6996f79082b 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go +++ b/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 {