From 6acfa3cb4ac876e46ead5ba4772ba18e480435ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Fri, 21 Jul 2023 11:35:21 +0200 Subject: [PATCH 1/3] Graduate APIListChunking to GA --- cluster/gce/config-default.sh | 2 +- cluster/gce/config-test.sh | 2 +- cmd/kube-apiserver/app/aggregator.go | 2 +- pkg/controlplane/apiserver/apiextensions.go | 4 +--- pkg/features/kube_features.go | 2 +- .../k8s.io/apiserver/pkg/features/kube_features.go | 3 ++- .../pkg/server/storage/storage_factory.go | 12 +----------- .../k8s.io/apiserver/pkg/storage/cacher/cacher.go | 5 ++--- .../flowcontrol/request/list_work_estimator.go | 8 +++----- .../sample-apiserver/pkg/cmd/server/start.go | 2 +- test/e2e/apimachinery/chunking.go | 4 ++-- test/integration/apiserver/apiserver_test.go | 14 ++------------ 12 files changed, 18 insertions(+), 42 deletions(-) diff --git a/cluster/gce/config-default.sh b/cluster/gce/config-default.sh index d0641c9cfa4..12ff7f39b76 100755 --- a/cluster/gce/config-default.sh +++ b/cluster/gce/config-default.sh @@ -265,7 +265,7 @@ fi RUN_CCM_CONTROLLERS="${RUN_CCM_CONTROLLERS:-*,-gkenetworkparamset}" # List of the set of feature gates recognized by the GCP CCM -export CCM_FEATURE_GATES="APIListChunking,APIPriorityAndFairness,APIResponseCompression,APIServerIdentity,APIServerTracing,AllAlpha,AllBeta,CustomResourceValidationExpressions,KMSv2,OpenAPIEnums,OpenAPIV3,RemainingItemCount,ServerSideFieldValidation,StorageVersionAPI,StorageVersionHash" +export CCM_FEATURE_GATES="APIPriorityAndFairness,APIResponseCompression,APIServerIdentity,APIServerTracing,AllAlpha,AllBeta,CustomResourceValidationExpressions,KMSv2,OpenAPIEnums,OpenAPIV3,RemainingItemCount,ServerSideFieldValidation,StorageVersionAPI,StorageVersionHash" # Optional: set feature gates # shellcheck disable=SC2034 # Variables sourced in other scripts. diff --git a/cluster/gce/config-test.sh b/cluster/gce/config-test.sh index 770c3f752d6..122d84992b8 100755 --- a/cluster/gce/config-test.sh +++ b/cluster/gce/config-test.sh @@ -316,7 +316,7 @@ if [[ -n "${NODE_ACCELERATORS}" ]]; then fi # List of the set of feature gates recognized by the GCP CCM -export CCM_FEATURE_GATES="APIListChunking,APIPriorityAndFairness,APIResponseCompression,APIServerIdentity,APIServerTracing,AllAlpha,AllBeta,CustomResourceValidationExpressions,KMSv2,OpenAPIEnums,OpenAPIV3,RemainingItemCount,ServerSideFieldValidation,StorageVersionAPI,StorageVersionHash" +export CCM_FEATURE_GATES="APIPriorityAndFairness,APIResponseCompression,APIServerIdentity,APIServerTracing,AllAlpha,AllBeta,CustomResourceValidationExpressions,KMSv2,OpenAPIEnums,OpenAPIV3,RemainingItemCount,ServerSideFieldValidation,StorageVersionAPI,StorageVersionHash" # Optional: Install cluster DNS. # Set CLUSTER_DNS_CORE_DNS to 'false' to install kube-dns instead of CoreDNS. diff --git a/cmd/kube-apiserver/app/aggregator.go b/cmd/kube-apiserver/app/aggregator.go index 48264016b71..d04f6f80df2 100644 --- a/cmd/kube-apiserver/app/aggregator.go +++ b/cmd/kube-apiserver/app/aggregator.go @@ -92,7 +92,7 @@ func createAggregatorConfig( // we assume that the etcd options have been completed already. avoid messing with anything outside // of changes to StorageConfig as that may lead to unexpected behavior when the options are applied. etcdOptions := *commandOptions.Etcd - etcdOptions.StorageConfig.Paging = utilfeature.DefaultFeatureGate.Enabled(genericfeatures.APIListChunking) + etcdOptions.StorageConfig.Paging = true etcdOptions.StorageConfig.Codec = aggregatorscheme.Codecs.LegacyCodec(v1.SchemeGroupVersion, v1beta1.SchemeGroupVersion) etcdOptions.StorageConfig.EncodeVersioner = runtime.NewMultiGroupVersioner(v1.SchemeGroupVersion, schema.GroupKind{Group: v1beta1.GroupName}) etcdOptions.SkipHealthEndpoints = true // avoid double wiring of health checks diff --git a/pkg/controlplane/apiserver/apiextensions.go b/pkg/controlplane/apiserver/apiextensions.go index 67c5ee83a58..4b58dd76d61 100644 --- a/pkg/controlplane/apiserver/apiextensions.go +++ b/pkg/controlplane/apiserver/apiextensions.go @@ -24,9 +24,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/admission" - "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/server" - "k8s.io/apiserver/pkg/util/feature" "k8s.io/apiserver/pkg/util/webhook" "k8s.io/client-go/informers" @@ -52,7 +50,7 @@ func CreateAPIExtensionsConfig( // we assume that the etcd options have been completed already. avoid messing with anything outside // of changes to StorageConfig as that may lead to unexpected behavior when the options are applied. etcdOptions := *commandOptions.Etcd - etcdOptions.StorageConfig.Paging = feature.DefaultFeatureGate.Enabled(features.APIListChunking) + etcdOptions.StorageConfig.Paging = true // this is where the true decodable levels come from. etcdOptions.StorageConfig.Codec = apiextensionsapiserver.Codecs.LegacyCodec(v1beta1.SchemeGroupVersion, v1.SchemeGroupVersion) // prefer the more compact serialization (v1beta1) for storage until https://issue.k8s.io/82292 is resolved for objects whose v1 serialization is too big but whose v1beta1 serialization can be stored diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index dde237e8b08..b36d56ae6ef 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -1191,7 +1191,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS genericfeatures.AggregatedDiscoveryEndpoint: {Default: true, PreRelease: featuregate.Beta}, - genericfeatures.APIListChunking: {Default: true, PreRelease: featuregate.Beta}, + genericfeatures.APIListChunking: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.32 genericfeatures.APIPriorityAndFairness: {Default: true, PreRelease: featuregate.Beta}, 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 392e8a77753..857e65b4eb5 100644 --- a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go +++ b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go @@ -54,6 +54,7 @@ const ( // owner: @smarterclayton // alpha: v1.8 // beta: v1.9 + // stable: 1.29 // // Allow API clients to retrieve resource lists in chunks rather than // all at once. @@ -231,7 +232,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS AdmissionWebhookMatchConditions: {Default: true, PreRelease: featuregate.Beta}, - APIListChunking: {Default: true, PreRelease: featuregate.Beta}, + APIListChunking: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.32 APIPriorityAndFairness: {Default: true, PreRelease: featuregate.Beta}, diff --git a/staging/src/k8s.io/apiserver/pkg/server/storage/storage_factory.go b/staging/src/k8s.io/apiserver/pkg/server/storage/storage_factory.go index be4d0390d60..f1b83b2bbda 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/storage/storage_factory.go +++ b/staging/src/k8s.io/apiserver/pkg/server/storage/storage_factory.go @@ -25,9 +25,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/storage/storagebackend" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog/v2" ) @@ -156,7 +154,7 @@ func NewDefaultStorageFactory( resourceConfig APIResourceConfigSource, specialDefaultResourcePrefixes map[schema.GroupResource]string, ) *DefaultStorageFactory { - config.Paging = utilfeature.DefaultFeatureGate.Enabled(features.APIListChunking) + config.Paging = true if len(defaultMediaType) == 0 { defaultMediaType = runtime.ContentTypeJSON } @@ -185,14 +183,6 @@ func (s *DefaultStorageFactory) SetEtcdPrefix(groupResource schema.GroupResource s.Overrides[groupResource] = overrides } -// SetDisableAPIListChunking allows a specific resource to disable paging at the storage layer, to prevent -// exposure of key names in continuations. This may be overridden by feature gates. -func (s *DefaultStorageFactory) SetDisableAPIListChunking(groupResource schema.GroupResource) { - overrides := s.Overrides[groupResource] - overrides.disablePaging = true - s.Overrides[groupResource] = overrides -} - // SetResourceEtcdPrefix sets the prefix for a resource, but not the base-dir. You'll end up in `etcdPrefix/resourceEtcdPrefix`. func (s *DefaultStorageFactory) SetResourceEtcdPrefix(groupResource schema.GroupResource, prefix string) { overrides := s.Overrides[groupResource] diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go index 0796f591d7f..d3cce555082 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go @@ -725,15 +725,14 @@ func shouldDelegateList(opts storage.ListOptions) bool { resourceVersion := opts.ResourceVersion pred := opts.Predicate match := opts.ResourceVersionMatch - pagingEnabled := utilfeature.DefaultFeatureGate.Enabled(features.APIListChunking) consistentListFromCacheEnabled := utilfeature.DefaultFeatureGate.Enabled(features.ConsistentListFromCache) // Serve consistent reads from storage if ConsistentListFromCache is disabled consistentReadFromStorage := resourceVersion == "" && !consistentListFromCacheEnabled // Watch cache doesn't support continuations, so serve them from etcd. - hasContinuation := pagingEnabled && len(pred.Continue) > 0 + hasContinuation := len(pred.Continue) > 0 // Serve paginated requests about revision "0" from watch cache to avoid overwhelming etcd. - hasLimit := pagingEnabled && pred.Limit > 0 && resourceVersion != "0" + hasLimit := pred.Limit > 0 && resourceVersion != "0" // Watch cache only supports ResourceVersionMatchNotOlderThan (default). unsupportedMatch := match != "" && match != metav1.ResourceVersionMatchNotOlderThan diff --git a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/request/list_work_estimator.go b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/request/list_work_estimator.go index 8d20867d6dd..6b941cb7fe1 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/request/list_work_estimator.go +++ b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/request/list_work_estimator.go @@ -117,8 +117,7 @@ func (e *listWorkEstimator) estimate(r *http.Request, flowSchemaName, priorityLe } limit := numStored - if utilfeature.DefaultFeatureGate.Enabled(features.APIListChunking) && listOptions.Limit > 0 && - listOptions.Limit < numStored { + if listOptions.Limit > 0 && listOptions.Limit < numStored { limit = listOptions.Limit } @@ -165,15 +164,14 @@ func key(requestInfo *apirequest.RequestInfo) string { func shouldListFromStorage(query url.Values, opts *metav1.ListOptions) bool { resourceVersion := opts.ResourceVersion match := opts.ResourceVersionMatch - pagingEnabled := utilfeature.DefaultFeatureGate.Enabled(features.APIListChunking) consistentListFromCacheEnabled := utilfeature.DefaultFeatureGate.Enabled(features.ConsistentListFromCache) // Serve consistent reads from storage if ConsistentListFromCache is disabled consistentReadFromStorage := resourceVersion == "" && !consistentListFromCacheEnabled // Watch cache doesn't support continuations, so serve them from etcd. - hasContinuation := pagingEnabled && len(opts.Continue) > 0 + hasContinuation := len(opts.Continue) > 0 // Serve paginated requests about revision "0" from watch cache to avoid overwhelming etcd. - hasLimit := pagingEnabled && opts.Limit > 0 && resourceVersion != "0" + hasLimit := opts.Limit > 0 && resourceVersion != "0" // Watch cache only supports ResourceVersionMatchNotOlderThan (default). unsupportedMatch := match != "" && match != metav1.ResourceVersionMatchNotOlderThan diff --git a/staging/src/k8s.io/sample-apiserver/pkg/cmd/server/start.go b/staging/src/k8s.io/sample-apiserver/pkg/cmd/server/start.go index 1f57e82d02a..2be535e7cb0 100644 --- a/staging/src/k8s.io/sample-apiserver/pkg/cmd/server/start.go +++ b/staging/src/k8s.io/sample-apiserver/pkg/cmd/server/start.go @@ -123,7 +123,7 @@ func (o *WardleServerOptions) Config() (*apiserver.Config, error) { return nil, fmt.Errorf("error creating self-signed certificates: %v", err) } - o.RecommendedOptions.Etcd.StorageConfig.Paging = utilfeature.DefaultFeatureGate.Enabled(features.APIListChunking) + o.RecommendedOptions.Etcd.StorageConfig.Paging = true o.RecommendedOptions.ExtraAdmissionInitializers = func(c *genericapiserver.RecommendedConfig) ([]admission.PluginInitializer, error) { client, err := clientset.NewForConfig(c.LoopbackClientConfig) diff --git a/test/e2e/apimachinery/chunking.go b/test/e2e/apimachinery/chunking.go index 8013d996897..bd1536149bc 100644 --- a/test/e2e/apimachinery/chunking.go +++ b/test/e2e/apimachinery/chunking.go @@ -76,7 +76,7 @@ var _ = SIGDescribe("Servers with support for API chunking", func() { }) }) - ginkgo.It("should return chunks of results for list calls", func(ctx context.Context) { + framework.ConformanceIt("should return chunks of results for list calls", func(ctx context.Context) { ns := f.Namespace.Name c := f.ClientSet client := c.CoreV1().PodTemplates(ns) @@ -123,7 +123,7 @@ var _ = SIGDescribe("Servers with support for API chunking", func() { gomega.Expect(list.Items).To(gomega.HaveLen(numberOfTotalResources)) }) - ginkgo.It("should support continue listing from the last key if the original version has been compacted away, though the list is inconsistent [Slow]", func(ctx context.Context) { + framework.ConformanceIt("should support continue listing from the last key if the original version has been compacted away, though the list is inconsistent [Slow]", func(ctx context.Context) { ns := f.Namespace.Name c := f.ClientSet client := c.CoreV1().PodTemplates(ns) diff --git a/test/integration/apiserver/apiserver_test.go b/test/integration/apiserver/apiserver_test.go index a6e46eaa194..13a46da2cb5 100644 --- a/test/integration/apiserver/apiserver_test.go +++ b/test/integration/apiserver/apiserver_test.go @@ -53,16 +53,13 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apimachinery/pkg/watch" "k8s.io/apiserver/pkg/endpoints/handlers" - "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/storage/storagebackend" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/dynamic" clientset "k8s.io/client-go/kubernetes" appsv1 "k8s.io/client-go/kubernetes/typed/apps/v1" "k8s.io/client-go/metadata" restclient "k8s.io/client-go/rest" "k8s.io/client-go/tools/pager" - featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/klog/v2" "k8s.io/kubernetes/cmd/kube-apiserver/app/options" kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" @@ -381,8 +378,6 @@ func TestListOptions(t *testing.T) { ctx, cancel := context.WithCancel(ctx) defer cancel() - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.APIListChunking, true)() - var storageTransport *storagebackend.TransportConfig clientSet, _, tearDownFn := framework.StartTestServer(ctx, t, framework.TestServerSetup{ ModifyServerRunOptions: func(opts *options.ServerRunOptions) { @@ -576,9 +571,8 @@ func testListOptionsCase(t *testing.T, rsClient appsv1.ReplicaSetInterface, watc // Cacher.GetList defines this for logic to decide if the watch cache is skipped. We need to know it to know if // the limit is respected when testing here. - pagingEnabled := utilfeature.DefaultFeatureGate.Enabled(features.APIListChunking) - hasContinuation := pagingEnabled && len(opts.Continue) > 0 - hasLimit := pagingEnabled && opts.Limit > 0 && opts.ResourceVersion != "0" + hasContinuation := len(opts.Continue) > 0 + hasLimit := opts.Limit > 0 && opts.ResourceVersion != "0" skipWatchCache := opts.ResourceVersion == "" || hasContinuation || hasLimit || isExact usingWatchCache := watchCacheEnabled && !skipWatchCache @@ -627,8 +621,6 @@ func TestListResourceVersion0(t *testing.T) { ctx, cancel := context.WithCancel(ctx) defer cancel() - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.APIListChunking, true)() - clientSet, _, tearDownFn := framework.StartTestServer(ctx, t, framework.TestServerSetup{ ModifyServerRunOptions: func(opts *options.ServerRunOptions) { opts.Etcd.EnableWatchCache = tc.watchCacheEnabled @@ -685,7 +677,6 @@ func TestListResourceVersion0(t *testing.T) { } func TestAPIListChunking(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.APIListChunking, true)() ctx, clientSet, _, tearDownFn := setup(t) defer tearDownFn() @@ -753,7 +744,6 @@ func TestAPIListChunking(t *testing.T) { } func TestAPIListChunkingWithLabelSelector(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.APIListChunking, true)() ctx, clientSet, _, tearDownFn := setup(t) defer tearDownFn() From 4e2e059c7b205d2e4b246a262128223258a49498 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Fri, 21 Jul 2023 15:22:51 +0200 Subject: [PATCH 2/3] Graduate RemainingItemCount to GA --- cluster/gce/config-default.sh | 2 +- cluster/gce/config-test.sh | 2 +- .../apiserver/pkg/features/kube_features.go | 3 +- .../apiserver/pkg/storage/etcd3/store.go | 10 ++-- .../pkg/storage/testing/store_tests.go | 7 --- test/e2e/apimachinery/chunking.go | 54 +++++++------------ 6 files changed, 27 insertions(+), 51 deletions(-) diff --git a/cluster/gce/config-default.sh b/cluster/gce/config-default.sh index 12ff7f39b76..b797683f040 100755 --- a/cluster/gce/config-default.sh +++ b/cluster/gce/config-default.sh @@ -265,7 +265,7 @@ fi RUN_CCM_CONTROLLERS="${RUN_CCM_CONTROLLERS:-*,-gkenetworkparamset}" # List of the set of feature gates recognized by the GCP CCM -export CCM_FEATURE_GATES="APIPriorityAndFairness,APIResponseCompression,APIServerIdentity,APIServerTracing,AllAlpha,AllBeta,CustomResourceValidationExpressions,KMSv2,OpenAPIEnums,OpenAPIV3,RemainingItemCount,ServerSideFieldValidation,StorageVersionAPI,StorageVersionHash" +export CCM_FEATURE_GATES="APIPriorityAndFairness,APIResponseCompression,APIServerIdentity,APIServerTracing,AllAlpha,AllBeta,CustomResourceValidationExpressions,KMSv2,OpenAPIEnums,OpenAPIV3,ServerSideFieldValidation,StorageVersionAPI,StorageVersionHash" # Optional: set feature gates # shellcheck disable=SC2034 # Variables sourced in other scripts. diff --git a/cluster/gce/config-test.sh b/cluster/gce/config-test.sh index 122d84992b8..e93c9a8e682 100755 --- a/cluster/gce/config-test.sh +++ b/cluster/gce/config-test.sh @@ -316,7 +316,7 @@ if [[ -n "${NODE_ACCELERATORS}" ]]; then fi # List of the set of feature gates recognized by the GCP CCM -export CCM_FEATURE_GATES="APIPriorityAndFairness,APIResponseCompression,APIServerIdentity,APIServerTracing,AllAlpha,AllBeta,CustomResourceValidationExpressions,KMSv2,OpenAPIEnums,OpenAPIV3,RemainingItemCount,ServerSideFieldValidation,StorageVersionAPI,StorageVersionHash" +export CCM_FEATURE_GATES="APIPriorityAndFairness,APIResponseCompression,APIServerIdentity,APIServerTracing,AllAlpha,AllBeta,CustomResourceValidationExpressions,KMSv2,OpenAPIEnums,OpenAPIV3,ServerSideFieldValidation,StorageVersionAPI,StorageVersionHash" # Optional: Install cluster DNS. # Set CLUSTER_DNS_CORE_DNS to 'false' to install kube-dns instead of CoreDNS. 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 857e65b4eb5..323f8eb7c9b 100644 --- a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go +++ b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go @@ -147,6 +147,7 @@ const ( // owner: @caesarxuchao // alpha: v1.15 // beta: v1.16 + // stable: 1.29 // // Allow apiservers to show a count of remaining items in the response // to a chunking list request. @@ -256,7 +257,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS OpenAPIV3: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.29 - RemainingItemCount: {Default: true, PreRelease: featuregate.Beta}, + RemainingItemCount: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.32 RemoveSelfLink: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, 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 7374152239c..6c24c1cb132 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go @@ -40,11 +40,9 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/watch" "k8s.io/apiserver/pkg/audit" - "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/storage" "k8s.io/apiserver/pkg/storage/etcd3/metrics" "k8s.io/apiserver/pkg/storage/value" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/component-base/tracing" "k8s.io/klog/v2" ) @@ -826,11 +824,9 @@ func (s *store) GetList(ctx context.Context, key string, opts storage.ListOption // 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 utilfeature.DefaultFeatureGate.Enabled(features.RemainingItemCount) { - if pred.Empty() { - c := int64(getResp.Count - pred.Limit) - remainingItemCount = &c - } + 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/testing/store_tests.go b/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go index aa0a8559d94..8b311f88908 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go @@ -36,11 +36,8 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/watch" "k8s.io/apiserver/pkg/apis/example" - "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/storage" "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" ) @@ -479,8 +476,6 @@ func RunTestPreconditionalDeleteWithSuggestion(ctx context.Context, t *testing.T } func RunTestList(ctx context.Context, t *testing.T, store storage.Interface, compaction Compaction, ignoreWatchCacheTests bool) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RemainingItemCount, true)() - initialRV, preset, err := seedMultiLevelData(ctx, store) if err != nil { t.Fatal(err) @@ -1113,8 +1108,6 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface, com } func RunTestListWithoutPaging(ctx context.Context, t *testing.T, store storage.Interface) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RemainingItemCount, true)() - _, preset, err := seedMultiLevelData(ctx, store) if err != nil { t.Fatal(err) diff --git a/test/e2e/apimachinery/chunking.go b/test/e2e/apimachinery/chunking.go index bd1536149bc..86d2ed05c62 100644 --- a/test/e2e/apimachinery/chunking.go +++ b/test/e2e/apimachinery/chunking.go @@ -30,18 +30,12 @@ import ( apierrors "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" admissionapi "k8s.io/pod-security-admission/api" ) -func shouldCheckRemainingItem() bool { - return utilfeature.DefaultFeatureGate.Enabled(features.RemainingItemCount) -} - const numberOfTotalResources = 400 var _ = SIGDescribe("Servers with support for API chunking", func() { @@ -96,13 +90,11 @@ var _ = SIGDescribe("Servers with support for API chunking", func() { lastRV = list.ResourceVersion } framework.ExpectEqual(list.ResourceVersion, lastRV) - 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)) - } + 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 { framework.ExpectEqual(item.Name, fmt.Sprintf("template-%04d", found)) @@ -136,13 +128,11 @@ 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 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)) - } + 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)) } framework.Logf("Retrieved %d/%d results with rv %s and continue %s", len(list.Items), opts.Limit, list.ResourceVersion, firstToken) @@ -179,13 +169,11 @@ var _ = SIGDescribe("Servers with support for API chunking", func() { gomega.Expect(len(list.Items)).To(gomega.BeNumerically("==", opts.Limit)) found := int(oneTenth) - 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)) - } + 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 { framework.ExpectEqual(item.Name, fmt.Sprintf("template-%04d", found)) @@ -198,13 +186,11 @@ var _ = SIGDescribe("Servers with support for API chunking", func() { for { list, err := client.List(ctx, opts) framework.ExpectNoError(err, "failed to list pod templates in namespace: %s, given limit: %d", ns, opts.Limit) - 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)) - } + 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)) } framework.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 f752ca2dc8487d8592d329a8a1014e86f2b1f8c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Wed, 23 Aug 2023 12:18:23 +0200 Subject: [PATCH 3/3] Make API chunking conformance test deterministic --- test/e2e/apimachinery/chunking.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/e2e/apimachinery/chunking.go b/test/e2e/apimachinery/chunking.go index 86d2ed05c62..32a7a6a5c5f 100644 --- a/test/e2e/apimachinery/chunking.go +++ b/test/e2e/apimachinery/chunking.go @@ -19,7 +19,6 @@ package apimachinery import ( "context" "fmt" - "math/rand" "reflect" "time" @@ -80,7 +79,9 @@ var _ = SIGDescribe("Servers with support for API chunking", func() { found := 0 var lastRV string for { - opts.Limit = int64(rand.Int31n(numberOfTotalResources/10) + 1) + // With numberOfTotalResources=400, we want to ensure that both + // number of items per page and number of pages are non-trivial. + opts.Limit = 17 list, err := client.List(ctx, opts) framework.ExpectNoError(err, "failed to list pod templates in namespace: %s, given limit: %d", ns, opts.Limit) framework.Logf("Retrieved %d/%d results with rv %s and continue %s", len(list.Items), opts.Limit, list.ResourceVersion, list.Continue)