Simplify build and install for CCF #468

cmickeyb · 2024-02-14T23:13:15Z

Several changes to improve the performance (and remove dependencies) for CCF startup. Many of the changes are centered in the ccf Makefile to separate the creation of the ccf clients (like the ping test, ledger authority retrieval and policy scripts that might reasonably be executed from anywhere) from the environment needed for a ccf service node (the script to start a ccf network).

The big win from the separation is that we can remove dependency on the sandbox script & pre-install all of the python package dependencies independently from the execution of the ccf nodes. The result is that the ledger containers start in a couple seconds instead of several minutes.

Note that a future PR will package the ccf client scripts separately and install them with other PDO packages so they are available on all PDO installations.

@bvavala your review is requested because the second commit adds the code to copy the member keys into the
ledger key directory.

Several changes to improve the performance (and remove dependencies) for CCF startup. Many of the changes are centered in the ccf Makefile to separate the creation of the ccf clients (like the ping test, ledger authority retrieval and policy scripts that might reasonably be executed from anywhere) from the environment needed for a ccf service node (the script to start a ccf network). The big win from the separation is that we can remove dependency on the sandbox script & pre-install all of the python package dependencies independently from the execution of the ccf nodes. The result is that the ledger containers start in a couple seconds instead of several minutes. Note that a future PR will package the ccf client scripts separately and install them with other PDO packages so they are available on all PDO installations. Signed-off-by: Mic Bowman <[email protected]>

When registration is requested, we need the CCF member keys in order to perform the registration Check for the keys & copy them to ${PDO_LEDGER_KEY_ROOT}. Signed-off-by: Mic Bowman <[email protected]>

bvavala · 2024-02-14T23:35:53Z

docker/tools/run_services_tests.sh

+    try cp ${XFER_DIR}/ccf/keys/memberccf_cert.pem ${PDO_LEDGER_KEY_ROOT}/
+    try cp ${XFER_DIR}/ccf/keys/memberccf_privk.pem ${PDO_LEDGER_KEY_ROOT}/


Yes, these should make PDO_LEDGER_KEY_ROOT the authoritative place where to look for the ledger keys -- to be double checked with HW mode tests.

umm... it already was (and it is pretty well documented):

PDO_LEDGER_KEY_ROOT is the root directory where the system keys are stored for ledger integration; files in this directory are not automatically generated. When ccf is used as ledger, the ccf network cert {networkcert.pem} must be placed under this folder. These keys get generated during ccf deployment.

and:

xfer -- this directory is used to pass configuration information and keys between the container and the host; for example, to push a previously built configuration into the container, put the files in the appropriate subdirectory in xfer.

bvavala · 2024-02-14T23:36:03Z

docker/tools/start_services.sh

+    try cp ${XFER_DIR}/ccf/keys/memberccf_cert.pem ${PDO_LEDGER_KEY_ROOT}/
+    try cp ${XFER_DIR}/ccf/keys/memberccf_privk.pem ${PDO_LEDGER_KEY_ROOT}/


(same as above)

same as above.

g2flyer

Definitely speeds up drastically the start up time of ccf container although a make test still takes quite a long time (10 mins), even with warm caches. Code also looks good, liked the additional comments. See a few minor comments & suggestions below.

PS: benchmarking the reduced time for the actual test via make test_no_reset (now 2:53 on my laptop) i realized that we are missing a dependency to clean_config for target test_no_reset in make.dev. Should we just sneak in a change in this PR (with HW mode-related changes it already is now a bit multi-purpose ;-) or should i create a separate PR for that?

g2flyer · 2024-02-15T01:49:34Z

ledgers/ccf/Makefile

+
+pdo-environment : $(PDO_INSTALL_ROOT)
+
+ledger-environment : $(CCF_LEDGER_DIR)/lib/python3.8


Worth declaring the python version as a variable to prepare for the maybe-case that ccf starts supporting also ubuntu >20.04? In any case, maybe having it close to CCF_VERSION and commenting that this comes from the current intrinsic 20.04-implying-python-3.8 invariant?

possibly. although in this case the python version is completely determined by ccf.

g2flyer · 2024-02-15T01:50:21Z

ledgers/ccf/Makefile

+
+# This directory indicates whether the ccf ledger python
+# virtual environment has been created
+$(CCF_LEDGER_DIR)/lib/python3.8 : pdo-environment


See above comment on python version. Given that we use it at multiple places (also below) i think it would be even more useful to define as variable?

g2flyer · 2024-02-15T01:51:37Z

ledgers/ccf/Makefile

+	virtualenv -p python3.8 --no-download $(PDO_HOME)/ccf
+	$(CCF_LEDGER_DIR)/bin/pip install --upgrade pip
+	$(CCF_LEDGER_DIR)/bin/pip install --upgrade -r $(CCF_BASE)/bin/requirements.txt
+	$(CCF_LEDGER_DIR)/bin/pip install ccf==$(CCF_VERSION)


Curious, the pre-reqs installed in the prior step do not depend on the ccf version?

BTW: this target in principle is also cacheable and takes even with docker-cached packages one minute but will be re-executed after a make clean as it's after the COPY in the docker file. Maybe this 1 minute is not worth optimizing given that make clean test takes 10 mins, but just a thought in case we want to improve that time eventually ...

well... (and i think this is in one of the comments) for the MOMENT we require ccf 1.0.19 for the PDO installation and for the ccf pdo scripts. HOWEVER, the actual ccf nodes require a different version. i tried to get the version out of the CCF_BASE (something like: $(<"${CCF_BASE}/share/version")) but i couldn't get make to execute it in a reasonable way. feel free to suggest an alternative that would compute it. AND... the CCF_BASE requirements already have the correct list for that version

