hubcio commented on code in PR #3384:
URL: https://github.com/apache/iggy/pull/3384#discussion_r3339900371
##########
.github/config/components.yml:
##########
@@ -281,6 +281,8 @@ components:
paths:
- "scripts/run-bdd-tests.sh"
- "bdd/docker-compose.yml"
+ - "bdd/docker-compose.server.yml"
+ - "bdd/docker-compose.cluster.yml"
Review Comment:
i like what you did here
##########
bdd/docker-compose.coverage.yml:
##########
@@ -52,7 +55,7 @@ services:
- sh
- -c
- >-
- go test -v ${GO_TEST_EXTRA_FLAGS}
+ go test -v
Review Comment:
the go coverage command lost `${GO_TEST_EXTRA_FLAGS}` that the base
`docker-compose.yml` go-bdd command still has. a compose override replaces the
command rather than merging it, so under `--coverage go-race` and `--coverage
all` the go coverage run no longer gets `-race`. restore it: `go test -v
${GO_TEST_EXTRA_FLAGS:-} -coverpkg=...
-coverprofile=/reports/go-bdd-coverage.out ./...`
##########
scripts/run-bdd-tests.sh:
##########
@@ -28,23 +28,59 @@ for arg in "$@"; do
done
SDK="${ARGS[0]:-all}"
-FEATURE="${ARGS[1]:-scenarios/basic_messaging.feature}"
+FEATURE="${ARGS[1]:-all}"
-export DOCKER_BUILDKIT=1 FEATURE GO_TEST_EXTRA_FLAGS="${GO_TEST_EXTRA_FLAGS:-}"
+log(){ printf "%b\n" "$*"; }
+
+usage(){
+ log "Usage: $0 [--coverage] <sdk> [feature]"
+ log ""
+ log " sdk: rust | python | go | go-race | node | csharp | java | all |
clean (default: all)"
+ log " feature: basic_messaging | leader_redirection | all (default: all)"
+ log ""
+ log "Examples:"
+ log " $0 rust # run all features for Rust"
+ log " $0 rust basic_messaging # run only basic_messaging for Rust"
+ log " $0 all leader_redirection # run leader_redirection for all
supporting SDKs"
+ log " $0 --coverage go basic_messaging"
+}
+
+case "$FEATURE" in
+ basic_messaging|leader_redirection|all) ;;
+ *)
+ log "Unknown feature: ${FEATURE}"
+ usage
+ exit 2 ;;
+esac
+
+export DOCKER_BUILDKIT=1 BDD_FEATURE="$FEATURE"
GO_TEST_EXTRA_FLAGS="${GO_TEST_EXTRA_FLAGS:-}"
cd "$(dirname "$0")/../bdd"
-COMPOSE_CMD=(docker compose -f docker-compose.yml)
+ALL_COMPOSE_FILES=(
+ -f docker-compose.yml
+ -f docker-compose.server.yml
+ -f docker-compose.cluster.yml
+ -f docker-compose.coverage.yml
+)
+
+COMPOSE_FILES=(-f docker-compose.yml)
+case "$FEATURE" in
Review Comment:
this `case` matches `basic_messaging|leader_redirection|all`, which is every
value `FEATURE` can hold (already validated above), so server.yml is added
unconditionally - the branch is a no-op. just seed the array:
`COMPOSE_FILES=(-f docker-compose.yml -f docker-compose.server.yml)` and keep
the single `case` below for cluster.yml.
##########
bdd/README.md:
##########
@@ -35,16 +39,22 @@ bdd/
### Quick Start
```bash
-# Run all SDK tests
+# Usage: ../scripts/run-bdd-tests.sh [--coverage] <sdk> [feature]
+# sdk: rust | python | go | node | csharp | java | all | clean
(default: all)
Review Comment:
sdk list is missing `go-race`, which the script's own usage text lists and
accepts as a valid first-arg. add it here so the doc matches the script.
##########
scripts/run-bdd-tests.sh:
##########
@@ -56,12 +92,28 @@ fi
run_suite(){
local svc="$1" emoji="$2" label="$3"
- log "${emoji} ${label}…"
- set +e
- "${COMPOSE_CMD[@]}" up --build --abort-on-container-exit --exit-code-from
"$svc" "$svc"
- local code=$?
- set -e
- "${COMPOSE_CMD[@]}" down -v --remove-orphans >/dev/null 2>&1 || true
+ if [ "$FEATURE" = "leader_redirection" ]; then
+ case "$svc" in
+ rust-bdd|go-bdd|csharp-bdd) ;;
+ *)
+ if [ "$SDK" = "all" ]; then
+ log "⚠️ skipping ${SDK} (does not support ${FEATURE})"
Review Comment:
this logs `${SDK}`, but the skip branch is only reachable when `SDK=all` (a
single unsupported sdk hits the `return 1` else branch), so it always prints
"skipping all (does not support leader_redirection)". use the service being
skipped instead, e.g. strip `-bdd` from `$svc`.
##########
scripts/run-bdd-tests.sh:
##########
@@ -56,12 +92,28 @@ fi
run_suite(){
local svc="$1" emoji="$2" label="$3"
- log "${emoji} ${label}…"
- set +e
- "${COMPOSE_CMD[@]}" up --build --abort-on-container-exit --exit-code-from
"$svc" "$svc"
- local code=$?
- set -e
- "${COMPOSE_CMD[@]}" down -v --remove-orphans >/dev/null 2>&1 || true
+ if [ "$FEATURE" = "leader_redirection" ]; then
+ case "$svc" in
+ rust-bdd|go-bdd|csharp-bdd) ;;
+ *)
+ if [ "$SDK" = "all" ]; then
+ log "⚠️ skipping ${SDK} (does not support ${FEATURE})"
+ return 0
+ else
+ log "❌ ${SDK} does not support feature '${FEATURE}'"
+ return 1
+ fi
+ ;;
+ esac
+ fi
+
+ log "${emoji} ${label}..."
+ local code=0
+ docker compose "${COMPOSE_FILES[@]}" \
+ up --build --abort-on-container-exit --exit-code-from "$svc" "$svc" \
Review Comment:
`--exit-code-from` already implies `--abort-on-container-exit` (per `docker
compose up --help`), so passing both is redundant. can drop
`--abort-on-container-exit`.
##########
bdd/docker-compose.server.yml:
##########
@@ -0,0 +1,106 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you 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.
+
+# Single Iggy server test setup.
+# Activated by: ./scripts/run-bdd-tests.sh <sdk> basic_messaging
+# ./scripts/run-bdd-tests.sh <sdk> leader_redirection
+# ./scripts/run-bdd-tests.sh <sdk> all
+
+services:
+ iggy-server:
+ image: iggy-bdd-server
+ platform: linux/amd64
+ build:
+ context: ..
+ dockerfile: core/server/Dockerfile
+ target: runtime-prebuilt
+ args:
+ PREBUILT_IGGY_SERVER: ${IGGY_SERVER_PATH:-target/debug/iggy-server}
+ PREBUILT_IGGY_CLI: ${IGGY_CLI_PATH:-target/debug/iggy}
+ LIBC: glibc
+ PROFILE: debug
+ command: [ "--fresh", "--with-default-root-credentials" ]
+ cap_add:
+ - SYS_NICE
+ security_opt:
+ - seccomp:unconfined
+ ulimits:
+ memlock:
+ soft: -1
+ hard: -1
+ healthcheck:
+ test: [ "CMD", "/usr/local/bin/iggy", "--tcp-server-address",
"127.0.0.1:8090", "ping" ]
+ interval: 20s
+ timeout: 3s
+ retries: 30
+ start_period: 2s
Review Comment:
not introduced by this PR, but flagging since it now lives in a fresh file:
with `start_period: 2s` and `interval: 20s` the container isn't marked healthy
until the first 20s probe even though the debug server is usually up in a
couple seconds, and every per-suite reboot pays that wait. bumping
`start_period` or lowering `interval` would speed the run up. fine as a
follow-up.
##########
bdd/README.md:
##########
@@ -35,16 +39,22 @@ bdd/
### Quick Start
```bash
-# Run all SDK tests
+# Usage: ../scripts/run-bdd-tests.sh [--coverage] <sdk> [feature]
+# sdk: rust | python | go | node | csharp | java | all | clean
(default: all)
+# feature: basic_messaging | leader_redirection | all (default: all)
+
+# Run all features for all SDKs
../scripts/run-bdd-tests.sh all
-# Run specific SDK tests
+# Run specific SDK tests (all features)
../scripts/run-bdd-tests.sh rust
../scripts/run-bdd-tests.sh python
-../scripts/run-bdd-tests.sh go
-../scripts/run-bdd-tests.sh node
-../scripts/run-bdd-tests.sh csharp
-../scripts/run-bdd-tests.sh java
+
+# Run only basic_messaging feature for Rust SDK
+../scripts/run-bdd-tests.sh rust basic_messaging
+
+# Run only leader_redirection
+../scripts/run-bdd-tests.sh all leader_redirection
# Clean up Docker resources
../scripts/run-bdd-tests.sh clean
Review Comment:
two more README nits further down, outside the diff so noting them here:
- the "How it Works" section (~70-73) still describes a single "Server
Container" that "builds and runs the latest iggy server" - stale now that it's
split into a single-server file plus a leader+follower cluster file.
- ~84 and ~94: rust impl path is given as `core/bdd/` but it's `bdd/rust/`,
and it points at `.github/changed-files-config.json` when the real config is
`.github/config/components.yml`.
##########
bdd/docker-compose.cluster.yml:
##########
@@ -0,0 +1,165 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you 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.
+
+# Iggy leader + follower cluster test setup.
+# Activated by: ./scripts/run-bdd-tests.sh <sdk> leader_redirection
+# ./scripts/run-bdd-tests.sh <sdk> all
+
+services:
+ iggy-leader:
+ image: iggy-bdd-server
+ platform: linux/amd64
+ build:
+ context: ..
+ dockerfile: core/server/Dockerfile
+ target: runtime-prebuilt
+ args:
+ PREBUILT_IGGY_SERVER: ${IGGY_SERVER_PATH:-target/debug/iggy-server}
+ PREBUILT_IGGY_CLI: ${IGGY_CLI_PATH:-target/debug/iggy}
+ LIBC: glibc
+ PROFILE: debug
+ command: [ "--fresh", "--with-default-root-credentials", "--replica-id",
"0" ]
+ cap_add:
+ - SYS_NICE
+ security_opt:
+ - seccomp:unconfined
+ ulimits:
+ memlock:
+ soft: -1
+ hard: -1
+ healthcheck:
+ test: [ "CMD", "/usr/local/bin/iggy", "--tcp-server-address",
"127.0.0.1:8091", "ping" ]
+ interval: 20s
+ timeout: 3s
+ retries: 30
+ start_period: 2s
+ environment:
+ - RUST_LOG=info
+ - IGGY_SYSTEM_PATH=local_data_leader
+ - IGGY_TCP_ADDRESS=0.0.0.0:8091
+ - IGGY_HTTP_ADDRESS=0.0.0.0:3001
+ - IGGY_QUIC_ADDRESS=0.0.0.0:8081
+ - IGGY_WEBSOCKET_ADDRESS=0.0.0.0:8071
+ - IGGY_CLUSTER_ENABLED=true
+ - IGGY_CLUSTER_NAME=test-cluster
+ - IGGY_CLUSTER_NODES_0_NAME=leader-node
+ - IGGY_CLUSTER_NODES_0_IP=iggy-leader
+ - IGGY_CLUSTER_NODES_0_REPLICA_ID=0
+ - IGGY_CLUSTER_NODES_0_PORTS_TCP=8091
+ - IGGY_CLUSTER_NODES_0_PORTS_QUIC=8081
+ - IGGY_CLUSTER_NODES_0_PORTS_HTTP=3001
+ - IGGY_CLUSTER_NODES_0_PORTS_WEBSOCKET=8071
+ - IGGY_CLUSTER_NODES_1_NAME=follower-node
+ - IGGY_CLUSTER_NODES_1_IP=iggy-follower
+ - IGGY_CLUSTER_NODES_1_REPLICA_ID=1
+ - IGGY_CLUSTER_NODES_1_PORTS_TCP=8092
+ - IGGY_CLUSTER_NODES_1_PORTS_QUIC=8082
+ - IGGY_CLUSTER_NODES_1_PORTS_HTTP=3002
+ - IGGY_CLUSTER_NODES_1_PORTS_WEBSOCKET=8072
+ volumes:
+ - iggy_leader_data:/app/local_data_leader
+ networks:
+ - iggy-bdd-network
+
+ iggy-follower:
+ image: iggy-bdd-server
+ platform: linux/amd64
+ build:
+ context: ..
+ dockerfile: core/server/Dockerfile
+ target: runtime-prebuilt
+ args:
+ PREBUILT_IGGY_SERVER: ${IGGY_SERVER_PATH:-target/debug/iggy-server}
+ PREBUILT_IGGY_CLI: ${IGGY_CLI_PATH:-target/debug/iggy}
+ LIBC: glibc
+ PROFILE: debug
+ command: [ "--fresh", "--with-default-root-credentials", "--follower",
"--replica-id", "1" ]
+ cap_add:
+ - SYS_NICE
+ security_opt:
+ - seccomp:unconfined
+ ulimits:
+ memlock:
+ soft: -1
+ hard: -1
+ healthcheck:
+ test: [ "CMD", "/usr/local/bin/iggy", "--tcp-server-address",
"127.0.0.1:8092", "ping" ]
+ interval: 20s
+ timeout: 3s
+ retries: 30
+ start_period: 2s
+ environment:
+ - RUST_LOG=info
+ - IGGY_SYSTEM_PATH=local_data_follower
+ - IGGY_TCP_ADDRESS=0.0.0.0:8092
+ - IGGY_HTTP_ADDRESS=0.0.0.0:3002
+ - IGGY_QUIC_ADDRESS=0.0.0.0:8082
+ - IGGY_WEBSOCKET_ADDRESS=0.0.0.0:8072
+ - IGGY_CLUSTER_ENABLED=true
+ - IGGY_CLUSTER_NAME=test-cluster
+ - IGGY_CLUSTER_NODES_0_NAME=leader-node
Review Comment:
this `IGGY_CLUSTER_*` roster block is byte-identical to the leader's above,
and the build/cap_add/security_opt/ulimits blocks are duplicated across leader,
follower, and the iggy-server in server.yml. a yaml anchor for the shared base
plus one for the roster would cut a chunk of this and remove the foot-gun where
the two rosters silently drift.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
