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

Overhaul releasing process #779

Merged
merged 27 commits into from
Mar 8, 2021
Merged

Overhaul releasing process #779

merged 27 commits into from
Mar 8, 2021

Conversation

TriplEight
Copy link
Contributor

@TriplEight TriplEight commented Mar 1, 2021

-> Due to the long image builds we have to use the self-hosted runner.
-> A self-hosted runner can't buildah
Have to update to the newer version of the deprecated official docker action.
This is, in fact, not bad. Now the action uses BuildX.
-> BuildX has an ugly bug

Overhaul of publishing

  • remove multistage build as it is a CI recursion
  • upgrade to ubuntu:20.04 base
  • CI: move to use buildah actions
  • chore

Dockerfile Outdated Show resolved Hide resolved
@HCastano HCastano marked this pull request as draft March 1, 2021 19:28
@HCastano HCastano changed the title wip: CI: move to buildah to build images Move to buildah to build images Mar 1, 2021
@TriplEight TriplEight self-assigned this Mar 1, 2021
@TriplEight TriplEight requested a review from rcny March 1, 2021 19:41
@TriplEight TriplEight changed the title Move to buildah to build images Migrate to the new docker action Mar 1, 2021
@TriplEight TriplEight changed the title Migrate to the new docker action Migrate to the new docker action version Mar 1, 2021
@TriplEight TriplEight changed the title Migrate to the new docker action version WIP: Migrate to the new docker action version Mar 1, 2021
@TriplEight
Copy link
Contributor Author

TriplEight commented Mar 2, 2021

Got these images building sending them to the public runners with buildah action.

Caching is not helpful if it gets removed every day, invalidated every day with the nightly rust update, executes every time on a fresh VM, etc. Also, there was a lot of duplicated processes and esp when launched 4 times in parallel it doesn't make anything faster. Besides, --package and other flags are not yet working in cargo-chef.

Still, they take a surprisingly long time to build (the last image gets built in 46min), normally it's 3.5 min for each without any cache.

I'd suggest an optimization - produce the binaries separately (it's faster and probs you'll want the binaries somewhere anyway) in CI and on the images stage just put them in the containers.

@TriplEight
Copy link
Contributor Author

Oh btw I've updated the images and now they run off ubuntu:20.04 please check if they are working properly.

@TriplEight
Copy link
Contributor Author

The next step is I'll steal your CI dockerfile to build it nightly with the other images in #paritytech/scripts.

@TriplEight
Copy link
Contributor Author

TriplEight commented Mar 2, 2021

Regarding

Besides, --package and other flags are not yet working in cargo-chef.

I lied, just found that they've addressed my issue.

@TriplEight
Copy link
Contributor Author

please check if these images work correctly before merging:
paritytech/millau-bridge-node:ubuntu
paritytech/ethereum-poa-relay:ubuntu
paritytech/rialto-bridge-node:ubuntu
paritytech/substrate-relay:ubuntu
paritytech/bridge-dependencies:latest

@TriplEight TriplEight marked this pull request as ready for review March 5, 2021 09:08
@TriplEight TriplEight changed the title WIP: Migrate to the new docker action version CI: Migrate to the new docker action version Mar 5, 2021
@TriplEight TriplEight changed the title CI: Migrate to the new docker action version CI: Overhaul releasing process Mar 5, 2021
@tomusdrw
Copy link
Contributor

tomusdrw commented Mar 5, 2021

please check if these images work correctly before merging:

We will learn that on our testnet deployment :)

@tomusdrw tomusdrw enabled auto-merge (squash) March 5, 2021 13:03
@HCastano HCastano changed the title CI: Overhaul releasing process Overhaul releasing process Mar 5, 2021
@HCastano HCastano disabled auto-merge March 5, 2021 16:54
Copy link
Contributor

@HCastano HCastano left a comment

Choose a reason for hiding this comment

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

Looks good!

@@ -2,9 +2,10 @@
#
# This image is meant to be used as a building block when building images for
# the various components in the bridge repo, such as nodes and relayers.
FROM ubuntu:xenial
FROM ubuntu:20.04

ENV LAST_DEPS_UPDATE 2020-12-21
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 bump this and the one on line 21.

The idea behind these lines is that we can use them to break the cache and update dependencies if needed.

Dockerfile Outdated
@@ -8,51 +8,33 @@
#
# See the `deployments/README.md` for all the available `PROJECT` values.

# This first stage prepares our dependencies to be built by `cargo-chef`.
FROM rust as planner
FROM docker.io/paritytech/bridge-dependencies as builder
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it not good enough to just do paritytech/bridge-dependencies?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's better to have the full names when building with something that's not docker, we use buildah.

Dockerfile Outdated

ARG PROJECT=ethereum-poa-relay
RUN cargo build --release --verbose -p ${PROJECT}
RUN strip ./target/release/${PROJECT}

# In this final stage we copy over the final binary and do some checks
# to make sure that everything looks good.
FROM ubuntu:xenial as runtime
FROM docker.io/ubuntu:20.04 as runtime
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question here, why do we need to specify docker.io?

Dockerfile Outdated
libssl-dev curl && \
groupadd -g 1000 user && \
useradd -u 1000 -g user -s /bin/sh -m user && \
# apt clean up
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# apt clean up
# apt clean up

echo ::set-output name=DATE::$(date +%d-%m-%Y)

- name: Build image for ${{ matrix.project }}
uses: redhat-actions/buildah-build@v2.2
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you choose buildah over build-push-action?

@HCastano
Copy link
Contributor

HCastano commented Mar 8, 2021

Pinging @TriplEight since I've got some questions here

@HCastano HCastano enabled auto-merge (squash) March 8, 2021 21:16
@HCastano
Copy link
Contributor

HCastano commented Mar 8, 2021

I fixed some of my nitpicks since I want this to get used on tonight's testnet deployment.

@HCastano HCastano merged commit a363ec0 into master Mar 8, 2021
@HCastano HCastano deleted the 3x8_use_buildah branch March 8, 2021 22:07
svyatonik pushed a commit that referenced this pull request Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants