diff --git a/pkg/rest/server_test.go b/pkg/rest/server_test.go index a5a5a6b1..73f62bd0 100644 --- a/pkg/rest/server_test.go +++ b/pkg/rest/server_test.go @@ -21,10 +21,12 @@ package rest import ( "context" "encoding/json" + "errors" "io" "net" "net/http" "net/url" + "syscall" "testing" "time" @@ -72,26 +74,55 @@ func startServerCustom(t *testing.T, srv *Server) (string, func()) { }) } - ctx, cancel := context.WithCancel(t.Context()) - errCh := make(chan error, 1) - go func() { errCh <- srv.Start(ctx) }() - - dialer := &net.Dialer{Timeout: 200 * time.Millisecond} - deadline := time.Now().Add(5 * time.Second) + // Readiness is this server's own OnReady, not a dial: the address comes + // from pickFreeAddr, which closes its probe listener before Start binds, + // and a parallel test can take the port in between. A dial then reaches + // that test's server, this one has already failed to bind, and every + // request is answered by a store that never heard of the fixture, which + // read as a 404 from the handler under test. A bind that loses the race + // is retried on a fresh port. + ownReady := srv.OnReady + + var ( + cancel context.CancelFunc + errCh chan error + ) + + for attempt := 0; ; attempt++ { + ready := make(chan struct{}) + srv.OnReady = func() { + if ownReady != nil { + ownReady() + } - for { - c, dErr := dialer.DialContext(ctx, "tcp", srv.Addr) - if dErr == nil { - _ = c.Close() - break + close(ready) } - if time.Now().After(deadline) { + var ctx context.Context + + ctx, cancel = context.WithCancel(t.Context()) + errCh = make(chan error, 1) + + go func(ch chan<- error, ctx context.Context) { ch <- srv.Start(ctx) }(errCh, ctx) + + select { + case <-ready: + case err := <-errCh: cancel() - t.Fatalf("server never became reachable: %v", dErr) + + if attempt < 5 && errors.Is(err, syscall.EADDRINUSE) { + srv.Addr = pickFreeAddr(t) + + continue + } + + t.Fatalf("server did not start: %v", err) + case <-time.After(5 * time.Second): + cancel() + t.Fatalf("server never became ready on %s", srv.Addr) } - time.Sleep(50 * time.Millisecond) + break } stop := func() { diff --git a/tests/e2e/lib.sh b/tests/e2e/lib.sh index 4af3397c..6de85f77 100755 --- a/tests/e2e/lib.sh +++ b/tests/e2e/lib.sh @@ -78,10 +78,34 @@ lctl_idempotent() { # them as $WORKER_1, $WORKER_2, $WORKER_3 instead of hardcoding a # specific cluster prefix (parallel stands name workers `-worker-N`). # Sorted alphabetically so $WORKER_1 == worker-1, etc. -mapfile -t _BS_WORKERS < <( - kubectl get nodes -l '!node-role.kubernetes.io/control-plane' \ - -o jsonpath='{.items[*].metadata.name}' | tr ' ' '\n' | sort -) +# +# What the cells actually need is nodes RUNNING A SATELLITE, and on the +# project's own stands "not a control-plane node" answers that. It does not +# answer it everywhere: a Cozystack cluster of three control-plane nodes runs +# satellites on all three and has no node without the label, so the selector +# returns nothing and every cell skips itself as unexercisable. +# +# So the satellite DaemonSet answers when the label selector cannot. It is the +# authority on the question being asked, and it costs one extra call only on +# the clusters where the first answer was empty. BS_WORKERS overrides both, for +# a stand whose shape neither rule fits. +if [[ -n "${BS_WORKERS:-}" ]]; then + # shellcheck disable=SC2086 # BS_WORKERS is a space-separated node list + mapfile -t _BS_WORKERS < <(printf '%s\n' $BS_WORKERS | sort) +else + mapfile -t _BS_WORKERS < <( + kubectl get nodes -l '!node-role.kubernetes.io/control-plane' \ + -o jsonpath='{.items[*].metadata.name}' | tr ' ' '\n' | sort + ) + + if (( ${#_BS_WORKERS[@]} == 0 )); then + mapfile -t _BS_WORKERS < <( + kubectl get pods -A -l app=blockstor-satellite \ + -o jsonpath='{.items[*].spec.nodeName}' 2>/dev/null \ + | tr ' ' '\n' | grep -v '^$' | sort -u + ) + fi +fi WORKER_1="${_BS_WORKERS[0]:-}" WORKER_2="${_BS_WORKERS[1]:-}" WORKER_3="${_BS_WORKERS[2]:-}" @@ -937,8 +961,20 @@ skip() { require_workers() { local want=$1 local got - got=$(kubectl get nodes -l '!node-role.kubernetes.io/control-plane' --no-headers 2>/dev/null \ - | awk '$2 == "Ready"' | wc -l) + # Counted off the same discovery $WORKER_* came from, so a cluster whose + # satellites run on control-plane nodes is not reported as having none. + got=0 + # The length test guards the expansion: under `set -u` a bash older than + # 4.4 treats "${empty[@]}" as an unbound variable and aborts, which would + # turn "this cluster has no satellites" into a crash inside the preflight + # that exists to report exactly that. + if (( ${#_BS_WORKERS[@]} > 0 )); then + for _w in "${_BS_WORKERS[@]}"; do + if [[ "$(kubectl get node "$_w" --no-headers 2>/dev/null | awk '{print $2}')" == "Ready" ]]; then + got=$(( got + 1 )) + fi + done + fi if (( got < want )); then skip "scenario needs $want satellite workers, found $got" diff --git a/tests/operator-harness/replay-runner.sh b/tests/operator-harness/replay-runner.sh index 186020b9..8880793a 100755 --- a/tests/operator-harness/replay-runner.sh +++ b/tests/operator-harness/replay-runner.sh @@ -177,10 +177,28 @@ source "$HARNESS_DIR/lib.sh" # Discover worker nodes for {{node1..3}} substitution. The runner is happy # with 2 or 3 nodes; workflows that need more declare it via prerequisites # and the runner skips with a clear message. -mapfile -t WORKERS < <( - kubectl get nodes -l '!node-role.kubernetes.io/control-plane' \ - -o jsonpath='{.items[*].metadata.name}' 2>/dev/null | tr ' ' '\n' | sort -) +# Same discovery the cli-matrix lib does, and for the same reason: the +# question is "which nodes run a satellite", and "not a control-plane node" +# only answers it on stands shaped like the project's own. A Cozystack cluster +# of three control-plane nodes runs satellites on all three, and the selector +# returns nothing there — so every workflow skipped itself as unexercisable. +if [[ -n "${BS_WORKERS:-}" ]]; then + # shellcheck disable=SC2086 # BS_WORKERS is a space-separated node list + mapfile -t WORKERS < <(printf '%s\n' $BS_WORKERS | sort) +else + mapfile -t WORKERS < <( + kubectl get nodes -l '!node-role.kubernetes.io/control-plane' \ + -o jsonpath='{.items[*].metadata.name}' 2>/dev/null | tr ' ' '\n' | sort + ) + + if (( ${#WORKERS[@]} == 0 )); then + mapfile -t WORKERS < <( + kubectl get pods -A -l app=blockstor-satellite \ + -o jsonpath='{.items[*].spec.nodeName}' 2>/dev/null \ + | tr ' ' '\n' | grep -v '^$' | sort -u + ) + fi +fi NODE1="${WORKERS[0]:-}" NODE2="${WORKERS[1]:-}" NODE3="${WORKERS[2]:-}" @@ -201,6 +219,12 @@ RD=$(yaml_get "$WORKFLOW" "vars.rd") RD=${RD:-$DEFAULT_RD} SP=$(yaml_get "$WORKFLOW" "vars.sp") SP=${SP:-stand} +# BS_SP overrides the workflow's pool. The pool NAME is a property of the +# stand, not of the workflow: the project's own stands call it `stand`, a +# Cozystack cluster calls it whatever its LinstorCluster declares. Without this +# every workflow is pinned to one stand's naming and cannot be replayed +# anywhere else, which is the difference between a harness and a fixture. +SP=${BS_SP:-$SP} # {{rg}} resolves from vars.rg (resource-group workflows). No synthetic # default — a workflow that references {{rg}} without declaring vars.rg # would otherwise substitute an empty string and create an unnamed group.