From 1ad5cb5bb16c83bc7f0a3b78ad196a665a45996d Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Fri, 31 May 2019 15:32:26 -0700 Subject: [PATCH 1/2] Protect remainingItemCount behind a feature flag. Also updating the API doc --- .../apimachinery/pkg/apis/meta/v1/types.go | 15 +++-- .../apiserver/pkg/features/kube_features.go | 8 +++ .../apiserver/pkg/storage/etcd3/store.go | 10 +++- .../apiserver/pkg/storage/etcd3/store_test.go | 4 ++ test/e2e/apimachinery/chunking.go | 55 ++++++++++++------- 5 files changed, 65 insertions(+), 27 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go index 10ae143c20b..46ef65f457f 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go @@ -82,11 +82,18 @@ type ListMeta struct { // message. Continue string `json:"continue,omitempty" protobuf:"bytes,3,opt,name=continue"` - // RemainingItemCount is the number of subsequent items in the list which are not included in this + // remainingItemCount is the number of subsequent items in the list which are not included in this // list response. If the list request contained label or field selectors, then the number of - // remaining items is unknown and this field will be unset. If the list is complete (either - // because it is unpaginated or because this is the last page), then there are no more remaining - // items and this field will also be unset. Servers older than v1.15 do not set this field. + // remaining items is unknown and the field will be left unset and omitted during serialization. + // If the list is complete (either because it is not chunking or because this is the last chunk), + // then there are no more remaining items and this field will be left unset and omitted during + // serialization. + // Servers older than v1.15 do not set this field. + // The intended use of the remainingItemCount is *estimating* the size of a collection. Clients + // should not rely on the remainingItemCount to be set or to be exact. + // + // This field is alpha and can be changed or removed without notice. + // // +optional RemainingItemCount *int64 `json:"remainingItemCount,omitempty" protobuf:"bytes,4,opt,name=remainingItemCount"` } diff --git a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go index f74fb108dbb..fb3a1e77be9 100644 --- a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go +++ b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go @@ -86,6 +86,13 @@ const ( // committing. DryRun featuregate.Feature = "DryRun" + // owner: @caesarxuchao + // alpha: v1.15 + // + // Allow apiservers to show a count of remaining items in the response + // to a chunking list request. + RemainingItemCount featuregate.Feature = "RemainingItemCount" + // owner: @apelisse, @lavalamp // alpha: v1.14 // @@ -140,6 +147,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS APIResponseCompression: {Default: false, PreRelease: featuregate.Alpha}, APIListChunking: {Default: true, PreRelease: featuregate.Beta}, DryRun: {Default: true, PreRelease: featuregate.Beta}, + RemainingItemCount: {Default: false, PreRelease: featuregate.Alpha}, ServerSideApply: {Default: false, PreRelease: featuregate.Alpha}, StorageVersionHash: {Default: false, PreRelease: featuregate.Alpha}, WinOverlay: {Default: false, PreRelease: featuregate.Alpha}, 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 c9ca0a976be..cf1dc284857 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go @@ -36,10 +36,12 @@ import ( "k8s.io/apimachinery/pkg/conversion" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/watch" + "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/storage" "k8s.io/apiserver/pkg/storage/etcd" "k8s.io/apiserver/pkg/storage/etcd/metrics" "k8s.io/apiserver/pkg/storage/value" + utilfeature "k8s.io/apiserver/pkg/util/feature" utiltrace "k8s.io/utils/trace" ) @@ -622,9 +624,11 @@ func (s *store) List(ctx context.Context, key, resourceVersion string, pred stor // getResp.Count counts in objects that do not match the pred. // Instead of returning inaccurate count for non-empty selectors, we return nil. // Only set remainingItemCount if the predicate is empty. - if pred.Empty() { - c := int64(getResp.Count - pred.Limit) - remainingItemCount = &c + if utilfeature.DefaultFeatureGate.Enabled(features.RemainingItemCount) { + if pred.Empty() { + c := int64(getResp.Count - pred.Limit) + remainingItemCount = &c + } } return s.versioner.UpdateList(listObj, uint64(returnedRV), next, remainingItemCount) } diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go index 72ed4227b2b..ba5275bb63a 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go @@ -43,10 +43,13 @@ import ( "k8s.io/apimachinery/pkg/watch" "k8s.io/apiserver/pkg/apis/example" examplev1 "k8s.io/apiserver/pkg/apis/example/v1" + "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/storage" "k8s.io/apiserver/pkg/storage/etcd" storagetests "k8s.io/apiserver/pkg/storage/tests" "k8s.io/apiserver/pkg/storage/value" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" utilpointer "k8s.io/utils/pointer" ) @@ -751,6 +754,7 @@ func TestTransformationFailure(t *testing.T) { } func TestList(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RemainingItemCount, true)() codec := apitesting.TestCodec(codecs, examplev1.SchemeGroupVersion) cluster := integration.NewClusterV3(t, &integration.ClusterConfig{Size: 1}) defer cluster.Terminate(t) diff --git a/test/e2e/apimachinery/chunking.go b/test/e2e/apimachinery/chunking.go index c6f3910dc23..ebd461261c3 100644 --- a/test/e2e/apimachinery/chunking.go +++ b/test/e2e/apimachinery/chunking.go @@ -30,12 +30,18 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/storage/storagebackend" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/util/workqueue" "k8s.io/kubernetes/test/e2e/framework" e2elog "k8s.io/kubernetes/test/e2e/framework/log" ) +func shouldCheckRemainingItem() bool { + return utilfeature.DefaultFeatureGate.Enabled(features.RemainingItemCount) +} + const numberOfTotalResources = 400 var _ = SIGDescribe("Servers with support for API chunking", func() { @@ -89,11 +95,13 @@ var _ = SIGDescribe("Servers with support for API chunking", func() { lastRV = list.ResourceVersion } gomega.Expect(list.ResourceVersion).To(gomega.Equal(lastRV)) - if list.GetContinue() == "" { - gomega.Expect(list.GetRemainingItemCount()).To(gomega.BeNil()) - } else { - gomega.Expect(list.GetRemainingItemCount()).ToNot(gomega.BeNil()) - gomega.Expect(int(*list.GetRemainingItemCount()) + len(list.Items) + found).To(gomega.BeNumerically("==", numberOfTotalResources)) + if shouldCheckRemainingItem() { + if list.GetContinue() == "" { + gomega.Expect(list.GetRemainingItemCount()).To(gomega.BeNil()) + } else { + gomega.Expect(list.GetRemainingItemCount()).ToNot(gomega.BeNil()) + gomega.Expect(int(*list.GetRemainingItemCount()) + len(list.Items) + found).To(gomega.BeNumerically("==", numberOfTotalResources)) + } } for _, item := range list.Items { gomega.Expect(item.Name).To(gomega.Equal(fmt.Sprintf("template-%04d", found))) @@ -127,11 +135,13 @@ var _ = SIGDescribe("Servers with support for API chunking", func() { framework.ExpectNoError(err, "failed to list pod templates in namespace: %s, given limit: %d", ns, opts.Limit) firstToken := list.Continue firstRV := list.ResourceVersion - if list.GetContinue() == "" { - gomega.Expect(list.GetRemainingItemCount()).To(gomega.BeNil()) - } else { - gomega.Expect(list.GetRemainingItemCount()).ToNot(gomega.BeNil()) - gomega.Expect(int(*list.GetRemainingItemCount()) + len(list.Items)).To(gomega.BeNumerically("==", numberOfTotalResources)) + if shouldCheckRemainingItem() { + if list.GetContinue() == "" { + gomega.Expect(list.GetRemainingItemCount()).To(gomega.BeNil()) + } else { + gomega.Expect(list.GetRemainingItemCount()).ToNot(gomega.BeNil()) + gomega.Expect(int(*list.GetRemainingItemCount()) + len(list.Items)).To(gomega.BeNumerically("==", numberOfTotalResources)) + } } e2elog.Logf("Retrieved %d/%d results with rv %s and continue %s", len(list.Items), opts.Limit, list.ResourceVersion, firstToken) @@ -167,11 +177,14 @@ var _ = SIGDescribe("Servers with support for API chunking", func() { gomega.Expect(list.ResourceVersion).ToNot(gomega.Equal(firstRV)) gomega.Expect(len(list.Items)).To(gomega.BeNumerically("==", opts.Limit)) found := int(oneTenth) - if list.GetContinue() == "" { - gomega.Expect(list.GetRemainingItemCount()).To(gomega.BeNil()) - } else { - gomega.Expect(list.GetRemainingItemCount()).ToNot(gomega.BeNil()) - gomega.Expect(int(*list.GetRemainingItemCount()) + len(list.Items) + found).To(gomega.BeNumerically("==", numberOfTotalResources)) + + if shouldCheckRemainingItem() { + if list.GetContinue() == "" { + gomega.Expect(list.GetRemainingItemCount()).To(gomega.BeNil()) + } else { + gomega.Expect(list.GetRemainingItemCount()).ToNot(gomega.BeNil()) + gomega.Expect(int(*list.GetRemainingItemCount()) + len(list.Items) + found).To(gomega.BeNumerically("==", numberOfTotalResources)) + } } for _, item := range list.Items { gomega.Expect(item.Name).To(gomega.Equal(fmt.Sprintf("template-%04d", found))) @@ -184,11 +197,13 @@ var _ = SIGDescribe("Servers with support for API chunking", func() { for { list, err := client.List(opts) framework.ExpectNoError(err, "failed to list pod templates in namespace: %s, given limit: %d", ns, opts.Limit) - if list.GetContinue() == "" { - gomega.Expect(list.GetRemainingItemCount()).To(gomega.BeNil()) - } else { - gomega.Expect(list.GetRemainingItemCount()).ToNot(gomega.BeNil()) - gomega.Expect(int(*list.GetRemainingItemCount()) + len(list.Items) + found).To(gomega.BeNumerically("==", numberOfTotalResources)) + if shouldCheckRemainingItem() { + if list.GetContinue() == "" { + gomega.Expect(list.GetRemainingItemCount()).To(gomega.BeNil()) + } else { + gomega.Expect(list.GetRemainingItemCount()).ToNot(gomega.BeNil()) + gomega.Expect(int(*list.GetRemainingItemCount()) + len(list.Items) + found).To(gomega.BeNumerically("==", numberOfTotalResources)) + } } e2elog.Logf("Retrieved %d/%d results with rv %s and continue %s", len(list.Items), opts.Limit, list.ResourceVersion, list.Continue) gomega.Expect(len(list.Items)).To(gomega.BeNumerically("<=", opts.Limit)) From a64b3890e7b4bd615daf4d557f0c7e4692d2ecc3 Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Fri, 31 May 2019 15:32:36 -0700 Subject: [PATCH 2/2] generated --- api/openapi-spec/swagger.json | 2 +- .../pkg/generated/openapi/zz_generated.openapi.go | 2 +- .../apimachinery/pkg/apis/meta/v1/generated.proto | 15 +++++++++++---- .../apis/meta/v1/types_swagger_doc_generated.go | 2 +- .../src/k8s.io/apiserver/pkg/storage/etcd3/BUILD | 5 +++++ test/e2e/apimachinery/BUILD | 2 ++ 6 files changed, 21 insertions(+), 7 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index 67c37dd0f9f..0f866c72677 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -17478,7 +17478,7 @@ "type": "string" }, "remainingItemCount": { - "description": "RemainingItemCount is the number of subsequent items in the list which are not included in this list response. If the list request contained label or field selectors, then the number of remaining items is unknown and this field will be unset. If the list is complete (either because it is unpaginated or because this is the last page), then there are no more remaining items and this field will also be unset. Servers older than v1.15 do not set this field.", + "description": "remainingItemCount is the number of subsequent items in the list which are not included in this list response. If the list request contained label or field selectors, then the number of remaining items is unknown and the field will be left unset and omitted during serialization. If the list is complete (either because it is not chunking or because this is the last chunk), then there are no more remaining items and this field will be left unset and omitted during serialization. Servers older than v1.15 do not set this field. The intended use of the remainingItemCount is *estimating* the size of a collection. Clients should not rely on the remainingItemCount to be set or to be exact.\n\nThis field is alpha and can be changed or removed without notice.", "format": "int64", "type": "integer" }, diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/generated/openapi/zz_generated.openapi.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/generated/openapi/zz_generated.openapi.go index 906b49377d8..a52bb613cb2 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/generated/openapi/zz_generated.openapi.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/generated/openapi/zz_generated.openapi.go @@ -1853,7 +1853,7 @@ func schema_pkg_apis_meta_v1_ListMeta(ref common.ReferenceCallback) common.OpenA }, "remainingItemCount": { SchemaProps: spec.SchemaProps{ - Description: "RemainingItemCount is the number of subsequent items in the list which are not included in this list response. If the list request contained label or field selectors, then the number of remaining items is unknown and this field will be unset. If the list is complete (either because it is unpaginated or because this is the last page), then there are no more remaining items and this field will also be unset. Servers older than v1.15 do not set this field.", + Description: "remainingItemCount is the number of subsequent items in the list which are not included in this list response. If the list request contained label or field selectors, then the number of remaining items is unknown and the field will be left unset and omitted during serialization. If the list is complete (either because it is not chunking or because this is the last chunk), then there are no more remaining items and this field will be left unset and omitted during serialization. Servers older than v1.15 do not set this field. The intended use of the remainingItemCount is *estimating* the size of a collection. Clients should not rely on the remainingItemCount to be set or to be exact.\n\nThis field is alpha and can be changed or removed without notice.", Type: []string{"integer"}, Format: "int64", }, diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/generated.proto b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/generated.proto index 0e00a0cdb40..cc9099a6570 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/generated.proto +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/generated.proto @@ -396,11 +396,18 @@ message ListMeta { // message. optional string continue = 3; - // RemainingItemCount is the number of subsequent items in the list which are not included in this + // remainingItemCount is the number of subsequent items in the list which are not included in this // list response. If the list request contained label or field selectors, then the number of - // remaining items is unknown and this field will be unset. If the list is complete (either - // because it is unpaginated or because this is the last page), then there are no more remaining - // items and this field will also be unset. Servers older than v1.15 do not set this field. + // remaining items is unknown and the field will be left unset and omitted during serialization. + // If the list is complete (either because it is not chunking or because this is the last chunk), + // then there are no more remaining items and this field will be left unset and omitted during + // serialization. + // Servers older than v1.15 do not set this field. + // The intended use of the remainingItemCount is *estimating* the size of a collection. Clients + // should not rely on the remainingItemCount to be set or to be exact. + // + // This field is alpha and can be changed or removed without notice. + // // +optional optional int64 remainingItemCount = 4; } diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types_swagger_doc_generated.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types_swagger_doc_generated.go index cc3cd1edcf4..f35c22bf162 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types_swagger_doc_generated.go @@ -201,7 +201,7 @@ var map_ListMeta = map[string]string{ "selfLink": "selfLink is a URL representing this object. Populated by the system. Read-only.", "resourceVersion": "String that identifies the server's internal version of this object that can be used by clients to determine when objects have changed. Value must be treated as opaque by clients and passed unmodified back to the server. Populated by the system. Read-only. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#concurrency-control-and-consistency", "continue": "continue may be set if the user set a limit on the number of items returned, and indicates that the server has more data available. The value is opaque and may be used to issue another request to the endpoint that served this list to retrieve the next set of available objects. Continuing a consistent list may not be possible if the server configuration has changed or more than a few minutes have passed. The resourceVersion field returned when using this continue value will be identical to the value in the first response, unless you have received this token from an error message.", - "remainingItemCount": "RemainingItemCount is the number of subsequent items in the list which are not included in this list response. If the list request contained label or field selectors, then the number of remaining items is unknown and this field will be unset. If the list is complete (either because it is unpaginated or because this is the last page), then there are no more remaining items and this field will also be unset. Servers older than v1.15 do not set this field.", + "remainingItemCount": "remainingItemCount is the number of subsequent items in the list which are not included in this list response. If the list request contained label or field selectors, then the number of remaining items is unknown and the field will be left unset and omitted during serialization. If the list is complete (either because it is not chunking or because this is the last chunk), then there are no more remaining items and this field will be left unset and omitted during serialization. Servers older than v1.15 do not set this field. The intended use of the remainingItemCount is *estimating* the size of a collection. Clients should not rely on the remainingItemCount to be set or to be exact.\n\nThis field is alpha and can be changed or removed without notice.", } func (ListMeta) SwaggerDoc() map[string]string { diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/BUILD b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/BUILD index f03ccf7060b..5c2d9082977 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/BUILD @@ -30,10 +30,13 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library", "//staging/src/k8s.io/apiserver/pkg/apis/example:go_default_library", "//staging/src/k8s.io/apiserver/pkg/apis/example/v1:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/features:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage/etcd:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage/tests:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage/value:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", + "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", "//vendor/github.com/coreos/etcd/clientv3:go_default_library", "//vendor/github.com/coreos/etcd/etcdserver/api/v3rpc/rpctypes:go_default_library", "//vendor/github.com/coreos/etcd/integration:go_default_library", @@ -65,10 +68,12 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/features:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage/etcd:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage/etcd/metrics:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage/value:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//vendor/github.com/coreos/etcd/clientv3:go_default_library", "//vendor/github.com/coreos/etcd/etcdserver/api/v3rpc/rpctypes:go_default_library", "//vendor/github.com/coreos/etcd/mvcc/mvccpb:go_default_library", diff --git a/test/e2e/apimachinery/BUILD b/test/e2e/apimachinery/BUILD index df91e7781e2..1b8ff61b354 100644 --- a/test/e2e/apimachinery/BUILD +++ b/test/e2e/apimachinery/BUILD @@ -65,8 +65,10 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library", "//staging/src/k8s.io/apiserver/pkg/endpoints/discovery:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/features:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage/names:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage/storagebackend:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/discovery:go_default_library", "//staging/src/k8s.io/client-go/dynamic:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library",