From 4e32d183d0257c9f6c7f8342d1f9aa7f28458f2f Mon Sep 17 00:00:00 2001 From: Antoine Pelisse Date: Thu, 16 May 2019 11:07:47 -0700 Subject: [PATCH] fieldManager: Ignore conversion errors to internal types Errors on updates are bad because they usually come from controllers and it's very hard to take actions on them. We also don't want to start breaking kubernetes clusters if something in a schema happens a way we didn't foresee (even though we've tried to be diligent and test as much as possible, these can still happen). Log an identifiable error when they happen. Ideally people can look in the logs to find these and report them, or providers can look for these in logs and make sure they don't happen. Only conversion to internal types are going to be logged and ignored. It means that we're still failing for: - Version conversions. If we can't convert the object from one version to another, - Unions. If we can't normalize the union, - Invalid MangedFields sent in the object. If something has changed the ManagedFields to an invalid value. - Failure to serialize the manager information, this really shouldn't happen. - Encoding the ManagedFields --- .../apiserver/pkg/endpoints/handlers/fieldmanager/BUILD | 1 + .../pkg/endpoints/handlers/fieldmanager/fieldmanager.go | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) 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 10379edd553..dda52401348 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/BUILD @@ -14,6 +14,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal:go_default_library", + "//vendor/k8s.io/klog:go_default_library", "//vendor/k8s.io/kube-openapi/pkg/util/proto:go_default_library", "//vendor/sigs.k8s.io/structured-merge-diff/fieldpath:go_default_library", "//vendor/sigs.k8s.io/structured-merge-diff/merge: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 212bcadfe2c..885330d965b 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 @@ -27,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal" + "k8s.io/klog" openapiproto "k8s.io/kube-openapi/pkg/util/proto" "sigs.k8s.io/structured-merge-diff/fieldpath" "sigs.k8s.io/structured-merge-diff/merge" @@ -115,11 +116,15 @@ func (f *FieldManager) Update(liveObj, newObj runtime.Object, manager string) (r internal.RemoveObjectManagedFields(newObjVersioned) newObjTyped, err := f.typeConverter.ObjectToTyped(newObjVersioned) if err != nil { - return nil, fmt.Errorf("failed to create typed new object: %v", err) + // Return newObj and just by-pass fields update. This really shouldn't happen. + klog.Errorf("[SHOULD NOT HAPPEN] failed to create typed new object: %v", err) + return newObj, nil } liveObjTyped, err := f.typeConverter.ObjectToTyped(liveObjVersioned) if err != nil { - return nil, fmt.Errorf("failed to create typed live object: %v", err) + // Return newObj and just by-pass fields update. This really shouldn't happen. + klog.Errorf("[SHOULD NOT HAPPEN] failed to create typed live object: %v", err) + return newObj, nil } apiVersion := fieldpath.APIVersion(f.groupVersion.String()) manager, err = f.buildManagerInfo(manager, metav1.ManagedFieldsOperationUpdate)