sablierapp / sablier

Start your containers on demand, shut them down automatically when there's no activity. Docker, Docker Swarm Mode and Kubernetes compatible.
https://sablierapp.dev/
GNU Affero General Public License v3.0
1.36k stars 46 forks source link

Add `Caddy` plugin #67

Closed acouvreur closed 1 year ago

acouvreur commented 1 year ago

Add Caddy plugin

EmberLightVFX commented 1 year ago

+1 for a Caddy plugin!

acouvreur commented 1 year ago

:tada: This PR is included in version 1.4.0-beta.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

ab623 commented 1 year ago

Hi @acouvreur.

Thanks for the great work so far. I have been trying to implement the Caddy solution from the beta branch, but have been uncovering some issues. I have worked around some of these and documented my findings below. They could result in either docs being clearer or some code changes perhaps?

Building Caddy

When building Caddy using the Caddyfile as per the instructions here https://acouvreur.github.io/sablier/#/plugins/caddy I had been getting error.

I think adding the repo and then overwriting the plugin folder is overly complex. I managed to build successfully using the following config

ARG CADDY_VERSION=2

FROM caddy:${CADDY_VERSION}-builder-alpine AS builder

RUN xcaddy build \
    --with github.com/acouvreur/sablier/plugins/caddy@beta

FROM caddy:${CADDY_VERSION}-alpine

COPY --from=builder /usr/bin/caddy /usr/bin/caddy

Note: As you can see I'm running directly from the beta branch.

Directive Ordering

I was getting an error in Caddy when using the standard configuration

....directive 'sablier' is not an ordered HTTP handler, so it cannot be used here"...

This can easily be resolved by setting the order in the global scope

{
    order sablier before reverse_proxy
}

Names / Groups

Eventually I manage to get the service up and running

I have the following dockerfile

  whoami:
    container_name: whoami
    image: containous/whoami:latest
    labels:
      - "sablier.enable=true"
      - "sablier.group=whoami"
    networks:
      - caddy
    restart: always

Question: Is group really necessary?

My Caddyfile has the following excerpt:

    @host_int_whoami host whoami.example.com
    handle @host_int_whoami {
        sablier {
            group whoami
            dynamic
        }
        reverse_proxy whoami:80
    }

When the group was used, i was getting 404 errors. Switching over to the following config, using names, seemed to work.

    @host_int_whoami host whoami.example.com
    handle @host_int_whoami {
        sablier {
            names whoami
            dynamic
        }
        reverse_proxy whoami:80
    }

Edit: After some digging. I found that I was rming the containers instead of stopping them, during testing. And then Sablier didn't think they existed. Potentially could be a more helpful error message to indicate the 404 is because the container may have been removed rather than stopped. This MIGHT have something to do with the above error, but groups should be looking for docker labels rather than containers, so it doesn't fully add up to me.

I would be cool if somehow Sabiler was able to start containers from a dockerfile...

I think otherwise the plugin works rather nicely. The Docs however do need some work (I know you are working on this), as there is a lot of information spread across the Readme and the new docs site. It was rather hard to get things going on the initial run! But after a break and digging into the code a bit, I managed to do so.

Thanks

acouvreur commented 1 year ago

Hi @ab623

Thanks for your feedback!

Building Caddy

When building Caddy using the Caddyfile as per the instructions here https://acouvreur.github.io/sablier/#/plugins/caddy I had been getting error.

I think adding the repo and then overwriting the plugin folder is overly complex. I managed to build successfully using the following config

ARG CADDY_VERSION=2

FROM caddy:${CADDY_VERSION}-builder-alpine AS builder

RUN xcaddy build \
    --with github.com/acouvreur/sablier/plugins/caddy@beta

FROM caddy:${CADDY_VERSION}-alpine

COPY --from=builder /usr/bin/caddy /usr/bin/caddy

Note: As you can see I'm running directly from the beta branch.

Thanks for this, I didnt know you could directly refer to a git repo using caddy directive!

I'll probably update the docs to use this version instead.

Directive Ordering

I was getting an error in Caddy when using the standard configuration

....directive 'sablier' is not an ordered HTTP handler, so it cannot be used here"...

This can easily be resolved by setting the order in the global scope

{
    order sablier before reverse_proxy
}

Seems weird that I didnt encounter this issue, is it something I can fix on my end?

Names / Groups

Eventually I manage to get the service up and running

