From e392ef85968708003e058b3c3e0d6312268d12d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ver=C3=B3nica=20L=C3=B3pez?= Date: Tue, 4 Aug 2026 02:10:00 +0200 Subject: [PATCH] fix(e2e): serialize templated runs and preserve diagnostics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Run templated tests after the other shared packages and wait for their namespaces to terminate, avoiding contention while preserving the existing query assertion and timeout. - Capture sanitised namespace diagnostics before cleanup, and align the deletion fixture and naming documentation with the enforced 25-character cluster-name limit. Signed-off-by: Verónica López --- .github/workflows/_reusable-e2e.yaml | 34 ++++-- Makefile | 77 ++++++++++-- docs/development/naming-strategy.md | 6 +- test/e2e/framework/diagnostics.go | 138 ++++++++++++++++++++++ test/e2e/framework/diagnostics_test.go | 65 ++++++++++ test/e2e/framework/helpers.go | 3 + test/e2e/shared/deletion/deletion_test.go | 2 +- 7 files changed, 305 insertions(+), 20 deletions(-) create mode 100644 test/e2e/framework/diagnostics.go create mode 100644 test/e2e/framework/diagnostics_test.go diff --git a/.github/workflows/_reusable-e2e.yaml b/.github/workflows/_reusable-e2e.yaml index 088abf95..c1a9d42f 100644 --- a/.github/workflows/_reusable-e2e.yaml +++ b/.github/workflows/_reusable-e2e.yaml @@ -217,10 +217,10 @@ jobs: esac package_file="$RUNNER_TEMP/e2e-packages" + : > "$package_file" if [ -z "$TEST_SPEC" ]; then - printf './test/e2e/%s/...\n' "$MODE" > "$package_file" + go list -tags=e2e "./test/e2e/${MODE}/..." > "$package_file" else - : > "$package_file" IFS=',' read -r -a tests <<< "$TEST_SPEC" for test_name in "${tests[@]}"; do [[ "$test_name" =~ ^[a-z0-9][a-z0-9_-]*$ ]] || @@ -234,19 +234,21 @@ jobs: echo "::error::Unknown e2e test '$test_name' for mode '$MODE'" exit 1 } - printf '%s\n' "$package" >> "$package_file" + go list -tags=e2e "$package" >> "$package_file" done fi - name: Run e2e tests env: E2E_KEEP_CLUSTERS: never + E2E_FAILURE_LOG_DIR: ${{ runner.temp }}/e2e-failure-logs E2E_POSTGRES_IMAGE: ${{ inputs.postgres-image }} E2E_MULTIADMIN_IMAGE: ${{ inputs.multiadmin-image }} E2E_MULTIADMIN_WEB_IMAGE: ${{ inputs.multiadmin-web-image }} E2E_MULTIORCH_IMAGE: ${{ inputs.multiorch-image }} E2E_MULTIPOOLER_IMAGE: ${{ inputs.multipooler-image }} E2E_MULTIGATEWAY_IMAGE: ${{ inputs.multigateway-image }} + MODE: ${{ inputs.mode }} TIMEOUT_MINUTES: ${{ inputs.timeout-minutes }} run: | [[ "$TIMEOUT_MINUTES" =~ ^[0-9]+$ ]] || @@ -261,10 +263,20 @@ jobs: exit 1 } - OPERATOR_IMG="$(make -s print-img)" \ - REPO_ROOT="$GITHUB_WORKSPACE" \ - go test -tags=e2e "${packages[@]}" -v -count=1 \ - "-timeout=${TIMEOUT_MINUTES}m" + OPERATOR_IMG="$(make -s print-img)" + export OPERATOR_IMG + REPO_ROOT="$GITHUB_WORKSPACE" + export REPO_ROOT + + if [ "$MODE" != "shared" ]; then + go test -tags=e2e "${packages[@]}" -v -count=1 \ + "-timeout=${TIMEOUT_MINUTES}m" + exit 0 + fi + + make run-e2e-shared \ + E2E_PACKAGE_FILE="$RUNNER_TEMP/e2e-packages" \ + E2E_TIMEOUT="${TIMEOUT_MINUTES}m" - name: Collect Kind logs on failure if: failure() @@ -279,6 +291,10 @@ jobs: uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 with: name: kind-logs-${{ inputs.test-spec || 'all' }} - path: /tmp/kind-logs-* - retention-days: 7 + # Public-repository artifact: only collect from this disposable Kind + # cluster, and never inject production credentials into E2E workloads. + path: | + /tmp/kind-logs-* + ${{ runner.temp }}/e2e-failure-logs + retention-days: 5 if-no-files-found: ignore diff --git a/Makefile b/Makefile index 4f5a7f5a..6a7fb8b3 100644 --- a/Makefile +++ b/Makefile @@ -317,21 +317,84 @@ test-coverage: manifests generate fmt vet setup-envtest ## Generate coverage rep E2E_TEST_SPEC ?= E2E_PACKAGES_SHARED = $(if $(E2E_TEST_SPEC),$(foreach t,$(subst $(comma),$(space),$(E2E_TEST_SPEC)),./test/e2e/shared/$(t)/),./test/e2e/shared/...) E2E_PACKAGES_DEDICATED = $(if $(E2E_TEST_SPEC),$(foreach t,$(subst $(comma),$(space),$(E2E_TEST_SPEC)),./test/e2e/dedicated/$(t)/),./test/e2e/dedicated/...) +E2E_PACKAGE_FILE ?= +E2E_TIMEOUT ?= 20m comma := , space := $(empty) $(empty) +.PHONY: run-e2e-shared +run-e2e-shared: + @packages=(); \ + if [ -n "$(E2E_PACKAGE_FILE)" ]; then \ + while IFS= read -r package; do packages+=("$$package"); done < "$(E2E_PACKAGE_FILE)"; \ + else \ + while IFS= read -r package; do packages+=("$$package"); done < <( \ + go list -tags=e2e $(E2E_PACKAGES_SHARED) \ + ); \ + fi; \ + [ "$${#packages[@]}" -gt 0 ] || { echo "No shared E2E packages selected"; exit 1; }; \ + export OPERATOR_IMG="$${OPERATOR_IMG:-$(IMG)}"; \ + export REPO_ROOT="$${REPO_ROOT:-$(shell pwd)}"; \ + run_e2e() { \ + local package_parallelism="$$1"; \ + shift; \ + go test -tags=e2e -p "$$package_parallelism" "$$@" \ + -v -count=1 "-timeout=$(E2E_TIMEOUT)"; \ + }; \ + wait_for_shared_namespaces() { \ + if ! kind get clusters | grep -Fxq e2e-shared; then return 0; fi; \ + local kubeconfig; \ + kubeconfig="$$(mktemp "$${TMPDIR:-/tmp}/e2e-shared-kubeconfig.XXXXXX")"; \ + if ! kind get kubeconfig --name e2e-shared > "$$kubeconfig"; then \ + rm -f "$$kubeconfig"; \ + return 1; \ + fi; \ + local deadline=$$((SECONDS + 180)); \ + local namespaces=(); \ + while true; do \ + namespaces=(); \ + while IFS= read -r namespace; do namespaces+=("$$namespace"); done < <( \ + kubectl --kubeconfig "$$kubeconfig" get namespaces -o name | \ + sed -n '/^namespace\/e2e-ns-/p' \ + ); \ + if [ "$${#namespaces[@]}" -eq 0 ]; then \ + rm -f "$$kubeconfig"; \ + return 0; \ + fi; \ + if [ "$$SECONDS" -ge "$$deadline" ]; then \ + echo "Timed out waiting for prior E2E namespaces to terminate: $${namespaces[*]}"; \ + kubectl --kubeconfig "$$kubeconfig" get "$${namespaces[@]}" || true; \ + rm -f "$$kubeconfig"; \ + return 1; \ + fi; \ + sleep 2; \ + done; \ + }; \ + templated_package="$$(go list -tags=e2e ./test/e2e/shared/templated/)"; \ + regular_packages=(); \ + run_templated=false; \ + for package in "$${packages[@]}"; do \ + if [ "$$package" = "$$templated_package" ]; then \ + run_templated=true; \ + else \ + regular_packages+=("$$package"); \ + fi; \ + done; \ + if [ "$${#regular_packages[@]}" -gt 0 ]; then \ + run_e2e 3 "$${regular_packages[@]}"; \ + fi; \ + if [ "$$run_templated" = true ]; then \ + if [ "$${#regular_packages[@]}" -gt 0 ]; then wait_for_shared_namespaces; fi; \ + run_e2e 1 "$$templated_package"; \ + fi + .PHONY: test-e2e test-e2e: manifests generate fmt vet container ## Run e2e tests (shared cluster, fast) - OPERATOR_IMG=$(IMG) \ - REPO_ROOT=$(shell pwd) \ - go test -tags=e2e $(E2E_PACKAGES_SHARED) -p 3 -v -count=1 -timeout=20m + $(MAKE) run-e2e-shared .PHONY: test-e2e-keep test-e2e-keep: manifests generate fmt vet container ## Run e2e tests; keep cluster on failure - OPERATOR_IMG=$(IMG) \ - REPO_ROOT=$(shell pwd) \ - E2E_KEEP_CLUSTERS=on-failure \ - go test -tags=e2e $(E2E_PACKAGES_SHARED) -p 3 -v -count=1 -timeout=20m + E2E_KEEP_CLUSTERS=on-failure $(MAKE) run-e2e-shared .PHONY: test-e2e-full test-e2e-full: manifests generate fmt vet container ## Run e2e tests (dedicated cluster per test, full isolation) diff --git a/docs/development/naming-strategy.md b/docs/development/naming-strategy.md index e5c970db..41b75d5d 100644 --- a/docs/development/naming-strategy.md +++ b/docs/development/naming-strategy.md @@ -60,14 +60,14 @@ To ensure generated resource names stay within Kubernetes limits even after addi | Field | MaxLength | Rationale | |:---|:---:|:---| -| **Cluster Name** | 30 | Root of all names; must leave room for 5-6 more levels | +| **Cluster Name** | 25 | Root of all names; must leave room for 5-6 more levels | | **Database Name** | 30 | Typically short; allows deep nesting | | **TableGroup Name** | 25 | Reduces risk of truncation in shard/pool names | | **Shard Name** | 25 | Often simple (e.g., `0-inf`, `shard1`) | | **Pool Name** | 25 | Conservative to prevent pod name truncation | | **Cell Name** | 30 | Typically az names (e.g., `us-east-1a`, `z1`) | -These limits are enforced via **CRD validation** (`+kubebuilder:validation:MaxLength=X`) in `api/v1alpha1/common_types.go`. +These limits are enforced via **CRD validation** markers in the API type definitions under `api/v1alpha1/`. ## Resources Without Hashes (No Collision Risk) @@ -80,7 +80,7 @@ Some resources use **simple string concatenation** without hashes: **Why no hash?** 1. **1:1 Relationship:** Each cluster has exactly one GlobalTopoServer and one MultiAdmin. -2. **Predictable and Short:** The cluster name is already validated to be ≤30 chars, and we only append a fixed suffix. +2. **Predictable and Short:** The cluster name is already validated to be ≤25 chars, and we only append a fixed suffix. 3. **No User-Defined Nesting:** Unlike cells/shards/pools where users can define arbitrary names, these components are static. 4. **No Collision Possible:** Since there's only ever one instance per cluster, there's no scenario where two different logical paths could produce the same name after truncation. diff --git a/test/e2e/framework/diagnostics.go b/test/e2e/framework/diagnostics.go new file mode 100644 index 00000000..d87c465e --- /dev/null +++ b/test/e2e/framework/diagnostics.go @@ -0,0 +1,138 @@ +//go:build e2e + +package framework + +import ( + "context" + "encoding/json" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" +) + +const ( + diagnosticTimeout = 2 * time.Minute + diagnosticDirectoryEnv = "E2E_FAILURE_LOG_DIR" +) + +// dumpNamespaceDiagnostics captures failure evidence before namespace cleanup. +// kubectl cluster-info dump collects standard workload state, events, and pod +// logs, but does not enumerate Secret objects. +func (c *Cluster) dumpNamespaceDiagnostics(t testing.TB, ns string) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), diagnosticTimeout) + defer cancel() + + root := os.Getenv(diagnosticDirectoryEnv) + if root == "" { + root = filepath.Join(os.TempDir(), "e2e-failure-logs") + } + dir := filepath.Join(root, safeDiagnosticName(ns)) + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Logf("create diagnostics directory for namespace %s: %v", ns, err) + return + } + c.dumpMultigresResourceState(ctx, t, ns, dir) + + cmd := exec.CommandContext( + ctx, + "kubectl", + "--kubeconfig", c.Kubeconfig, + "cluster-info", "dump", + "--namespaces", ns, + "--output-directory", dir, + "--output", "yaml", + ) + output, err := cmd.CombinedOutput() + if err != nil { + t.Logf( + "collect diagnostics for namespace %s: %v: %s", + ns, err, strings.TrimSpace(string(output)), + ) + return + } + t.Logf("captured failure diagnostics for namespace %s in %s", ns, dir) +} + +func (c *Cluster) dumpMultigresResourceState( + ctx context.Context, + t testing.TB, + ns, dir string, +) { + t.Helper() + cmd := exec.CommandContext( + ctx, + "kubectl", + "--kubeconfig", c.Kubeconfig, + "get", + "multigresclusters.multigres.com,"+ + "cells.multigres.com,"+ + "shards.multigres.com,"+ + "toposervers.multigres.com", + "--namespace", ns, + "--output", "json", + ) + output, err := cmd.Output() + if err != nil { + detail := "" + if exitErr, ok := err.(*exec.ExitError); ok { + detail = strings.TrimSpace(string(exitErr.Stderr)) + } + t.Logf("collect Multigres resource state for namespace %s: %v: %s", ns, err, detail) + return + } + output, err = resourceStatusOutput(output) + if err != nil { + t.Logf("sanitize Multigres resource state for namespace %s: %v", ns, err) + return + } + path := filepath.Join(dir, "multigres-resource-status.json") + if err := os.WriteFile(path, append(output, '\n'), 0o600); err != nil { + t.Logf("write Multigres resource state for namespace %s: %v", ns, err) + } +} + +type diagnosticResourceList struct { + Items []diagnosticResource `json:"items"` +} + +type diagnosticResource struct { + APIVersion string `json:"apiVersion"` + Kind string `json:"kind"` + Metadata diagnosticResourceMetadata `json:"metadata"` + Status json.RawMessage `json:"status,omitempty"` +} + +type diagnosticResourceMetadata struct { + Name string `json:"name"` + Namespace string `json:"namespace"` + Generation int64 `json:"generation"` +} + +func resourceStatusOutput(input []byte) ([]byte, error) { + resources := diagnosticResourceList{} + if err := json.Unmarshal(input, &resources); err != nil { + return nil, err + } + return json.MarshalIndent(resources, "", " ") +} + +func safeDiagnosticName(value string) string { + name := strings.Map(func(r rune) rune { + if (r >= 'a' && r <= 'z') || + (r >= 'A' && r <= 'Z') || + (r >= '0' && r <= '9') || + r == '.' || r == '-' || r == '_' { + return r + } + return '_' + }, value) + name = strings.Trim(name, ".") + if name == "" { + return "unnamed" + } + return name +} diff --git a/test/e2e/framework/diagnostics_test.go b/test/e2e/framework/diagnostics_test.go new file mode 100644 index 00000000..d2d1a255 --- /dev/null +++ b/test/e2e/framework/diagnostics_test.go @@ -0,0 +1,65 @@ +//go:build e2e + +package framework + +import ( + "bytes" + "testing" +) + +func TestSafeDiagnosticName(t *testing.T) { + t.Parallel() + tests := []struct { + name string + input string + want string + }{ + {name: "kubernetes name", input: "e2e-ns-1234", want: "e2e-ns-1234"}, + {name: "path separators", input: "../secret/name", want: "_secret_name"}, + {name: "dot segment", input: "..", want: "unnamed"}, + {name: "empty", input: "", want: "unnamed"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := safeDiagnosticName(tt.input); got != tt.want { + t.Fatalf("safeDiagnosticName(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} + +func TestResourceStatusOutputExcludesSpecAndAnnotations(t *testing.T) { + t.Parallel() + input := []byte(`{ + "items": [{ + "apiVersion": "multigres.com/v1alpha1", + "kind": "Shard", + "metadata": { + "name": "test-shard", + "namespace": "e2e-ns-1234", + "generation": 2, + "annotations": {"internal.example/context": "private"} + }, + "spec": {"password": "do-not-copy"}, + "status": {"podRoles": {"pooler-0": "PRIMARY"}} + }] + }`) + output, err := resourceStatusOutput(input) + if err != nil { + t.Fatalf("resourceStatusOutput: %v", err) + } + for _, excluded := range [][]byte{ + []byte(`"spec"`), + []byte(`"annotations"`), + []byte("do-not-copy"), + []byte("private"), + } { + if bytes.Contains(output, excluded) { + t.Errorf("output contains excluded context %q: %s", excluded, output) + } + } + if !bytes.Contains(output, []byte(`"pooler-0": "PRIMARY"`)) { + t.Fatalf("output does not contain pod role status: %s", output) + } +} diff --git a/test/e2e/framework/helpers.go b/test/e2e/framework/helpers.go index a7cfe933..377533d2 100644 --- a/test/e2e/framework/helpers.go +++ b/test/e2e/framework/helpers.go @@ -55,6 +55,9 @@ func (c *Cluster) CreateNamespace(t testing.TB) string { t.Fatalf("create postgres password secret: %v", err) } t.Cleanup(func() { + if t.Failed() { + c.dumpNamespaceDiagnostics(t, ns) + } _ = c.Clientset.CoreV1().Namespaces().Delete( context.Background(), ns, metav1.DeleteOptions{}) }) diff --git a/test/e2e/shared/deletion/deletion_test.go b/test/e2e/shared/deletion/deletion_test.go index a74dab33..f43a9107 100644 --- a/test/e2e/shared/deletion/deletion_test.go +++ b/test/e2e/shared/deletion/deletion_test.go @@ -103,7 +103,7 @@ func TestClusterDeletionAfterSwitchingToExternalTopo(t *testing.T) { ctx := context.Background() cr := framework.MustLoadCluster("config/samples/minimal.yaml", ns) - cr.Name = "delete-after-external-topo" + cr.Name = "delete-external-topo" cr.Spec.PVCDeletionPolicy = &multigresv1alpha1.PVCDeletionPolicy{ WhenDeleted: multigresv1alpha1.RetainPVCRetentionPolicy, WhenScaled: multigresv1alpha1.RetainPVCRetentionPolicy,