Fix createChannel wrongly recreating an already-running network - #1446
Open
wakqasahmed wants to merge 1 commit into
Open
wakqasahmed wants to merge 1 commit into
wakqasahmed wants to merge 1 commit into
Conversation
Refs hyperledger#1243. createChannel() decides whether the network is already up by counting running hyperledger/-prefixed containers and treating fewer than 4 as 'not running'. That threshold was correct in 2022 (hyperledger#641, back when a persistent cli container was also part of the default network), but the default network today only launches 3 (orderer + 2 peers) - the cli container is gone. So createChannel always sees 3 < 4, decides the network isn't up, and brings it up again via networkUp(), which does not inherit -s couchdb from the original up command - overwriting the running CouchDB-backed network with a fresh LevelDB one. Verified against real containers, not just reasoning from the compose files: reproduced with ./network.sh up -s couchdb followed by a separate ./network.sh createChannel -c mychannel - confirmed the peers get Recreated, couchdb0/couchdb1 are orphaned, and the log switches to 'using database leveldb'. With the threshold corrected to 3 (both the count check on line 337 and the bring-up decision on line 342, which share the same stale assumption), the same steps now print 'Network Running Already' and the existing containers are left untouched. Signed-off-by: wakqasahmed <wakqasahmed@gmail.com>
wakqasahmed
force-pushed
the
fix/createchannel-container-count-1243
branch
from
September 3, 2026 12:41
52a25fa to
de51c29
Compare
Author
|
Hi @bestbeforetoday — not sure who owns review for this one — it's been a little while, green and mergeable. Could you take a look or redirect me? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1243.
createChannel()decides whether the network is already up by counting runninghyperledger/-prefixed containers and treating fewer than 4 as "not running". That threshold was correct back in 2022 (#641, when a persistentclicontainer was still part of the default network) but the default network today only launches 3 (orderer + 2 peers) - theclicontainer's gone. SocreateChannelalways sees 3 < 4, decides the network isn't up, and callsnetworkUp()again - which doesn't inherit-s couchdbfrom the originalupcommand, so it silently overwrites the running CouchDB-backed network with a fresh LevelDB one, exactly as reported.I verified this against real containers rather than just reasoning from the compose files: reproduced with
./network.sh up -s couchdbfollowed by a separate./network.sh createChannel -c mychannel- confirmed the peers getRecreated,couchdb0/couchdb1end up orphaned, and the log switches tousing database leveldb. With the threshold corrected to 3, the same steps now printNetwork Running Alreadyand leave the running containers untouched.Fixed both occurrences of the stale
4- the count check that decides whether to bring the network down to resync certs (line 337) has the same assumption baked in and would stay broken if only the reported line were changed.