Merge pull request #47645 from jsafrane/integration-test-speedup

Automatic merge from submit-queue (batch tested with PRs 48139, 48042, 47645, 48054, 48003)

Speed up attach/detach controller integration tests

Internal attach/detach controller timers should be configurable and tests should use much shorter values.

`reconcilerSyncDuration` is deliberately left out of `TimerConfig` because it's the only one that's not a constant one, it's configurable by user.

Fixes #47129 

Before:
```
--- PASS: TestPodDeletionWithDswp (63.21s)
--- PASS: TestPodUpdateWithWithADC (13.68s)
--- PASS: TestPodUpdateWithKeepTerminatedPodVolumes (13.55s)
--- PASS: TestPodAddedByDswp (183.01s)
--- PASS: TestPersistentVolumeRecycler (12.55s)
--- PASS: TestPersistentVolumeDeleter (12.54s)
--- PASS: TestPersistentVolumeBindRace (3.51s)
--- PASS: TestPersistentVolumeClaimLabelSelector (12.50s)
--- PASS: TestPersistentVolumeClaimLabelSelectorMatchExpressions (12.54s)
--- PASS: TestPersistentVolumeMultiPVs (3.05s)
--- PASS: TestPersistentVolumeMultiPVsPVCs (4.36s)
--- PASS: TestPersistentVolumeControllerStartup (7.29s)
--- PASS: TestPersistentVolumeProvisionMultiPVCs (5.02s)
--- PASS: TestPersistentVolumeMultiPVsDiffAccessModes (12.48s)
ok  	k8s.io/kubernetes/test/integration/volume	359.727s
```

After:
```
--- PASS: TestPodDeletionWithDswp (3.71s)
--- PASS: TestPodUpdateWithWithADC (3.63s)
--- PASS: TestPodUpdateWithKeepTerminatedPodVolumes (3.70s)
--- PASS: TestPodAddedByDswp (5.68s)
--- PASS: TestPersistentVolumeRecycler (12.54s)
--- PASS: TestPersistentVolumeDeleter (12.55s)
--- PASS: TestPersistentVolumeBindRace (3.55s)
--- PASS: TestPersistentVolumeClaimLabelSelector (12.50s)
--- PASS: TestPersistentVolumeClaimLabelSelectorMatchExpressions (12.52s)
--- PASS: TestPersistentVolumeMultiPVs (3.98s)
--- PASS: TestPersistentVolumeMultiPVsPVCs (3.85s)
--- PASS: TestPersistentVolumeControllerStartup (7.18s)
--- PASS: TestPersistentVolumeProvisionMultiPVCs (5.23s)
--- PASS: TestPersistentVolumeMultiPVsDiffAccessModes (12.48s)
ok  	k8s.io/kubernetes/test/integration/volume	103.267s
```

PV controller tests are the slowest ones now.

@kubernetes/sig-storage-pr-reviews 
/assign @gnufied 

```release-note
NONE
```

cmd/kube-controller-manager/app/core.go

@@ -166,7 +166,9 @@ func startAttachDetachController(ctx ControllerContext) (bool, error) {
 			ctx.Cloud,
 			ProbeAttachableVolumePlugins(ctx.Options.VolumeConfiguration),
 			ctx.Options.DisableAttachDetachReconcilerSync,
-			ctx.Options.ReconcilerSyncLoopPeriod.Duration)
+			ctx.Options.ReconcilerSyncLoopPeriod.Duration,
+			attachdetach.DefaultTimerConfig,
+		)
 	if attachDetachControllerErr != nil {
 		return true, fmt.Errorf("failed to start attach/detach controller: %v", attachDetachControllerErr)
 	}

pkg/controller/volume/attachdetach/attach_detach_controller.go

@@ -50,26 +50,38 @@ import (
 	"k8s.io/kubernetes/pkg/volume/util/volumehelper"
 )
 
