Merge pull request #96393 from thockin/rest-hooks

Add BeginCreate and BeginUpdate REST hooks

pkg/registry/core/service/storage/storage.go

@@ -131,42 +131,33 @@ func (r *StatusREST) Update(ctx context.Context, name string, objInfo rest.Updat
 // applies on Get, Create, and Update, but we need to distinguish between them.
 //
 // This will be called on both Service and ServiceList types.
-func (r *GenericREST) defaultOnRead(obj runtime.Object) error {
+func (r *GenericREST) defaultOnRead(obj runtime.Object) {
-	service, ok := obj.(*api.Service)
+	switch s := obj.(type) {
-	if ok {
+	case *api.Service:
-		return r.defaultOnReadService(service)
+		r.defaultOnReadService(s)
+	case *api.ServiceList:
+		r.defaultOnReadServiceList(s)
+	default:
+		// This was not an object we can default.  This is not an error, as the
+		// caching layer can pass through here, too.
 	}
-
-	serviceList, ok := obj.(*api.ServiceList)
-	if ok {
-		return r.defaultOnReadServiceList(serviceList)
-	}
-
-	// This was not an object we can default.  This is not an error, as the
-	// caching layer can pass through here, too.
-	return nil
 }
 
 // defaultOnReadServiceList defaults a ServiceList.
-func (r *GenericREST) defaultOnReadServiceList(serviceList *api.ServiceList) error {
+func (r *GenericREST) defaultOnReadServiceList(serviceList *api.ServiceList) {
 	if serviceList == nil {
-		return nil
+		return
 	}
 
 	for i := range serviceList.Items {
-		err := r.defaultOnReadService(&serviceList.Items[i])
+		r.defaultOnReadService(&serviceList.Items[i])
-		if err != nil {
-			return err
-		}
 	}
-
-	return nil
 }
 
 // defaultOnReadService defaults a single Service.
-func (r *GenericREST) defaultOnReadService(service *api.Service) error {
+func (r *GenericREST) defaultOnReadService(service *api.Service) {
 	if service == nil {
-		return nil
+		return
 	}
 
 	// We might find Services that were written before ClusterIP became plural.
@@ -176,11 +167,11 @@ func (r *GenericREST) defaultOnReadService(service *api.Service) error {
 
 	// The rest of this does not apply unless dual-stack is enabled.
 	if !utilfeature.DefaultFeatureGate.Enabled(features.IPv6DualStack) {
-		return nil
+		return
 	}
 
 	if len(service.Spec.IPFamilies) > 0 {
-		return nil // already defaulted
+		return // already defaulted
 	}
 
 	// set clusterIPs based on ClusterIP
@@ -241,6 +232,4 @@ func (r *GenericREST) defaultOnReadService(service *api.Service) error {
 			}
 		}
 	}
-
-	return nil
 }

pkg/registry/core/service/storage/storage_test.go

@@ -358,10 +358,9 @@ func TestServiceDefaultOnRead(t *testing.T) {
 	}
 
 	testCases := []struct {
-		name      string
+		name   string
-		input     runtime.Object
+		input  runtime.Object
-		expectErr bool
+		expect runtime.Object
-		expect    runtime.Object
 	}{{
 		name:   "no change v4",
 		input:  makeService(nil),
@@ -403,9 +402,8 @@ func TestServiceDefaultOnRead(t *testing.T) {
 		}),
 		expect: makeService(nil),
 	}, {
-		name:      "not Service or ServiceList",
+		name:  "not Service or ServiceList",
-		input:     &api.Pod{},
+		input: &api.Pod{},
-		expectErr: false,
 	}}
 
 	for _, tc := range testCases {
@@ -435,13 +433,7 @@ func TestServiceDefaultOnRead(t *testing.T) {
 			defer storage.Store.DestroyFunc()
 
 			tmp := tc.input.DeepCopyObject()
-			err := storage.defaultOnRead(tmp)
+			storage.defaultOnRead(tmp)
-			if err != nil && !tc.expectErr {
-				t.Errorf("unexpected error: %v", err)
-			}
-			if err == nil && tc.expectErr {
-				t.Errorf("unexpected success")
-			}
 
 			svc, ok := tmp.(*api.Service)
 			if !ok {

staging/src/k8s.io/apiserver/pkg/registry/generic/registry/BUILD

@@ -46,6 +46,7 @@ go_test(
         "//staging/src/k8s.io/apiserver/pkg/storage/storagebackend/factory:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/storage/testing:go_default_library",
         "//staging/src/k8s.io/client-go/tools/cache:go_default_library",
+        "//vendor/github.com/google/gofuzz:go_default_library",
     ],
 )
 

staging/src/k8s.io/apiserver/pkg/registry/generic/registry/decorated_watcher.go

@@ -21,17 +21,18 @@ import (
 	"net/http"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/watch"
 )
 
 type decoratedWatcher struct {
 	w         watch.Interface
-	decorator ObjectFunc
+	decorator func(runtime.Object)
 	cancel    context.CancelFunc
 	resultCh  chan watch.Event
 }
 
-func newDecoratedWatcher(w watch.Interface, decorator ObjectFunc) *decoratedWatcher {
+func newDecoratedWatcher(w watch.Interface, decorator func(runtime.Object)) *decoratedWatcher {
 	ctx, cancel := context.WithCancel(context.Background())
 	d := &decoratedWatcher{
 		w:         w,
@@ -56,11 +57,7 @@ func (d *decoratedWatcher) run(ctx context.Context) {
 			}
 			switch recv.Type {
 			case watch.Added, watch.Modified, watch.Deleted, watch.Bookmark:
-				err := d.decorator(recv.Object)
+				d.decorator(recv.Object)
-				if err != nil {
-					send = makeStatusErrorEvent(err)
-					break
-				}
 				send = recv
 			case watch.Error:
 				send = recv

staging/src/k8s.io/apiserver/pkg/registry/generic/registry/decorated_watcher_test.go

@@ -17,7 +17,6 @@ limitations under the License.
 package registry
 
 import (
-	"fmt"
 	"testing"
 	"time"
 
@@ -30,10 +29,9 @@ import (
 
 func TestDecoratedWatcher(t *testing.T) {
 	w := watch.NewFake()
-	decorator := func(obj runtime.Object) error {
+	decorator := func(obj runtime.Object) {
 		pod := obj.(*example.Pod)
 		pod.Annotations = map[string]string{"decorated": "true"}
-		return nil
 	}
 	dw := newDecoratedWatcher(w, decorator)
 	defer dw.Stop()
@@ -53,23 +51,3 @@ func TestDecoratedWatcher(t *testing.T) {
 		t.Errorf("timeout after %v", wait.ForeverTestTimeout)
 	}
 }
-
-func TestDecoratedWatcherError(t *testing.T) {
-	w := watch.NewFake()
-	expErr := fmt.Errorf("expected error")
-	decorator := func(obj runtime.Object) error {
-		return expErr
-	}
-	dw := newDecoratedWatcher(w, decorator)
-	defer dw.Stop()
-
-	go w.Add(&example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}})
-	select {
-	case e := <-dw.ResultChan():
-		if e.Type != watch.Error {
-			t.Errorf("event type want=%v, get=%v", watch.Error, e.Type)
-		}
-	case <-time.After(wait.ForeverTestTimeout):
-		t.Errorf("timeout after %v", wait.ForeverTestTimeout)
-	}
-}

staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go

@@ -50,10 +50,23 @@ import (
 	"k8s.io/klog/v2"
 )
 
-// ObjectFunc is a function to act on a given object. An error may be returned
+// FinishFunc is a function returned by Begin hooks to complete an operation.
-// if the hook cannot be completed. An ObjectFunc may transform the provided
+type FinishFunc func(ctx context.Context, success bool)
-// object.
+
-type ObjectFunc func(obj runtime.Object) error
+// AfterDeleteFunc is the type used for the Store.AfterDelete hook.
+type AfterDeleteFunc func(obj runtime.Object, options *metav1.DeleteOptions)
+
+// BeginCreateFunc is the type used for the Store.BeginCreate hook.
+type BeginCreateFunc func(ctx context.Context, obj runtime.Object, options *metav1.CreateOptions) (FinishFunc, error)
+
+// AfterCreateFunc is the type used for the Store.AfterCreate hook.
+type AfterCreateFunc func(obj runtime.Object, options *metav1.CreateOptions)
+
+// BeginUpdateFunc is the type used for the Store.BeginUpdate hook.
+type BeginUpdateFunc func(ctx context.Context, obj, old runtime.Object, options *metav1.UpdateOptions) (FinishFunc, error)
+
+// AfterUpdateFunc is the type used for the Store.AfterUpdate hook.
+type AfterUpdateFunc func(obj runtime.Object, options *metav1.UpdateOptions)
 
 // GenericStore interface can be used for type assertions when we need to access the underlying strategies.
 type GenericStore interface {
@@ -63,10 +76,11 @@ type GenericStore interface {
 	GetExportStrategy() rest.RESTExportStrategy
 }
 
-// Store implements pkg/api/rest.StandardStorage. It's intended to be
+// Store implements k8s.io/apiserver/pkg/registry/rest.StandardStorage. It's
-// embeddable and allows the consumer to implement any non-generic functions
+// intended to be embeddable and allows the consumer to implement any
-// that are required. This object is intended to be copyable so that it can be
+// non-generic functions that are required. This object is intended to be
-// used in different ways but share the same underlying behavior.
+// copyable so that it can be used in different ways but share the same
+// underlying behavior.
 //
 // All fields are required unless specified.
 //
@@ -145,24 +159,37 @@ type Store struct {
 	// integrations that are above storage and should only be used for
 	// specific cases where storage of the value is not appropriate, since
 	// they cannot be watched.
-	Decorator ObjectFunc
+	Decorator func(runtime.Object)
+
 	// CreateStrategy implements resource-specific behavior during creation.
 	CreateStrategy rest.RESTCreateStrategy
+	// BeginCreate is an optional hook that returns a "transaction-like"
+	// commit/revert function which will be called at the end of the operation,
+	// but before AfterCreate and Decorator, indicating via the argument
+	// whether the operation succeeded.  If this returns an error, the function
+	// is not called.  Almost nobody should use this hook.
+	BeginCreate BeginCreateFunc
 	// AfterCreate implements a further operation to run after a resource is
 	// created and before it is decorated, optional.
-	AfterCreate ObjectFunc
+	AfterCreate AfterCreateFunc
 
 	// UpdateStrategy implements resource-specific behavior during updates.
 	UpdateStrategy rest.RESTUpdateStrategy
+	// BeginUpdate is an optional hook that returns a "transaction-like"
+	// commit/revert function which will be called at the end of the operation,
+	// but before AfterUpdate and Decorator, indicating via the argument
+	// whether the operation succeeded.  If this returns an error, the function
+	// is not called.  Almost nobody should use this hook.
+	BeginUpdate BeginUpdateFunc
 	// AfterUpdate implements a further operation to run after a resource is
 	// updated and before it is decorated, optional.
-	AfterUpdate ObjectFunc
+	AfterUpdate AfterUpdateFunc
 
 	// DeleteStrategy implements resource-specific behavior during deletion.
 	DeleteStrategy rest.RESTDeleteStrategy
 	// AfterDelete implements a further operation to run after a resource is
 	// deleted and before it is decorated, optional.
-	AfterDelete ObjectFunc
+	AfterDelete AfterDeleteFunc
 	// ReturnDeletedObject determines whether the Store returns the object
 	// that was deleted. Otherwise, return a generic success status response.
 	ReturnDeletedObject bool
@@ -171,9 +198,11 @@ type Store struct {
 	// If specified, this is checked in addition to standard finalizer,
 	// deletionTimestamp, and deletionGracePeriodSeconds checks.
 	ShouldDeleteDuringUpdate func(ctx context.Context, key string, obj, existing runtime.Object) bool
+
 	// ExportStrategy implements resource-specific behavior during export,
 	// optional. Exported objects are not decorated.
 	ExportStrategy rest.RESTExportStrategy
+
 	// TableConvertor is an optional interface for transforming items or lists
 	// of items into tabular output. If unset, the default will be used.
 	TableConvertor rest.TableConvertor
@@ -304,9 +333,7 @@ func (e *Store) List(ctx context.Context, options *metainternalversion.ListOptio
 		return nil, err
 	}
 	if e.Decorator != nil {
-		if err := e.Decorator(out); err != nil {
+		e.Decorator(out)
-			return nil, err
-		}
 	}
 	return out, nil
 }
@@ -335,8 +362,24 @@ func (e *Store) ListPredicate(ctx context.Context, p storage.SelectionPredicate,
 	return list, storeerr.InterpretListError(err, qualifiedResource)
 }
 
+// finishNothing is a do-nothing FinishFunc.
+func finishNothing(context.Context, bool) {}
+
 // Create inserts a new item according to the unique key from the object.
 func (e *Store) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
+	var finishCreate FinishFunc = finishNothing
+
+	if e.BeginCreate != nil {
+		fn, err := e.BeginCreate(ctx, obj, options)
+		if err != nil {
+			return nil, err
+		}
+		finishCreate = fn
+		defer func() {
+			finishCreate(ctx, false)
+		}()
+	}
+
 	if err := rest.BeforeCreate(e.CreateStrategy, ctx, obj); err != nil {
 		return nil, err
 	}
@@ -381,15 +424,17 @@ func (e *Store) Create(ctx context.Context, obj runtime.Object, createValidation
 		}
 		return nil, err
 	}
+	// The operation has succeeded.  Call the finish function if there is one,
+	// and then make sure the defer doesn't call it again.
+	fn := finishCreate
+	finishCreate = finishNothing
+	fn(ctx, true)
+
 	if e.AfterCreate != nil {
-		if err := e.AfterCreate(out); err != nil {
+		e.AfterCreate(out, options)
-			return nil, err
-		}
 	}
 	if e.Decorator != nil {
-		if err := e.Decorator(out); err != nil {
+		e.Decorator(out)
-			return nil, err
-		}
 	}
 	return out, nil
 }
@@ -424,16 +469,16 @@ func ShouldDeleteDuringUpdate(ctx context.Context, key string, obj, existing run
 
 // deleteWithoutFinalizers handles deleting an object ignoring its finalizer list.
 // Used for objects that are either been finalized or have never initialized.
-func (e *Store) deleteWithoutFinalizers(ctx context.Context, name, key string, obj runtime.Object, preconditions *storage.Preconditions, dryRun bool) (runtime.Object, bool, error) {
+func (e *Store) deleteWithoutFinalizers(ctx context.Context, name, key string, obj runtime.Object, preconditions *storage.Preconditions, options *metav1.DeleteOptions) (runtime.Object, bool, error) {
 	out := e.NewFunc()
 	klog.V(6).Infof("going to delete %s from registry, triggered by update", name)
 	// Using the rest.ValidateAllObjectFunc because the request is an UPDATE request and has already passed the admission for the UPDATE verb.
-	if err := e.Storage.Delete(ctx, key, out, preconditions, rest.ValidateAllObjectFunc, dryRun, nil); err != nil {
+	if err := e.Storage.Delete(ctx, key, out, preconditions, rest.ValidateAllObjectFunc, dryrun.IsDryRun(options.DryRun), nil); err != nil {
 		// Deletion is racy, i.e., there could be multiple update
 		// requests to remove all finalizers from the object, so we
 		// ignore the NotFound error.
 		if storage.IsNotFound(err) {
-			_, err := e.finalizeDelete(ctx, obj, true)
+			_, err := e.finalizeDelete(ctx, obj, true, options)
 			// clients are expecting an updated object if a PUT succeeded,
 			// but finalizeDelete returns a metav1.Status, so return
 			// the object in the request instead.
@@ -441,7 +486,7 @@ func (e *Store) deleteWithoutFinalizers(ctx context.Context, name, key string, o
 		}
 		return nil, false, storeerr.InterpretDeleteError(err, e.qualifiedResourceFromContext(ctx), name)
 	}
-	_, err := e.finalizeDelete(ctx, out, true)
+	_, err := e.finalizeDelete(ctx, out, true, options)
 	// clients are expecting an updated object if a PUT succeeded, but
 	// finalizeDelete returns a metav1.Status, so return the object in
 	// the request instead.
@@ -500,6 +545,19 @@ func (e *Store) Update(ctx context.Context, name string, objInfo rest.UpdatedObj
 		doUnconditionalUpdate := newResourceVersion == 0 && e.UpdateStrategy.AllowUnconditionalUpdate()
 
 		if existingResourceVersion == 0 {
+			var finishCreate FinishFunc = finishNothing
+
+			if e.BeginCreate != nil {
+				fn, err := e.BeginCreate(ctx, obj, newCreateOptionsFromUpdateOptions(options))
+				if err != nil {
+					return nil, nil, err
+				}
+				finishCreate = fn
+				defer func() {
+					finishCreate(ctx, false)
+				}()
+			}
+
 			creating = true
 			creatingObj = obj
 			if err := rest.BeforeCreate(e.CreateStrategy, ctx, obj); err != nil {
@@ -517,6 +575,12 @@ func (e *Store) Update(ctx context.Context, name string, objInfo rest.UpdatedObj
 				return nil, nil, err
 			}
 
+			// The operation has succeeded.  Call the finish function if there is one,
+			// and then make sure the defer doesn't call it again.
+			fn := finishCreate
+			finishCreate = finishNothing
+			fn(ctx, true)
+
 			return obj, &ttl, nil
 		}
 
@@ -544,6 +608,20 @@ func (e *Store) Update(ctx context.Context, name string, objInfo rest.UpdatedObj
 				return nil, nil, apierrors.NewConflict(qualifiedResource, name, fmt.Errorf(OptimisticLockErrorMsg))
 			}
 		}
+
+		var finishUpdate FinishFunc = finishNothing
+
+		if e.BeginUpdate != nil {
+			fn, err := e.BeginUpdate(ctx, obj, existing, options)
+			if err != nil {
+				return nil, nil, err
+			}
+			finishUpdate = fn
+			defer func() {
+				finishUpdate(ctx, false)
+			}()
+		}
+
 		if err := rest.BeforeUpdate(e.UpdateStrategy, ctx, obj, existing); err != nil {
 			return nil, nil, err
 		}
@@ -564,6 +642,13 @@ func (e *Store) Update(ctx context.Context, name string, objInfo rest.UpdatedObj
 		if err != nil {
 			return nil, nil, err
 		}
+
+		// The operation has succeeded.  Call the finish function if there is one,
+		// and then make sure the defer doesn't call it again.
+		fn := finishUpdate
+		finishUpdate = finishNothing
+		fn(ctx, true)
+
 		if int64(ttl) != res.TTL {
 			return obj, &ttl, nil
 		}
@@ -573,7 +658,7 @@ func (e *Store) Update(ctx context.Context, name string, objInfo rest.UpdatedObj
 	if err != nil {
 		// delete the object
 		if err == errEmptiedFinalizers {
-			return e.deleteWithoutFinalizers(ctx, name, key, deleteObj, storagePreconditions, dryrun.IsDryRun(options.DryRun))
+			return e.deleteWithoutFinalizers(ctx, name, key, deleteObj, storagePreconditions, newDeleteOptionsFromUpdateOptions(options))
 		}
 		if creating {
 			err = storeerr.InterpretCreateError(err, qualifiedResource, name)
@@ -586,25 +671,40 @@ func (e *Store) Update(ctx context.Context, name string, objInfo rest.UpdatedObj
 
 	if creating {
 		if e.AfterCreate != nil {
-			if err := e.AfterCreate(out); err != nil {
+			e.AfterCreate(out, newCreateOptionsFromUpdateOptions(options))
-				return nil, false, err
-			}
 		}
 	} else {
 		if e.AfterUpdate != nil {
-			if err := e.AfterUpdate(out); err != nil {
+			e.AfterUpdate(out, options)
-				return nil, false, err
-			}
 		}
 	}
 	if e.Decorator != nil {
-		if err := e.Decorator(out); err != nil {
+		e.Decorator(out)
-			return nil, false, err
-		}
 	}
 	return out, creating, nil
 }
 
+// This is a helper to convert UpdateOptions to CreateOptions for the
+// create-on-update path.
+func newCreateOptionsFromUpdateOptions(in *metav1.UpdateOptions) *metav1.CreateOptions {
+	co := &metav1.CreateOptions{
+		DryRun:       in.DryRun,
+		FieldManager: in.FieldManager,
+	}
+	co.TypeMeta.SetGroupVersionKind(metav1.SchemeGroupVersion.WithKind("CreateOptions"))
+	return co
+}
+
+// This is a helper to convert UpdateOptions to DeleteOptions for the
+// delete-on-update path.
+func newDeleteOptionsFromUpdateOptions(in *metav1.UpdateOptions) *metav1.DeleteOptions {
+	do := &metav1.DeleteOptions{
+		DryRun: in.DryRun,
+	}
+	do.TypeMeta.SetGroupVersionKind(metav1.SchemeGroupVersion.WithKind("DeleteOptions"))
+	return do
+}
+
 // Get retrieves the item from storage.
 func (e *Store) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) {
 	obj := e.NewFunc()
@@ -616,9 +716,7 @@ func (e *Store) Get(ctx context.Context, name string, options *metav1.GetOptions
 		return nil, storeerr.InterpretGetError(err, e.qualifiedResourceFromContext(ctx), name)
 	}
 	if e.Decorator != nil {
-		if err := e.Decorator(obj); err != nil {
+		e.Decorator(obj)
-			return nil, err
-		}
 	}
 	return obj, nil
 }
@@ -879,7 +977,7 @@ func (e *Store) updateForGracefulDeletionAndFinalizers(ctx context.Context, name
 		// we should fall through and truly delete the object.
 		return nil, false, true, out, lastExisting
 	case errAlreadyDeleting:
-		out, err = e.finalizeDelete(ctx, in, true)
+		out, err = e.finalizeDelete(ctx, in, true, options)
 		return err, false, false, out, lastExisting
 	default:
 		return storeerr.InterpretUpdateError(err, e.qualifiedResourceFromContext(ctx), name), false, false, out, lastExisting
@@ -913,7 +1011,7 @@ func (e *Store) Delete(ctx context.Context, name string, deleteValidation rest.V
 	}
 	// this means finalizers cannot be updated via DeleteOptions if a deletion is already pending
 	if pendingGraceful {
-		out, err := e.finalizeDelete(ctx, obj, false)
+		out, err := e.finalizeDelete(ctx, obj, false, options)
 		return out, false, err
 	}
 	// check if obj has pending finalizers
@@ -969,12 +1067,12 @@ func (e *Store) Delete(ctx context.Context, name string, deleteValidation rest.V
 		if storage.IsNotFound(err) && ignoreNotFound && lastExisting != nil {
 			// The lastExisting object may not be the last state of the object
 			// before its deletion, but it's the best approximation.
-			out, err := e.finalizeDelete(ctx, lastExisting, true)
+			out, err := e.finalizeDelete(ctx, lastExisting, true, options)
 			return out, true, err
 		}
 		return nil, false, storeerr.InterpretDeleteError(err, qualifiedResource, name)
 	}
-	out, err = e.finalizeDelete(ctx, out, true)
+	out, err = e.finalizeDelete(ctx, out, true, options)
 	return out, true, err
 }
 
@@ -1072,17 +1170,13 @@ func (e *Store) DeleteCollection(ctx context.Context, deleteValidation rest.Vali
 
 // finalizeDelete runs the Store's AfterDelete hook if runHooks is set and
 // returns the decorated deleted object if appropriate.
-func (e *Store) finalizeDelete(ctx context.Context, obj runtime.Object, runHooks bool) (runtime.Object, error) {
+func (e *Store) finalizeDelete(ctx context.Context, obj runtime.Object, runHooks bool, options *metav1.DeleteOptions) (runtime.Object, error) {
 	if runHooks && e.AfterDelete != nil {
-		if err := e.AfterDelete(obj); err != nil {
+		e.AfterDelete(obj, options)
-			return nil, err
-		}
 	}
 	if e.ReturnDeletedObject {
 		if e.Decorator != nil {
-			if err := e.Decorator(obj); err != nil {
+			e.Decorator(obj)
-				return nil, err
-			}
 		}
 		return obj, nil
 	}

staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go

@@ -18,6 +18,7 @@ package registry
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"path"
 	"reflect"
@@ -27,6 +28,7 @@ import (
 	"testing"
 	"time"
 
+	fuzz "github.com/google/gofuzz"
 	apitesting "k8s.io/apimachinery/pkg/api/apitesting"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -310,11 +312,6 @@ func TestStoreCreate(t *testing.T) {
 	// re-define delete strategy to have graceful delete capability
 	defaultDeleteStrategy := testRESTStrategy{scheme, names.SimpleNameGenerator, true, false, true}
 	registry.DeleteStrategy = testGracefulStrategy{defaultDeleteStrategy}
-	registry.Decorator = func(obj runtime.Object) error {
-		pod := obj.(*example.Pod)
-		pod.Status.Phase = example.PodPhase("Testing")
-		return nil
-	}
 
 	// create the object with denying admission
 	_, err := registry.Create(testContext, podA, denyCreateValidation, &metav1.CreateOptions{})
@@ -328,11 +325,6 @@ func TestStoreCreate(t *testing.T) {
 		t.Errorf("Unexpected error: %v", err)
 	}
 
-	// verify the decorator was called
-	if objA.(*example.Pod).Status.Phase != example.PodPhase("Testing") {
-		t.Errorf("Decorator was not called: %#v", objA)
-	}
-
 	// get the object
 	checkobj, err := registry.Get(testContext, podA.Name, &metav1.GetOptions{})
 	if err != nil {
@@ -376,6 +368,261 @@ func TestStoreCreate(t *testing.T) {
 	}
 }
 
+func TestNewCreateOptionsFromUpdateOptions(t *testing.T) {
+	f := fuzz.New().NilChance(0.0).NumElements(1, 1)
+
+	// The goal here is to trigger when any changes are made to either
+	// CreateOptions or UpdateOptions types, so we can update the converter.
+	for i := 0; i < 20; i++ {
+		in := &metav1.UpdateOptions{}
+		f.Fuzz(in)
+		in.TypeMeta.SetGroupVersionKind(metav1.SchemeGroupVersion.WithKind("CreateOptions"))
+
+		out := newCreateOptionsFromUpdateOptions(in)
+
+		// This sequence is intending to elide type information, but produce an
+		// intermediate structure (map) that can be manually patched up to make
+		// the comparison work as needed.
+
+		// Convert both structs to maps of primitives.
+		inBytes, err := json.Marshal(in)
+		if err != nil {
+			t.Fatalf("failed to json.Marshal(in): %v", err)
+		}
+		outBytes, err := json.Marshal(out)
+		if err != nil {
+			t.Fatalf("failed to json.Marshal(out): %v", err)
+		}
+		inMap := map[string]interface{}{}
+		if err := json.Unmarshal(inBytes, &inMap); err != nil {
+			t.Fatalf("failed to json.Unmarshal(in): %v", err)
+		}
+		outMap := map[string]interface{}{}
+		if err := json.Unmarshal(outBytes, &outMap); err != nil {
+			t.Fatalf("failed to json.Unmarshal(out): %v", err)
+		}
+
+		// Patch the maps to handle any expected differences before we compare
+		// - none for now.
+
+		// Compare the results.
+		inBytes, err = json.Marshal(inMap)
+		if err != nil {
+			t.Fatalf("failed to json.Marshal(in): %v", err)
+		}
+		outBytes, err = json.Marshal(outMap)
+		if err != nil {
+			t.Fatalf("failed to json.Marshal(out): %v", err)
+		}
+		if i, o := string(inBytes), string(outBytes); i != o {
+			t.Fatalf("output != input:\n  want: %s\n   got: %s", i, o)
+		}
+	}
+}
+
+func TestNewDeleteOptionsFromUpdateOptions(t *testing.T) {
+	f := fuzz.New().NilChance(0.0).NumElements(1, 1)
+
+	// The goal here is to trigger when any changes are made to either
+	// DeleteOptions or UpdateOptions types, so we can update the converter.
+	for i := 0; i < 20; i++ {
+		in := &metav1.UpdateOptions{}
+		f.Fuzz(in)
+		in.TypeMeta.SetGroupVersionKind(metav1.SchemeGroupVersion.WithKind("DeleteOptions"))
+
+		out := newDeleteOptionsFromUpdateOptions(in)
+
+		// This sequence is intending to elide type information, but produce an
+		// intermediate structure (map) that can be manually patched up to make
+		// the comparison work as needed.
+
+		// Convert both structs to maps of primitives.
+		inBytes, err := json.Marshal(in)
+		if err != nil {
+			t.Fatalf("failed to json.Marshal(in): %v", err)
+		}
+		outBytes, err := json.Marshal(out)
+		if err != nil {
+			t.Fatalf("failed to json.Marshal(out): %v", err)
+		}
+		inMap := map[string]interface{}{}
+		if err := json.Unmarshal(inBytes, &inMap); err != nil {
+			t.Fatalf("failed to json.Unmarshal(in): %v", err)
+		}
+		outMap := map[string]interface{}{}
+		if err := json.Unmarshal(outBytes, &outMap); err != nil {
+			t.Fatalf("failed to json.Unmarshal(out): %v", err)
+		}
+
+		// Patch the maps to handle any expected differences before we compare.
+
+		// DeleteOptions does not have these fields.
+		delete(inMap, "fieldManager")
+
+		// UpdateOptions does not have these fields.
+		delete(outMap, "gracePeriodSeconds")
+		delete(outMap, "preconditions")
+		delete(outMap, "orphanDependents")
+		delete(outMap, "propagationPolicy")
+
+		// Compare the results.
+		inBytes, err = json.Marshal(inMap)
+		if err != nil {
+			t.Fatalf("failed to json.Marshal(in): %v", err)
+		}
+		outBytes, err = json.Marshal(outMap)
+		if err != nil {
+			t.Fatalf("failed to json.Marshal(out): %v", err)
+		}
+		if i, o := string(inBytes), string(outBytes); i != o {
+			t.Fatalf("output != input:\n  want: %s\n   got: %s", i, o)
+		}
+	}
+}
+
+func TestStoreCreateHooks(t *testing.T) {
+	// To track which hooks were called in what order.  Not all hooks can
+	// mutate the object.
+	var milestones []string
+
+	setAnn := func(obj runtime.Object, key string) {
+		pod := obj.(*example.Pod)
+		if pod.Annotations == nil {
+			pod.Annotations = make(map[string]string)
+		}
+		pod.Annotations[key] = "true"
+	}
+	mile := func(s string) {
+		milestones = append(milestones, s)
+	}
+
+	testCases := []struct {
+		name        string
+		decorator   func(runtime.Object)
+		beginCreate BeginCreateFunc
+		afterCreate AfterCreateFunc
+		// the TTLFunc is an easy hook to force a failure
+		ttl              func(obj runtime.Object, existing uint64, update bool) (uint64, error)
+		expectError      bool
+		expectAnnotation string   // to test object mutations
+		expectMilestones []string // to test sequence
+	}{{
+		name: "no hooks",
+	}, {
+		name: "Decorator mutation",
+		decorator: func(obj runtime.Object) {
+			setAnn(obj, "DecoratorWasCalled")
+		},
+		expectAnnotation: "DecoratorWasCalled",
+	}, {
+		name: "AfterCreate mutation",
+		afterCreate: func(obj runtime.Object, opts *metav1.CreateOptions) {
+			setAnn(obj, "AfterCreateWasCalled")
+		},
+		expectAnnotation: "AfterCreateWasCalled",
+	}, {
+		name: "BeginCreate mutation",
+		beginCreate: func(_ context.Context, obj runtime.Object, _ *metav1.CreateOptions) (FinishFunc, error) {
+			setAnn(obj, "BeginCreateWasCalled")
+			return func(context.Context, bool) {}, nil
+		},
+		expectAnnotation: "BeginCreateWasCalled",
+	}, {
+		name: "success ordering",
+		decorator: func(obj runtime.Object) {
+			mile("Decorator")
+		},
+		afterCreate: func(obj runtime.Object, opts *metav1.CreateOptions) {
+			mile("AfterCreate")
+		},
+		beginCreate: func(_ context.Context, obj runtime.Object, _ *metav1.CreateOptions) (FinishFunc, error) {
+			mile("BeginCreate")
+			return func(_ context.Context, success bool) {
+				mile(fmt.Sprintf("FinishCreate(%v)", success))
+			}, nil
+		},
+		expectMilestones: []string{"BeginCreate", "FinishCreate(true)", "AfterCreate", "Decorator"},
+	}, {
+		name: "fail ordering",
+		decorator: func(obj runtime.Object) {
+			mile("Decorator")
+		},
+		afterCreate: func(obj runtime.Object, opts *metav1.CreateOptions) {
+			mile("AfterCreate")
+		},
+		beginCreate: func(_ context.Context, obj runtime.Object, _ *metav1.CreateOptions) (FinishFunc, error) {
+			mile("BeginCreate")
+			return func(_ context.Context, success bool) {
+				mile(fmt.Sprintf("FinishCreate(%v)", success))
+			}, nil
+		},
+		ttl: func(_ runtime.Object, existing uint64, _ bool) (uint64, error) {
+			mile("TTLError")
+			return existing, fmt.Errorf("TTL fail")
+		},
+		expectMilestones: []string{"BeginCreate", "TTLError", "FinishCreate(false)"},
+		expectError:      true,
+	}, {
+		name:        "fail BeginCreate ordering",
+		expectError: true,
+		decorator: func(obj runtime.Object) {
+			mile("Decorator")
+		},
+		afterCreate: func(obj runtime.Object, opts *metav1.CreateOptions) {
+			mile("AfterCreate")
+		},
+		beginCreate: func(_ context.Context, obj runtime.Object, _ *metav1.CreateOptions) (FinishFunc, error) {
+			mile("BeginCreate")
+			return func(_ context.Context, success bool) {
+				mile(fmt.Sprintf("FinishCreate(%v)", success))
+			}, fmt.Errorf("begin")
+		},
+		expectMilestones: []string{"BeginCreate"},
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			pod := &example.Pod{
+				ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "test"},
+				Spec:       example.PodSpec{NodeName: "machine"},
+			}
+
+			testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), "test")
+			destroyFunc, registry := NewTestGenericStoreRegistry(t)
+			defer destroyFunc()
+			registry.Decorator = tc.decorator
+			registry.BeginCreate = tc.beginCreate
+			registry.AfterCreate = tc.afterCreate
+			registry.TTLFunc = tc.ttl
+
+			// create the object
+			milestones = nil
+			obj, err := registry.Create(testContext, pod, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
+			if err != nil && !tc.expectError {
+				t.Fatalf("Unexpected error: %v", err)
+			}
+			if err == nil && tc.expectError {
+				t.Fatalf("Unexpected success")
+			}
+
+			// verify the results
+			if tc.expectAnnotation != "" {
+				out := obj.(*example.Pod)
+				if v, found := out.Annotations[tc.expectAnnotation]; !found {
+					t.Errorf("Expected annotation %q not found", tc.expectAnnotation)
+				} else if v != "true" {
+					t.Errorf("Expected annotation %q has wrong value: %q", tc.expectAnnotation, v)
+				}
+			}
+			if tc.expectMilestones != nil {
+				if !reflect.DeepEqual(milestones, tc.expectMilestones) {
+					t.Errorf("Unexpected milestones: wanted %v, got %v", tc.expectMilestones, milestones)
+				}
+			}
+		})
+	}
+}
+
 func isQualifiedResource(err error, kind, group string) bool {
 	if err.(errors.APIStatus).Status().Details.Kind != kind || err.(errors.APIStatus).Status().Details.Group != group {
 		return false
@@ -531,6 +778,390 @@ func TestNoOpUpdates(t *testing.T) {
 	}
 }
 
+func TestStoreUpdateHooks(t *testing.T) {
+	// To track which hooks were called in what order.  Not all hooks can
+	// mutate the object.
+	var milestones []string
+
+	setAnn := func(obj runtime.Object, key string) {
+		pod := obj.(*example.Pod)
+		if pod.Annotations == nil {
+			pod.Annotations = make(map[string]string)
+		}
+		pod.Annotations[key] = "true"
+	}
+	mile := func(s string) {
+		milestones = append(milestones, s)
+	}
+
+	testCases := []struct {
+		name      string
+		decorator func(runtime.Object)
+		// create-on-update is tested elsewhere, but this proves non-use here
+		beginCreate      BeginCreateFunc
+		afterCreate      AfterCreateFunc
+		beginUpdate      BeginUpdateFunc
+		afterUpdate      AfterUpdateFunc
+		expectError      bool
+		expectAnnotation string   // to test object mutations
+		expectMilestones []string // to test sequence
+	}{{
+		name: "no hooks",
+	}, {
+		name: "Decorator mutation",
+		decorator: func(obj runtime.Object) {
+			setAnn(obj, "DecoratorWasCalled")
+		},
+		expectAnnotation: "DecoratorWasCalled",
+	}, {
+		name: "AfterUpdate mutation",
+		afterUpdate: func(obj runtime.Object, opts *metav1.UpdateOptions) {
+			setAnn(obj, "AfterUpdateWasCalled")
+		},
+		expectAnnotation: "AfterUpdateWasCalled",
+	}, {
+		name: "BeginUpdate mutation",
+		beginUpdate: func(_ context.Context, obj, _ runtime.Object, _ *metav1.UpdateOptions) (FinishFunc, error) {
+			setAnn(obj, "BeginUpdateWasCalled")
+			return func(context.Context, bool) {}, nil
+		},
+		expectAnnotation: "BeginUpdateWasCalled",
+	}, {
+		name: "success ordering",
+		decorator: func(obj runtime.Object) {
+			mile("Decorator")
+		},
+		afterCreate: func(obj runtime.Object, opts *metav1.CreateOptions) {
+			mile("AfterCreate")
+		},
+		beginCreate: func(_ context.Context, obj runtime.Object, _ *metav1.CreateOptions) (FinishFunc, error) {
+			mile("BeginCreate")
+			return func(_ context.Context, success bool) {
+				mile(fmt.Sprintf("FinishCreate(%v)", success))
+			}, nil
+		},
+		afterUpdate: func(obj runtime.Object, opts *metav1.UpdateOptions) {
+			mile("AfterUpdate")
+		},
+		beginUpdate: func(_ context.Context, obj, _ runtime.Object, _ *metav1.UpdateOptions) (FinishFunc, error) {
+			mile("BeginUpdate")
+			return func(_ context.Context, success bool) {
+				mile(fmt.Sprintf("FinishUpdate(%v)", success))
+			}, nil
+		},
+		expectMilestones: []string{"BeginUpdate", "FinishUpdate(true)", "AfterUpdate", "Decorator"},
+	}, /* fail ordering is covered in TestStoreUpdateHooksInnerRetry */ {
+		name:        "fail BeginUpdate ordering",
+		expectError: true,
+		decorator: func(obj runtime.Object) {
+			mile("Decorator")
+		},
+		afterUpdate: func(obj runtime.Object, opts *metav1.UpdateOptions) {
+			mile("AfterUpdate")
+		},
+		beginUpdate: func(_ context.Context, obj, _ runtime.Object, _ *metav1.UpdateOptions) (FinishFunc, error) {
+			mile("BeginUpdate")
+			return func(_ context.Context, success bool) {
+				mile(fmt.Sprintf("FinishUpdate(%v)", success))
+			}, fmt.Errorf("begin")
+		},
+		expectMilestones: []string{"BeginUpdate"},
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			pod := &example.Pod{
+				ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "test"},
+				Spec:       example.PodSpec{NodeName: "machine"},
+			}
+
+			testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), "test")
+			destroyFunc, registry := NewTestGenericStoreRegistry(t)
+			defer destroyFunc()
+			registry.BeginUpdate = tc.beginUpdate
+			registry.AfterUpdate = tc.afterUpdate
+			registry.BeginCreate = tc.beginCreate
+			registry.AfterCreate = tc.afterCreate
+
+			_, err := registry.Create(testContext, pod, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
+			if err != nil {
+				t.Fatalf("Unexpected error: %v", err)
+			}
+			milestones = nil
+			registry.Decorator = tc.decorator
+			obj, _, err := registry.Update(testContext, pod.Name, rest.DefaultUpdatedObjectInfo(pod), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{})
+			if err != nil && !tc.expectError {
+				t.Fatalf("Unexpected error: %v", err)
+			}
+			if err == nil && tc.expectError {
+				t.Fatalf("Unexpected success")
+			}
+
+			// verify the results
+			if tc.expectAnnotation != "" {
+				out := obj.(*example.Pod)
+				if v, found := out.Annotations[tc.expectAnnotation]; !found {
+					t.Errorf("Expected annotation %q not found", tc.expectAnnotation)
+				} else if v != "true" {
+					t.Errorf("Expected annotation %q has wrong value: %q", tc.expectAnnotation, v)
+				}
+			}
+			if tc.expectMilestones != nil {
+				if !reflect.DeepEqual(milestones, tc.expectMilestones) {
+					t.Errorf("Unexpected milestones: wanted %v, got %v", tc.expectMilestones, milestones)
+				}
+			}
+		})
+	}
+}
+
+func TestStoreCreateOnUpdateHooks(t *testing.T) {
+	// To track which hooks were called in what order.  Not all hooks can
+	// mutate the object.
+	var milestones []string
+
+	mile := func(s string) {
+		milestones = append(milestones, s)
+	}
+
+	testCases := []struct {
+		name        string
+		decorator   func(runtime.Object)
+		beginCreate BeginCreateFunc
+		afterCreate AfterCreateFunc
+		beginUpdate BeginUpdateFunc
+		afterUpdate AfterUpdateFunc
+		// the TTLFunc is an easy hook to force a failure
+		ttl              func(obj runtime.Object, existing uint64, update bool) (uint64, error)
+		expectError      bool
+		expectMilestones []string // to test sequence
+	}{{
+		name: "no hooks",
+	}, {
+		name: "success ordering",
+		decorator: func(obj runtime.Object) {
+			mile("Decorator")
+		},
+		afterCreate: func(obj runtime.Object, opts *metav1.CreateOptions) {
+			mile("AfterCreate")
+		},
+		beginCreate: func(_ context.Context, obj runtime.Object, _ *metav1.CreateOptions) (FinishFunc, error) {
+			mile("BeginCreate")
+			return func(_ context.Context, success bool) {
+				mile(fmt.Sprintf("FinishCreate(%v)", success))
+			}, nil
+		},
+		afterUpdate: func(obj runtime.Object, opts *metav1.UpdateOptions) {
+			mile("AfterUpdate")
+		},
+		beginUpdate: func(_ context.Context, obj, _ runtime.Object, _ *metav1.UpdateOptions) (FinishFunc, error) {
+			mile("BeginUpdate")
+			return func(_ context.Context, success bool) {
+				mile(fmt.Sprintf("FinishUpdate(%v)", success))
+			}, nil
+		},
+		expectMilestones: []string{"BeginCreate", "FinishCreate(true)", "AfterCreate", "Decorator"},
+	}, {
+		name: "fail ordering",
+		decorator: func(obj runtime.Object) {
+			mile("Decorator")
+		},
+		afterCreate: func(obj runtime.Object, opts *metav1.CreateOptions) {
+			mile("AfterCreate")
+		},
+		beginCreate: func(_ context.Context, obj runtime.Object, _ *metav1.CreateOptions) (FinishFunc, error) {
+			mile("BeginCreate")
+			return func(_ context.Context, success bool) {
+				mile(fmt.Sprintf("FinishCreate(%v)", success))
+			}, nil
+		},
+		afterUpdate: func(obj runtime.Object, opts *metav1.UpdateOptions) {
+			mile("AfterUpdate")
+		},
+		beginUpdate: func(_ context.Context, obj, _ runtime.Object, _ *metav1.UpdateOptions) (FinishFunc, error) {
+			mile("BeginUpdate")
+			return func(_ context.Context, success bool) {
+				mile(fmt.Sprintf("FinishUpdate(%v)", success))
+			}, nil
+		},
+		ttl: func(_ runtime.Object, existing uint64, _ bool) (uint64, error) {
+			mile("TTLError")
+			return existing, fmt.Errorf("TTL fail")
+		},
+		expectMilestones: []string{"BeginCreate", "TTLError", "FinishCreate(false)"},
+		expectError:      true,
+	}, {
+		name:        "fail BeginCreate ordering",
+		expectError: true,
+		decorator: func(obj runtime.Object) {
+			mile("Decorator")
+		},
+		afterCreate: func(obj runtime.Object, opts *metav1.CreateOptions) {
+			mile("AfterCreate")
+		},
+		beginCreate: func(_ context.Context, obj runtime.Object, _ *metav1.CreateOptions) (FinishFunc, error) {
+			mile("BeginCreate")
+			return func(_ context.Context, success bool) {
+				mile(fmt.Sprintf("FinishCreate(%v)", success))
+			}, fmt.Errorf("begin")
+		},
+		afterUpdate: func(obj runtime.Object, opts *metav1.UpdateOptions) {
+			mile("AfterUpdate")
+		},
+		beginUpdate: func(_ context.Context, obj, _ runtime.Object, _ *metav1.UpdateOptions) (FinishFunc, error) {
+			mile("BeginUpdate")
+			return func(_ context.Context, success bool) {
+				mile(fmt.Sprintf("FinishUpdate(%v)", success))
+			}, nil
+		},
+		expectMilestones: []string{"BeginCreate"},
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			pod := &example.Pod{
+				ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "test"},
+				Spec:       example.PodSpec{NodeName: "machine"},
+			}
+
+			testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), "test")
+			destroyFunc, registry := NewTestGenericStoreRegistry(t)
+			defer destroyFunc()
+			registry.Decorator = tc.decorator
+			registry.UpdateStrategy.(*testRESTStrategy).allowCreateOnUpdate = true
+			registry.BeginUpdate = tc.beginUpdate
+			registry.AfterUpdate = tc.afterUpdate
+			registry.BeginCreate = tc.beginCreate
+			registry.AfterCreate = tc.afterCreate
+			registry.TTLFunc = tc.ttl
+
+			// NB: did not create it first.
+			milestones = nil
+			_, _, err := registry.Update(testContext, pod.Name, rest.DefaultUpdatedObjectInfo(pod), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{})
+			if err != nil && !tc.expectError {
+				t.Fatalf("Unexpected error: %v", err)
+			}
+			if err == nil && tc.expectError {
+				t.Fatalf("Unexpected success")
+			}
+
+			// verify the results
+			if tc.expectMilestones != nil {
+				if !reflect.DeepEqual(milestones, tc.expectMilestones) {
+					t.Errorf("Unexpected milestones: wanted %v, got %v", tc.expectMilestones, milestones)
+				}
+			}
+		})
+	}
+}
+
+func TestStoreUpdateHooksInnerRetry(t *testing.T) {
+	// To track which hooks were called in what order.  Not all hooks can
+	// mutate the object.
+	var milestones []string
+
+	mile := func(s string) {
+		milestones = append(milestones, s)
+	}
+	ttlFailDone := false
+	ttlFailOnce := func(_ runtime.Object, existing uint64, _ bool) (uint64, error) {
+		if ttlFailDone {
+			mile("TTL")
+			return existing, nil
+		}
+		ttlFailDone = true
+		mile("TTLError")
+		return existing, fmt.Errorf("TTL fail")
+	}
+	ttlFailAlways := func(_ runtime.Object, existing uint64, _ bool) (uint64, error) {
+		mile("TTLError")
+		return existing, fmt.Errorf("TTL fail")
+	}
+
+	testCases := []struct {
+		name        string
+		decorator   func(runtime.Object)
+		beginUpdate func(context.Context, runtime.Object, runtime.Object, *metav1.UpdateOptions) (FinishFunc, error)
+		afterUpdate AfterUpdateFunc
+		// the TTLFunc is an easy hook to force an inner-loop retry
+		ttl              func(obj runtime.Object, existing uint64, update bool) (uint64, error)
+		expectError      bool
+		expectMilestones []string // to test sequence
+	}{{
+		name: "inner retry success",
+		decorator: func(obj runtime.Object) {
+			mile("Decorator")
+		},
+		afterUpdate: func(obj runtime.Object, opts *metav1.UpdateOptions) {
+			mile("AfterUpdate")
+		},
+		beginUpdate: func(_ context.Context, obj, _ runtime.Object, _ *metav1.UpdateOptions) (FinishFunc, error) {
+			mile("BeginUpdate")
+			return func(_ context.Context, success bool) {
+				mile(fmt.Sprintf("FinishUpdate(%v)", success))
+			}, nil
+		},
+		ttl:              ttlFailOnce,
+		expectMilestones: []string{"BeginUpdate", "TTLError", "FinishUpdate(false)", "BeginUpdate", "TTL", "FinishUpdate(true)", "AfterUpdate", "Decorator"},
+	}, {
+		name: "inner retry fail",
+		decorator: func(obj runtime.Object) {
+			mile("Decorator")
+		},
+		afterUpdate: func(obj runtime.Object, opts *metav1.UpdateOptions) {
+			mile("AfterUpdate")
+		},
+		beginUpdate: func(_ context.Context, obj, _ runtime.Object, _ *metav1.UpdateOptions) (FinishFunc, error) {
+			mile("BeginUpdate")
+			return func(_ context.Context, success bool) {
+				mile(fmt.Sprintf("FinishUpdate(%v)", success))
+			}, nil
+		},
+		ttl:              ttlFailAlways,
+		expectError:      true,
+		expectMilestones: []string{"BeginUpdate", "TTLError", "FinishUpdate(false)", "BeginUpdate", "TTLError", "FinishUpdate(false)"},
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			pod := &example.Pod{
+				ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "test"},
+				Spec:       example.PodSpec{NodeName: "machine"},
+			}
+
+			testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), "test")
+			destroyFunc, registry := NewTestGenericStoreRegistry(t)
+			defer destroyFunc()
+			registry.BeginUpdate = tc.beginUpdate
+			registry.AfterUpdate = tc.afterUpdate
+
+			created, err := registry.Create(testContext, pod, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
+			if err != nil {
+				t.Fatalf("Unexpected error: %v", err)
+			}
+			milestones = nil
+			registry.Decorator = tc.decorator
+			ttlFailDone = false
+			registry.TTLFunc = tc.ttl
+			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 {
+				t.Fatalf("Unexpected error: %v", err)
+			}
+			if err == nil && tc.expectError {
+				t.Fatalf("Unexpected success")
+			}
+
+			// verify the results
+			if tc.expectMilestones != nil {
+				if !reflect.DeepEqual(milestones, tc.expectMilestones) {
+					t.Errorf("Unexpected milestones: wanted %v, got %v", tc.expectMilestones, milestones)
+				}
+			}
+		})
+	}
+}
+
 // TODO: Add a test to check no-op update if we have object with ResourceVersion
 // already stored in etcd. Currently there is no easy way to store object with
 // ResourceVersion in etcd.
@@ -660,11 +1291,19 @@ func TestStoreDelete(t *testing.T) {
 	destroyFunc, registry := NewTestGenericStoreRegistry(t)
 	defer destroyFunc()
 
+	afterWasCalled := false
+	registry.AfterDelete = func(obj runtime.Object, options *metav1.DeleteOptions) {
+		afterWasCalled = true
+	}
+
 	// test failure condition
 	_, _, err := registry.Delete(testContext, podA.Name, rest.ValidateAllObjectFunc, nil)
 	if !errors.IsNotFound(err) {
 		t.Errorf("Unexpected error: %v", err)
 	}
+	if afterWasCalled {
+		t.Errorf("Unexpected call to AfterDelete")
+	}
 
 	// create pod
 	_, err = registry.Create(testContext, podA, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
@@ -680,6 +1319,9 @@ func TestStoreDelete(t *testing.T) {
 	if !wasDeleted {
 		t.Errorf("unexpected, pod %s should have been deleted immediately", podA.Name)
 	}
+	if !afterWasCalled {
+		t.Errorf("Expected call to AfterDelete, but got none")
+	}
 
 	// try to get a item which should be deleted
 	_, err = registry.Get(testContext, podA.Name, &metav1.GetOptions{})
@@ -795,11 +1437,18 @@ func TestGracefulStoreHandleFinalizers(t *testing.T) {
 	registry.DeleteStrategy = testGracefulStrategy{defaultDeleteStrategy}
 	defer destroyFunc()
 
+	afterWasCalled := false
+	registry.AfterDelete = func(obj runtime.Object, options *metav1.DeleteOptions) {
+		afterWasCalled = true
+	}
+
 	gcStates := []bool{true, false}
 	for _, gcEnabled := range gcStates {
 		t.Logf("garbage collection enabled: %t", gcEnabled)
 		registry.EnableGarbageCollection = gcEnabled
 
+		afterWasCalled = false // reset
+
 		// create pod
 		_, err := registry.Create(testContext, podWithFinalizer, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
 		if err != nil {
@@ -814,6 +1463,9 @@ func TestGracefulStoreHandleFinalizers(t *testing.T) {
 		if wasDeleted {
 			t.Errorf("unexpected, pod %s should not have been deleted immediately", podWithFinalizer.Name)
 		}
+		if afterWasCalled {
+			t.Errorf("unexpected, AfterDelete() was called")
+		}
 		_, err = registry.Get(testContext, podWithFinalizer.Name, &metav1.GetOptions{})
 		if err != nil {
 			t.Fatalf("Unexpected error: %v", err)
@@ -827,6 +1479,9 @@ func TestGracefulStoreHandleFinalizers(t *testing.T) {
 		if err != nil {
 			t.Fatalf("Unexpected error: %v", err)
 		}
+		if afterWasCalled {
+			t.Errorf("unexpected, AfterDelete() was called")
+		}
 
 		// the object should still exist, because it still has a finalizer
 		_, err = registry.Get(testContext, podWithFinalizer.Name, &metav1.GetOptions{})
@@ -842,6 +1497,9 @@ func TestGracefulStoreHandleFinalizers(t *testing.T) {
 		if err != nil {
 			t.Fatalf("Unexpected error: %v", err)
 		}
+		if !afterWasCalled {
+			t.Errorf("unexpected, AfterDelete() was not called")
+		}
 		// the pod should be removed, because its finalizer is removed
 		_, err = registry.Get(testContext, podWithFinalizer.Name, &metav1.GetOptions{})
 		if !errors.IsNotFound(err) {
@@ -859,13 +1517,20 @@ func TestNonGracefulStoreHandleFinalizers(t *testing.T) {
 
 	testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), "test")
 	destroyFunc, registry := NewTestGenericStoreRegistry(t)
-
 	defer destroyFunc()
+
+	afterWasCalled := false
+	registry.AfterDelete = func(obj runtime.Object, options *metav1.DeleteOptions) {
+		afterWasCalled = true
+	}
+
 	gcStates := []bool{true, false}
 	for _, gcEnabled := range gcStates {
 		t.Logf("garbage collection enabled: %t", gcEnabled)
 		registry.EnableGarbageCollection = gcEnabled
 
+		afterWasCalled = false // reset
+
 		// create pod
 		_, err := registry.Create(testContext, podWithFinalizer, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
 		if err != nil {
@@ -880,6 +1545,9 @@ func TestNonGracefulStoreHandleFinalizers(t *testing.T) {
 		if wasDeleted {
 			t.Errorf("unexpected, pod %s should not have been deleted immediately", podWithFinalizer.Name)
 		}
+		if afterWasCalled {
+			t.Errorf("unexpected, AfterDelete() was called")
+		}
 
 		// the object should still exist
 		obj, err := registry.Get(testContext, podWithFinalizer.Name, &metav1.GetOptions{})
@@ -908,6 +1576,9 @@ func TestNonGracefulStoreHandleFinalizers(t *testing.T) {
 		if err != nil {
 			t.Errorf("Unexpected error: %v", err)
 		}
+		if afterWasCalled {
+			t.Errorf("unexpected, AfterDelete() was called")
+		}
 
 		// the object should still exist, because it still has a finalizer
 		obj, err = registry.Get(testContext, podWithFinalizer.Name, &metav1.GetOptions{})
@@ -927,6 +1598,9 @@ func TestNonGracefulStoreHandleFinalizers(t *testing.T) {
 		if err != nil {
 			t.Errorf("Unexpected error: %v", err)
 		}
+		if !afterWasCalled {
+			t.Errorf("unexpected, AfterDelete() was not called")
+		}
 		// the pod should be removed, because its finalizer is removed
 		_, err = registry.Get(testContext, podWithFinalizer.Name, &metav1.GetOptions{})
 		if !errors.IsNotFound(err) {
@@ -1677,7 +2351,7 @@ func TestFinalizeDelete(t *testing.T) {
 	obj := &example.Pod{
 		ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "random-uid"},
 	}
-	result, err := s.finalizeDelete(genericapirequest.NewContext(), obj, false)
+	result, err := s.finalizeDelete(genericapirequest.NewContext(), obj, false, &metav1.DeleteOptions{})
 	if err != nil {
 		t.Fatalf("unexpected err: %s", err)
 	}

staging/src/k8s.io/apiserver/pkg/registry/rest/rest.go

@@ -214,7 +214,7 @@ type UpdatedObjectInfo interface {
 }
 
 // ValidateObjectFunc is a function to act on a given object. An error may be returned
-// if the hook cannot be completed. An ObjectFunc may NOT transform the provided
+// if the hook cannot be completed. A ValidateObjectFunc may NOT transform the provided
 // object.
 type ValidateObjectFunc func(ctx context.Context, obj runtime.Object) error