Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix leaking goroutine in multiple integration tests by migrating to common StartTestServer utility #110229

Merged
merged 20 commits into from
May 27, 2022
Merged

Fix leaking goroutine in multiple integration tests by migrating to common StartTestServer utility #110229

Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
6ae4bbb
Add namespace utility test functions
wojtek-t May 21, 2022
3b98f59
Clean shutdown of service integration tests
wojtek-t May 20, 2022
f68e72a
Clean shutdown of volume integration tests
wojtek-t May 20, 2022
c6e3bd7
Clean shutdown of persistentvolume integration tests
wojtek-t May 20, 2022
284509f
Clean shutdown of ttlcontroller integration tests
wojtek-t May 21, 2022
054c489
Clean shutdown of metrics integration tests
wojtek-t May 21, 2022
7928929
Clean shutdown of pods integration tests
wojtek-t May 21, 2022
368802d
Clean shutdown of deployment integration tests
wojtek-t May 15, 2022
33831c9
Clean shutdown of replicaset integration tests
wojtek-t May 21, 2022
7ac6638
Clean shutdown of replicationcontroller integration tests
wojtek-t May 21, 2022
7add7ba
Clean shutdown of endpoints integration tests
wojtek-t May 21, 2022
492bad0
Clean shutdown of endpointslice integration tests
wojtek-t May 21, 2022
7b44e64
Clean shutdown of secret integration tests
wojtek-t May 21, 2022
fbd2184
Clean shutdown of namespace integration tests
wojtek-t May 21, 2022
b8d28ef
Clean shutdown of storageclasses integration tests
wojtek-t May 21, 2022
5fc1c39
Clean shutdown of configmap integration tests
wojtek-t May 21, 2022
2583942
Clean shutdown of daemonset integration tests
wojtek-t May 21, 2022
c0149f7
Clean shutdown of certificates integration tests
wojtek-t May 21, 2022
2893ad3
Clean shutdown of evictions integration tests
wojtek-t May 21, 2022
8ca1ec2
Clean shutdown of cronjob integration tests
wojtek-t May 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
157 changes: 81 additions & 76 deletions test/integration/volume/persistent_volumes_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"context"
"fmt"
"math/rand"
"net/http/httptest"
"os"
"strconv"
"testing"
Expand All @@ -36,13 +35,13 @@ import (
restclient "k8s.io/client-go/rest"
ref "k8s.io/client-go/tools/reference"
fakecloud "k8s.io/cloud-provider/fake"
kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing"
"k8s.io/kubernetes/pkg/api/legacyscheme"
persistentvolumecontroller "k8s.io/kubernetes/pkg/controller/volume/persistentvolume"
"k8s.io/kubernetes/pkg/volume"
volumetest "k8s.io/kubernetes/pkg/volume/testing"
"k8s.io/kubernetes/test/integration/framework"

"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/klog/v2"
)

