diff --git a/pkg/webhook/validate.go b/pkg/webhook/validate.go index 69129c855..42c647460 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,30 @@ 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 + } + } + break + } + + if !interfaceSelected { + for nodeName, messages := range nodeInterfaceErrorList { + for _, message := range messages { + glog.V(2).Infof("%s: %s", nodeName, message) + } } + return fmt.Errorf("no supported NIC is selected by the nicSelector in CR %s", cr.GetName()) } + // 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 +238,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..958dea046 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" @@ -213,7 +216,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 +242,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 +498,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 +525,9 @@ 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()) + g.Expect(logsProperly).To(BeNil()) } func TestValidatePolicyForNodeStateWithInvalidPfName(t *testing.T) { @@ -542,9 +546,10 @@ 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()) + g.Expect(logsProperly).To(BeNil()) } func TestValidatePolicyForNodeStateWithValidPfName(t *testing.T) { @@ -564,7 +569,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 +607,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 +669,6 @@ func TestValidatePolicyForNodeStateWithValidVFAndNetFilter(t *testing.T) { }, } g := NewGomegaWithT(t) - err := validatePolicyForNodeState(policy, state, NewNode()) + _, err := validatePolicyForNodeState(policy, state, NewNode()) g.Expect(err).NotTo(HaveOccurred()) }