Merge pull request #83756 from hex108/permit

Refactor scheduler's framework permit API

test/integration/scheduler/framework_test.go

@@ -93,6 +93,7 @@ type UnreservePlugin struct {
 }
 
 type PermitPlugin struct {
+	name                string
 	numPermitCalled     int
 	failPermit          bool
 	rejectPermit        bool
@@ -381,7 +382,7 @@ func (up *UnreservePlugin) reset() {
 
 // Name returns name of the plugin.
 func (pp *PermitPlugin) Name() string {
-	return permitPluginName
+	return pp.name
 }
 
 // Permit implements the permit test plugin.
@@ -416,13 +417,18 @@ func (pp *PermitPlugin) Permit(ctx context.Context, state *framework.CycleState,
 			return framework.NewStatus(framework.Unschedulable, fmt.Sprintf("reject pod %v", pod.Name)), 0
 		}
 		if pp.waitAndAllowPermit {
-			pp.fh.IterateOverWaitingPods(func(wp framework.WaitingPod) { wp.Allow() })
+			pp.allowAllPods()
 			return nil, 0
 		}
 	}
 	return nil, 0
 }
 
+// allowAllPods allows all waiting pods.
+func (pp *PermitPlugin) allowAllPods() {
+	pp.fh.IterateOverWaitingPods(func(wp framework.WaitingPod) { wp.Allow(pp.name) })
+}
+
 // reset used to reset permit plugin.
 func (pp *PermitPlugin) reset() {
 	pp.numPermitCalled = 0
@@ -684,17 +690,10 @@ func TestPrebindPlugin(t *testing.T) {
 			},
 		},
 	}
-	// Set reserve prebind config for testing
-	preBindPluginConfig := []schedulerconfig.PluginConfig{
-		{
-			Name: preBindPluginName,
-			Args: runtime.Unknown{},
-		},
-	}
+
 	// Create the master and the scheduler with the test plugin set.
 	context := initTestSchedulerForFrameworkTest(t, initTestMaster(t, "prebind-plugin", nil), 2,
 		scheduler.WithFrameworkPlugins(plugins),
-		scheduler.WithFrameworkPluginConfig(preBindPluginConfig),
 		scheduler.WithFrameworkDefaultRegistry(registry))
 	defer cleanupTest(t, context)
 
@@ -774,22 +773,10 @@ func TestUnreservePlugin(t *testing.T) {
 			},
 		},
 	}
-	// Set unreserve and prebind plugin config for testing
-	pluginConfig := []schedulerconfig.PluginConfig{
-		{
-			Name: unreservePluginName,
-			Args: runtime.Unknown{},
-		},
-		{
-			Name: preBindPluginName,
-			Args: runtime.Unknown{},
-		},
-	}
 
 	// Create the master and the scheduler with the test plugin set.
 	context := initTestSchedulerForFrameworkTest(t, initTestMaster(t, "unreserve-plugin", nil), 2,
 		scheduler.WithFrameworkPlugins(plugins),
-		scheduler.WithFrameworkPluginConfig(pluginConfig),
 		scheduler.WithFrameworkDefaultRegistry(registry))
 	defer cleanupTest(t, context)
 
@@ -876,30 +863,10 @@ func TestBindPlugin(t *testing.T) {
 			Enabled: []schedulerconfig.Plugin{{Name: postBindPlugin.Name()}},
 		},
 	}
-	// Set reserve and bind config for testing
-	pluginConfig := []schedulerconfig.PluginConfig{
-		{
-			Name: unreservePlugin.Name(),
-			Args: runtime.Unknown{},
-		},
-		{
-			Name: bindPlugin1.Name(),
-			Args: runtime.Unknown{},
-		},
-		{
-			Name: bindPlugin2.Name(),
-			Args: runtime.Unknown{},
-		},
-		{
-			Name: postBindPlugin.Name(),
-			Args: runtime.Unknown{},
-		},
-	}
 
 	// Create the master and the scheduler with the test plugin set.
 	context := initTestSchedulerWithOptions(t, testContext, false, nil, time.Second,
 		scheduler.WithFrameworkPlugins(plugins),
-		scheduler.WithFrameworkPluginConfig(pluginConfig),
 		scheduler.WithFrameworkDefaultRegistry(registry),
 		scheduler.WithFrameworkConfigProducerRegistry(nil))
 	defer cleanupTest(t, context)
