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

Add basic Docker compatibility #2

Merged
merged 5 commits into from
Aug 22, 2024
Merged

Conversation

ellwoodb
Copy link
Contributor

Hey, I saw your Reddit post on r/homeassistant and was intrigued. I really like the project idea and wanted to contribute.

Many people in the posts comments wanted a Docker image, so I created a Dockerfile to build and run Bifrost on Docker. I'm not sure if there is a way to publish a prebuilt image with GitHub Actions for example. For now, the docker-compose file I added just builds the image from source.

I haven't yet gotten to test the whole program, because I am running my Z2M instance on Home Assistant and Bifrost can't reach it by the Home Assistants IP. Maybe you have an idea?

I was also working on a Home Assistant addon and have gotten it to start, but passing the configuration file to the addon seems to be another story and I wanted to at least get the Docker image out for review.

I hope it helps, feel free to make changes / ask, maybe I can help.

(Sorry if there are any spelling mistakes btw, English isn't my first language - see the commit....)

@ellwoodb
Copy link
Contributor Author

Found out what my issue with connecting to an instance of Z2M, which is running on HA was, I needed to specify the port for the webui in the addon config, which was disabled by default.

Dockerfile Outdated

WORKDIR /app/bifrost

COPY . .
Copy link
Owner

@chrivers chrivers Aug 22, 2024

Choose a reason for hiding this comment

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

cargo build generates a lot of temporary build products, that we don't want in the docker output.

I've found a (slightly complicated) example here:

https://docs.docker.com/language/rust/develop/#run-a-database-in-a-container

I think we could simplify it to something like this:

################################################################################
# Create a stage for building the application.

ARG RUST_VERSION=1.80.1
FROM rust:${RUST_VERSION}-slim-bullseye AS build
WORKDIR /app

RUN --mount=type=bind,source=src,target=src \
    --mount=type=bind,source=Cargo.toml,target=Cargo.toml \
    --mount=type=bind,source=Cargo.lock,target=Cargo.lock \
    <<EOF
set -e
cargo build --locked --release
cp target/release/bifrost /bifrost
EOF


################################################################################
# Build the final docker image

FROM debian:bullseye-slim AS final

# Copy the executable from the "build" stage.
COPY --from=build /bifrost /app/bifrost

# Expose the port that the application listens on.
EXPOSE 80
EXPOSE 443

WORKDIR /app

# What the container should run when it is started.
CMD ["/app/bifrost"]

This also solves another important issue, which is that we need to build in release mode (using --release). Otherwise, the binary will be much bigger and slower :)

Dockerfile Outdated

RUN cargo build

CMD ["cargo", "run"]
Copy link
Owner

Choose a reason for hiding this comment

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

We're missing a newline at the end of the file :)

@chrivers
Copy link
Owner

@ellwoodb hello! And welcome to the project :)

Thank you for your contribution, I appreciate it! I've reviewed your MR, and I think we need to make a few changes.

I've given a few pointers - I hope it's enough to get you going. Otherwise, feel free to ask :)

@ellwoodb
Copy link
Contributor Author

Hi, thanks for the input. Glad you like it.

It was really meant to be just very basic support for all the Redditors demanding it ;)

I'll get going with the changes you mentioned. Hopefully I'll have something ready by tomorrow.

@ellwoodb
Copy link
Contributor Author

@chrivers Just had time to try some stuff. I added the Dockerfile you suggested. It runs nicely. Just pushed the changes.

Two questions:

  1. Is there a specific reason you used Debian Bullseye instead of Bookworm? I tried both and both seemed to work the same.
  2. In the Dockerfile we expose ports 80 and 443 but when I tried that in the docker-compose, the Hue app didn't recognize the bridge anymore, so I left it at network_mode: host. Is there a fix for that? Do we need to expose other ports or maybe in tcp / udp mode?

@ellwoodb
Copy link
Contributor Author

