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

services started with docker compose won't be stopped by sablier unless hit with a request first #153

Closed
xsolinsx opened this issue May 9, 2023 · 34 comments · Fixed by #346
Labels

Comments

@xsolinsx
Copy link

xsolinsx commented May 9, 2023

Describe the bug
I have a docker compose which starts a multitude of services, sablier is correctly configured as it is able to start/stop services as needed but (here's the bug) ONLY when the services have been hit by a request.
If I only start the services with docker compose up -d all services would stay up & running forever.
In order for them to be "detected" by sablier it seems I need to hit them with a request.

Context

  • Sablier version: 1.3.0
  • Provider: docker 23.0.5
  • Reverse proxy: traefik 2.10.1
  • Sablier running inside a container? YES

Expected behavior
After starting services with docker compose up -d Sablier should wait for the time configured in the config and then stop them without the need to hit them first with a request.

@xsolinsx xsolinsx added the bug Something isn't working label May 9, 2023
@acouvreur
Copy link
Member

Hi @xsolinsx,

Without any prior request, Sablier has no way of knowing which containers to start and stop.

The beta feature of group using autodiscovery of containers with sablier labels could implement such feature.

@xsolinsx
Copy link
Author

xsolinsx commented May 9, 2023

Nice, so this is something that will be implemented in the next release right?
I will leave this open as I do not know whether you want to close this right now or when you release the next version, I will leave the decision to you 😄
Thanks

Edit: I've found a sketchy workaround to execute right after the docker compose up -d
Grep the line with the names of the containers, get only the container(s) names for each middleware, replace comma if present, reduce to one line and stop the containers

grep -h "names: " /path/to/traefik/file_dynamic_conf/sablier_middlewares_definitions.yml |
    cut -d':' -f 2 |
    sed "s/,/ /g" |
    tr '\n' ' ' |
    xargs docker stop

@IzeQube
Copy link

IzeQube commented Jun 20, 2023

I have just started implementing sablier with caddy and the new groups function.

Do you think it would be possible to include the function to shutdown not needed containers if they are up, but shouldn't be up because of missing requests...

@ab623
Copy link

ab623 commented Jul 14, 2023

Hi @xsolinsx,

Without any prior request, Sablier has no way of knowing which containers to start and stop.

The beta feature of group using autodiscovery of containers with sablier labels could implement such feature.

On startup why cant sablier get a list of all containers with the sablier.enable label set to true, and apply the default session time to them?

I've also noticed that even through the service is up, once sablier first starts, it still shows the "waiting" screen on the first hit, even though it should bypass it as the service is up. I assume its for the same underlying reason, it hasnt built this "internal registry" of services.

@webysther
Copy link

webysther commented Jul 18, 2023

Add healthcheck to you services fix the problem or configure a status server to check.

@ab623
Copy link

ab623 commented Jul 19, 2023

There already is a healthcheck on the service.

Currently

  1. Service A comes up (showing healthy)
  2. Sablier comes up
  3. Wait for an hour
  4. Service A is still up

Expected

  1. Service A comes up (showing healthy)
  2. Sablier comes up
  3. Wait for an hour
  4. Sablier brings down Service A

Add healthcheck to you services fix the problem or configure a status server to check.

What is a status server?

@acouvreur
Copy link
Member

acouvreur commented Jul 19, 2023

Sablier does not (yet) pick up already started services, you need to access them at least once in order for them to be downscaled.

I will probably change this behavior with an option in the future.

@webysther
Copy link

There already is a healthcheck on the service.

Currently

  1. Service A comes up (showing healthy)
  2. Sablier comes up
  3. Wait for an hour
  4. Service A is still up

Expected

  1. Service A comes up (showing healthy)
  2. Sablier comes up
  3. Wait for an hour
  4. Sablier brings down Service A

Add healthcheck to you services fix the problem or configure a status server to check.

What is a status server?

like uptime kuma, but, nevermind, I was wrong about it.

Copy link

github-actions bot commented Feb 7, 2024

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Feb 7, 2024
@xsolinsx
Copy link
Author

xsolinsx commented Feb 7, 2024

unstale

@github-actions github-actions bot removed the Stale label Feb 9, 2024
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Mar 10, 2024
@xsolinsx
Copy link
Author

unstale

@github-actions github-actions bot removed the Stale label Mar 11, 2024
@MundMBo
Copy link

MundMBo commented Mar 15, 2024

I am also quite interested in the resolution of this bug. Is there anything planned (roughly) when it will be solved? Will it be part of the next release?

My current workaround for this bug is to extend my CICD-pipeline. After deploying and starting the container I am sending one request to the container without waiting for any response.

curl --head --max-time 1 "https://route.to/docker/service" >/dev/null 2>&1

This is working for the dynamic and blocking strategy of sablier, as well as for all my containers regardless of their response and the http response status code.

@ab623
Copy link

ab623 commented Mar 15, 2024

I do the same. When I restart services I have to hit them once with a request so Sablier kicks in. Not ideal.

@acouvreur
Copy link
Member

acouvreur commented Jul 3, 2024

Reviving this issue, I will create this feature but I'd like to gather some feedback here.


The goal is to create a startup behavior after everything has been loaded properly, workloads that have been auto-discovered (using labels), will be shutdown if they are currently running and not in an active session.

How would you name such option ?

  • auto-stop
  • auto-stop-on-startup
  • shutdown-running-discovered-workloads
  • etc.

if you have some ideas, that'd be great. I know that @gorositopablo you'd be interested by this.

@IzeQube
Copy link

IzeQube commented Jul 3, 2024

I think I like auto-stop-on-startup.

And I am really looking forward to this new function.

acouvreur added a commit that referenced this issue Jul 4, 2024
This feature adds the capability to stop unregistered running instances upon startup.

Previously, you had to stop running instances manually or issue an initial request that will shut down instances afterwards.

With this change, all discovered instances will be shutdown. They need to be registered using labels. E.g.: sablier.enable=true

Fixes #153
@pablogorosito
Copy link

Agree, I believe that auto-stop-on-startup would be better.

@acouvreur acouvreur linked a pull request Jul 4, 2024 that will close this issue
acouvreur added a commit that referenced this issue Jul 4, 2024
This feature adds the capability to stop unregistered running instances upon startup.

Previously, you had to stop running instances manually or issue an initial request that will shut down instances afterwards.

With this change, all discovered instances will be shutdown. They need to be registered using labels. E.g.: sablier.enable=true

Fixes #153
@acouvreur
Copy link
Member

🎉 This issue has been resolved in version 1.8.0-beta.7 🎉

The release is available on:

Your semantic-release bot 📦🚀

@acouvreur
Copy link
Member

acouvreur commented Jul 4, 2024

You can try using the docker tag 1.8.0-beta.7.

This is enabled by default. Because as you all stated, this is the behavior we'd actually expect from Sablier. And I agree.

It's not a breaking change in the way that old configurations are still valid. You can opt-out by setting provider.auto-stop-on-startup to false.

I'm waiting for your feedback!

@valankar
Copy link
Contributor

valankar commented Jul 4, 2024

Thanks for implementing this. Just a quick question. If I'm using names in my Caddyfile, I also need to set the label sablier.enable=true in the separate containers, correct?

@acouvreur
Copy link
Member

Thanks for implementing this. Just a quick question. If I'm using names in my Caddyfile, I also need to set the label sablier.enable=true in the separate containers, correct?

Yes.

Sablier needs to know which container to stop. For that, you need to add the following label sablier.enable=true.

You can still use names to target containers, there's no issue with that.

This is purely auto-discovery done by Sablier at startup, and it does not involve any reverse proxy configuration or adjustment.

@valankar
Copy link
Contributor

valankar commented Jul 4, 2024

Just tested with the beta and it works well.

@xsolinsx
Copy link
Author

xsolinsx commented Jul 4, 2024

if I run twice the same docker compose up -d sablier only kills the containers the first time, not the second one

@deimosfr
Copy link

deimosfr commented Jul 5, 2024

I confirm I have the same behavior

@acouvreur
Copy link
Member

acouvreur commented Jul 5, 2024

if I run twice the same docker compose up -d sablier only kills the containers the first time, not the second one

I confirm I have the same behavior

Can you please share a quick reproducible compose file ?
Also, with the debug log enabled, what do you see in the logs ?


One behavior of the auto-stop is to stop all containers that are currently running, but that Sablier is not aware of.
By default, Sablier will save the current state upon shutdown.


So upon restart, Sablier loads back your containers session from the previous run and it will not shut them down.
You can try by disabling storage completely, I think this issue won't happen again.

@valankar
Copy link
Contributor

valankar commented Jul 5, 2024

I think what they are referring to is the auto-stopping is only run at startup and not periodically. Then, if you restart your container (not the sablier one), it does not get stopped again until you make another request. i.e. The containers are stopped at "startup" of sablier only.

Perhaps it should run periodically?

@acouvreur
Copy link
Member

I think what they are referring to is the auto-stopping is only run at startup and not periodically. Then, if you restart your container (not the sablier one), it does not get stopped again until you make another request. i.e. The containers are stopped at "startup" of sablier only.

Perhaps it should run periodically?

The current behavior is to stop the found containers at startup only, not periodically.

I think that the desired behavior is actually to register all your containers, and have Sablier stop them for you.
If you start the container manually while Sablier is running, it will not stop it again for you.

If the current behavior does not meet your needs, maybe we can refine, change or add configuration option to get to the desired behavior.

What behaviors would you like to have ?

@valankar
Copy link
Contributor

valankar commented Jul 5, 2024

For my case, I'm happy with the current behavior. However, I can imagine the scenario described in:

https://github.com/acouvreur/sablier/issues/153#issuecomment-1999167626

For example, say you have some automatic updates of your container, or you are doing some development requiring updating it. You may be only hitting the service on an internal address and not via Sablier. Every time that happens, you'd need to access it with Sablier to have it notice the new up state.

You can of course update the image of your containers while they are stopped, which is one way around this. Perhaps others can chime in on their use case/reasoning.

@deimosfr
Copy link

deimosfr commented Jul 5, 2024

I think what they are referring to is the auto-stopping is only run at startup and not periodically. Then, if you restart your container (not the sablier one), it does not get stopped again until you make another request. i.e. The containers are stopped at "startup" of sablier only.

Perhaps it should run periodically?

Exactly the issue and behavior I expect! Check periodically would be the solution from my POV.

@valankar
Copy link
Contributor

valankar commented Jul 5, 2024

I think doing it periodically makes sense, though we should be a bit careful on the period. Imagine a scenario where you bring up this container at the end of the time period. It will immediately get stopped.

It should probably work like:

  1. Sablier detects that a container is up that should not be.
  2. Sablier resets its timer for the service just as if a request has been made.
  3. Container is shutdown when the timer is up.

@acouvreur
Copy link
Member

Perhaps this feature should be some kind of "reconciliation" that could happen periodically.

I better understand the use case now.

We can maybe add this as an extra feature, and also have some opt-in/opt-out labels.

We can go further and have some labels with pre-defined hours for reconciliation. Any way, we might have w new feature here, which is not the auto-stop-on-startup.

From my point of view, the startup part of auto-stop-on-startup was referring to Sablier. What do you think ?

@gorositopablo
Copy link

I'm not sure about all the comments with docker-compose, but I've tried with sablier:1.8.0-beta.7 in Kubernetes with Traefik and the containers are only terminated at Sablier boot up.

So, let's say that I:
1- Deploy Sablier
2- Deploy my workload (using labels)
3- Nothing happens
4a- restarting Sablier it kill the pod, or (new behavior with sablier:1.8.0-beta.7)
4b- making a request to the backend, it kill the pod. (classic behavior)

While Sablier does kill the pod at the start of its lifecycle, it means that I need to restart Sablier each time that I have a new release.

Thanks @acouvreur

@deimosfr
Copy link

Good Idea @acouvreur . I think we can start simple without specific hours and add it later if requested? I personaly don't have this need

@acouvreur
Copy link
Member

🎉 This issue has been resolved in version 1.8.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants