Add context to validatePolicyForNodeState errors

`validatePolicyForNodeState(...)` errors should contain the node they were validated againsti, making the user understand where the configuration problem is located. Signed-off-by: Andrea Panattoni <[email protected]>
k8snetworkplumbingwg · Sep 18, 2023 · f189cd6 · f189cd6
1 parent 08badf6
commit f189cd6
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 8 deletions.
diff --git a/pkg/webhook/validate.go b/pkg/webhook/validate.go
@@ -255,7 +255,7 @@ func validatePolicyForNodeStateAndPolicy(nsList *sriovnetworkv1.SriovNetworkNode
 	for _, ns := range nsList.Items {
 		if ns.GetName() == node.GetName() {
 			if err := validatePolicyForNodeState(cr, &ns, node); err != nil {
-				return err
+				return fmt.Errorf("%s node(%s)", err.Error(), node.Name)
 			}
 		}
 	}
@@ -271,7 +271,6 @@ 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())
 	for _, iface := range state.Status.Interfaces {
 		err := validateNicModel(&policy.Spec.NicSelector, &iface, node)
 		if err == nil {
@@ -280,14 +279,14 @@ func validatePolicyForNodeState(policy *sriovnetworkv1.SriovNetworkNodePolicy, s
 				return 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 fmt.Errorf("numVfs(%d) in CR %s exceed the maximum allowed value(%d) interface(%s)", policy.Spec.NumVfs, policy.GetName(), iface.TotalVfs, iface.Name)
 			}
 			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 fmt.Errorf("numVfs(%d) in CR %s exceed the maximum allowed value(%d) interface(%s)", policy.Spec.NumVfs, policy.GetName(), MlxMaxVFs, iface.Name)
 			}
 			// vdpa: only mellanox cards are supported
 			if (policy.Spec.VdpaType == constants.VdpaTypeVirtio || policy.Spec.VdpaType == constants.VdpaTypeVhost) && iface.Vendor != MellanoxID {
-				return fmt.Errorf("vendor(%s) in CR %s not supported for vdpa", iface.Vendor, policy.GetName())
+				return fmt.Errorf("vendor(%s) in CR %s not supported for vdpa interface(%s)", iface.Vendor, policy.GetName(), iface.Name)
 			}
 		}
 	}

diff --git a/pkg/webhook/validate_test.go b/pkg/webhook/validate_test.go
@@ -280,7 +280,7 @@ func TestValidatePolicyForNodeStateWithInvalidNumVfsPolicy(t *testing.T) {
 	}
 	g := NewGomegaWithT(t)
 	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))))
+	g.Expect(err).To(MatchError("numVfs(65) in CR p1 exceed the maximum allowed value(64) interface(ens803f0)"))
 }
 
 func TestValidatePolicyForNodePolicyWithOverlappedVfRange(t *testing.T) {
@@ -656,7 +656,7 @@ func TestValidatePolicyForNodeStateVirtioVdpaWithNotSupportedVendor(t *testing.T
 	}
 	g := NewGomegaWithT(t)
 	err := validatePolicyForNodeState(policy, state, NewNode())
-	g.Expect(err).To(MatchError(ContainSubstring(fmt.Sprintf("vendor(%s) in CR %s not supported for vdpa", state.Status.Interfaces[0].Vendor, policy.Name))))
+	g.Expect(err).To(MatchError("vendor(8086) in CR p1 not supported for vdpa interface(ens803f0)"))
 }
 
 func TestValidatePolicyForNodeStateVhostVdpaWithNotSupportedVendor(t *testing.T) {
@@ -683,7 +683,7 @@ func TestValidatePolicyForNodeStateVhostVdpaWithNotSupportedVendor(t *testing.T)
 	}
 	g := NewGomegaWithT(t)
 	err := validatePolicyForNodeState(policy, state, NewNode())
-	g.Expect(err).To(MatchError(ContainSubstring(fmt.Sprintf("vendor(%s) in CR %s not supported for vdpa", state.Status.Interfaces[0].Vendor, policy.Name))))
+	g.Expect(err).To(MatchError("vendor(8086) in CR p1 not supported for vdpa interface(ens803f0)"))
 }
 
 func TestValidatePolicyForNodeStateWithInvalidDevice(t *testing.T) {