one more note continuing the python version discussion... all of the ccf scripts have the python version hardcoded in them. there is no place i could find where the python dependency is expressed in a way i can automate configuration. so 1) i don't think hard coding it is such a problem right now and 2) i don't think there is an easy way to derive it from the ccf version

g2flyer · 2024-02-15T01:56:30Z

ledgers/ccf/Makefile

-	cd $(BLDDIR) && cmake .. -GNinja \
-		-DCCF_DIR=$(CCF_BASE) \
-		-DCOMPILE_TARGET=$(COMPILE_TARGET) \
-		-DCMAKE_INSTALL_PREFIX=$(CCFDSTDIR)
 	cd $(BLDDIR) && $(NINJA)


isn't that somewhat cleaner with newer cmake as 'cmake --build $(BLDDIR)? (I guess we don't have a test target for an additionalcmake --test $(BLDDIR)`?)

yes. i'm getting there. there are several other modern cmake improvements that can be made. in this case i also don't know the best way to build with ninja in cmake. all things queued up for later investigation.

ledgers/ccf/scripts/start_ccf_network.sh

cmickeyb · 2024-02-15T16:02:57Z

PS: benchmarking the reduced time for the actual test via make test_no_reset (now 2:53 on my laptop) i realized that we are missing a dependency to clean_config for target test_no_reset in make.dev. Should we just sneak in a change in this PR (with HW mode-related changes it already is now a bit multi-purpose ;-) or should i create a separate PR for that?

I've snuck in a couple other small fixes. I'll just add this.

prakashngit · 2024-02-15T18:40:24Z

ledgers/ccf/Makefile

+# environment has been created
+$(PDO_INSTALL_ROOT) :
+	make -C $(PDO_SOURCE_ROOT)/build environment
+	make -C $(PDO_SOURCE_ROOT)/bin install


installing all of PDO environment inside the CCF container seems to be an overkill. Doesn't hurt, I guess all that we want is ccf 1.0.19, do we need anything else ?

i think you have a misunderstanding of how your code worked in the past. you were already installing everything in python_requirements.txt which is all this does (except cleaner).

AND...

I absolutely think we should be installing pdo client packages (and all of these scripts should be installed in every pdo client). there is a separate issue to track that discussion (#469 )

prakashngit

thanks, looks good.

* add dependency checking to remove parallel make issues * add secp256k1 to PDO CCF docker * more use of cmake, add clean_config to test_no_reset * wrap makefiles with check for failure Signed-off-by: Mic Bowman <[email protected]>

cmickeyb requested review from g2flyer and prakashngit February 14, 2024 23:13

Copy the member keys from the xfer directory

6b1d6cd

When registration is requested, we need the CCF member keys in order to perform the registration Check for the keys & copy them to ${PDO_LEDGER_KEY_ROOT}. Signed-off-by: Mic Bowman <[email protected]>

cmickeyb requested a review from bvavala February 14, 2024 23:17

cmickeyb assigned prakashngit Feb 14, 2024

bvavala reviewed Feb 14, 2024

View reviewed changes

g2flyer approved these changes Feb 15, 2024

View reviewed changes

prakashngit reviewed Feb 15, 2024

View reviewed changes

prakashngit approved these changes Feb 15, 2024

View reviewed changes

Small fixes to address PR feedback

27ea18b

* add dependency checking to remove parallel make issues * add secp256k1 to PDO CCF docker * more use of cmake, add clean_config to test_no_reset * wrap makefiles with check for failure Signed-off-by: Mic Bowman <[email protected]>

cmickeyb force-pushed the mic.feb12.simplify_ccf_builds branch from c4ad438 to 27ea18b Compare February 15, 2024 20:14

prakashngit merged commit a159e30 into hyperledger-labs:main Feb 15, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify build and install for CCF #468

Simplify build and install for CCF #468

cmickeyb commented Feb 14, 2024 •

edited

Loading

bvavala Feb 14, 2024

cmickeyb Feb 15, 2024 •

edited

Loading

bvavala Feb 14, 2024

cmickeyb Feb 15, 2024

g2flyer left a comment

g2flyer Feb 15, 2024

cmickeyb Feb 15, 2024

g2flyer Feb 15, 2024

g2flyer Feb 15, 2024

cmickeyb Feb 15, 2024

cmickeyb Feb 15, 2024

g2flyer Feb 15, 2024

cmickeyb Feb 15, 2024 •

edited

Loading

cmickeyb commented Feb 15, 2024

prakashngit Feb 15, 2024

cmickeyb Feb 15, 2024

prakashngit left a comment

		try cp ${XFER_DIR}/ccf/keys/memberccf_cert.pem ${PDO_LEDGER_KEY_ROOT}/
		try cp ${XFER_DIR}/ccf/keys/memberccf_privk.pem ${PDO_LEDGER_KEY_ROOT}/


		pdo-environment : $(PDO_INSTALL_ROOT)

		ledger-environment : $(CCF_LEDGER_DIR)/lib/python3.8

Simplify build and install for CCF #468

Simplify build and install for CCF #468

Conversation

cmickeyb commented Feb 14, 2024 • edited Loading

Choose a reason for hiding this comment

cmickeyb Feb 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

g2flyer 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

cmickeyb Feb 15, 2024 • edited Loading

Choose a reason for hiding this comment

cmickeyb commented Feb 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prakashngit left a comment

Choose a reason for hiding this comment

cmickeyb commented Feb 14, 2024 •

edited

Loading

cmickeyb Feb 15, 2024 •

edited

Loading

cmickeyb Feb 15, 2024 •

edited

Loading