From bb7222fcb6aa55b4431236c0a51a539af539f7d5 Mon Sep 17 00:00:00 2001 From: Viktor Voytovych Date: Tue, 3 Nov 2026 16:30:57 +0200 Subject: [PATCH 1/2] test: Start the controller package's API servers once The controller package started an envtest API server for nearly every test, about 180 of them, and spent over half its time on their starts and stops. The tests on NSO's and the locations CRDs now share the package's two planes, started at first use and stopped by TestMain, as kubebuilder's suite_test.go shares one environment. A plane now outlives a test, so a name a scenario takes from its test's name ends in a number no other scenario holds, also under -count, and a test deletes the cluster-scoped objects it writes. Refs #837 --- .../controller/connector_lease_watch_test.go | 30 ++-- .../connectoradvertisement_repoint_test.go | 22 +-- .../edgereachability_controller_test.go | 5 +- .../controller/network_controller_test.go | 3 +- .../networkcontext_controller_test.go | 5 +- .../networkinterface_controller_test.go | 2 + .../networkinterface_projection_test.go | 72 +------- .../networkinterfaceclaim_controller_test.go | 44 +---- ...kloadbalancercellintent_controller_test.go | 7 +- .../networkpresence_controller_test.go | 8 +- .../controller/networkpresence_watch_test.go | 2 +- .../networkservice_controller_test.go | 13 +- internal/controller/subnet_controller_test.go | 8 +- internal/controller/suite_test.go | 155 ++++++++++++++++++ .../vpcendpointslice_writeback_test.go | 5 +- 15 files changed, 198 insertions(+), 183 deletions(-) create mode 100644 internal/controller/suite_test.go diff --git a/internal/controller/connector_lease_watch_test.go b/internal/controller/connector_lease_watch_test.go index 28fb4068..f0b76a18 100644 --- a/internal/controller/connector_lease_watch_test.go +++ b/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,26 +32,13 @@ 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)) @@ -59,8 +46,9 @@ func TestConnectorEngagesClusterWithoutLeaseDiscovery(t *testing.T) { 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"}, diff --git a/internal/controller/connectoradvertisement_repoint_test.go b/internal/controller/connectoradvertisement_repoint_test.go index f1aa30e8..1d304c88 100644 --- a/internal/controller/connectoradvertisement_repoint_test.go +++ b/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 } diff --git a/internal/controller/edgereachability_controller_test.go b/internal/controller/edgereachability_controller_test.go index 30c5ebb3..e3b73b46 100644 --- a/internal/controller/edgereachability_controller_test.go +++ b/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)) diff --git a/internal/controller/network_controller_test.go b/internal/controller/network_controller_test.go index d669518c..37bcb6b2 100644 --- a/internal/controller/network_controller_test.go +++ b/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{ diff --git a/internal/controller/networkcontext_controller_test.go b/internal/controller/networkcontext_controller_test.go index 1aa9b51f..48fe5e1b 100644 --- a/internal/controller/networkcontext_controller_test.go +++ b/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 { diff --git a/internal/controller/networkinterface_controller_test.go b/internal/controller/networkinterface_controller_test.go index 545785b6..ed669073 100644 --- a/internal/controller/networkinterface_controller_test.go +++ b/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 } diff --git a/internal/controller/networkinterface_projection_test.go b/internal/controller/networkinterface_projection_test.go index 95c1f914..4cb21a9d 100644 --- a/internal/controller/networkinterface_projection_test.go +++ b/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{} diff --git a/internal/controller/networkinterfaceclaim_controller_test.go b/internal/controller/networkinterfaceclaim_controller_test.go index d0e77ef0..1882a870 100644 --- a/internal/controller/networkinterfaceclaim_controller_test.go +++ b/internal/controller/networkinterfaceclaim_controller_test.go @@ -27,7 +27,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation" - clientgoscheme "k8s.io/client-go/kubernetes/scheme" eventsv1client "k8s.io/client-go/kubernetes/typed/events/v1" "k8s.io/client-go/rest" "k8s.io/client-go/tools/events" @@ -35,7 +34,6 @@ import ( fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" "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" "sigs.k8s.io/yaml" @@ -390,31 +388,12 @@ func publicV4Class() *ipamv1alpha1.IPClass { return class } +// startNetworkInterfaceEnv returns the package's first plane, which a scenario +// here uses as its cell, its hub or its project's control plane. func startNetworkInterfaceEnv(t *testing.T) (client.Client, *rest.Config) { t.Helper() - if os.Getenv("KUBEBUILDER_ASSETS") == "" { - t.Skip("KUBEBUILDER_ASSETS unset; run via `make test` to exercise envtest") - } - - testScheme := runtime.NewScheme() - require.NoError(t, clientgoscheme.AddToScheme(testScheme)) - require.NoError(t, networkingv1alpha.AddToScheme(testScheme)) - require.NoError(t, locationsv1alpha1.AddToScheme(testScheme)) - - 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() }) - - cl, err := client.New(cfg, client.Options{Scheme: testScheme}) - require.NoError(t, err) - return cl, cfg + cell := sharedPlanes(t)[0] + return cell.client, cell.config } type scenario struct { @@ -435,7 +414,7 @@ func newScenario(t *testing.T, labelled bool, networkFamilies []networkingv1alph cl, restConfig := startNetworkInterfaceEnv(t) ctx := context.Background() - namespaceName := "ns-" + sanitizeName(strings.ToLower(t.Name())) + namespaceName := "ns-" + scenarioSuffix(t) namespace := &corev1.Namespace{} namespace.Name = namespaceName @@ -536,19 +515,6 @@ func (c *transientNamespaceClient) Get( return c.Client.Get(ctx, key, obj, opts...) } -func sanitizeName(in string) string { - var b strings.Builder - for _, r := range in { - switch { - case r >= 'a' && r <= 'z', r >= '0' && r <= '9', r == '-': - b.WriteRune(r) - default: - b.WriteRune('-') - } - } - return strings.Trim(b.String(), "-") -} - func (s *scenario) createClaim(name string, spec networkingv1alpha.NetworkInterfaceClaimSpec) *networkingv1alpha.NetworkInterfaceClaim { s.t.Helper() claim := &networkingv1alpha.NetworkInterfaceClaim{} diff --git a/internal/controller/networkloadbalancercellintent_controller_test.go b/internal/controller/networkloadbalancercellintent_controller_test.go index c61ff28e..d0456395 100644 --- a/internal/controller/networkloadbalancercellintent_controller_test.go +++ b/internal/controller/networkloadbalancercellintent_controller_test.go @@ -496,19 +496,20 @@ func TestCellIntentPublishKeepsLabelsItDoesNotOwn(t *testing.T) { parent.UID = types.UID("parent-1") parent.Generation = 5 parent.Status.Endpoints = []networkingv1alpha.NetworkLoadBalancerEndpointStatus{allocatedCellIntentEndpoint("2001:db8::1", parent.Generation)} - namespace := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test", UID: types.UID("project-ns")}} + namespace := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test", UID: types.UID(scenarioSuffix(t))}} project := fake.NewClientBuilder().WithScheme(networkServiceTestScheme(t)). WithObjects(namespace, route, parent, readyIntentNetwork(), testRouteNetworkService(), testRouteMember("member", true, "fd00::1/128")). WithStatusSubresource(route, parent).Build() hub, _ := startNetworkInterfaceEnv(t) - require.NoError(t, hub.Create(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ns-project-ns"}})) + hubNamespace := "ns-" + string(namespace.UID) + require.NoError(t, hub.Create(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: hubNamespace}})) reconciler := &NetworkLoadBalancerCellIntentReconciler{HubCluster: &fakeCluster{cl: hub}} key := client.ObjectKeyFromObject(parent) _, err := reconciler.reconcileParentIntents(ctx, "project", project, key) require.NoError(t, err) var published networkingv1alpha.NetworkLoadBalancerCellIntentList - require.NoError(t, hub.List(ctx, &published)) + require.NoError(t, hub.List(ctx, &published, client.InNamespace(hubNamespace))) require.Len(t, published.Items, 1) claimed := &published.Items[0] claimed.Labels[karmadaClaimLabel] = "nso-resources-id" diff --git a/internal/controller/networkpresence_controller_test.go b/internal/controller/networkpresence_controller_test.go index 56692d5f..ac62a6b5 100644 --- a/internal/controller/networkpresence_controller_test.go +++ b/internal/controller/networkpresence_controller_test.go @@ -5,7 +5,6 @@ package controller import ( "context" "fmt" - "strings" "testing" "time" @@ -84,10 +83,7 @@ func newPresenceScenario(t *testing.T, opts presenceOptions) *presenceScenario { cl, cfg := startNetworkInterfaceEnv(t) ctx := context.Background() - suffix := sanitizeName(strings.ToLower(t.Name())) - if len(suffix) > 48 { - suffix = suffix[:48] - } + suffix := scenarioSuffix(t) s := &presenceScenario{ t: t, ctx: ctx, @@ -414,7 +410,7 @@ func TestNetworkPresenceLeavesProjectPlaneBindingsAlone(t *testing.T) { s := newPresenceScenario(t, presenceOptions{}) plain := &corev1.Namespace{} - plain.Name = "plane-" + sanitizeName(strings.ToLower(t.Name()))[:40] + plain.Name = "plane-" + scenarioSuffix(t) require.NoError(t, s.hub.Create(s.ctx, plain)) binding := &networkingv1alpha.NetworkBinding{} diff --git a/internal/controller/networkpresence_watch_test.go b/internal/controller/networkpresence_watch_test.go index 9fc358b2..8daa1fae 100644 --- a/internal/controller/networkpresence_watch_test.go +++ b/internal/controller/networkpresence_watch_test.go @@ -412,7 +412,7 @@ func (s *presenceScenario) hubCopy(namespace, network, location string) reconcil func TestANetworkMirrorMapsToItsPresencesInItsNamespace(t *testing.T) { s := newPresenceScenario(t, presenceOptions{}) other := &corev1.Namespace{} - other.Name = "other-" + s.hubNamespace[:40] + other.Name = "other-" + s.hubNamespace require.NoError(t, s.hub.Create(s.ctx, other)) want := []reconcile.Request{ diff --git a/internal/controller/networkservice_controller_test.go b/internal/controller/networkservice_controller_test.go index f5ea8f6d..fa09c9db 100644 --- a/internal/controller/networkservice_controller_test.go +++ b/internal/controller/networkservice_controller_test.go @@ -4,8 +4,6 @@ package controller import ( "context" - "os" - "strings" "testing" "github.com/stretchr/testify/require" @@ -27,17 +25,10 @@ type networkServiceScenario struct { func newNetworkServiceScenario(t *testing.T) *networkServiceScenario { 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) - + cl := sharedPlanes(t)[0].client ctx := context.Background() - cl := clients[0] - namespaceName := "ns-" + sanitizeName(strings.ToLower(t.Name())) + namespaceName := "ns-" + scenarioSuffix(t) namespace := &corev1.Namespace{} namespace.Name = namespaceName require.NoError(t, cl.Create(ctx, namespace)) diff --git a/internal/controller/subnet_controller_test.go b/internal/controller/subnet_controller_test.go index dce25732..739d9c25 100644 --- a/internal/controller/subnet_controller_test.go +++ b/internal/controller/subnet_controller_test.go @@ -4,7 +4,6 @@ package controller import ( "context" - "strings" "testing" "github.com/stretchr/testify/require" @@ -30,7 +29,7 @@ func TestSubnetPublishesTheRangeItsSpecCarries(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)) subnet := &networkingv1alpha.Subnet{} @@ -67,7 +66,7 @@ func TestSubnetWithoutARangeIsStillAllocatedFromItsLocation(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)) location := &locationsv1alpha1.Location{} @@ -77,6 +76,7 @@ func TestSubnetWithoutARangeIsStillAllocatedFromItsLocation(t *testing.T) { Topology: map[string]string{locationsv1alpha1.TopologyCityCodeKey: "DFW"}, } require.NoError(t, cl.Create(ctx, location)) + t.Cleanup(func() { require.NoError(t, client.IgnoreNotFound(cl.Delete(ctx, location))) }) networkContext := &networkingv1alpha.NetworkContext{} networkContext.Namespace = namespace.Name @@ -199,7 +199,7 @@ func TestSubnetReadyFollowsTheCellsReport(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)) subnet := &networkingv1alpha.Subnet{} diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go new file mode 100644 index 00000000..b7915d04 --- /dev/null +++ b/internal/controller/suite_test.go @@ -0,0 +1,155 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package controller + +import ( + "os" + "path/filepath" + "strconv" + "strings" + "sync" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + + locationsv1alpha1 "go.miloapis.com/locations/api/v1alpha1" + + networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" +) + +// plane is one of the package's API servers, with a client for it. +type plane struct { + client client.Client + config *rest.Config +} + +// The planes are started once for the package, at first use, and TestMain +// stops them, as kubebuilder's suite_test.go starts one environment for a +// package. An apiserver takes about 3 s to start, so one per test came to over +// half of the package's time. Every scenario names its own namespaces, so +// sharing them isolates nothing less. +var planes = sync.OnceValues(func() ([]plane, 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 + } + + started := make([]plane, 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 + } + started = append(started, plane{client: cl, config: cfg}) + } + + return started, nil +}) + +var planeEnvironments []*envtest.Environment + +func TestMain(m *testing.M) { + code := m.Run() + for _, env := range planeEnvironments { + _ = env.Stop() + } + os.Exit(code) +} + +// sharedPlanes returns the package's two planes, or skips a test run without +// envtest's binaries. +func sharedPlanes(t *testing.T) []plane { + t.Helper() + if os.Getenv("KUBEBUILDER_ASSETS") == "" { + t.Skip("KUBEBUILDER_ASSETS unset; run via `make test` to exercise envtest") + } + + started, err := planes() + require.NoError(t, err) + return started +} + +func startPlanes(t *testing.T) (cellPlane, hubPlane client.Client) { + t.Helper() + started := sharedPlanes(t) + return started[0].client, started[1].client +} + +// scenarioSuffixLength leaves a scenario 17 characters for a prefix in front of +// its suffix: a namespace's name is a DNS label of at most 63. +const scenarioSuffixLength = 46 + +// scenarios numbers the scenarios of this test binary. +var scenarios atomic.Uint64 + +// scenarioSuffix returns t's name as a DNS label, cut to fit, ending in a +// number no other scenario in this test binary holds. The planes outlive a +// test, so a name taken from t's name alone meets the name of another test +// that starts the same way, or its own under -count. +func scenarioSuffix(t *testing.T) string { + t.Helper() + number := "-" + strconv.FormatUint(scenarios.Add(1), 10) + name := sanitizeName(strings.ToLower(t.Name())) + if room := scenarioSuffixLength - len(number); len(name) > room { + name = name[:room] + } + return name + number +} + +// Two tests whose names share their start, and one test run twice as under +// -count, still get suffixes of their own, each fitting a namespace's name +// behind the longest prefix. +func TestScenarioSuffixesAreUniqueAndFit(t *testing.T) { + const longestPrefix = "proj-unfederated-" + shared := strings.Repeat("a-shared-start-", 4) + seen := map[string]bool{} + for _, name := range []string{shared + "one", shared + "two", "short"} { + t.Run(name, func(t *testing.T) { + for range 2 { + suffix := scenarioSuffix(t) + require.Empty(t, validation.IsDNS1123Label(longestPrefix+suffix), suffix) + require.False(t, seen[suffix], "%s is taken", suffix) + seen[suffix] = true + } + }) + } +} + +func sanitizeName(in string) string { + var b strings.Builder + for _, r := range in { + switch { + case r >= 'a' && r <= 'z', r >= '0' && r <= '9', r == '-': + b.WriteRune(r) + default: + b.WriteRune('-') + } + } + return strings.Trim(b.String(), "-") +} diff --git a/internal/controller/vpcendpointslice_writeback_test.go b/internal/controller/vpcendpointslice_writeback_test.go index 8481f1bd..ced338ad 100644 --- a/internal/controller/vpcendpointslice_writeback_test.go +++ b/internal/controller/vpcendpointslice_writeback_test.go @@ -49,8 +49,9 @@ func newReachability(t *testing.T) *reachability { cellPlane, hubPlane := startPlanes(t) ctx := context.Background() - namespace := "ns-" + sanitizeName(strings.ToLower(t.Name())) - projectNamespace := "proj-" + sanitizeName(strings.ToLower(t.Name())) + suffix := scenarioSuffix(t) + namespace := "ns-" + suffix + projectNamespace := "proj-" + suffix for _, cl := range []client.Client{cellPlane, hubPlane} { ns := &corev1.Namespace{} From d55947158c7209ddbb778e9f3e71ddc8a06b5c66 Mon Sep 17 00:00:00 2001 From: Viktor Voytovych Date: Tue, 3 Nov 2026 18:13:41 +0200 Subject: [PATCH 2/2] ci: Give the test run Go's default timeout again The test timeout went to 20 minutes and then 25 because the controller package's envtest suite outgrew Go's default of 10. With its API servers started once, the package takes under 3 minutes in CI, so the flag and its comments go, and a hung test binary is stopped after 10 minutes. Refs #837 --- .github/workflows/ci.yaml | 4 +--- Makefile | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index eb240c30..b766d233 100644 --- a/.github/workflows/ci.yaml +++ b/.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) diff --git a/Makefile b/Makefile index f2c9405f..536bad55 100644 --- a/Makefile +++ b/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`