Skip to content

Commit

Permalink
validation:cleanup: remove unused code
Browse files Browse the repository at this point in the history 
Now that each platform is implementing it own validation,
we can remove the old validation which used to branch per
each platform.

Signed-off-by: Talor Itzhak <[email protected]>
  • Loading branch information
@Tal-or
Tal-or committed Jan 7, 2025
1 parent acaa9c7 commit 6d0b0c3
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 326 deletions.
138 changes: 0 additions & 138 deletions pkg/validation/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@ import (
"errors"
"fmt"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/k8stopologyawareschedwg/deployer/pkg/deployer/platform"

nropv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1"
nodegroupv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1/helper/nodegroup"
"github.com/openshift-kni/numaresources-operator/internal/api/annotations"
)
Expand All @@ -48,139 +43,6 @@ func MachineConfigPoolDuplicates(trees []nodegroupv1.Tree) error {
return duplicateErrors
}

// NodeGroups validates the node groups for nil values and duplicates.
func NodeGroups(nodeGroups []nropv1.NodeGroup, platf platform.Platform) error {
if platf == platform.HyperShift {
if err := nodeGroupForHypershift(nodeGroups); err != nil {
return err
}
}

if err := nodeGroupPools(nodeGroups); err != nil {
return err
}

if err := nodeGroupsDuplicatesByMCPSelector(nodeGroups); err != nil {
return err
}

if err := nodeGroupsValidPoolName(nodeGroups); err != nil {
return err
}

if err := nodeGroupsDuplicatesByPoolName(nodeGroups); err != nil {
return err
}

if err := nodeGroupMachineConfigPoolSelector(nodeGroups); err != nil {
return err
}

return nil
}

func nodeGroupForHypershift(nodeGroups []nropv1.NodeGroup) error {
for idx, nodeGroup := range nodeGroups {
if nodeGroup.MachineConfigPoolSelector != nil {
return fmt.Errorf("node group %d specifies MachineConfigPoolSelector on Hypershift platform; Should specify PoolName only", idx)
}
if nodeGroup.PoolName == nil {
return fmt.Errorf("node group %d must specify PoolName on Hypershift platform", idx)
}
}
return nil
}

func nodeGroupPools(nodeGroups []nropv1.NodeGroup) error {
for idx, nodeGroup := range nodeGroups {
if nodeGroup.MachineConfigPoolSelector == nil && nodeGroup.PoolName == nil {
return fmt.Errorf("node group %d missing any pool specifier", idx)
}
if nodeGroup.MachineConfigPoolSelector != nil && nodeGroup.PoolName != nil {
return fmt.Errorf("node group %d must have only a single specifier set: either PoolName or MachineConfigPoolSelector", idx)
}
}

return nil
}

func nodeGroupsValidPoolName(nodeGroups []nropv1.NodeGroup) error {
var err error
for idx, nodeGroup := range nodeGroups {
if nodeGroup.PoolName == nil {
continue
}
if *nodeGroup.PoolName != "" {
continue
}
err = errors.Join(err, fmt.Errorf("pool name for pool #%d cannot be empty", idx))
}
return err
}

func nodeGroupsDuplicatesByMCPSelector(nodeGroups []nropv1.NodeGroup) error {
duplicates := map[string]int{}
for _, nodeGroup := range nodeGroups {
if nodeGroup.MachineConfigPoolSelector == nil {
continue
}

key := nodeGroup.MachineConfigPoolSelector.String()
if _, ok := duplicates[key]; !ok {
duplicates[key] = 0
}
duplicates[key] += 1
}

var duplicateErrors error
for selector, count := range duplicates {
if count > 1 {
duplicateErrors = errors.Join(duplicateErrors, fmt.Errorf("the node group with the machineConfigPoolSelector %q has duplicates", selector))
}
}

return duplicateErrors
}

func nodeGroupsDuplicatesByPoolName(nodeGroups []nropv1.NodeGroup) error {
duplicates := map[string]int{}
for _, nodeGroup := range nodeGroups {
if nodeGroup.PoolName == nil {
continue
}

key := *nodeGroup.PoolName
if _, ok := duplicates[key]; !ok {
duplicates[key] = 0
}
duplicates[key] += 1
}

var duplicateErrors error
for name, count := range duplicates {
if count > 1 {
duplicateErrors = errors.Join(duplicateErrors, fmt.Errorf("the pool name %q has duplicates", name))
}
}

return duplicateErrors
}

func nodeGroupMachineConfigPoolSelector(nodeGroups []nropv1.NodeGroup) error {
var selectorsErrors error
for _, nodeGroup := range nodeGroups {
if nodeGroup.MachineConfigPoolSelector == nil {
continue
}

if _, err := metav1.LabelSelectorAsSelector(nodeGroup.MachineConfigPoolSelector); err != nil {
selectorsErrors = errors.Join(selectorsErrors, err)
}
}

return selectorsErrors
}

