From f49cc5eb113b839a5f5c6121ff0b89f82a483011 Mon Sep 17 00:00:00 2001 From: HirazawaUi <695097494plus@gmail.com> Date: Wed, 3 May 2023 01:33:28 +0800 Subject: [PATCH 1/6] add remove_file in client-go util directory --- .../client-go/util/testing/remove_file.go | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 staging/src/k8s.io/client-go/util/testing/remove_file.go diff --git a/staging/src/k8s.io/client-go/util/testing/remove_file.go b/staging/src/k8s.io/client-go/util/testing/remove_file.go new file mode 100644 index 00000000000..976b9afafed --- /dev/null +++ b/staging/src/k8s.io/client-go/util/testing/remove_file.go @@ -0,0 +1,40 @@ +/* +Copyright 2023 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 testing + +import ( + "os" + "testing" +) + +// CloseAndRemove is a helper to close and remove test file. +func CloseAndRemove(t *testing.T, files ...*os.File) { + t.Helper() + // We should close it first before remove a file, it's not only a good practice, + // but also can avoid failed file removing on Windows OS. + for _, f := range files { + if f == nil { + continue + } + if err := f.Close(); err != nil { + t.Fatalf("Error closing %s: %v", f.Name(), err) + } + if err := os.Remove(f.Name()); err != nil { + t.Fatalf("Error removing %s: %v", f.Name(), err) + } + } +} From 73aeed8766c2c42a6cb4fc8632b1b974f4508dde Mon Sep 17 00:00:00 2001 From: HirazawaUi <695097494plus@gmail.com> Date: Wed, 3 May 2023 01:35:16 +0800 Subject: [PATCH 2/6] fix fd leaks and failed file removing for pkg client-go --- .../src/k8s.io/client-go/rest/request_test.go | 2 +- .../tools/clientcmd/api/helpers_test.go | 30 +++++--------- .../tools/clientcmd/client_config_test.go | 13 +++--- .../client-go/tools/clientcmd/loader_test.go | 40 ++++++++----------- .../tools/clientcmd/validation_test.go | 8 ++-- 5 files changed, 39 insertions(+), 54 deletions(-) diff --git a/staging/src/k8s.io/client-go/rest/request_test.go b/staging/src/k8s.io/client-go/rest/request_test.go index b4d6ea09af7..e26562bb065 100644 --- a/staging/src/k8s.io/client-go/rest/request_test.go +++ b/staging/src/k8s.io/client-go/rest/request_test.go @@ -322,7 +322,7 @@ func TestRequestBody(t *testing.T) { } // test error set when failing to read file - f, err := os.CreateTemp("", "test") + f, err := os.CreateTemp("", "") if err != nil { t.Fatalf("unable to create temp file") } diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/api/helpers_test.go b/staging/src/k8s.io/client-go/tools/clientcmd/api/helpers_test.go index 9cab27b1453..f823ef0f899 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/api/helpers_test.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/api/helpers_test.go @@ -23,6 +23,8 @@ import ( "reflect" "testing" + utiltesting "k8s.io/client-go/util/testing" + "sigs.k8s.io/yaml" ) @@ -53,11 +55,9 @@ func newMergedConfig(certFile, certContent, keyFile, keyContent, caFile, caConte func TestMinifySuccess(t *testing.T) { certFile, _ := os.CreateTemp("", "") - defer os.Remove(certFile.Name()) keyFile, _ := os.CreateTemp("", "") - defer os.Remove(keyFile.Name()) caFile, _ := os.CreateTemp("", "") - defer os.Remove(caFile.Name()) + defer utiltesting.CloseAndRemove(t, certFile, keyFile, caFile) mutatingConfig := newMergedConfig(certFile.Name(), "cert", keyFile.Name(), "key", caFile.Name(), "ca", t) @@ -89,11 +89,9 @@ func TestMinifySuccess(t *testing.T) { func TestMinifyMissingContext(t *testing.T) { certFile, _ := os.CreateTemp("", "") - defer os.Remove(certFile.Name()) keyFile, _ := os.CreateTemp("", "") - defer os.Remove(keyFile.Name()) caFile, _ := os.CreateTemp("", "") - defer os.Remove(caFile.Name()) + defer utiltesting.CloseAndRemove(t, certFile, keyFile, caFile) mutatingConfig := newMergedConfig(certFile.Name(), "cert", keyFile.Name(), "key", caFile.Name(), "ca", t) mutatingConfig.CurrentContext = "missing" @@ -107,11 +105,9 @@ func TestMinifyMissingContext(t *testing.T) { func TestMinifyMissingCluster(t *testing.T) { certFile, _ := os.CreateTemp("", "") - defer os.Remove(certFile.Name()) keyFile, _ := os.CreateTemp("", "") - defer os.Remove(keyFile.Name()) caFile, _ := os.CreateTemp("", "") - defer os.Remove(caFile.Name()) + defer utiltesting.CloseAndRemove(t, certFile, keyFile, caFile) mutatingConfig := newMergedConfig(certFile.Name(), "cert", keyFile.Name(), "key", caFile.Name(), "ca", t) delete(mutatingConfig.Clusters, mutatingConfig.Contexts[mutatingConfig.CurrentContext].Cluster) @@ -125,11 +121,9 @@ func TestMinifyMissingCluster(t *testing.T) { func TestMinifyMissingAuthInfo(t *testing.T) { certFile, _ := os.CreateTemp("", "") - defer os.Remove(certFile.Name()) keyFile, _ := os.CreateTemp("", "") - defer os.Remove(keyFile.Name()) caFile, _ := os.CreateTemp("", "") - defer os.Remove(caFile.Name()) + defer utiltesting.CloseAndRemove(t, certFile, keyFile, caFile) mutatingConfig := newMergedConfig(certFile.Name(), "cert", keyFile.Name(), "key", caFile.Name(), "ca", t) delete(mutatingConfig.AuthInfos, mutatingConfig.Contexts[mutatingConfig.CurrentContext].AuthInfo) @@ -143,11 +137,9 @@ func TestMinifyMissingAuthInfo(t *testing.T) { func TestFlattenSuccess(t *testing.T) { certFile, _ := os.CreateTemp("", "") - defer os.Remove(certFile.Name()) keyFile, _ := os.CreateTemp("", "") - defer os.Remove(keyFile.Name()) caFile, _ := os.CreateTemp("", "") - defer os.Remove(caFile.Name()) + defer utiltesting.CloseAndRemove(t, certFile, keyFile, caFile) certData := "cert" keyData := "key" @@ -208,11 +200,9 @@ func TestFlattenSuccess(t *testing.T) { func Example_minifyAndShorten() { certFile, _ := os.CreateTemp("", "") - defer os.Remove(certFile.Name()) keyFile, _ := os.CreateTemp("", "") - defer os.Remove(keyFile.Name()) caFile, _ := os.CreateTemp("", "") - defer os.Remove(caFile.Name()) + defer utiltesting.CloseAndRemove(&testing.T{}, certFile, keyFile, caFile) certData := "cert" keyData := "key" @@ -245,11 +235,9 @@ func Example_minifyAndShorten() { func TestShortenSuccess(t *testing.T) { certFile, _ := os.CreateTemp("", "") - defer os.Remove(certFile.Name()) keyFile, _ := os.CreateTemp("", "") - defer os.Remove(keyFile.Name()) caFile, _ := os.CreateTemp("", "") - defer os.Remove(caFile.Name()) + defer utiltesting.CloseAndRemove(t, certFile, keyFile, caFile) certData := "cert" keyData := "key" diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go b/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go index ee78d8e6b32..64efc901623 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go @@ -22,6 +22,8 @@ import ( "strings" "testing" + utiltesting "k8s.io/client-go/util/testing" + "github.com/imdario/mergo" "k8s.io/apimachinery/pkg/runtime" @@ -177,7 +179,7 @@ func TestCAOverridesCAData(t *testing.T) { if err != nil { t.Fatalf("could not create tempfile: %v", err) } - defer os.Remove(file.Name()) + defer utiltesting.CloseAndRemove(t, file) config := createCAValidTestConfig() clientBuilder := NewNonInteractiveClientConfig(*config, "clean", &ConfigOverrides{ @@ -312,8 +314,7 @@ func TestModifyContext(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - defer os.Remove(tempPath.Name()) - + defer utiltesting.CloseAndRemove(t, tempPath) pathOptions := NewDefaultPathOptions() config := createValidTestConfig() @@ -498,7 +499,7 @@ func TestBasicTokenFile(t *testing.T) { t.Errorf("Unexpected error: %v", err) return } - defer os.Remove(f.Name()) + defer utiltesting.CloseAndRemove(t, f) if err := os.WriteFile(f.Name(), []byte(token), 0644); err != nil { t.Errorf("Unexpected error: %v", err) return @@ -534,7 +535,7 @@ func TestPrecedenceTokenFile(t *testing.T) { t.Errorf("Unexpected error: %v", err) return } - defer os.Remove(f.Name()) + defer utiltesting.CloseAndRemove(t, f) if err := os.WriteFile(f.Name(), []byte(token), 0644); err != nil { t.Errorf("Unexpected error: %v", err) return @@ -923,7 +924,7 @@ users: if err != nil { t.Error(err) } - defer os.Remove(tmpfile.Name()) + defer utiltesting.CloseAndRemove(t, tmpfile) if err := os.WriteFile(tmpfile.Name(), []byte(content), 0666); err != nil { t.Error(err) } diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/loader_test.go b/staging/src/k8s.io/client-go/tools/clientcmd/loader_test.go index aa6741b8b5f..30e6161f5bc 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/loader_test.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/loader_test.go @@ -26,6 +26,8 @@ import ( "strings" "testing" + utiltesting "k8s.io/client-go/util/testing" + "github.com/google/go-cmp/cmp" "sigs.k8s.io/yaml" @@ -132,7 +134,7 @@ func TestToleratingMissingFiles(t *testing.T) { func TestErrorReadingFile(t *testing.T) { commandLineFile, _ := os.CreateTemp("", "") - defer os.Remove(commandLineFile.Name()) + defer utiltesting.CloseAndRemove(t, commandLineFile) if err := os.WriteFile(commandLineFile.Name(), []byte("bogus value"), 0644); err != nil { t.Fatalf("Error creating tempfile: %v", err) @@ -173,9 +175,8 @@ func TestErrorReadingNonFile(t *testing.T) { func TestConflictingCurrentContext(t *testing.T) { commandLineFile, _ := os.CreateTemp("", "") - defer os.Remove(commandLineFile.Name()) envVarFile, _ := os.CreateTemp("", "") - defer os.Remove(envVarFile.Name()) + defer utiltesting.CloseAndRemove(t, commandLineFile, envVarFile) mockCommandLineConfig := clientcmdapi.Config{ CurrentContext: "any-context-value", @@ -254,7 +255,7 @@ users: null func TestLoadingEmptyMaps(t *testing.T) { configFile, _ := os.CreateTemp("", "") - defer os.Remove(configFile.Name()) + defer utiltesting.CloseAndRemove(t, configFile) mockConfig := clientcmdapi.Config{ CurrentContext: "any-context-value", @@ -280,7 +281,7 @@ func TestLoadingEmptyMaps(t *testing.T) { func TestDuplicateClusterName(t *testing.T) { configFile, _ := os.CreateTemp("", "") - defer os.Remove(configFile.Name()) + defer utiltesting.CloseAndRemove(t, configFile) err := os.WriteFile(configFile.Name(), []byte(` kind: Config @@ -322,7 +323,7 @@ users: func TestDuplicateContextName(t *testing.T) { configFile, _ := os.CreateTemp("", "") - defer os.Remove(configFile.Name()) + defer utiltesting.CloseAndRemove(t, configFile) err := os.WriteFile(configFile.Name(), []byte(` kind: Config @@ -364,7 +365,7 @@ users: func TestDuplicateUserName(t *testing.T) { configFile, _ := os.CreateTemp("", "") - defer os.Remove(configFile.Name()) + defer utiltesting.CloseAndRemove(t, configFile) err := os.WriteFile(configFile.Name(), []byte(` kind: Config @@ -404,7 +405,7 @@ users: func TestDuplicateExtensionName(t *testing.T) { configFile, _ := os.CreateTemp("", "") - defer os.Remove(configFile.Name()) + defer utiltesting.CloseAndRemove(t, configFile) err := os.WriteFile(configFile.Name(), []byte(` kind: Config @@ -560,7 +561,7 @@ func TestResolveRelativePaths(t *testing.T) { func TestMigratingFile(t *testing.T) { sourceFile, _ := os.CreateTemp("", "") - defer os.Remove(sourceFile.Name()) + defer utiltesting.CloseAndRemove(t, sourceFile) destinationFile, _ := os.CreateTemp("", "") // delete the file so that we'll write to it os.Remove(destinationFile.Name()) @@ -574,9 +575,8 @@ func TestMigratingFile(t *testing.T) { if _, err := loadingRules.Load(); err != nil { t.Errorf("unexpected error %v", err) } - // the load should have recreated this file - defer os.Remove(destinationFile.Name()) + defer utiltesting.CloseAndRemove(t, destinationFile) sourceContent, err := os.ReadFile(sourceFile.Name()) if err != nil { @@ -594,9 +594,8 @@ func TestMigratingFile(t *testing.T) { func TestMigratingFileLeaveExistingFileAlone(t *testing.T) { sourceFile, _ := os.CreateTemp("", "") - defer os.Remove(sourceFile.Name()) destinationFile, _ := os.CreateTemp("", "") - defer os.Remove(destinationFile.Name()) + defer utiltesting.CloseAndRemove(t, sourceFile, destinationFile) WriteToFile(testConfigAlfa, sourceFile.Name()) @@ -622,7 +621,7 @@ func TestMigratingFileSourceMissingSkip(t *testing.T) { sourceFilename := "some-missing-file" destinationFile, _ := os.CreateTemp("", "") // delete the file so that we'll write to it - os.Remove(destinationFile.Name()) + utiltesting.CloseAndRemove(t, destinationFile) loadingRules := ClientConfigLoadingRules{ MigrationRules: map[string]string{destinationFile.Name(): sourceFilename}, @@ -639,7 +638,7 @@ func TestMigratingFileSourceMissingSkip(t *testing.T) { func TestFileLocking(t *testing.T) { f, _ := os.CreateTemp("", "") - defer os.Remove(f.Name()) + defer utiltesting.CloseAndRemove(t, f) err := lockFile(f.Name()) if err != nil { @@ -655,9 +654,8 @@ func TestFileLocking(t *testing.T) { func Example_noMergingOnExplicitPaths() { commandLineFile, _ := os.CreateTemp("", "") - defer os.Remove(commandLineFile.Name()) envVarFile, _ := os.CreateTemp("", "") - defer os.Remove(envVarFile.Name()) + defer utiltesting.CloseAndRemove(&testing.T{}, commandLineFile, envVarFile) WriteToFile(testConfigAlfa, commandLineFile.Name()) WriteToFile(testConfigConflictAlfa, envVarFile.Name()) @@ -704,9 +702,8 @@ func Example_noMergingOnExplicitPaths() { func Example_mergingSomeWithConflict() { commandLineFile, _ := os.CreateTemp("", "") - defer os.Remove(commandLineFile.Name()) envVarFile, _ := os.CreateTemp("", "") - defer os.Remove(envVarFile.Name()) + defer utiltesting.CloseAndRemove(&testing.T{}, commandLineFile, envVarFile) WriteToFile(testConfigAlfa, commandLineFile.Name()) WriteToFile(testConfigConflictAlfa, envVarFile.Name()) @@ -760,13 +757,10 @@ func Example_mergingSomeWithConflict() { func Example_mergingEverythingNoConflicts() { commandLineFile, _ := os.CreateTemp("", "") - defer os.Remove(commandLineFile.Name()) envVarFile, _ := os.CreateTemp("", "") - defer os.Remove(envVarFile.Name()) currentDirFile, _ := os.CreateTemp("", "") - defer os.Remove(currentDirFile.Name()) homeDirFile, _ := os.CreateTemp("", "") - defer os.Remove(homeDirFile.Name()) + defer utiltesting.CloseAndRemove(&testing.T{}, commandLineFile, envVarFile, currentDirFile, homeDirFile) WriteToFile(testConfigAlfa, commandLineFile.Name()) WriteToFile(testConfigBravo, envVarFile.Name()) diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/validation_test.go b/staging/src/k8s.io/client-go/tools/clientcmd/validation_test.go index 3c56498a44f..13296637099 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/validation_test.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/validation_test.go @@ -23,6 +23,8 @@ import ( "strings" "testing" + utiltesting "k8s.io/client-go/util/testing" + utilerrors "k8s.io/apimachinery/pkg/util/errors" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) @@ -296,7 +298,7 @@ func TestValidateCleanClusterInfo(t *testing.T) { func TestValidateCleanWithCAClusterInfo(t *testing.T) { tempFile, _ := os.CreateTemp("", "") - defer os.Remove(tempFile.Name()) + defer utiltesting.CloseAndRemove(t, tempFile) config := clientcmdapi.NewConfig() config.Clusters["clean"] = &clientcmdapi.Cluster{ @@ -339,7 +341,7 @@ func TestValidateCertFilesNotFoundAuthInfo(t *testing.T) { func TestValidateCertDataOverridesFiles(t *testing.T) { tempFile, _ := os.CreateTemp("", "") - defer os.Remove(tempFile.Name()) + defer utiltesting.CloseAndRemove(t, tempFile) config := clientcmdapi.NewConfig() config.AuthInfos["clean"] = &clientcmdapi.AuthInfo{ @@ -359,7 +361,7 @@ func TestValidateCertDataOverridesFiles(t *testing.T) { func TestValidateCleanCertFilesAuthInfo(t *testing.T) { tempFile, _ := os.CreateTemp("", "") - defer os.Remove(tempFile.Name()) + defer utiltesting.CloseAndRemove(t, tempFile) config := clientcmdapi.NewConfig() config.AuthInfos["clean"] = &clientcmdapi.AuthInfo{ From 982d2966cd33d79026a5d111dcb8bfeae62e657f Mon Sep 17 00:00:00 2001 From: HirazawaUi <695097494plus@gmail.com> Date: Wed, 3 May 2023 01:36:00 +0800 Subject: [PATCH 3/6] fix fd leaks and failed file removing for pkg controller-manager and apiserver --- .../apiserver/pkg/server/egressselector/config_test.go | 4 +++- .../src/k8s.io/apiserver/pkg/util/webhook/webhook_test.go | 5 +++++ .../plugin/pkg/authorizer/webhook/webhook_v1_test.go | 4 +++- .../pkg/leadermigration/config/config_test.go | 4 +++- .../pkg/leadermigration/options/options_test.go | 4 +++- 5 files changed, 17 insertions(+), 4 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/server/egressselector/config_test.go b/staging/src/k8s.io/apiserver/pkg/server/egressselector/config_test.go index b6b17f56592..4cbd940d4cc 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/egressselector/config_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/egressselector/config_test.go @@ -23,6 +23,8 @@ import ( "reflect" "testing" + utiltesting "k8s.io/client-go/util/testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apiserver/pkg/apis/apiserver" ) @@ -281,7 +283,7 @@ spec: if err != nil { t.Fatal(err) } - defer os.Remove(f.Name()) + defer utiltesting.CloseAndRemove(t, f) if err := ioutil.WriteFile(f.Name(), []byte(tc.contents), os.FileMode(0755)); err != nil { t.Fatal(err) } diff --git a/staging/src/k8s.io/apiserver/pkg/util/webhook/webhook_test.go b/staging/src/k8s.io/apiserver/pkg/util/webhook/webhook_test.go index 0857cc36cb8..cd0f6d69a09 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/webhook/webhook_test.go +++ b/staging/src/k8s.io/apiserver/pkg/util/webhook/webhook_test.go @@ -704,6 +704,7 @@ func bootstrapTestDir(t *testing.T) string { // Write the certificate files to disk or fail for fileName, fileData := range files { if err := ioutil.WriteFile(filepath.Join(dir, fileName), fileData, 0400); err != nil { + os.RemoveAll(dir) t.Fatal(err) } } @@ -713,6 +714,10 @@ func bootstrapTestDir(t *testing.T) string { func newKubeConfigFile(config v1.Config) (string, error) { configFile, err := ioutil.TempFile("", "") + if err != nil { + return "", err + } + defer configFile.Close() if err != nil { return "", fmt.Errorf("unable to create the Kubernetes client config file: %v", err) diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go index 6d21e40a2a1..52fa60b9e8d 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go @@ -34,6 +34,8 @@ import ( "text/template" "time" + utiltesting "k8s.io/client-go/util/testing" + "github.com/google/go-cmp/cmp" authorizationv1 "k8s.io/api/authorization/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -185,7 +187,7 @@ current-context: default return err } p := tempfile.Name() - defer os.Remove(p) + defer utiltesting.CloseAndRemove(t, tempfile) tmpl, err := template.New("test").Parse(tt.configTmpl) if err != nil { diff --git a/staging/src/k8s.io/controller-manager/pkg/leadermigration/config/config_test.go b/staging/src/k8s.io/controller-manager/pkg/leadermigration/config/config_test.go index 07b3c430478..de209698126 100644 --- a/staging/src/k8s.io/controller-manager/pkg/leadermigration/config/config_test.go +++ b/staging/src/k8s.io/controller-manager/pkg/leadermigration/config/config_test.go @@ -21,6 +21,8 @@ import ( "reflect" "testing" + utiltesting "k8s.io/client-go/util/testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" internal "k8s.io/controller-manager/config" ) @@ -117,7 +119,7 @@ controllerLeaders: if err != nil { t.Fatal(err) } - defer os.Remove(configFile.Name()) + defer utiltesting.CloseAndRemove(t, configFile) err = os.WriteFile(configFile.Name(), []byte(tc.content), os.FileMode(0755)) if err != nil { t.Fatal(err) diff --git a/staging/src/k8s.io/controller-manager/pkg/leadermigration/options/options_test.go b/staging/src/k8s.io/controller-manager/pkg/leadermigration/options/options_test.go index f78be12adc8..ebd57a5b402 100644 --- a/staging/src/k8s.io/controller-manager/pkg/leadermigration/options/options_test.go +++ b/staging/src/k8s.io/controller-manager/pkg/leadermigration/options/options_test.go @@ -21,6 +21,8 @@ import ( "reflect" "testing" + utiltesting "k8s.io/client-go/util/testing" + "github.com/spf13/pflag" "k8s.io/controller-manager/config" @@ -188,7 +190,7 @@ controllerLeaders: if err != nil { t.Fatal(err) } - defer os.Remove(configFile.Name()) + defer utiltesting.CloseAndRemove(t, configFile) err = os.WriteFile(configFile.Name(), []byte(tc.configContent), os.FileMode(0755)) if err != nil { t.Fatal(err) From b94c6daa0b7602e2d30e35cf2b94f9d1e8fde56e Mon Sep 17 00:00:00 2001 From: HirazawaUi <695097494plus@gmail.com> Date: Wed, 3 May 2023 01:36:46 +0800 Subject: [PATCH 4/6] fix fd leaks and failed file removing for pkg kubectl and pod-security-admission --- .../k8s.io/kubectl/pkg/cmd/config/config_test.go | 15 ++++++++------- .../pkg/cmd/config/current_context_test.go | 3 ++- .../kubectl/pkg/cmd/config/delete_cluster_test.go | 3 ++- .../kubectl/pkg/cmd/config/delete_context_test.go | 3 ++- .../kubectl/pkg/cmd/config/get_clusters_test.go | 4 +++- .../kubectl/pkg/cmd/config/get_contexts_test.go | 4 +++- .../kubectl/pkg/cmd/config/rename_context_test.go | 4 +++- .../kubectl/pkg/cmd/config/set_cluster_test.go | 4 +++- .../kubectl/pkg/cmd/config/set_context_test.go | 4 +++- .../pkg/cmd/config/set_credentials_test.go | 3 ++- .../src/k8s.io/kubectl/pkg/cmd/config/set_test.go | 4 +++- .../k8s.io/kubectl/pkg/cmd/config/unset_test.go | 4 +++- .../kubectl/pkg/cmd/config/use_context_test.go | 4 +++- .../k8s.io/kubectl/pkg/cmd/config/view_test.go | 4 +++- .../src/k8s.io/kubectl/pkg/cmd/testing/fake.go | 1 + .../admission/api/load/load_test.go | 5 +++-- 16 files changed, 47 insertions(+), 22 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/config/config_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/config/config_test.go index 9f5cc01c774..611b85e4dad 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/config/config_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/config/config_test.go @@ -29,6 +29,7 @@ import ( "k8s.io/cli-runtime/pkg/genericiooptions" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" + utiltesting "k8s.io/client-go/util/testing" cmdutil "k8s.io/kubectl/pkg/cmd/util" ) @@ -52,7 +53,7 @@ func Example_view() { expectedConfig: expectedConfig, } - output := test.run(nil) + output := test.run(&testing.T{}) fmt.Printf("%v", output) // Output: // apiVersion: v1 @@ -261,7 +262,7 @@ func TestAdditionalAuth(t *testing.T) { func TestEmbedClientCert(t *testing.T) { fakeCertFile, _ := os.CreateTemp(os.TempDir(), "") - defer os.Remove(fakeCertFile.Name()) + defer utiltesting.CloseAndRemove(t, fakeCertFile) fakeData := []byte("fake-data") os.WriteFile(fakeCertFile.Name(), fakeData, 0600) expectedConfig := newRedFederalCowHammerConfig() @@ -280,7 +281,7 @@ func TestEmbedClientCert(t *testing.T) { func TestEmbedClientKey(t *testing.T) { fakeKeyFile, _ := os.CreateTemp(os.TempDir(), "") - defer os.Remove(fakeKeyFile.Name()) + defer utiltesting.CloseAndRemove(t, fakeKeyFile) fakeData := []byte("fake-data") os.WriteFile(fakeKeyFile.Name(), fakeData, 0600) expectedConfig := newRedFederalCowHammerConfig() @@ -326,7 +327,7 @@ func TestEmbedNoKeyOrCertDisallowed(t *testing.T) { func TestEmptyTokenAndCertAllowed(t *testing.T) { fakeCertFile, _ := os.CreateTemp(os.TempDir(), "cert-file") - defer os.Remove(fakeCertFile.Name()) + defer utiltesting.CloseAndRemove(t, fakeCertFile) expectedConfig := newRedFederalCowHammerConfig() authInfo := clientcmdapi.NewAuthInfo() authInfo.ClientCertificate = path.Base(fakeCertFile.Name()) @@ -569,7 +570,7 @@ func TestUnsetBytes(t *testing.T) { func TestCAClearsInsecure(t *testing.T) { fakeCAFile, _ := os.CreateTemp(os.TempDir(), "ca-file") - defer os.Remove(fakeCAFile.Name()) + defer utiltesting.CloseAndRemove(t, fakeCAFile) clusterInfoWithInsecure := clientcmdapi.NewCluster() clusterInfoWithInsecure.InsecureSkipTLSVerify = true @@ -638,7 +639,7 @@ func TestInsecureClearsCA(t *testing.T) { func TestCADataClearsCA(t *testing.T) { fakeCAFile, _ := os.CreateTemp(os.TempDir(), "") - defer os.Remove(fakeCAFile.Name()) + defer utiltesting.CloseAndRemove(t, fakeCAFile) fakeData := []byte("cadata") os.WriteFile(fakeCAFile.Name(), fakeData, 0600) @@ -852,7 +853,7 @@ func TestToBool(t *testing.T) { func testConfigCommand(args []string, startingConfig clientcmdapi.Config, t *testing.T) (string, clientcmdapi.Config) { fakeKubeFile, _ := os.CreateTemp(os.TempDir(), "") - defer os.Remove(fakeKubeFile.Name()) + defer utiltesting.CloseAndRemove(t, fakeKubeFile) err := clientcmd.WriteToFile(startingConfig, fakeKubeFile.Name()) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/config/current_context_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/config/current_context_test.go index e545d36495e..617a3668a80 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/config/current_context_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/config/current_context_test.go @@ -18,6 +18,7 @@ package config import ( "bytes" + utiltesting "k8s.io/client-go/util/testing" "os" "strings" "testing" @@ -60,7 +61,7 @@ func (test currentContextTest) run(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - defer os.Remove(fakeKubeFile.Name()) + defer utiltesting.CloseAndRemove(t, fakeKubeFile) err = clientcmd.WriteToFile(test.startingConfig, fakeKubeFile.Name()) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/config/delete_cluster_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/config/delete_cluster_test.go index 69d4a21d1ff..d98bd3a22f2 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/config/delete_cluster_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/config/delete_cluster_test.go @@ -19,6 +19,7 @@ package config import ( "bytes" "fmt" + utiltesting "k8s.io/client-go/util/testing" "os" "reflect" "testing" @@ -56,7 +57,7 @@ func (test deleteClusterTest) run(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - defer os.Remove(fakeKubeFile.Name()) + defer utiltesting.CloseAndRemove(t, fakeKubeFile) err = clientcmd.WriteToFile(test.config, fakeKubeFile.Name()) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/config/delete_context_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/config/delete_context_test.go index 26fd1a48f13..9492cf72cc3 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/config/delete_context_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/config/delete_context_test.go @@ -19,6 +19,7 @@ package config import ( "bytes" "fmt" + utiltesting "k8s.io/client-go/util/testing" "os" "reflect" "testing" @@ -56,7 +57,7 @@ func (test deleteContextTest) run(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - defer os.Remove(fakeKubeFile.Name()) + defer utiltesting.CloseAndRemove(t, fakeKubeFile) err = clientcmd.WriteToFile(test.config, fakeKubeFile.Name()) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/config/get_clusters_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/config/get_clusters_test.go index 52ff8e67d1f..7aeb94ce5d0 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/config/get_clusters_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/config/get_clusters_test.go @@ -21,6 +21,8 @@ import ( "os" "testing" + utiltesting "k8s.io/client-go/util/testing" + "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) @@ -60,7 +62,7 @@ func (test getClustersTest) run(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - defer os.Remove(fakeKubeFile.Name()) + defer utiltesting.CloseAndRemove(t, fakeKubeFile) err = clientcmd.WriteToFile(test.config, fakeKubeFile.Name()) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/config/get_contexts_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/config/get_contexts_test.go index 5bff7a5bd1a..96b31033680 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/config/get_contexts_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/config/get_contexts_test.go @@ -20,6 +20,8 @@ import ( "os" "testing" + utiltesting "k8s.io/client-go/util/testing" + "k8s.io/cli-runtime/pkg/genericiooptions" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" @@ -147,7 +149,7 @@ func (test getContextsTest) run(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - defer os.Remove(fakeKubeFile.Name()) + defer utiltesting.CloseAndRemove(t, fakeKubeFile) err = clientcmd.WriteToFile(test.startingConfig, fakeKubeFile.Name()) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/config/rename_context_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/config/rename_context_test.go index 6c40408f671..b084f742f67 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/config/rename_context_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/config/rename_context_test.go @@ -23,6 +23,8 @@ import ( "strings" "testing" + utiltesting "k8s.io/client-go/util/testing" + "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) @@ -103,7 +105,7 @@ func TestRenameToAlreadyExistingContext(t *testing.T) { func (test renameContextTest) run(t *testing.T) { fakeKubeFile, _ := os.CreateTemp(os.TempDir(), "") - defer os.Remove(fakeKubeFile.Name()) + defer utiltesting.CloseAndRemove(t, fakeKubeFile) err := clientcmd.WriteToFile(test.initialConfig, fakeKubeFile.Name()) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/config/set_cluster_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/config/set_cluster_test.go index d19818419ce..d33feca0a0b 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/config/set_cluster_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/config/set_cluster_test.go @@ -21,6 +21,8 @@ import ( "os" "testing" + utiltesting "k8s.io/client-go/util/testing" + "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) @@ -186,7 +188,7 @@ func (test setClusterTest) run(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - defer os.Remove(fakeKubeFile.Name()) + defer utiltesting.CloseAndRemove(t, fakeKubeFile) err = clientcmd.WriteToFile(test.config, fakeKubeFile.Name()) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/config/set_context_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/config/set_context_test.go index 3f08a7b3c43..1aaeffe1223 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/config/set_context_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/config/set_context_test.go @@ -21,6 +21,8 @@ import ( "os" "testing" + utiltesting "k8s.io/client-go/util/testing" + "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) @@ -110,7 +112,7 @@ func (test setContextTest) run(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - defer os.Remove(fakeKubeFile.Name()) + defer utiltesting.CloseAndRemove(t, fakeKubeFile) err = clientcmd.WriteToFile(test.config, fakeKubeFile.Name()) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/config/set_credentials_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/config/set_credentials_test.go index 60c52611677..58c95a4a26e 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/config/set_credentials_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/config/set_credentials_test.go @@ -18,6 +18,7 @@ package config import ( "bytes" + utiltesting "k8s.io/client-go/util/testing" "os" "reflect" "testing" @@ -457,7 +458,7 @@ func (test setCredentialsTest) run(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - defer os.Remove(fakeKubeFile.Name()) + defer utiltesting.CloseAndRemove(t, fakeKubeFile) err = clientcmd.WriteToFile(test.config, fakeKubeFile.Name()) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/config/set_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/config/set_test.go index a22561a49ef..2ab35252eec 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/config/set_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/config/set_test.go @@ -21,6 +21,8 @@ import ( "os" "testing" + utiltesting "k8s.io/client-go/util/testing" + "reflect" "k8s.io/client-go/tools/clientcmd" @@ -58,7 +60,7 @@ func (test setConfigTest) run(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - defer os.Remove(fakeKubeFile.Name()) + defer utiltesting.CloseAndRemove(t, fakeKubeFile) err = clientcmd.WriteToFile(test.config, fakeKubeFile.Name()) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/config/unset_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/config/unset_test.go index 0a6a2f79f94..de26ac2b8a2 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/config/unset_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/config/unset_test.go @@ -21,6 +21,8 @@ import ( "os" "testing" + utiltesting "k8s.io/client-go/util/testing" + "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) @@ -109,7 +111,7 @@ func (test unsetConfigTest) run(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - defer os.Remove(fakeKubeFile.Name()) + defer utiltesting.CloseAndRemove(t, fakeKubeFile) err = clientcmd.WriteToFile(test.config, fakeKubeFile.Name()) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/config/use_context_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/config/use_context_test.go index f36e9058215..d8fd7cc1010 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/config/use_context_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/config/use_context_test.go @@ -21,6 +21,8 @@ import ( "os" "testing" + utiltesting "k8s.io/client-go/util/testing" + "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) @@ -74,7 +76,7 @@ func (test useContextTest) run(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - defer os.Remove(fakeKubeFile.Name()) + defer utiltesting.CloseAndRemove(t, fakeKubeFile) err = clientcmd.WriteToFile(test.config, fakeKubeFile.Name()) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/config/view_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/config/view_test.go index 1421f72fc16..ddc12945a2d 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/config/view_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/config/view_test.go @@ -20,6 +20,8 @@ import ( "os" "testing" + utiltesting "k8s.io/client-go/util/testing" + "k8s.io/cli-runtime/pkg/genericiooptions" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" @@ -297,7 +299,7 @@ func (test viewClusterTest) run(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - defer os.Remove(fakeKubeFile.Name()) + defer utiltesting.CloseAndRemove(t, fakeKubeFile) err = clientcmd.WriteToFile(test.config, fakeKubeFile.Name()) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/testing/fake.go b/staging/src/k8s.io/kubectl/pkg/cmd/testing/fake.go index cde7f19ef1e..b10d1739371 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/testing/fake.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/testing/fake.go @@ -483,6 +483,7 @@ func (f *TestFactory) Cleanup() { return } + f.tempConfigFile.Close() os.Remove(f.tempConfigFile.Name()) } diff --git a/staging/src/k8s.io/pod-security-admission/admission/api/load/load_test.go b/staging/src/k8s.io/pod-security-admission/admission/api/load/load_test.go index bf0d9da6382..e5a5356493e 100644 --- a/staging/src/k8s.io/pod-security-admission/admission/api/load/load_test.go +++ b/staging/src/k8s.io/pod-security-admission/admission/api/load/load_test.go @@ -19,11 +19,12 @@ package load import ( "bytes" "io/ioutil" - "os" "reflect" "strings" "testing" + utiltesting "k8s.io/client-go/util/testing" + "github.com/google/go-cmp/cmp" "k8s.io/pod-security-admission/admission/api" @@ -44,7 +45,7 @@ func writeTempFile(t *testing.T, content string) string { t.Fatal(err) } t.Cleanup(func() { - os.Remove(file.Name()) + utiltesting.CloseAndRemove(t, file) }) if err := ioutil.WriteFile(file.Name(), []byte(content), 0600); err != nil { t.Fatal(err) From 5289a7b029fe26e309131bf80e554a2f70cbec19 Mon Sep 17 00:00:00 2001 From: HirazawaUi <695097494plus@gmail.com> Date: Wed, 3 May 2023 01:37:20 +0800 Subject: [PATCH 5/6] fix fd leaks and failed file removing for test directory --- test/e2e/apimachinery/certs.go | 8 +++++++- test/e2e_node/services/apiserver.go | 5 ++++- .../apiserver/admissionwebhook/client_auth_test.go | 6 ++++-- .../apiserver/admissionwebhook/reinvocation_test.go | 3 ++- test/integration/apiserver/tracing/tracing_test.go | 3 ++- test/integration/auth/auth_test.go | 4 +++- test/integration/auth/dynamic_client_test.go | 4 +++- test/integration/controlplane/audit/audit_test.go | 3 ++- test/integration/etcd/server.go | 4 +++- test/integration/examples/apiserver_test.go | 1 + test/integration/framework/test_server.go | 5 +++-- 11 files changed, 34 insertions(+), 12 deletions(-) diff --git a/test/e2e/apimachinery/certs.go b/test/e2e/apimachinery/certs.go index 4f18333a82a..cc3db706686 100644 --- a/test/e2e/apimachinery/certs.go +++ b/test/e2e/apimachinery/certs.go @@ -19,11 +19,14 @@ package apimachinery import ( "crypto/x509" "os" + "testing" + + utiltesting "k8s.io/client-go/util/testing" + "k8s.io/kubernetes/test/utils" "k8s.io/client-go/util/cert" "k8s.io/client-go/util/keyutil" "k8s.io/kubernetes/test/e2e/framework" - "k8s.io/kubernetes/test/utils" ) type certContext struct { @@ -52,6 +55,7 @@ func setupServerCert(namespaceName, serviceName string) *certContext { if err != nil { framework.Failf("Failed to create a temp file for ca cert generation %v", err) } + defer utiltesting.CloseAndRemove(&testing.T{}, caCertFile) if err := os.WriteFile(caCertFile.Name(), utils.EncodeCertPEM(signingCert), 0644); err != nil { framework.Failf("Failed to write CA cert %v", err) } @@ -74,6 +78,7 @@ func setupServerCert(namespaceName, serviceName string) *certContext { if err != nil { framework.Failf("Failed to create a temp file for cert generation %v", err) } + defer utiltesting.CloseAndRemove(&testing.T{}, certFile) keyFile, err := os.CreateTemp(certDir, "server.key") if err != nil { framework.Failf("Failed to create a temp file for key generation %v", err) @@ -88,6 +93,7 @@ func setupServerCert(namespaceName, serviceName string) *certContext { if err = os.WriteFile(keyFile.Name(), privateKeyPEM, 0644); err != nil { framework.Failf("Failed to write key file %v", err) } + defer utiltesting.CloseAndRemove(&testing.T{}, keyFile) return &certContext{ cert: utils.EncodeCertPEM(signedCert), key: privateKeyPEM, diff --git a/test/e2e_node/services/apiserver.go b/test/e2e_node/services/apiserver.go index 98d5ce9f52d..957f594df21 100644 --- a/test/e2e_node/services/apiserver.go +++ b/test/e2e_node/services/apiserver.go @@ -19,6 +19,9 @@ package services import ( "fmt" "os" + "testing" + + utiltesting "k8s.io/client-go/util/testing" "k8s.io/apiserver/pkg/storage/storagebackend" netutils "k8s.io/utils/net" @@ -79,7 +82,7 @@ func (a *APIServer) Start() error { if err != nil { return fmt.Errorf("create temp file failed: %w", err) } - defer os.RemoveAll(saSigningKeyFile.Name()) + defer utiltesting.CloseAndRemove(&testing.T{}, saSigningKeyFile) if err = os.WriteFile(saSigningKeyFile.Name(), []byte(ecdsaPrivateKey), 0666); err != nil { return fmt.Errorf("write file %s failed: %w", saSigningKeyFile.Name(), err) } diff --git a/test/integration/apiserver/admissionwebhook/client_auth_test.go b/test/integration/apiserver/admissionwebhook/client_auth_test.go index 816a373eab3..8e0315a9d3a 100644 --- a/test/integration/apiserver/admissionwebhook/client_auth_test.go +++ b/test/integration/apiserver/admissionwebhook/client_auth_test.go @@ -31,6 +31,8 @@ import ( "testing" "time" + utiltesting "k8s.io/client-go/util/testing" + "k8s.io/api/admission/v1beta1" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" corev1 "k8s.io/api/core/v1" @@ -87,7 +89,7 @@ func testWebhookClientAuth(t *testing.T, enableAggregatorRouting bool) { if err != nil { t.Fatal(err) } - defer os.Remove(kubeConfigFile.Name()) + defer utiltesting.CloseAndRemove(t, kubeConfigFile) if err := os.WriteFile(kubeConfigFile.Name(), []byte(` apiVersion: v1 @@ -113,7 +115,7 @@ users: if err != nil { t.Fatal(err) } - defer os.Remove(admissionConfigFile.Name()) + defer utiltesting.CloseAndRemove(t, admissionConfigFile) if err := os.WriteFile(admissionConfigFile.Name(), []byte(` apiVersion: apiserver.k8s.io/v1alpha1 diff --git a/test/integration/apiserver/admissionwebhook/reinvocation_test.go b/test/integration/apiserver/admissionwebhook/reinvocation_test.go index 61051f1101c..9d8f6ca163c 100644 --- a/test/integration/apiserver/admissionwebhook/reinvocation_test.go +++ b/test/integration/apiserver/admissionwebhook/reinvocation_test.go @@ -44,6 +44,7 @@ import ( auditv1 "k8s.io/apiserver/pkg/apis/audit/v1" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" + utiltesting "k8s.io/client-go/util/testing" kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" "k8s.io/kubernetes/test/integration/framework" "k8s.io/kubernetes/test/utils" @@ -281,7 +282,7 @@ func testWebhookReinvocationPolicy(t *testing.T, watchCache bool) { if err != nil { t.Fatalf("Failed to create audit log file: %v", err) } - defer os.Remove(logFile.Name()) + defer utiltesting.CloseAndRemove(t, logFile) s := kubeapiservertesting.StartTestServerOrDie(t, kubeapiservertesting.NewDefaultTestServerOptions(), []string{ "--disable-admission-plugins=ServiceAccount", diff --git a/test/integration/apiserver/tracing/tracing_test.go b/test/integration/apiserver/tracing/tracing_test.go index d284de19857..23594843a8f 100644 --- a/test/integration/apiserver/tracing/tracing_test.go +++ b/test/integration/apiserver/tracing/tracing_test.go @@ -38,6 +38,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/strategicpatch" client "k8s.io/client-go/kubernetes" + utiltesting "k8s.io/client-go/util/testing" kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" "k8s.io/kubernetes/test/integration/framework" ) @@ -73,7 +74,7 @@ egressSelections: if err != nil { t.Fatal(err) } - defer os.Remove(tracingConfigFile.Name()) + defer utiltesting.CloseAndRemove(t, tracingConfigFile) if err := os.WriteFile(tracingConfigFile.Name(), []byte(fmt.Sprintf(` apiVersion: apiserver.config.k8s.io/v1beta1 diff --git a/test/integration/auth/auth_test.go b/test/integration/auth/auth_test.go index 674f98dffd7..7412aa84b01 100644 --- a/test/integration/auth/auth_test.go +++ b/test/integration/auth/auth_test.go @@ -41,6 +41,8 @@ import ( "testing" "time" + utiltesting "k8s.io/client-go/util/testing" + "github.com/google/go-cmp/cmp" authenticationv1beta1 "k8s.io/api/authentication/v1beta1" @@ -86,7 +88,7 @@ func getTestWebhookTokenAuth(serverURL string, customDial utilnet.DialFunc) (aut if err != nil { return nil, err } - defer os.Remove(kubecfgFile.Name()) + defer utiltesting.CloseAndRemove(&testing.T{}, kubecfgFile) config := v1.Config{ Clusters: []v1.NamedCluster{ { diff --git a/test/integration/auth/dynamic_client_test.go b/test/integration/auth/dynamic_client_test.go index be862869d0b..90c5454105a 100644 --- a/test/integration/auth/dynamic_client_test.go +++ b/test/integration/auth/dynamic_client_test.go @@ -22,6 +22,8 @@ import ( "testing" "time" + utiltesting "k8s.io/client-go/util/testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apiserver/pkg/authentication/authenticator" clientset "k8s.io/client-go/kubernetes" @@ -38,7 +40,7 @@ func TestDynamicClientBuilder(t *testing.T) { if err != nil { t.Fatalf("create temp file failed: %v", err) } - defer os.RemoveAll(tmpfile.Name()) + defer utiltesting.CloseAndRemove(t, tmpfile) if err = os.WriteFile(tmpfile.Name(), []byte(ecdsaPrivateKey), 0666); err != nil { t.Fatalf("write file %s failed: %v", tmpfile.Name(), err) diff --git a/test/integration/controlplane/audit/audit_test.go b/test/integration/controlplane/audit/audit_test.go index b934a9ec444..316f152a6c9 100644 --- a/test/integration/controlplane/audit/audit_test.go +++ b/test/integration/controlplane/audit/audit_test.go @@ -41,6 +41,7 @@ import ( auditinternal "k8s.io/apiserver/pkg/apis/audit" auditv1 "k8s.io/apiserver/pkg/apis/audit/v1" clientset "k8s.io/client-go/kubernetes" + utiltesting "k8s.io/client-go/util/testing" kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" "k8s.io/kubernetes/test/integration/framework" "k8s.io/kubernetes/test/utils" @@ -245,7 +246,7 @@ func runTestWithVersion(t *testing.T, version string) { if err != nil { t.Fatalf("Failed to create audit log file: %v", err) } - defer os.Remove(logFile.Name()) + defer utiltesting.CloseAndRemove(t, logFile) // start api server result := kubeapiservertesting.StartTestServerOrDie(t, nil, diff --git a/test/integration/etcd/server.go b/test/integration/etcd/server.go index f8c9455eb10..34bab086199 100644 --- a/test/integration/etcd/server.go +++ b/test/integration/etcd/server.go @@ -26,6 +26,8 @@ import ( "testing" "time" + utiltesting "k8s.io/client-go/util/testing" + clientv3 "go.etcd.io/etcd/client/v3" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -81,7 +83,7 @@ func StartRealAPIServerOrDie(t *testing.T, configFuncs ...func(*options.ServerRu if err != nil { t.Fatalf("create temp file failed: %v", err) } - defer os.RemoveAll(saSigningKeyFile.Name()) + defer utiltesting.CloseAndRemove(t, saSigningKeyFile) if err = os.WriteFile(saSigningKeyFile.Name(), []byte(ecdsaPrivateKey), 0666); err != nil { t.Fatalf("write file %s failed: %v", saSigningKeyFile.Name(), err) } diff --git a/test/integration/examples/apiserver_test.go b/test/integration/examples/apiserver_test.go index 0ce4944f65e..6adfd47ec36 100644 --- a/test/integration/examples/apiserver_test.go +++ b/test/integration/examples/apiserver_test.go @@ -522,6 +522,7 @@ func writeKubeConfigForWardleServerToKASConnection(t *testing.T, kubeClientConfi t.Fatal(err) } + defer wardleToKASKubeConfigFile.Close() return wardleToKASKubeConfigFile.Name() } diff --git a/test/integration/framework/test_server.go b/test/integration/framework/test_server.go index ce1599ea544..931c5a4a573 100644 --- a/test/integration/framework/test_server.go +++ b/test/integration/framework/test_server.go @@ -99,6 +99,7 @@ func StartTestServer(ctx context.Context, t testing.TB, setup TestServerSetup) ( if err := os.WriteFile(proxyCACertFile.Name(), utils.EncodeCertPEM(proxySigningCert), 0644); err != nil { t.Fatal(err) } + defer proxyCACertFile.Close() clientSigningKey, err := utils.NewPrivateKey() if err != nil { t.Fatal(err) @@ -111,7 +112,7 @@ func StartTestServer(ctx context.Context, t testing.TB, setup TestServerSetup) ( if err := os.WriteFile(clientCACertFile.Name(), utils.EncodeCertPEM(clientSigningCert), 0644); err != nil { t.Fatal(err) } - + defer clientCACertFile.Close() listener, _, err := genericapiserveroptions.CreateListener("tcp", "127.0.0.1:0", net.ListenConfig{}) if err != nil { t.Fatal(err) @@ -121,7 +122,7 @@ func StartTestServer(ctx context.Context, t testing.TB, setup TestServerSetup) ( if err != nil { t.Fatalf("create temp file failed: %v", err) } - defer os.RemoveAll(saSigningKeyFile.Name()) + defer saSigningKeyFile.Close() if err = os.WriteFile(saSigningKeyFile.Name(), []byte(ecdsaPrivateKey), 0666); err != nil { t.Fatalf("write file %s failed: %v", saSigningKeyFile.Name(), err) } From 82e3fa0930eaf88e9c8fb639d96523c8d91d97cf Mon Sep 17 00:00:00 2001 From: HirazawaUi <695097494plus@gmail.com> Date: Wed, 3 May 2023 01:37:53 +0800 Subject: [PATCH 6/6] fix fd leaks and failed file removing for main pkg and cmd --- cmd/kubeadm/app/preflight/checks_test.go | 18 ++++++----- cmd/kubeadm/app/util/patches/patches_test.go | 4 ++- pkg/credentialprovider/plugin/config_test.go | 4 ++- .../certificate/bootstrap/bootstrap_test.go | 6 ++-- pkg/kubelet/kuberuntime/logs/logs_test.go | 4 ++- .../admission/imagepolicy/admission_test.go | 32 +++++++++---------- 6 files changed, 39 insertions(+), 29 deletions(-) diff --git a/cmd/kubeadm/app/preflight/checks_test.go b/cmd/kubeadm/app/preflight/checks_test.go index 52c1a125f6d..b5927fa0f7c 100644 --- a/cmd/kubeadm/app/preflight/checks_test.go +++ b/cmd/kubeadm/app/preflight/checks_test.go @@ -26,6 +26,8 @@ import ( "strings" "testing" + utiltesting "k8s.io/client-go/util/testing" + "github.com/google/go-cmp/cmp" "github.com/lithammer/dedent" "github.com/pkg/errors" @@ -195,7 +197,7 @@ func TestFileExistingCheck(t *testing.T) { if err != nil { t.Fatalf("Failed to create file: %v", err) } - defer os.Remove(f.Name()) + defer utiltesting.CloseAndRemove(t, f) var tests = []struct { name string check FileExistingCheck @@ -234,7 +236,7 @@ func TestFileAvailableCheck(t *testing.T) { if err != nil { t.Fatalf("Failed to create file: %v", err) } - defer os.Remove(f.Name()) + defer utiltesting.CloseAndRemove(t, f) var tests = []struct { name string check FileAvailableCheck @@ -461,8 +463,8 @@ func TestConfigRootCAs(t *testing.T) { if err != nil { t.Errorf("failed configRootCAs:\n\texpected: succeed creating temp CA file\n\tactual:%v", err) } - defer os.Remove(f.Name()) - if err := os.WriteFile(f.Name(), []byte(externalEtcdRootCAFileContent), 0644); err != nil { + defer utiltesting.CloseAndRemove(t, f) + if _, err := f.Write([]byte(externalEtcdRootCAFileContent)); err != nil { t.Errorf("failed configRootCAs:\n\texpected: succeed writing contents to temp CA file %s\n\tactual:%v", f.Name(), err) } @@ -490,8 +492,8 @@ func TestConfigCertAndKey(t *testing.T) { err, ) } - defer os.Remove(certFile.Name()) - if err := os.WriteFile(certFile.Name(), []byte(externalEtcdCertFileContent), 0644); err != nil { + defer utiltesting.CloseAndRemove(t, certFile) + if _, err := certFile.Write([]byte(externalEtcdCertFileContent)); err != nil { t.Errorf( "failed configCertAndKey:\n\texpected: succeed writing contents to temp CertFile file %s\n\tactual:%v", certFile.Name(), @@ -506,8 +508,8 @@ func TestConfigCertAndKey(t *testing.T) { err, ) } - defer os.Remove(keyFile.Name()) - if err := os.WriteFile(keyFile.Name(), []byte(externalEtcdKeyFileContent), 0644); err != nil { + defer utiltesting.CloseAndRemove(t, keyFile) + if _, err := keyFile.Write([]byte(externalEtcdKeyFileContent)); err != nil { t.Errorf( "failed configCertAndKey:\n\texpected: succeed writing contents to temp KeyFile file %s\n\tactual:%v", keyFile.Name(), diff --git a/cmd/kubeadm/app/util/patches/patches_test.go b/cmd/kubeadm/app/util/patches/patches_test.go index b704c820928..5213c5f7f70 100644 --- a/cmd/kubeadm/app/util/patches/patches_test.go +++ b/cmd/kubeadm/app/util/patches/patches_test.go @@ -24,6 +24,8 @@ import ( "reflect" "testing" + utiltesting "k8s.io/client-go/util/testing" + "github.com/pkg/errors" v1 "k8s.io/api/core/v1" @@ -172,7 +174,7 @@ func TestGetPatchSetsForPathMustBeDirectory(t *testing.T) { if err != nil { t.Errorf("error creating temporary file: %v", err) } - defer os.Remove(tempFile.Name()) + defer utiltesting.CloseAndRemove(t, tempFile) _, _, _, err = getPatchSetsFromPath(tempFile.Name(), testKnownTargets, io.Discard) var pathErr *os.PathError diff --git a/pkg/credentialprovider/plugin/config_test.go b/pkg/credentialprovider/plugin/config_test.go index 2d1e10bf966..3ca2104ea67 100644 --- a/pkg/credentialprovider/plugin/config_test.go +++ b/pkg/credentialprovider/plugin/config_test.go @@ -22,6 +22,8 @@ import ( "testing" "time" + utiltesting "k8s.io/client-go/util/testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config" ) @@ -320,7 +322,7 @@ providers: if err != nil { t.Fatal(err) } - defer os.Remove(file.Name()) + defer utiltesting.CloseAndRemove(t, file) _, err = file.WriteString(testcase.configData) if err != nil { diff --git a/pkg/kubelet/certificate/bootstrap/bootstrap_test.go b/pkg/kubelet/certificate/bootstrap/bootstrap_test.go index e5dee33dae0..71fa56efdfb 100644 --- a/pkg/kubelet/certificate/bootstrap/bootstrap_test.go +++ b/pkg/kubelet/certificate/bootstrap/bootstrap_test.go @@ -25,6 +25,8 @@ import ( "reflect" "testing" + utiltesting "k8s.io/client-go/util/testing" + "github.com/google/go-cmp/cmp" certificatesv1 "k8s.io/api/certificates/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -324,8 +326,8 @@ users: if err != nil { t.Fatal(err) } - defer os.Remove(f.Name()) - os.WriteFile(f.Name(), testData, os.FileMode(0755)) + defer utiltesting.CloseAndRemove(t, f) + f.Write(testData) config, err := loadRESTClientConfig(f.Name()) if err != nil { diff --git a/pkg/kubelet/kuberuntime/logs/logs_test.go b/pkg/kubelet/kuberuntime/logs/logs_test.go index c7d2ed5e396..f2c17cb8161 100644 --- a/pkg/kubelet/kuberuntime/logs/logs_test.go +++ b/pkg/kubelet/kuberuntime/logs/logs_test.go @@ -26,6 +26,8 @@ import ( "testing" "time" + utiltesting "k8s.io/client-go/util/testing" + v1 "k8s.io/api/core/v1" apitesting "k8s.io/cri-api/pkg/apis/testing" "k8s.io/utils/pointer" @@ -79,7 +81,7 @@ func TestReadLogs(t *testing.T) { if err != nil { t.Fatalf("unable to create temp file") } - defer os.Remove(file.Name()) + defer utiltesting.CloseAndRemove(t, file) file.WriteString(`{"log":"line1\n","stream":"stdout","time":"2020-09-27T11:18:01.00000000Z"}` + "\n") file.WriteString(`{"log":"line2\n","stream":"stdout","time":"2020-09-27T11:18:02.00000000Z"}` + "\n") file.WriteString(`{"log":"line3\n","stream":"stdout","time":"2020-09-27T11:18:03.00000000Z"}` + "\n") diff --git a/plugin/pkg/admission/imagepolicy/admission_test.go b/plugin/pkg/admission/imagepolicy/admission_test.go index d1f81d51950..267b163eeda 100644 --- a/plugin/pkg/admission/imagepolicy/admission_test.go +++ b/plugin/pkg/admission/imagepolicy/admission_test.go @@ -29,6 +29,8 @@ import ( "testing" "time" + utiltesting "k8s.io/client-go/util/testing" + "k8s.io/api/imagepolicy/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apiserver/pkg/admission" @@ -67,7 +69,7 @@ imagePolicy: ` func TestNewFromConfig(t *testing.T) { - dir, err := ioutil.TempDir("", "") + dir, err := os.MkdirTemp("", "") if err != nil { t.Fatal(err) } @@ -200,8 +202,7 @@ current-context: default if err != nil { return err } - p := tempfile.Name() - defer os.Remove(p) + defer utiltesting.CloseAndRemove(t, tempfile) tmpl, err := template.New("test").Parse(tt.kubeConfigTmpl) if err != nil { @@ -215,8 +216,7 @@ current-context: default if err != nil { return err } - pc := tempconfigfile.Name() - defer os.Remove(pc) + defer os.Remove(tempconfigfile.Name()) configTmpl, err := template.New("testconfig").Parse(defaultConfigTmplJSON) if err != nil { @@ -229,7 +229,7 @@ current-context: default RetryBackoff int DefaultAllow bool }{ - KubeConfig: p, + KubeConfig: tempfile.Name(), AllowTTL: 500, DenyTTL: 500, RetryBackoff: 500, @@ -240,7 +240,7 @@ current-context: default } // Create a new admission controller - configFile, err := os.Open(pc) + configFile, err := os.Open(tempconfigfile.Name()) if err != nil { return fmt.Errorf("failed to read test config: %v", err) } @@ -358,13 +358,13 @@ func (m *mockService) HTTPStatusCode() int { return m.statusCode } // newImagePolicyWebhook creates a temporary kubeconfig file from the provided arguments and attempts to load // a new newImagePolicyWebhook from it. -func newImagePolicyWebhook(callbackURL string, clientCert, clientKey, ca []byte, cacheTime time.Duration, defaultAllow bool) (*Plugin, error) { +func newImagePolicyWebhook(t *testing.T, callbackURL string, clientCert, clientKey, ca []byte, cacheTime time.Duration, defaultAllow bool) (*Plugin, error) { tempfile, err := ioutil.TempFile("", "") if err != nil { return nil, err } p := tempfile.Name() - defer os.Remove(p) + defer utiltesting.CloseAndRemove(t, tempfile) config := v1.Config{ Clusters: []v1.NamedCluster{ { @@ -386,7 +386,7 @@ func newImagePolicyWebhook(callbackURL string, clientCert, clientKey, ca []byte, return nil, err } pc := tempconfigfile.Name() - defer os.Remove(pc) + defer utiltesting.CloseAndRemove(t, tempconfigfile) configTmpl, err := template.New("testconfig").Parse(defaultConfigTmplYAML) if err != nil { @@ -478,7 +478,7 @@ func TestTLSConfig(t *testing.T) { } defer server.Close() - wh, err := newImagePolicyWebhook(server.URL, tt.clientCert, tt.clientKey, tt.clientCA, -1, false) + wh, err := newImagePolicyWebhook(t, server.URL, tt.clientCert, tt.clientKey, tt.clientCA, -1, false) if err != nil { t.Errorf("%s: failed to create client: %v", tt.test, err) return @@ -559,7 +559,7 @@ func TestWebhookCache(t *testing.T) { defer s.Close() // Create an admission controller that caches successful responses. - wh, err := newImagePolicyWebhook(s.URL, clientCert, clientKey, caCert, 200, false) + wh, err := newImagePolicyWebhook(t, s.URL, clientCert, clientKey, caCert, 200, false) if err != nil { t.Fatal(err) } @@ -753,7 +753,7 @@ func TestContainerCombinations(t *testing.T) { } defer server.Close() - wh, err := newImagePolicyWebhook(server.URL, clientCert, clientKey, caCert, 0, false) + wh, err := newImagePolicyWebhook(t, server.URL, clientCert, clientKey, caCert, 0, false) if err != nil { t.Errorf("%s: failed to create client: %v", tt.test, err) return @@ -847,7 +847,7 @@ func TestDefaultAllow(t *testing.T) { } defer server.Close() - wh, err := newImagePolicyWebhook(server.URL, clientCert, clientKey, caCert, 0, tt.defaultAllow) + wh, err := newImagePolicyWebhook(t, server.URL, clientCert, clientKey, caCert, 0, tt.defaultAllow) if err != nil { t.Errorf("%s: failed to create client: %v", tt.test, err) return @@ -954,7 +954,7 @@ func TestAnnotationFiltering(t *testing.T) { } defer server.Close() - wh, err := newImagePolicyWebhook(server.URL, clientCert, clientKey, caCert, 0, true) + wh, err := newImagePolicyWebhook(t, server.URL, clientCert, clientKey, caCert, 0, true) if err != nil { t.Errorf("%s: failed to create client: %v", tt.test, err) return @@ -1047,7 +1047,7 @@ func TestReturnedAnnotationAdd(t *testing.T) { } defer server.Close() - wh, err := newImagePolicyWebhook(server.URL, clientCert, clientKey, caCert, 0, true) + wh, err := newImagePolicyWebhook(t, server.URL, clientCert, clientKey, caCert, 0, true) if err != nil { t.Errorf("%s: failed to create client: %v", tt.test, err) return