kubernetes
/
apiserver
mirror of https://github.com/kubernetes/apiserver.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #104182 from liggitt/suggestion-double-call 

Avoid spurious calls to update/delete validation

Kubernetes-commit: 7f6d463b34301b6b8840af8f642184df367ab262
This commit is contained in:
Kubernetes Publisher 2021-08-06 05:41:19 -07:00
parent 1dd7efb8f2 29227fd4ee
commit 72710fb766
5 changed files with 71 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
go.mod
View File

@ -47,7 +47,7 @@ require (
k8s.io/api v0.0.0-20210806000319-499b6f90564c
k8s.io/apimachinery v0.0.0-20210805051055-f7769293e6f1
k8s.io/client-go v0.0.0-20210806000600-0f5acb8c39dd
k8s.io/component-base v0.0.0-20210805120716-92bebfd2c985
k8s.io/component-base v0.0.0-20210806040719-3cb663ee50af
k8s.io/klog/v2 v2.9.0
k8s.io/kube-openapi v0.0.0-20210421082810-95288971da7e
k8s.io/utils v0.0.0-20210707171843-4b05e18ac7d9
@ -60,5 +60,5 @@ replace (
k8s.io/api => k8s.io/api v0.0.0-20210806000319-499b6f90564c
k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20210805051055-f7769293e6f1
k8s.io/client-go => k8s.io/client-go v0.0.0-20210806000600-0f5acb8c39dd
k8s.io/component-base => k8s.io/component-base v0.0.0-20210805120716-92bebfd2c985
k8s.io/component-base => k8s.io/component-base v0.0.0-20210806040719-3cb663ee50af
)

4
go.sum
View File

@ -786,8 +786,8 @@ k8s.io/apimachinery v0.0.0-20210805051055-f7769293e6f1 h1:cVpwhaGeh/tNPBeYbFff3t
k8s.io/apimachinery v0.0.0-20210805051055-f7769293e6f1/go.mod h1:O3oNtNadZdeOMxHFVxOreoznohCpy0z6mocxbZr7oJ0=
k8s.io/client-go v0.0.0-20210806000600-0f5acb8c39dd h1:rJtT7gZ4LkZG+xtM6/iJvnzOx5XW+fNicRxMOYT0u/w=
k8s.io/client-go v0.0.0-20210806000600-0f5acb8c39dd/go.mod h1:AptIKtgqoSsdZsOhDruqHKBn7GeBrnbH5CgVkI7tpYU=
k8s.io/component-base v0.0.0-20210805120716-92bebfd2c985 h1:5AICcEUyTnjI3rCqtY3n7sjZRLrCtI8Ejt9jnY+24kQ=
k8s.io/component-base v0.0.0-20210805120716-92bebfd2c985/go.mod h1:uuSJv3vcTE8Ays2CdTgQdaDyg2nYrkK3jVrLDSV3uAo=
k8s.io/component-base v0.0.0-20210806040719-3cb663ee50af h1:TvSosXU6sV/t6L77kj/UEyYFTzmLdxRnKIOJN5UcAls=
k8s.io/component-base v0.0.0-20210806040719-3cb663ee50af/go.mod h1:68YquEVTuJRDLUMJsRvsUyaFu7wBQnbVZlAV1oMkH58=
k8s.io/gengo v0.0.0-20200413195148-3a45101e95ac/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0=
k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE=
k8s.io/klog/v2 v2.9.0 h1:D7HV+n1V57XeZ0m6tdRkfknthUaM06VFbWldOFh8kzM=

2
pkg/registry/generic/registry/store_test.go
View File

@ -1158,6 +1158,8 @@ func TestStoreUpdateHooksInnerRetry(t *testing.T) {
registry.Decorator = tc.decorator
ttlFailDone = false
registry.TTLFunc = tc.ttl
// force storage to use a cached object with a non-matching resourceVersion to guarantee a live lookup + retry
created.(*example.Pod).ResourceVersion += "0"
registry.Storage.Storage = &staleGuaranteedUpdateStorage{Interface: registry.Storage.Storage, cachedObj: created}
_, _, err = registry.Update(testContext, pod.Name, rest.DefaultUpdatedObjectInfo(pod), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{})
if err != nil && !tc.expectError {

30
pkg/storage/etcd3/store.go
View File

@ -230,12 +230,22 @@ func (s *store) conditionalDelete(
}
// It's possible we're working with stale data.
// Remember the revision of the potentially stale data and the resulting update error
cachedRev := origState.rev
cachedUpdateErr := err
// Actually fetch
origState, err = getCurrentState()
if err != nil {
return err
}
origStateIsCurrent = true
// it turns out our cached data was not stale, return the error
if cachedRev == origState.rev {
return cachedUpdateErr
}
// Retry
continue
}
@ -246,12 +256,22 @@ func (s *store) conditionalDelete(
}
// It's possible we're working with stale data.
// Remember the revision of the potentially stale data and the resulting update error
cachedRev := origState.rev
cachedUpdateErr := err
// Actually fetch
origState, err = getCurrentState()
if err != nil {
return err
}
origStateIsCurrent = true
// it turns out our cached data was not stale, return the error
if cachedRev == origState.rev {
return cachedUpdateErr
}
// Retry
continue
}
@ -345,12 +365,22 @@ func (s *store) GuaranteedUpdate(
}
// It's possible we were working with stale data
// Remember the revision of the potentially stale data and the resulting update error
cachedRev := origState.rev
cachedUpdateErr := err
// Actually fetch
origState, err = getCurrentState()
if err != nil {
return err
}
origStateIsCurrent = true
// it turns out our cached data was not stale, return the error
if cachedRev == origState.rev {
return cachedUpdateErr
}
// Retry
continue
}

