diff --git a/pkg/performanceprofile/profilecreator/profilecreator.go b/pkg/performanceprofile/profilecreator/profilecreator.go index 66d915cc1d..27bf80c3f4 100644 --- a/pkg/performanceprofile/profilecreator/profilecreator.go +++ b/pkg/performanceprofile/profilecreator/profilecreator.go @@ -708,6 +708,8 @@ func EnsureNodesHaveTheSameHardware(nodeHandlers []*GHWHandler) error { } func ensureSameTopology(topology1, topology2 *topology.Info) error { + // the assumption here is that both topologies are deep sorted (e.g. slices of numa nodes, cores, processors ..); + // see handle.SortedTopology() if topology1.Architecture != topology2.Architecture { return fmt.Errorf("the architecture is different: %v vs %v", topology1.Architecture, topology2.Architecture) } @@ -730,8 +732,16 @@ func ensureSameTopology(topology1, topology2 *topology.Info) error { } for j, core1 := range cores1 { - if !reflect.DeepEqual(core1, cores2[j]) { - return fmt.Errorf("the CPU corres differ: %v vs %v", core1, cores2[j]) + // skip comparing index because it's fine if they deffer; see https://github.com/jaypipes/ghw/issues/345#issuecomment-1620274077 + // ghw.ProcessorCore.Index is completely removed starting v0.11.0 + if core1.ID != cores2[j].ID { + return fmt.Errorf("the CPU core ids in NUMA node %d differ: %d vs %d", node1.ID, core1.ID, cores2[j].ID) + } + if core1.NumThreads != cores2[j].NumThreads { + return fmt.Errorf("number of threads for CPU %d in NUMA node %d differs: %d vs %d", core1.ID, node1.ID, core1.NumThreads, cores2[j].NumThreads) + } + if !reflect.DeepEqual(core1.LogicalProcessors, cores2[j].LogicalProcessors) { + return fmt.Errorf("logical processors for CPU %d in NUMA node %d differs: %d vs %d", core1.ID, node1.ID, core1.NumThreads, cores2[j].NumThreads) } } } diff --git a/pkg/performanceprofile/profilecreator/profilecreator_test.go b/pkg/performanceprofile/profilecreator/profilecreator_test.go index 597b8cb827..6556792bc9 100644 --- a/pkg/performanceprofile/profilecreator/profilecreator_test.go +++ b/pkg/performanceprofile/profilecreator/profilecreator_test.go @@ -1402,8 +1402,10 @@ var _ = Describe("PerformanceProfileCreator: Ensuring Nodes hardware equality", }) var _ = Describe("PerformanceProfileCreator: Test Helper Function ensureSameTopology", func() { + // the below set of tests compares two topologies original one is originTopology while the one to be mutated is mutatedTopology + // updates must not be done on originTopology or its items var nodes2 []*topology.Node - var topology2 topology.Info + var mutatedTopology topology.Info nodes1 := []*topology.Node{ { @@ -1421,12 +1423,13 @@ var _ = Describe("PerformanceProfileCreator: Test Helper Function ensureSameTopo }, }, } - topology1 := topology.Info{ + originTopology := topology.Info{ Architecture: topology.ARCHITECTURE_NUMA, Nodes: nodes1, } BeforeEach(func() { + // this must be executed before each It() to avoid data pollution nodes2 = []*topology.Node{ { ID: 0, @@ -1443,7 +1446,7 @@ var _ = Describe("PerformanceProfileCreator: Test Helper Function ensureSameTopo }, }, } - topology2 = topology.Info{ + mutatedTopology = topology.Info{ Architecture: topology.ARCHITECTURE_NUMA, Nodes: nodes2, } @@ -1451,29 +1454,35 @@ var _ = Describe("PerformanceProfileCreator: Test Helper Function ensureSameTopo Context("Check if ensureSameTopology is working correctly", func() { It("nodes with similar topology should not return error", func() { - err := ensureSameTopology(&topology1, &topology2) + err := ensureSameTopology(&originTopology, &mutatedTopology) Expect(err).ToNot(HaveOccurred()) }) It("nodes with different architecture should return error", func() { - topology2.Architecture = topology.ARCHITECTURE_SMP - err := ensureSameTopology(&topology1, &topology2) + mutatedTopology.Architecture = topology.ARCHITECTURE_SMP + err := ensureSameTopology(&originTopology, &mutatedTopology) Expect(err).To(HaveOccurred()) }) It("nodes with different number of NUMA nodes should return error", func() { - topology2.Nodes = topology2.Nodes[1:] - err := ensureSameTopology(&topology1, &topology2) + mutatedTopology.Nodes = mutatedTopology.Nodes[1:] + err := ensureSameTopology(&originTopology, &mutatedTopology) Expect(err).To(HaveOccurred()) }) It("nodes with different number threads per core should return error", func() { - topology2.Nodes[1].Cores[1].NumThreads = 1 - err := ensureSameTopology(&topology1, &topology2) + mutatedTopology.Nodes[1].Cores[1].NumThreads = 1 + err := ensureSameTopology(&originTopology, &mutatedTopology) Expect(err).To(HaveOccurred()) }) It("nodes with different thread IDs should return error", func() { - topology2.Nodes[1].Cores[1].LogicalProcessors[1] = 15 - err := ensureSameTopology(&topology1, &topology2) + mutatedTopology.Nodes[1].Cores[1].LogicalProcessors[1] = 15 + err := ensureSameTopology(&originTopology, &mutatedTopology) Expect(err).To(HaveOccurred()) }) + It("same cores with different indices should still considered equivalent", func() { + mutatedTopology.Nodes[0].Cores[0].Index = 1 + mutatedTopology.Nodes[0].Cores[1].Index = 0 + err := ensureSameTopology(&originTopology, &mutatedTopology) + Expect(err).ToNot(HaveOccurred()) + }) }) })