Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/fabric builder k8s #739

Merged
merged 3 commits into from
May 26, 2022
Merged

Feature/fabric builder k8s #739

merged 3 commits into from
May 26, 2022

Conversation

jkneubuh
Copy link
Contributor

@jkneubuh jkneubuh commented May 13, 2022

This PR adds support for the fabric-builder-k8s in the Kube Test Network.

fabric-builder-k8s is the missing "easy button" for working with Hyperledger Fabric Chaincode. The builder is triggered by the existing "external builder" peer lifecycle events, managing the lifecycle of CC routines as pods under an RBAC-enabled service account.

With the k8s builder approach, chaincode ... "just works."

Background Context

Installation / Activation

export TEST_NETWORK_CHAINCODE_BUILDER="k8s"

network kind 
network cluster init
network up 
network channel create
network chaincode deploy asset-transfer-basic ../asset-transfer-basic/chaincode-java
network chaincode query asset-transfer-basic '{"Args":["org.hyperledger.fabric:GetMetadata"]}'  | jq

That's it. Chaincode "just works."

@jkneubuh jkneubuh requested a review from a team as a code owner May 13, 2022 16:42
@jkneubuh jkneubuh marked this pull request as draft May 13, 2022 16:42
@jkneubuh jkneubuh requested a review from lehors May 13, 2022 17:38
@jkneubuh
Copy link
Contributor Author

/azp run

test flake

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

Signed-off-by: Josh Kneubuhl <jkneubuh@us.ibm.com>
@jkneubuh jkneubuh requested review from mbwhite and jt-nti May 19, 2022 20:11
@jkneubuh jkneubuh marked this pull request as ready for review May 19, 2022 20:13
Signed-off-by: Josh Kneubuhl <jkneubuh@us.ibm.com>
Copy link
Member

@jt-nti jt-nti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, although it's probably worth noting that the k8s builder is still an early proof of concept and liable to break often!

apiVersion: batch/v1
kind: Job
metadata:
name: org1-install-k8s-builder
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sure there's a good reason but I'm slightly puzzled by the approach of using a peer image which already has the k8s builder preconfigured, just to copy out the builder binaries for use by the peer image which the k8s peer image is based on

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is a "good reason" but I ran into a case yesterday where I needed to test some changes in the fabric-builder-k8s binaries. I was queued up with some code to switch over to the wget/curl style download from the binary release archive, and then realized that this meant there was no way for me to test my builder updates without publishing custom binaries to a web server. AND the only way to build a Linux binary is to ... run linux ... or wade through Golang args to figure out how to cross-compile the builders natively on my machine. AND ... no makefile in the builder package ... AND ... I.e.. the only way to test an edit to the builder is to publish a release on the builder project and let the GitHub action turn the crank.

Seems like a little thing but it spun out of control pretty quickly. I just wanted to test out a one line update in the golang code, but couldn't do so without a few hours of hacking on the environment.

So the workaround that seemed to do well was to use the Docker build of the builders, tag it, and load it into KIND so that the cluster would not go out to pull an image (or binary) from ghcr.io. This only worked with the distribution of binaries via Docker. Something like:

cd fabric-builder-k8s 

docker build -t ghcr.io/hyperledgendary/k8s-fabric-peer:v0.4.0-joshfancy .

kind load docker-image ghcr.io/hyperledgendary/k8s-fabric-peer:v0.4.0-joshfancy

and ...

cd test-network-k8s 

export TEST_NETWORK_K8S_CHAINCODE_BUILDER_VERSION=v0.4.0-joshfancy

...

So some work in this area -- somewhere -- still needs to be done. I agree it is a bit unconventional with the current scheme but it seemed to work out well for the task at hand.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was more that k8s-fabric-peer already has everything you need in it as-is without any messing about with wget, which is why I added/use TEST_NETWORK_FABRIC_PEER_IMAGE all the time. I'm sure I'm just missing something obvious.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aha!

The line in the sand here is : "no Kube dependencies in Fabric." (No fair building out of-band peer images that pre-bundle the builder.) The builder is either IN the peer image, or it's NOT in the peer image.

Answer: it's NOT. This PR works around the constraint of "no builder in peer."

Let's not re-visit / open : fabric #3405 or fabric #3407

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a difference between putting the k8s builder code in the fabric repo with the CCaaS builder, which seems like a fair line in the sand to preserve, and publishing a handy preconfigured peer image from the k8s builder repo. I don't mind either way but there is no "the peer image" but rather a sample peer image provided "out of the box" by Fabric, which as far as I know is not actually intended for serious use. Other peer images are available, such as the very nice k8s builder peer image :)

test-network-k8s/scripts/chaincode.sh Outdated Show resolved Hide resolved
@@ -0,0 +1,100 @@
#!/bin/sh -l
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was expecting the package_k8s_chaincode function to use this script but it doesn't seem to. Why is there a copy here instead of using the original? Hopefully the peer command will be able to handle more packages soon :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is lame ... but:

  • pkgk8scc.sh will write outputs into $PWD. It needs a flag and/or arg to set the -o "output" path for the tgz

  • Before adding this to scripts/, I was burned a bit by having routines that need to download routines and then run them locally (albeit in a container...) Little questions like ... "what happens if the remote script is broken? Someone will call me and complain that test network no longer works..." and "how can we end up with a one-liner installation in a docs / guide, that does not ask users to download a script before running?" ... etc.

  • I'm not sold on the script being in this code line at all. It is very convenient, but better homed in the builder docs / guide, or ultimately landing as a core routine in the fabric-cli (vaporware) or peer binary.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One day fabric-cli will condense!

Signed-off-by: Josh Kneubuhl <jkneubuh@us.ibm.com>
@mbwhite mbwhite merged commit 525ff9f into hyperledger:main May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants