Upgrade fabric test-network #770

munapower · 2024-07-22T15:01:30Z

What this PR does / why we need it:
It updates the fabric version used in the test-network samples directory to align with the docker versions
Which issue(s) this PR fixes:

Fixes #768

Special notes for your reviewer:

Does this PR introduce a user-facing changes and/or breaks backward compatability?:

mbrandenburger · 2024-07-23T15:37:06Z

@munapower I've pushed a bunch of changes as promised. Can you please have a look?

I still would like to take the opportunity to rework the "Prepare FPC Containers" section and maybe even the "Interact with the FPC Chaincode" section. This could also go into a seperate PR - Let's discuss/

mbrandenburger · 2024-07-23T15:37:27Z

@osamamagdy can you please check and test this PR as well?

munapower · 2024-07-23T18:01:33Z

@munapower I've pushed a bunch of changes as promised. Can you please have a look?

I still would like to take the opportunity to rework the "Prepare FPC Containers" section and maybe even the "Interact with the FPC Chaincode" section. This could also go into a seperate PR - Let's discuss/

@mbrandenburger I tried it, up to standing the network. Had to make changes to setup.sh and Readme for it to work .... the first part where we set echo and do a make build ... that doesn't work on my laptop ...

munapower · 2024-07-23T18:02:34Z

samples/deployment/test-network/README.md

@@ -26,68 +27,52 @@ Use `CC_ID` and `CC_PATH` to define the FPC Chaincode you want to build.
 cd $FPC_PATH/samples/deployment/test-network
 export CC_ID=echo
 export CC_PATH=$FPC_PATH/samples/chaincode/echo
+# TODO this needs to be changed!
+export CC_VER=$(cat CC_PATH/)


this doesn't work

Didn't work with me either. I also tried to type export CC_VER=$(echo $CC_PATH) as cat was erroring out No such file or directory but the make also failed with

# github.com/hyperledger/fabric-private-chaincode/ercc/attestation attestation/pdo.go:16:19: undefined: pdo.NewEpidLinkableVerifier attestation/pdo.go:17:19: undefined: pdo.NewEpidUnlinkableVerifier make[1]: *** [Makefile:18: ercc] Error 1

I think it should point to the mrenclave file like the example here https://github.com/munapower/fabric-private-chaincode/tree/upgradeTestNetwork/samples/chaincode/simple-asset-go#start-the-test-network

And since the mrenclave should not be created at this point I think the command should be moved a little further

After applying

cd $FPC_PATH/samples/deployment/test-network export CC_ID=echo export CC_PATH=$FPC_PATH/samples/chaincode/echo make build export CC_VER=$(cat $CC_PATH/mrenclave)

It failed with

docker build --build-arg FPC_VERSION=main -t fpc/fpc-echo:main -f Dockerfile.fpc-app \ --build-arg HW_EXTENSION=\ ERROR: "docker buildx build" requires exactly 1 argument. See 'docker buildx build --help'. Usage: docker buildx build [OPTIONS] PATH | URL | - Start a build make[1]: *** [Makefile:73: docker-fpc-app] Error 1 make[1]: Leaving directory '/project/src/github.com/hyperledger/fabric-private-chaincode/ecc' make: *** [Makefile:22: ecc-container] Error 2

The Makefile in the $FPC_PATH/samples/deployment/test-network is not working for any example I test. But the Makefile inside echo-go and simple-asset-go works.
It's noteworthy that even the fpc-echo-go image was built successfully and the ./installFPC worked for it but I wasn't able to initialize the enclave and faced this error

cd $FPC_PATH/samples/application/simple-go CC_ID=echo ORG_NAME=Org1 go run . -withLifecycleInitEnclave 2024-07-23 21:52:51.870 UTC 0001 INFO [sdk-test] main -> --> Invoke LifecycleInitEnclave 2024-07-23 21:52:51.885 UTC 0002 FATA [sdk-test] main -> Failed to invoke LifecycleInitEnclave: Failed to query init enclave: Transaction processing for endorser [localhost:7051]: Endorser Client Status Code: (23) CHAINCODE_NAME_NOT_FOUND. Description: make sure the chaincode echo has been successfully defined on channel mychannel and try again: chaincode echo not found exit status 1

osamamagdy · 2024-07-23T20:50:24Z

samples/deployment/test-network/setup.sh

+if [ ! -d "${FABRIC_SAMPLES}/bin" ]; then
+  echo "Error: no fabric binaries found. Let's run 'network.sh prereq'"
+  pushd "${FABRIC_SAMPLES}/test-network"
+  ./network.sh prereq


Before deleting my already present fabric_samples folder, I run ./setup.sh and the bin folder wasn't there but ./network.sh prereq doesn't work if you run it

@osamamagdy be careful because setup is now using a newer commit of fabric samples ....

