Skip to content

Commit

Permalink
connectivity: Add flag --expected-drop-reasons
Browse files Browse the repository at this point in the history
This new flag can be used to customize the set of expected reasons for
packet drops, for the new test that ensure we don't have any unexpected
packet drops.

Signed-off-by: Paul Chaignon <[email protected]>
  • Loading branch information
pchaigno committed Dec 8, 2023
1 parent 7782c29 commit ca0fa23
Show file tree
Hide file tree
Showing 6 changed files with 132 additions and 7 deletions.
7 changes: 5 additions & 2 deletions connectivity/check/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,11 @@ type Parameters struct {
ConnDisruptTestRestartsPath string
ConnDisruptTestXfrmErrorsPath string
ConnDisruptDispatchInterval time.Duration
FlushCT bool
SecondaryNetworkIface string

ExpectedDropReasons []string

FlushCT bool
SecondaryNetworkIface string

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

ct.NewTest("no-unexpected-packet-drops").WithScenarios(tests.NoUnexpectedPacketDrops())
ct.NewTest("no-unexpected-packet-drops").WithScenarios(tests.NoUnexpectedPacketDrops(ct.Params().ExpectedDropReasons))

// Run all tests without any policies in place.
noPoliciesScenarios := []check.Scenario{
Expand Down
55 changes: 51 additions & 4 deletions connectivity/tests/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package tests

import (
"context"
"fmt"
"strings"
"time"

Expand Down Expand Up @@ -76,21 +77,25 @@ func (n *noErrorsInLogs) Run(ctx context.Context, t *check.Test) {

// NoUnexpectedPacketDrops checks whether there were no drops due to expected
// packet drops.
func NoUnexpectedPacketDrops() check.Scenario {
return &noUnexpectedPacketDrops{}
func NoUnexpectedPacketDrops(expectedDrops []string) check.Scenario {
return &noUnexpectedPacketDrops{expectedDrops}
}

type noUnexpectedPacketDrops struct{}
type noUnexpectedPacketDrops struct {
expectedDrops []string
}

func (n *noUnexpectedPacketDrops) Name() string {
return "no-unexpected-packet-drops"
}

func (n *noUnexpectedPacketDrops) Run(ctx context.Context, t *check.Test) {
ct := t.Context()

filter := computeExpectedDropReasons(defaults.ExpectedDropReasons, n.expectedDrops)
cmd := []string{
"/bin/sh", "-c",
"cilium metrics list -o json | jq '.[] | select((.name == \"cilium_drop_count_total\") and (.labels.reason | IN(\"Policy denied\", \"Policy denied by denylist\") | not))'",
fmt.Sprintf("cilium metrics list -o json | jq '.[] | select((.name == \"cilium_drop_count_total\") and (.labels.reason | IN(%s) | not))'", filter),
}

for _, pod := range ct.CiliumPods() {
Expand All @@ -104,7 +109,49 @@ func (n *noUnexpectedPacketDrops) Run(ctx context.Context, t *check.Test) {
t.Fatalf("Found unexpected packet drops:\n%s", countStr)
}
}
}

func computeExpectedDropReasons(defaultReasons, inputReasons []string) string {
filter := ""
if len(inputReasons) > 0 {
// Build final list of drop reasons based on default reasons, added
// reasons (+ prefix), and removed reasons (- prefix).
addedAllDefaults := false
dropReasons := map[string]bool{}
for _, reason := range inputReasons {
if reason[0] == '+' || reason[0] == '-' {
if !addedAllDefaults {
for _, r := range defaultReasons {
dropReasons[r] = true
}
addedAllDefaults = true
}
}
switch reason[0] {
case '+':
dropReasons[reason[1:]] = true
case '-':
dropReasons[reason[1:]] = false
default:
dropReasons[reason] = true
}
}

// Build output string in form of '"reason1", "reason2"'.
i := 0
for reason, isIn := range dropReasons {
if !isIn {
continue
}
if i == 0 {
filter = fmt.Sprintf("%q", reason)
} else {
filter = fmt.Sprintf("%s, %q", filter, reason)
}
i++
}
}
return filter
}

func (n *noErrorsInLogs) checkErrorsInLogs(logs string, t *check.Test) {
Expand Down
66 changes: 66 additions & 0 deletions connectivity/tests/errors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package tests

import (
"fmt"
"strings"
"testing"
)

func TestComputeExpectedDropReasons(t *testing.T) {
defaultReasons := []string{"reason0", "reason1"}
tests := []struct {
inputReasons []string
expectedReasons []string
}{
// Empty list of reasons.
{
inputReasons: []string{},
expectedReasons: []string{},
},
// Add a reason to default list.
{
inputReasons: []string{"+reason2"},
expectedReasons: []string{"reason0", "reason1", "reason2"},
},
// Remove a reason from default list.
{
inputReasons: []string{"-reason1"},
expectedReasons: []string{"reason0"},
},
// Add a reason then remove it.
{
inputReasons: []string{"+reason2", "-reason2"},
expectedReasons: []string{"reason0", "reason1"},
},
// Remove a reason then add it back.
{
inputReasons: []string{"-reason1", "+reason1"},
expectedReasons: []string{"reason0", "reason1"},
},
}

for _, test := range tests {
t.Run(fmt.Sprintf("InputReasons: %v", test.inputReasons), func(t *testing.T) {
result := computeExpectedDropReasons(defaultReasons, test.inputReasons)

// computeExpectedDropReasons doesn't guarantee the order of the
// emitted string so instead we check the length and that all
// expected elements are listed.

for _, expReason := range test.expectedReasons {
if !strings.Contains(result, fmt.Sprintf("%q", expReason)) {
t.Errorf("Expected filter to contain %q, but got: %v", expReason, result)
}
}

// We know the expected length because all our test reasons are the same length.
expectedLength := len(test.expectedReasons) * 9
if len(test.expectedReasons) > 1 {
expectedLength += (len(test.expectedReasons) - 1) * 2
}
if len(result) != expectedLength {
t.Errorf("Expected filter of length %d, but got: %v", expectedLength, result)
}
})
}
}
5 changes: 5 additions & 0 deletions defaults/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,4 +190,9 @@ var (
"authentication.mutual.spire.install.agent.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0].matchExpressions[0].operator=NotIn",
"authentication.mutual.spire.install.agent.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0].matchExpressions[0].values[0]=true",
}

ExpectedDropReasons = []string{
"Policy denied",
"Policy denied by denylist",
}
)
4 changes: 4 additions & 0 deletions internal/cli/cmd/connectivity.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,10 @@ func newCmdConnectivityTest(hooks Hooks) *cobra.Command {
cmd.Flags().StringVar(&params.ConnDisruptTestRestartsPath, "conn-disrupt-test-restarts-path", "/tmp/cilium-conn-disrupt-restarts", "Conn disrupt test temporary result file (used internally)")
cmd.Flags().StringVar(&params.ConnDisruptTestXfrmErrorsPath, "conn-disrupt-test-xfrm-errors-path", "/tmp/cilium-conn-disrupt-xfrm-errors", "Conn disrupt test temporary result file (used internally)")
cmd.Flags().DurationVar(&params.ConnDisruptDispatchInterval, "conn-disrupt-dispatch-interval", 0, "TCP packet dispatch interval")

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

cmd.Flags().BoolVar(&params.FlushCT, "flush-ct", false, "Flush conntrack of Cilium on each node")
cmd.Flags().MarkHidden("flush-ct")
cmd.Flags().StringVar(&params.SecondaryNetworkIface, "secondary-network-iface", "", "Secondary network iface name (e.g., to test NodePort BPF on multiple networks)")
Expand Down

0 comments on commit ca0fa23

Please sign in to comment.