Decouple fabric tools image from fabric sample

[SamYuan1990]

Decouple fabric tools image from fabric sample.
change list for running on github action:

• network for orderer.
• file path for add org3.
• core.yaml missing?

The current PR seems has many code smells... but it seems works.

[SamYuan1990]

Creating config transaction to add org3 to network
Using organization 1
++ peer channel fetch config config_block.pb -o localhost:7050 --ordererTLSHostnameOverride orderer.example.com -c mychannel --tls --cafile /home/runner/work/fabric-samples/fabric-samples/test-network/addOrg3/organizations/ordererOrganizations/example.com/tlsca/tlsca.example.com-cert.pem
Fetching the most recent configuration block for the channel
2024-03-20 15:09:14.997 UTC 0001 ERRO [main] InitCmd -> Fatal error when initializing core config : error when reading core config file: Config File "core" Not Found in "[/home/runner/work/fabric-samples/fabric-samples/test-network/addOrg3]"

@denyeart , I am got confused a little, could you please help with which core config is missing here? when we try to add org3?

[SamYuan1990]

@denyeart , please help retry failed test case and review this PR. :-)

[satota2]

@SamYuan1990 Thank you for submitting the PR.
I have some review comments:

• I noticed the use of symbolic links for scripts and organizations folders, which might be confusing for beginners looking at the code. I believe it would be better to use relative paths instead of symbolic links. Would it be difficult to make this change?
• Many of the modified files contain comments stating like "This script is designed to be run in the cli container," which should also be updated.
• For org3, I think it's preferable to refer to core.yaml in fabric-samples/config, similar to org1 and 2, by setting the FABRIC_CFG_PATH instead of adding a new core file.
• Additionally, the intermediate artifacts related to configtx that were previously generated within cli container, such as *.json, *.tx, and *.pb files, remain in the root of the test-network or addOrg3 directories. It might be better to generate these in a dedicated directory, like channel-artifacts, to keep things organized. Would this approach be feasible?
• The Test Network SBE GitHub test seems to be failing due to a different issue unrelated to this PR, so it might be okay to ignore it for now.

[satota2]

For org3, I think it's preferable to refer to core.yaml in fabric-samples/config, similar to org1 and 2, by setting the FABRIC_CFG_PATH instead of adding a new core file. Specifically, adjusting FABRIC_CFG_PATH in here to FABRIC_CFG_PATH=$PWD/../../config/ and appending this setting in here could potentially ensure it works as intended without the core.yaml.

[SamYuan1990]

@SamYuan1990 Thank you for submitting the PR. I have some review comments:

• I noticed the use of symbolic links for scripts and organizations folders, which might be confusing for beginners looking at the code. I believe it would be better to use relative paths instead of symbolic links. Would it be difficult to make this change?
• Many of the modified files contain comments stating like "This script is designed to be run in the cli container," which should also be updated.
• For org3, I think it's preferable to refer to core.yaml in fabric-samples/config, similar to org1 and 2, by setting the FABRIC_CFG_PATH instead of adding a new core file.
• Additionally, the intermediate artifacts related to configtx that were previously generated within cli container, such as *.json, *.tx, and *.pb files, remain in the root of the test-network or addOrg3 directories. It might be better to generate these in a dedicated directory, like channel-artifacts, to keep things organized. Would this approach be feasible?
• The Test Network SBE GitHub test seems to be failing due to a different issue unrelated to this PR, so it might be okay to ignore it for now.

@satota2, with my hands on, the most challenging for this PR is the loop of relative paths.
for example/to be details:
as https://github.com/hyperledger/fabric-samples/blob/main/test-network/compose/compose-test-net.yaml#L155
the docker compose provides a good beginning as the /scripts folder.

which makes https://github.com/hyperledger/fabric-samples/blob/main/test-network/scripts/org3-scripts/joinChannel.sh#L26 org3 related script starts from this absolute directory.

the symbolic links... provides the same folder structure, specific for the loop case.
for example a loop like:
https://github.com/hyperledger/fabric-samples/blob/main/test-network/scripts/org3-scripts/updateChannelConfig.sh#L27
https://github.com/hyperledger/fabric-samples/blob/main/test-network/scripts/configUpdate.sh#L9

as those scripts also reused by org1 and org2, if we just change the scripts for org3, which will break the relative paths for org1 and org2.
hence, can we adjust org3's folder structure make it parallel with org1 and org2 to make all the orgs share the same behavior for every org, such as relative paths for shell, FABRIC_CFG_PATH and generate files?

[satota2]

@satota2, with my hands on, the most challenging for this PR is the loop of relative paths. for example/to be details: as https://github.com/hyperledger/fabric-samples/blob/main/test-network/compose/compose-test-net.yaml#L155 the docker compose provides a good beginning as the /scripts folder.

which makes https://github.com/hyperledger/fabric-samples/blob/main/test-network/scripts/org3-scripts/joinChannel.sh#L26 org3 related script starts from this absolute directory.

