-
Notifications
You must be signed in to change notification settings - Fork 37
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
Dockerize bindle #343
Dockerize bindle #343
Conversation
Hi @vdice, I add |
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 looks great @FrankYang0529.
As added context for other reviewers (@thomastaylor312 has been most active as of late), @FrankYang0529 has a PR up to both scale out an installation of Fermyon on AWS as well as add persistence for various components (transcending VM/host state, etc).
@FrankYang0529 had already gone down the path of an EBS-backed volume for the postgres DB that Hippo uses, which runs as a container and which utilizes a Nomad-supported CSI approach to tying it all together. Then we realized, a quick win to get persistence for Bindle would be to containerize this service as well -- and then use the same aforementioned plumbing (EBS/CSI). Seeing as Bindle doesn't yet have another option for persistence outside of a host's filesystem, this seemed the most prudent approach for our needs at this time.
We imagine this containerization approach might prove generally useful to others as well.
I tested this PR and looks good. The real win is seeing a stopped/restarted container re-use the bindle data from the mounted volume, as we'd expect.
For this PR, I had the following thoughts:
Ideally added in this PR:
- a doc/readme somewhere with the first section from the testing notes above, showing how to build and run the bindle server in its containerized form
Potential follow-up(s):
- CI/CD: perhaps some steps to build the image on PRs and some steps to publish the image somewhere eg to this repo's GitHub Container Registry?. Could publish an image tagged with
latest
on merges to main and immutable release versions on any git tag/GitHub release of this project
However, before going down this CI/CD path, I'd want to make sure we get sign-off from at least one other maintainer around the additions in this PR and the potential vision for build/publish in the near future.
I think we may want to ensure that the docs example can run successfully with this image at HEAD as well. Or at least by the next tagged release -- so we'll want to create issue(s) around the current failing behavior so that they can be fixed. |
|
Test for Using Bindle example with master branch: Build and setup bindle-server
Sign and push invoice
|
e72438b
to
b69770a
Compare
Hi @vdice, it looks like we can't use the latest bindle with Spin. Bindle rejects un-signed invoices after this change #300. I will focus on fermyon/spin#689 to fix this issue first. |
@FrankYang0529 Yeah that is why the 0.9.0 release is still in an RC. We were giving everyone time to update to the required signing. |
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 agree with @vdice here that we should probably have a followup PR with some additional documentation about building the docker image (or at least a make docker
target) and a CI/CD pipeline for building and pushing this image somewhere
Dockerfile
Outdated
ENV BINDLE_IP_ADDRESS_PORT="0.0.0.0:8080" | ||
ENV BINDLE_DIRECTORY="/bindle-data/bindles" | ||
|
||
COPY --from=builder /app/target/release/bindle-server / |
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.
One small nit: We might want to create and run as a bindle
user instead of root
as that is generally good for hygiene. But that is not blocking
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.
Updated it. Thank you.
@vdice leaving final approval up to you. If you'd like to see a |
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 for adding this @FrankYang0529! Prior to merging, my two requests are:
- A docs mention on the ability to run bindle-server in a container: it could be as simple as a brief mention in the README somewhere around the 'Using Bindle' section. Since the flow that already exists applies whether or not the bindle server process is running in a container, perhaps with a few updates (eg
bindle keys fetch
) we should be able to add a note along the lines of 'Note that the server can also run as a container...' Or we could add a whole new section/flow for the containerized approach; either way. - As @thomastaylor312 suggested, make target(s) for running the server in a container would be nice. I see that the other serve* targets have options for embedded and/or tls. I'm not sure the most ideal way to do this... use existing targets but have a
USE_DOCKER
env var option? Or have docker variants for all the serve* targets? WDYT?
Signed-off-by: Frank Yang <yangpoan@gmail.com>
b69770a
to
fae013f
Compare
Signed-off-by: Frank Yang <yangpoan@gmail.com>
fae013f
to
4ce8959
Compare
Hi @vdice, I updated |
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.
Looks great! Thank you @FrankYang0529
ref: fermyon/installer#100
It looks like Using Bindle example can't work on the current master branch, so I use#343 (comment)v0.8.0
for testing.Start Bindle
git reset HEAD~1
to keepDockerfile
.git checkout v0.8.0
to check out tov0.8.0
.docker build -t bindle .
.BINDLE_TEMP=$(mktemp -d)
.docker run --name bindle -d --restart=unless-stopped -e RUST_LOG=debug -v $BINDLE_TEMP:/bindle-data -p 8080:8080 bindle
.Start consul & nomad
consul agent --dev
nomad agent --dev
Start Hippo
admin
and passwordp@ssword
.Run Spin
make build
.spin new http-rust myapp
.myapp
folder, runspin build
.spin deploy
.