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 Aug 9, 2023
1 parent 680654f commit 0907f22
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 17 deletions.
40 changes: 31 additions & 9 deletions pkg/webhook/validate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ func staticValidateSriovNetworkNodePolicy(cr *sriovnetworkv1.SriovNetworkNodePol
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 @@ -234,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 @@ -245,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 {
interfaceAndErrorList, err := validatePolicyForNodeState(cr, &ns, node)
if err != nil {
return err
}
if interfaceAndErrorList != nil {
&nodeInterfaceErrorList[ns.GetName()] = interfaceAndErrorList

Check failure on line 268 in pkg/webhook/validate.go

View workflow job for this annotation

GitHub Actions / build 

invalid operation: cannot index nodeInterfaceErrorList (variable of type *map[string][]string)

Check failure on line 268 in pkg/webhook/validate.go

View workflow job for this annotation

GitHub Actions / test 

invalid operation: cannot index nodeInterfaceErrorList (variable of type *map[string][]string)

Check failure on line 268 in pkg/webhook/validate.go

View workflow job for this annotation

GitHub Actions / Golangci-lint 

invalid operation: cannot index nodeInterfaceErrorList (variable of type *map[string][]string)) (typecheck)

Check failure on line 268 in pkg/webhook/validate.go

View workflow job for this annotation

GitHub Actions / Golangci-lint 

invalid operation: cannot index nodeInterfaceErrorList (variable of type *map[string][]string) (typecheck)

Check failure on line 268 in pkg/webhook/validate.go

View workflow job for this annotation

GitHub Actions / test-coverage 

invalid operation: cannot index nodeInterfaceErrorList (variable of type *map[string][]string)
}
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 @@ -270,28 +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
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)", policy.Spec.NumVfs, policy.GetName(), iface.TotalVfs)
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)", policy.Spec.NumVfs, policy.GetName(), MlxMaxVFs)
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 && iface.Vendor != MellanoxID {
return fmt.Errorf("vendor(%s) in CR %s not supported for virtio-vdpa", iface.Vendor, policy.GetName())
return nil, fmt.Errorf("vendor(%s) in CR %s not supported for virtio-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
16 changes: 8 additions & 8 deletions pkg/webhook/validate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ func TestValidatePolicyForNodeStateWithValidPolicy(t *testing.T) {
},
}
g := NewGomegaWithT(t)
err := validatePolicyForNodeState(policy, state, NewNode())
_, err := validatePolicyForNodeState(policy, state, NewNode())
g.Expect(err).NotTo(HaveOccurred())
}

Expand All @@ -279,7 +279,7 @@ func TestValidatePolicyForNodeStateWithInvalidNumVfsPolicy(t *testing.T) {
},
}
g := NewGomegaWithT(t)
err := validatePolicyForNodeState(policy, state, NewNode())
_, err := validatePolicyForNodeState(policy, state, NewNode())
g.Expect(err).To(MatchError(ContainSubstring(fmt.Sprintf("numVfs(%d) in CR %s exceed the maximum allowed value(%d)", policy.Spec.NumVfs, policy.GetName(), state.Status.Interfaces[0].TotalVfs))))
}

Expand Down Expand Up @@ -608,7 +608,7 @@ func TestValidatePolicyForNodeStateVdpaWithNotSupportedVendor(t *testing.T) {
},
}
g := NewGomegaWithT(t)
err := validatePolicyForNodeState(policy, state, NewNode())
_, err := validatePolicyForNodeState(policy, state, NewNode())
g.Expect(err).To(MatchError(ContainSubstring(fmt.Sprintf("vendor(%s) in CR %s not supported for virtio-vdpa", state.Status.Interfaces[0].Vendor, policy.Name))))
}

Expand All @@ -635,7 +635,7 @@ func TestValidatePolicyForNodeStateWithInvalidDevice(t *testing.T) {
g.Expect(err).ToNot(HaveOccurred())
g.Expect(cfg).ToNot(BeNil())
kubeclient = kubernetes.NewForConfigOrDie(cfg)
err = validatePolicyForNodeState(policy, state, NewNode())
_, err = validatePolicyForNodeState(policy, state, NewNode())
g.Expect(err).NotTo(HaveOccurred())
}

Expand All @@ -657,7 +657,7 @@ func TestValidatePolicyForNodeStateWithInvalidPfName(t *testing.T) {
},
}
g := NewGomegaWithT(t)
err := validatePolicyForNodeState(policy, state, NewNode())
_, err := validatePolicyForNodeState(policy, state, NewNode())
g.Expect(err).NotTo(HaveOccurred())
g.Expect(interfaceSelected).To(Equal(false))
}
Expand All @@ -680,7 +680,7 @@ func TestValidatePolicyForNodeStateWithValidPfName(t *testing.T) {
},
}
g := NewGomegaWithT(t)
err := validatePolicyForNodeState(policy, state, NewNode())
_, err := validatePolicyForNodeState(policy, state, NewNode())
g.Expect(err).NotTo(HaveOccurred())
g.Expect(interfaceSelected).To(Equal(true))
}
Expand Down Expand Up @@ -720,7 +720,7 @@ func TestValidatePolicyForNodeStateWithValidNetFilter(t *testing.T) {
},
}
g := NewGomegaWithT(t)
err := validatePolicyForNodeState(policy, state, NewNode())
_, err := validatePolicyForNodeState(policy, state, NewNode())
g.Expect(err).NotTo(HaveOccurred())
g.Expect(interfaceSelected).To(Equal(true))
}
Expand Down Expand Up @@ -784,7 +784,7 @@ func TestValidatePolicyForNodeStateWithValidVFAndNetFilter(t *testing.T) {
},
}
g := NewGomegaWithT(t)
err := validatePolicyForNodeState(policy, state, NewNode())
_, err := validatePolicyForNodeState(policy, state, NewNode())
g.Expect(err).NotTo(HaveOccurred())
g.Expect(interfaceSelected).To(Equal(true))
}

0 comments on commit 0907f22

Please sign in to comment.