Okay, maybe 2.) is related to #1? I never have had problems exposing ports 80 and 443 with Docker, but maybe that is it, because I'm trying it on my desktop instead of a sever.

@chrivers
Copy link
Owner

Hi again - thanks for keeping with it! :)

Ah yes, the "bullseye" vs "bookworm" was just leftover from the docker example.

Bookworm is preferred, so feel free to upgrade :)

Regarding the EXPOSES - I didn't think it through. Bifrost uses mDNS, which runs on port 5353 and the special IPv4 address 224.0.0.251 (for multicast).

Setting network_mode: host in the docker-compose file is definitely a good way forward. It's known to work with all "special" requirements like this. But it also makes it a little harder to bundle for more constrained settings (such as a home assistant add-on in the future, for example).

I found an article about the problem:

https://medium.com/@andrejtaneski/using-mdns-from-a-docker-container-b516a408a66b

They suggest some modifications, that include mounting the avahi and dbus sockets into the container.

I don't know if this will make bifrost work, but maybe you can play around with it, see if it helps?

If that doesn't work, we should remove both EXPOSE statements for now, and just rely on the host network mode 👍

@ellwoodb
Copy link
Contributor Author

Okay, thanks for the input. I tried adding the mounts for dbus and avahi but it didn't work. I think we'll need to stick to using network_mode: host for now.

I'll maybe be doing some research on other Home Assistant addons that use mdns (I think stuff to do with Apple Airplay need it too). Maybe I can get a solution from there.

@ellwoodb
Copy link
Contributor Author

ellwoodb commented Aug 22, 2024

So, I found two Home Assistant Addons using avahi also using network_mode: host (Shairport-Sync, AirPlay 2)

I think that's just the solution. Not optimal, but practical.

Edit: And in this quick search, I saw, many addons are actually using host network.

@chrivers
Copy link
Owner

Thanks for the updates! It's looking good :)

And I much agree with your description - not optimal, but practical 👍

I'll merge this 🥳

@chrivers chrivers merged commit d66c72d into chrivers:master Aug 22, 2024
1 check passed
@ellwoodb
Copy link
Contributor Author

Nice, glad I could contribute! Should I also write up something for the readme or will you be doing that?

@chrivers
Copy link
Owner

@ellwoodb I'm glad too! Contributions always welcome :)

Feel free to give it a go - I'm thinking something along the lines of:

  1. Change Installation guide to Installation guide [manual]
  2. Add a section below that Installation guide [docker]
  3. Put in a notice in the beginning of Installation guide [manual] that links to the docker install guide below.

Even if the docker method ends up being more popular, I'd like the manual way first, since it also explains the basics of how the program works.

Want to give it a try? :)

@ellwoodb
Copy link
Contributor Author

Nice to hear that. Sure I'll give it a try.

I was thinking about the same thing. So I'll do two sections, leave the manual method you already provide as the first/default one and add a Docker version.

I also thought about explicitly mentioning that the Docker version is not optimal and you should prefer the manual one.

I will do that tomorrow though if that's okay? It's getting late.

@chrivers
Copy link
Owner

Sounds great - any no worries, any time works 👍

@zacs
Copy link

zacs commented Aug 22, 2024

+1 you'll need host mode networking without using a reflector or something, at least in my experience.

Any chance this would work on an Alpine base image? It would be far smaller, but I've never run a Rust project on Alpine so it could be a nonstarter. I can also experiment on my own.

Thanks for adding @ellwoodb !

@ellwoodb
Copy link
Contributor Author

@zacs, I'll try that out if I have time, thanks for the feedback!

@chrivers
Copy link
Owner

Yes, an alpine image can make sense.

I've taken care not to have any external dependencies, even for things like creating a certificate, so bifrost should run on pretty much any docker. (It needs libc, but that's it)

I would say, that a non-alpine image is a little nicer for debugging, testing, etc, but since we volume mount all the important state and config, that's not really an issue.

I'm open for a PR that switches to lighter packaging 👍

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