test: Start the controller package's API servers once

.github/workflows/ci.yaml

@@ -13,9 +13,7 @@ jobs:
     uses: datum-cloud/actions/.github/workflows/ci-basic.yaml@v1.26.0
     with:
       skip: docker,test
-      # The controller package's envtest suite outgrew 20 minutes; 25 until it is
-      # split (#837). The job must stay under ci-basic's 30-minute cap.
       run: |
         make locations-crds
         go install sigs.k8s.io/controller-runtime/tools/setup-envtest@32759fab31191897ae86091d8831b2d4b890aedd
-        KUBEBUILDER_ASSETS="$(setup-envtest use 1.31.0 -p path)" go test $(go list ./... | grep -v /e2e) -timeout 25m
+        KUBEBUILDER_ASSETS="$(setup-envtest use 1.31.0 -p path)" go test $(go list ./... | grep -v /e2e)

Makefile

@@ -65,10 +65,9 @@ locations-crds: kustomize ## Render the locations service CRDs envtest installs.
 	mkdir -p $(LOCALBIN)/crds/locations
 	$(KUSTOMIZE) build config/tools/locations-crds -o $(LOCALBIN)/crds/locations
 
-# The test timeout matches ci.yaml's, which says why.
 .PHONY: test
 test: manifests generate fmt vet envtest locations-crds ## Run tests.
-	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test $$(go list ./... | grep -v /e2e) -timeout 25m -coverprofile cover.out
+	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test $$(go list ./... | grep -v /e2e) -coverprofile cover.out
 
 # The e2e suite runs against the two-cluster prod-fidelity env; bring it up and
 # run it with `task test-infra:up` then `task test-infra:test-e2e`

internal/controller/connector_lease_watch_test.go

@@ -5,23 +5,23 @@ package controller
 import (
 	"context"
 	"fmt"
-	"os"
-	"path/filepath"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/require"
 	coordinationv1 "k8s.io/api/coordination/v1"
+	corev1 "k8s.io/api/core/v1"
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/cluster"
-	"sigs.k8s.io/controller-runtime/pkg/envtest"
+	ctrlconfig "sigs.k8s.io/controller-runtime/pkg/config"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
@@ -32,35 +32,23 @@ import (
 )
 
 func TestConnectorEngagesClusterWithoutLeaseDiscovery(t *testing.T) {
-	if os.Getenv("KUBEBUILDER_ASSETS") == "" {
-		t.Skip("KUBEBUILDER_ASSETS unset; run via `make test` to exercise envtest")
-	}
+	cfg := sharedPlanes(t)[0].config
 
 	log.SetLogger(zap.New(zap.UseDevMode(true)))
 
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	env := &envtest.Environment{
-		CRDDirectoryPaths: []string{
-			filepath.Join("..", "..", "config", "crd", "bases"),
-			filepath.Join("..", "..", "bin", "crds", "locations"),
-		},
-		ErrorIfCRDPathMissing: true,
-	}
-	cfg, err := env.Start()
-	require.NoError(t, err)
-	t.Cleanup(func() { _ = env.Stop() })
-
 	testScheme := runtime.NewScheme()
 	require.NoError(t, clientgoscheme.AddToScheme(testScheme))
 	require.NoError(t, networkingv1alpha1.AddToScheme(testScheme))
 
 	provider := &staticProvider{clusters: map[string]cluster.Cluster{}}
 
 	mgr, err := mcmanager.New(cfg, provider, ctrl.Options{
-		Scheme:  testScheme,
-		Metrics: metricsserver.Options{BindAddress: "0"},
+		Scheme:     testScheme,
+		Metrics:    metricsserver.Options{BindAddress: "0"},
+		Controller: ctrlconfig.Controller{SkipNameValidation: ptr.To(true)},
 	})
 	require.NoError(t, err)
 
@@ -71,7 +59,9 @@ func TestConnectorEngagesClusterWithoutLeaseDiscovery(t *testing.T) {
 	directClient, err := client.New(cfg, client.Options{Scheme: testScheme})
 	require.NoError(t, err)
 
-	connectorKey := client.ObjectKey{Namespace: "default", Name: "connector"}
+	namespace := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "lease-" + scenarioSuffix(t)}}
+	require.NoError(t, directClient.Create(ctx, namespace))
+	connectorKey := client.ObjectKey{Namespace: namespace.Name, Name: "connector"}
 	connector := &networkingv1alpha1.Connector{
 		ObjectMeta: metav1.ObjectMeta{Name: connectorKey.Name, Namespace: connectorKey.Namespace},
 		Spec:       networkingv1alpha1.ConnectorSpec{ConnectorClassName: "missing"},

internal/controller/connectoradvertisement_repoint_test.go

@@ -6,8 +6,6 @@ import (
 	"context"
 	"fmt"
 	"math/rand/v2"
-	"os"
-	"path/filepath"
 	"sync"
 	"sync/atomic"
 	"testing"
@@ -29,7 +27,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
-	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	mcreconcile "sigs.k8s.io/multicluster-runtime/pkg/reconcile"
 
@@ -541,20 +538,10 @@ func connectorControllers(ad networkingv1alpha1.ConnectorAdvertisement) int {
 	return count
 }
 
-// startRepointEnv starts an API server with NSO's CRDs and returns a client
-// for it.
+// startRepointEnv returns a client for the package's first plane.
 func startRepointEnv(t *testing.T) client.WithWatch {
 	t.Helper()
-	if os.Getenv("KUBEBUILDER_ASSETS") == "" {
-		t.Skip("KUBEBUILDER_ASSETS unset; run via `make test` to exercise envtest")
-	}
-	env := &envtest.Environment{
-		CRDDirectoryPaths:     []string{filepath.Join("..", "..", "config", "crd", "bases")},
-		ErrorIfCRDPathMissing: true,
-	}
-	cfg, err := env.Start()
-	require.NoError(t, err)
-	t.Cleanup(func() { _ = env.Stop() })
+	cfg := sharedPlanes(t)[0].config
 
 	testScheme := runtime.NewScheme()
 	require.NoError(t, clientgoscheme.AddToScheme(testScheme))
@@ -627,12 +614,9 @@ func requireAcceptedForCurrentGeneration(t *testing.T, ad *networkingv1alpha1.Co
 	require.Equal(t, ad.Generation, cond.ObservedGeneration, "Accepted must describe the spec that names the current connector")
 }
 
-var namespaceSeq int
-
 func newNamespace(t *testing.T, cl client.Client) string {
 	t.Helper()
-	namespaceSeq++
-	ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("repoint-%d", namespaceSeq)}}
+	ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "repoint-" + scenarioSuffix(t)}}
 	require.NoError(t, cl.Create(context.Background(), ns))
 	return ns.Name
 }

