-
Notifications
You must be signed in to change notification settings - Fork 4
Add Dockerfile for portable builds and deployments #270
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: main
Are you sure you want to change the base?
Conversation
I confirm that my contribution is made under the terms of the Apache 2.0 license. |
1b43483
to
ec31c55
Compare
I have this As usual, GitHub Actions' |
7b48875
to
66fa6f0
Compare
Updated GitHub Actions run from latest code: https://github.com/mcinglis/fusion-java/actions/runs/15590360669 |
66fa6f0
to
4ba27aa
Compare
OK, fixed the issue of New full build/publish on my fork running at: https://github.com/mcinglis/fusion-java/actions/runs/15599567790 👀 |
4ba27aa
to
217a306
Compare
Reworked the https://github.com/mcinglis/fusion-java/actions/runs/15602461476 👀 |
36fbe8b
to
e86fad8
Compare
I separated the image build and push, to have a basic intermediary test step for each of the SDK and runtime images. Running atop this latest code at: https://github.com/mcinglis/fusion-java/actions/runs/15614047323 Per last night's run, the SDK and runtime builds are (mostly) not sharing the built layers, still. I don't yet know why. The Apparently GitHub now supports arm64 runners for OSS: https://github.blog/changelog/2025-01-16-linux-arm64-hosted-runners-now-available-for-free-in-public-repositories-public-preview/ . So, it's probably worth looking at how to leverage those here to achieve faster multi-platform image builds. That will require more tomfoolery in conjoining the Actions jobs though, but it should be possible. Reviewers - please let me know your thoughts.
Both efforts should bring down the build times considerably - but it will also mean additional complexity and code in this PR. Happy to work to merge this roughly as-is, or also happy to keep iterating in those directions on this PR. |
OK, so the runtime test is failing. I'm going to pursue those changes suggested ^. The Alternatively if folks would rather pull out the GHA stuff from this PR, fine to do that too, and follow up with a dedicated PR for that. |
I think it's reasonable to pare this PR down to just getting the working Dockerfile, then do the Gradle build and GHA as 2 more subsequent PRs. Should help separate concerns and make it easier on the review side. |
e86fad8
to
ce042b3
Compare
Alright, I've significantly pared down the GHA workflow to just build the images - it builds each base image, across GitHub's Here's a run on my fork: https://github.com/mcinglis/fusion-java/actions/runs/15647643354 So, I'll follow up with another PR or two to improve this further - bring into Gradle (if I can do that nicely), and update the workflow to test and publish the images to GHCR. |
ce042b3
to
d90689a
Compare
(just removed the mention of the for-now-not-available |
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.
I am aligned with discussion here and with this incremental progress. I left some minor comments/questions.
One question though: with respect to pulling more of the build logic out of the GH actions config, what's the reasoning behind using a gradle task rather than a shell script?
fusion/src/howto_build.md
Outdated
docker build -t fusion . | ||
alias fusion='docker run --rm -it fusion' | ||
fusion help |
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.
Do you think it'd be worth labeling this specifically as a "quick start" or enumerating and linking the various possibilities for building/running described later in the file so that it's clear that you don't have to have docker?
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.
Hmm, maybe. This file isn't that long though - https://github.com/mcinglis/fusion-java/blob/mcinglis/main/fusion/src/howto_build.md on my 1920x1080 display is ~three screens long. https://docs.ion-fusion.dev/latest/howto_build.html will be even shorter (currently, without styling).
I also didn't like hardcoding docker
up-front here (without the preamble later, that all of this should work with any container CLI). But, I felt it would be valuable to give folks a quick start set of commands. However, because the file isn't that long, it's basically just repeating the commands shown immediately later anyway (e.g. the git clone
repetition ...).
So, I've replaced this with just a list of jump links to each section, so folks can get a quicker sense of what's available here. LMK what you think. Happy to revert back to this, or adjust further, or even just edit/tune prior to merging per taste.
Since @SharkBaitDLS has reviewed this as well I'd like to get his check before merging as well. |
* Add a `Dockerfile` for building the Ion Fusion SDK and CLI, across a variety of base environments, with multi-stage image builds to support in-development vs runtime container deployment contexts. * Add a new GitHub Actions workflow for building the container images across all base environments, on GitHub's `ubuntu-latest` and `ubuntu-latest-arm` runners; doing it this way is *so much* faster than using the docker/build-push-action's `platforms` facility. * Rewrite most of `howto_build.md` documentation to provide a guide to building Fusion via the Dockerfile, in various contexts.
d90689a
to
c27cb83
Compare
Thanks for the great feedback @rationull ! Agreed, happy to wait for others' OKs here too, also @toddjonker if he can get time/space. No rush at all - it's given me time to keep improving the overall proposal here. 🙏 I've pushed some changes per your review (adjusting the
|
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.
Thanks Malcolm! The changes LGTM.
Noting it's a bit easier to review incremental progress with multiple commits added to the PR and then squashed at the end. I'm still rough in my github habits though so I'm curious if there's a reason to amend previous commits instead.
Hrm, CI failure in the rhel-openjdk-8 build, with a bunch of errors like
Of course that code didn't change, and this build passed on the previous rev. Maybe related to upgrading the source versions for some reason? |
Ack, apologies for the difficulty - I'm also adjusting/learning per the project's preferences. I'll move to pushing sequential commits in my changes, rather than force-pushing. My reasoning for amending prior commits is that it keeps the PR's holistic proposal of "this is what I think we should merge to For reviewing changes across pushes, are the Compare links sufficient? Or, rather, in what ways do they fall short of reviewing progressive commits? |
I'm not really speaking for anyone else, I don't think we have an official project preference on this. Compare links are probably sufficient and I'm just not used to them enough yet :) Technically better anyway b/c you need them if you want to view differences across multiple commits. Also 100% agree on default message behavior with Github's squashes. |
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.
Ah, thanks for fixing the locale issue!
This LGTM as well. I should've been clearer before that I'm not railing against all uses of .dockerignore
especially for keeping local builds faster by avoiding a bunch of extra context. I am partial to copying files explicitly as you're doing here but please use your judgment on the best balance; I'm not trying to dictate the result here.
GHA run looks good on my fork: https://github.com/mcinglis/fusion-java/actions/runs/15655998456 @rationull : ack, thanks. Then, unless others disagree, I'll move back to pushing/amending the single proposed commit (or in rare PRs which are better communicated as a sequencing of commits) on any subsequent changes. I'll leave it as-is with two commits for now, though - if/once others have reviewed & approved, probably better to just ensure this second commit doesn't get awkwardly thrown onto the bottom of the squashed commit message. |
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.
Just realized that I started a review on Friday but didn't send it; here's the comments I had at that point. (I realize more changes have come through.)
I'll also note that...
- I'm not terribly worried about GHA times, though that's under the assumption that said time is free.
- I'm not terribly worried about the cross-OS matrix, since this project is 99% Java. I don't expect the build to be any different across platforms, so AFAIC we could just build once on whatever is the easiest.
- I'm more interested in having the tests that ensure things work when running-on later JDKs. And maybe run-on different architectures though our exposure is (AFAIK) limited to the launcher shell script.
labels: | | ||
org.opencontainers.image.title=Ion Fusion SDK (${{ matrix.tag }}) | ||
org.opencontainers.image.description=Ion Fusion SDK container image (${{ matrix.tag }}) built on ${{ matrix.base }} |
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.
I'm guessing this metadata will make its way through the container registry. Is there a way to tag these as experimental
or unstable
or anything?
|
||
env: | ||
REGISTRY: ghcr.io | ||
IMAGE_NAME: ${{ github.repository }} |
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 one's a head-scratcher, the LHS and RHS seem unrelated.
tags: ${{ steps.meta.outputs.tags }} | ||
|
||
|
||
# Build job for the Ion Fusion runtime images: |
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.
I can't say that I understand what this job accomplishes... what is a "runtime image"?
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.
I later figured out that this image contains the CLI.
@@ -0,0 +1,166 @@ | |||
name: push-container-images |
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.
I propose that we approach CI/CD configuration with the expectation that the typical project contributor will not have a good understanding of how things work. The implication is that there should be more commentary/docs/explanation/warnings/etc than we'd generally expect in, say, Java or Fusion code, where its reasonable to expect a higher baseline competency.
docker run --rm --entrypoint sh fusion-sdk -c ' | ||
echo "public class Example { | ||
public static void main(String[] args) { | ||
dev.ionfusion.fusion.cli.Cli.main(args); |
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.
[nitpick?] This (...fusion.cli.*
) isn't a public API.
Ideally, we'd have examples embedded in the SDK.
Dockerfile
Outdated
# Ion Fusion runtime image | ||
# ------------------------ | ||
|
||
FROM ${BASE_JRE} | ||
COPY --from=sdk \ | ||
/usr/local/bin \ | ||
/usr/local/lib \ | ||
/usr/local/ | ||
ENTRYPOINT ["fusion"] | ||
|
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.
IIUC this image is effectively just the fusion
CLI, so it may be better called something like "Ion Fusion CLI image". Note that "Fusion Runtime" has a specific technical meaning that's more low-level than what "Java Runtime" means: you can run multiple independent FusionRuntime
s inside the same JVM instance.
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.
Context for some comments: this project has adopted the Diátaxis framework for documentation, which gives us fairly concrete guidance on what content should go where.
My overall request is to break this down into more focused parts.
The most valuable part, strategically, is having a container-based fast-track onboarding experience for customers to try out the CLI. In other words, the parts supporting the tutorial_cli
content.
Beyond that, let's be intentional about working backwards from specific outcomes. This is where I'm still feeling lost WRT the SDK image (and, TBH, the current SDK itself).
javac -cp "$cp" Example.java | ||
java -cp ".:$cp" Example version' | ||
|
||
### Varying the OpenJDK |
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.
The material from here down opens up beyond what feels right for a How-to Guide, and turns into Reference material. Or perhaps it belongs in a separate guide, because it seems like it's addressing a separate goal.
|
||
[Amazon Corretto]: https://aws.amazon.com/corretto | ||
[Git]: https://git-scm.com/ | ||
### Runtime image |
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.
Per Diátaxis, this section is arguably the beginnings of a separate "How to run the Ion Fusion CLI" guide.
The SDK image supports building applications that integrate Ion Fusion: | ||
|
||
docker run --rm --entrypoint sh fusion-sdk -c ' | ||
echo "public class Example { | ||
public static void main(String[] args) { |
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.
Ditto on changing goals, here were into "How to integrate Ion Fusion into your application" territory.
But I gotta say, from that perspective this example is confusing, because it's not really how one would want to go about things.
@@ -12,7 +12,7 @@ Here we'll walk through some basic use cases with Ion Fusion's command-line inte | |||
> * Download the [Ion Fusion SDK][SDK], unpack it somewhere, and add its | |||
> `bin` directory to your `PATH`. | |||
> | |||
> Alternatively, you can [build the CLI from source](howto_build.html). | |||
> Alternatively, you can [build Ion Fusion from source](howto_build.html). |
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.
I think the most valuable part of the Docker proposal is what's needed to let folks perform the following steps using an OCI image (and its included JRE), rather than using the raw SDK (and bring-your-own JRE).
I should confess that the current "SDK" was not designed intentionally; it was simply what I got while hacking together Gradle logic to ensure it could build all the main components. I'm not sure that it's actually of any value in its current state, especially now that the documentation is online. And it'll be even less valuable as soon as we start publishing to Maven. |
Thanks for the review and comments Todd! I'm feeling like the PR's current state is pretty far off from what would be mergeable, then - given the desire to minimize support & testing for deploying Fusion across varying environments and JDK/JREs (ideally one, not many, as you write). This current proposal, by contrast, is predicated on a desire to reasonably maximize Fusion's support and testing for varying environments. The current Dockerfile ensures that Fusion is runnable and deployable (and thus CI'd ontop of) across a wide variety of OpenJDK distributions - including their JDKs and JREs. JREs are conventionally used for runtime deployments of Java apps, as they're smaller; fusion's SDK image (w/ JDK) here is 392MB, vs the runtime image (w/ JRE) is 339MB. Well-heard on the Diataxis documentation approach (love that!); agreed the current A Dockerfile targeting just a single environment and output would be a lot simpler than here - and would also lead to different outcomes, and documentation thereof. To take this PR further, I'd have to align on the philosophy of Fusion's desired build/runtime environment variability; if we want Fusion to be portable (and thus tested and packaged/able across other environments), or not. I believe it should be portable: and systematically so, not just via intentions. However, above all, I don't want to push for changes that aren't aligned with the philosophy and legibility of Fusion's core folks. 🙇 I'd also necessarily need more time & interest to dedicate to this PR, which is admittedly waning here - and anyway will be more difficult for me to find over the next few weeks. As I conveyed in person, you absolutely should push back on things you aren't keen to support, and/or that you're not seeing the value in. If this proposal is in that bucket, that's totally fine. I'm OK to close this PR, or also if yall would like to take it as a basis for a more aligned approach to containerizing Fusion, very welcome to do so. Cheers! |
Hi Malcolm, I'm sure you're frustrated by the apparent contention over your proposals here. I apologize for that; my feedback is likely muddled by my not having a good mental model for everything that's happening. As a team, we should've encouraged you right from the start to decompose this into separate concerns, so the conversation could be more focused. I created #272 to add relevant guidance to the To clarify, I don't "desire to minimize support & testing for deploying Fusion across varying environments", quite the opposite. But I read this PR as running builds across environments, whereas I'd rather build exactly once and then test those artifacts across dimensions, which better emulates what will happen when we're publishing to Maven. In any case, I respect the need to time-box things. I'll pick up your improvements to the extant how-to-build content so we can get it merged independently of the Docker parts. I'll also create some issues covering (my interpretation of) those other parts, in the hopes that'll help build alignment on the desired functionality. Likely most of this will make it through once the goals are clarified! Thank you for pushing in this direction. It's good to be stretched into unfamiliar territory. |
Add a
Dockerfile
for building the Ion Fusion SDK and CLI, across avariety of base environments, with multi-stage image builds to support
in-development vs runtime container deployment contexts.
Add a new GitHub Actions workflow for building the container images
across all base environments, on GitHub's
ubuntu-latest
andubuntu-latest-arm
runners; doing it this way is so much faster thanusing the docker/build-push-action's
platforms
facility.Rewrite most of
howto_build.md
documentation to provide a guide tobuilding Fusion via the Dockerfile, in various contexts.