From 512879fd567af07e511f5dc98f2fe3d231b143de Mon Sep 17 00:00:00 2001 From: Alexander Maslennikov Date: Tue, 4 Aug 2026 16:31:10 +0200 Subject: [PATCH] Split profile render tests by fabric Group Ethernet and InfiniBand rendering coverage under stable Go subtest paths so either fabric can run independently while the default suite continues to run both. Signed-off-by: Alexander Maslennikov --- CONTRIBUTING.md | 5 +- Makefile | 10 +- README.md | 12 +- .../sriov_render_test.go | 172 +++++++++++------- 4 files changed, 131 insertions(+), 68 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5ffc69d..c49f272 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -69,6 +69,9 @@ git checkout -b dev/some-topic-branch * Make your changes to the code and add tests to cover contributed code. * Run `make build && make test` to validate it builds and will not break current functionality. + During profile-render development, `make test-ethernet` and + `make test-infiniband` run the corresponding fabric suite independently; + `make test` remains the required all-tests check. * Commit your changes and push them to your fork. * Open a pull request for the appropriate project. -* Maintainers will review your pull request, suggest changes, run tests and eventually merge or close the request. \ No newline at end of file +* Maintainers will review your pull request, suggest changes, run tests and eventually merge or close the request. diff --git a/Makefile b/Makefile index 09b71d9..ffd3703 100644 --- a/Makefile +++ b/Makefile @@ -32,7 +32,7 @@ PCI_IDS_NVIDIA=pkg/networkoperatorplugin/internal/pciids/nvidia.ids NIC_CONFIG_CRDS_DIR=pkg/nicconfigdaemon/assets/crds NIC_CONFIG_OPERATOR_MODULE=github.com/Mellanox/nic-configuration-operator -.PHONY: all build clean test coverage deps lint docker-build docker-build-local docker-run update-readme download-sosreport update-pci-ids sync-network-operator-releases sync-nic-config-crds release release-snapshot help +.PHONY: all build clean test test-ethernet test-infiniband coverage deps lint docker-build docker-build-local docker-run update-readme download-sosreport update-pci-ids sync-network-operator-releases sync-nic-config-crds release release-snapshot help ## Build the binary build: @@ -74,6 +74,14 @@ clean: test: $(GOTEST) -v ./... +## Run Ethernet profile-render tests +test-ethernet: + $(GOTEST) -v ./pkg/networkoperatorplugin -run '^TestFabricProfileRendering/Ethernet($$|/)' + +## Run InfiniBand profile-render tests +test-infiniband: + $(GOTEST) -v ./pkg/networkoperatorplugin -run '^TestFabricProfileRendering/InfiniBand($$|/)' + ## Run tests with coverage coverage: $(GOTEST) -v -coverprofile=coverage.out ./... diff --git a/README.md b/README.md index 7389ac1..c8b611f 100644 --- a/README.md +++ b/README.md @@ -1391,10 +1391,18 @@ make clean # Clean build artifacts ### Testing ```bash -make test # Run tests -make coverage # Run tests with coverage +make test # Run all tests (Ethernet + InfiniBand + common) +make test-ethernet # Run the Ethernet profile-render suite +make test-infiniband # Run the InfiniBand profile-render suite +make coverage # Run all tests with coverage ``` +The fabric-specific targets use Go named subtests rather than build tags, so +plain `go test ./...` and `make test` continue to run both fabrics by default. +They cover profile rendering, strict multi-document YAML parsing, meta-plugin +output, and SR-IOV separator regressions; the Ethernet suite also covers +Spectrum-X and per-namespace rendering. + ### Linting ```bash diff --git a/pkg/networkoperatorplugin/sriov_render_test.go b/pkg/networkoperatorplugin/sriov_render_test.go index a7851e6..c4b05ef 100644 --- a/pkg/networkoperatorplugin/sriov_render_test.go +++ b/pkg/networkoperatorplugin/sriov_render_test.go @@ -193,22 +193,69 @@ func countKindLines(content string) int { return n } -// TestProfileManifestsAreValidMultiDocYAML is the broad regression guard for -// the glued-`---`-separator class of bug across EVERY standard profile — -// including the IPPool / Network / NodePolicy templates beyond the four SR-IOV -// files that originally regressed. For each rendered manifest it strict-parses -// every document and asserts the document count equals the number of `kind:` -// headers: a glued separator (e.g. `resourceName: foo---`) merges two docs into -// one, which both drops the count and trips UnmarshalStrict on the duplicate -// top-level keys. -func TestProfileManifestsAreValidMultiDocYAML(t *testing.T) { - profilesUnderTest := []struct{ dir, fabric, deployment string }{ - {"sriov-ethernet-rdma", "ethernet", "sriov"}, - {"sriov-ib-rdma", "infiniband", "sriov"}, - {"host-device-rdma", "ethernet", "host_device"}, - {"macvlan-rdma-shared", "ethernet", "rdma_shared"}, - {"ipoib-rdma-shared", "infiniband", "rdma_shared"}, +type profileRenderTestCase struct { + dir string + fabric string + deployment string + fileSubstr string +} + +// TestFabricProfileRendering groups the profile-render regression tests by +// fabric. The named subtests are intentional selectors: Go runs both by +// default, while contributors can use -run '/Ethernet($|/)' or +// -run '/InfiniBand($|/)' to exercise one fabric without build tags. +func TestFabricProfileRendering(t *testing.T) { + ethernetProfiles := []profileRenderTestCase{ + {dir: "sriov-ethernet-rdma", fabric: "ethernet", deployment: "sriov", fileSubstr: "50-sriovnetwork"}, + {dir: "host-device-rdma", fabric: "ethernet", deployment: "host_device", fileSubstr: "30-hostdevicenetwork"}, + {dir: "macvlan-rdma-shared", fabric: "ethernet", deployment: "rdma_shared", fileSubstr: "30-macvlannetwork"}, } + infinibandProfiles := []profileRenderTestCase{ + {dir: "sriov-ib-rdma", fabric: "infiniband", deployment: "sriov", fileSubstr: "50-sriovibnetwork"}, + {dir: "host-device-rdma", fabric: "infiniband", deployment: "host_device", fileSubstr: "30-hostdevicenetwork"}, + {dir: "ipoib-rdma-shared", fabric: "infiniband", deployment: "rdma_shared", fileSubstr: "30-ipoibnetwork"}, + } + + t.Run("Ethernet", func(t *testing.T) { + t.Run("MultiDocumentYAML", func(t *testing.T) { + testProfileManifestsAreValidMultiDocYAML(t, ethernetProfiles) + }) + t.Run("SpectrumXIPv6CIDRPools", testSpectrumXIPv6CIDRPoolRendering) + t.Run("MetaPlugins", func(t *testing.T) { + testNonSpectrumXProfilesRenderMetaPlugins(t, ethernetProfiles) + }) + t.Run("DefaultMetaPlugins", func(t *testing.T) { + testDefaultProfilesDoNotRenderMetaPlugins(t, ethernetProfiles) + }) + t.Run("SRIOVMultiDocSeparators", func(t *testing.T) { + testSRIOVMultiDocSeparators(t, "ethernet") + }) + t.Run("NetworkNamespaces", testNetworkNamespacesFanOut) + }) + + t.Run("InfiniBand", func(t *testing.T) { + t.Run("MultiDocumentYAML", func(t *testing.T) { + testProfileManifestsAreValidMultiDocYAML(t, infinibandProfiles) + }) + t.Run("MetaPlugins", func(t *testing.T) { + testNonSpectrumXProfilesRenderMetaPlugins(t, infinibandProfiles) + }) + t.Run("DefaultMetaPlugins", func(t *testing.T) { + testDefaultProfilesDoNotRenderMetaPlugins(t, infinibandProfiles) + }) + t.Run("SRIOVMultiDocSeparators", func(t *testing.T) { + testSRIOVMultiDocSeparators(t, "infiniband") + }) + }) +} + +// testProfileManifestsAreValidMultiDocYAML is the broad regression guard for +// the glued-`---`-separator class of bug across the supplied profiles. It also +// covers IPPool / Network / NodePolicy templates beyond the four SR-IOV files +// that originally regressed. For each rendered manifest it strict-parses every +// document and asserts the document count equals the number of `kind:` headers. +func testProfileManifestsAreValidMultiDocYAML(t *testing.T, profilesUnderTest []profileRenderTestCase) { + t.Helper() for _, p := range profilesUnderTest { t.Run(p.dir, func(t *testing.T) { rendered := renderProfile(t, p.dir, p.fabric, p.deployment) @@ -245,7 +292,7 @@ func TestProfileManifestsAreValidMultiDocYAML(t *testing.T) { } } -func TestSpectrumXIPv6CIDRPoolRendering(t *testing.T) { +func testSpectrumXIPv6CIDRPoolRendering(t *testing.T) { profilesUnderTest := []struct { dir string spcxVersion string @@ -389,20 +436,8 @@ func TestSecondaryNetworkMetaPluginsHelper(t *testing.T) { }) } -func TestNonSpectrumXProfilesRenderMetaPlugins(t *testing.T) { - profilesUnderTest := []struct { - dir string - fabric string - deployment string - fileSubstr string - }{ - {"sriov-ethernet-rdma", "ethernet", "sriov", "50-sriovnetwork"}, - {"sriov-ib-rdma", "infiniband", "sriov", "50-sriovibnetwork"}, - {"host-device-rdma", "ethernet", "host_device", "30-hostdevicenetwork"}, - {"macvlan-rdma-shared", "ethernet", "rdma_shared", "30-macvlannetwork"}, - {"ipoib-rdma-shared", "infiniband", "rdma_shared", "30-ipoibnetwork"}, - } - +func testNonSpectrumXProfilesRenderMetaPlugins(t *testing.T, profilesUnderTest []profileRenderTestCase) { + t.Helper() for _, p := range profilesUnderTest { t.Run(p.dir, func(t *testing.T) { rendered := renderProfileWithProfile(t, p.dir, &config.Profile{ @@ -430,49 +465,58 @@ func TestNonSpectrumXProfilesRenderMetaPlugins(t *testing.T) { } } -func TestDefaultProfilesDoNotRenderMetaPlugins(t *testing.T) { - rendered := renderProfile(t, "sriov-ethernet-rdma", "ethernet", "sriov") - name, content := fileMatching(t, rendered, "50-sriovnetwork") - for _, doc := range parseDocs(t, name, content) { - _, ok := optionalSpecString(t, doc, "metaPlugins") - require.False(t, ok, "default destination-based routing with ignoreARP=false must not render metaPlugins") +func testDefaultProfilesDoNotRenderMetaPlugins(t *testing.T, profilesUnderTest []profileRenderTestCase) { + t.Helper() + for _, profile := range profilesUnderTest { + t.Run(profile.dir, func(t *testing.T) { + rendered := renderProfile(t, profile.dir, profile.fabric, profile.deployment) + name, content := fileMatching(t, rendered, profile.fileSubstr) + for _, doc := range parseDocs(t, name, content) { + _, ok := optionalSpecString(t, doc, "metaPlugins") + require.False(t, ok, "default destination-based routing with ignoreARP=false must not render metaPlugins") + } + }) } } -// TestSRIOVMultiDocSeparators is the regression test for the broken-formatting +// testSRIOVMultiDocSeparators is the regression test for the broken-formatting // bug: the multirail SR-IOV templates used to glue the `---` document // separator (and, for SriovIBNetwork, the `linkState: enable` key) directly // onto the `resourceName:` line, producing manifests that `kubectl apply` // rejected. Each multi-doc network file must now split into one valid document // per east-west rail with the expected, un-glued field values. -func TestSRIOVMultiDocSeparators(t *testing.T) { +func testSRIOVMultiDocSeparators(t *testing.T, fabric string) { + t.Helper() const wantRails = 8 // mixed-same-type.yaml merges to one 8-rail bucket - t.Run("ethernet sriovnetworknodepolicy", func(t *testing.T) { - rendered := renderSRIOV(t, "sriov-ethernet-rdma", "ethernet", "50-sriovnetwork.yaml") - name, content := fileMatching(t, rendered, "40-sriovnetworknodepolicy") - require.NotContains(t, content, "---apiVersion", "separator must sit on its own line") - docs := parseDocs(t, name, content) - require.Len(t, docs, wantRails) - for i, doc := range docs { - require.Equal(t, "SriovNetworkNodePolicy", doc["kind"]) - require.Equal(t, fmt.Sprintf("sriov_resource_rail_%d", i), specString(t, doc, "resourceName")) - } - }) + if fabric == "ethernet" { + t.Run("sriovnetworknodepolicy", func(t *testing.T) { + rendered := renderSRIOV(t, "sriov-ethernet-rdma", "ethernet", "50-sriovnetwork.yaml") + name, content := fileMatching(t, rendered, "40-sriovnetworknodepolicy") + require.NotContains(t, content, "---apiVersion", "separator must sit on its own line") + docs := parseDocs(t, name, content) + require.Len(t, docs, wantRails) + for i, doc := range docs { + require.Equal(t, "SriovNetworkNodePolicy", doc["kind"]) + require.Equal(t, fmt.Sprintf("sriov_resource_rail_%d", i), specString(t, doc, "resourceName")) + } + }) - t.Run("ethernet sriovnetwork", func(t *testing.T) { - rendered := renderSRIOV(t, "sriov-ethernet-rdma", "ethernet", "50-sriovnetwork.yaml") - name, content := fileMatching(t, rendered, "50-sriovnetwork") - require.NotContains(t, content, "---apiVersion", "separator must sit on its own line") - docs := parseDocs(t, name, content) - require.Len(t, docs, wantRails) - for i, doc := range docs { - require.Equal(t, "SriovNetwork", doc["kind"]) - require.Equal(t, fmt.Sprintf("sriov_resource_rail_%d", i), specString(t, doc, "resourceName")) - } - }) + t.Run("sriovnetwork", func(t *testing.T) { + rendered := renderSRIOV(t, "sriov-ethernet-rdma", "ethernet", "50-sriovnetwork.yaml") + name, content := fileMatching(t, rendered, "50-sriovnetwork") + require.NotContains(t, content, "---apiVersion", "separator must sit on its own line") + docs := parseDocs(t, name, content) + require.Len(t, docs, wantRails) + for i, doc := range docs { + require.Equal(t, "SriovNetwork", doc["kind"]) + require.Equal(t, fmt.Sprintf("sriov_resource_rail_%d", i), specString(t, doc, "resourceName")) + } + }) + return + } - t.Run("infiniband sriovnetworknodepolicy", func(t *testing.T) { + t.Run("sriovnetworknodepolicy", func(t *testing.T) { rendered := renderSRIOV(t, "sriov-ib-rdma", "infiniband", "50-sriovibnetwork.yaml") name, content := fileMatching(t, rendered, "40-sriovnetworknodepolicy") require.NotContains(t, content, "---apiVersion", "separator must sit on its own line") @@ -484,7 +528,7 @@ func TestSRIOVMultiDocSeparators(t *testing.T) { } }) - t.Run("infiniband sriovibnetwork keeps resourceName and linkState separate", func(t *testing.T) { + t.Run("sriovibnetwork keeps resourceName and linkState separate", func(t *testing.T) { rendered := renderSRIOV(t, "sriov-ib-rdma", "infiniband", "50-sriovibnetwork.yaml") name, content := fileMatching(t, rendered, "50-sriovibnetwork") require.NotContains(t, content, "---apiVersion", "separator must sit on its own line") @@ -501,12 +545,12 @@ func TestSRIOVMultiDocSeparators(t *testing.T) { }) } -// TestNetworkNamespacesFanOut covers --network-namespaces: the secondary-network +// testNetworkNamespacesFanOut covers --network-namespaces: the secondary-network // CRs and the example test DaemonSet are duplicated once per namespace (each // pointed at its namespace, with namespace-suffixed names so the copies don't // collide), while shared resources — IPPool, SriovNetworkNodePolicy, // NicClusterPolicy — are rendered exactly once. -func TestNetworkNamespacesFanOut(t *testing.T) { +func testNetworkNamespacesFanOut(t *testing.T) { t.Run("two namespaces duplicate networks + DS but not shared CRs", func(t *testing.T) { rendered := renderSRIOVWithNamespaces(t, "sriov-ethernet-rdma", "ethernet", "50-sriovnetwork.yaml", []string{"ns1", "ns2"})