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

Update github action CI workflow #3199

Merged
merged 37 commits into from
Jun 6, 2023
Merged

Update github action CI workflow #3199

merged 37 commits into from
Jun 6, 2023

Conversation

wileyj
Copy link
Contributor

@wileyj wileyj commented Jul 15, 2022

Updates the CI workflow

Applicable issues

Additional info

Lots of proposed changes here so I'll do my best to articulate them:

  • migrates the existing ci.yml workflow into a more modular approach with semi-reusable workflow files
  • updates all Debian builds to use bullseye (latest debian)
  • adds arm64 binaries/docker image
  • update all Dockerfiles to use ramdisk for building
  • create checksum of binary artifacts for release
  • build all release images from binaries (PR's and branch builds still build from source)
  • create checksum of binary artifacts for release
  • adds clippy and crate advisory actions

The workflow behaves similarly to how the existing workflow runs - manual execution with a tag will create a new release. on PR's, a simplified workflow is run.

The ./build-scripts Dockerfiles are all updated to be resuable but also renamed to reflect how the arches appear in the build process, i.e. linux-glibc-arm64.

Clippy has been added, and right now it's set to run on PR's to develop - as of now this action is quite messy. Opinions here are welcome, it may be desired to remove entirely, or perhaps run it on a cron so the data is available. Using defaults, it's very pedantic but it can be adjusted to ignore trivial lint complaints.

A crate advisory action is also added (I believe it's something that should remain in some form to ensure we're keeping up with dependency updates) - currently this action is to be scheduled nightly at 0330 UTC, and will create a new issue if any crate has a listed advisory.

Last, the build process is changed slightly:

  • sha1sum of all archives is stored in the file CHECKSUMS.txt which is included in the github release
  • All release images are built using the binary archives for the specified architecture
  • ./github/actions/dockerhub/** is created to build the new images for both Alpine and Debian

Caveats

  • the linux-musl-arm64 build is using an external musl-cross build image (for now), no other way to natively build for this arch. has been found (https://musl.cc/)
  • bitcoin-tests.yml is set to run only when PR's are opened to the master branch (this is debatable, but in testing i felt it was running too often)
  • source build will trigger on a push to the default branch automatically
  • naming of the tags is slightly adjusted from something like x.y.z-stretch -> x.y.z-debian

Examples of builds

@wileyj wileyj added enhancement Iterations on existing features or infrastructure. deployment labels Jul 15, 2022
@wileyj
Copy link
Contributor Author

wileyj commented Jul 15, 2022

One more decision I made in this PR (that I did not use for testing) - the jobs all use the current model like this:

    uses: stacks-network/stacks-blockchain/.github/workflows/stacks-blockchain-tests.yml@master

so i left it as-is. however, this does make it hard to make changes to the ci workflow since a push to master is required to get changes into github actions.

during testing, i was using the local path to the action, ex:

    uses: ./.github/workflows/stacks-blockchain-tests.yml

Both methods have their pros/cons - I felt it was important to note that here in this PR.

Copy link
Member

@CharlieC3 CharlieC3 left a comment

Choose a reason for hiding this comment

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

Amazing work! It's already polished, but I left some comments for minor changes, questions, and alternative ideas in some places if interested.

.github/workflows/ci.yml Show resolved Hide resolved
.github/workflows/build-source-binary.yml Outdated Show resolved Hide resolved
.github/workflows/github-release.yml Show resolved Hide resolved
.github/workflows/image-build-alpine-binary.yml Outdated Show resolved Hide resolved
.github/workflows/image-build-debian-binary.yml Outdated Show resolved Hide resolved
.github/workflows/image-build-debian-source.yml Outdated Show resolved Hide resolved
.github/workflows/image-build-alpine-binary.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/actions/dockerhub/Dockerfile.debian-source Outdated Show resolved Hide resolved
Copy link
Member

@kantai kantai left a comment

Choose a reason for hiding this comment

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

These changes look good to me -- I'll defer to Charlie and Jude for final approvals.

My only request is that you update the README.md (or add a RELEASING.md) with two instructions:

  1. How to tag a release (I think this is already kind of mentioned in the release checklist, but it could have it's own subsubsection in the README), and what happens automatically when the release is tagged (docker images pushed? binaries included in the git release? changelog?)
  2. How to trigger a "feature" build: if I have a PR, and I want to publish a docker image with a specific tag (not a version number) that doesn't create a new release, how can I do that? I think before these changes, that was done "automatically" on every feat/... PR, but with these changes, I think it requires a manual trigger, right?

@wileyj wileyj mentioned this pull request Jul 20, 2022
5 tasks
@wileyj
Copy link
Contributor Author

wileyj commented Aug 15, 2022

  • How to tag a release (I think this is already kind of mentioned in the release checklist, but it could have it's own subsubsection in the README), and what happens automatically when the release is tagged (docker images pushed? binaries included in the git release? changelog?)
  • How to trigger a "feature" build: if I have a PR, and I want to publish a docker image with a specific tag (not a version number) that doesn't create a new release, how can I do that? I think before these changes, that was done "automatically" on every feat/... PR, but with these changes, I think it requires a manual trigger, right?

sorry, i missed this comment when i was addressing other changes. I'll check and reply later, optionally with a commit to address the questions

@wileyj
Copy link
Contributor Author

wileyj commented Aug 17, 2022

These changes look good to me -- I'll defer to Charlie and Jude for final approvals.

My only request is that you update the README.md (or add a RELEASING.md) with two instructions:

  1. How to tag a release (I think this is already kind of mentioned in the release checklist, but it could have it's own subsubsection in the README), and what happens automatically when the release is tagged (docker images pushed? binaries included in the git release? changelog?)
  2. How to trigger a "feature" build: if I have a PR, and I want to publish a docker image with a specific tag (not a version number) that doesn't create a new release, how can I do that? I think before these changes, that was done "automatically" on every feat/... PR, but with these changes, I think it requires a manual trigger, right?
  1. Releases are only tagged if a tag is manually provided when the action is triggered -> this PR doesn't deviate from the current process. Updating the README.md might be better suited in Simplify repo README.md #3196 and Simplify repo README #3247
  2. Pushing a new feature branch -> nothing is triggered automatically. PR's are required (same process as current), or the ci workflow can be triggered manually on a specific branch to build assets.

PR a branch to develop:

  • rust format is run (this happens on every run)
  • docker image built from source on a debian distribution is built and pushed with the branch name and PR number as tags
    • blockstack/stacks-blockchain:feat-112-fix-something
    • blockstack/stacks-blockchain:pr-112

Merging a PR to develop:

  • rust format is run (this happens on every run)
  • docker image built from source on a debian distribution is built and pushed with the branch name and PR number as tags
    • blockstack/stacks-blockchain:develop
    • blockstack/stacks-blockchain:pr-113

Merging a PR develop to master:

  • rust format is run (this happens on every run)
  • docker image built from source on a debian distribution is built and pushed with the branch name as a tag
    • blockstack/stacks-blockchain:master

Manually triggering workflow without tag (any branch):

  • rust format is run (this happens on every run)
  • no binaries are built
  • no github release
  • no integration tests
  • docker image built from source on a debian distribution is built and pushed with the branch name as a tag
    • blockstack/stacks-blockchain:<branch name>

Manually triggering workflow with tag (non-default branch):

  • rust format is run (this happens on every run)
  • no binaries are built
  • no github release
  • no integration tests
  • docker image built from source on a debian distribution is built and pushed with the branch name
    • blockstack/stacks-blockchain:<branch name>

Manually triggering workflow with tag on default branch (i.e. tag of 2.05.0.3.0):

  • rust format is run (this happens on every run)
  • integration tests
  • leaked credential test
  • binaries built for specified architectures
    • archives and checksum added to github release
  • github release (with artifacts/checksum) is created using the manual input tag
  • docker image built from source on a debian distribution is built and pushed with the provided nput tag and latest
    • blockstack/stacks-blockchain:2.05.0.3.0-debian
    • blockstack/stacks-blockchain:latest-debian
    • blockstack/stacks-blockchain:2.05.0.3.0
    • blockstack/stacks-blockchain:latest

TL;DR

Creating a release doesn't deviate from the current process. Because the builds for a release will now use a binary vs building from source for each arch, the debian-based source build docker image is not created when a new release is tagged.
a release tag will produce 2 versions of the docker image (along with all binary archives):

  1. Alpine image tagged with the x.x.x.x.x and latest
  2. Debian image tagged with x.x.x.x.x-debian and latest-debian
    The debian-based docker image is created when feature branches etc are PR'ed (or manually triggered via the CI workflow, choosing the appropriate branch), resulting in a docker image being pushed with a tag like feat-fix-1234

clippy workflow is being added, but it's currently disabled until we can tame it a bit so it's not overly verbose.
audit workflow is being added, but it's currently disabled. this workflow is not triggered unless some conditions are met (Cargo.toml/Cargo.lock are changed); it is also configured to run on a schedule and create issues for crate advisories. I've disabled it for now so we can evaluate if it's worth keeping (i feel that it is, but in the short-term it may add extra noise to the open issues).

@kantai
Copy link
Member

kantai commented Aug 17, 2022

Can you add that info either to the README.md or a new RELEASING.md file? That way, we'll have a place to look if we want to figure it out down the line.

@wileyj
Copy link
Contributor Author

wileyj commented Aug 17, 2022

Can you add that info either to the README.md or a new RELEASING.md file? That way, we'll have a place to look if we want to figure it out down the line.

Can do - will add a new file and bump the issue dealing with the readme updates

@wileyj
Copy link
Contributor Author

wileyj commented Aug 17, 2022

Can you add that info either to the README.md or a new RELEASING.md file? That way, we'll have a place to look if we want to figure it out down the line.

wileyj@97ae563

@wileyj
Copy link
Contributor Author

wileyj commented Aug 18, 2022

@CharlieC3 i think there's one more change i have to make here that i missed in my tests.
when a release candidate is created, it should create all binaries/release/images and currently it does not - only realized this as i'm working on the current 2.05.0.3.0 release and watching the CI process

@CharlieC3
Copy link
Member

CharlieC3 commented Aug 19, 2022

Merging a PR to develop:
rust format is run (this happens on every run)
docker image built from source on a debian distribution is built and pushed with the branch name and PR number as tags
blockstack/stacks-blockchain:develop
blockstack/stacks-blockchain:pr-113

Why would merging a PR to develop (effectively making a commit to develop and closing a PR) render a new PR Docker tag? I would think this operation should only create a tag where the new commit/merge was made (e.g. blockstack/stacks-blockchain:develop)

Manually triggering workflow with tag (non-default branch):
rust format is run (this happens on every run)
no binaries are built
no github release
no integration tests
docker image built from source on a debian distribution is built and pushed with the branch name
blockstack/stacks-blockchain:

Sometimes we create release-candidate Github tags off the develop branch. With this change, I'm not sure we'll be able to do that anymore. Is that intended?

@wileyj
Copy link
Contributor Author

wileyj commented Aug 19, 2022

Merging a PR to develop:
rust format is run (this happens on every run)
docker image built from source on a debian distribution is built and pushed with the branch name and PR number as tags
blockstack/stacks-blockchain:develop
blockstack/stacks-blockchain:pr-113

Why would merging a PR to develop (effectively making a commit to develop and closing a PR) render a new PR Docker tag? I would think this operation should only create a tag where the new commit/merge was made (e.g. blockstack/stacks-blockchain:develop)

Manually triggering workflow with tag (non-default branch):
rust format is run (this happens on every run)
no binaries are built
no github release
no integration tests
docker image built from source on a debian distribution is built and pushed with the branch name
blockstack/stacks-blockchain:

Sometimes we create release-candidate Github tags off the develop branch. With this change, I'm not sure we'll be able to do that anymore. Is that intended?

there are a few typos in the comment i made updated here: https://github.com/wileyj/stacks-blockchain/blob/feat/update-ci/RELEASING.md

however, your second point is correct and only something i noticed myself the other day (#3199 (comment)
) . it needs to be fixed so i'm prepping a change so that still works as exected (currently, trying to cut a release from a non-default branch wouldn't actually create the release, just the debian image).

Once i get those changes committed, it will also update the RELEASING.md file.

I'll ping you again when I think it's ready, it was a use case i wasn't thinking about when i sent this PR.

@wileyj
Copy link
Contributor Author

wileyj commented Mar 8, 2023

related - #3559

@pavitthrap pavitthrap mentioned this pull request Mar 8, 2023
testing the locally built packages/docker images is successful, this reverts those changes need for testing the resulting artifacts
@wileyj wileyj changed the base branch from develop to master March 9, 2023 19:22
@wileyj wileyj marked this pull request as ready for review March 9, 2023 19:31
@wileyj
Copy link
Contributor Author

wileyj commented Mar 9, 2023

These changes look good to me -- I'll defer to Charlie and Jude for final approvals.

My only request is that you update the README.md (or add a RELEASING.md) with two instructions:

  1. How to tag a release (I think this is already kind of mentioned in the release checklist, but it could have it's own subsubsection in the README), and what happens automatically when the release is tagged (docker images pushed? binaries included in the git release? changelog?)
  2. How to trigger a "feature" build: if I have a PR, and I want to publish a docker image with a specific tag (not a version number) that doesn't create a new release, how can I do that? I think before these changes, that was done "automatically" on every feat/... PR, but with these changes, I think it requires a manual trigger, right?

moved this to ./docs/ci-release.md in line with recent changes to the repo.

use current settings from master branch
@wileyj
Copy link
Contributor Author

wileyj commented Mar 9, 2023

Since we are seeing these codecov errors very frequently, can we just change this setting to false in the various places it is used? At least until we come up with a better solution for code coverage.

      fail_ci_if_error: true

done, matching what's currently in the master branch

Copy link
Member

@CharlieC3 CharlieC3 left a comment

Choose a reason for hiding this comment

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

I scanned through all the changes made and comments addressed since my last review, it looks great. There's a lot of changes being made here though, so catching any logical or functional errors is not easy and very time-consuming. Assuming it all works as described in the ci-release doc, I think we're good to go! :shipit:

However, we may need to be prepared to make some quick followup adjustments after merging if we find something important is not quite right with the new processes.

@pavitthrap pavitthrap added CI and removed deployment labels Apr 3, 2023
@pavitthrap pavitthrap added the frozen PRs that are on hold label Apr 25, 2023
@pavitthrap pavitthrap removed the frozen PRs that are on hold label May 23, 2023
@wileyj wileyj marked this pull request as draft June 2, 2023 18:34
* master: (108 commits)
  chore: add sip-024 text
  Chore - adding 2.4.0.0.0 to changelog
  sanitize-gate the depth check
  add epoch.truncate
  add comment to affirmation module explaining change
  update mainnet 2.4 height with sip-024 value
  fix: affirmation calc should skip any ops whose parent is <= first_block_height
  address PR feedback
  address PR feedback, enable sanitization in from-consensus-buff
  chore: improve comments
  fix: allow change of delegation data
  added more test coverage for epoch 2.4
  more sanitization unit cases
  fix pox_3 auto_unlock tests
  feat: implement proposed SIP-024 sanitization logic for epoch-2.4
  chore: update testnet 2.4 activation
  fixed test, added to gh workflow
  added epoch2.4 test
  chore: PR feedback (pox_3_first_cycle off-by-one)
  Revert "chore: PR feedback (pox_3_first_cycle off-by-one)"
  ...
@wileyj wileyj marked this pull request as ready for review June 6, 2023 17:03
@wileyj wileyj merged commit 8a7a371 into stacks-network:master Jun 6, 2023
fess-v pushed a commit to fess-v/stacks-blockchain that referenced this pull request Sep 10, 2023
* Updating Github Action

- build all release images from binaries
- create checksum of binary artifacts for release
- adds clippy and crate advisory actions
- update all dockerfiles to use ramdisk for building
- separate actions to relevant files
- adds arm64 binaries/docker image
- update all debian builds to use bullseye (latest debian)

* only run btc int tests on default branch

* enable btc int test on develop

* Feat/update ci (stacks-network#38)

* Updating Github Action

- build all release images from binaries
- create checksum of binary artifacts for release
- adds clippy and crate advisory actions
- update all dockerfiles to use ramdisk for building
- separate actions to relevant files
- adds arm64 binaries/docker image
- update all debian builds to use bullseye (latest debian)

* only run btc int tests on default branch

* final action test

disabled a lot of the long-running tests

* Update ci.yml

* Update ci.yml

* Update bitcoin-tests.yml

* Update bitcoin-tests.yml

* run on push to master (merge)

* Update Github Actions

- build all release images from binaries
- create checksum of binary artifacts for release
- adds clippy and crate advisory actions
- update all dockerfiles to use ramdisk for building
- separate actions to relevant files
- adds arm64 binaries/docker image
- update all debian builds to use bullseye (latest debian)

* adding  build features to dockerfiles

* update repo org to stacks-network

missed a ref to wileyj forked repo

* addressing comments in pr 3199

see stacks-network#3199 for changes requested

* cleaning up docker tags

prevent overwriting of docker image branch tags

* disabling audit workflow

disabling this workflow until we can test further

* Adding a release file

* Update to trigger logic

Updating the logic of how/when builds and releases happen based on comments in PR.
Updated the RELEASING.md file to reflect these changes

* chore: delete circle.yml

CircleCI hasn't been used in 11+ months.

Fixes stacks-network#3072

* switch repo&owner to var

remove hardcoded value in favor or `GITHUB_REPOSITORY`

* use local workflows

* fix: don't assume that the bitcoin node always gives a non-zero number of headers

* fix: add unit test and change log entry

* fix: Exclude benchmarks from compilation

* fix: Resolve conflicts and the remaining two errors

* clean: remove benchmark files

* fix: use explicit version number

* minor update to reconcile diffs

since some files were renamed, some minor changes had to be made manually here, i.e. fail_ci_if_error: false

* hardcode some vals for testing

* revert

* use org/repo locations for jobs

testing the locally built packages/docker images is successful, this reverts those changes need for testing the resulting artifacts

* Moving file to docs dir

* continue build if unit-tests fail

use current settings from master branch

* 3199 - minor updates for recent upstream changes

---------

Co-authored-by: Diwaker Gupta <[email protected]>
Co-authored-by: Jude Nelson <[email protected]>
Co-authored-by: Stjepan Golemac <[email protected]>
fess-v pushed a commit to fess-v/stacks-blockchain that referenced this pull request Sep 10, 2023
* Updating Github Action

- build all release images from binaries
- create checksum of binary artifacts for release
- adds clippy and crate advisory actions
- update all dockerfiles to use ramdisk for building
- separate actions to relevant files
- adds arm64 binaries/docker image
- update all debian builds to use bullseye (latest debian)

* only run btc int tests on default branch

* enable btc int test on develop

* Feat/update ci (stacks-network#38)

* Updating Github Action

- build all release images from binaries
- create checksum of binary artifacts for release
- adds clippy and crate advisory actions
- update all dockerfiles to use ramdisk for building
- separate actions to relevant files
- adds arm64 binaries/docker image
- update all debian builds to use bullseye (latest debian)

* only run btc int tests on default branch

* final action test

disabled a lot of the long-running tests

* Update ci.yml

* Update ci.yml

* Update bitcoin-tests.yml

* Update bitcoin-tests.yml

* run on push to master (merge)

* Update Github Actions

- build all release images from binaries
- create checksum of binary artifacts for release
- adds clippy and crate advisory actions
- update all dockerfiles to use ramdisk for building
- separate actions to relevant files
- adds arm64 binaries/docker image
- update all debian builds to use bullseye (latest debian)

* adding  build features to dockerfiles

* update repo org to stacks-network

missed a ref to wileyj forked repo

* addressing comments in pr 3199

see stacks-network#3199 for changes requested

* cleaning up docker tags

prevent overwriting of docker image branch tags

* disabling audit workflow

disabling this workflow until we can test further

* Adding a release file

* Update to trigger logic

Updating the logic of how/when builds and releases happen based on comments in PR.
Updated the RELEASING.md file to reflect these changes

* chore: delete circle.yml

CircleCI hasn't been used in 11+ months.

Fixes stacks-network#3072

* switch repo&owner to var

remove hardcoded value in favor or `GITHUB_REPOSITORY`

* use local workflows

* fix: don't assume that the bitcoin node always gives a non-zero number of headers

* fix: add unit test and change log entry

* fix: Exclude benchmarks from compilation

* fix: Resolve conflicts and the remaining two errors

* clean: remove benchmark files

* fix: use explicit version number

* minor update to reconcile diffs

since some files were renamed, some minor changes had to be made manually here, i.e. fail_ci_if_error: false

* hardcode some vals for testing

* revert

* use org/repo locations for jobs

testing the locally built packages/docker images is successful, this reverts those changes need for testing the resulting artifacts

* Moving file to docs dir

* continue build if unit-tests fail

use current settings from master branch

* 3199 - minor updates for recent upstream changes

---------

Co-authored-by: Diwaker Gupta <[email protected]>
Co-authored-by: Jude Nelson <[email protected]>
Co-authored-by: Stjepan Golemac <[email protected]>
@blockstack-devops
Copy link
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@stacks-network stacks-network locked as resolved and limited conversation to collaborators Nov 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI enhancement Iterations on existing features or infrastructure. locked
Projects
Status: Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

arm64 docker image tag overwrites amd64 tag