Skip to content

Commit d585527

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#91690 from apelisse/ignore-failures
fieldManager: Ignore and log all errors when updating managedFields
2 parents c0455a1 + 5378a78 commit d585527

File tree

7 files changed

+193
-29
lines changed

7 files changed

+193
-29
lines changed

‎staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,7 @@ func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Int
156156
if err != nil {
157157
return nil, fmt.Errorf("failed to create new object (Create for %v): %v", scope.Kind, err)
158158
}
159-
obj, err = scope.FieldManager.Update(liveObj, obj, managerOrUserAgent(options.FieldManager, req.UserAgent()))
160-
if err != nil {
161-
return nil, fmt.Errorf("failed to update object (Create for %v) managed fields: %v", scope.Kind, err)
162-
}
159+
obj = scope.FieldManager.UpdateNoErrors(liveObj, obj, managerOrUserAgent(options.FieldManager, req.UserAgent()))
163160
}
164161
if mutatingAdmission, ok := admit.(admission.MutationInterface); ok && mutatingAdmission.Handles(admission.Create) {
165162
if err := mutatingAdmission.Admit(ctx, admissionAttributes, scope); err != nil {

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

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,14 @@ package fieldmanager
1818

1919
import (
2020
"fmt"
21+
"time"
2122

2223
"k8s.io/apimachinery/pkg/api/meta"
2324
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2425
"k8s.io/apimachinery/pkg/runtime"
2526
"k8s.io/apimachinery/pkg/runtime/schema"
2627
"k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal"
28+
"k8s.io/klog/v2"
2729
openapiproto "k8s.io/kube-openapi/pkg/util/proto"
2830
"sigs.k8s.io/structured-merge-diff/v3/fieldpath"
2931
)
@@ -37,6 +39,8 @@ const DefaultMaxUpdateManagers int = 10
3739
// starts being tracked from the object's creation, instead of from the first time the object is applied to.
3840
const DefaultTrackOnCreateProbability float32 = 1
3941

42+
var atMostEverySecond = internal.NewAtMostEvery(time.Second)
43+
4044
// Managed groups a fieldpath.ManagedFields together with the timestamps associated with each operation.
4145
type Managed interface {
4246
// Fields gets the fieldpath.ManagedFields.
@@ -120,7 +124,9 @@ func (f *FieldManager) Update(liveObj, newObj runtime.Object, manager string) (o
120124
// don't understand managedFields from deleting it accidentally.
121125
managed, err = internal.DecodeObjectManagedFields(liveObj)
122126
if err != nil {
123-
return nil, fmt.Errorf("failed to decode managed fields: %v", err)
127+
// If we also can't decode the liveObject, then
128+
// just restart managedFields from scratch.
129+
managed = internal.NewEmptyManaged()
124130
}
125131
}
126132

@@ -138,6 +144,25 @@ func (f *FieldManager) Update(liveObj, newObj runtime.Object, manager string) (o
138144
return object, nil
139145
}
140146

147+
// UpdateNoErrors is the same as Update, but it will not return
148+
// errors. If an error happens, the object is returned with
149+
// managedFields cleared.
150+
func (f *FieldManager) UpdateNoErrors(liveObj, newObj runtime.Object, manager string) runtime.Object {
151+
obj, err := f.Update(liveObj, newObj, manager)
152+
if err != nil {
153+
atMostEverySecond.Do(func() {
154+
klog.Errorf("[SHOULD NOT HAPPEN] failed to update managedFields for %v: %v",
155+
newObj.GetObjectKind().GroupVersionKind(),
156+
err)
157+
})
158+
// Explicitly remove managedFields on failure, so that
159+
// we can't have garbage in it.
160+
internal.RemoveObjectManagedFields(newObj)
161+
return newObj
162+
}
163+
return obj
164+
}
165+
141166
// Apply is used when server-side apply is called, as it merges the
142167
// object and updates the managed fields.
143168
func (f *FieldManager) Apply(liveObj, appliedObj runtime.Object, manager string, force bool) (object runtime.Object, err error) {

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ func (m *managedStruct) Times() map[string]*metav1.Time {
5353
return m.times
5454
}
5555

56+
// NewEmptyManaged creates an empty ManagedInterface.
57+
func NewEmptyManaged() ManagedInterface {
58+
return NewManaged(fieldpath.ManagedFields{}, map[string]*metav1.Time{})
59+
}
60+
5661
// NewManaged creates a ManagedInterface from a fieldpath.ManagedFields and the timestamps associated with each operation.
5762
func NewManaged(f fieldpath.ManagedFields, t map[string]*metav1.Time) ManagedInterface {
5863
return &managedStruct{

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

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,12 @@ package fieldmanager
1818

1919
import (
2020
"fmt"
21-
"time"
2221

2322
"k8s.io/apimachinery/pkg/api/errors"
2423
"k8s.io/apimachinery/pkg/api/meta"
2524
"k8s.io/apimachinery/pkg/runtime"
2625
"k8s.io/apimachinery/pkg/runtime/schema"
2726
"k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal"
28-
"k8s.io/klog/v2"
2927
openapiproto "k8s.io/kube-openapi/pkg/util/proto"
3028
"sigs.k8s.io/structured-merge-diff/v3/fieldpath"
3129
"sigs.k8s.io/structured-merge-diff/v3/merge"
@@ -41,7 +39,6 @@ type structuredMergeManager struct {
4139
}
4240

4341
var _ Manager = &structuredMergeManager{}
44-
var atMostEverySecond = internal.NewAtMostEvery(time.Second)
4542

4643
// NewStructuredMergeManager creates a new Manager that merges apply requests
4744
// and update managed fields for other types of requests.
@@ -98,19 +95,11 @@ func (f *structuredMergeManager) Update(liveObj, newObj runtime.Object, managed
9895
}
9996
newObjTyped, err := f.typeConverter.ObjectToTyped(newObjVersioned)
10097
if err != nil {
101-
// Return newObj and just by-pass fields update. This really shouldn't happen.
102-
atMostEverySecond.Do(func() {
103-
klog.Errorf("[SHOULD NOT HAPPEN] failed to create typed new object of type %v: %v", newObjVersioned.GetObjectKind().GroupVersionKind(), err)
104-
})
105-
return newObj, managed, nil
98+
return nil, nil, fmt.Errorf("failed to convert new object (%v) to smd typed: %v", newObjVersioned.GetObjectKind().GroupVersionKind(), err)
10699
}
107100
liveObjTyped, err := f.typeConverter.ObjectToTyped(liveObjVersioned)
108101
if err != nil {
109-
// Return newObj and just by-pass fields update. This really shouldn't happen.
110-
atMostEverySecond.Do(func() {
111-
klog.Errorf("[SHOULD NOT HAPPEN] failed to create typed live object of type %v: %v", liveObjVersioned.GetObjectKind().GroupVersionKind(), err)
112-
})
113-
return newObj, managed, nil
102+
return nil, nil, fmt.Errorf("failed to convert live object (%v) to smd typed: %v", liveObjVersioned.GetObjectKind().GroupVersionKind(), err)
114103
}
115104
apiVersion := fieldpath.APIVersion(f.groupVersion.String())
116105

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -323,9 +323,7 @@ func (p *jsonPatcher) applyPatchToCurrentObject(currentObject runtime.Object) (r
323323
}
324324

325325
if p.fieldManager != nil {
326-
if objToUpdate, err = p.fieldManager.Update(currentObject, objToUpdate, managerOrUserAgent(p.options.FieldManager, p.userAgent)); err != nil {
327-
return nil, fmt.Errorf("failed to update object (json PATCH for %v) managed fields: %v", p.kind, err)
328-
}
326+
objToUpdate = p.fieldManager.UpdateNoErrors(currentObject, objToUpdate, managerOrUserAgent(p.options.FieldManager, p.userAgent))
329327
}
330328
return objToUpdate, nil
331329
}
@@ -408,9 +406,7 @@ func (p *smpPatcher) applyPatchToCurrentObject(currentObject runtime.Object) (ru
408406
}
409407

410408
if p.fieldManager != nil {
411-
if newObj, err = p.fieldManager.Update(currentObject, newObj, managerOrUserAgent(p.options.FieldManager, p.userAgent)); err != nil {
412-
return nil, fmt.Errorf("failed to update object (smp PATCH for %v) managed fields: %v", p.kind, err)
413-
}
409+
newObj = p.fieldManager.UpdateNoErrors(currentObject, newObj, managerOrUserAgent(p.options.FieldManager, p.userAgent))
414410
}
415411
return newObj, nil
416412
}

‎staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -131,11 +131,7 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa
131131
if scope.FieldManager != nil {
132132
transformers = append(transformers, func(_ context.Context, newObj, liveObj runtime.Object) (runtime.Object, error) {
133133
if shouldUpdateManagedFields {
134-
obj, err := scope.FieldManager.Update(liveObj, newObj, managerOrUserAgent(options.FieldManager, req.UserAgent()))
135-
if err != nil {
136-
return nil, fmt.Errorf("failed to update object (Update for %v) managed fields: %v", scope.Kind, err)
137-
}
138-
return obj, nil
134+
return scope.FieldManager.UpdateNoErrors(liveObj, newObj, managerOrUserAgent(options.FieldManager, req.UserAgent())), nil
139135
}
140136
return newObj, nil
141137
})

‎test/integration/apiserver/apply/apply_test.go‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,6 +1422,162 @@ func TestClearManagedFieldsWithUpdate(t *testing.T) {
14221422
}
14231423
}
14241424

1425+
// TestErrorsDontFail
1426+
func TestErrorsDontFail(t *testing.T) {
1427+
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ServerSideApply, true)()
1428+
1429+
_, client, closeFn := setup(t)
1430+
defer closeFn()
1431+
1432+
// Tries to create with a managed fields that has an empty `fieldsType`.
1433+
_, err := client.CoreV1().RESTClient().Post().
1434+
Namespace("default").
1435+
Resource("configmaps").
1436+
Param("fieldManager", "apply_test").
1437+
Body([]byte(`{
1438+
"apiVersion": "v1",
1439+
"kind": "ConfigMap",
1440+
"metadata": {
1441+
"name": "test-cm",
1442+
"namespace": "default",
1443+
"managedFields": [{
1444+
"manager": "apply_test",
1445+
"operation": "Apply",
1446+
"apiVersion": "v1",
1447+
"time": "2019-07-08T09:31:18Z",
1448+
"fieldsType": "",
1449+
"fieldsV1": {}
1450+
}],
1451+
"labels": {
1452+
"test-label": "test"
1453+
}
1454+
},
1455+
"data": {
1456+
"key": "value"
1457+
}
1458+
}`)).
1459+
Do(context.TODO()).
1460+
Get()
1461+
if err != nil {
1462+
t.Fatalf("Failed to create object with empty fieldsType: %v", err)
1463+
}
1464+
}
1465+
1466+
func TestErrorsDontFailUpdate(t *testing.T) {
1467+
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ServerSideApply, true)()
1468+
1469+
_, client, closeFn := setup(t)
1470+
defer closeFn()
1471+
1472+
_, err := client.CoreV1().RESTClient().Post().
1473+
Namespace("default").
1474+
Resource("configmaps").
1475+
Param("fieldManager", "apply_test").
1476+
Body([]byte(`{
1477+
"apiVersion": "v1",
1478+
"kind": "ConfigMap",
1479+
"metadata": {
1480+
"name": "test-cm",
1481+
"namespace": "default",
1482+
"labels": {
1483+
"test-label": "test"
1484+
}
1485+
},
1486+
"data": {
1487+
"key": "value"
1488+
}
1489+
}`)).
1490+
Do(context.TODO()).
1491+
Get()
1492+
if err != nil {
1493+
t.Fatalf("Failed to create object: %v", err)
1494+
}
1495+
1496+
_, err = client.CoreV1().RESTClient().Put().
1497+
Namespace("default").
1498+
Resource("configmaps").
1499+
Name("test-cm").
1500+
Param("fieldManager", "apply_test").
1501+
Body([]byte(`{
1502+
"apiVersion": "v1",
1503+
"kind": "ConfigMap",
1504+
"metadata": {
1505+
"name": "test-cm",
1506+
"namespace": "default",
1507+
"managedFields": [{
1508+
"manager": "apply_test",
1509+
"operation": "Apply",
1510+
"apiVersion": "v1",
1511+
"time": "2019-07-08T09:31:18Z",
1512+
"fieldsType": "",
1513+
"fieldsV1": {}
1514+
}],
1515+
"labels": {
1516+
"test-label": "test"
1517+
}
1518+
},
1519+
"data": {
1520+
"key": "value"
1521+
}
1522+
}`)).
1523+
Do(context.TODO()).
1524+
Get()
1525+
if err != nil {
1526+
t.Fatalf("Failed to update object with empty fieldsType: %v", err)
1527+
}
1528+
}
1529+
1530+
func TestErrorsDontFailPatch(t *testing.T) {
1531+
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ServerSideApply, true)()
1532+
1533+
_, client, closeFn := setup(t)
1534+
defer closeFn()
1535+
1536+
_, err := client.CoreV1().RESTClient().Post().
1537+
Namespace("default").
1538+
Resource("configmaps").
1539+
Param("fieldManager", "apply_test").
1540+
Body([]byte(`{
1541+
"apiVersion": "v1",
1542+
"kind": "ConfigMap",
1543+
"metadata": {
1544+
"name": "test-cm",
1545+
"namespace": "default",
1546+
"labels": {
1547+
"test-label": "test"
1548+
}
1549+
},
1550+
"data": {
1551+
"key": "value"
1552+
}
1553+
}`)).
1554+
Do(context.TODO()).
1555+
Get()
1556+
if err != nil {
1557+
t.Fatalf("Failed to create object: %v", err)
1558+
}
1559+
1560+
_, err = client.CoreV1().RESTClient().Patch(types.JSONPatchType).
1561+
Namespace("default").
1562+
Resource("configmaps").
1563+
Name("test-cm").
1564+
Param("fieldManager", "apply_test").
1565+
Body([]byte(`[{"op": "replace", "path": "/metadata/managedFields", "value": [{
1566+
"manager": "apply_test",
1567+
"operation": "Apply",
1568+
"apiVersion": "v1",
1569+
"time": "2019-07-08T09:31:18Z",
1570+
"fieldsType": "",
1571+
"fieldsV1": {}
1572+
}]}]`)).
1573+
Do(context.TODO()).
1574+
Get()
1575+
if err != nil {
1576+
t.Fatalf("Failed to patch object with empty FieldsType: %v", err)
1577+
}
1578+
1579+
}
1580+
14251581
var podBytes = []byte(`
14261582
apiVersion: v1
14271583
kind: Pod

0 commit comments

Comments
 (0)