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

Package distribution deb #2153

Draft
wants to merge 8 commits into
base: unstable
Choose a base branch
from

Conversation

realbigsean
Copy link
Member

@realbigsean realbigsean commented Jan 12, 2021

Issue Addressed

#2082

Proposed Changes

I used this Polkadot pull request as a reference for most of this (not the rpm portion): https://github.com/paritytech/polkadot/pull/1676/files

  • the tools to build a .deb package
  • systemd service files for the beacon and validator
  • used a combination of config found in that polkadot PR, as well as in lighthouse-ansible
  • a postinst script that creates separate users for the bn and vc, configures the datadir
  • managing cli flags in /etc/default files

Additional Info

TODOs:

  • Distributing the .deb. I think we could self host it (parity does this), or use something like launchpad.net (geth does this)
  • figure out how to help users transition to using this for updates
  • figure how to make using the account manager minimally painful
  • adding default cli flags in the /etc/default files, maybe adding common flags in comments

Would love some feedback! I'm learning most of this as I go, so anyone with expertise/experience in this area, please chime in.

@realbigsean realbigsean added the work-in-progress PR is a work-in-progress label Jan 12, 2021
@torfbolt
Copy link

Nice work!

  • For distributing I would vote for launchpad, this has the advantage that it's very easy for a user to setup as an additional package source and automatically receive future updates. (basically just apt-add-repository ...)
  • Can we activate this build process for both AMD64 and ARM64?
  • Why do the users have home directories configured? As far as I can see they are not used as data directories.

@realbigsean
Copy link
Member Author

@torfbolt thanks a lot for the feedback!

For distributing I would vote for launchpad, this has the advantage that it's very easy for a user to setup as an additional package source and automatically receive future updates. (basically just apt-add-repository ...)

I am also thinking launchpad is the better option. It would be a little less maintenance on Sigma Prime's end, I think it will make it easier for us to build for more architectures, and it handles PGP key verification, which is nice for users. Unfortunately I've learned uploading to launchpad will take a bit of reworking of this PR. Rather than cargo-deb which generates a .deb, we'll have to use something like debcargo in order to generate a .dsc file. Unfortunately debcargo either requires all of lighthouse's dependencies to be uploaded to crates.io or we have to be able to vendor lighthouse's dependencies. The latter would be easier to accomplish but is blocked on this PR: #1712. I'm going to start looking into that PR and going to mark this one as blocked for now.

Can we activate this build process for both AMD64 and ARM64?

I made an attempt to do this using cargo deb, but it's not quite working. I'm going to hold of on trying again until we make progress towards being able to upload to launchpad. But it's definitely a priority.

Why do the users have home directories configured? As far as I can see they are not used as data directories.

Thanks for pointing this out! This was a remnant of what I was looking at in Polkadot's CI, they seem to use the home directory to store data. But I've updated it now.

bors bot pushed a commit that referenced this pull request Feb 10, 2021
## Issue Addressed

resolves #2129
resolves #2099 
addresses some of #1712
unblocks #2076
unblocks #2153 

## Proposed Changes

- Updates all the dependencies mentioned in #2129, except for web3. They haven't merged their tokio 1.0 update because they are waiting on some dependencies of their own. Since we only use web3 in tests, I think updating it in a separate issue is fine. If they are able to merge soon though, I can update in this PR. 

- Updates `tokio_util` to 0.6.2 and `bytes` to 1.0.1.

- We haven't made a discv5 release since merging tokio 1.0 updates so I'm using a commit rather than release atm. **Edit:** I think we should merge an update of `tokio_util` to 0.6.2 into discv5 before this release because it has panic fixes in `DelayQueue`  --> PR in discv5:  sigp/discv5#58

## Additional Info

tokio 1.0 changes that required some changes in lighthouse:

- `interval.next().await.is_some()` -> `interval.tick().await`
- `sleep` future is now `!Unpin` -> tokio-rs/tokio#3028
- `try_recv` has been temporarily removed from `mpsc` -> tokio-rs/tokio#3350
- stream features have moved to `tokio-stream` and `broadcast::Receiver::into_stream()` has been temporarily removed -> `tokio-rs/tokio#2870
- I've copied over the `BroadcastStream` wrapper from this PR, but can update to use `tokio-stream` once it's merged tokio-rs/tokio#3384

Co-authored-by: realbigsean <seananderson33@gmail.com>
@jmcph4
Copy link
Member

jmcph4 commented Apr 3, 2021

@realbigsean FWIW, your fix in 7a71977 went in after the most recent CI job fell over, so I think this should be passing CI checks now?

Very keen to see .deb packaging for this project!

@realbigsean
Copy link
Member Author

Hey @jmcph4,

I've left this as blocked because although we can now successfully vendor lighthouse dependencies using cargo vendor, this command doesn't work in our CI (#2076 (comment)). But I saw in discord that you were interested in helping us publish all of our crates to crates.io that would be very helpful -- it's likely an easier route to unblocking this issue at this point.

@bachp
Copy link
Contributor

bachp commented Oct 29, 2021

@realbigsean Once #2076 is merged cargo vendorwill be tested in CI.

The PR also contains a workaround that makes cargo vendorwork on Github Actions by setting CARGO_HOME=$(readlink -f $HOME).

@realbigsean
Copy link
Member Author

I do plan on picking this back up! but will need to think about how we want package distribution to work post-merge

@paulhauner
Copy link
Member

will need to think about how we want package distribution to work post-merge

I think we'd still distribute as a stand-alone lighthouse package. Users would need to install lighthouse plus an execution client to get a "full node". Do you have thoughts otherwise?

@realbigsean
Copy link
Member Author

I think we'd still distribute as a stand-alone lighthouse package. Users would need to install lighthouse plus an execution client to get a "full node". Do you have thoughts otherwise?

I'm not sure if it's possible but it'd be nice to do something like check if a compatible version of an execution client is installed. Generally though, yeah I think we should probably just keep the installation simple and leave it up to the user to make sure the execution client is properly configured

@realbigsean realbigsean added the backlog PR is not currently prioritized label Jul 14, 2022
@michaelsproul michaelsproul mentioned this pull request Aug 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog PR is not currently prioritized work-in-progress PR is a work-in-progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants