From 5527ae736cd78fbd8c5d3888eb31f592b574c0b7 Mon Sep 17 00:00:00 2001 From: Iceber Gu Date: Thu, 14 Jul 2022 15:14:26 +0800 Subject: [PATCH] the collection resource support `withRemainingCount` and `withContinue` Signed-off-by: Iceber Gu --- .../clusterpedia/collectionresources/rest.go | 10 +++-- .../collectionresource_storage.go | 42 ++++++++++++------- .../internalstorage/resource_storage.go | 11 +---- pkg/storage/internalstorage/util.go | 14 +++++-- pkg/storage/internalstorage/util_test.go | 4 +- .../clusterpedia-io/api/clusterpedia/types.go | 3 ++ .../api/clusterpedia/v1beta1/types.go | 6 +++ .../v1beta1/zz_generated.conversion.go | 4 ++ .../v1beta1/zz_generated.deepcopy.go | 5 +++ .../api/clusterpedia/zz_generated.deepcopy.go | 5 +++ 10 files changed, 70 insertions(+), 34 deletions(-) diff --git a/pkg/apiserver/registry/clusterpedia/collectionresources/rest.go b/pkg/apiserver/registry/clusterpedia/collectionresources/rest.go index b4f513258..7abeb47e1 100644 --- a/pkg/apiserver/registry/clusterpedia/collectionresources/rest.go +++ b/pkg/apiserver/registry/clusterpedia/collectionresources/rest.go @@ -12,7 +12,9 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/duration" + genericfeatures "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/registry/rest" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog/v2" internal "github.com/clusterpedia-io/api/clusterpedia" @@ -104,9 +106,11 @@ func (s *REST) Get(ctx context.Context, name string, _ *metav1.GetOptions) (runt } } - // collection resources don't support with remaining count - // ignore opts.WithRemainingCount - opts.WithRemainingCount = nil + if opts.WithRemainingCount == nil { + if enabled := utilfeature.DefaultFeatureGate.Enabled(genericfeatures.RemainingItemCount); enabled { + opts.WithRemainingCount = &enabled + } + } storage, ok := s.storages[name] if !ok { diff --git a/pkg/storage/internalstorage/collectionresource_storage.go b/pkg/storage/internalstorage/collectionresource_storage.go index bcf464a70..e614cffc7 100644 --- a/pkg/storage/internalstorage/collectionresource_storage.go +++ b/pkg/storage/internalstorage/collectionresource_storage.go @@ -5,6 +5,7 @@ import ( "fmt" "net/url" "sort" + "strconv" "strings" "gorm.io/gorm" @@ -98,7 +99,7 @@ func (s *CollectionResourceStorage) Get(ctx context.Context, opts *internal.List if err != nil { return nil, err } - _, query, err = applyListOptionsToCollectionResourceQuery(query, opts) + offset, amount, query, err := applyListOptionsToCollectionResourceQuery(query, opts) if err != nil { return nil, err } @@ -106,22 +107,26 @@ func (s *CollectionResourceStorage) Get(ctx context.Context, opts *internal.List if err := list.From(query); err != nil { return nil, InterpretDBError(s.collectionResource.Name, err) } + items := list.Items() + collection := &internal.CollectionResource{ + TypeMeta: s.collectionResource.TypeMeta, + ObjectMeta: s.collectionResource.ObjectMeta, + Items: make([]runtime.Object, 0, len(items)), + } gvrs := make(map[schema.GroupVersionResource]struct{}) - types := []internal.CollectionResourceType{} - objs := make([]runtime.Object, 0) - for _, resource := range list.Items() { + for _, resource := range items { obj, err := resource.ConvertToUnstructured() if err != nil { return nil, err } - objs = append(objs, obj) + collection.Items = append(collection.Items, obj) if resourceType := resource.GetResourceType(); !resourceType.Empty() { gvr := resourceType.GroupVersionResource() if _, ok := gvrs[gvr]; !ok { gvrs[gvr] = struct{}{} - types = append(types, internal.CollectionResourceType{ + collection.ResourceTypes = append(collection.ResourceTypes, internal.CollectionResourceType{ Group: resourceType.Group, Resource: resourceType.Resource, Version: resourceType.Version, @@ -130,14 +135,22 @@ func (s *CollectionResourceStorage) Get(ctx context.Context, opts *internal.List } } } - sortCollectionResourceTypes(types) + sortCollectionResourceTypes(collection.ResourceTypes) + + if opts.WithContinue != nil && *opts.WithContinue { + if int64(len(items)) == opts.Limit { + collection.Continue = strconv.FormatInt(offset+opts.Limit, 10) + } + } + + if amount != nil { + // When offset is too large, the data in the response is empty and the remaining count is negative. + // This ensures that `amount = offset + len(objects) + remain` + remain := *amount - offset - int64(len(items)) + collection.RemainingItemCount = &remain + } - return &internal.CollectionResource{ - TypeMeta: s.collectionResource.TypeMeta, - ObjectMeta: s.collectionResource.ObjectMeta, - ResourceTypes: types, - Items: objs, - }, nil + return collection, nil } func resolveGVRsFromURLQuery(query url.Values) (gvrs []schema.GroupVersionResource, err error) { @@ -230,8 +243,7 @@ func parseGroupVersionResource(gvr string) (schema.GroupVersionResource, error) return schema.GroupVersionResource{}, fmt.Errorf("unexpected GroupVersionResource string: %v, expect / or //", gvr) } -// TODO(iceber): support with remaining count and continue -func applyListOptionsToCollectionResourceQuery(query *gorm.DB, opts *internal.ListOptions) (int64, *gorm.DB, error) { +func applyListOptionsToCollectionResourceQuery(query *gorm.DB, opts *internal.ListOptions) (int64, *int64, *gorm.DB, error) { return applyListOptionsToQuery(query, opts, nil) } diff --git a/pkg/storage/internalstorage/resource_storage.go b/pkg/storage/internalstorage/resource_storage.go index b5103511d..ce29da11b 100644 --- a/pkg/storage/internalstorage/resource_storage.go +++ b/pkg/storage/internalstorage/resource_storage.go @@ -273,25 +273,16 @@ func (s *ResourceStorage) List(ctx context.Context, listObject runtime.Object, o } func applyListOptionsToResourceQuery(db *gorm.DB, query *gorm.DB, opts *internal.ListOptions) (int64, *int64, *gorm.DB, error) { - var amount *int64 applyFn := func(query *gorm.DB, opts *internal.ListOptions) (*gorm.DB, error) { query, err := applyOwnerToResourceQuery(db, query, opts) if err != nil { return nil, err } - if opts.WithRemainingCount != nil && *opts.WithRemainingCount { - amount = new(int64) - query = query.Count(amount) - } return query, nil } - offset, query, err := applyListOptionsToQuery(query, opts, applyFn) - if err != nil { - return 0, nil, nil, err - } - return offset, amount, query, nil + return applyListOptionsToQuery(query, opts, applyFn) } func applyOwnerToResourceQuery(db *gorm.DB, query *gorm.DB, opts *internal.ListOptions) (*gorm.DB, error) { diff --git a/pkg/storage/internalstorage/util.go b/pkg/storage/internalstorage/util.go index 658713196..a45223f11 100644 --- a/pkg/storage/internalstorage/util.go +++ b/pkg/storage/internalstorage/util.go @@ -27,7 +27,7 @@ var ( supportedOrderByFields = sets.NewString("cluster", "namespace", "name", "created_at", "resource_version") ) -func applyListOptionsToQuery(query *gorm.DB, opts *internal.ListOptions, applyFn func(query *gorm.DB, opts *internal.ListOptions) (*gorm.DB, error)) (int64, *gorm.DB, error) { +func applyListOptionsToQuery(query *gorm.DB, opts *internal.ListOptions, applyFn func(query *gorm.DB, opts *internal.ListOptions) (*gorm.DB, error)) (int64, *int64, *gorm.DB, error) { switch len(opts.ClusterNames) { case 0: case 1: @@ -126,7 +126,7 @@ func applyListOptionsToQuery(query *gorm.DB, opts *internal.ListOptions, applyFn } if len(fieldErrors) != 0 { - return 0, nil, apierrors.NewInvalid(schema.GroupKind{Group: internal.GroupName, Kind: "ListOptions"}, "fieldSelector", fieldErrors) + return 0, nil, nil, apierrors.NewInvalid(schema.GroupKind{Group: internal.GroupName, Kind: "ListOptions"}, "fieldSelector", fieldErrors) } values := requirement.Values().List() @@ -156,10 +156,16 @@ func applyListOptionsToQuery(query *gorm.DB, opts *internal.ListOptions, applyFn var err error query, err = applyFn(query, opts) if err != nil { - return 0, nil, err + return 0, nil, nil, err } } + var amount *int64 + if opts.WithRemainingCount != nil && *opts.WithRemainingCount { + amount = new(int64) + query = query.Count(amount) + } + // Due to performance reasons, the default order by is not set. // https://github.com/clusterpedia-io/clusterpedia/pull/44 for _, orderby := range opts.OrderBy { @@ -186,5 +192,5 @@ func applyListOptionsToQuery(query *gorm.DB, opts *internal.ListOptions, applyFn if err == nil { query = query.Offset(offset) } - return int64(offset), query, nil + return int64(offset), amount, query, nil } diff --git a/pkg/storage/internalstorage/util_test.go b/pkg/storage/internalstorage/util_test.go index 048cb8873..4c064948b 100644 --- a/pkg/storage/internalstorage/util_test.go +++ b/pkg/storage/internalstorage/util_test.go @@ -24,7 +24,7 @@ func testApplyListOptionsToQuery(t *testing.T, name string, options *internal.Li t.Run(fmt.Sprintf("%s postgres", name), func(t *testing.T) { postgreSQL, err := toSQL(postgresDB, options, func(query *gorm.DB, options *internal.ListOptions) (*gorm.DB, error) { - _, query, err := applyListOptionsToQuery(query, options, nil) + _, _, query, err := applyListOptionsToQuery(query, options, nil) return query, err }, ) @@ -39,7 +39,7 @@ func testApplyListOptionsToQuery(t *testing.T, name string, options *internal.Li t.Run(fmt.Sprintf("%s mysql-%s", name, version), func(t *testing.T) { mysqlSQL, err := toSQL(mysqlDBs[version], options, func(query *gorm.DB, options *internal.ListOptions) (*gorm.DB, error) { - _, query, err := applyListOptionsToQuery(query, options, nil) + _, _, query, err := applyListOptionsToQuery(query, options, nil) return query, err }, ) diff --git a/staging/src/github.com/clusterpedia-io/api/clusterpedia/types.go b/staging/src/github.com/clusterpedia-io/api/clusterpedia/types.go index b9660bdfc..deb18e257 100644 --- a/staging/src/github.com/clusterpedia-io/api/clusterpedia/types.go +++ b/staging/src/github.com/clusterpedia-io/api/clusterpedia/types.go @@ -82,6 +82,9 @@ type CollectionResource struct { ResourceTypes []CollectionResourceType Items []runtime.Object + + Continue string + RemainingItemCount *int64 } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/staging/src/github.com/clusterpedia-io/api/clusterpedia/v1beta1/types.go b/staging/src/github.com/clusterpedia-io/api/clusterpedia/v1beta1/types.go index 1c311302e..723c36b47 100644 --- a/staging/src/github.com/clusterpedia-io/api/clusterpedia/v1beta1/types.go +++ b/staging/src/github.com/clusterpedia-io/api/clusterpedia/v1beta1/types.go @@ -75,6 +75,12 @@ type CollectionResource struct { // +optional Items []runtime.RawExtension `json:"items,omitempty"` + + // +optional + Continue string `json:"continue,omitempty"` + + // +optional + RemainingItemCount *int64 `json:"remainintItemCount,omitempty"` } type CollectionResourceType struct { diff --git a/staging/src/github.com/clusterpedia-io/api/clusterpedia/v1beta1/zz_generated.conversion.go b/staging/src/github.com/clusterpedia-io/api/clusterpedia/v1beta1/zz_generated.conversion.go index 5032d4e9f..e0c81f294 100644 --- a/staging/src/github.com/clusterpedia-io/api/clusterpedia/v1beta1/zz_generated.conversion.go +++ b/staging/src/github.com/clusterpedia-io/api/clusterpedia/v1beta1/zz_generated.conversion.go @@ -88,6 +88,8 @@ func autoConvert_v1beta1_CollectionResource_To_clusterpedia_CollectionResource(i } else { out.Items = nil } + out.Continue = in.Continue + out.RemainingItemCount = (*int64)(unsafe.Pointer(in.RemainingItemCount)) return nil } @@ -110,6 +112,8 @@ func autoConvert_clusterpedia_CollectionResource_To_v1beta1_CollectionResource(i } else { out.Items = nil } + out.Continue = in.Continue + out.RemainingItemCount = (*int64)(unsafe.Pointer(in.RemainingItemCount)) return nil } diff --git a/staging/src/github.com/clusterpedia-io/api/clusterpedia/v1beta1/zz_generated.deepcopy.go b/staging/src/github.com/clusterpedia-io/api/clusterpedia/v1beta1/zz_generated.deepcopy.go index ae0ca2e62..710cd4bf2 100644 --- a/staging/src/github.com/clusterpedia-io/api/clusterpedia/v1beta1/zz_generated.deepcopy.go +++ b/staging/src/github.com/clusterpedia-io/api/clusterpedia/v1beta1/zz_generated.deepcopy.go @@ -28,6 +28,11 @@ func (in *CollectionResource) DeepCopyInto(out *CollectionResource) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.RemainingItemCount != nil { + in, out := &in.RemainingItemCount, &out.RemainingItemCount + *out = new(int64) + **out = **in + } return } diff --git a/staging/src/github.com/clusterpedia-io/api/clusterpedia/zz_generated.deepcopy.go b/staging/src/github.com/clusterpedia-io/api/clusterpedia/zz_generated.deepcopy.go index cceef58d9..1e5aab42f 100644 --- a/staging/src/github.com/clusterpedia-io/api/clusterpedia/zz_generated.deepcopy.go +++ b/staging/src/github.com/clusterpedia-io/api/clusterpedia/zz_generated.deepcopy.go @@ -30,6 +30,11 @@ func (in *CollectionResource) DeepCopyInto(out *CollectionResource) { } } } + if in.RemainingItemCount != nil { + in, out := &in.RemainingItemCount, &out.RemainingItemCount + *out = new(int64) + **out = **in + } return }