Skip to content

Commit

Permalink
Merge pull request #117419 from alexzielenski/automated-cherry-pick-o…
Browse files Browse the repository at this point in the history
…f-#116865-upstream-release-1.25

Automated cherry pick of #116865: move check for noop managed field timestamp updates
  • Loading branch information
k8s-ci-robot committed Sep 18, 2023
2 parents 51a65b2 + 723357d commit 57b2c23
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 12 deletions.
3 changes: 0 additions & 3 deletions staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go
Expand Up @@ -660,9 +660,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 @@ -191,15 +191,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 @@ -37,6 +37,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 @@ -652,6 +653,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 @@ -28,6 +28,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 @@ -257,6 +258,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

0 comments on commit 57b2c23

Please sign in to comment.