From 2dd44d6226efc6b72ae56b5fb81b5b2921cf3e16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Wed, 11 May 2022 19:02:51 +0200 Subject: [PATCH] Cleanup no-longer used storage cleanup method --- cmd/kube-apiserver/app/testing/testserver.go | 18 ++----- .../pkg/cmd/server/testing/testserver.go | 26 ++-------- .../generic/registry/storage_factory.go | 47 ------------------- .../controlplane/kube_apiserver_test.go | 15 +----- 4 files changed, 10 insertions(+), 96 deletions(-) diff --git a/cmd/kube-apiserver/app/testing/testserver.go b/cmd/kube-apiserver/app/testing/testserver.go index 156d2bdcb8b..2c6a8ee402b 100644 --- a/cmd/kube-apiserver/app/testing/testserver.go +++ b/cmd/kube-apiserver/app/testing/testserver.go @@ -35,7 +35,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/apiserver/pkg/registry/generic/registry" "k8s.io/apiserver/pkg/storage/storagebackend" "k8s.io/apiserver/pkg/storageversion" "k8s.io/client-go/kubernetes" @@ -59,8 +58,6 @@ type TearDownFunc func() // TestServerInstanceOptions Instance options the TestServer type TestServerInstanceOptions struct { - // DisableStorageCleanup Disable the automatic storage cleanup - DisableStorageCleanup bool // Enable cert-auth for the kube-apiserver EnableCertAuth bool // Wrap the storage version interface of the created server's generic server. @@ -87,8 +84,7 @@ type Logger interface { // NewDefaultTestServerOptions Default options for TestServer instances func NewDefaultTestServerOptions() *TestServerInstanceOptions { return &TestServerInstanceOptions{ - DisableStorageCleanup: false, - EnableCertAuth: true, + EnableCertAuth: true, } } @@ -103,18 +99,10 @@ func StartTestServer(t Logger, instanceOptions *TestServerInstanceOptions, custo instanceOptions = NewDefaultTestServerOptions() } - // TODO : Remove TrackStorageCleanup below when PR - // https://github.com/kubernetes/kubernetes/pull/50690 - // merges as that shuts down storage properly - if !instanceOptions.DisableStorageCleanup { - registry.TrackStorageCleanup() - } - stopCh := make(chan struct{}) tearDown := func() { - if !instanceOptions.DisableStorageCleanup { - registry.CleanupStorage() - } + // Closing stopCh is stopping apiserver and cleaning up + // after itself, including shutting down its storage layer. close(stopCh) if len(result.TmpDir) != 0 { os.RemoveAll(result.TmpDir) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/cmd/server/testing/testserver.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/cmd/server/testing/testserver.go index f49ac177f44..af15959a21e 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/cmd/server/testing/testserver.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/cmd/server/testing/testserver.go @@ -30,7 +30,6 @@ import ( "k8s.io/apiextensions-apiserver/pkg/cmd/server/options" "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/apiserver/pkg/registry/generic/registry" genericapiserver "k8s.io/apiserver/pkg/server" "k8s.io/apiserver/pkg/storage/storagebackend" "k8s.io/client-go/kubernetes" @@ -42,8 +41,6 @@ type TearDownFunc func() // TestServerInstanceOptions Instance options the TestServer type TestServerInstanceOptions struct { - // DisableStorageCleanup Disable the automatic storage cleanup - DisableStorageCleanup bool } // TestServer return values supplied by kube-test-ApiServer @@ -63,9 +60,7 @@ type Logger interface { // NewDefaultTestServerOptions Default options for TestServer instances func NewDefaultTestServerOptions() *TestServerInstanceOptions { - return &TestServerInstanceOptions{ - DisableStorageCleanup: false, - } + return &TestServerInstanceOptions{} } // StartTestServer starts a apiextensions-apiserver. A rest client config and a tear-down func, @@ -74,23 +69,12 @@ func NewDefaultTestServerOptions() *TestServerInstanceOptions { // Note: we return a tear-down func instead of a stop channel because the later will leak temporary // files that because Golang testing's call to os.Exit will not give a stop channel go routine // enough time to remove temporary files. -func StartTestServer(t Logger, instanceOptions *TestServerInstanceOptions, customFlags []string, storageConfig *storagebackend.Config) (result TestServer, err error) { - if instanceOptions == nil { - instanceOptions = NewDefaultTestServerOptions() - } - - // TODO : Remove TrackStorageCleanup below when PR - // https://github.com/kubernetes/kubernetes/pull/50690 - // merges as that shuts down storage properly - if !instanceOptions.DisableStorageCleanup { - registry.TrackStorageCleanup() - } - +func StartTestServer(t Logger, _ *TestServerInstanceOptions, customFlags []string, storageConfig *storagebackend.Config) (result TestServer, err error) { stopCh := make(chan struct{}) tearDown := func() { - if !instanceOptions.DisableStorageCleanup { - registry.CleanupStorage() - } + // Closing stopCh is stopping apiextensions apiserver and its + // delegates, which itself is cleaning up after itself, + // including shutting down its storage layer. close(stopCh) if len(result.TmpDir) != 0 { os.RemoveAll(result.TmpDir) diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/storage_factory.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/storage_factory.go index 075170f14e4..7fbd655e7c3 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/storage_factory.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/storage_factory.go @@ -78,11 +78,6 @@ func StorageWithCacher() generic.StorageDecorator { }) } - // TODO : Remove RegisterStorageCleanup below when PR - // https://github.com/kubernetes/kubernetes/pull/50690 - // merges as that shuts down storage properly - RegisterStorageCleanup(destroyFunc) - return cacher, destroyFunc, nil } } @@ -98,45 +93,3 @@ func objectTypeToArgs(obj runtime.Object) []interface{} { // otherwise just return the type return []interface{}{"type", fmt.Sprintf("%T", obj)} } - -// TODO : Remove all the code below when PR -// https://github.com/kubernetes/kubernetes/pull/50690 -// merges as that shuts down storage properly -// HACK ALERT : Track the destroy methods to call them -// from the test harness. TrackStorageCleanup will be called -// only from the test harness, so Register/Cleanup will be -// no-op at runtime. - -var cleanupLock sync.Mutex -var cleanup []func() = nil - -func TrackStorageCleanup() { - cleanupLock.Lock() - defer cleanupLock.Unlock() - - if cleanup != nil { - panic("Conflicting storage tracking") - } - cleanup = make([]func(), 0) -} - -func RegisterStorageCleanup(fn func()) { - cleanupLock.Lock() - defer cleanupLock.Unlock() - - if cleanup == nil { - return - } - cleanup = append(cleanup, fn) -} - -func CleanupStorage() { - cleanupLock.Lock() - old := cleanup - cleanup = nil - cleanupLock.Unlock() - - for _, d := range old { - d() - } -} diff --git a/test/integration/controlplane/kube_apiserver_test.go b/test/integration/controlplane/kube_apiserver_test.go index d3e7d00bdab..0658516aa70 100644 --- a/test/integration/controlplane/kube_apiserver_test.go +++ b/test/integration/controlplane/kube_apiserver_test.go @@ -39,7 +39,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/apiserver/pkg/registry/generic/registry" "k8s.io/client-go/kubernetes" "k8s.io/kube-aggregator/pkg/apis/apiregistration" "k8s.io/kube-openapi/pkg/validation/spec" @@ -439,12 +438,7 @@ func testReconcilersAPIServerLease(t *testing.T, leaseCount int, apiServerCount var apiServerCountServers = make([]*kubeapiservertesting.TestServer, apiServerCount) etcd := framework.SharedEtcd() - instanceOptions := &kubeapiservertesting.TestServerInstanceOptions{ - DisableStorageCleanup: true, - } - - // cleanup the registry storage - defer registry.CleanupStorage() + instanceOptions := kubeapiservertesting.NewDefaultTestServerOptions() wg := sync.WaitGroup{} // 1. start apiServerCount api servers @@ -542,12 +536,7 @@ func TestMultiAPIServerNodePortAllocation(t *testing.T) { var clientAPIServers []*kubernetes.Clientset etcd := framework.SharedEtcd() - instanceOptions := &kubeapiservertesting.TestServerInstanceOptions{ - DisableStorageCleanup: true, - } - - // cleanup the registry storage - defer registry.CleanupStorage() + instanceOptions := kubeapiservertesting.NewDefaultTestServerOptions() // create 2 api servers and 2 clients for i := 0; i < 2; i++ {