From 629d24f68b13e583790e14139a638b3fd1ff5681 Mon Sep 17 00:00:00 2001 From: Andrei Kvapil Date: Wed, 7 Oct 2026 09:48:25 +0200 Subject: [PATCH 1/6] test(e2e): find the nodes the cells need, not the nodes that lack a label Worker discovery asked "which nodes are not control-plane", and every cell then used the answer as "which nodes run a satellite". Those are the same set on the project's own stands and not elsewhere: a Cozystack cluster of three control-plane nodes runs satellites on all three and has no node without the label, so the selector returned nothing, WORKER_1..3 came back empty and require_workers skipped every scenario as unexercisable. A stand where nothing can be exercised is indistinguishable from a stand where everything passes. The satellite DaemonSet is the authority on the question actually being asked, so it answers when the label selector cannot: one extra call, and only on the clusters where the first answer was empty. require_workers counts Ready off that same set rather than re-running the selector. BS_WORKERS overrides both, for a stand whose shape neither rule fits. The worker-count expansion is guarded for bash older than 4.4, which treats "${empty[@]}" as unbound under `set -u` and would turn "this cluster has no satellites" into a crash inside the preflight whose job is to report it. Verified against a three-control-plane Cozystack cluster: discovery returns all three nodes and the preflight passes where it previously skipped. Assisted-by: LLM Signed-off-by: Andrei Kvapil --- tests/e2e/lib.sh | 48 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 6 deletions(-) diff --git a/tests/e2e/lib.sh b/tests/e2e/lib.sh index dace71fc..aa3c4b65 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" From 9c4f40b0222563ab866a3104efa59919e41c8d45 Mon Sep 17 00:00:00 2001 From: Andrei Kvapil Date: Wed, 7 Oct 2026 09:48:25 +0200 Subject: [PATCH 2/6] test(rest): wait for the test server's own readiness, not a dial pickFreeAddr closes its probe listener before Start binds, so a parallel test can take the port in between. The readiness dial then reached that test's server, this one had already failed to bind, and every request was answered by a store that never held the fixture, which surfaced as a 404 from the handler under test. The harness now waits for the server's OnReady and retries a bind that lost the race on a fresh port. Assisted-by: LLM Signed-off-by: Andrei Kvapil --- pkg/rest/server_test.go | 59 +++++++++++++++++++++++++++++++---------- 1 file changed, 45 insertions(+), 14 deletions(-) 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() { From 00ff0505d6e18ebc285bc34fe4aad660511ba73a Mon Sep 17 00:00:00 2001 From: Andrei Kvapil Date: Wed, 7 Oct 2026 10:00:42 +0200 Subject: [PATCH 3/6] test(harness): make the replay runnable off the project's own stands The runner discovered nodes the way the cli-matrix lib did, as "not a control-plane node", so on a Cozystack cluster of three control-plane nodes it found none and skipped every workflow. The satellite DaemonSet answers when the label selector cannot, and BS_WORKERS overrides both. The pool name is a property of the stand, not of the workflow: the project's stands call it `stand`, a Cozystack cluster calls it whatever its LinstorCluster declares. BS_SP overrides it, which is the difference between a harness and a fixture. Assisted-by: LLM Signed-off-by: Andrei Kvapil --- tests/operator-harness/replay-runner.sh | 32 +++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/tests/operator-harness/replay-runner.sh b/tests/operator-harness/replay-runner.sh index c6191def..3cc68698 100755 --- a/tests/operator-harness/replay-runner.sh +++ b/tests/operator-harness/replay-runner.sh @@ -182,10 +182,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]:-}" @@ -206,6 +224,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. From 059e3eaa63cfcb6be51836856fede39caa78c6ff Mon Sep 17 00:00:00 2001 From: Andrei Kvapil Date: Wed, 4 Nov 2026 17:39:49 +0100 Subject: [PATCH 4/6] test(rest): close the shared client's unused connections before a stop The test server helper allowed two seconds for Start to return after cancel. Shutdown counts a connection no request has arrived on as idle only five seconds after it was opened, and the tests' shared client leaves such connections in its pool when it dials for a request that another connection then serves. A few random tests failed every run with "server did not stop within 2s" while no request was in flight. Assisted-by: LLM Signed-off-by: Andrei Kvapil --- pkg/rest/server_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/rest/server_test.go b/pkg/rest/server_test.go index 73f62bd0..8e9a415d 100644 --- a/pkg/rest/server_test.go +++ b/pkg/rest/server_test.go @@ -126,6 +126,13 @@ func startServerCustom(t *testing.T, srv *Server) (string, func()) { } 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: From 3b17f84f7932d0b006a9d00c8b10c7f16516faeb Mon Sep 17 00:00:00 2001 From: Andrei Kvapil Date: Wed, 4 Nov 2026 18:37:44 +0100 Subject: [PATCH 5/6] test(e2e): find satellites only in the namespace the run works in The fallback listed satellite pods in every namespace, while on_node and the harness exec into the run's namespace alone. A matching pod elsewhere on an earlier-sorted node became a worker no cell could reach. Assisted-by: LLM Signed-off-by: Andrei Kvapil --- tests/e2e/lib.sh | 2 +- tests/e2e/lib_test.go | 65 +++++++++++++++++++++++++ tests/operator-harness/replay-runner.sh | 2 +- 3 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 tests/e2e/lib_test.go diff --git a/tests/e2e/lib.sh b/tests/e2e/lib.sh index aa3c4b65..df544bd0 100755 --- a/tests/e2e/lib.sh +++ b/tests/e2e/lib.sh @@ -100,7 +100,7 @@ else if (( ${#_BS_WORKERS[@]} == 0 )); then mapfile -t _BS_WORKERS < <( - kubectl get pods -A -l app=blockstor-satellite \ + kubectl -n "$NS" get pods -l app=blockstor-satellite \ -o jsonpath='{.items[*].spec.nodeName}' 2>/dev/null \ | tr ' ' '\n' | grep -v '^$' | sort -u ) diff --git a/tests/e2e/lib_test.go b/tests/e2e/lib_test.go new file mode 100644 index 00000000..9622e06b --- /dev/null +++ b/tests/e2e/lib_test.go @@ -0,0 +1,65 @@ +// 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, 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"*) echo "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 namespace the run works +// in: on_node looks for the pod there, so a node with a satellite only in +// another namespace 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 satellites of blockstor-system only", got) + } +} diff --git a/tests/operator-harness/replay-runner.sh b/tests/operator-harness/replay-runner.sh index 3cc68698..8f662a72 100755 --- a/tests/operator-harness/replay-runner.sh +++ b/tests/operator-harness/replay-runner.sh @@ -198,7 +198,7 @@ else if (( ${#WORKERS[@]} == 0 )); then mapfile -t WORKERS < <( - kubectl get pods -A -l app=blockstor-satellite \ + kubectl -n "${NS:-blockstor-system}" get pods -l app=blockstor-satellite \ -o jsonpath='{.items[*].spec.nodeName}' 2>/dev/null \ | tr ' ' '\n' | grep -v '^$' | sort -u ) From cbd180f507102e21abefb722a5d12104b66d5232 Mon Sep 17 00:00:00 2001 From: Andrei Kvapil Date: Wed, 4 Nov 2026 23:55:57 +0100 Subject: [PATCH 6/6] test(e2e): take workers only from Running satellites on_node and the harness exec only into a Running satellite pod, so a completed pod on an earlier-sorted node became a worker no cell could reach. Assisted-by: LLM Signed-off-by: Andrei Kvapil --- tests/e2e/lib.sh | 1 + tests/e2e/lib_test.go | 16 +++++++++------- tests/operator-harness/replay-runner.sh | 1 + 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/tests/e2e/lib.sh b/tests/e2e/lib.sh index df544bd0..3f15ccaf 100755 --- a/tests/e2e/lib.sh +++ b/tests/e2e/lib.sh @@ -101,6 +101,7 @@ else 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 ) diff --git a/tests/e2e/lib_test.go b/tests/e2e/lib_test.go index 9622e06b..88eadb1d 100644 --- a/tests/e2e/lib_test.go +++ b/tests/e2e/lib_test.go @@ -28,21 +28,23 @@ import ( // 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, and a matching pod on an earlier-sorted node in another -// namespace. +// 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"*) echo "node-b node-a node-c" ;; + *"-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 namespace the run works -// in: on_node looks for the pod there, so a node with a satellite only in -// another namespace would be a worker no cell can reach. +// 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() @@ -60,6 +62,6 @@ func TestTheSatelliteFallbackStaysInTheRunsNamespace(t *testing.T) { } if got := strings.TrimSpace(string(out)); got != "node-a node-b node-c" { - t.Errorf("workers %q, want the satellites of blockstor-system only", got) + 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 8f662a72..fe6edc6b 100755 --- a/tests/operator-harness/replay-runner.sh +++ b/tests/operator-harness/replay-runner.sh @@ -199,6 +199,7 @@ else 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 )