From f189cd605b06be62191e51ff00bc465d6eaec29d Mon Sep 17 00:00:00 2001 From: Andrea Panattoni Date: Mon, 18 Sep 2023 19:00:11 +0200 Subject: [PATCH] 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 --- pkg/webhook/validate.go | 9 ++++----- pkg/webhook/validate_test.go | 6 +++--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/pkg/webhook/validate.go b/pkg/webhook/validate.go index 571d93d37..2fdff51db 100644 --- 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 index 965504770..9434f2784 100644 --- 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) {