From caa5824e7c5cfddd23fc57f7f5597a4338d1acbd Mon Sep 17 00:00:00 2001 From: Marco Hofstetter Date: Fri, 21 Jul 2023 15:27:57 +0200 Subject: [PATCH] connectivity: Special inconclusive result treatment for ping command When running the connectivity tests in AKS, we sometimes get interrupted commands that don't have any output [1]. Therefore, successful command terminations with an empty output are treated as inconclusive that result in a retry [2]. Unfortunately, the ping command might get stuck by only outputting its header line to the output. `PING 10.0.0.1 (10.0.0.1) 56(84) bytes of data.` This commit adds an additional check for the ping commands. If we see an output only containing the header, we treat it as inconclusive result. 1 - cilium/cilium#22162 2 - cilium/cilium-cli#1543 Signed-off-by: Marco Hofstetter --- connectivity/check/action.go | 11 ++++- connectivity/check/action_test.go | 68 +++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 connectivity/check/action_test.go diff --git a/connectivity/check/action.go b/connectivity/check/action.go index 3ad8db7a5b..f2436bc04b 100644 --- a/connectivity/check/action.go +++ b/connectivity/check/action.go @@ -33,6 +33,11 @@ const ( testCommandRetries = 3 ) +var ( + // PING 10.0.0.1 (10.0.0.1) 56(84) bytes of data. + pingHeaderPattern = regexp.MustCompile(`^PING .* bytes of data\.`) +) + // Action represents an individual action (e.g. a curl call) in a Scenario // between a source and a destination peer. type Action struct { @@ -281,6 +286,8 @@ func (a *Action) ExecInPod(ctx context.Context, cmd []string) { // deemed inconclusive when the command succeeded, but we don't have any // output. We've seen this happen when there are connectivity blips on the // k8s side. + // Ping Command is treated specially: its output may contain the header + // `PING 10.0.0.1 (10.0.0.1) 56(84) bytes of data.` even if the command hangs. // This check currently only works because all our test commands expect an // output. for i := 1; i <= testCommandRetries; i++ { @@ -288,14 +295,14 @@ func (a *Action) ExecInPod(ctx context.Context, cmd []string) { pod.Pod.Namespace, pod.Pod.Name, pod.Pod.Labels["name"], cmd) a.cmdOutput = output.String() // Check for inconclusive results. - if err == nil && strings.TrimSpace(a.cmdOutput) == "" { + if err == nil && strings.TrimSpace(pingHeaderPattern.ReplaceAllString(output.String(), "")) == "" { a.Debugf("retrying command %s due to inconclusive results", cmdStr) continue } break } // Check for inconclusive results. - if err == nil && strings.TrimSpace(a.cmdOutput) == "" { + if err == nil && strings.TrimSpace(pingHeaderPattern.ReplaceAllString(output.String(), "")) == "" { a.Failf("inconclusive results: command %q was successful but without output", cmdStr) } diff --git a/connectivity/check/action_test.go b/connectivity/check/action_test.go new file mode 100644 index 0000000000..80fdea273b --- /dev/null +++ b/connectivity/check/action_test.go @@ -0,0 +1,68 @@ +package check + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestAction_PingHeaderPattern(t *testing.T) { + type args struct { + output string + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "Only ping header output", + args: args{ + output: `PING 10.0.0.1 (10.0.0.1) 56(84) bytes of data.`, + }, + want: true, + }, + { + name: "Only ping header output without dot", + args: args{ + output: `PING 10.0.0.1 (10.0.0.1) 56(84) bytes of data`, + }, + want: false, + }, + { + name: "Splitted ping header output with newline", + args: args{ + output: `PING 10.0.0.1 (10.0.0.1) 56(84) bytes of +data.`, + }, + want: false, + }, + { + name: "Ping header output with preceding output", + args: args{ + output: `First PING 10.0.0.1 (10.0.0.1) 56(84) bytes of +data.`, + }, + want: false, + }, + { + name: "Full ping command output", + args: args{ + output: `PING www.google.com(zrh04s15-in-x04.1e100.net (2a00:1450:400a:803::2004)) 56 data bytes +64 bytes from zrh04s15-in-x04.1e100.net (2a00:1450:400a:803::2004): icmp_seq=1 ttl=119 time=5.78 ms +64 bytes from zrh04s15-in-x04.1e100.net (2a00:1450:400a:803::2004): icmp_seq=2 ttl=119 time=6.01 ms + +--- www.google.com ping statistics --- +2 packets transmitted, 2 received, 0% packet loss, time 1000ms +rtt min/avg/max/mdev = 5.780/5.895/6.010/0.115 ms`, + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, strings.TrimSpace(pingHeaderPattern.ReplaceAllString(tt.args.output, "")) == "") + }) + } +}