diff --git a/pkg/webhook/validate.go b/pkg/webhook/validate.go index 69129c855..94fb51a42 100644 --- a/pkg/webhook/validate.go +++ b/pkg/webhook/validate.go @@ -26,7 +26,10 @@ const ( MlxMaxVFs = 128 ) -var nodesSelected bool +var ( + nodesSelected bool + interfaceSelected bool +) func validateSriovOperatorConfig(cr *sriovnetworkv1.SriovOperatorConfig, operation v1.Operation) (bool, []string, error) { glog.V(2).Infof("validateSriovOperatorConfig: %v", cr) @@ -200,13 +203,27 @@ func dynamicValidateSriovNetworkNodePolicy(cr *sriovnetworkv1.SriovNetworkNodePo } func validatePolicyForNodeStateAndPolicy(nsList *sriovnetworkv1.SriovNetworkNodeStateList, npList *sriovnetworkv1.SriovNetworkNodePolicyList, node *corev1.Node, cr *sriovnetworkv1.SriovNetworkNodePolicy) error { + nodeInterfaceErrorList := make(map[string][]string) 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 + } + } + } + + if !interfaceSelected { + for node, messages := range nodeInterfaceErrorList { + for message := range messages { + glog.Infof("%s: %s", node, message) + } } } + // 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) { @@ -218,27 +235,39 @@ 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()) + interfaceSelected := false + 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( diff --git a/pkg/webhook/validate_test.go b/pkg/webhook/validate_test.go index 3199364ab..58e2ff726 100644 --- a/pkg/webhook/validate_test.go +++ b/pkg/webhook/validate_test.go @@ -1,7 +1,10 @@ package webhook import ( + "bytes" + "flag" "fmt" + "io" "os" "testing" @@ -16,6 +19,33 @@ import ( constants "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/consts" ) +func captureStderr(f func()) (string, error) { + old := os.Stderr // keep backup of the real stderr + r, w, err := os.Pipe() + if err != nil { + return "", err + } + os.Stderr = w + + outC := make(chan string) + // copy the output in a separate goroutine so printing can't block indefinitely + go func() { + var buf bytes.Buffer + io.Copy(&buf, r) + outC <- buf.String() + }() + + // calling function which stderr we are going to capture: + flag.Set("alsologtostderr", "true") + f() + + // back to normal state + w.Close() + flag.Set("alsologtodtderr", "false") + os.Stderr = old // restoring the real stderr + return <-outC, nil +} + func TestMain(m *testing.M) { NicIDMap = []string{ "8086 158a 154c", // I40e XXV710 @@ -524,6 +554,11 @@ func TestValidatePolicyForNodeStateWithInvalidDevice(t *testing.T) { kubeclient = kubernetes.NewForConfigOrDie(cfg) err = validatePolicyForNodeState(policy, state, NewNode()) g.Expect(err).NotTo(HaveOccurred()) + stdErr, logsProperly := captureStderr(func() { + validatePolicyForNodeState(policy, state, NewNode()) + }) + g.Expect(stdErr).Should(ContainSubstring("selector device ID: is not equal to the interface device ID: 8086")) + g.Expect(logsProperly).To(BeNil()) } func TestValidatePolicyForNodeStateWithInvalidPfName(t *testing.T) { @@ -542,9 +577,14 @@ func TestValidatePolicyForNodeStateWithInvalidPfName(t *testing.T) { ResourceName: "p0", }, } - g := NewGomegaWithT(t) err := validatePolicyForNodeState(policy, state, NewNode()) + g := NewGomegaWithT(t) g.Expect(err).NotTo(HaveOccurred()) + stdErr, logsProperly := captureStderr(func() { + validatePolicyForNodeState(policy, state, NewNode()) + }) + g.Expect(stdErr).Should(ContainSubstring("interface name: 0000:86:00.0 not found in physical function names")) + g.Expect(logsProperly).To(BeNil()) } func TestValidatePolicyForNodeStateWithValidPfName(t *testing.T) {