internal/controller/edgereachability_controller_test.go

@@ -4,7 +4,6 @@ package controller
 
 import (
 	"context"
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -39,7 +38,7 @@ func newExposure(t *testing.T) *exposure {
 	ctx := context.Background()
 
 	namespace := &corev1.Namespace{}
-	namespace.Name = "proj-" + sanitizeName(strings.ToLower(t.Name()))
+	namespace.Name = "proj-" + scenarioSuffix(t)
 	require.NoError(t, projectPlane.Create(ctx, namespace))
 
 	hubNamespace := &corev1.Namespace{}
@@ -278,7 +277,7 @@ func TestNoRecordIsWrittenWithoutAHubNamespace(t *testing.T) {
 	e := newExposure(t)
 
 	orphan := &corev1.Namespace{}
-	orphan.Name = "proj-unfederated-" + sanitizeName(strings.ToLower(t.Name()))
+	orphan.Name = "proj-unfederated-" + scenarioSuffix(t)
 	require.NoError(t, e.project.Create(e.ctx, orphan))
 
 	require.NoError(t, e.reconciler.record(e.ctx, "cluster-"+testProject, e.project, orphan.Name))

internal/controller/network_controller_test.go

@@ -6,7 +6,6 @@ import (
 	"context"
 	"errors"
 	"net/netip"
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -51,7 +50,7 @@ func newNetworkScenario(t *testing.T, ipam *fakeIPAM) *networkScenario {
 	cl, _ := startNetworkInterfaceEnv(t)
 	ctx := context.Background()
 
-	namespaceName := "ns-" + sanitizeName(strings.ToLower(t.Name()))
+	namespaceName := "ns-" + scenarioSuffix(t)
 	namespace := &corev1.Namespace{}
 	namespace.Name = namespaceName
 	namespace.Labels = map[string]string{

internal/controller/networkcontext_controller_test.go

@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"net/netip"
 	"slices"
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -52,7 +51,7 @@ func newNetworkContextScenario(t *testing.T, ipam *fakeIPAM) *networkContextScen
 	cl, _ := startNetworkInterfaceEnv(t)
 	ctx := context.Background()
 
-	namespaceName := "ns-" + sanitizeName(strings.ToLower(t.Name()))
+	namespaceName := "ns-" + scenarioSuffix(t)
 	namespace := &corev1.Namespace{}
 	namespace.Name = namespaceName
 	namespace.Labels = map[string]string{
@@ -721,7 +720,7 @@ func TestANetworkTakesDownOnlyItsOwnLocations(t *testing.T) {
 	ctx := context.Background()
 
 	namespace := &corev1.Namespace{}
-	namespace.Name = "ns-" + sanitizeName(strings.ToLower(t.Name()))
+	namespace.Name = "ns-" + scenarioSuffix(t)
 	require.NoError(t, cl.Create(ctx, namespace))
 
 	newNetwork := func(name string) *networkingv1alpha.Network {

internal/controller/networkinterface_controller_test.go

@@ -54,6 +54,8 @@ func (s *scenario) servedByDelivery() *locationsv1alpha1.ServingLocation {
 	served.Name = testLocationName
 	served.Spec.Topology = map[string]string{"topology.datum.net/city-code": "DFW"}
 	require.NoError(s.t, s.client.Create(s.ctx, served))
+	// A cell has one ServingLocation, and the planes outlive this test.
+	s.t.Cleanup(func() { require.NoError(s.t, client.IgnoreNotFound(s.client.Delete(s.ctx, served))) })
 	return served
 }
 

internal/controller/networkinterface_projection_test.go

@@ -5,10 +5,7 @@ package controller
 import (
 	"context"
 	"net/http"
-	"os"
-	"path/filepath"
 	"strings"
-	"sync"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -17,18 +14,14 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/cluster"
-	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	"sigs.k8s.io/controller-runtime/pkg/recorder"
 
-	locationsv1alpha1 "go.miloapis.com/locations/api/v1alpha1"
-
 	networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha"
 	"go.datum.net/network-services-operator/internal/config"
 	"go.datum.net/network-services-operator/internal/downstreamclient"
@@ -85,74 +78,13 @@ type visibility struct {
 	collector *NetworkInterfaceProjectionGCReconciler
 }
 
-// The planes are started once for the package. An apiserver takes long enough
-// that one per test does not finish, and every scenario names its own
-// namespaces, so sharing them isolates nothing less.
-var planes = sync.OnceValues(func() ([]client.Client, error) {
-	testScheme := runtime.NewScheme()
-	if err := clientgoscheme.AddToScheme(testScheme); err != nil {
-		return nil, err
-	}
-	if err := networkingv1alpha.AddToScheme(testScheme); err != nil {
-		return nil, err
-	}
-	if err := locationsv1alpha1.AddToScheme(testScheme); err != nil {
-		return nil, err
-	}
-
-	clients := make([]client.Client, 0, 2)
-	for range 2 {
-		env := &envtest.Environment{
-			CRDDirectoryPaths: []string{
-				filepath.Join("..", "..", "config", "crd", "bases"),
-				filepath.Join("..", "..", "bin", "crds", "locations"),
-			},
-			ErrorIfCRDPathMissing: true,
-		}
-		cfg, err := env.Start()
-		if err != nil {
-			return nil, err
-		}
-		planeEnvironments = append(planeEnvironments, env)
-
-		cl, err := client.New(cfg, client.Options{Scheme: testScheme})
-		if err != nil {
-			return nil, err
-		}
-		clients = append(clients, cl)
-	}
-
-	return clients, nil
-})
-
-var planeEnvironments []*envtest.Environment
-
-func TestMain(m *testing.M) {
-	code := m.Run()
-	for _, env := range planeEnvironments {
-		_ = env.Stop()
-	}
-	os.Exit(code)
-}
-
-func startPlanes(t *testing.T) (cellPlane, hubPlane client.Client) {
-	t.Helper()
-	if os.Getenv("KUBEBUILDER_ASSETS") == "" {
-		t.Skip("KUBEBUILDER_ASSETS unset; run via `make test` to exercise envtest")
-	}
-
-	clients, err := planes()
-	require.NoError(t, err)
-	return clients[0], clients[1]
-}
-
 func newVisibility(t *testing.T) *visibility {
 	t.Helper()
 
 	cellPlane, hubPlane := startPlanes(t)
 	ctx := context.Background()
 
-	suffix := sanitizeName(strings.ToLower(t.Name()))
+	suffix := scenarioSuffix(t)
 	cellNamespace := "ns-" + suffix
 	projectNamespace := "proj-" + suffix
 
@@ -655,7 +587,7 @@ func TestAnUnroutableInterfaceIsNotPublished(t *testing.T) {
 	v := newVisibility(t)
 
 	namespace := &corev1.Namespace{}
-	namespace.Name = "unlabelled-" + sanitizeName(strings.ToLower(t.Name()))
+	namespace.Name = "unlabelled-" + scenarioSuffix(t)
 	require.NoError(t, v.cell.Create(v.ctx, namespace))
 
 	iface := &networkingv1alpha.NetworkInterface{}