Skip to content

Commit

Permalink
connectivity: Flag for expected XFRM errors
Browse files Browse the repository at this point in the history
We have a couple XFRM errors, XfrmInError and XfrmFwdHdrError, that can
be expected to slightly increase in specific scenarios. Those have
started occuring in CI so we need a way to ignore them.

This commit implements that, as well as a connectivity test flag to
control the list of XFRM errors to ignore. This is similar to what I did
for drop reasons in commit 4880c91 ("connectivity: Check for
unexpected packet drops") and b5e40ba ("connectivity: Add flag
--expected-drop-reasons").

In addition, because those errors are only expected to happen in very
small amount, we only ignore them if there are less than 10 such errors.
It is critical that we fail tests if these errors become more frequent.

Signed-off-by: Paul Chaignon <[email protected]>
  • Loading branch information
pchaigno committed Dec 15, 2023
1 parent 7e4951b commit 257c4f2
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 8 deletions.
1 change: 1 addition & 0 deletions connectivity/check/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ type Parameters struct {
ConnDisruptDispatchInterval time.Duration

ExpectedDropReasons []string
ExpectedXFRMErrors []string

FlushCT bool
SecondaryNetworkIface string
Expand Down
2 changes: 1 addition & 1 deletion connectivity/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ func Run(ctx context.Context, ct *check.ConnectivityTest, addExtraTests func(*ch

ct.NewTest("no-ipsec-xfrm-errors").
WithFeatureRequirements(features.RequireMode(features.EncryptionPod, "ipsec")).
WithScenarios(tests.NoIPsecXfrmErrors())
WithScenarios(tests.NoIPsecXfrmErrors(ct.Params().ExpectedXFRMErrors))

if ct.Params().ConnDisruptTestSetup {
// Exit early, as --conn-disrupt-test-setup is only needed to deploy pods which
Expand Down
25 changes: 18 additions & 7 deletions connectivity/tests/ipsec_xfrm.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,17 @@ import (
gojson "encoding/json"
"fmt"
"os"
"slices"
"sort"
"strings"

"github.com/cilium/cilium-cli/connectivity/check"
"github.com/cilium/cilium-cli/defaults"
"github.com/cilium/cilium-cli/utils/features"
)

const (
maxExpectedErrors = 10
)

type ciliumMetricsXfrmError struct {
Expand All @@ -25,11 +31,15 @@ type ciliumMetricsXfrmError struct {
Value uint64 `json:"value"`
}

func NoIPsecXfrmErrors() check.Scenario {
return &noIPsecXfrmErrors{}
func NoIPsecXfrmErrors(expectedErrors []string) check.Scenario {
return &noIPsecXfrmErrors{
features.ComputeFailureExceptions(defaults.ExpectedXFRMErrors, expectedErrors),
}
}

type noIPsecXfrmErrors struct{}
type noIPsecXfrmErrors struct {
expectedErrors []string
}

func (n *noIPsecXfrmErrors) Name() string {
return "no-ipsec-xfrm-error"
Expand All @@ -56,7 +66,6 @@ func (n *noIPsecXfrmErrors) Run(ctx context.Context, t *check.Test) {
}

func (n *noIPsecXfrmErrors) collectXfrmErrors(ctx context.Context, t *check.Test) map[string]string {

ct := t.Context()
xfrmErrors := map[string]string{}
cmd := []string{"cilium", "metrics", "list", "-ojson", "-pcilium_ipsec_xfrm_error"}
Expand All @@ -74,10 +83,12 @@ func (n *noIPsecXfrmErrors) collectXfrmErrors(ctx context.Context, t *check.Test
t.Fatalf("Unable to unmarshal cilium ipsec xfrm error metrics: %s", err)
}
for _, xfrmMetric := range xfrmMetrics {
name := fmt.Sprintf("%s_%s", xfrmMetric.Labels.Type, xfrmMetric.Labels.Error)
if slices.Contains(n.expectedErrors, name) && xfrmMetric.Value < maxExpectedErrors {
continue
}
if xfrmMetric.Value > 0 {
xErrors = append(xErrors,
fmt.Sprintf("%s_%s:%d",
xfrmMetric.Labels.Type, xfrmMetric.Labels.Error, xfrmMetric.Value))
xErrors = append(xErrors, fmt.Sprintf("%s:%d", name, xfrmMetric.Value))
}
sort.Strings(xErrors)
xfrmErrors[pod.Pod.Status.HostIP] = strings.Join(xErrors, ",")
Expand Down
5 changes: 5 additions & 0 deletions defaults/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,4 +203,9 @@ var (
"Unknown L3 target address",
"No tunnel/encapsulation endpoint (datapath BUG!)",
}

ExpectedXFRMErrors = []string{
"inbound_forward_header", // XfrmFwdHdrError
"inbound_other", // XfrmInError
}
)
2 changes: 2 additions & 0 deletions internal/cli/cmd/connectivity.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ func newCmdConnectivityTest(hooks Hooks) *cobra.Command {

cmd.Flags().StringSliceVar(&params.ExpectedDropReasons, "expected-drop-reasons", defaults.ExpectedDropReasons, "List of expected drop reasons")
cmd.Flags().MarkHidden("expected-drop-reasons")
cmd.Flags().StringSliceVar(&params.ExpectedXFRMErrors, "expected-xfrm-errors", defaults.ExpectedXFRMErrors, "List of expected XFRM errors")
cmd.Flags().MarkHidden("expected-xfrm-errors")

cmd.Flags().BoolVar(&params.FlushCT, "flush-ct", false, "Flush conntrack of Cilium on each node")
cmd.Flags().MarkHidden("flush-ct")
Expand Down

0 comments on commit 257c4f2

Please sign in to comment.