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

_ci_ Build docker containers automatically for butterflynet, calibnet, and debug #9625

Merged
merged 7 commits into from
Nov 29, 2022

Conversation

ianconsolata
Copy link
Contributor

@ianconsolata ianconsolata commented Nov 10, 2022

Related Issues

Proposed Changes

This is a major refactor of our dockerfile to support the following

  • The lotus image will remain as is.
  • The lotus-test image will be deprecated.
  • The lotus-all-in-one image will also ship with the lotus-seed and lotus-fountain binaries, which it currently does not.
  • The lotus-all-in-one image will be built in debug, calibnet, and butterflynet modes in addition to the (current) mainnet mode.
  • The lotus-all-in-one image will now be published regularly using the following tags:
    • 1.18.0-rc1 , 1.18.0-rc1-debug, 1.18.0-rc1-calibnet, 1.18.0-rc1-butterflynet . This pattern will be used for all lotus releases, including RC releases.
    • nightly, nightly-debug, nightly-calibnet, nightly-butterflynet
    • stable, stable-debug, stable-calibnet, stable-butterflynet

I left the existing Dockerfile.lotus unchanged for backwards compatibility, but we can probably safely remove it after this is merged and released.

This also removes:

  • old publish to dockerhub job
  • publishing to ECR (a private amazon container registry like dockerhub that no one seems to be using anymore).

Checklist

Before you mark the PR ready for review, please make sure that:

  • Commits have a clear commit message.
  • PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
    • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
  • New features have usage guidelines and / or documentation updates in
  • Tests exist for new functionality or change in behavior
  • CI is green

This is a major refactor of our dockerfile to support the following
- The lotus image will remain as is.
- The lotus-test image will be deprecated.
- The lotus-all-in-one image will also ship with the lotus-seed and lotus-fountain binaries, which it currently does not.
- The lotus-all-in-one image will be built in debug, calibnet, and butterflynet modes in addition to the (current) mainnet mode.
- The lotus-all-in-one image will now be published regularly using the following tags:
  - 1.18.0-rc1 , 1.18.0-rc1-debug, 1.18.0-rc1-calibnet, 1.18.0-rc1-butterflynet . This pattern will be used for all lotus releases, including RC releases.
  - nightly, nightly-debug, nightly-calibnet, nightly-butterflynet
  - stable, stable-debug, stable-calibnet, stable-butterflynet
@ianconsolata ianconsolata requested a review from a team as a code owner November 10, 2022 15:12
@ianconsolata ianconsolata changed the title _ci_ Btrfly calib all in one _ci_ Build docker containers automatically for butterflynet, calibnet, and debug Nov 10, 2022
Copy link
Contributor

@geoff-vball geoff-vball left a comment

Choose a reason for hiding this comment

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

@ianconsolata Looks good, just needs a make gen

Copy link
Contributor

@magik6k magik6k left a comment

Choose a reason for hiding this comment

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

2 Nits, but generally looks good.

(also pushed a make-gen fix)

Dockerfile Outdated

ENV FILECOIN_PARAMETER_CACHE /var/tmp/filecoin-proof-parameters
ENV LOTUS_PATH /var/lib/lotus
ENV DOCKER_LOTUS_IMPORT_SNAPSHOT https://fil-chain-snapshots-fallback.s3.amazonaws.com/mainnet/minimal_finality_stateroots_latest.car
Copy link
Contributor

Choose a reason for hiding this comment

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

We should swap this to the new snapshots - https://snapshots.${network}.filops.net/minimal/latest, e.g. https://snapshots.mainnet.filops.net/minimal/latest

(not sure how it's used under the hood, if the url needs to be resolved first, that can be done with - curl -ILs -w %{url_effective} -o /dev/null "https://snapshots.mainnet.filops.net/minimal/latest")

The new snapshots have a slightly more robust setup, and come from cloudflare R2, instead of S3, and are usually much faster to fetch (I can fetch them at 2Gbps in EU, which is all my internet can do, fetching from S3 is 10x slower)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, right now we don't really support building the lotus image with anything but mainnet, so I will make this default to the mainnet string, and then whoever is running the container can supply a different string if they built it for that network.

I could also extend this image to support supplying a network parameter, instead of the currently method which relies on the docker build command to supply GOFLAGS to build a current image, but that's more work and I think this method is just as configurable.

Dockerfile Outdated
@@ -0,0 +1,133 @@
#####################################
FROM golang:1.18.1-buster AS lotus-builder
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably want a newer Go version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1.18.1 is what we currently build all our builds with (see https://github.com/filecoin-project/lotus/blob/master/.circleci/config.yml#L9 and https://github.com/filecoin-project/lotus/blob/master/GO_VERSION_MIN).

If we want a newer version, we should update that across the board, and probably in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Discussed in standup: @ianconsolata can you please update to 1.18.8, and enforce it in the Makefile (in a separate PR)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here it is: #9712

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once that one is merged, I will update this PR to include the changes.

@ianconsolata ianconsolata mentioned this pull request Nov 14, 2022
5 tasks
@ianconsolata ianconsolata force-pushed the btrfly-calib-all-in-one branch from 545c14d to 17a130d Compare November 29, 2022 14:50
@ianconsolata
Copy link
Contributor Author

#9712 is merged, and I brought those changes over to this PR, so I think this is good to go too now?

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.

4 participants