Merge pull request #50019 from deads2k/server-38-storage

Automatic merge from submit-queue (batch tested with PRs 48237, 50084, 50019, 50069, 50090)

create default storage selection functions

The vast majority of our REST storage needs either default selection predicates or minor tweaks the the field selection criteria.  This pull makes that work without any special fields set, but also allows the more complex configuration where it is required.  I only changed one storage an example.  Once this merges, I'll update the others.

@kubernetes/sig-api-machinery-misc @wojtek-t @smarterclayton @sttts @cheftako ptal.

pkg/registry/batch/cronjob/BUILD

@@ -19,14 +19,10 @@ go_library(
         "//pkg/api:go_default_library",
         "//pkg/apis/batch:go_default_library",
         "//pkg/apis/batch/validation:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/fields:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/labels:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library",
-        "//vendor/k8s.io/apiserver/pkg/registry/generic:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/registry/rest:go_default_library",
-        "//vendor/k8s.io/apiserver/pkg/storage:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/storage/names:go_default_library",
     ],
 )
@@ -38,7 +34,6 @@ go_test(
     tags = ["automanaged"],
     deps = [
         "//pkg/api:go_default_library",
-        "//pkg/api/testing:go_default_library",
         "//pkg/apis/batch:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library",

pkg/registry/batch/cronjob/storage/storage.go

@@ -40,7 +40,6 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) {
 		Copier:            api.Scheme,
 		NewFunc:           func() runtime.Object { return &batch.CronJob{} },
 		NewListFunc:       func() runtime.Object { return &batch.CronJobList{} },
-		PredicateFunc:     cronjob.MatchCronJob,
 		QualifiedResource: batch.Resource("cronjobs"),
 		WatchCacheSize:    cachesize.GetWatchCacheSizeByResource("cronjobs"),
 
@@ -48,7 +47,7 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) {
 		UpdateStrategy: cronjob.Strategy,
 		DeleteStrategy: cronjob.Strategy,
 	}
-	options := &generic.StoreOptions{RESTOptions: optsGetter, AttrFunc: cronjob.GetAttrs}
+	options := &generic.StoreOptions{RESTOptions: optsGetter}
 	if err := store.CompleteWithOptions(options); err != nil {
 		panic(err) // TODO: Propagate error up
 	}

pkg/registry/batch/cronjob/strategy.go

@@ -17,16 +17,10 @@ limitations under the License.
 package cronjob
 
 import (
-	"fmt"
-
-	"k8s.io/apimachinery/pkg/fields"
-	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
-	"k8s.io/apiserver/pkg/registry/generic"
 	"k8s.io/apiserver/pkg/registry/rest"
-	"k8s.io/apiserver/pkg/storage"
 	"k8s.io/apiserver/pkg/storage/names"
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/apis/batch"
@@ -105,28 +99,3 @@ func (cronJobStatusStrategy) PrepareForUpdate(ctx genericapirequest.Context, obj
 func (cronJobStatusStrategy) ValidateUpdate(ctx genericapirequest.Context, obj, old runtime.Object) field.ErrorList {
 	return field.ErrorList{}
 }
-
-// CronJobToSelectableFields returns a field set that represents the object for matching purposes.
-func CronJobToSelectableFields(cronJob *batch.CronJob) fields.Set {
-	return generic.ObjectMetaFieldsSet(&cronJob.ObjectMeta, true)
-}
-
-// GetAttrs returns labels and fields of a given object for filtering purposes.
-func GetAttrs(obj runtime.Object) (labels.Set, fields.Set, bool, error) {
-	cronJob, ok := obj.(*batch.CronJob)
-	if !ok {
-		return nil, nil, false, fmt.Errorf("given object is not a scheduled job.")
-	}
-	return labels.Set(cronJob.ObjectMeta.Labels), CronJobToSelectableFields(cronJob), cronJob.Initializers != nil, nil
-}
-
-// MatchCronJob is the filter used by the generic etcd backend to route
-// watch events from etcd to clients of the apiserver only interested in specific
-// labels/fields.
-func MatchCronJob(label labels.Selector, field fields.Selector) storage.SelectionPredicate {
-	return storage.SelectionPredicate{
-		Label:    label,
-		Field:    field,
-		GetAttrs: GetAttrs,
-	}
-}

pkg/registry/batch/cronjob/strategy_test.go

@@ -23,7 +23,6 @@ import (
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/registry/rest"
 	"k8s.io/kubernetes/pkg/api"
-	apitesting "k8s.io/kubernetes/pkg/api/testing"
 	"k8s.io/kubernetes/pkg/apis/batch"
 )
 
@@ -170,13 +169,3 @@ func TestCronJobStatusStrategy(t *testing.T) {
 		t.Errorf("Incoming resource version on update should not be mutated")
 	}
 }
-
-// FIXME: this is failing conversion.go
-func TestSelectableFieldLabelConversions(t *testing.T) {
-	apitesting.TestSelectableFieldLabelConversionsOfKind(t,
-		"batch/v2alpha1",
-		"CronJob",
-		CronJobToSelectableFields(&batch.CronJob{}),
-		nil,
-	)
-}

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

@@ -1267,8 +1267,23 @@ func (e *Store) CompleteWithOptions(options *generic.StoreOptions) error {
 	if options.RESTOptions == nil {
 		return fmt.Errorf("options for %s must have RESTOptions set", e.QualifiedResource.String())
 	}
-	if options.AttrFunc == nil {
+
-		return fmt.Errorf("options for %s must have AttrFunc set", e.QualifiedResource.String())
+	attrFunc := options.AttrFunc
+	if attrFunc == nil {
+		if isNamespaced {
+			attrFunc = storage.DefaultNamespaceScopedAttr
+		} else {
+			attrFunc = storage.DefaultClusterScopedAttr
+		}
+	}
+	if e.PredicateFunc == nil {
+		e.PredicateFunc = func(label labels.Selector, field fields.Selector) storage.SelectionPredicate {
+			return storage.SelectionPredicate{
+				Label:    label,
+				Field:    field,
+				GetAttrs: attrFunc,
+			}
+		}
 	}
 
 	opts, err := options.RESTOptions.GetRESTOptions(e.QualifiedResource)
@@ -1349,7 +1364,7 @@ func (e *Store) CompleteWithOptions(options *generic.StoreOptions) error {
 			prefix,
 			keyFunc,
 			e.NewListFunc,
-			options.AttrFunc,
+			attrFunc,
 			triggerFunc,
 		)
 	}

staging/src/k8s.io/apiserver/pkg/storage/selection_predicate.go

@@ -17,6 +17,7 @@ limitations under the License.
 package storage
 
 import (
+	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -26,6 +27,48 @@ import (
 // In any failure to parse given object, it returns error.
 type AttrFunc func(obj runtime.Object) (labels.Set, fields.Set, bool, error)
 
+// FieldMutationFunc allows the mutation of the field selection fields.  It is mutating to
+// avoid the extra allocation on this common path
+type FieldMutationFunc func(obj runtime.Object, fieldSet fields.Set) error
+
+func DefaultClusterScopedAttr(obj runtime.Object) (labels.Set, fields.Set, bool, error) {
+	metadata, err := meta.Accessor(obj)
+	if err != nil {
+		return nil, nil, false, err
+	}
+	fieldSet := fields.Set{
+		"metadata.name": metadata.GetName(),
+	}
+
+	return labels.Set(metadata.GetLabels()), fieldSet, metadata.GetInitializers() != nil, nil
+}
+
+func DefaultNamespaceScopedAttr(obj runtime.Object) (labels.Set, fields.Set, bool, error) {
+	metadata, err := meta.Accessor(obj)
+	if err != nil {
+		return nil, nil, false, err
+	}
+	fieldSet := fields.Set{
+		"metadata.name":      metadata.GetName(),
+		"metadata.namespace": metadata.GetNamespace(),
+	}
+
+	return labels.Set(metadata.GetLabels()), fieldSet, metadata.GetInitializers() != nil, nil
+}
+
+func (f AttrFunc) WithFieldMutation(fieldMutator FieldMutationFunc) AttrFunc {
+	return func(obj runtime.Object) (labels.Set, fields.Set, bool, error) {
+		labelSet, fieldSet, initialized, err := f(obj)
+		if err != nil {
+			return nil, nil, false, err
+		}
+		if err := fieldMutator(obj, fieldSet); err != nil {
+			return nil, nil, false, err
+		}
+		return labelSet, fieldSet, initialized, nil
+	}
+}
+
 // SelectionPredicate is used to represent the way to select objects from api storage.
 type SelectionPredicate struct {
 	Label                labels.Selector