diff --git a/tests/e2e/harness/diag.go b/tests/e2e/harness/diag.go index c324555e1..9a43a59a2 100644 --- a/tests/e2e/harness/diag.go +++ b/tests/e2e/harness/diag.go @@ -54,24 +54,41 @@ func DumpVPCFlowDiagnostics(t *testing.T, c *AWSClient, instanceID, label string fmt.Printf("%s%s── END DIAGNOSTICS ──%s\n\n", colorBold, colorCyan, colorReset) } -func dumpConsoleOutput(t *testing.T, c *AWSClient, instanceID string) { - t.Helper() +// InstanceConsole returns an instance's serial console, base64-decoded. +// A payload that fails to decode is returned verbatim — the console is a +// diagnostic channel and a partially-written one is still worth reading. +// The error is returned rather than logged so a poll loop can fold it into +// its own timeout message instead of repeating it every interval. +func InstanceConsole(c *AWSClient, instanceID string) (string, error) { out, err := c.EC2.GetConsoleOutput(&ec2.GetConsoleOutputInput{ InstanceId: aws.String(instanceID), }) if err != nil { - fmt.Printf(" console-output: GetConsoleOutput failed: %v\n", err) + return "", fmt.Errorf("get-console-output %s: %w", instanceID, err) + } + encoded := aws.StringValue(out.Output) + if encoded == "" { + return "", nil + } + raw, derr := base64.StdEncoding.DecodeString(encoded) + if derr != nil { + return encoded, nil + } + return string(raw), nil +} + +func dumpConsoleOutput(t *testing.T, c *AWSClient, instanceID string) { + t.Helper() + console, err := InstanceConsole(c, instanceID) + if err != nil { + fmt.Printf(" console-output: %v\n", err) return } - if out.Output == nil || aws.StringValue(out.Output) == "" { + if console == "" { fmt.Println(" console-output: empty") return } - raw, derr := base64.StdEncoding.DecodeString(aws.StringValue(out.Output)) - if derr != nil { - raw = []byte(aws.StringValue(out.Output)) - } - tail := raw + tail := []byte(console) if len(tail) > 4096 { tail = tail[len(tail)-4096:] } @@ -88,23 +105,16 @@ func dumpConsoleOutput(t *testing.T, c *AWSClient, instanceID string) { // CI artifacts alone. Best-effort: failures log, they don't fail the test. func DumpInstanceConsole(t *testing.T, c *AWSClient, instanceID, dir, name string) { t.Helper() - out, err := c.EC2.GetConsoleOutput(&ec2.GetConsoleOutputInput{ - InstanceId: aws.String(instanceID), - }) + console, err := InstanceConsole(c, instanceID) if err != nil { - t.Logf("console capture: GetConsoleOutput(%s) failed: %v", instanceID, err) + t.Logf("console capture: %v", err) return } - encoded := aws.StringValue(out.Output) - if encoded == "" { + if console == "" { t.Logf("console capture: %s empty", instanceID) return } - raw, derr := base64.StdEncoding.DecodeString(encoded) - if derr != nil { - raw = []byte(encoded) - } - DumpFile(t, dir, name, raw) + DumpFile(t, dir, name, []byte(console)) } func dumpOVNState(t *testing.T, instanceID string) { diff --git a/tests/e2e/single/datapath_test.go b/tests/e2e/single/datapath_test.go index 140a5def1..145b112b5 100644 --- a/tests/e2e/single/datapath_test.go +++ b/tests/e2e/single/datapath_test.go @@ -21,11 +21,32 @@ import ( // run-e2e.sh:3243-3248 (Phase 8e step 2). Passed as base64 — aws-sdk-go // expects UserData already base64-encoded, unlike the AWS CLI which encodes // plaintext for you. +// +// The trailing loop reports whether the listener actually came up on the +// serial console: target-sg has no SSH ingress, so the console is the only +// channel back to the test, and without it a slow cloud-init here reads as a +// datapath failure in AllowedTraffic below. const targetUserData = `#!/bin/bash systemd-run --unit=sge-http --description="Phase 8e HTTP server" \ /usr/bin/python3 -m http.server 8080 --bind 0.0.0.0 +for _ in $(seq 1 60); do + if curl -fsS -m 2 -o /dev/null http://127.0.0.1:8080/; then + echo "SGE-TARGET-HTTP-READY" | tee /dev/console + exit 0 + fi + sleep 1 +done +echo "SGE-TARGET-HTTP-FAIL" | tee /dev/console +systemctl status sge-http --no-pager | tee /dev/console ` +// Serial-console markers targetUserData prints once it knows whether +// python3 -m http.server bound :8080. +const ( + targetHTTPReadyMarker = "SGE-TARGET-HTTP-READY" + targetHTTPFailMarker = "SGE-TARGET-HTTP-FAIL" +) + // sgDatapathRevokeRounds returns how many times the revoke/re-authorize // ingress round-trip repeats against the same client/target pair, // overridable via SPINIFEX_SGDATAPATH_REVOKE_ROUNDS. Defaults to 1 — a @@ -49,14 +70,21 @@ func sgDatapathRevokeRounds() int { // address_set) and waits for client SSH. Every later stage depends on a // working SSH session, so its failure aborts the rest of the scenario // rather than let four more stages time out for the same reason. +// - TargetHTTPReady waits for the target guest to report its listener up +// on the serial console. AllowedTraffic below cannot pass without it, and +// a curl probe run before it would fail with "connection refused" — +// indistinguishable at a glance from an ACL that never programmed, and +// the reason this scenario used to flake. So it gates AllowedTraffic, +// which then only has to cover chassis flow install. // - AllowedTraffic proves the sg-to-sg 8080 rule actually passes traffic. // DeniedTraffic (client -> target:22) checks a different port and an -// unrelated rule, so it runs regardless of AllowedTraffic's outcome — a -// real, independent signal either way. But the revoke/restore round-trip -// below is only meaningful against a *proven* working baseline: if -// AllowedTraffic never actually worked, "traffic is now blocked after -// revoke" would trivially and misleadingly pass for the wrong reason. So -// AllowedTraffic gates the revoke rounds. +// unrelated rule — and asserts a drop, so it needs no listener at all — +// so it runs regardless of either outcome above, a real independent +// signal either way. But the revoke/restore round-trip below is only +// meaningful against a *proven* working baseline: if AllowedTraffic never +// actually worked, "traffic is now blocked after revoke" would trivially +// and misleadingly pass for the wrong reason. So AllowedTraffic gates the +// revoke rounds. // - Each revoke round's restore half only depends on the revoke API call // having actually removed the rule (tracked separately from the // ICMP-style "verify blocked" assertion) — a flaky drop-detection @@ -237,22 +265,70 @@ func runSGPolicyDatapath(t *testing.T, fix *Fixture) { t.Fatalf("PortGroupMembership stage failed; skipping every later stage that depends on client SSH") } - // --- AllowedTraffic: client -> target:8080 must succeed --- + // --- TargetHTTPReady: the target's listener is actually up --- - allowedOK := t.Run("AllowedTraffic", func(t *testing.T) { - // Retry to give target's cloud-init time to start python3 -m http.server. - // Bash uses up to 30 attempts at 2s — keep the same outer budget. - harness.Step(t, "8e-4 allowed traffic client -> target:%s:8080", targetPriv) - harness.EventuallyErr(t, func() error { - out, err := runSSHCombined(clientTgt, curlCmd) - if err != nil { - return fmt.Errorf("client -> target:8080 failed: %w (out=%q)", err, out) + targetReadyOK := t.Run("TargetHTTPReady", func(t *testing.T) { + harness.Step(t, "8e-3 wait for target-vm HTTP listener marker on the serial console") + var console string + t.Cleanup(func() { + if t.Failed() { + harness.DumpFile(t, fix.ArtifactDir(t), "target-console.log", []byte(console)) } - return nil - }, 60*time.Second, 2*time.Second) - harness.Detail(t, "step4", "allowed_traffic_ok") + }) + // Boot-and-cloud-init scale, not propagation scale: the guest's own + // user-data spends up to 60s waiting on the listener before it gives + // up and reports the failure marker. + harness.EventuallyErr(t, func() error { + var err error + console, err = harness.InstanceConsole(fix.AWS, targetID) + if err != nil { + return err + } + if strings.Contains(console, targetHTTPReadyMarker) || strings.Contains(console, targetHTTPFailMarker) { + return nil + } + return fmt.Errorf("no HTTP readiness marker on target-vm console yet (%d bytes)", len(console)) + }, 5*time.Minute, 5*time.Second) + + if strings.Contains(console, targetHTTPFailMarker) { + t.Fatalf("target-vm reported %s: python3 -m http.server never bound :8080 (console saved to artifacts)", + targetHTTPFailMarker) + } + harness.Detail(t, "step3", "target_http_ready") }) + // --- AllowedTraffic: client -> target:8080 must succeed --- + // + // Skipped outright when the listener never came up: the probe could only + // re-report that failure under a name pointing at the wrong subsystem. + allowedOK := false + if targetReadyOK { + allowedOK = t.Run("AllowedTraffic", func(t *testing.T) { + harness.Step(t, "8e-4 allowed traffic client -> target:%s:8080", targetPriv) + t.Cleanup(func() { + if t.Failed() { + harness.DumpVPCFlowDiagnostics(t, fix.AWS, targetID, + fmt.Sprintf("8e-4 target %s:8080 unreachable from client %s", targetPriv, clientPriv), + harness.VPCDiagnosticsOpts{ + LogicalIP: targetPriv, + ArtifactDir: fix.ArtifactDir(t), + }) + } + }) + // Port groups, address set and listener are all proven by here, + // so the only variable left is ovn-controller installing the + // chassis flows. Bash used 30 attempts at 2s; keep that budget. + harness.EventuallyErr(t, func() error { + out, err := runSSHCombined(clientTgt, curlCmd) + if err != nil { + return fmt.Errorf("client -> target:8080 failed: %w (out=%q)", err, out) + } + return nil + }, 60*time.Second, 2*time.Second) + harness.Detail(t, "step4", "allowed_traffic_ok") + }) + } + // --- DeniedTraffic: client -> target:22 must fail --- // // Independent of AllowedTraffic: a different port and an unrelated rule @@ -272,7 +348,7 @@ func runSGPolicyDatapath(t *testing.T, fix *Fixture) { // never actually got through, "blocked after revoke" would trivially and // misleadingly pass for the wrong reason. if !allowedOK { - t.Fatalf("AllowedTraffic never succeeded; skipping the revoke/restore round-trip since it would have nothing to detect a change from") + t.Fatalf("no proven-working 8080 baseline (target listener readiness or AllowedTraffic failed); skipping the revoke/restore round-trip since it would have nothing to detect a change from") } rounds := sgDatapathRevokeRounds()