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.

Sign up for GitHub

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

Jump to bottom

Update docs for Caddy image with a fix to its reload feature #2483

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Update docs for Caddy image with a fix to its reload feature #2483

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5dd7111
Update docs for Caddy image with a fix to its reload feature
BjoernLudwigPTB Aug 24, 2024
0809d9e
Update caddy/content.md to be potentially less confusing
BjoernLudwigPTB Aug 28, 2024
d860ee4
Update caddy/content.md to be potentially less confusing
BjoernLudwigPTB Aug 28, 2024
a750af6
Update caddy/content.md with better wording
BjoernLudwigPTB Aug 28, 2024
61797fd
Update caddy/content.md with short flag for docker compose reload com…
BjoernLudwigPTB Aug 28, 2024
c798339
Update caddy/content.md to get rid of trailing whitespace
BjoernLudwigPTB Aug 28, 2024
c64b526
Update caddy/content.md with one more change completing the intended …
BjoernLudwigPTB Oct 19, 2024
d0add51
Update caddy/content.md with improved wording
BjoernLudwigPTB Oct 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions caddy/content.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,19 @@ $ curl http://localhost/
hello world
```

To override the default [`Caddyfile`](https://github.com/caddyserver/dist/blob/master/config/Caddyfile), you can mount a new one at `/etc/caddy/Caddyfile`:
To override the default [`Caddyfile`](https://github.com/caddyserver/dist/blob/master/config/Caddyfile), you can create one in the subfolder `caddyfile` at `$PWD/caddyfile/Caddyfile` and mount this folder at `/etc/caddy`:
BjoernLudwigPTB marked this conversation as resolved.
Show resolved Hide resolved

```console
$ docker run -d -p 80:80 \
-v $PWD/Caddyfile:/etc/caddy/Caddyfile \
-v $PWD/caddyfile:/etc/caddy \
BjoernLudwigPTB marked this conversation as resolved.
Show resolved Hide resolved
-v caddy_data:/data \
%%IMAGE%%
```

#### ⚠️ Do not mount the Caddyfile directly at `/etc/caddy/Caddyfile`

This effectively disables Caddy's graceful reload feature in many cases depending on the way you apply changes to the file as discussed in [this issue](https://github.com/caddyserver/caddy/issues/5735#issuecomment-1675896585).
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't agree with this wording/framing. This isn't quite true, it only breaks if you happen to be using vim/vi in which case the way it saves files isn't tracked properly by Docker. But if you use a different editor like nano then the issue doesn't happen.

So I would prefer if this warned about making note of which editor you use to edit the file, rather than saying that it "breaks reloading" in general.

Copy link
Author

Choose a reason for hiding this comment

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

Agree, I will come up with a better wording reflecting the actual scope of the circumstances in which there is something to warn about. I submitted this proposal from the perspective of someone having followed the documentation exactly and due to having used vim to edit the file, I ran into the described situation.

Copy link
Author

Choose a reason for hiding this comment

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

@francislavoie I have now suggested a wording that better reflects the situation that is causing the actual problem. This makes the paragraph a little lengthier, but also more precise. What do you think? Would you agree to these changes?


### Automatic TLS with the Caddy image

The default `Caddyfile` only listens to port `80`, and does not set up automatic TLS. However, if you have a domain name for your site, and its A/AAAA DNS records are properly pointed to this machine's public IP, then you can use this command to simply serve a site over HTTPS:
Expand Down Expand Up @@ -119,11 +123,9 @@ See https://github.com/quic-go/quic-go/wiki/UDP-Buffer-Sizes for more details.

### Docker Compose example

If you prefer to use `docker-compose` to run your stack, here's a sample service definition.
If you prefer to use `docker-compose` to run your stack, here's a sample service definition which goes into your `compose.yaml`.
BjoernLudwigPTB marked this conversation as resolved.
Show resolved Hide resolved

```yaml
version: "3.7"

services:
caddy:
image: %%IMAGE%%:<version>
Expand All @@ -135,7 +137,7 @@ services:
- "443:443"
- "443:443/udp"
volumes:
- $PWD/Caddyfile:/etc/caddy/Caddyfile
- $PWD/caddyfile:/etc/caddy
- $PWD/site:/srv
- caddy_data:/data
- caddy_config:/config
Expand All @@ -147,3 +149,5 @@ volumes:
```

Defining the data volume as [`external`](https://docs.docker.com/compose/compose-file/compose-file-v3/#external) makes sure `docker-compose down` does not delete the volume. You may need to create it manually using `docker volume create [project-name]_caddy_data`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Defining the data volume as [`external`](https://docs.docker.com/compose/compose-file/compose-file-v3/#external) makes sure `docker-compose down` does not delete the volume. You may need to create it manually using `docker volume create [project-name]_caddy_data`.
Defining the data volume as [`external`](https://docs.docker.com/compose/compose-file/compose-file-v3/#external) makes sure `docker compose down` does not delete the volume. You may need to create it manually using `docker volume create [project-name]_caddy_data`.

Copy link
Author

Choose a reason for hiding this comment

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

In fact, now that we are talking about this section as well: May I suggest to delete this paragraph and the parameter external = true in the suggested compose.yaml as a docker compose down wouldn't actually delete the volume. One would need to provide an addional -v or --volumes in addition to loose its content. In many cases providing this flag additionally would cause damage to applications, which usually do not mention this in their sample configurations or examples as Docker users usually seem to be aware of the consequences. If you agree @francislavoie I would simply delete these two parts in another commit.

Copy link
Author

@BjoernLudwigPTB BjoernLudwigPTB Oct 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@francislavoie And please review my comment above and give your opinion on that.

  • suggestion 1: remove this paragraph and the external = true parameter for the volume in the compose.yaml.
  • option 2: Apply your suggested changes and leave everything else as is.


Graceful reloads can then be conducted via `docker compose exec --workdir /etc/caddy caddy caddy reload`.
BjoernLudwigPTB marked this conversation as resolved.
Show resolved Hide resolved
Loading