feat: use nwaku docker instead of building binaries #1259

danisharora099 · 2023-03-23T11:12:49Z

Problem

When testing nwaku interop/running tests locally, the nwaku repo needs to be cloned (the first time) & built (along with building the Nim compiler) which takes a lot of time

Solution

Using tagged nwaku images directly within the js-waku context using dockerode that would allow for:

easily switching between nwaku versions without rebuilding binary
no need to clone nwaku within the repo
significantly reduce time it takes for CI to run (=$)
spawn/run multiple tests async (can leverage in future)

TODO:

fix CI for go-waku (use statusteam/go-waku:latest)

Notes

Resolves switch to nwaku docker images instead of locally building #1192
Testing against go-waku is currently done against latest go-waku image instead of the version as was previously being used. This is because the current infra does not automatically publish docker images for tagged releases. cc @richard-ramos

github-actions · 2023-03-23T11:17:49Z

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
Waku core	116.85 KB (0%)	2.4 s (0%)	3.3 s (+26.53% 🔺)	5.6 s
Waku default setup	387.98 KB (0%)	7.8 s (0%)	6.4 s (-2.12% 🔽)	14.1 s
ECIES encryption	28.02 KB (0%)	561 ms (0%)	1.3 s (-11.07% 🔽)	1.9 s
Symmetric encryption	28.03 KB (0%)	561 ms (0%)	1.8 s (+38.58% 🔺)	2.4 s
DNS discovery	108.07 KB (0%)	2.2 s (0%)	3.2 s (+9.98% 🔺)	5.4 s
Privacy preserving protocols	116.38 KB (0%)	2.4 s (0%)	1.7 s (-31.91% 🔽)	4 s
Light protocols	118.2 KB (0%)	2.4 s (0%)	2.8 s (-6.97% 🔽)	5.1 s
History retrieval protocols	118.21 KB (0%)	2.4 s (0%)	2.4 s (+25.88% 🔺)	4.7 s

fryorcraken

Being able to run the js-waku CI against a local binary is important when working on any interop matter.

I don't think running a node in docker should totally replace running a binary.

Happy for it to be the default method though.

OR if we want docker method to replace binary method, then how does it look like when I want to run the test with go-waku or nwaku compiled locally? Currently, only docker images supported are the one published.

weboko · 2023-03-27T22:39:19Z

packages/tests/tests/relay.node.spec.ts