@@ -1057,22 +1024,10 @@ func TestPostBindPlugin(t *testing.T) {
 			},
 		},
 	}
-	// Set reserve prebind and postbind config for testing
-	pluginConfig := []schedulerconfig.PluginConfig{
-		{
-			Name: preBindPluginName,
-			Args: runtime.Unknown{},
-		},
-		{
-			Name: postBindPluginName,
-			Args: runtime.Unknown{},
-		},
-	}
 
 	// Create the master and the scheduler with the test plugin set.
 	context := initTestSchedulerForFrameworkTest(t, initTestMaster(t, "postbind-plugin", nil), 2,
 		scheduler.WithFrameworkPlugins(plugins),
-		scheduler.WithFrameworkPluginConfig(pluginConfig),
 		scheduler.WithFrameworkDefaultRegistry(registry))
 	defer cleanupTest(t, context)
 
@@ -1123,7 +1078,7 @@ func TestPostBindPlugin(t *testing.T) {
 // TestPermitPlugin tests invocation of permit plugins.
 func TestPermitPlugin(t *testing.T) {
 	// Create a plugin registry for testing. Register only a permit plugin.
-	perPlugin := &PermitPlugin{}
+	perPlugin := &PermitPlugin{name: permitPluginName}
 	registry := framework.Registry{permitPluginName: newPermitPlugin(perPlugin)}
 
 	// Setup initial permit plugin for testing.
@@ -1136,18 +1091,10 @@ func TestPermitPlugin(t *testing.T) {
 			},
 		},
 	}
-	// Set permit plugin config for testing
-	pluginConfig := []schedulerconfig.PluginConfig{
-		{
-			Name: permitPluginName,
-			Args: runtime.Unknown{},
-		},
-	}
 
 	// Create the master and the scheduler with the test plugin set.
 	context := initTestSchedulerForFrameworkTest(t, initTestMaster(t, "permit-plugin", nil), 2,
 		scheduler.WithFrameworkPlugins(plugins),
-		scheduler.WithFrameworkPluginConfig(pluginConfig),
 		scheduler.WithFrameworkDefaultRegistry(registry))
 	defer cleanupTest(t, context)
 
@@ -1226,10 +1173,82 @@ func TestPermitPlugin(t *testing.T) {
 	}
 }
 
