hyperledger-labs / hyperledger-labs/fablo

test-04-v3-snapshot-ccaas still flaky - sleep insufficient, needs retry with || true pattern

Open
#734 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Shell
Stars
238
Forks
113
Avg merge
3d 20h
Merged PRs (30d)
10

Description

## Description

`test-04-v3-snapshot-ccaas.sh` fails intermittently in CI. The first `expectInvokeRest` call after snapshot restore (line 132) hits `DEADLINE_EXCEEDED` due to a race condition between the CCaaS container being ready and the peer re-establishing its gRPC connection.

This was previously addressed in #647 → #648, but the fix (`sleep 2`, later `sleep 3`) is insufficient under variable CI load.

---

## Root Cause

[`waitForContainer`](https://github.com/hyperledger-labs/fablo/blob/main/e2e-network/docker/test-04-v3-snapshot-ccaas.sh#L120-L121) confirms the CCaaS container logged `"Bootstrap process completed"` – meaning the gRPC **server** is listening. However, the Fabric **peer** re-establishing its gRPC **client** connection happens asynchronously after that log message. A fixed sleep cannot reliably account for this under variable CI load.

---
## Why `sleep` Alone Isn't Sufficient (and why the original retry was tricky)

PR #648 added a `sleep` before the invoke, which helps in most cases.The PR also originally explored a retry loop, but there's a subtle interaction with the test environment that made it ineffective:

1. [`test-04` uses `set -e`](https://github.com/hyperledger-labs/fablo/blob/main/e2e-network/docker/test-04-v3-snapshot-ccaas.sh#L3) – aborts on any non-zero exit
2. [`expectInvokeRest`](https://github.com/hyperledger-labs/fablo/blob/main/e2e-network/docker/test-04-v3-snapshot-ccaas.sh#L38-L40) calls `expect-invoke-rest.sh` via `sh`
3. [`expect-invoke-rest.sh` calls `exit 1` on failure](https://github.com/hyperledger-labs/fablo/blob/main/e2e-network/docker/expect-invoke-rest.sh#L54) – hard exit
4. Under `set -e`, the non-zero exit code aborts the entire script before the loop can retry

The `|| true` pattern from `test-01` addresses this by absorbing the exit code, allowing the loop to continue to the next attempt.

---

## Proven Solution Already in the Codebase

[`test-01-v3-simple.sh` lines 48–61](https://github.com/hyperledger-labs/fablo/blob/main/e2e-network/docker/test-01-v3-simple.sh#L48-L61) solve this exact class of problem with `expectQueryWithRetry`:

```bash
output="$(expectQuery "$1" "$2" "$3" "$4" "$5" 2>&1 || true)"
```

- `2>&1` captures both stdout and stderr into the variable
- `|| true` absorbs the non-zero exit code so `set -e` doesn't abort
- `grep` checks the captured output for the failure marker

---

## Proposed Fix

Add `expectInvokeRestWithRetry` using the same pattern:

```bash
expectInvokeRestWithRetry() {
local output=""
for i in $(seq 1 10); do
output="$(expectInvokeRest "$@" 2>&1 || true)"
if echo "$output" | grep -qF "ok (rest)"; then
echo "$output"
return 0
fi
echo "Invoke failed after restore, retrying... ($i/10)"
sleep 3
done
echo "$output"
echo "❌ All retries exhausted"
exit 1
}
```

We'll apply only to the **first** `expectInvokeRest` after snapshot restore (line 132). The second invoke runs normally since the connection is already established by then.

---

Happy to open a PR for this fix if it sounds good.

## Related

- #647 : original issue (closed)
- #648 : previous fix with `sleep` (merged, insufficient)
- #728 : re-confirmed as known issue by maintainer

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.