hyperledger-archives / fabric

THIS IS A READ-ONLY historic repository. Current development is at https://gerrit.hyperledger.org/r/#/admin/projects/fabric . pull requests not accepted
https://gerrit.hyperledger.org/
Apache License 2.0
1.17k stars 1.01k forks source link

Test chaincode deploy for proper error messaging when chanicodes exists locally as well as in remote locations #737

Open 2016Nishi opened 8 years ago

2016Nishi commented 8 years ago

Tried to send Deploy command using RestAPI from SoapUI/Postman using chaincodeexample_02 on a local peer without security setup as: [http://127.0.0.1:3000/devops/deploy]

{ "type": "GOLANG", "chaincodeID":{ "path": "https://github.com/openblockchain/obc-peer/openchain/example/chaincode/chaincode_example02" }, "ctorMsg": { "function":"init", "args":["a", "100", "b", "200"] } }

Expected Result:

It to return a unique hash code for the sample request in response from request.

Actual Result:

Response from Request: 20:00:51.278 [rest] Deploy -> ERRO 013 {"Error": "Deploying Chaincode -- Error getting chaincode package bytes: Error generating hashcode: code does not exist Download failer open /opt/gopath/usercode/925645731/src/github.com/openblockchain/obc-peer/openchain/example/chaincode/chaincode_example02: no such file or directory"}

This was on today's build:
394d5bed1e3a4e24efc6dec76413ee23f010c073

2016Nishi commented 8 years ago

Here are the combinations where Deploy responds successfully vs a failure in response:

  1. With Security Enabled/Disabled : having http:' , 'https' in the beginning of path value causes a failure.
  2. With Security Enabled/Disabled: "//github.com/openblockchain/obc-peer/openchain/example/chaincode/chaincode_example02" gets chaincode deployed successfully on peer.

Error Message:

When security was enabled seeing similar kind of error message as in one without-security

20:57:10.105 [devops] getChaincodeBytes -> ERRO 014 Error getting chaincode package bytes: Error generating hashcode: code does not exist Download failer open /opt/gopath/usercode/189210238/src/github.com/openblockchain/obc-peer/openchain/example/chaincode/chaincode_example02: no such file or directory 20:57:10.106 [devops] Deploy -> ERRO 015 Error deploying chaincode spec: type:GOLANG chaincodeID:<path:"http://github.com/openblockchain/obc-peer/openchain/example/chaincode/chaincode_example02" > ctorMsg:<function:"init" args:"a" args:"100" args:"b" args:"200" > secureContext:"jim" confidentialityLevel:CONFIDENTIAL

2016Nishi commented 8 years ago

Doing some more testing - tool that I was using input did not have the combinations as I expected it to. Will run my tests and will post observations.

2016Nishi commented 8 years ago

Reproduced the problem using Postman and SoapUI.

http:/https: do NOT seem to work (in both security/non--security env) when added in the beginning to path value during deployment of chaincodeexample_02.

build: 394d5bed1e3a4e24efc6dec76413ee23f010c073

muralisrini commented 8 years ago

713 fixed some basic issues with downloading remote chain code where we added local peer's GOPATH to the temporary GOPATH. This made downloaded chain code to reference running peer's OBC code, thus resulting in multiple benefits (better security and bandwidth savings).

But it exposed this new issue when user tries to download chain code whose HTTP path maps to a folder in obc-peer's GOPATH (e.g., the standard chaincode_example02 path). Just as OBC code is not downloaded, chaincode source code is not downloaded to the temporary directory either.

Seems there are 3 possible approaches to deal with this issue (are there any others ?)

  1. return error (something like "path exists in OBC. use local path")
  2. ignore local chain code path, download and deploy remote chain code
  3. ignore remote chain code path, use code from local peer and deploy local chain code

1) feels the right approach.... we neither want to download "github.com/openblockchain/obc-peer/openchain/examples/chaincode/chaincode_example02" remotely when the path exists locally (i.e., not 2) nor ignore remote code and silently copy local code (i.e., not 3), do we ?

If agreed, we can fix this issue so the following will return a meaningful error ./obc-peer chaincode deploy -p https://github.com/openblockchain/obc-peer/openchain/examples/chaincode/chaincode_example02 ...

Samples under obc-peer/openchain/examples can be deployed as usual using filesystem path.

We will be populating obi-samples repository which can used for testing remote chain code deployment.

Does this sound reasonable ?

2016Nishi commented 8 years ago

Murali,

When value of path in deploy command conflicts on remote OBC with the one in local path, user will be given a message to correct the situation. Does that mean earlier it was always fetching from remote repository and did not look into local repository? Now I am guessing deploy command would first check in local repository, if not in remote OBC repository, then download example code? So 'http' and 'https' are not needed any more for passing path value [cause for the error, after your fix #713]. Is that right?

Thanks! -Nishi

binhn commented 8 years ago

If we put the downloaded code at a diff loc than where obc is, we should just download it due to the potential cc code difference with the local one. Since we only download the cc, the rest of the dependencies are still local.

muralisrini commented 8 years ago

@binhn we could do that (option 2). GO will download OBC into the temporary location in addition to the chain code even if we don't use the downloaded OBC. A lot of unused code downloaded....

If we are moving to obc-samples repository and treat current samples in OBC for unit-tests, is there really a need for allowing current OBC example chaincodes to be deployed ?

Also, note that potential cc code difference with the samples implies remote OBC is different from local OBC at some level.

Seems rejecting with an error seems cleaner and safer.

markparz commented 8 years ago

@2016Nishi Can we close this?

2016Nishi commented 8 years ago

Works as per design right now. After speaking to @muralisrini we can improve on error messaging. And this is a low priority bug that can be left open.

srderson commented 8 years ago

@2016Nishi could you update the title/description in that case to reflect the current issue? Or we can close this issue and open a new one.

2016Nishi commented 8 years ago

Updated title to reflect current issue.