I have the following dockerfile

  whoami:
    container_name: whoami
    image: containous/whoami:latest
    labels:
      - "sablier.enable=true"
      - "sablier.group=whoami"
    networks:
      - caddy
    restart: always

Question: Is group really necessary?

So about the groups, this is something really going on again and again lately, but I see you figured it out at some point.

So with groups you can actually auto discover your containers and group them. Then you have to specify which group gets woken up by this plugin. You may want to include multiple containers inside your groups and that allows you to do it. Maybe later, labels will allow you to customize thing on a per container basis (for orchestrators, things such as scale, etc.)

But yeah, I'd like for this mapping to be automatic because it is attached to the container being proxied, but unfortunately it is not possible to reuse this.

My Caddyfile has the following excerpt:

    @host_int_whoami host whoami.example.com
    handle @host_int_whoami {
        sablier {
            group whoami
            dynamic
        }
        reverse_proxy whoami:80
    }

When the group was used, i was getting 404 errors. Switching over to the following config, using names, seemed to work.

I'll probably do an update so when a requested group does not exist or is empty then a specific error will be displayed.

Edit: After some digging. I found that I was rming the containers instead of stopping them, during testing. And then Sablier didn't think they existed. Potentially could be a more helpful error message to indicate the 404 is because the container may have been removed rather than stopped. This MIGHT have something to do with the above error, but groups should be looking for docker labels rather than containers, so it doesn't fully add up to me.

I would be cool if somehow Sabiler was able to start containers from a dockerfile...

The point is to be attached on already set up containers and bring them up and down on and on. Building an image from a Dockerfile and running it is not something that would fit in this project at this moment.

I think otherwise the plugin works rather nicely. The Docs however do need some work (I know you are working on this), as there is a lot of information spread across the Readme and the new docs site. It was rather hard to get things going on the initial run! But after a break and digging into the code a bit, I managed to do so.

Thanks

Thanks, docs are still in beta and Im working on it if I have time. Your feedback is very valuable

ab623 commented 1 year ago

Hi @acouvreur.

Thanks for the quick feedback. A few comments

Directive Ordering

I was getting an error in Caddy when using the standard configuration ....directive 'sablier' is not an ordered HTTP handler, so it cannot be used here"... This can easily be resolved by setting the order in the global scope

{
    order sablier before reverse_proxy
}

Seems weird that I didnt encounter this issue, is it something I can fix on my end?

I did a bit more digging into this and I think you didn't encounter this during your testing is because you use ROUTE blocks, which don't adhere to ordering. In a route, there is no internal reordering, directives run from top to bottom. (https://caddyserver.com/docs/caddyfile/directives/route). I use Handle blocks to handle the requests which are ordered internally, and therefore need the above config. Its a common thing to do, if a new directive is defined, as you have done so.

I don't know Caddy too well. But other implementation options could be to not create a directive, but utilise forward_auth directive, or utilising the multiple upstreams. Both of which though would not allow the additional customisation you allow in your directive, so I would leave as is, and just document the ordering issue.

Groups

So about the groups, this is something really going on again and again lately, but I see you figured it out at some point.

So with groups you can actually auto discover your containers and group them. Then you have to specify which group gets woken up by this plugin. You may want to include multiple containers inside your groups and that allows you to do it. Maybe later, labels will allow you to customize thing on a per container basis (for orchestrators, things such as scale, etc.)

But yeah, I'd like for this mapping to be automatic because it is attached to the container being proxied, but unfortunately it is not possible to reuse this.

I'm not following this entirely. What I have understood is you can group your services together under a common name, and then sablier can bring up the group, and therefore all the containers in that group. Is that right? I wasnt entirely following the auto discover part.

If my understanding is correct, whilst the groups makes sense, the only reason you want to group stuff together is if there is a dependency. Can't Sablier just read the depends on links and bring those up if they are down also?

Docker Compose

The point is to be attached on already set up containers and bring them up and down on and on. Building an image from a Dockerfile and running it is not something that would fit in this project at this moment.

I completely agree on this point.

API - Status

It would be nice to have an API and command that exposes the status of the container including

GET: /api/services/

API - Stop Now

It would be nice to have an API and command that stops all services that Sablier manages.

POST: /api/services/stop

Healthchecks

I used the following healthcheck.

    healthcheck:
      test: "sablier health"
      interval: 60s
      timeout: 30s
      retries: 3
      start_period: 30s

For some reason the /health endpoint is showing OK, even execing into the container to run the sablier health command comes back okay also. But docker continues to report as (health: starting). It never moves to healthy. This poses an issue, because I have another container which auto restarts unhealthy containers on a timed internal. So Sablier keeps getting restarted. For now I have removed the healthcheck, but have no idea why Docker continues to report as starting.

Sorry for the mammoth essay!

acouvreur commented 1 year ago

Thanks @ab623 !

A lot of very nice features :)