mbrandenburger · 2024-07-24T08:10:19Z

I was thinking that we maybe should remove anything related to "how to build" the fpc chaincode (and ercc) somewhere else .... and focus in the test-network tutorial on how to run the thing.

For instance, in could provide a "how to build" tutorial of the "auction-go" in https://github.com/hyperledger/fabric-private-chaincode/tree/main/samples/chaincode/auction-go and link it in the test network tutorial.

WDYT?

munapower · 2024-07-24T18:49:54Z

I was thinking that we maybe should remove anything related to "how to build" the fpc chaincode (and ercc) somewhere else .... and focus in the test-network tutorial on how to run the thing.

For instance, in could provide a "how to build" tutorial of the "auction-go" in https://github.com/hyperledger/fabric-private-chaincode/tree/main/samples/chaincode/auction-go and link it in the test network tutorial.

WDYT?

I would just leave that to be able to deploy a chaincode they need to have these env variables set and the chaincode compiled. They can choose any of the chaincode samples to test on the network.

mbrandenburger

LGTM

bvavala

I have several comments and questions. Overall, it seems unusual to me that a minor fabric (samples) version upgrade requires such a substantial overhaul under the samples deployment section -- also considering that this is not tested in the CI.

samples/deployment/test-network/Makefile

bvavala · 2024-07-25T15:59:44Z

integration/deployment_test.sh

