diff --git a/pkg/webhook/validate.go b/pkg/webhook/validate.go index 69129c855..fd5590713 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,28 @@ 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) + interfaceSelected = false 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 nodeName, messages := range nodeInterfaceErrorList { + for _, message := range messages { + glog.V(2).Infof("%s: %s", nodeName, 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 +236,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( diff --git a/pkg/webhook/validate_test.go b/pkg/webhook/validate_test.go index 3199364ab..35ef820db 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 @@ -213,7 +243,7 @@ func TestValidatePolicyForNodeStateWithValidPolicy(t *testing.T) { }, } g := NewGomegaWithT(t) - err := validatePolicyForNodeState(policy, state, NewNode()) + _, err := validatePolicyForNodeState(policy, state, NewNode()) g.Expect(err).NotTo(HaveOccurred()) } @@ -239,7 +269,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)))) } @@ -495,7 +525,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)))) } @@ -522,8 +552,13 @@ 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()) + 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,15 @@ func TestValidatePolicyForNodeStateWithInvalidPfName(t *testing.T) { ResourceName: "p0", }, } + _, err := validatePolicyForNodeState(policy, state, NewNode()) g := NewGomegaWithT(t) - err := validatePolicyForNodeState(policy, state, NewNode()) g.Expect(err).NotTo(HaveOccurred()) + //stdErr, logsProperly := captureStderr(func() { + //validatePolicyForNodeState(policy, state, NewNode()) + //}) + //fmt.Printf("bbbb %s", stdErr) + //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) { @@ -564,7 +605,7 @@ func TestValidatePolicyForNodeStateWithValidPfName(t *testing.T) { }, } g := NewGomegaWithT(t) - err := validatePolicyForNodeState(policy, state, NewNode()) + _, err := validatePolicyForNodeState(policy, state, NewNode()) g.Expect(err).NotTo(HaveOccurred()) } @@ -602,7 +643,7 @@ func TestValidatePolicyForNodeStateWithValidNetFilter(t *testing.T) { }, } g := NewGomegaWithT(t) - err := validatePolicyForNodeState(policy, state, NewNode()) + _, err := validatePolicyForNodeState(policy, state, NewNode()) g.Expect(err).NotTo(HaveOccurred()) } @@ -664,6 +705,6 @@ func TestValidatePolicyForNodeStateWithValidVFAndNetFilter(t *testing.T) { }, } g := NewGomegaWithT(t) - err := validatePolicyForNodeState(policy, state, NewNode()) + _, err := validatePolicyForNodeState(policy, state, NewNode()) g.Expect(err).NotTo(HaveOccurred()) }