Skip to content

Commit

Permalink
Update error messages to show why no interface is selected
Browse files Browse the repository at this point in the history 
When the SRIOV network node state is not properly initialized it can hit the error "no supported NIC is selected by the nicSelector" even though the NIC may be indeed be selected. This commit updates the error message to ensure that if the user is configuring a NIC that is supported, then the error is because the SRIOV network node state is not properly initialized.
  • Loading branch information
@vrindle
vrindle committed Oct 14, 2023
1 parent 49fff50 commit 6c0d99f
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 399 deletions.
81 changes: 33 additions & 48 deletions pkg/webhook/validate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,19 +210,13 @@ func staticValidateSriovNetworkNodePolicy(cr *sriovnetworkv1.SriovNetworkNodePol
if (cr.Spec.VdpaType == constants.VdpaTypeVirtio || cr.Spec.VdpaType == constants.VdpaTypeVhost) && cr.Spec.EswitchMode != sriovnetworkv1.ESwithModeSwitchDev {
return false, fmt.Errorf("vdpa requires the device to be configured in switchdev mode")
}

// Externally created: we don't support ExternallyManaged + EswitchMode
//TODO: if needed we will need to add this in the future as today EswitchMode is for HWOFFLOAD
if cr.Spec.ExternallyManaged && cr.Spec.EswitchMode == sriovnetworkv1.ESwithModeSwitchDev {
return false, fmt.Errorf("ExternallyManaged doesn't support the device to be configured in switchdev mode")
}

return true, nil
}