@@ -427,7 +428,8 @@ describe("Waku Relay [node only]", () => {
      let nwaku: Nwaku;

      afterEach(async function () {
-        !!nwaku && nwaku.stop();
+        !!nwaku &&
+          nwaku.stop().catch((e) => console.log("Nwaku failed to stop", e));


We can use everywhere ?. operator.

@weboko
Since this change addresses a larger change in the general codebase, and also is outside the scope of this PR, I opened an issue for it: #1301

fryorcraken · 2023-03-28T05:55:53Z

OR if we want docker method to replace binary method, then how does it look like when I want to run the test with go-waku or nwaku compiled locally? Currently, only docker images supported are the one published.

As discussed in a call, an acceptable outcome would be to add a doc that explains how to run a local instance of nwaku or go-waku with the js-waku CI. I imagine something like:

cd nwaku; make wakunode2
docker build .
set some env to point to locally built docker and use it to run js-waku tests.

danisharora099 · 2023-03-28T16:43:55Z

OR if we want docker method to replace binary method, then how does it look like when I want to run the test with go-waku or nwaku compiled locally? Currently, only docker images supported are the one published.

As discussed in a call, an acceptable outcome would be to add a doc that explains how to run a local instance of nwaku or go-waku with the js-waku CI. I imagine something like:

cd nwaku; make wakunode2

docker build .

set some env to point to locally built docker and use it to run js-waku tests.

Addressed with 85c1968

fryorcraken

Would be good to add somewhere, probably the main readme or test readme that docker needs to be installed.

packages/tests/README.md

packages/tests/src/nwaku.ts

packages/tests/package.json

fryorcraken

Getting

     Error: (HTTP code 404) no such container - No such image: statusteam/nim-waku:v0.16.0 
      at /home/fryorcraken/src/waku-org/js-waku/node_modules/docker-modem/lib/modem.js:343:17
      at getCause (/home/fryorcraken/src/waku-org/js-waku/node_modules/docker-modem/lib/modem.js:373:7)
      at Modem.buildPayload (/home/fryorcraken/src/waku-org/js-waku/node_modules/docker-modem/lib/modem.js:342:5)
      at IncomingMessage.<anonymous> (/home/fryorcraken/src/waku-org/js-waku/node_modules/docker-modem/lib/modem.js:310:16)

Because I did not pull the image name. Any chance you could a try catch around this error and ask the dev have you run docker pull <imagename`"?

packages/tests/README.md

fryorcraken · 2023-03-29T04:46:08Z

packages/core/package.json

@@ -59,10 +59,6 @@
    "fix:prettier": "prettier . --write",
    "fix:lint": "eslint src *.js --fix",
    "pretest": "run-s pretest:*",
-    "pretest:1-init-git-submodules": "[ -f '../../nwaku/build/wakunode2' ] || git submodule update --init --recursive",
-    "pretest:2-build-nwaku": "[ -f '../../nwaku/build/wakunode2' ] || run-s nwaku:build",
-    "nwaku:build": "(PROC=$(nproc --all 2>/dev/null || echo 2); cd ../../nwaku; make -j$PROC update; NIMFLAGS=\"-d:chronicles_colors=off -d:chronicles_sinks=textlines -d:chronicles_log_level=TRACE\" make -j$PROC wakunode2)",


The docker image are not build with these options so logs contain some color or other random chars:

� �TRC 2023-03-29 04:42:56.808+00:00 registering protocols topics="libp2p multistream" tid=1 file=multistream.nim:224 protos="@[\"/libp2p/autonat/1.0.0\"]"

@jm-clius do we have an option to pass to wakunode2 to get clean ASCII logs?

Unfortunately compiled binary can only set logLevel and logFormat (as TEXT or JSON).

fryorcraken

works for me but first try was not pleasant. Would be good to improve the docs as suggested above.

danisharora099 · 2023-03-29T10:39:26Z

This PR is currently blocked as described in #1280

fryorcraken

bit of tidying up needed

.github/workflows/ci.yml

packages/tests/package.json

packages/tests/README.md

.github/workflows/ci.yml

fryorcraken · 2023-04-03T06:19:50Z

.github/workflows/ci.yml

      - uses: ./.github/actions/npm
      - run: npm run build:esm
+      - run: npm run pretest


ah yeah that's annoying

removed it from ci.yml by adding npm run precheck inside of test:node before running mocha within /tests specifically

CONTRIBUTING.md

packages/tests/README.md

weboko · 2023-04-04T21:05:39Z

packages/core/CHANGELOG.md

 ### ⚠ BREAKING CHANGES

-* add and implement IReceiver ([#1219](https://github.com/waku-org/js-waku/issues/1219))
+- add and implement IReceiver ([#1219](https://github.com/waku-org/js-waku/issues/1219))


Changelogs should be reverted.
Did it happen after you got my fix for prettier?

Thanks for flagging!
Fixed! Won't be a problem anymore.

packages/tests/package.json

…docker

weboko · 2023-04-11T09:08:14Z

packages/tests/src/run-tests.js

+
+import debug from "debug";
+
+const log = debug("waku:test:run");


Now it should be safe to use console.log since this is merged #1291

Addressed here: cdde79d

weboko

From my side no additional comments.
Good work 👍

danisharora099 force-pushed the feat/nwaku-docker branch from 0941b70 to c9b3bdc Compare March 23, 2023 11:14

danisharora099 changed the title ~~Feat/nwaku docker~~ feat: dockerize nwaku for tests Mar 23, 2023

danisharora099 changed the title ~~feat: dockerize nwaku for tests~~ feat: use nwaku docker instead of building binaries Mar 23, 2023

danisharora099 force-pushed the feat/nwaku-docker branch 3 times, most recently from 30fca72 to cc466ac Compare March 23, 2023 14:14

danisharora099 marked this pull request as ready for review March 23, 2023 14:21

danisharora099 requested a review from fryorcraken as a code owner March 23, 2023 14:21

danisharora099 requested a review from weboko March 23, 2023 14:21

danisharora099 force-pushed the feat/nwaku-docker branch 3 times, most recently from ff7ac51 to 56dad13 Compare March 24, 2023 10:48

fryorcraken requested changes Mar 27, 2023

View reviewed changes

weboko reviewed Mar 27, 2023

View reviewed changes

danisharora099 force-pushed the feat/nwaku-docker branch from af37a3d to 85c1968 Compare March 28, 2023 16:42

danisharora099 requested a review from fryorcraken March 28, 2023 16:43

fryorcraken reviewed Mar 29, 2023

View reviewed changes

packages/tests/README.md Outdated Show resolved Hide resolved

packages/tests/src/nwaku.ts Outdated Show resolved Hide resolved

packages/tests/package.json Outdated Show resolved Hide resolved

fryorcraken reviewed Mar 29, 2023

View reviewed changes

packages/tests/README.md Outdated Show resolved Hide resolved

fryorcraken reviewed Mar 29, 2023

View reviewed changes

packages/tests/README.md Outdated Show resolved Hide resolved

fryorcraken reviewed Mar 29, 2023

View reviewed changes

fryorcraken approved these changes Mar 29, 2023

View reviewed changes

danisharora099 force-pushed the feat/nwaku-docker branch 3 times, most recently from 376ee75 to d82aea5 Compare March 29, 2023 10:23

fryorcraken reviewed Mar 30, 2023

View reviewed changes

fryorcraken reviewed Apr 3, 2023

View reviewed changes

danisharora099 force-pushed the feat/nwaku-docker branch 4 times, most recently from b76b2fc to 258ddec Compare April 3, 2023 12:06

danisharora099 requested review from fryorcraken and weboko April 3, 2023 12:06

weboko reviewed Apr 4, 2023

View reviewed changes

packages/tests/package.json Outdated Show resolved Hide resolved

danisharora099 added 3 commits April 6, 2023 12:47

merge: master

307c020

Merge branch 'master' of github.com:waku-org/js-waku into feat/nwaku-…

1575d5a

…docker

fix: CI

e0cb86d

danisharora099 force-pushed the feat/nwaku-docker branch from 258ddec to e0cb86d Compare April 6, 2023 07:18

use: js script instead of shell script to run precheck and mocha

9921c82

danisharora099 force-pushed the feat/nwaku-docker branch from 322aec2 to 9921c82 Compare April 10, 2023 08:45

danisharora099 mentioned this pull request Apr 10, 2023

fix: peer-exchange go-waku interop #1300

Merged

weboko reviewed Apr 11, 2023

View reviewed changes

danisharora099 mentioned this pull request Apr 11, 2023

consider replacing !! to optional chaining in codebase #1301

Closed

weboko approved these changes Apr 12, 2023

View reviewed changes

use console.log instead of debug.log for run-tests.js

cdde79d

fryorcraken approved these changes Apr 17, 2023

View reviewed changes

danisharora099 merged commit dc3774c into master Apr 17, 2023

danisharora099 deleted the feat/nwaku-docker branch April 17, 2023 04:59

fryorcraken mentioned this pull request Apr 17, 2023

chore: release master #1296

Merged

danisharora099 mentioned this pull request Apr 18, 2023

infrastructure for releasing of docker images waku-org/go-waku#516

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: use nwaku docker instead of building binaries #1259

feat: use nwaku docker instead of building binaries #1259

danisharora099 commented Mar 23, 2023 •

edited

Loading

github-actions bot commented Mar 23, 2023 •

edited

Loading

fryorcraken left a comment •

edited

Loading

weboko Mar 27, 2023

danisharora099 Apr 11, 2023 •

edited

Loading

fryorcraken commented Mar 28, 2023

danisharora099 commented Mar 28, 2023

fryorcraken left a comment

fryorcraken left a comment

fryorcraken Mar 29, 2023

jm-clius Mar 29, 2023

fryorcraken left a comment

danisharora099 commented Mar 29, 2023 •

edited

Loading

fryorcraken left a comment

fryorcraken Apr 3, 2023

danisharora099 Apr 3, 2023

weboko Apr 4, 2023

danisharora099 Apr 6, 2023 •

edited

Loading

weboko Apr 11, 2023

danisharora099 Apr 12, 2023

weboko left a comment


		import debug from "debug";

		const log = debug("waku:test:run");

feat: use nwaku docker instead of building binaries #1259

feat: use nwaku docker instead of building binaries #1259

Conversation

danisharora099 commented Mar 23, 2023 • edited Loading

Problem

Solution

TODO:

Notes

github-actions bot commented Mar 23, 2023 • edited Loading

size-limit report 📦

fryorcraken left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danisharora099 Apr 11, 2023 • edited Loading

Choose a reason for hiding this comment

fryorcraken commented Mar 28, 2023

danisharora099 commented Mar 28, 2023

fryorcraken left a comment

Choose a reason for hiding this comment

fryorcraken left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fryorcraken left a comment

Choose a reason for hiding this comment

danisharora099 commented Mar 29, 2023 • edited Loading

fryorcraken 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

danisharora099 Apr 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weboko left a comment

Choose a reason for hiding this comment

danisharora099 commented Mar 23, 2023 •

edited

Loading

github-actions bot commented Mar 23, 2023 •

edited

Loading

fryorcraken left a comment •

edited

Loading

danisharora099 Apr 11, 2023 •

edited

Loading

danisharora099 commented Mar 29, 2023 •

edited

Loading

danisharora099 Apr 6, 2023 •

edited

Loading