-const (
-	// loopPeriod is the amount of time the reconciler loop waits between
-	// successive executions
-	reconcilerLoopPeriod time.Duration = 100 * time.Millisecond
+// TimerConfig contains configuration of internal attach/detach timers and
+// should be used only to speed up tests. DefaultTimerConfig is the suggested
+// timer configuration for production.
+type TimerConfig struct {
+	// ReconcilerLoopPeriod is the amount of time the reconciler loop waits
+	// between successive executions
+	ReconcilerLoopPeriod time.Duration
 
-	// reconcilerMaxWaitForUnmountDuration is the maximum amount of time the
+	// ReconcilerMaxWaitForUnmountDuration is the maximum amount of time the
 	// attach detach controller will wait for a volume to be safely unmounted
 	// from its node. Once this time has expired, the controller will assume the
 	// node or kubelet are unresponsive and will detach the volume anyway.
-	reconcilerMaxWaitForUnmountDuration time.Duration = 6 * time.Minute
+	ReconcilerMaxWaitForUnmountDuration time.Duration
 
-	// desiredStateOfWorldPopulatorLoopSleepPeriod is the amount of time the
+	// DesiredStateOfWorldPopulatorLoopSleepPeriod is the amount of time the
 	// DesiredStateOfWorldPopulator loop waits between successive executions
-	desiredStateOfWorldPopulatorLoopSleepPeriod time.Duration = 1 * time.Minute
+	DesiredStateOfWorldPopulatorLoopSleepPeriod time.Duration
 
-	// desiredStateOfWorldPopulatorListPodsRetryDuration is the amount of
+	// DesiredStateOfWorldPopulatorListPodsRetryDuration is the amount of
 	// time the DesiredStateOfWorldPopulator loop waits between list pods
 	// calls.
-	desiredStateOfWorldPopulatorListPodsRetryDuration time.Duration = 3 * time.Minute
-)
+	DesiredStateOfWorldPopulatorListPodsRetryDuration time.Duration
+}
+
+// DefaultTimerConfig is the default configuration of Attach/Detach controller
+// timers.
+var DefaultTimerConfig TimerConfig = TimerConfig{
+	ReconcilerLoopPeriod:                              100 * time.Millisecond,
+	ReconcilerMaxWaitForUnmountDuration:               6 * time.Minute,
+	DesiredStateOfWorldPopulatorLoopSleepPeriod:       1 * time.Minute,
+	DesiredStateOfWorldPopulatorListPodsRetryDuration: 3 * time.Minute,
+}
 
 // AttachDetachController defines the operations supported by this controller.
 type AttachDetachController interface {
@@ -87,7 +99,8 @@ func NewAttachDetachController(
 	cloud cloudprovider.Interface,
 	plugins []volume.VolumePlugin,
 	disableReconciliationSync bool,
-	reconcilerSyncDuration time.Duration) (AttachDetachController, error) {
+	reconcilerSyncDuration time.Duration,
+	timerConfig TimerConfig) (AttachDetachController, error) {
 	// TODO: The default resyncPeriod for shared informers is 12 hours, this is
 	// unacceptable for the attach/detach controller. For example, if a pod is
 	// skipped because the node it is scheduled to didn't set its annotation in
@@ -137,8 +150,8 @@ func NewAttachDetachController(
 
 	// Default these to values in options
 	adc.reconciler = reconciler.NewReconciler(
-		reconcilerLoopPeriod,
-		reconcilerMaxWaitForUnmountDuration,
+		timerConfig.ReconcilerLoopPeriod,
+		timerConfig.ReconcilerMaxWaitForUnmountDuration,
 		reconcilerSyncDuration,
 		disableReconciliationSync,
 		adc.desiredStateOfWorld,
@@ -148,8 +161,8 @@ func NewAttachDetachController(
 		recorder)
 
 	adc.desiredStateOfWorldPopulator = populator.NewDesiredStateOfWorldPopulator(
-		desiredStateOfWorldPopulatorLoopSleepPeriod,
-		desiredStateOfWorldPopulatorListPodsRetryDuration,
+		timerConfig.DesiredStateOfWorldPopulatorLoopSleepPeriod,
+		timerConfig.DesiredStateOfWorldPopulatorListPodsRetryDuration,
 		podInformer.Lister(),
 		adc.desiredStateOfWorld,
 		&adc.volumePluginMgr,

pkg/controller/volume/attachdetach/attach_detach_controller_test.go

@@ -46,7 +46,8 @@ func Test_NewAttachDetachController_Positive(t *testing.T) {
 		nil, /* cloud */
 		nil, /* plugins */
 		false,
-		time.Second*5)
+		5*time.Second,
+		DefaultTimerConfig)
 
 	// Assert
 	if err != nil {
@@ -212,7 +213,8 @@ func attachDetachRecoveryTestCase(t *testing.T, extraPods1 []*v1.Pod, extraPods2
 		nil, /* cloud */
 		plugins,
 		false,
-		time.Second*1)
+		1*time.Second,
+		DefaultTimerConfig)
 
 	if err != nil {
 		t.Fatalf("Run failed with error. Expected: <no error> Actual: <%v>", err)

test/integration/volume/attach_detach_test.go

@@ -82,7 +82,6 @@ func TestPodDeletionWithDswp(t *testing.T) {
 	_, server, closeFn := framework.RunAMaster(framework.NewIntegrationTestMasterConfig())
 	defer closeFn()
 	namespaceName := "test-pod-deletion"
-
 	node := &v1.Node{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "node-sandbox",
@@ -283,7 +282,7 @@ func TestPodUpdateWithKeepTerminatedPodVolumes(t *testing.T) {
 // running the RC manager to prevent the rc manager from creating new pods
 // rather than adopting the existing ones.
 func waitToObservePods(t *testing.T, podInformer cache.SharedIndexInformer, podNum int) {
-	if err := wait.Poll(10*time.Second, 60*time.Second, func() (bool, error) {
+	if err := wait.Poll(100*time.Millisecond, 60*time.Second, func() (bool, error) {
 		objects := podInformer.GetIndexer().List()
 		if len(objects) == podNum {
 			return true, nil
@@ -347,6 +346,12 @@ func createAdClients(ns *v1.Namespace, t *testing.T, server *httptest.Server, sy
 	plugins := []volume.VolumePlugin{plugin}
 	cloud := &fakecloud.FakeCloud{}
 	informers := informers.NewSharedInformerFactory(testClient, resyncPeriod)
+	timers := attachdetach.TimerConfig{
+		ReconcilerLoopPeriod:                              100 * time.Millisecond,
+		ReconcilerMaxWaitForUnmountDuration:               6 * time.Second,
+		DesiredStateOfWorldPopulatorLoopSleepPeriod:       1 * time.Second,
+		DesiredStateOfWorldPopulatorListPodsRetryDuration: 3 * time.Second,
+	}
 	ctrl, err := attachdetach.NewAttachDetachController(
 		testClient,
 		informers.Core().V1().Pods(),
@@ -356,7 +361,8 @@ func createAdClients(ns *v1.Namespace, t *testing.T, server *httptest.Server, sy
 		cloud,
 		plugins,
 		false,
-		time.Second*5)
+		5*time.Second,
+		timers)
 
 	if err != nil {
 		t.Fatalf("Error creating AttachDetach : %v", err)