func dynamicValidateSriovNetworkNodePolicy(cr *sriovnetworkv1.SriovNetworkNodePolicy) (bool, error) {
nodesSelected = false
interfaceSelected = false
nodeInterfaceErrorList := make(map[string][]string)

nodeList, err := kubeclient.CoreV1().Nodes().List(context.Background(), metav1.ListOptions{
LabelSelector: labels.Set(cr.Spec.NodeSelector).String(),
Expand All @@ -241,7 +235,7 @@ func dynamicValidateSriovNetworkNodePolicy(cr *sriovnetworkv1.SriovNetworkNodePo
for _, node := range nodeList.Items {
if cr.Selected(&node) {
nodesSelected = true
err = validatePolicyForNodeStateAndPolicy(nsList, npList, &node, cr)
err = validatePolicyForNodeStateAndPolicy(nsList, npList, &node, cr, nodeInterfaceErrorList)
if err != nil {
return false, err
}
Expand All @@ -252,20 +246,31 @@ func dynamicValidateSriovNetworkNodePolicy(cr *sriovnetworkv1.SriovNetworkNodePo
return false, fmt.Errorf("no matched node is selected by the nodeSelector in CR %s", cr.GetName())
}
if !interfaceSelected {
for nodeName, messages := range nodeInterfaceErrorList {
for _, message := range messages {
glog.V(2).Infof("%s: %s", nodeName, message)
}
}
return false, fmt.Errorf("no supported NIC is selected by the nicSelector in CR %s", cr.GetName())
}

return true, nil
}

func validatePolicyForNodeStateAndPolicy(nsList *sriovnetworkv1.SriovNetworkNodeStateList, npList *sriovnetworkv1.SriovNetworkNodePolicyList, node *corev1.Node, cr *sriovnetworkv1.SriovNetworkNodePolicy) error {
func validatePolicyForNodeStateAndPolicy(nsList *sriovnetworkv1.SriovNetworkNodeStateList, npList *sriovnetworkv1.SriovNetworkNodePolicyList, node *corev1.Node, cr *sriovnetworkv1.SriovNetworkNodePolicy, nodeInterfaceErrorList map[string][]string) error {
for _, ns := range nsList.Items {
if ns.GetName() == node.GetName() {
if err := validatePolicyForNodeState(cr, &ns, node); err != nil {
return fmt.Errorf("%s node(%s)", err.Error(), node.Name)
interfaceAndErrorList, err := validatePolicyForNodeState(cr, &ns, node)
if err != nil {
return err
}
if interfaceAndErrorList != nil {
nodeInterfaceErrorList[ns.GetName()] = interfaceAndErrorList
}
break
}
}

// validate current policy against policies in API (may not be converted to SriovNetworkNodeState yet)
for _, np := range npList.Items {
if np.GetName() != cr.GetName() && np.Selected(node) {
Expand All @@ -277,42 +282,38 @@ func validatePolicyForNodeStateAndPolicy(nsList *sriovnetworkv1.SriovNetworkNode
return nil
}

func validatePolicyForNodeState(policy *sriovnetworkv1.SriovNetworkNodePolicy, state *sriovnetworkv1.SriovNetworkNodeState, node *corev1.Node) error {
func validatePolicyForNodeState(policy *sriovnetworkv1.SriovNetworkNodePolicy, state *sriovnetworkv1.SriovNetworkNodeState, node *corev1.Node) ([]string, error) {
glog.V(2).Infof("validatePolicyForNodeState(): validate policy %s for node %s.", policy.GetName(), state.GetName())
interfaceSelectedForNode := false
var noInterfacesSelectedLog []string
for _, iface := range state.Status.Interfaces {
err := validateNicModel(&policy.Spec.NicSelector, &iface, node)
if err == nil {
interfaceSelected = true
interfaceSelectedForNode = true
if policy.GetName() != constants.DefaultPolicyName && policy.Spec.NumVfs == 0 {
return fmt.Errorf("numVfs(%d) in CR %s is not allowed", policy.Spec.NumVfs, policy.GetName())
return nil, fmt.Errorf("numVfs(%d) in CR %s is not allowed", policy.Spec.NumVfs, policy.GetName())
}
if policy.Spec.NumVfs > iface.TotalVfs && iface.Vendor == IntelID {
return fmt.Errorf("numVfs(%d) in CR %s exceed the maximum allowed value(%d) interface(%s)", policy.Spec.NumVfs, policy.GetName(), iface.TotalVfs, iface.Name)
return nil, fmt.Errorf("numVfs(%d) in CR %s exceed the maximum allowed value(%d)", policy.Spec.NumVfs, policy.GetName(), iface.TotalVfs)
}
if policy.Spec.NumVfs > MlxMaxVFs && iface.Vendor == MellanoxID {
return fmt.Errorf("numVfs(%d) in CR %s exceed the maximum allowed value(%d) interface(%s)", policy.Spec.NumVfs, policy.GetName(), MlxMaxVFs, iface.Name)
}

// Externally create validations
if policy.Spec.ExternallyManaged {
if policy.Spec.NumVfs > iface.NumVfs {
return fmt.Errorf("numVfs(%d) in CR %s is higher than the virtual functions allocated for the PF externally value(%d)", policy.Spec.NumVfs, policy.GetName(), iface.NumVfs)
}

if policy.Spec.Mtu != 0 && policy.Spec.Mtu > iface.Mtu {
return fmt.Errorf("MTU(%d) in CR %s is higher than the MTU for the PF externally value(%d)", policy.Spec.Mtu, policy.GetName(), iface.Mtu)
}

if policy.Spec.LinkType != "" && strings.ToLower(policy.Spec.LinkType) != strings.ToLower(iface.LinkType) {
return fmt.Errorf("LinkType(%s) in CR %s is not equal to the LinkType for the PF externally value(%s)", policy.Spec.LinkType, policy.GetName(), iface.LinkType)
}
return nil, fmt.Errorf("numVfs(%d) in CR %s exceed the maximum allowed value(%d)", policy.Spec.NumVfs, policy.GetName(), MlxMaxVFs)
}
// vdpa: only mellanox cards are supported
if (policy.Spec.VdpaType == constants.VdpaTypeVirtio || policy.Spec.VdpaType == constants.VdpaTypeVhost) && iface.Vendor != MellanoxID {
return fmt.Errorf("vendor(%s) in CR %s not supported for vdpa interface(%s)", iface.Vendor, policy.GetName(), iface.Name)
return nil, fmt.Errorf("vendor(%s) in CR %s not supported for vdpa", iface.Vendor, policy.GetName())
}
} else {
errorMessage := fmt.Sprintf("Interface: %s was not selected, since NIC model could not be validated due to the following error: %s \n", iface.Name, err)
noInterfacesSelectedLog = append(noInterfacesSelectedLog, errorMessage)
}
}
return nil

if !interfaceSelectedForNode {
return noInterfacesSelectedLog, nil
}
return nil, nil
}

func validatePolicyForNodePolicy(current *sriovnetworkv1.SriovNetworkNodePolicy, previous *sriovnetworkv1.SriovNetworkNodePolicy) error {
Expand Down Expand Up @@ -347,22 +348,6 @@ func validatePfNames(current *sriovnetworkv1.SriovNetworkNodePolicy, previous *s
// since it should already be evaluated in previous run.
preName, preRngSt, preRngEnd, _ := sriovnetworkv1.ParsePFName(prePf)
if curName == preName {
// reject policy with externallyManage if there is a policy on the same PF without it
if current.Spec.ExternallyManaged != previous.Spec.ExternallyManaged {
return fmt.Errorf("externallyManage is inconsistent with existing policy %s", previous.GetName())
}

// reject policy with externallyManage if there is a policy on the same PF with switch dev
if current.Spec.ExternallyManaged && previous.Spec.EswitchMode == sriovnetworkv1.ESwithModeSwitchDev {
return fmt.Errorf("externallyManage overlap with switchdev mode in existing policy %s", previous.GetName())
}

// reject policy with externallyManage if there is a policy on the same PF with switch dev
if previous.Spec.ExternallyManaged && current.Spec.EswitchMode == sriovnetworkv1.ESwithModeSwitchDev {
return fmt.Errorf("switchdev overlap with externallyManage mode in existing policy %s", previous.GetName())
}

// Check for overlapping ranges
if curRngEnd < preRngSt || curRngSt > preRngEnd {
return nil
} else {
Expand Down
Loading

0 comments on commit 6c0d99f

Please sign in to comment.