Expand Down Expand Up @@ -105,16 +104,17 @@ func testSleep() {

func TestPersistentVolumeRecycler(t *testing.T) {
klog.V(2).Infof("TestPersistentVolumeRecycler started")
_, s, closeFn := framework.RunAnAPIServer(nil)
defer closeFn()
s := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{"--disable-admission-plugins=ServiceAccount,StorageObjectInUseProtection"}, framework.SharedEtcd())
defer s.TearDownFn()
namespaceName := "pv-recycler"

ns := framework.CreateTestingNamespace("pv-recycler", t)
defer framework.DeleteTestingNamespace(ns, t)

testClient, ctrl, informers, watchPV, watchPVC := createClients(ns, t, s, defaultSyncPeriod)
testClient, ctrl, informers, watchPV, watchPVC := createClients(namespaceName, t, s, defaultSyncPeriod)
defer watchPV.Stop()
defer watchPVC.Stop()

ns := framework.CreateNamespaceOrDie(testClient, namespaceName, t)
defer framework.DeleteNamespaceOrDie(testClient, ns, t)

// NOTE: This test cannot run in parallel, because it is creating and deleting
// non-namespaced objects (PersistenceVolumes).
defer testClient.CoreV1().PersistentVolumes().DeleteCollection(context.TODO(), metav1.DeleteOptions{}, metav1.ListOptions{})
Expand Down Expand Up @@ -160,16 +160,17 @@ func TestPersistentVolumeRecycler(t *testing.T) {

func TestPersistentVolumeDeleter(t *testing.T) {
klog.V(2).Infof("TestPersistentVolumeDeleter started")
_, s, closeFn := framework.RunAnAPIServer(nil)
defer closeFn()

ns := framework.CreateTestingNamespace("pv-deleter", t)
defer framework.DeleteTestingNamespace(ns, t)
s := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{"--disable-admission-plugins=ServiceAccount,StorageObjectInUseProtection"}, framework.SharedEtcd())
defer s.TearDownFn()
namespaceName := "pv-deleter"

testClient, ctrl, informers, watchPV, watchPVC := createClients(ns, t, s, defaultSyncPeriod)
testClient, ctrl, informers, watchPV, watchPVC := createClients(namespaceName, t, s, defaultSyncPeriod)
defer watchPV.Stop()
defer watchPVC.Stop()

ns := framework.CreateNamespaceOrDie(testClient, namespaceName, t)
defer framework.DeleteNamespaceOrDie(testClient, ns, t)

// NOTE: This test cannot run in parallel, because it is creating and deleting
// non-namespaced objects (PersistenceVolumes).
defer testClient.CoreV1().PersistentVolumes().DeleteCollection(context.TODO(), metav1.DeleteOptions{}, metav1.ListOptions{})
Expand Down Expand Up @@ -220,16 +221,17 @@ func TestPersistentVolumeBindRace(t *testing.T) {
// Test a race binding many claims to a PV that is pre-bound to a specific
// PVC. Only this specific PVC should get bound.
klog.V(2).Infof("TestPersistentVolumeBindRace started")
_, s, closeFn := framework.RunAnAPIServer(nil)
defer closeFn()

ns := framework.CreateTestingNamespace("pv-bind-race", t)
defer framework.DeleteTestingNamespace(ns, t)
s := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{"--disable-admission-plugins=ServiceAccount,StorageObjectInUseProtection"}, framework.SharedEtcd())
defer s.TearDownFn()
namespaceName := "pv-bind-race"

testClient, ctrl, informers, watchPV, watchPVC := createClients(ns, t, s, defaultSyncPeriod)
testClient, ctrl, informers, watchPV, watchPVC := createClients(namespaceName, t, s, defaultSyncPeriod)
defer watchPV.Stop()
defer watchPVC.Stop()

ns := framework.CreateNamespaceOrDie(testClient, namespaceName, t)
defer framework.DeleteNamespaceOrDie(testClient, ns, t)

// NOTE: This test cannot run in parallel, because it is creating and deleting
// non-namespaced objects (PersistenceVolumes).
defer testClient.CoreV1().PersistentVolumes().DeleteCollection(context.TODO(), metav1.DeleteOptions{}, metav1.ListOptions{})
Expand Down Expand Up @@ -290,16 +292,17 @@ func TestPersistentVolumeBindRace(t *testing.T) {

// TestPersistentVolumeClaimLabelSelector test binding using label selectors
func TestPersistentVolumeClaimLabelSelector(t *testing.T) {
_, s, closeFn := framework.RunAnAPIServer(nil)
defer closeFn()

ns := framework.CreateTestingNamespace("pvc-label-selector", t)
defer framework.DeleteTestingNamespace(ns, t)
s := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{"--disable-admission-plugins=ServiceAccount,StorageObjectInUseProtection"}, framework.SharedEtcd())
defer s.TearDownFn()
namespaceName := "pvc-label-selector"

testClient, controller, informers, watchPV, watchPVC := createClients(ns, t, s, defaultSyncPeriod)
testClient, controller, informers, watchPV, watchPVC := createClients(namespaceName, t, s, defaultSyncPeriod)
defer watchPV.Stop()
defer watchPVC.Stop()

ns := framework.CreateNamespaceOrDie(testClient, namespaceName, t)
defer framework.DeleteNamespaceOrDie(testClient, ns, t)

// NOTE: This test cannot run in parallel, because it is creating and deleting
// non-namespaced objects (PersistenceVolumes).
defer testClient.CoreV1().PersistentVolumes().DeleteCollection(context.TODO(), metav1.DeleteOptions{}, metav1.ListOptions{})
Expand Down Expand Up @@ -371,16 +374,17 @@ func TestPersistentVolumeClaimLabelSelector(t *testing.T) {
// TestPersistentVolumeClaimLabelSelectorMatchExpressions test binding using
// MatchExpressions label selectors
func TestPersistentVolumeClaimLabelSelectorMatchExpressions(t *testing.T) {
_, s, closeFn := framework.RunAnAPIServer(nil)
defer closeFn()
s := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{"--disable-admission-plugins=ServiceAccount,StorageObjectInUseProtection"}, framework.SharedEtcd())
defer s.TearDownFn()
namespaceName := "pvc-match-expressions"

ns := framework.CreateTestingNamespace("pvc-match-expressions", t)
defer framework.DeleteTestingNamespace(ns, t)

testClient, controller, informers, watchPV, watchPVC := createClients(ns, t, s, defaultSyncPeriod)
testClient, controller, informers, watchPV, watchPVC := createClients(namespaceName, t, s, defaultSyncPeriod)
defer watchPV.Stop()
defer watchPVC.Stop()

ns := framework.CreateNamespaceOrDie(testClient, namespaceName, t)
defer framework.DeleteNamespaceOrDie(testClient, ns, t)

// NOTE: This test cannot run in parallel, because it is creating and deleting
// non-namespaced objects (PersistenceVolumes).
defer testClient.CoreV1().PersistentVolumes().DeleteCollection(context.TODO(), metav1.DeleteOptions{}, metav1.ListOptions{})
Expand Down Expand Up @@ -471,16 +475,17 @@ func TestPersistentVolumeClaimLabelSelectorMatchExpressions(t *testing.T) {
// TestPersistentVolumeMultiPVs tests binding of one PVC to 100 PVs with
// different size.
func TestPersistentVolumeMultiPVs(t *testing.T) {
_, s, closeFn := framework.RunAnAPIServer(nil)
defer closeFn()
s := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{"--disable-admission-plugins=ServiceAccount,StorageObjectInUseProtection"}, framework.SharedEtcd())
defer s.TearDownFn()
namespaceName := "multi-pvs"

ns := framework.CreateTestingNamespace("multi-pvs", t)
defer framework.DeleteTestingNamespace(ns, t)

testClient, controller, informers, watchPV, watchPVC := createClients(ns, t, s, defaultSyncPeriod)
testClient, controller, informers, watchPV, watchPVC := createClients(namespaceName, t, s, defaultSyncPeriod)
defer watchPV.Stop()
defer watchPVC.Stop()

ns := framework.CreateNamespaceOrDie(testClient, namespaceName, t)
defer framework.DeleteNamespaceOrDie(testClient, ns, t)

// NOTE: This test cannot run in parallel, because it is creating and deleting
// non-namespaced objects (PersistenceVolumes).
defer testClient.CoreV1().PersistentVolumes().DeleteCollection(context.TODO(), metav1.DeleteOptions{}, metav1.ListOptions{})
Expand Down Expand Up @@ -561,16 +566,17 @@ func TestPersistentVolumeMultiPVs(t *testing.T) {
// TestPersistentVolumeMultiPVsPVCs tests binding of 100 PVC to 100 PVs.
// This test is configurable by KUBE_INTEGRATION_PV_* variables.
func TestPersistentVolumeMultiPVsPVCs(t *testing.T) {
_, s, closeFn := framework.RunAnAPIServer(nil)
defer closeFn()

ns := framework.CreateTestingNamespace("multi-pvs-pvcs", t)
defer framework.DeleteTestingNamespace(ns, t)
s := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{"--disable-admission-plugins=ServiceAccount,StorageObjectInUseProtection"}, framework.SharedEtcd())
defer s.TearDownFn()
namespaceName := "multi-pvs-pvcs"

testClient, binder, informers, watchPV, watchPVC := createClients(ns, t, s, defaultSyncPeriod)
testClient, binder, informers, watchPV, watchPVC := createClients(namespaceName, t, s, defaultSyncPeriod)
defer watchPV.Stop()
defer watchPVC.Stop()

ns := framework.CreateNamespaceOrDie(testClient, namespaceName, t)
defer framework.DeleteNamespaceOrDie(testClient, ns, t)

// NOTE: This test cannot run in parallel, because it is creating and deleting
// non-namespaced objects (PersistenceVolumes).
defer testClient.CoreV1().PersistentVolumes().DeleteCollection(context.TODO(), metav1.DeleteOptions{}, metav1.ListOptions{})
Expand Down Expand Up @@ -721,21 +727,22 @@ func TestPersistentVolumeMultiPVsPVCs(t *testing.T) {
// TestPersistentVolumeControllerStartup tests startup of the controller.
// The controller should not unbind any volumes when it starts.
func TestPersistentVolumeControllerStartup(t *testing.T) {
_, s, closeFn := framework.RunAnAPIServer(nil)
defer closeFn()

ns := framework.CreateTestingNamespace("controller-startup", t)
defer framework.DeleteTestingNamespace(ns, t)
s := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{"--disable-admission-plugins=ServiceAccount,StorageObjectInUseProtection"}, framework.SharedEtcd())
defer s.TearDownFn()
namespaceName := "controller-startup"

objCount := getObjectCount()

const shortSyncPeriod = 2 * time.Second
syncPeriod := getSyncPeriod(shortSyncPeriod)

testClient, binder, informers, watchPV, watchPVC := createClients(ns, t, s, shortSyncPeriod)
testClient, binder, informers, watchPV, watchPVC := createClients(namespaceName, t, s, shortSyncPeriod)
defer watchPV.Stop()
defer watchPVC.Stop()

ns := framework.CreateNamespaceOrDie(testClient, namespaceName, t)
defer framework.DeleteNamespaceOrDie(testClient, ns, t)

// Create *bound* volumes and PVCs
pvs := make([]*v1.PersistentVolume, objCount)
pvcs := make([]*v1.PersistentVolumeClaim, objCount)
Expand Down Expand Up @@ -850,16 +857,17 @@ func TestPersistentVolumeControllerStartup(t *testing.T) {
// TestPersistentVolumeProvisionMultiPVCs tests provisioning of many PVCs.
// This test is configurable by KUBE_INTEGRATION_PV_* variables.
func TestPersistentVolumeProvisionMultiPVCs(t *testing.T) {
_, s, closeFn := framework.RunAnAPIServer(nil)
defer closeFn()

ns := framework.CreateTestingNamespace("provision-multi-pvs", t)
defer framework.DeleteTestingNamespace(ns, t)
s := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{"--disable-admission-plugins=ServiceAccount,StorageObjectInUseProtection"}, framework.SharedEtcd())
defer s.TearDownFn()
namespaceName := "provision-multi-pvs"

testClient, binder, informers, watchPV, watchPVC := createClients(ns, t, s, defaultSyncPeriod)
testClient, binder, informers, watchPV, watchPVC := createClients(namespaceName, t, s, defaultSyncPeriod)
defer watchPV.Stop()
defer watchPVC.Stop()

ns := framework.CreateNamespaceOrDie(testClient, namespaceName, t)
defer framework.DeleteNamespaceOrDie(testClient, ns, t)

// NOTE: This test cannot run in parallel, because it is creating and deleting
// non-namespaced objects (PersistenceVolumes and StorageClasses).
defer testClient.CoreV1().PersistentVolumes().DeleteCollection(context.TODO(), metav1.DeleteOptions{}, metav1.ListOptions{})
Expand Down Expand Up @@ -945,16 +953,17 @@ func TestPersistentVolumeProvisionMultiPVCs(t *testing.T) {
// TestPersistentVolumeMultiPVsDiffAccessModes tests binding of one PVC to two
// PVs with different access modes.
func TestPersistentVolumeMultiPVsDiffAccessModes(t *testing.T) {
_, s, closeFn := framework.RunAnAPIServer(nil)
defer closeFn()
s := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{"--disable-admission-plugins=ServiceAccount,StorageObjectInUseProtection"}, framework.SharedEtcd())
defer s.TearDownFn()
namespaceName := "multi-pvs-diff-access"

ns := framework.CreateTestingNamespace("multi-pvs-diff-access", t)
defer framework.DeleteTestingNamespace(ns, t)

testClient, controller, informers, watchPV, watchPVC := createClients(ns, t, s, defaultSyncPeriod)
testClient, controller, informers, watchPV, watchPVC := createClients(namespaceName, t, s, defaultSyncPeriod)
defer watchPV.Stop()
defer watchPVC.Stop()

ns := framework.CreateNamespaceOrDie(testClient, namespaceName, t)
defer framework.DeleteNamespaceOrDie(testClient, ns, t)
Comment on lines +960 to +965
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you always create the namespace after create the clients in this commit

watchPVC, err := testClient.CoreV1().PersistentVolumeClaims(namespaceName).Watch(context.TODO(), metav1.ListOptions{})

I think that doesn't matter, but is there any special reason?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this PR we effectively weren't creating the namespace (CreateTestingNamespace isn't effectively creating a namespace:
https://github.com/kubernetes/kubernetes/blob/master/test/integration/framework/util.go#L54

So we didn't need the client for that.

After switching to more realistic apiserver in this PR (that e.g. has namespace admission plugins enabled) we have to actually create a namespace (or alternatively disable namespace admission plugins, but I think it's better to those test be as realistic as possible), which requires having a client.

So I need to do that after creating a client.

Now the options are:

  • refactor the code by splitting client creation and setting up those watches (and create namespaces in the middle)
  • leave as is and create namespace after calling createClients

Since this doesn't matter really, I chose a cheaper to execute option :)


// NOTE: This test cannot run in parallel, because it is creating and deleting
// non-namespaced objects (PersistenceVolumes).
defer testClient.CoreV1().PersistentVolumes().DeleteCollection(context.TODO(), metav1.DeleteOptions{}, metav1.ListOptions{})
Expand Down Expand Up @@ -1093,21 +1102,17 @@ func waitForAnyPersistentVolumeClaimPhase(w watch.Interface, phase v1.Persistent
}
}

func createClients(ns *v1.Namespace, t *testing.T, s *httptest.Server, syncPeriod time.Duration) (*clientset.Clientset, *persistentvolumecontroller.PersistentVolumeController, informers.SharedInformerFactory, watch.Interface, watch.Interface) {
func createClients(namespaceName string, t *testing.T, s *kubeapiservertesting.TestServer, syncPeriod time.Duration) (*clientset.Clientset, *persistentvolumecontroller.PersistentVolumeController, informers.SharedInformerFactory, watch.Interface, watch.Interface) {
// Use higher QPS and Burst, there is a test for race conditions which
// creates many objects and default values were too low.
binderClient := clientset.NewForConfigOrDie(&restclient.Config{
Host: s.URL,
ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}},
QPS: 1000000,
Burst: 1000000,
})
testClient := clientset.NewForConfigOrDie(&restclient.Config{
Host: s.URL,
ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}},
QPS: 1000000,
Burst: 1000000,
})
binderConfig := restclient.CopyConfig(s.ClientConfig)
binderConfig.QPS = 1000000
binderConfig.Burst = 1000000
binderClient := clientset.NewForConfigOrDie(binderConfig)
testConfig := restclient.CopyConfig(s.ClientConfig)
testConfig.QPS = 1000000
testConfig.Burst = 1000000
testClient := clientset.NewForConfigOrDie(testConfig)

host := volumetest.NewFakeVolumeHost(t, "/tmp/fake", nil, nil)
plugin := &volumetest.FakeVolumePlugin{
Expand Down Expand Up @@ -1146,7 +1151,7 @@ func createClients(ns *v1.Namespace, t *testing.T, s *httptest.Server, syncPerio
if err != nil {
t.Fatalf("Failed to watch PersistentVolumes: %v", err)
}
watchPVC, err := testClient.CoreV1().PersistentVolumeClaims(ns.Name).Watch(context.TODO(), metav1.ListOptions{})
watchPVC, err := testClient.CoreV1().PersistentVolumeClaims(namespaceName).Watch(context.TODO(), metav1.ListOptions{})
if err != nil {
t.Fatalf("Failed to watch PersistentVolumeClaims: %v", err)
}
Expand Down