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

docker: Add docker to the release pipeline #50

Merged
merged 3 commits into from
Apr 19, 2023

Conversation

winder
Copy link
Contributor

@winder winder commented Mar 31, 2023

Summary

Add a docker multi-arch build + deployment configuration.

For now the docker hub deployment is disabled. Hyperflow has added credentials to the repo so that the container can be deployed.

Goreleaser provides binaries from the build step to the Dockerfile to ensure containers have the same files as the archives.

Test Plan

Tested with goreleaser release --skip-publish --snapshot --clean

The files are here:

~$ docker images|grep conduit
algorand/conduit   1.0.1-next-amd64   0863fc121046   About a minute ago   96.9MB
algorand/conduit   latest-amd64       0863fc121046   About a minute ago   96.9MB
algorand/conduit   1.0.1-next-arm64   907990ee4310   About a minute ago   90.5MB
algorand/conduit   latest-arm64       907990ee4310   About a minute ago   90.5MB

The architecture appears as expected:

~$ docker image inspect algorand/conduit:latest-arm64 | grep Architecture
        "Architecture": "arm64",
~$ docker image inspect algorand/conduit:latest-amd64 | grep Architecture
        "Architecture": "amd64",

@winder winder requested a review from a team March 31, 2023 15:49
@winder winder self-assigned this Mar 31, 2023
Copy link
Contributor

@algochoi algochoi left a comment

Choose a reason for hiding this comment

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

Looking good to me + Docker image builds successfully on my local machine.

I can't seem to run it locally due to exec: "./conduit": permission denied: unknown errors.

@tzaffi
Copy link
Contributor

tzaffi commented Mar 31, 2023

Looking good to me + Docker image builds successfully on my local machine.

I can't seem to run it locally due to exec: "./conduit": permission denied: unknown errors.

cd cmd/conduit and then run the command

I was playing around with possibly giving more informative suggestions by looking at the command's exec path, the current working directory, and computing a rel-path. But then I convinced myself that it wasn't worth it.

@winder
Copy link
Contributor Author

winder commented Mar 31, 2023

This Dockerfile needs goreleaser to build it, it does something special which puts the precompiled binaries at the root of the filesystem. I'm not exactly sure what the unknown permission denied error is, but I'm sure it's related.

The benefit is you know the artifacts in the tar files are identical to the ones in the docker image. The downside is you can't build it normally.

Can you think of a way to help people avoid trying to build it manually?

Copy link
Contributor

@algochoi algochoi left a comment

Choose a reason for hiding this comment

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

goreleaser release --skip-publish --snapshot --clean (instead of docker build) did fix my issue, LGTM

@algochoi algochoi requested a review from tzaffi March 31, 2023 20:26
@codecov
Copy link

codecov bot commented Apr 4, 2023

Codecov Report

Merging #50 (661902c) into master (442791a) will decrease coverage by 1.43%.
The diff coverage is n/a.

❗ Current head 661902c differs from pull request most recent head 33a0eb8. Consider uploading reports for the commit 33a0eb8 to get more accurate results

@@            Coverage Diff             @@
##           master      #50      +/-   ##
==========================================
- Coverage   67.66%   66.23%   -1.43%     
==========================================
  Files          32       33       +1     
  Lines        1976     1869     -107     
==========================================
- Hits         1337     1238      -99     
+ Misses        570      566       -4     
+ Partials       69       65       -4     

see 13 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@tzaffi tzaffi left a comment

Choose a reason for hiding this comment

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

LGTM but I echo @algochoi 's observation that --clean is prefered over the now deprecated --rm-dist

@winder winder merged commit f654b12 into algorand:master Apr 19, 2023
@winder winder deleted the will/docker branch April 19, 2023 19:30
@winder winder restored the will/docker branch April 19, 2023 19:42
@winder winder deleted the will/docker branch April 19, 2023 19:42
tzaffi pushed a commit that referenced this pull request May 8, 2023
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