the symbolic links... provides the same folder structure, specific for the loop case. for example a loop like: https://github.com/hyperledger/fabric-samples/blob/main/test-network/scripts/org3-scripts/updateChannelConfig.sh#L27 https://github.com/hyperledger/fabric-samples/blob/main/test-network/scripts/configUpdate.sh#L9

as those scripts also reused by org1 and org2, if we just change the scripts for org3, which will break the relative paths for org1 and org2. hence, can we adjust org3's folder structure make it parallel with org1 and org2 to make all the orgs share the same behavior for every org, such as relative paths for shell, FABRIC_CFG_PATH and generate files?

@SamYuan1990
Thank you for your response.
I understand that this solution is challenging due to scripts being called from structurally different folders.
However, I believe we should avoid using symbolic links as it may lead to confusion.

As an alternative, what about using absolute paths in each script instead of relative ones?
For example, we could use dirname $0 in each script to dynamically obtain the absolute path of the script itself and use that as a starting point. What are your thoughts on this approach?

[SamYuan1990]

@satota2, with my hands on, the most challenging for this PR is the loop of relative paths. for example/to be details: as https://github.com/hyperledger/fabric-samples/blob/main/test-network/compose/compose-test-net.yaml#L155 the docker compose provides a good beginning as the /scripts folder.
which makes https://github.com/hyperledger/fabric-samples/blob/main/test-network/scripts/org3-scripts/joinChannel.sh#L26 org3 related script starts from this absolute directory.
the symbolic links... provides the same folder structure, specific for the loop case. for example a loop like: https://github.com/hyperledger/fabric-samples/blob/main/test-network/scripts/org3-scripts/updateChannelConfig.sh#L27 https://github.com/hyperledger/fabric-samples/blob/main/test-network/scripts/configUpdate.sh#L9
as those scripts also reused by org1 and org2, if we just change the scripts for org3, which will break the relative paths for org1 and org2. hence, can we adjust org3's folder structure make it parallel with org1 and org2 to make all the orgs share the same behavior for every org, such as relative paths for shell, FABRIC_CFG_PATH and generate files?

@SamYuan1990 Thank you for your response. I understand that this solution is challenging due to scripts being called from structurally different folders. However, I believe we should avoid using symbolic links as it may lead to confusion.

As an alternative, what about using absolute paths in each script instead of relative ones? For example, we could use dirname $0 in each script to dynamically obtain the absolute path of the script itself and use that as a starting point. What are your thoughts on this approach?

I see, let's try.

[SamYuan1990]

2024-03-30 08:37:06.594 UTC 0001 ERRO [main] InitCmd -> Cannot run peer because cannot init crypto, specified path "/home/runner/work/fabric-samples/fabric-samples/test-network/addOrg3/organizations/peerOrganizations/org1.example.com/users/Admin@org1.example.com/msp" does not exist or cannot be accessed: stat /home/runner/work/fabric-samples/fabric-samples/test-network/addOrg3/organizations/peerOrganizations/org1.example.com/users/Admin@org1.example.com/msp: no such file or directory

we can't simply using FABRIC_CFG_PATH=$PWD/../../config/ ... otherwise the msp folder will be wrong.

[SamYuan1990]

@satota2, with my hands on, the most challenging for this PR is the loop of relative paths. for example/to be details: as https://github.com/hyperledger/fabric-samples/blob/main/test-network/compose/compose-test-net.yaml#L155 the docker compose provides a good beginning as the /scripts folder.
which makes https://github.com/hyperledger/fabric-samples/blob/main/test-network/scripts/org3-scripts/joinChannel.sh#L26 org3 related script starts from this absolute directory.
the symbolic links... provides the same folder structure, specific for the loop case. for example a loop like: https://github.com/hyperledger/fabric-samples/blob/main/test-network/scripts/org3-scripts/updateChannelConfig.sh#L27 https://github.com/hyperledger/fabric-samples/blob/main/test-network/scripts/configUpdate.sh#L9
as those scripts also reused by org1 and org2, if we just change the scripts for org3, which will break the relative paths for org1 and org2. hence, can we adjust org3's folder structure make it parallel with org1 and org2 to make all the orgs share the same behavior for every org, such as relative paths for shell, FABRIC_CFG_PATH and generate files?

@SamYuan1990 Thank you for your response. I understand that this solution is challenging due to scripts being called from structurally different folders. However, I believe we should avoid using symbolic links as it may lead to confusion.
As an alternative, what about using absolute paths in each script instead of relative ones? For example, we could use dirname $0 in each script to dynamically obtain the absolute path of the script itself and use that as a starting point. What are your thoughts on this approach?

I see, let's try.

@satota2 , I suppose I found a way to make it by a env named as test_network_home please help review this PR. @denyeart , once this PR been merged, which means we decoupled fabric tools from fabric sample, then we can deprecated fabric tools image.

[satota2]

@SamYuan1990

@satota2 , I suppose I found a way to make it by a env named as test_network_home please help review this PR. @denyeart , once this PR been merged, which means we decoupled fabric tools from fabric sample, then we can deprecated fabric tools image.