+// TestMultiplePermitPlugins tests multiple permit plugins returning wait for a same pod.
+func TestMultiplePermitPlugins(t *testing.T) {
+	// Create a plugin registry for testing.
+	perPlugin1 := &PermitPlugin{name: "permit-plugin-1"}
+	perPlugin2 := &PermitPlugin{name: "permit-plugin-2"}
+	registry := framework.Registry{
+		perPlugin1.Name(): newPermitPlugin(perPlugin1),
+		perPlugin2.Name(): newPermitPlugin(perPlugin2),
+	}
+
+	// Setup initial permit plugins for testing.
+	plugins := &schedulerconfig.Plugins{
+		Permit: &schedulerconfig.PluginSet{
+			Enabled: []schedulerconfig.Plugin{
+				{
+					Name: perPlugin1.Name(),
+				},
+				{
+					Name: perPlugin2.Name(),
+				},
+			},
+		},
+	}
+
+	// Create the master and the scheduler with the test plugin set.
+	context := initTestSchedulerForFrameworkTest(t, initTestMaster(t, "multi-permit-plugin", nil), 2,
+		scheduler.WithFrameworkPlugins(plugins),
+		scheduler.WithFrameworkDefaultRegistry(registry))
+	defer cleanupTest(t, context)
+
+	// Both permit plugins will return Wait for permitting
+	perPlugin1.timeoutPermit = true
+	perPlugin2.timeoutPermit = true
+
+	// Create a test pod.
+	podName := "test-pod"
+	pod, err := createPausePod(context.clientSet,
+		initPausePod(context.clientSet, &pausePodConfig{Name: podName, Namespace: context.ns.Name}))
+	if err != nil {
+		t.Errorf("Error while creating a test pod: %v", err)
+	}
+
+	var waitingPod framework.WaitingPod
+	// Wait until the test pod is actually waiting.
+	wait.Poll(10*time.Millisecond, 30*time.Second, func() (bool, error) {
+		waitingPod = perPlugin1.fh.GetWaitingPod(pod.UID)
+		return waitingPod != nil, nil
+	})
+
+	// Check the number of pending permits
+	if l := len(waitingPod.GetPendingPlugins()); l != 2 {
+		t.Errorf("Expected the number of pending plugins is 2, but got %d", l)
+	}
+
+	perPlugin1.allowAllPods()
+	// Check the number of pending permits
+	if l := len(waitingPod.GetPendingPlugins()); l != 1 {
+		t.Errorf("Expected the number of pending plugins is 1, but got %d", l)
+	}
+
+	perPlugin2.allowAllPods()
+	if err = waitForPodToSchedule(context.clientSet, pod); err != nil {
+		t.Errorf("Expected the pod to be scheduled. error: %v", err)
+	}
+
+	if perPlugin1.numPermitCalled == 0 || perPlugin2.numPermitCalled == 0 {
+		t.Errorf("Expected the permit plugin to be called.")
+	}
+
+	cleanupPods(context.clientSet, t, []*v1.Pod{pod})
+}
+
 // TestCoSchedulingWithPermitPlugin tests invocation of permit plugins.
 func TestCoSchedulingWithPermitPlugin(t *testing.T) {
 	// Create a plugin registry for testing. Register only a permit plugin.
-	permitPlugin := &PermitPlugin{}
+	permitPlugin := &PermitPlugin{name: permitPluginName}
 	registry := framework.Registry{permitPluginName: newPermitPlugin(permitPlugin)}
 
 	// Setup initial permit plugin for testing.
@@ -1242,18 +1261,10 @@ func TestCoSchedulingWithPermitPlugin(t *testing.T) {
 			},
 		},
 	}
-	// Set permit plugin config for testing
-	pluginConfig := []schedulerconfig.PluginConfig{
-		{
-			Name: permitPluginName,
-			Args: runtime.Unknown{},
-		},
-	}
 
 	// Create the master and the scheduler with the test plugin set.
 	context := initTestSchedulerForFrameworkTest(t, initTestMaster(t, "permit-plugin", nil), 2,
 		scheduler.WithFrameworkPlugins(plugins),
-		scheduler.WithFrameworkPluginConfig(pluginConfig),
 		scheduler.WithFrameworkDefaultRegistry(registry))
 	defer cleanupTest(t, context)
 
@@ -1433,18 +1444,10 @@ func TestPreemptWithPermitPlugin(t *testing.T) {
 			},
 		},
 	}
-	// Set permit plugin config for testing
-	pluginConfig := []schedulerconfig.PluginConfig{
-		{
-			Name: permitPluginName,
-			Args: runtime.Unknown{},
-		},
-	}
 
 	// Create the master and the scheduler with the test plugin set.
 	context := initTestSchedulerForFrameworkTest(t, initTestMaster(t, "preempt-with-permit-plugin", nil), 0,
 		scheduler.WithFrameworkPlugins(plugins),
-		scheduler.WithFrameworkPluginConfig(pluginConfig),
 		scheduler.WithFrameworkDefaultRegistry(registry))
 	defer cleanupTest(t, context)