From 25d211bd9e7317b36db48a1166b4c060130bf1ab Mon Sep 17 00:00:00 2001
From: vrindle <vrinda.punj30@gmail.com>
Date: Fri, 28 Apr 2023 13:15:26 -0400
Subject: [PATCH] Update error messages to show why no interface is selected

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.
---
 pkg/webhook/validate.go      | 11 +++++++++
 pkg/webhook/validate_test.go | 43 +++++++++++++++++++++++++++++++++++-
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/pkg/webhook/validate.go b/pkg/webhook/validate.go
index 69129c855..6610c6a04 100644
--- a/pkg/webhook/validate.go
+++ b/pkg/webhook/validate.go
@@ -220,9 +220,12 @@ func validatePolicyForNodeStateAndPolicy(nsList *sriovnetworkv1.SriovNetworkNode
 
 func validatePolicyForNodeState(policy *sriovnetworkv1.SriovNetworkNodePolicy, state *sriovnetworkv1.SriovNetworkNodeState, node *corev1.Node) error {
 	glog.V(2).Infof("validatePolicyForNodeState(): validate policy %s for node %s.", policy.GetName(), state.GetName())
+	interface_selected := false
+	no_interfaces_selected_log := []string{}
 	for _, iface := range state.Status.Interfaces {
 		err := validateNicModel(&policy.Spec.NicSelector, &iface, node)
 		if err == nil {
+			interface_selected = 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())
 			}
@@ -236,6 +239,14 @@ func validatePolicyForNodeState(policy *sriovnetworkv1.SriovNetworkNodePolicy, s
 			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())
 			}
+		} else {
+			error_message := fmt.Sprintf("Interface: %s was not selected, since NIC model could not be validated due to the following error: %s \n", iface.Name, err)
+			no_interfaces_selected_log = append(no_interfaces_selected_log, error_message)
+		}
+	}
+	if interface_selected == false {
+		for _, interface_and_error := range no_interfaces_selected_log {
+			glog.Infof("%s", interface_and_error)
 		}
 	}
 	return nil
diff --git a/pkg/webhook/validate_test.go b/pkg/webhook/validate_test.go
index 3199364ab..cfebca4a2 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,12 @@ 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("interface name: 0000:86:00.0 not found in physical function names"))
+	g.Expect(logsProperly).To(BeNil())
+
 }
 
 func TestValidatePolicyForNodeStateWithInvalidPfName(t *testing.T) {
@@ -542,9 +578,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) {