Thank you for your work. I believe this way of using an environment variable named test_network_home simplifies the process compared to the previous way that involved symbolic links, making it more understandable for the readers. Could you please refine the implementation following this direction?
I also think that using uppercase for the test_network_home environment variable would be preferable since it is a convention for environment variables.
Furthermore, I would appreciate it if you could reconsider the following comments:

• For org3, it seems better to refer to core.yaml in fabric-samples/config, similar to org1 and 2, by setting the FABRIC_CFG_PATH instead of adding a new core file.
• Additionally, the intermediate artifacts related to configtx that were previously generated within the cli container, such as *.json, *.tx, and *.pb files, remain in the root of the test-network or addOrg3 directories. It might be more organized to generate these in a dedicated directory, like channel-artifacts, to keep things tidy. Would this approach be workable?

[satota2]

Suggested change

	jq -s '.[0] * {"channel_group":{"groups":{"Application":{"groups": {"Org3MSP":.[1]}}}}}' config.json ../organizations/peerOrganizations/org3.example.com/org3.json > modified_config.json
	jq -s '.[0] * {"channel_group":{"groups":{"Application":{"groups": {"Org3MSP":.[1]}}}}}' config.json ${test_network_home}/organizations/peerOrganizations/org3.example.com/org3.json > modified_config.json

[satota2]

@SamYuan1990, Cc: @denyeart

Thank you for the update. The code has been refined, and I was able to verify that Org3 can now be added in the local environment as well.
However, it appears that the following improvements I previously suggested have not yet been incorporated. Would it be possible for you to address these areas? We might consider merging this PR as it stands and then either you, I, or someone else could quickly create a new PR to tackle these areas for further code cleanup. How does that sound?

Comments for improvements I previously suggested:

• I think that using uppercase for the test_network_home environment variable would be preferable since it is a convention for environment variables.
• For org3, it seems better to refer to core.yaml in fabric-samples/config, similar to org1 and 2, by setting the FABRIC_CFG_PATH instead of adding a new core file.
• The intermediate artifacts related to configtx that were previously generated within the cli container, such as *.json, *.tx, and *.pb files, remain in the root of the test-network or addOrg3 directories. It might be more organized to generate these in a dedicated directory, like channel-artifacts, to keep things tidy.

[SamYuan1990]

@SamYuan1990, Cc: @denyeart

Thank you for the update. The code has been refined, and I was able to verify that Org3 can now be added in the local environment as well. However, it appears that the following improvements I previously suggested have not yet been incorporated. Would it be possible for you to address these areas? We might consider merging this PR as it stands and then either you, I, or someone else could quickly create a new PR to tackle these areas for further code cleanup. How does that sound?

Comments for improvements I previously suggested:

• I think that using uppercase for the test_network_home environment variable would be preferable since it is a convention for environment variables.
• For org3, it seems better to refer to core.yaml in fabric-samples/config, similar to org1 and 2, by setting the FABRIC_CFG_PATH instead of adding a new core file.
• The intermediate artifacts related to configtx that were previously generated within the cli container, such as *.json, *.tx, and *.pb files, remain in the root of the test-network or addOrg3 directories. It might be more organized to generate these in a dedicated directory, like channel-artifacts, to keep things tidy.

hi @satota2 , my goal to have this PR is to proof we can deprecated fabric tools image but not make a perfect change for multiple org solution. Hence the enhancement or totally refactor can be in other PRs.

[satota2]

@SamYuan1990 Cc: @denyeart

hi @satota2 , my goal to have this PR is to proof we can deprecated fabric tools image but not make a perfect change for multiple org solution. Hence the enhancement or totally refactor can be in other PRs.

Thank you for your response. We'll mark the tasks up to this point as completed within this PR and do the refactoring and improvements in a new PR. Thanks for all your hard work so far.

Would you be interested in handling the refactoring? Your involvement would be welcome. However, if you prefer not to, that's perfectly fine. Either I or someone else can take it from there.

[SamYuan1990]

@SamYuan1990 Cc: @denyeart

hi @satota2 , my goal to have this PR is to proof we can deprecated fabric tools image but not make a perfect change for multiple org solution. Hence the enhancement or totally refactor can be in other PRs.

Thank you for your response. We'll mark the tasks up to this point as completed within this PR and do the refactoring and improvements in a new PR. Thanks for all your hard work so far.

Would you be interested in handling the refactoring? Your involvement would be welcome. However, if you prefer not to, that's perfectly fine. Either I or someone else can take it from there.

Hi @satota2 , you can try with refactor and I would like to learn from it.

[satota2]

Hi @SamYuan1990,

Hi @satota2 , you can try with refactor and I would like to learn from it.
OK! I will attempt the refactoring once we have merged this PR. Thank you once again for the hard work!

[satota2]

Hi @SamYuan1990,

Hi @satota2 , you can try with refactor and I would like to learn from it.

OK! I will attempt the refactoring once we have merged this PR. Thank you once again for the hard work!

I've merged the PR and have now submitted a new PR (#1193) for the refactoring as discussed. Please take a look when you have a moment.

Hi @denyeart
Once the new PR (#1193) is merged, I think that the tasks related to removing fabric-tools on the fabric-samples side will be completed.