Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update error messages to show why no interface is selected #434

Merged
merged 1 commit into from
Oct 16, 2023
Merged

Update error messages to show why no interface is selected #434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 36 additions & 13 deletions pkg/webhook/validate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,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 @@ -241,7 +242,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 +253,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
}
bn222 marked this conversation as resolved.
Show resolved Hide resolved
}

// 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 +289,53 @@ 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) interface(%s)", policy.Spec.NumVfs, policy.GetName(), iface.TotalVfs, iface.Name)
}
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)
return nil, 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)
return nil, 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)
return nil, 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("LinkType(%s) in CR %s is not equal to the LinkType for the PF externally value(%s)", policy.Spec.LinkType, policy.GetName(), iface.LinkType)
}
}
// 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 interface(%s)", iface.Vendor, policy.GetName(), iface.Name)
}
} 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)
bn222 marked this conversation as resolved.
Show resolved Hide resolved
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
36 changes: 18 additions & 18 deletions pkg/webhook/validate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,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 @@ -282,7 +282,7 @@ func TestValidatePolicyForNodeStateWithInvalidNumVfsPolicy(t *testing.T) {
},
}
g := NewGomegaWithT(t)
err := validatePolicyForNodeState(policy, state, NewNode())
_, err := validatePolicyForNodeState(policy, state, NewNode())
g.Expect(err).To(MatchError("numVfs(65) in CR p1 exceed the maximum allowed value(64) interface(ens803f0)"))
}

Expand All @@ -309,7 +309,7 @@ func TestValidatePolicyForNodeStateWithInvalidNumVfsExternallyCreated(t *testing
},
}
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 is higher than the virtual functions allocated for the PF externally value(%d)", policy.Spec.NumVfs, policy.GetName(), state.Status.Interfaces[0].NumVfs))))
}

Expand All @@ -336,7 +336,7 @@ func TestValidatePolicyForNodeStateWithValidNumVfsExternallyCreated(t *testing.T
},
}
g := NewGomegaWithT(t)
err := validatePolicyForNodeState(policy, state, NewNode())
_, err := validatePolicyForNodeState(policy, state, NewNode())
g.Expect(err).ToNot(HaveOccurred())
}

Expand All @@ -363,7 +363,7 @@ func TestValidatePolicyForNodeStateWithValidLowerNumVfsExternallyCreated(t *test
},
}
g := NewGomegaWithT(t)
err := validatePolicyForNodeState(policy, state, NewNode())
_, err := validatePolicyForNodeState(policy, state, NewNode())
g.Expect(err).ToNot(HaveOccurred())
}

Expand Down Expand Up @@ -500,7 +500,7 @@ func TestValidatePolicyForNodeStateWithExternallyManageAndMTU(t *testing.T) {
},
}
g := NewGomegaWithT(t)
err := validatePolicyForNodeState(policy, state, NewNode())
_, err := validatePolicyForNodeState(policy, state, NewNode())
g.Expect(err).ToNot(HaveOccurred())
}

Expand Down Expand Up @@ -528,7 +528,7 @@ func TestValidatePolicyForNodeStateWithExternallyManageAndDifferentMTU(t *testin
},
}
g := NewGomegaWithT(t)
err := validatePolicyForNodeState(policy, state, NewNode())
_, err := validatePolicyForNodeState(policy, state, NewNode())
g.Expect(err).To(HaveOccurred())
}

Expand Down Expand Up @@ -556,16 +556,16 @@ func TestValidatePolicyForNodeStateWithExternallyManageAndLinkType(t *testing.T)
},
}
g := NewGomegaWithT(t)
err := validatePolicyForNodeState(policy, state, NewNode())
_, err := validatePolicyForNodeState(policy, state, NewNode())
g.Expect(err).ToNot(HaveOccurred())

policy.Spec.LinkType = "eth"
err = validatePolicyForNodeState(policy, state, NewNode())
_, err = validatePolicyForNodeState(policy, state, NewNode())
g.Expect(err).ToNot(HaveOccurred())

policy.Spec.LinkType = "ETH"
state.Status.Interfaces[0].LinkType = "eth"
err = validatePolicyForNodeState(policy, state, NewNode())
_, err = validatePolicyForNodeState(policy, state, NewNode())
g.Expect(err).ToNot(HaveOccurred())
}

Expand Down Expand Up @@ -594,7 +594,7 @@ func TestValidatePolicyForNodeStateWithExternallyManageAndDifferentLinkType(t *t
},
}
g := NewGomegaWithT(t)
err := validatePolicyForNodeState(policy, state, NewNode())
_, err := validatePolicyForNodeState(policy, state, NewNode())
g.Expect(err).To(HaveOccurred())
}

Expand Down Expand Up @@ -994,7 +994,7 @@ func TestValidatePolicyForNodeStateVirtioVdpaWithNotSupportedVendor(t *testing.T
},
}
g := NewGomegaWithT(t)
err := validatePolicyForNodeState(policy, state, NewNode())
_, err := validatePolicyForNodeState(policy, state, NewNode())
g.Expect(err).To(MatchError("vendor(8086) in CR p1 not supported for vdpa interface(ens803f0)"))
}

Expand All @@ -1021,7 +1021,7 @@ func TestValidatePolicyForNodeStateVhostVdpaWithNotSupportedVendor(t *testing.T)
},
}
g := NewGomegaWithT(t)
err := validatePolicyForNodeState(policy, state, NewNode())
_, err := validatePolicyForNodeState(policy, state, NewNode())
g.Expect(err).To(MatchError("vendor(8086) in CR p1 not supported for vdpa interface(ens803f0)"))
}

Expand All @@ -1048,7 +1048,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 @@ -1070,7 +1070,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 @@ -1093,7 +1093,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 @@ -1133,7 +1133,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 @@ -1197,7 +1197,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))
}
Loading