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

Please Make Container Restart Policy Configurable #13

Closed
ryantfowler opened this issue Sep 20, 2019 · 5 comments
Closed

Please Make Container Restart Policy Configurable #13

ryantfowler opened this issue Sep 20, 2019 · 5 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@ryantfowler
Copy link
Contributor

I just noticed that when I stop the primary warden containers ( traefik, dnsmasq, portainer, and the ssh tunnel ), and restart my Docker daemon, there's restart policies defined for these containers. In my opinion, this is a bit intrusive and assumes that a user of this project wants this behavior.

Is it possible to have system-level warden configurations stored in the ~/.warden directory somewhere? If so, then it would provide finer grained control over warden's behavior.

In the context of restart policies, due to these being defined here https://github.com/davidalger/warden/blob/master/docker/docker-compose.yml, it would appear to be possible to use the same logic you have in the environment docker-compose files, such as this here: https://github.com/davidalger/warden/blob/master/environments/magento2.base.yml#L5, where you have a default value if no environment variable is found.

If you're open to this addition to the project, I can open a PR with my suggested changes. I am not leading with a PR for this suggestion, as I'd prefer to get your feedback before doing work.

Thanks

@davidalger
Copy link
Collaborator

davidalger commented Jan 25, 2020

@ryantfowler If you still have interest in pursuing this, I wanted to followup and let you know I'd be interested in opening the door to allowing something like. What I'm thinking is it should be done something like the following:

  1. If the file ~/.warden/global-services.override.yml exists then it would be passed as an added compose file wherever docker/docker-compose.yml is used today.

  2. In doing this, the logic in up, down, start, stop, sign-certificate, and restart (all the places previously mentioned config file is referenced) would need to be unified to avoid further code duplication. This could be done by adding a function to say utils/global.sh and loading it similar to how utils/env.sh and others are in other places.

  3. Once the above is in place, the restart policy could be adjusted by placing the following overlay to adjust the restart policy as desired:

version: "3.5"
services:
  traefik:
    restart: never

  portainer:
    restart: never

  dnsmasq:
    restart: never

  tunnel:
    restart: never

Taking this approach has the advantage that it's flexible and also opens the door to other user-specific modifications to any of the global services deployed by Warden (including adding new ones to your own local setup).

Let me know. If this isn't something you wish to take on, I'll probably go ahead and close it out as I have not the use case to justify investing my own time in the endeavor at this point in time.

@davidalger davidalger assigned davidalger and unassigned davidalger Jan 25, 2020
@davidalger davidalger added the up for grabs Extra attention is needed label Jan 25, 2020
@ryantfowler
Copy link
Contributor Author

@davidalger thanks for the follow up. Yes, I'd be interested in pursuing this. Please assign this to me.

@davidalger
Copy link
Collaborator

davidalger commented Jan 29, 2020

This morning a different and simpler concept came to me, inspired from a tweet by @talesh:
https://twitter.com/_Talesh/status/1222305123268825088

See what I just pushed up in 5073c5b. To use it you'd simple need to leverage one of the following lines in ~/.warden/.env:

WARDEN_RESTART_POLICY=no
WARDEN_SERVICE_DOMAIN=warden.test
TRAEFIK_LISTEN=127.0.0.1

Thoughts on this? The solution previously suggested may be overly complex here. Also do you think each service should warrant a different restart policy or is a single toggle sufficient for your use case?

@ryantfowler
Copy link
Contributor Author

@davidalger Yea, I agree that is a simpler approach, and looks good to me.

Regarding a different reset policy for each service, or just one for all, I think a single toggle is good enough right now.

That said, I don't see any additional suggestions for this solution right now. You handled it 👍 Any general idea of when this could go out in a release (I can always patch my local with this until then, so I'm not being pushy).

davidalger added a commit that referenced this issue Jan 30, 2020
…(ships with Fedora 30) doesn't support --env-file param
@davidalger davidalger added this to the Warden 0.2.1 milestone Jan 30, 2020
@davidalger davidalger added enhancement New feature or request and removed up for grabs Extra attention is needed labels Jan 30, 2020
@davidalger davidalger self-assigned this Jan 30, 2020
davidalger added a commit that referenced this issue Jan 30, 2020
davidalger added a commit that referenced this issue Jan 30, 2020
@davidalger
Copy link
Collaborator

Going to publish this in Warden 0.2.1 release very shortly.

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

No branches or pull requests

2 participants