-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat: Multi-platform rippled image #6049
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
base: develop
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #6049 +/- ##
=========================================
- Coverage 78.6% 78.5% -0.0%
=========================================
Files 817 818 +1
Lines 68976 68981 +5
Branches 8242 8265 +23
=========================================
- Hits 54196 54170 -26
- Misses 14780 14811 +31 🚀 New features to boost your workflow:
|
| EOF | ||
|
|
||
| COPY --from=build /opt/ripple/bin/ /opt/ripple/bin/ | ||
| COPY --from=build /opt/ripple/etc/ /opt/ripple/etc/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of doing all of the above, let's make build-image depend on the build, add needed targets to the matrix, and needed artifacts and in reusable-build-image.yml just download these artifacts and here put them in the right place
The reason is that when you build in 2 different places, you actually end up with 2 different binaries, and we want to avoid this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and we want to avoid this
Why do we want to avoid this?
@bthomee Suggested building the CI built binary into the image while also providing a Dockerfile with the build but I'd rather image the binary than have 2 Dockerfiles.
Let's go even further with a multi-stage image build where we have a container action obtain/build the dependencies and another that builds rippled (and why not, another to install it.) Something like that would actually address all of our concerns, be extremely explicit and copy-paste-able for anyone to use. That's for another day.
This does the thing it purports to do today and is needed yesterday.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello, this is Keshava from the DGE team. We use the ripple docker images in our integration test pipelines for the client libraries.
Personally, I find it more readable if I can find the HEAD commit associated with a docker build. Recently, we have had to investigate several CI failures in the client libraries CI runners due to updates to the amendments' status.
Instead of doing all of the above, let's make build-image depend on the build, add needed targets to the matrix
I feel adding the workflow dependency makes it harder to associate the docker image with a commit-hash, given that the dependency graph is large for the rippled build process.
Let's go even further with a multi-stage image build where we have a container action obtain/build the dependencies and another that builds rippled
Something like this is definitely more informative ^^ For example, if I can see what amendments were retired/enabled/removed in a rippled-docker image, that will save me a lot of time.
| with: | ||
| images: ${{ env.lowercase_repo }} | ||
| labels: | | ||
| org.opencontainers.image.authors=For inquiries, please use https://github.com/${{ github.repository }}/issues |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't look like authors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copypasta from XRPLF/ci where we leaned into "freeform."
org.opencontainers.image.authors
contact details of the people or organization responsible for the image (freeform string)
| - name: Create manifest list and push | ||
| working-directory: /tmp/digests | ||
| run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's quite a long piece of code, let's make it a script?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This step uses output from the docker/metadata-action@v5 step is tightly coupled to this implementation. What utility would a separate script provide?
It could be generalized and not use docker/metadata-action@v5 and it was previously done like this but this is here now and how all of the other workflows we've written in GitHub work.
Co-authored-by: Ayaz Salikhov <[email protected]>
Co-authored-by: Ayaz Salikhov <[email protected]>
High Level Overview of Change
Builds and pushes
rippledmulti-platform Docker images to GHCR.Context of Change
We need to start building and testing canonical multi-platform images ASAP.
Type of Change
Future Tasks
Use CMake
configure_file()such that the bare minimum build ofrippledgets populated to theBUILD.mddocument and the Dockerfile so there is only one place to update it.Build and push the voidstar images from here so any feature/branch can easily be tested.
These images should also be tagged by the release version so this will need to be manually updated until that procedure is integrated into CI workflows but for now this will be good for
developimages.