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 Dockerfile to be called from docker build #176

Closed
wants to merge 1 commit into from
Closed

Conversation

lucaslorentz
Copy link
Owner

How to use it before merging this PR:

docker build --build-arg "PLUGINS=github.com/caddyserver/nginx-adapter github.com/hairyhenderson/caddyprom" -t your-tag-name https://github.com/lucaslorentz/caddy-docker-proxy.git#builder

After merging this PR, we can drop the #builder (branch).

@flaviostutz
What do you think about this as as fix for #175?

PS: I still need to udpate the README file.

@francislavoie
Copy link
Collaborator

francislavoie commented Aug 10, 2020

FWIW we probably plan to move the official docker image to use xcaddy instead eventually. We might leave the caddy-builder script in for backwards compatibility though. /cc @hairyhenderson

xcaddy has several advantages - can build from scratch against any commit of Caddy (i.e. build from master if you want), and it's the same script used by the build server backing the download page

@lucaslorentz
Copy link
Owner Author

@francislavoie
That will be a nice improvement.

@hairyhenderson
Do you think this could be done in the official caddy docker repo?

Create a Dockerfile in the root of https://github.com/caddyserver/caddy-docker, which accepts the plugins and caddy version as args. Like this

That way it would be even simpler to build a custom image, just one command:

docker build --build-arg "PLUGINS=plugin1 plugin2" https://github.com/caddyserver/caddy-docker

@flaviostutz
Copy link

I think this is a good resolution for #175.

@hairyhenderson
Copy link

On the topic of moving to xcaddy in the official image, I just logged caddyserver/caddy-docker#107

As for a docker build that uses args, that reminds me of what Grafana supports: https://grafana.com/docs/grafana/latest/installation/docker/#build-and-run-a-docker-image-with-pre-installed-plugins - they also have the option of installing them at runtime which I've used, but is a little awkward.

Personally I prefer just using a straight Dockerfile, because then there's no ambiguity - the build is self-contained and reproducible, whereas with build-args you would need to save the command somewhere to make sure you have the right mix of plugins. And if you're going to save a shell script with a docker build command, why not just save a Dockerfile?

@lucaslorentz
Copy link
Owner Author

@hairyhenderson
Exactly! I saw it the first time in grafana, I couldn't remember that anymore.
There is nothing more appealing than short and simple instructions in docs, that's probably why they adopted that approach.

Dockerfile is simple, but it is undeniably one more step for users, while they could just point to a remote Dockerfile.

I will add the Dockerfile to this repo.

If Caddy officially adopts this way as well in the future I will be happy to point users to official Caddy instructions.

@hairyhenderson
Copy link

Dockerfile is simple, but it is undeniably one more step for users, while they could just point to a remote Dockerfile.

IMO it's the same number of steps: either users must commit a build script to their source control, or they must commit a Dockerfile. Though I suppose the former only requires running a shell script, whereas the latter does require knowing to run a docker build command. 🤔

Personally I've been teaching people to write Dockerfiles instead of shell scripts due to the improvements in portability and reproducibility, but there's nothing wrong with providing alternate methods.

I'll consider the build-args approach for the caddy-docker repo, but it is going to be a bit more complex than simply pointing at https://github.com/caddyserver/caddy-docker, because we need to be able to support multiple different Dockerfiles for different versions of Caddy. Plus it also means that users of this method would not be using the official caddy:builder image, but instead rebuilding it themselves.

@lucaslorentz
Copy link
Owner Author

@hairyhenderson
Have you seen the Dockerfile I created in this PR?
https://github.com/lucaslorentz/caddy-docker-proxy/blob/b6d2b7e3486d96252d61f41fc69e985ba426e950/Dockerfile

I did versioning as just another argument and it still uses caddy:builder.
I basically copied the example from official caddy docker docs and added 2 args + a default plugin.
It is working fine by just running:

docker build --build-arg "PLUGINS=github.com/hairyhenderson/caddyprom" https://github.com/lucaslorentz/caddy-docker-proxy.git#builder

@lucaslorentz
Copy link
Owner Author

I would suggest maintaining only one Dockerfile like that.
And add more arguments for architecture/os, that way people can use windows images as well, and so on.

@hairyhenderson
Copy link

Hmm! Thanks @lucaslorentz - I hadn't actually read the Dockerfile that closely (still on vacation so not paying enough attention 😅). It is actually pretty straightforward.

I think the only blocker for supporting it in the caddy-docker repo is that repo is focused on the official image, and official images have some restrictions, one of which is that build-args are not allowed (due to requirements on reproducibility). I've opened caddyserver/caddy-docker#108 to continue that discussion, though.

@francislavoie
Copy link
Collaborator

francislavoie commented Oct 8, 2020

I think this can now be updated to use xcaddy instead of caddy-builder.

Comment on lines +7 to +9
RUN caddy-builder \
github.com/lucaslorentz/caddy-docker-proxy/plugin/v2 \
${PLUGINS}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
RUN caddy-builder \
github.com/lucaslorentz/caddy-docker-proxy/plugin/v2 \
${PLUGINS}
RUN xcaddy build \
--with github.com/lucaslorentz/caddy-docker-proxy/plugin/v2 \
${PLUGINS}

Copy link
Collaborator

Choose a reason for hiding this comment

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

The ${PLUGINS} part might need to be revised to run a shell command to prefix each one with --with to be compatible with xcaddy.

@@ -1,19 +1,13 @@
FROM alpine:3.11 as alpine
RUN apk add -U --no-cache ca-certificates
ARG CADDY_VERSION=2.1.1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ARG CADDY_VERSION=2.1.1
ARG CADDY_VERSION=2.4.0

COPY --from=builder /usr/bin/caddy /usr/bin/caddy
Copy link
Collaborator

Choose a reason for hiding this comment

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

It'll still be necessary to override the CMD

Suggested change
COPY --from=builder /usr/bin/caddy /usr/bin/caddy
COPY --from=builder /usr/bin/caddy /usr/bin/caddy
CMD ["caddy", "docker-proxy"]

@lucaslorentz
Copy link
Owner Author

lucaslorentz commented May 13, 2021

Thanks @francislavoie
But I will abandon this PR, my recommendation is to follow Caddy Docs and use official caddy images.

@lucaslorentz lucaslorentz deleted the builder branch May 13, 2021 14:10
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.

None yet

4 participants