OCPBUGS-44372: PPC: skip comparing ProcessorCore.Index between NUMA c…

…ores (#1213)

* PPC: skip comparing ProcessorCore.Index between NUMA cores

ProcessorCore.Index indicates the zero-based index of the core in the
Cores slice. While core might be shown in a different order, they can still
be equivalent. See: jaypipes/ghw#346.

Adjust the equality check to skip this field to fix this:

```
  Error: targeted nodes differ: nodes host1.development.lab and host2.development.lab have different topology: the CPU cores differ: processor core #20 (2 threads), logical processors [2 66] vs processor core #20 (2 threads), logical processors [2 66]
```

And add a unit test to cover this scenario.

Signed-off-by: Shereen Haj <[email protected]>

* PPC: unit: rename test variables to avoid misusing

Rename test variables and add clarifying comments to avoid misusing them
while writing tests.

Signed-off-by: Shereen Haj <[email protected]>

---------

Signed-off-by: Shereen Haj <[email protected]>

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)
 			}
 		}
 	}

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,37 +1446,43 @@ var _ = Describe("PerformanceProfileCreator: Test Helper Function ensureSameTopo
 				},
 			},
 		}
-		topology2 = topology.Info{
+		mutatedTopology = topology.Info{
 			Architecture: topology.ARCHITECTURE_NUMA,
 			Nodes:        nodes2,
 		}
 	})
 
 	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())
+		})
 	})
 })