If my understanding is correct, whilst the groups makes sense, the only reason you want to group stuff together is if there is a dependency. Can't Sablier just read the depends on links and bring those up if they are down also?

With the way sablier is designed, as connector between any reverse proxy and any provider, I cannot truely rely and special feature from certain provider.

More than that, depends_on might not even be telling everything, somnetimes services communicate together but hey don't depend on each other.

For now, you need to register the containers you want to wake up with labels (at least on beta), and it will become the default.


For your health I believe you have to use

    healthcheck:
-    test: "sablier health"
+    test: ["CMD", "sablier", "health"]
      interval: 60s
      timeout: 30s
      retries: 3
      start_period: 30s
ab623 commented 1 year ago

Makes sense to me. I think labels are a perfect suitable. I guess what I'm confusing is name can only refer to a single container whilst you can assign multiple containers to a group and therefore start an entire group.


For heath that also did not work, so I'm not sure whats going on here.

serfriz commented 8 months ago

Hello! It's been 6 months and the Caddy build instructions haven't been updated, @acouvreur is it really necessary to include the ADD statement in the Dockerfile? Also, is it possible to use the main branch instead of the beta? Any downsides? Thank you!

acouvreur commented 8 months ago

Hello! It's been 6 months and the Caddy build instructions haven't been updated, @acouvreur is it really necessary to include the ADD statement in the Dockerfile? Also, is it possible to use the main branch instead of the beta? Any downsides? Thank you!

Hi @serfriz, you have to build a custom image to pack the plugin as stated in the documentation image

And Caddy has been available since the version 1.4.0, so you should be able to use any tag from then. (I would not recommend using a branch tag such as main or beta, you should stick with one version to avoid breaking changes!)

However, it seems that version 1.5.0 was never pushed upstream because of github actions issues. I'll fix that.

serfriz commented 8 months ago

Thank you for your reply @acouvreur, what I was specifically asking was if the Dockerfile from @ab623 was more correct:

ARG CADDY_VERSION=2
FROM caddy:${CADDY_VERSION}-builder-alpine 

AS builder RUN xcaddy build \
    --with github.com/acouvreur/sablier/plugins/caddy@beta (maybe without beta, or some specific release)

FROM caddy:${CADDY_VERSION}-alpine

COPY --from=builder /usr/bin/caddy /usr/bin/caddy

Than also adding the statement from the wiki:

ADD https://github.com  /acouvreur/sablier.git#v1.4.0-beta.3 /sablier
acouvreur commented 8 months ago

Thank you for your reply @acouvreur, what I was specifically asking was if the Dockerfile from @ab623 was more correct:

`` ARG CADDY_VERSION=2 FROM caddy:${CADDY_VERSION}-builder-alpine

AS builder RUN xcaddy build --with github.com/acouvreur/sablier/plugins/caddy@beta (maybe without beta, or some specific release)

FROM caddy:${CADDY_VERSION}-alpine

COPY --from=builder /usr/bin/caddy /usr/bin/caddy ``

Instead of also adding the command (from the wiki):

ADD https://github.com /acouvreur/sablier.git#v1.4.0-beta.3 /sablier

You're absolutely right.

The version on the documentation got messed up. I will make the changes on the next release this week.

You can use the

AS builder RUN xcaddy build \
    --with github.com/acouvreur/sablier/plugins/caddy@beta (maybe without beta, or some specific release)

if you want, or replace beta with v1.4.0 or v1.5.0.

Tell me if that works for you.

serfriz commented 8 months ago

You're absolutely right.

The version on the documentation got messed up. I will make the changes on the next release this week.

You can use the

AS builder RUN xcaddy build \
    --with github.com/acouvreur/sablier/plugins/caddy@beta (maybe without beta, or some specific release)

if you want, or replace beta with v1.4.0 or v1.5.0.

Tell me if that works for you.

Thank you! I asked to make sure how to build it correctly before closing serfriz/caddy-custom-builds#12. I'll let you know if I have any issues!