From d0726e4b1354b1c8c3978b96ab7b01d13a2b6340 Mon Sep 17 00:00:00 2001 From: wojtekt Date: Wed, 16 Dec 2020 09:28:44 +0100 Subject: [PATCH] Unify variable naming between GuaranteedUpdate and Delete in storage --- .../pkg/registry/generic/registry/dryrun.go | 4 ++-- .../apiserver/pkg/storage/cacher/cacher.go | 2 ++ .../apiserver/pkg/storage/etcd3/store.go | 24 +++++++++---------- .../apiserver/pkg/storage/interfaces.go | 8 +++---- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun.go index 469893cd1ef..e25684a8a53 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun.go @@ -78,7 +78,7 @@ func (s *DryRunnableStorage) List(ctx context.Context, key string, opts storage. func (s *DryRunnableStorage) GuaranteedUpdate( ctx context.Context, key string, ptrToType runtime.Object, ignoreNotFound bool, - preconditions *storage.Preconditions, tryUpdate storage.UpdateFunc, dryRun bool, suggestion runtime.Object) error { + preconditions *storage.Preconditions, tryUpdate storage.UpdateFunc, dryRun bool, cachedExistingObject runtime.Object) error { if dryRun { err := s.Storage.Get(ctx, key, storage.GetOptions{IgnoreNotFound: ignoreNotFound}, ptrToType) if err != nil { @@ -98,7 +98,7 @@ func (s *DryRunnableStorage) GuaranteedUpdate( } return s.copyInto(out, ptrToType) } - return s.Storage.GuaranteedUpdate(ctx, key, ptrToType, ignoreNotFound, preconditions, tryUpdate, suggestion) + return s.Storage.GuaranteedUpdate(ctx, key, ptrToType, ignoreNotFound, preconditions, tryUpdate, cachedExistingObject) } func (s *DryRunnableStorage) Count(key string) (int64, error) { diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go index 9170a63a46e..8fd6199445d 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go @@ -752,6 +752,8 @@ func (c *Cacher) GuaranteedUpdate( if elem, exists, err := c.watchCache.GetByKey(key); err != nil { klog.Errorf("GetByKey returned error: %v", err) } else if exists { + // DeepCopy the object since we modify resource version when serializing the + // current object. currObj := elem.(*storeElement).Object.DeepCopyObject() return c.storage.GuaranteedUpdate(ctx, key, ptrToType, ignoreNotFound, preconditions, tryUpdate, currObj) } diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go index b5de5a84c8b..c62e408d647 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go @@ -285,7 +285,7 @@ func (s *store) conditionalDelete( // GuaranteedUpdate implements storage.Interface.GuaranteedUpdate. func (s *store) GuaranteedUpdate( ctx context.Context, key string, out runtime.Object, ignoreNotFound bool, - preconditions *storage.Preconditions, tryUpdate storage.UpdateFunc, suggestion runtime.Object) error { + preconditions *storage.Preconditions, tryUpdate storage.UpdateFunc, cachedExistingObject runtime.Object) error { trace := utiltrace.New("GuaranteedUpdate etcd3", utiltrace.Field{"type", getTypeName(out)}) defer trace.LogIfLong(500 * time.Millisecond) @@ -306,12 +306,12 @@ func (s *store) GuaranteedUpdate( } var origState *objState - var mustCheckData bool - if suggestion != nil { - origState, err = s.getStateFromObject(suggestion) - mustCheckData = true + var origStateIsCurrent bool + if cachedExistingObject != nil { + origState, err = s.getStateFromObject(cachedExistingObject) } else { origState, err = getCurrentState() + origStateIsCurrent = true } if err != nil { return err @@ -322,7 +322,7 @@ func (s *store) GuaranteedUpdate( for { if err := preconditions.Check(key, origState.obj); err != nil { // If our data is already up to date, return the error - if !mustCheckData { + if origStateIsCurrent { return err } @@ -332,7 +332,7 @@ func (s *store) GuaranteedUpdate( if err != nil { return err } - mustCheckData = false + origStateIsCurrent = true // Retry continue } @@ -340,7 +340,7 @@ func (s *store) GuaranteedUpdate( ret, ttl, err := s.updateState(origState, tryUpdate) if err != nil { // If our data is already up to date, return the error - if !mustCheckData { + if origStateIsCurrent { return err } @@ -350,7 +350,7 @@ func (s *store) GuaranteedUpdate( if err != nil { return err } - mustCheckData = false + origStateIsCurrent = true // Retry continue } @@ -363,12 +363,12 @@ func (s *store) GuaranteedUpdate( // if we skipped the original Get in this loop, we must refresh from // etcd in order to be sure the data in the store is equivalent to // our desired serialization - if mustCheckData { + if !origStateIsCurrent { origState, err = getCurrentState() if err != nil { return err } - mustCheckData = false + origStateIsCurrent = true if !bytes.Equal(data, origState.data) { // original data changed, restart loop continue @@ -412,7 +412,7 @@ func (s *store) GuaranteedUpdate( return err } trace.Step("Retry value restored") - mustCheckData = false + origStateIsCurrent = true continue } putResp := txnResp.Responses[0].GetResponsePut() diff --git a/staging/src/k8s.io/apiserver/pkg/storage/interfaces.go b/staging/src/k8s.io/apiserver/pkg/storage/interfaces.go index 7de3bf9363a..ccfe98467e9 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/interfaces.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/interfaces.go @@ -220,9 +220,9 @@ type Interface interface { // or zero value in 'ptrToType' parameter otherwise. // If the object to update has the same value as previous, it won't do any update // but will return the object in 'ptrToType' parameter. - // If 'suggestion' is non-nil, it can be used as a suggestion about the current version - // of the object to avoid read operation from storage to get it. However, the - // implementations have to retry in case suggestion is stale. + // If 'cachedExistingObject' is non-nil, it can be used as a suggestion about the + // current version of the object to avoid read operation from storage to get it. + // However, the implementations have to retry in case suggestion is stale. // // Example: // @@ -244,7 +244,7 @@ type Interface interface { // ) GuaranteedUpdate( ctx context.Context, key string, ptrToType runtime.Object, ignoreNotFound bool, - precondtions *Preconditions, tryUpdate UpdateFunc, suggestion runtime.Object) error + precondtions *Preconditions, tryUpdate UpdateFunc, cachedExistingObject runtime.Object) error // Count returns number of different entries under the key (generally being path prefix). Count(key string) (int64, error)