From 4abc2b387b188d694e369e05c08effce9d23e7e7 Mon Sep 17 00:00:00 2001 From: chenk008 Date: Mon, 8 Apr 2024 11:09:27 +0800 Subject: [PATCH 1/3] Fix: StorageObjectCountTracker is nil, apf estimator got ObjectCountNotFoundErr --- pkg/controlplane/apiserver/config.go | 3 + pkg/controlplane/apiserver/config_test.go | 58 +++++++++++++++++++ .../apiserver/pkg/server/options/etcd.go | 2 +- .../apiserver/pkg/server/options/etcd_test.go | 15 +++++ 4 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 pkg/controlplane/apiserver/config_test.go diff --git a/pkg/controlplane/apiserver/config.go b/pkg/controlplane/apiserver/config.go index 5ffa9950e87..9b12fceb501 100644 --- a/pkg/controlplane/apiserver/config.go +++ b/pkg/controlplane/apiserver/config.go @@ -140,6 +140,9 @@ func BuildGenericConfig( if lastErr != nil { return } + // storageFactory.StorageConfig is copied from etcdOptions.StorageConfig, + // the StorageObjectCountTracker is still nil. Here we copy from genericConfig. + storageFactory.StorageConfig.StorageObjectCountTracker = genericConfig.StorageObjectCountTracker if lastErr = s.Etcd.ApplyWithStorageFactoryTo(storageFactory, genericConfig); lastErr != nil { return } diff --git a/pkg/controlplane/apiserver/config_test.go b/pkg/controlplane/apiserver/config_test.go new file mode 100644 index 00000000000..4496bb357cb --- /dev/null +++ b/pkg/controlplane/apiserver/config_test.go @@ -0,0 +1,58 @@ +package apiserver + +import ( + extensionsapiserver "k8s.io/apiextensions-apiserver/pkg/apiserver" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + apiserveroptions "k8s.io/apiserver/pkg/server/options" + aggregatorscheme "k8s.io/kube-aggregator/pkg/apiserver/scheme" + "k8s.io/kubernetes/pkg/api/legacyscheme" + "k8s.io/kubernetes/pkg/controlplane/apiserver/options" + generatedopenapi "k8s.io/kubernetes/pkg/generated/openapi" + netutils "k8s.io/utils/net" + "net" + "testing" +) + +func TestBuildGenericConfig(t *testing.T) { + opts := options.NewOptions() + s := (&apiserveroptions.SecureServingOptions{ + BindAddress: netutils.ParseIPSloppy("127.0.0.1"), + }).WithLoopback() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("failed to listen on 127.0.0.1:0") + } + defer ln.Close() + s.Listener = ln + s.BindPort = ln.Addr().(*net.TCPAddr).Port + opts.SecureServing = s + + completedOptions, err := opts.Complete(nil, nil) + if err != nil { + t.Fatalf("Failed to complete apiserver options: %v", err) + } + + genericConfig, _, storageFactory, err := BuildGenericConfig( + completedOptions, + []*runtime.Scheme{legacyscheme.Scheme, extensionsapiserver.Scheme, aggregatorscheme.Scheme}, + generatedopenapi.GetOpenAPIDefinitions, + ) + if err != nil { + t.Fatalf("Failed to build generic config: %v", err) + } + if genericConfig.StorageObjectCountTracker == nil { + t.Errorf("genericConfig StorageObjectCountTracker is absent") + } + if genericConfig.StorageObjectCountTracker != storageFactory.StorageConfig.StorageObjectCountTracker { + t.Errorf("There are different StorageObjectCountTracker in genericConfig and storageFactory") + } + + restOptions, err := genericConfig.RESTOptionsGetter.GetRESTOptions(schema.GroupResource{Group: "", Resource: ""}) + if err != nil { + t.Fatal(err) + } + if restOptions.StorageConfig.StorageObjectCountTracker != genericConfig.StorageObjectCountTracker { + t.Errorf("There are different StorageObjectCountTracker in restOptions and serverConfig") + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/etcd.go b/staging/src/k8s.io/apiserver/pkg/server/options/etcd.go index a1fc3168c5d..a3516cf8805 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/etcd.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/etcd.go @@ -396,7 +396,7 @@ func (f *StorageFactoryRestOptionsFactory) GetRESTOptions(resource schema.GroupR EnableGarbageCollection: f.Options.EnableGarbageCollection, ResourcePrefix: f.StorageFactory.ResourcePrefix(resource), CountMetricPollPeriod: f.Options.StorageConfig.CountMetricPollPeriod, - StorageObjectCountTracker: f.Options.StorageConfig.StorageObjectCountTracker, + StorageObjectCountTracker: storageConfig.StorageObjectCountTracker, } if f.Options.EnableWatchCache { diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/etcd_test.go b/staging/src/k8s.io/apiserver/pkg/server/options/etcd_test.go index f24077a349a..f5bf81f25b0 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/etcd_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/etcd_test.go @@ -430,3 +430,18 @@ func healthChecksAreEqual(t *testing.T, want []string, healthChecks []healthz.He t.Errorf("%s checks are not equal, missing=%q, extra=%q", checkerType, wantSet.Difference(gotSet).List(), gotSet.Difference(wantSet).List()) } } + +func TestRestOptionsStorageObjectCountTracker(t *testing.T) { + serverConfig := server.NewConfig(codecs) + etcdOptions := &EtcdOptions{} + if err := etcdOptions.ApplyTo(serverConfig); err != nil { + t.Fatalf("Failed to apply etcd options error: %v", err) + } + restOptions, err := serverConfig.RESTOptionsGetter.GetRESTOptions(schema.GroupResource{Group: "", Resource: ""}) + if err != nil { + t.Fatal(err) + } + if restOptions.StorageConfig.StorageObjectCountTracker != serverConfig.StorageObjectCountTracker { + t.Errorf("There are different StorageObjectCountTracker in restOptions and serverConfig") + } +} From 587ce02d90f3c1e1bb7418753009baf63f6039b7 Mon Sep 17 00:00:00 2001 From: chenk008 Date: Thu, 18 Apr 2024 23:02:16 +0800 Subject: [PATCH 2/3] prioritize user EtcdOptions.StorageConfig.StorageObjectCountTracker --- staging/src/k8s.io/apiserver/pkg/server/options/etcd.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/etcd.go b/staging/src/k8s.io/apiserver/pkg/server/options/etcd.go index a3516cf8805..10f9775efcc 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/etcd.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/etcd.go @@ -396,7 +396,11 @@ func (f *StorageFactoryRestOptionsFactory) GetRESTOptions(resource schema.GroupR EnableGarbageCollection: f.Options.EnableGarbageCollection, ResourcePrefix: f.StorageFactory.ResourcePrefix(resource), CountMetricPollPeriod: f.Options.StorageConfig.CountMetricPollPeriod, - StorageObjectCountTracker: storageConfig.StorageObjectCountTracker, + StorageObjectCountTracker: f.Options.StorageConfig.StorageObjectCountTracker, + } + + if ret.StorageObjectCountTracker == nil { + ret.StorageObjectCountTracker = storageConfig.StorageObjectCountTracker } if f.Options.EnableWatchCache { From c5d0e59d451306b96767630b0c40286aa9f8be29 Mon Sep 17 00:00:00 2001 From: chenk008 Date: Fri, 19 Apr 2024 11:11:21 +0800 Subject: [PATCH 3/3] Update pkg/controlplane/apiserver/config_test.go Co-authored-by: Paco Xu --- pkg/controlplane/apiserver/config_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/pkg/controlplane/apiserver/config_test.go b/pkg/controlplane/apiserver/config_test.go index 4496bb357cb..9065fcf889d 100644 --- a/pkg/controlplane/apiserver/config_test.go +++ b/pkg/controlplane/apiserver/config_test.go @@ -1,3 +1,19 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package apiserver import (