Skip to content

Commit

Permalink
validation: fix error string
Browse files Browse the repository at this point in the history
Instead of pointers display readable string.

```
found multiple pools matches for node group 0xc00467dca0 but expected one. Pools found [0xc00129aa08 0xc00129ac78]
```

To:

```
found multiple pools matches for node group &{&LabelSelector{MatchLabels:map[string]string{test: common,},MatchExpressions:[]LabelSelectorRequirement{},} <nil> <nil> map[]} but expected one. Pools found [test1 test2]
```

Signed-off-by: Shereen Haj <[email protected]>
  • Loading branch information
shajmakh committed Feb 3, 2025
1 parent 28becd0 commit ed08e4e
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 3 deletions.
17 changes: 17 additions & 0 deletions api/v1/helper/nodegroup/nodegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,23 @@ 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 {
names := []string{}
if tree.NodeGroup == nil {
return names
}

if tree.MachineConfigPools == nil {
names = append(names, *tree.NodeGroup.PoolName)
}

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 {
Expand Down
57 changes: 57 additions & 0 deletions api/v1/helper/nodegroup/nodegroup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: []string{},
},
{
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)
}
})
}
}
6 changes: 5 additions & 1 deletion api/v1/numaresourcesoperator_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,5 +236,9 @@ func (ng *NodeGroup) ToString() string {
if ng == nil {
return ""
}
return fmt.Sprintf("PoolName: %s MachineConfigPoolSelector: %s Config: %s", *ng.PoolName, ng.MachineConfigPoolSelector.String(), ng.Config.ToString())
return fmt.Sprintf("PoolName: %s "+
"MachineConfigPoolSelector: %s "+
"Config: %s "+
"Annotations: %s",
*ng.PoolName, ng.MachineConfigPoolSelector.String(), ng.Config.ToString(), ng.Annotations)
}
5 changes: 4 additions & 1 deletion api/v1/numaresourcesoperator_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,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 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit ed08e4e

Please sign in to comment.