func MultipleMCPsPerTree(annot map[string]string, trees []nodegroupv1.Tree) error {
multiMCPsPerTree := annotations.IsMultiplePoolsPerTreeEnabled(annot)
if multiMCPsPerTree {
Expand Down
188 changes: 0 additions & 188 deletions pkg/validation/validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,8 @@ import (
"strings"
"testing"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/k8stopologyawareschedwg/deployer/pkg/deployer/platform"
machineconfigv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"

nropv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1"
nodegroupv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1/helper/nodegroup"
"github.com/openshift-kni/numaresources-operator/internal/api/annotations"
testobjs "github.com/openshift-kni/numaresources-operator/internal/objects"
Expand Down Expand Up @@ -84,190 +80,6 @@ func TestMachineConfigPoolDuplicates(t *testing.T) {
}
}

func TestNodeGroupsSanity(t *testing.T) {
type testCase struct {
name string
nodeGroups []nropv1.NodeGroup
expectedError bool
expectedErrorMessage string
platf platform.Platform
}

emptyString := ""
poolName := "poolname-test"
config := nropv1.DefaultNodeGroupConfig()

testCases := []testCase{
{
name: "both source pools are nil",
nodeGroups: []nropv1.NodeGroup{
{
MachineConfigPoolSelector: nil,
PoolName: nil,
},
{
MachineConfigPoolSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"test": "test",
},
},
},
},
expectedError: true,
expectedErrorMessage: "missing any pool specifier",
platf: platform.OpenShift,
},
{
name: "both source pools are set",
nodeGroups: []nropv1.NodeGroup{
{
MachineConfigPoolSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"test": "test",
},
},
PoolName: &poolName,
},
},
expectedError: true,
expectedErrorMessage: "must have only a single specifier set",
platf: platform.OpenShift,
},
{
name: "with duplicates - mcp selector",
nodeGroups: []nropv1.NodeGroup{
{
MachineConfigPoolSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"test": "test",
},
},
},
{
MachineConfigPoolSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"test": "test",
},
},
},
},
expectedError: true,
expectedErrorMessage: "has duplicates",
platf: platform.OpenShift,
},
{
name: "with duplicates - pool name",
nodeGroups: []nropv1.NodeGroup{
{
PoolName: &poolName,
},
{
PoolName: &poolName,
},
},
expectedError: true,
expectedErrorMessage: "has duplicates",
platf: platform.OpenShift,
},
{
name: "bad MCP selector",
nodeGroups: []nropv1.NodeGroup{
{
MachineConfigPoolSelector: &metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: "test",
Operator: "bad-operator",
Values: []string{"test"},
},
},
},
},
},
expectedError: true,
expectedErrorMessage: "not a valid label selector operator",
platf: platform.OpenShift,
},
{
name: "correct values",
nodeGroups: []nropv1.NodeGroup{
{
MachineConfigPoolSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"test": "test",
},
},
},
{
MachineConfigPoolSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"test1": "test1",
},
},
},
{
PoolName: &poolName,
},
},
platf: platform.OpenShift,
},
{
name: "MCP selector set on Hypershift platform",
nodeGroups: []nropv1.NodeGroup{
{
MachineConfigPoolSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"test": "test",
},
},
PoolName: &poolName,
},
},
expectedError: true,
expectedErrorMessage: "MachineConfigPoolSelector on Hypershift platform",
platf: platform.HyperShift,
},
{
name: "empty PoolName on Hypershift platform",
nodeGroups: []nropv1.NodeGroup{
{
Config: &config,
},
},
expectedError: true,
expectedErrorMessage: "must specify PoolName on Hypershift platform",
platf: platform.HyperShift,
},
{
name: "empty pool name",
nodeGroups: []nropv1.NodeGroup{
{
PoolName: &emptyString,
},
},
expectedError: true,
expectedErrorMessage: "cannot be empty",
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
err := NodeGroups(tc.nodeGroups, tc.platf)
if err == nil && tc.expectedError {
t.Errorf("expected error, succeeded")
}
if err != nil && !tc.expectedError {
t.Errorf("expected success, failed")
}
if tc.expectedErrorMessage != "" {
if !strings.Contains(err.Error(), tc.expectedErrorMessage) {
t.Errorf("unexpected error: %v (expected %q)", err, tc.expectedErrorMessage)
}
}
})
}
}

func TestMultipleMCPsPerTree(t *testing.T) {
testCases := []struct {
name string
Expand Down

0 comments on commit 6d0b0c3

Please sign in to comment.