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` 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{}