Merge pull request #43 from kubernetes-sigs/refactor/status-subresource

Refactor: Homogenizing status subresource storage support

pkg/builder/builder_resource.go

@@ -126,15 +126,7 @@ func (a *Server) WithResourceAndHandler(obj resource.Object, sp rest.ResourceHan
 func (a *Server) WithResourceAndStorage(obj resource.Object, fn rest.StoreFn) *Server {
 	gvr := obj.GetGroupVersionResource()
 	a.schemeBuilder.Register(resource.AddToScheme(obj))
-
-	_ = a.forGroupVersionResource(gvr, rest.NewWithFn(obj, fn))
-
-	// automatically create status subresource if the object implements the status interface
-	if _, ok := obj.(resource.ObjectWithStatusSubResource); ok {
-		st := gvr.GroupVersion().WithResource(gvr.Resource + "/status")
-		_ = a.forGroupVersionResource(st, rest.NewStatusWithFn(obj, fn))
-	}
-	return a
+	return a.forGroupVersionResource(gvr, rest.NewWithFn(obj, fn))
 }
 
 // forGroupVersionResource manually registers storage for a specific resource.
@@ -188,10 +180,9 @@ func (a *Server) withGroupVersions(versions ...schema.GroupVersion) *Server {
 func (a *Server) withSubResourceIfExists(obj resource.Object, parentStorageProvider rest.ResourceHandlerProvider) {
 	parentGVR := obj.GetGroupVersionResource()
 	// automatically create status subresource if the object implements the status interface
-	if sgs, ok := obj.(resource.ObjectWithStatusSubResource); ok {
+	if _, ok := obj.(resource.ObjectWithStatusSubResource); ok {
 		statusGVR := parentGVR.GroupVersion().WithResource(parentGVR.Resource + "/status")
-		_, _, _, sp := rest.NewStatus(sgs)
-		a.forGroupVersionSubResource(statusGVR, parentStorageProvider, sp)
+		a.forGroupVersionSubResource(statusGVR, parentStorageProvider, nil)
 	}
 	if _, ok := obj.(resource.ObjectWithScaleSubResource); ok {
 		subResourceGVR := parentGVR.GroupVersion().WithResource(parentGVR.Resource + "/scale")

pkg/builder/rest/rest.go

@@ -43,23 +43,6 @@ func New(obj resource.Object) ResourceHandlerProvider {
 	}
 }
 
-// NewStatus returns a new etcd backed request handler for the resource "status" subresource.
-func NewStatus(obj resource.ObjectWithStatusSubResource) (
-	parent resource.Object,
-	path string,
-	request resource.Object,
-	handler ResourceHandlerProvider) {
-	return obj, "status", obj, func(scheme *runtime.Scheme, optsGetter generic.RESTOptionsGetter) (rest.Storage, error) {
-		gvr := obj.GetGroupVersionResource()
-		s := &StatusSubResourceStrategy{Strategy: &DefaultStrategy{
-			Object:         obj,
-			ObjectTyper:    scheme,
-			TableConvertor: rest.NewDefaultTableConvertor(gvr.GroupResource()),
-		}}
-		return newStore(obj.New, obj.NewList, gvr, s, optsGetter, nil)
-	}
-}
-
 // NewWithStrategy returns a new etcd backed request handler using the provided Strategy.
 func NewWithStrategy(obj resource.Object, s Strategy) ResourceHandlerProvider {
 	return func(scheme *runtime.Scheme, optsGetter generic.RESTOptionsGetter) (rest.Storage, error) {
@@ -68,15 +51,6 @@ func NewWithStrategy(obj resource.Object, s Strategy) ResourceHandlerProvider {
 	}
 }
 
-// NewStatusWithStrategy returns a new etcd backed request handler using the provided Strategy for the "status" subresource.
-func NewStatusWithStrategy(obj resource.Object, s Strategy) ResourceHandlerProvider {
-	return func(scheme *runtime.Scheme, optsGetter generic.RESTOptionsGetter) (rest.Storage, error) {
-		gvr := obj.GetGroupVersionResource()
-		s = StatusSubResourceStrategy{Strategy: s}
-		return newStore(obj.New, obj.NewList, gvr, s, optsGetter, nil)
-	}
-}
-
 // StoreFn defines a function which modifies the Store before it is initialized.
 type StoreFn func(*genericregistry.Store, *generic.StoreOptions)
 
@@ -93,20 +67,6 @@ func NewWithFn(obj resource.Object, fn StoreFn) ResourceHandlerProvider {
 	}
 }
 
-// NewStatusWithFn returns a new etcd backed request handler for the "status" subresource, applying the
-// StoreFn to the Store.
-func NewStatusWithFn(obj resource.Object, fn StoreFn) ResourceHandlerProvider {
-	return func(scheme *runtime.Scheme, optsGetter generic.RESTOptionsGetter) (rest.Storage, error) {
-		gvr := obj.GetGroupVersionResource()
-		s := &DefaultStrategy{
-			Object:         obj,
-			ObjectTyper:    scheme,
-			TableConvertor: rest.NewDefaultTableConvertor(gvr.GroupResource()),
-		}
-		return newStore(obj.New, obj.NewList, gvr, &StatusSubResourceStrategy{Strategy: s}, optsGetter, fn)
-	}
-}
-
 // newStore returns a RESTStorage object that will work against API services.
 func newStore(
 	single, list func() runtime.Object, gvr schema.GroupVersionResource,

pkg/builder/storage_provider.go

@@ -12,10 +12,12 @@ import (
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apiserver/pkg/registry/generic"
+	"k8s.io/apiserver/pkg/registry/generic/registry"
 	registryrest "k8s.io/apiserver/pkg/registry/rest"
 	"sigs.k8s.io/apiserver-runtime/pkg/builder/resource"
-	"sigs.k8s.io/apiserver-runtime/pkg/builder/resource/resourcerest"
+	"sigs.k8s.io/apiserver-runtime/pkg/builder/resource/util"
 	"sigs.k8s.io/apiserver-runtime/pkg/builder/rest"
 	contextutil "sigs.k8s.io/apiserver-runtime/pkg/util/context"
 )
@@ -63,12 +65,7 @@ func (s *subResourceStorageProvider) Get(scheme *runtime.Scheme, optsGetter gene
 
 	// status subresource
 	if strings.HasSuffix(s.subResourceGVR.Resource, "/status") {
-		return &commonSubResourceStorage{
-			parentStorage:          stdParentStorage,
-			subResourceConstructor: subResourceStorage,
-			subResourceGetter:      subResourceStorage.(resourcerest.Getter),
-			subResourceUpdater:     subResourceStorage.(resourcerest.Updater),
-		}, nil
+		return createStatusSubResourceStorage(stdParentStorage)
 	}
 	// scale subresource
 	if strings.HasSuffix(s.subResourceGVR.Resource, "/scale") {
@@ -100,6 +97,63 @@ func (s *subResourceStorageProvider) Get(scheme *runtime.Scheme, optsGetter gene
 	return s.subResourceStorageProvider(scheme, optsGetter)
 }
 
+func createStatusSubResourceStorage(parentStorage registryrest.StandardStorage) (registryrest.Storage, error) {
+	parentStore, ok := parentStorage.(*registry.Store)
+	if !ok {
+		return nil, fmt.Errorf("parent type implementing ObjectWithStatusSubResource must be a cananical resource")
+	}
+	statusStore := *parentStore
+	statusStore.UpdateStrategy = &statusSubResourceStrategy{RESTUpdateStrategy: parentStore.UpdateStrategy}
+	return &statusSubResourceStorage{
+		store: &statusStore,
+	}, nil
+}
+
+// status subresource storage
+type statusSubResourceStorage struct {
+	store *registry.Store
+}
+
+var _ registryrest.Getter = &statusSubResourceStorage{}
+var _ registryrest.Updater = &statusSubResourceStorage{}
+
+func (s *statusSubResourceStorage) Get(ctx context.Context, name string, options *v1.GetOptions) (runtime.Object, error) {
+	return s.store.Get(ctx, name, options)
+}
+
+func (s *statusSubResourceStorage) New() runtime.Object {
+	return s.store.New()
+}
+
+func (s *statusSubResourceStorage) Update(ctx context.Context,
+	name string,
+	objInfo registryrest.UpdatedObjectInfo,
+	createValidation registryrest.ValidateObjectFunc,
+	updateValidation registryrest.ValidateObjectUpdateFunc,
+	forceAllowCreate bool,
+	options *v1.UpdateOptions) (runtime.Object, bool, error) {
+	return s.store.Update(ctx, name, objInfo, createValidation, updateValidation, forceAllowCreate, options)
+}
+
+var _ registryrest.RESTUpdateStrategy = &statusSubResourceStrategy{}
+
+// StatusSubResourceStrategy defines a default Strategy for the status subresource.
+type statusSubResourceStrategy struct {
+	registryrest.RESTUpdateStrategy
+}
+
+// PrepareForUpdate calls the PrepareForUpdate function on obj if supported, otherwise does nothing.
+func (s *statusSubResourceStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
+	// should panic/fail-fast upon casting failure
+	statusObj := obj.(resource.ObjectWithStatusSubResource)
+	statusOld := old.(resource.ObjectWithStatusSubResource)
+	// only modifies status
+	statusObj.GetStatus().CopyTo(statusOld)
+	if err := util.DeepCopy(statusOld, statusObj); err != nil {
+		utilruntime.HandleError(err)
+	}
+}
+
 // common subresource storage
 type commonSubResourceStorage struct {
 	parentStorage          registryrest.StandardStorage
@@ -208,14 +262,18 @@ func (s *scaleSubResourceStorage) Update(ctx context.Context,
 	updateValidation registryrest.ValidateObjectUpdateFunc,
 	forceAllowCreate bool,
 	options *v1.UpdateOptions) (runtime.Object, bool, error) {
-	return s.parentStorage.Update(
+	updatedObj, updated, err := s.parentStorage.Update(
 		contextutil.WithParentStorage(ctx, s.parentStorage),
 		name,
 		&scaleUpdatedObjectInfo{reqObjInfo: objInfo},
 		toScaleCreateValidation(createValidation),
 		toScaleUpdateValidation(updateValidation),
 		forceAllowCreate,
 		options)
+	if err != nil {
+		return nil, false, err
+	}
+	return updatedObj.(resource.ObjectWithScaleSubResource).GetScale(), updated, nil
 }
 
 var _ registryrest.UpdatedObjectInfo = &scaleUpdatedObjectInfo{}