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: Shift workflows to local build, cleanup pipelines #2717

Merged
merged 25 commits into from
Nov 24, 2023
Merged

Conversation

sieniven
Copy link
Member

@sieniven sieniven commented Nov 23, 2023

Summary

This PR is the first part of the CI workflow cleanup.

  • Refactor workflows to implement matrices for build strategies when running multi-arch builds for our build workflows in dev, staging and release. Cleans up and removes unnecessary / repeated code blocks.
  • Shift all workflows to build defi binaries locally. Rationale for this is that the second part of the CI workflow cleanups will implement cpp caching into all of our build workflows to speed up CI runtime
  • All test workflows to use pre-warmed container (ain-builder)
  • Upgrade checkout action version to v4 in all workflows
  • Implement rust caching on build dev, staging and release workflows
  • Switch building defi docker images to pull built binaries into docker image.
  • Fix rust caching on frontier tests, causing incorrect cache rust builds when using pre-warmed ain-builder container.
  • Shift signing of package into make.sh

Implications

  • Storage

    • Database reindex required
    • Database reindex optional
    • Database reindex not required
    • None
  • Consensus

    • Network upgrade required
    • Includes backward compatible changes
    • Includes consensus workarounds
    • Includes consensus refactors
    • None

@sieniven sieniven marked this pull request as ready for review November 24, 2023 06:18
@Bushstar Bushstar merged commit cd29ac3 into master Nov 24, 2023
39 of 67 checks passed
@Bushstar Bushstar deleted the niven/ci-cleanup branch November 24, 2023 08:55

WORKDIR /app
COPY ${PACKAGE} ./
RUN tar -xvzf ${PACKAGE} --strip-components 1
Copy link
Member

Choose a reason for hiding this comment

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

This puts both on the image, the tar as well extracted versions, perhaps add another build state to have it clean?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in PR #2721

@@ -25,7 +25,7 @@ git_check_in_repo() {
DESC=""
SUFFIX=""
CURRENT_BRANCH=""
if [ -z "$BUILD_VERSION" ] && [ "${BITCOIN_GENBUILD_NO_GIT}" != "1" ] && [ -e "$(command -v git)" ] && [ "$(git rev-parse --is-inside-work-tree 2>/dev/null)" = "true" ] && git_check_in_repo contrib/genbuild.sh; then
if [ "${BITCOIN_GENBUILD_NO_GIT}" != "1" ] && [ -e "$(command -v git)" ] && [ "$(git rev-parse --is-inside-work-tree 2>/dev/null)" = "true" ] && git_check_in_repo contrib/genbuild.sh; then
Copy link
Member

Choose a reason for hiding this comment

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

Is there a specific reason to remove this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Skipping the dirty tag check when BUILD_VERSION is specified is incorrect in our previous docker build workflow because BUILD_VERSION was not passed on into the docker image itself which resulted in the defid version still being tagged as dirty. This was removed and reverted because with CI on local builds inside the runner, build should be clean and there is no need to explicitly skip the check.

In addition, BUILD_VERSION specified in CI will vary depending on if it is a tagged commit. Thus on build-release workflow, it should be passed directly as BUILD_DESC, while on merge to master, BUILD_VERSION should be defined as BUILD_SUFFIX. It would be easier to simply revert back to the old genbuild pipeline, which will resolve the incorrect dirty tag once we shift our build locally.

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.

3 participants