diff --git a/pkg/rest/server_test.go b/pkg/rest/server_test.go index a5a5a6b1..8e9a415d 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,29 +74,65 @@ 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() { + // Shutdown counts a connection no request has arrived on as idle + // only five seconds after it was opened, longer than this helper + // waits. The tests' shared client leaves such connections behind: + // it dials for a request that another connection then serves, and + // pools the new one unused. Closing them first lets Shutdown see + // only the requests still being served. + http.DefaultClient.CloseIdleConnections() cancel() select { case <-errCh: diff --git a/tests/e2e/lib.sh b/tests/e2e/lib.sh index dace71fc..3f15ccaf 100755 --- a/tests/e2e/lib.sh +++ b/tests/e2e/lib.sh @@ -78,10 +78,35 @@ 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 -n "$NS" get pods -l app=blockstor-satellite \ + --field-selector status.phase=Running \ + -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 +962,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/e2e/lib_test.go b/tests/e2e/lib_test.go new file mode 100644 index 00000000..88eadb1d --- /dev/null +++ b/tests/e2e/lib_test.go @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: Apache-2.0 + +/* +Copyright 2026 Cozystack contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package e2e_test + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// fakeKubectl answers the node and satellite queries the e2e lib makes at +// source time: no node without the control-plane label, three satellites in +// blockstor-system, a Succeeded one there on an earlier-sorted node, and a +// matching pod on an earlier-sorted node in another namespace. +const fakeKubectl = `#!/usr/bin/env bash +args="$*" +case "$args" in + *"get nodes"*) ;; + *"-n blockstor-system get pods"*"status.phase=Running"*) echo "node-b node-a node-c" ;; + *"-n blockstor-system get pods"*) echo "completed-0 node-b node-a node-c" ;; + *"get pods -A"*) echo "foreign-0 node-b node-a node-c" ;; + *) echo "unexpected kubectl $args" >&2; exit 1 ;; +esac +` + +// The satellite fallback picks the workers from the Running satellites of the +// namespace the run works in: on_node looks for a Running pod there, so a node +// with a satellite only elsewhere, or only a completed one, would be a worker +// no cell can reach. +func TestTheSatelliteFallbackStaysInTheRunsNamespace(t *testing.T) { + t.Parallel() + + bin := t.TempDir() + if err := os.WriteFile(filepath.Join(bin, "kubectl"), []byte(fakeKubectl), 0o755); err != nil { + t.Fatalf("write stub: %v", err) + } + + cmd := exec.CommandContext(t.Context(), "bash", "-c", `source ./lib.sh && echo "$WORKER_1 $WORKER_2 $WORKER_3"`) + cmd.Env = append(os.Environ(), "PATH="+bin+string(os.PathListSeparator)+os.Getenv("PATH"), "NS=blockstor-system", "BS_WORKERS=") + + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("source lib.sh: %v\n%s", err, out) + } + + if got := strings.TrimSpace(string(out)); got != "node-a node-b node-c" { + t.Errorf("workers %q, want the Running satellites of blockstor-system only", got) + } +} diff --git a/tests/operator-harness/replay-runner.sh b/tests/operator-harness/replay-runner.sh index c6191def..fe6edc6b 100755 --- a/tests/operator-harness/replay-runner.sh +++ b/tests/operator-harness/replay-runner.sh @@ -182,10 +182,29 @@ 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 -n "${NS:-blockstor-system}" get pods -l app=blockstor-satellite \ + --field-selector status.phase=Running \ + -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]:-}" @@ -206,6 +225,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.