diff --git a/pkg/webhook/validate.go b/pkg/webhook/validate.go index 91b3922fa..126c2459f 100644 --- a/pkg/webhook/validate.go +++ b/pkg/webhook/validate.go @@ -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(), @@ -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 } @@ -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 + } + 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) { @@ -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 { diff --git a/pkg/webhook/validate_test.go b/pkg/webhook/validate_test.go index a82796312..379a53420 100644 --- a/pkg/webhook/validate_test.go +++ b/pkg/webhook/validate_test.go @@ -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()) } @@ -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)))) } @@ -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)))) } @@ -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()) } @@ -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)) } @@ -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)) } @@ -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)) } @@ -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)) }