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 - #1543

Signed-off-by: Marco Hofstetter <[email protected]>

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,21 +286,23 @@ 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++ {
 		output, err = pod.K8sClient.ExecInPod(ctx,
 			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)
 	}
 

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, "")) == "")
+		})
+	}
+}