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

feat: Zinnia Runtime and Peer Checker module #92

Merged
merged 29 commits into from
May 4, 2023
Merged

Conversation

bajtos
Copy link
Member

@bajtos bajtos commented Apr 12, 2023

  • feat: install Zinnia runtime

  • feat: install mod-peer-checker

  • feat: run peer checker module via Zinnia

  • feat: switch base Docker image to full node:18

    We cannot use node:18-alpine because Zinnia does not support that yet.

    I tried to use node:18-slim, but Saturn L2 was not able to start in that system.
    The Node.js project recommends to use node:18, so we should be fine. See https://github.com/nodejs/docker-node#image-variants

  • test: move mocha config to config file and add --exit

  • test: make test/station.js easier to troubleshoot

  • test: fix expected Saturn messages

See CheckerNetwork/zinnia#75

TODO

Out of Scope

  • Access logs of individual Zinnia modules. It's tricky! I'll create a follow-up issue for that.

  • Documentation for Core developers

    • how to update Zinnia Runtime to a new version
    • how to update a Zinnia Module to a new version
    • how to add a new Zinnia Module to Station Core

bajtos added 3 commits April 12, 2023 13:43
Add another post-install step to download `zinniad` for running
Zinnia modules.

Signed-off-by: Miroslav Bajtoš <[email protected]>
Signed-off-by: Miroslav Bajtoš <[email protected]>
Signed-off-by: Miroslav Bajtoš <[email protected]>
@bajtos bajtos changed the title Integrate Zinnia and Peer Checker module feat: Zinnia Runtime and Peer Checker module Apr 12, 2023
bajtos added 2 commits April 12, 2023 15:04
Signed-off-by: Miroslav Bajtoš <[email protected]>
Signed-off-by: Miroslav Bajtoš <[email protected]>
lib/zinnia.js Outdated Show resolved Hide resolved
Copy link
Member

@juliangruber juliangruber left a comment

Choose a reason for hiding this comment

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

over to me 😏

bin/station.js Outdated Show resolved Hide resolved
scripts/post-install.js Outdated Show resolved Hide resolved
commands/station.js Outdated Show resolved Hide resolved
juliangruber added a commit that referenced this pull request Apr 17, 2023
juliangruber added a commit that referenced this pull request Apr 19, 2023
scripts/post-install.js Outdated Show resolved Hide resolved
@bajtos
Copy link
Member Author

bajtos commented Apr 25, 2023

It seems the Ubuntu Docker images are coming in minimal flavour by default, so hopefully there won't be too much bloat introduced by switching from Alpine to Debian 🤞🏻

https://canonical.com/blog/minimal-ubuntu-released

Minimal Ubuntu is a set of Ubuntu images designed for automated deployment at scale and made available across a range of cloud substrates. They use the optimised kernels and optimised boot process on their target compute substrate. These images have a greatly reduced default package set, without many convenience tools for interactive usage. They are much smaller, boot faster, and will require fewer security updates over time since they have fewer packages installed.

https://hub.docker.com/_/ubuntu

Given that it is a minimal install of Ubuntu, this image only includes the C, C.UTF-8, and POSIX locales by default.

@juliangruber
Copy link
Member

Any idea what may be wrong and/or how to troubleshoot?

I'll debug it on my Windows machine 👍 Basically for anything Windows specific you can defer that to me, as the team budget bought my Thinkpad.

@juliangruber
Copy link
Member

And if you need to sit at the machine, because it's more complicated etc, I'm happy to start the machine and give you remote access

@juliangruber
Copy link
Member

.zip extraction previously wasn't required on windows, so it didn't yet have logic for attaching the .exe extension. Should be fixed 👍

bin/station.js Outdated Show resolved Hide resolved
lib/zinnia.js Outdated Show resolved Hide resolved
@bajtos
Copy link
Member Author

bajtos commented May 3, 2023

.zip extraction previously wasn't required on windows, so it didn't yet have logic for attaching the .exe extension. Should be fixed 👍

Perfect, thank you for stepping in and fixing the issue!

Windows tests are passing on most versions, but there is a timeout on Node.js 16.

https://github.com/filecoin-station/core/actions/runs/4869576016/jobs/8684249782?pr=92#step:7:72

  1) Activity
       outputs activity:
     Error: Timeout of 15000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (D:\a\core\core\test\activity.js)
      at listOnTimeout (node:internal/timers:559:17)
      at processTimers (node:internal/timers:502:7)

  2) Activity
       can be read while station is running:
     Error: Timeout of 15000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (D:\a\core\core\test\activity.js)
      at listOnTimeout (node:internal/timers:559:17)
      at processTimers (node:internal/timers:502:7)

I'll re-run the test to see if this is intermittent. It would be nice to make our tests more robust, but I'd rather leave that out of this PR.

@juliangruber
Copy link
Member

Agreed, mostly I just re-run these but a better fix would be great

lib/zinnia.js Show resolved Hide resolved
lib/zinnia.js Show resolved Hide resolved
@bajtos bajtos requested a review from juliangruber May 3, 2023 15:25
lib/zinnia.js Show resolved Hide resolved
Signed-off-by: Miroslav Bajtoš <[email protected]>
@bajtos
Copy link
Member Author

bajtos commented May 4, 2023

@juliangruber I added three more commits to make the tests more robust, easier to troubleshoot and less prone to timing errors. PTAL at the changes and also let me know if I should extract those three commits into a standalone PR.

@bajtos bajtos requested a review from juliangruber May 4, 2023 07:25
@bajtos bajtos enabled auto-merge (squash) May 4, 2023 11:34
@juliangruber juliangruber disabled auto-merge May 4, 2023 12:16
@juliangruber juliangruber merged commit 53d4912 into main May 4, 2023
@juliangruber juliangruber deleted the feat/zinnia branch May 4, 2023 12:16
@juliangruber
Copy link
Member

One of my commits wasn't signed, which paused auto merging. Sorry about that!

@juliangruber
Copy link
Member

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.

2 participants