@@ -42,9 +53,11 @@ run_test() {

    # install something non-fpc
    # following the official fabric docs at https://hyperledger-fabric.readthedocs.io/en/release-2.5/deploy_chaincode.html
-    CC_PATH="${FPC_PATH}/samples/deployment/test-network/fabric-samples/asset-transfer-basic/chaincode-go/"
+    local tmp_dir=$(mktemp -d -t fabric-samples-XXXXXXXXXX --tmpdir="${TMP_DIR}")
+    download_chaincode "${tmp_dir}"


Is there a reason to move from using a module to clone a repo?
What was wrong with the previous approach?

samples/deployment/test-network/README.md

samples/deployment/test-network/docker-compose.yml

bvavala · 2024-07-25T16:12:25Z

samples/deployment/test-network/installFPC.sh

-# Note: to see only log and not peer command output (which is all on stderr),
-#       run the script with '... 2>/dev/null' ..
+CHANNEL_NAME=${CHANNEL_NAME:-mychannel}
+FABRIC_SAMPLES=${FPC_PATH}/samples/deployment/test-network/fabric-samples


This does not look right.
Isn't the fabric-samples module removed from this PR, and placed in a temp folder?

no, it is still cloned into the test-network directory

no, the tutorial uses the setup script to clone the fabric samples on demand. The integration test just does a spatial clone to get a single chaincode. While github allows to download individual files via curl, they don't allow to download entire folders.

bvavala · 2024-07-25T16:15:18Z

samples/deployment/test-network/setup.sh

+  git clone https://github.com/hyperledger/fabric-samples "${FABRIC_SAMPLES}"
+  pushd "${FABRIC_SAMPLES}"
+  git checkout -b "works" 50b69f6
+  popd


So now there is a download in a temp folder, and a clone in what was the original module folder.
Any reason for this added complexity?

@bvavala what temp directory? this was always like this, but before we had the user manually execute these instructions, now we have just automated it for them .... the git clone and git checkout were in the Readme before.

See deployment-test.sh.
I see that the commands were in the repo, but now I'm wondering:

why do we have a git clone in a folder where the same repo is already supposed to be initialized as a module?

also, we have the module initialized at commit X, but we clone and checkout commit Y. Why so?

I have mixed feelings about having fabric-samples as git submodule. When working with the test-network is often happens that you want to wipe out the entire folder and start from scratch. Unfortunately, their test-network does not have a neat make clean to do that job. Well one could argue one could just use git to reset. I don't see any issue with a quick git clone. But maybe keeping fabric samples a submodule would be better. mhhhhh

@bvavala @mbrandenburger but, for this PR we are just upgrading from what we currently had. I do think we can think about changing it, but I would do that in another PR ... Remember the objective for this PR is to upgrade the fabric network used in the test-network samples because it broke when we upgraded the docker version used in the project.

samples/deployment/test-network/setup.sh

bvavala · 2024-08-06T18:39:01Z

Just run the CI on main and it passes -- the PR needs some debug.

- Simplify test-network setup - Remove fpc ccaas scripts - Use network.sh CCaaS install script - Update Blockchain explorer setup - Add test-network sample test Signed-off-by: munapower <mmunaro@hotmail.com> Signed-off-by: Marcus Brandenburger <bur@zurich.ibm.com>

Signed-off-by: Marcus Brandenburger <bur@zurich.ibm.com>

mbrandenburger · 2024-09-03T15:44:58Z

@munapower @bvavala Can you please have a look at the this PR again. As discussed today in the FPC community meeting today, it should be "feature" complete and works fine on my local linux box.

munapower

With the small change to how to compile the chaincode everything that I could test of this sample on my M1 worked.

samples/deployment/test-network/README.md

Signed-off-by: Marcus Brandenburger <bur@zurich.ibm.com>

munapower requested a review from a team as a code owner July 22, 2024 15:01

munapower force-pushed the upgradeTestNetwork branch from 5218343 to a4f648a Compare July 22, 2024 15:30

mbrandenburger force-pushed the upgradeTestNetwork branch from a4f648a to 722f1a5 Compare July 23, 2024 15:25

munapower commented Jul 23, 2024

View reviewed changes

osamamagdy reviewed Jul 23, 2024

View reviewed changes

mbrandenburger force-pushed the upgradeTestNetwork branch from 153ac14 to 6ab2e84 Compare July 24, 2024 12:03

mbrandenburger approved these changes Jul 25, 2024

View reviewed changes

mbrandenburger requested a review from osamamagdy July 25, 2024 15:25

mbrandenburger added the bug Something isn't working label Jul 25, 2024

bvavala reviewed Jul 25, 2024

View reviewed changes

mbrandenburger force-pushed the upgradeTestNetwork branch 3 times, most recently from 65b1cfb to 4236ad9 Compare July 26, 2024 16:07

mbrandenburger requested a review from bvavala July 26, 2024 16:07

mbrandenburger marked this pull request as draft August 30, 2024 15:55

munapower and others added 2 commits September 3, 2024 15:56

Update test network

3744d88

- Simplify test-network setup - Remove fpc ccaas scripts - Use network.sh CCaaS install script - Update Blockchain explorer setup - Add test-network sample test Signed-off-by: munapower <mmunaro@hotmail.com> Signed-off-by: Marcus Brandenburger <bur@zurich.ibm.com>

Bump fabric 2.5.9

0bb029f

Signed-off-by: Marcus Brandenburger <bur@zurich.ibm.com>

mbrandenburger force-pushed the upgradeTestNetwork branch from 0270443 to 0bb029f Compare September 3, 2024 13:57

mbrandenburger changed the title ~~Upgrade test network fabric version~~ Upgrade fabric test-network Sep 3, 2024

mbrandenburger marked this pull request as ready for review September 3, 2024 14:33

munapower commented Sep 3, 2024

View reviewed changes

samples/deployment/test-network/README.md Show resolved Hide resolved

fixup! Update test network

dec9ecf

Signed-off-by: Marcus Brandenburger <bur@zurich.ibm.com>

mbrandenburger force-pushed the upgradeTestNetwork branch from 3afeadb to dec9ecf Compare September 5, 2024 14:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade fabric test-network #770

Upgrade fabric test-network #770

munapower commented Jul 22, 2024

mbrandenburger commented Jul 23, 2024

mbrandenburger commented Jul 23, 2024

munapower commented Jul 23, 2024 •

edited

Loading

munapower Jul 23, 2024

osamamagdy Jul 23, 2024

osamamagdy Jul 23, 2024 •

edited

Loading

osamamagdy Jul 23, 2024 •

edited

Loading

osamamagdy Jul 23, 2024

osamamagdy Jul 23, 2024

munapower Jul 23, 2024

mbrandenburger commented Jul 24, 2024 •

edited

Loading

munapower commented Jul 24, 2024

mbrandenburger left a comment

bvavala left a comment

bvavala Jul 25, 2024

bvavala Jul 25, 2024

munapower Jul 25, 2024

mbrandenburger Jul 26, 2024

bvavala Jul 25, 2024

munapower Jul 25, 2024

bvavala Jul 26, 2024

mbrandenburger Jul 26, 2024

munapower Jul 26, 2024

bvavala commented Aug 6, 2024

mbrandenburger commented Sep 3, 2024

munapower left a comment

Upgrade fabric test-network #770

Are you sure you want to change the base?

Upgrade fabric test-network #770

Conversation

munapower commented Jul 22, 2024

mbrandenburger commented Jul 23, 2024

mbrandenburger commented Jul 23, 2024

munapower commented Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

osamamagdy Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

osamamagdy Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbrandenburger commented Jul 24, 2024 • edited Loading

munapower commented Jul 24, 2024

mbrandenburger left a comment

Choose a reason for hiding this comment

bvavala left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bvavala commented Aug 6, 2024

mbrandenburger commented Sep 3, 2024

munapower left a comment

Choose a reason for hiding this comment

munapower commented Jul 23, 2024 •

edited

Loading

osamamagdy Jul 23, 2024 •

edited

Loading

osamamagdy Jul 23, 2024 •

edited

Loading

mbrandenburger commented Jul 24, 2024 •

edited

Loading