Merge pull request #3489 from xrstf/improve-auth-webhook-tests

Improve authorizer webhook tests

Author: kcp-ci-bot

test/e2e/authorizer/authorizationorder_test.go

@@ -19,6 +19,7 @@ package authorizer
 import (
 	"context"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 
@@ -42,8 +43,8 @@ func TestAuthorizationOrder(t *testing.T) {
 		webhookPort := "8080"
 		ctx, cancelFunc := context.WithCancel(context.Background())
 		t.Cleanup(cancelFunc)
-		webhookStop := RunWebhook(ctx, t, webhookPort, "kubernetes:authz:allow")
-		t.Cleanup(webhookStop)
+		webhook1Stop := RunWebhook(ctx, t, webhookPort, "kubernetes:authz:allow")
+		t.Cleanup(webhook1Stop)
 
 		server, kcpClusterClient, kubeClusterClient := setupTest(t, "AlwaysAllowGroups,AlwaysAllowPaths,Webhook,RBAC", "testdata/webhook1.kubeconfig")
 
@@ -52,8 +53,9 @@ func TestAuthorizationOrder(t *testing.T) {
 		require.NoError(t, err)
 
 		// stop the webhook and switch to a deny policy
-		webhookStop()
-		RunWebhook(ctx, t, webhookPort, "kubernetes:authz:deny")
+		webhook1Stop()
+		webhook2Stop := RunWebhook(ctx, t, webhookPort, "kubernetes:authz:deny")
+		t.Cleanup(webhook2Stop)
 
 		t.Log("Admin should not be allowed to list ConfigMaps.")
 		_, err = kubeClusterClient.Cluster(logicalcluster.NewPath("root")).CoreV1().ConfigMaps("default").List(ctx, metav1.ListOptions{})
@@ -68,8 +70,8 @@ func TestAuthorizationOrder(t *testing.T) {
 		webhookPort := "8081"
 		ctx, cancelFunc := context.WithCancel(context.Background())
 		t.Cleanup(cancelFunc)
-		webhookStop := RunWebhook(ctx, t, webhookPort, "kubernetes:authz:allow")
-		t.Cleanup(webhookStop)
+		webhook1Stop := RunWebhook(ctx, t, webhookPort, "kubernetes:authz:allow")
+		t.Cleanup(webhook1Stop)
 
 		server, kcpClusterClient, kubeClusterClient := setupTest(t, "Webhook,AlwaysAllowGroups,AlwaysAllowPaths,RBAC", "testdata/webhook2.kubeconfig")
 
@@ -81,8 +83,9 @@ func TestAuthorizationOrder(t *testing.T) {
 		require.NoError(t, err)
 
 		// stop the webhook and switch to a deny policy
-		webhookStop()
-		RunWebhook(ctx, t, webhookPort, "kubernetes:authz:deny")
+		webhook1Stop()
+		webhook2Stop := RunWebhook(ctx, t, webhookPort, "kubernetes:authz:deny")
+		t.Cleanup(webhook2Stop)
 
 		t.Log("Admin should not be allowed now to list Logical clusters.")
 		_, err = kcpClusterClient.Cluster(logicalcluster.NewPath("root")).CoreV1alpha1().LogicalClusters().List(ctx, metav1.ListOptions{})
@@ -124,6 +127,14 @@ func setupTest(t *testing.T, authOrder, webhookConfigFile string) (kcptestingser
 
 	server := kcptesting.PrivateKcpServer(t, kcptestingserver.WithCustomArguments(args...))
 
+	// The testing framework has a rare race condition where if you stop kcp too early after it became "ready",
+	// it will run into loads of shutdown issues and the shutdown will take 3-4 minutes.
+	// This can be easily avoided by simply waiting a few seconds here. Since the tests that use setupTest()
+	// are very, very short anyway, this will not harm the test runtime overall, but make them much more
+	// stable on some certain PCs/laptops.
+	// See https://github.com/kcp-dev/kcp/issues/3488 for more information.
+	time.Sleep(3 * time.Second)
+
 	kcpConfig := server.BaseConfig(t)
 	kubeClusterClient, err := kcpkubernetesclientset.NewForConfig(kcpConfig)
 	require.NoError(t, err)

test/e2e/authorizer/utils.go

@@ -22,6 +22,7 @@ import (
 	"net"
 	"os"
 	"os/exec"
+	"sync"
 	"testing"
 	"time"
 
@@ -66,11 +67,11 @@ func RunWebhook(ctx context.Context, t *testing.T, port string, response string)
 		return true, ""
 	}, wait.ForeverTestTimeout, time.Millisecond*200)
 
-	return func() {
+	return sync.OnceFunc(func() {
 		t.Log("Stopping webhook...")
 		cancel()
-		if err := cmd.Wait(); err != nil {
-			t.Logf("error waiting for webhook to finish: %v", err)
+		if err := cmd.Wait(); err != nil && err.Error() != "signal: killed" {
+			t.Logf("Error waiting for webhook to finish: %v", err)
 		}
-	}
+	})
 }