Merge pull request #1176 from shajmakh/fix-pnt

validation: fix error string

api/v1/helper/nodegroup/nodegroup.go

@@ -132,6 +132,26 @@ func FindMachineConfigPools(mcps *mcov1.MachineConfigPoolList, nodeGroups []nrop
 	return flattenTrees(trees), nil
 }
 
+// GetTreePoolsNames returns a slice of all the MachineConfigPool matching the configured node groups
+func GetTreePoolsNames(tree Tree) []string {
+	if tree.NodeGroup == nil {
+		return nil
+	}
+
+	if tree.MachineConfigPools == nil {
+		if tree.NodeGroup.PoolName == nil {
+			return nil
+		}
+		return []string{*tree.NodeGroup.PoolName}
+	}
+
+	names := []string{}
+	for _, mcp := range tree.MachineConfigPools {
+		names = append(names, mcp.Name)
+	}
+	return names
+}
+
 func flattenTrees(trees []Tree) []*mcov1.MachineConfigPool {
 	var result []*mcov1.MachineConfigPool
 	for _, tree := range trees {

api/v1/helper/nodegroup/nodegroup_test.go

@@ -511,3 +511,60 @@ func findListByNodeGroups(mcps *mcov1.MachineConfigPoolList, nodeGroups []nropv1
 
 	return result, nil
 }
+
+func TestGetTreePoolsNames(t *testing.T) {
+	poolName := "pool1"
+
+	tests := []struct {
+		name     string
+		tree     Tree
+		expected []string
+	}{
+		{
+			name:     "empty tree",
+			tree:     Tree{},
+			expected: nil,
+		},
+		{
+			name: "with mcps",
+			tree: Tree{
+				NodeGroup: &nropv1.NodeGroup{
+					MachineConfigPoolSelector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"label": "test",
+						},
+					},
+				},
+				MachineConfigPools: []*mcov1.MachineConfigPool{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "mcp1",
+						},
+					},
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "mcp2",
+						},
+					},
+				},
+			},
+			expected: []string{"mcp1", "mcp2"},
+		},
+		{
+			name: "with node pool",
+			tree: Tree{
+				NodeGroup: &nropv1.NodeGroup{
+					PoolName: &poolName,
+				},
+			},
+			expected: []string{poolName},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := GetTreePoolsNames(tt.tree); !reflect.DeepEqual(got, tt.expected) {
+				t.Errorf("got %v, expected %v", got, tt.expected)
+			}
+		})
+	}
+}

api/v1/numaresourcesoperator_types.go

@@ -226,15 +226,25 @@ func init() {
 
 func (ngc *NodeGroupConfig) ToString() string {
 	if ngc == nil {
-		return ""
+		return "nil"
 	}
 	ngc.SetDefaults()
 	return fmt.Sprintf("PodsFingerprinting mode: %s InfoRefreshMode: %s InfoRefreshPeriod: %s InfoRefreshPause: %s Tolerations: %+v", *ngc.PodsFingerprinting, *ngc.InfoRefreshMode, *ngc.InfoRefreshPeriod, *ngc.InfoRefreshPause, ngc.Tolerations)
 }
 
 func (ng *NodeGroup) ToString() string {
 	if ng == nil {
-		return ""
+		return "nil"
 	}
-	return fmt.Sprintf("PoolName: %s MachineConfigPoolSelector: %s Config: %s", *ng.PoolName, ng.MachineConfigPoolSelector.String(), ng.Config.ToString())
+
+	pn := "nil"
+	if ng.PoolName != nil {
+		pn = *ng.PoolName
+	}
+
+	return fmt.Sprintf("PoolName: %s "+
+		"MachineConfigPoolSelector: %s "+
+		"Config: %s "+
+		"Annotations: %s",
+		pn, ng.MachineConfigPoolSelector.String(), ng.Config.ToString(), ng.Annotations)
 }

api/v1/numaresourcesoperator_types_test.go

@@ -46,7 +46,7 @@ func TestNodeGroupConfigToString(t *testing.T) {
 	}{
 		{
 			name:     "nil config",
-			expected: "",
+			expected: "nil",
 		},
 		{
 			name:     "empty fields should reflect default values",
@@ -94,7 +94,17 @@ func TestNodeGroupToString(t *testing.T) {
 	}{
 		{
 			name:     "nil group",
-			expected: "",
+			expected: "nil",
+		},
+		{
+			name: "empty PoolName",
+			input: &NodeGroup{
+				PoolName: nil,
+				Annotations: map[string]string{
+					"ann1": "val1",
+				},
+			},
+			expected: "PoolName: nil MachineConfigPoolSelector: nil Config: nil Annotations: map[ann1:val1]",
 		},
 		{
 			name: "empty fields should reflect default values",
@@ -105,8 +115,11 @@ func TestNodeGroupToString(t *testing.T) {
 					InfoRefreshMode:    &refMode,
 				},
 				PoolName: &pn, // although not allowed more than a specifier but we still need to display and here is not the right place to perform validations
+				Annotations: map[string]string{
+					"ann1": "val1",
+				},
 			},
-			expected: "PoolName: pn MachineConfigPoolSelector: nil Config: PodsFingerprinting mode: Disabled InfoRefreshMode: Events InfoRefreshPeriod: {10s} InfoRefreshPause: Disabled Tolerations: []",
+			expected: "PoolName: pn MachineConfigPoolSelector: nil Config: PodsFingerprinting mode: Disabled InfoRefreshMode: Events InfoRefreshPeriod: {10s} InfoRefreshPause: Disabled Tolerations: [] Annotations: map[ann1:val1]",
 		},
 	}
 	for _, tc := range testcases {

pkg/validation/validation.go

@@ -72,7 +72,7 @@ func MultipleMCPsPerTree(annot map[string]string, trees []nodegroupv1.Tree) erro
 	var err error
 	for _, tree := range trees {
 		if len(tree.MachineConfigPools) > 1 {
-			err = errors.Join(err, fmt.Errorf("found multiple pools matches for node group %v but expected one. Pools found %v", &tree.NodeGroup, tree.MachineConfigPools))
+			err = errors.Join(err, fmt.Errorf("found multiple pools matches for node group %s but expected one. Pools found %v", tree.NodeGroup.ToString(), nodegroupv1.GetTreePoolsNames(tree)))
 		}
 	}
 	return err