Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

move check for noop managed field timestamp updates #116865

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 0 additions & 3 deletions staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go
Expand Up @@ -654,9 +654,6 @@ func (p *patcher) patchResource(ctx context.Context, scope *RequestScope) (runti
}

transformers := []rest.TransformFunc{p.applyPatch, p.applyAdmission, dedupOwnerReferencesTransformer}
if scope.FieldManager != nil {
transformers = append(transformers, fieldmanager.IgnoreManagedFieldsTimestampsTransformer)
}

wasCreated := false
p.updatedObjectInfo = rest.DefaultUpdatedObjectInfo(nil, transformers...)
Expand Down
9 changes: 0 additions & 9 deletions staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go
Expand Up @@ -189,15 +189,6 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa
})
}

// Ignore changes that only affect managed fields
// timestamps. FieldManager can't know about changes
// like normalized fields, defaulted fields and other
// mutations.
// Only makes sense when SSA field manager is being used
if scope.FieldManager != nil {
transformers = append(transformers, fieldmanager.IgnoreManagedFieldsTimestampsTransformer)
}

createAuthorizerAttributes := authorizer.AttributesRecord{
User: userInfo,
ResourceRequest: true,
Expand Down
Expand Up @@ -38,6 +38,7 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/apimachinery/pkg/watch"
"k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/registry/generic"
"k8s.io/apiserver/pkg/registry/rest"
Expand Down Expand Up @@ -671,6 +672,15 @@ func (e *Store) Update(ctx context.Context, name string, objInfo rest.UpdatedObj
if err := rest.BeforeUpdate(e.UpdateStrategy, ctx, obj, existing); err != nil {
return nil, nil, err
}

// Ignore changes that only affect managed fields timestamps.
// FieldManager can't know about changes like normalized fields, defaulted
// fields and other mutations.
obj, err = fieldmanager.IgnoreManagedFieldsTimestampsTransformer(ctx, obj, existing)
if err != nil {
return nil, nil, err
}

// at this point we have a fully formed object. It is time to call the validators that the apiserver
// handling chain wants to enforce.
if updateValidation != nil {
Expand Down
50 changes: 50 additions & 0 deletions test/integration/apiserver/apply/apply_test.go
Expand Up @@ -27,6 +27,7 @@ import (
"time"

"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/require"
"sigs.k8s.io/yaml"

appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -249,6 +250,55 @@ func getRV(obj runtime.Object) (string, error) {
return acc.GetResourceVersion(), nil
}

func TestNoopChangeCreationTime(t *testing.T) {
client, closeFn := setup(t)
defer closeFn()

ssBytes := []byte(`{
"apiVersion": "v1",
"kind": "ConfigMap",
"metadata": {
"name": "myconfig",
"creationTimestamp": null,
"resourceVersion": null
},
"data": {
"key": "value"
}
}`)

obj, err := client.CoreV1().RESTClient().Patch(types.ApplyPatchType).
Namespace("default").
Param("fieldManager", "apply_test").
Resource("configmaps").
Name("myconfig").
Body(ssBytes).
Do(context.TODO()).
Get()
if err != nil {
t.Fatalf("Failed to create object: %v", err)
}

require.NoError(t, err)
// Sleep for one second to make sure that the times of each update operation is different.
time.Sleep(1200 * time.Millisecond)

newObj, err := client.CoreV1().RESTClient().Patch(types.ApplyPatchType).
Namespace("default").
Param("fieldManager", "apply_test").
Resource("configmaps").
Name("myconfig").
Body(ssBytes).
Do(context.TODO()).
Get()
if err != nil {
t.Fatalf("Failed to create object: %v", err)
}

require.NoError(t, err)
require.Equal(t, obj, newObj)
}

// TestNoSemanticUpdateAppleSameResourceVersion makes sure that APPLY requests which makes no semantic changes
// will not change the resource version (no write to etcd is done)
//
Expand Down