36
pkg/storage/etcd3/store_test.go
View File

@ -453,15 +453,20 @@ func TestValidateDeletionWithSuggestion(t *testing.T) {
key, originalPod := testPropogateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "name"}})
// Check that validaing fresh object fails.
// Check that validaing fresh object fails is called once and fails.
validationCalls := 0
validationError := fmt.Errorf("validation error")
validateNothing := func(_ context.Context, _ runtime.Object) error {
validationCalls++
return validationError
}
out := &example.Pod{}
if err := store.Delete(ctx, key, out, nil, validateNothing, originalPod); err != validationError {
t.Errorf("Unexpected failure during deletion: %v", err)
}
if validationCalls != 1 {
t.Errorf("validate function should have been called once, called %d", validationCalls)
}
// First update, so originalPod is outdated.
updatedPod := &example.Pod{}
@ -961,6 +966,35 @@ func TestGuaranteedUpdateWithSuggestionAndConflict(t *testing.T) {
if updatedPod2.Name != "foo-3" {
t.Errorf("unexpected pod name: %q", updatedPod2.Name)
}
// Third, update using a current version as the suggestion.
// Return an error and make sure that SimpleUpdate is NOT called a second time,
// since the live lookup shows the suggestion was already up to date.
attempts := 0
updatedPod3 := &example.Pod{}
err = store.GuaranteedUpdate(ctx, key, updatedPod3, false, nil,
storage.SimpleUpdate(func(obj runtime.Object) (runtime.Object, error) {
pod := obj.(*example.Pod)
if pod.Name != updatedPod2.Name || pod.ResourceVersion != updatedPod2.ResourceVersion {
t.Errorf(
"unexpected live object (name=%s, rv=%s), expected name=%s, rv=%s",
pod.Name,
pod.ResourceVersion,
updatedPod2.Name,
updatedPod2.ResourceVersion,
)
}
attempts++
return nil, fmt.Errorf("validation or admission error")
}),
updatedPod2,
)
if err == nil {
t.Fatalf("expected error, got none")
}
if attempts != 1 {
t.Errorf("expected 1 attempt, got %d", attempts)
}
}
func TestTransformationFailure(t *testing.T) {