ofelia icon indicating copy to clipboard operation
ofelia copied to clipboard

Handle config changes on docker updates

Open rdelcorro opened this issue 4 years ago • 36 comments

The purpose of this PR is:

  • Update tasks if dockers are started, stopped, restarted, or changed
  • Do not require a dummy task on the ofelia container just to use ofelia
  • Support INI and docker labels at the same time. The configs will simply be merged
  • Do not require ofelia to restart in order to pick up new or remove tasks

rdelcorro avatar Dec 21 '20 03:12 rdelcorro

Hi @mcuadros your app is super useful and I would like to integrate my changes so I can contribute to this amazing project. This change allows me to start / stop containers and let ofelia keep running, without restarting or possibly interrupting other jobs. It also supports hybrid configs (INI + Docker) or just one or the other. If there is any suggestion or comment, please let me know

rdelcorro avatar Dec 21 '20 03:12 rdelcorro

Thanks for your contribution. This is quite a lot of changes. I will try to review it in the coming days.

Also, don't be shy to ping me as a reminder if it takes too long :)

taraspos avatar Dec 21 '20 08:12 taraspos

Hi @Trane9991, I have been using this for a while now and it seems to be working just fine. Re pinging as requested

rdelcorro avatar Jan 05 '21 00:01 rdelcorro

I am also very interested in this functionality getting merged! Thanks for this contribution!

bertbesser avatar Jan 15 '21 18:01 bertbesser

Sorry for the delays, will try to review it this weekend.

taraspos avatar Jan 16 '21 08:01 taraspos

@Trane9991 changes are now ready

rdelcorro avatar Feb 05 '21 16:02 rdelcorro

Since this PR is very related, please let me share a question/thought in this discussion.

There should never be more than one ofelia container reading job-exec configs from docker labels, in particular with live scanning. Otherwise each job-exec will be executed from multiple ofelia instances simultaneously.

When using job-run, I am kind of forced to have additional ofelia containers, namely one for each project using job-run. (Otherwise part of each project's config would be with the central ofelia container, which I do not want.) In the many additional ofelia containers, I cannot configure job-run from labels, since then I would be scanning job-exec labels from each of them, which is not good as observed above.

So, shouldn't job-run labels also be re-scanned from all containers, such that one docker daemon needs only one ofelia container?

Thx.

bertbesser avatar Feb 13 '21 09:02 bertbesser

@bertbesser let me start by some definitions:

job-exec: this job is executed inside of a running container. job-run: runs a command inside of a new container, using a specific image.

Docker labels are only available inside a running container, so I think the only job type that can be dynamic is job-exec (runs in the container that has the labels on). Job-run will create a new container, and as such, that container should have the labels attached that tell what container to execute, which makes the process recursive.

In my opinion, if you want to make job-run dynamic, I would simply update the ofelia ini file and let that be auto reloaded by ofelia, the same way that it does with the labels. The idea is to make not only labels but also ini files be reloadable. I do not think its advisable to have more than one ofelia container, as it would execute jobs N times, particularly the labels based ones.

As per docker docs, changing labels is not allowed after container creation, so you can't simply update the labels on the ofelia container either.

rdelcorro avatar Feb 13 '21 16:02 rdelcorro

@rdelcorro Thank you for taking the time to repond!

I believe that recursion is not a problem. Assume that my service container, e.g. nextcloud, has ofelia labels for a related cron job, e.g. contacts export. Then scanning nextcloud's ofelia labels would allow to job-run a new container for the export, and that new container does not need to carry any labels.

Regarding updates to the cron job definition. I would have to change nextcloud's labels and restart the nextcloud container. In particular, this way the cronjob config is stored in my nextcloud git repo. Instead, changing ofelia's ini file would require me to store nextcloud related configuration in my ofelia git repo. However, I would like to decentralize cron config, i.e. store it with nextcloud instead of ofelia. Hence the question if ofelia can be made to pick up job-run configs from other containers' labels :-D

(Note: I know that I could work around the problem by either job-exec-ing in the nextcloud service container, or job-exec-ing in a container that is dedicated to running the export job. However, this does not work well with ofelia's logging, i.e. logs would not appear in the container running the export job. Instead, they would appear in ofelia's container. Using job-run, logs will appear in the correct container.)

bertbesser avatar Feb 13 '21 19:02 bertbesser

@bertbesser I had the exact same use case like yours but due to file permissions and volume mounts I decided to use job-exec instead and run inside the same container. Since what you requested was trivial to implement, I already added that feature. Feel free to test the pre built container or build your own: docker pull rdelcorro/ofelia:handleChangesOnDocker

rdelcorro avatar Feb 14 '21 03:02 rdelcorro

@rdelcorro

Wow. Thanks for the quick addition. I tested and noticed that there is a race condition where does the job-run after the container defining the related labels has already stopped (this could potentially lead to errors when, say, the nextcloud service to export contacts from was already removed).

Before digging deeper into the issue, let's see what @Trane9991 has to add. I can imagine that picking up job-run labels from all containers is a breaking change. Why? Without the change each ofelia container will only respect job-run labels of itself. There might be users who run one ofelia container per project, where each ofelia container scans its own job-run docker labels. (This is easy to configure with e.g. docker-compose compared with building custom images including an ini file.) Picking up job-run labels from all containers will make those users' jobs run multiple times simultaneously, which is not intended.

Cheers :-)

bertbesser avatar Feb 14 '21 12:02 bertbesser

@bertbesser The ofelia container is polling docker for changes, so there is a chance were the source container (the one that has the labels) is either not fully started or it has terminated. I do not consider this a big deal as the “last run” will simply fail on a shutdown scenario. This is also a reason why I prefer job-exec for this. If the container is gone, the task is not even started.

Running more than one ofelia container or process will lead to problems in any case, as those jobs will run N times. Remember that ofelia does not know that other ofelias are running (same applies if you use an ini file in multiple ofelia binaries). Docker labels are global to the docker daemon, so multiple ofelia containers will also read all the labels. You can make this work with multiple different ini files though. This change just lets you specify a job-run inside an external container but I do not consider this breaking anything, besides adding this support. In the past, job-runs on containers != than ofelia were ignored. This is adding that feature. In the past, you should have not added them to external containers, so this is not breaking anything.

rdelcorro avatar Feb 14 '21 12:02 rdelcorro

I build your image, @rdelcorro - great iniative, something which was missing in the main version! However, when I try to start with command: daemon --docker I get the error mesage unknown flag docker'`

The README says to use it - but when I remove the flag, it seems to work as expected. Am I getting something wrong?

miggland avatar Feb 26 '21 17:02 miggland

@miggland the flag is now deprecated. I would have to update the readme. This should also be corrected: https://github.com/mcuadros/ofelia/pull/137/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R100 I will change those later today

rdelcorro avatar Feb 26 '21 17:02 rdelcorro

I've been using @rdelcorro's fork and it's been working great for me. Hi @Trane9991, is there any hope on merging this PR to the upstream?

shouya avatar Mar 20 '21 07:03 shouya

@shouya thanks for confirming that it works. You can count on this to be merged to the upstream. However, since this is quite a major change I need to do some extensive testing as well as a better review and I do not have much time to do it now. I will try to find a day to do it :)

taraspos avatar Mar 20 '21 21:03 taraspos

@bertbesser
Running more than one ofelia container or process will lead to problems in any case, as those jobs will run N times. Remember that ofelia does not know that other ofelias are running (same applies if you use an ini file in multiple ofelia binaries). Docker labels are global to the docker daemon, so multiple ofelia containers will also read all the labels.

What about running ofelia as a 'global' swarm service ? At least is what I was thinking of (approach taken from https://deck-chores.readthedocs.io/en/stable/usage.html#in-a-docker-swarm which I'm thinking to replace by ofelia)

  • Auto deploys one (and only one) ofelia instance per docker daemon in the swarm cluster (even for new nodes)
  • As is bound to local docker socket is scoped to only tasks/containers on that node (no multiple runs)
  • Each ofelia instance takes care of the other tasks running only on the same docker node

I know docker differentiates about labels to the task/container from the ones on the services, I'm not sure how this could impact ofelia.

By the way what is pending regarding this PR to be merged into master? As far as I browsed thru the project this will be a great feature and will solve #89 #78 and #57

markfqs avatar May 31 '21 20:05 markfqs

Hi all! Any news? @Trane9991

4n70w4 avatar Jul 10 '21 16:07 4n70w4

Sorry for the bump but this would be very helpful if implemented @Trane9991

Volence avatar Sep 02 '21 13:09 Volence

With the assistance of @githubsaturn, I was able to put together a working instance of Ofelia as a One Click App for the open source Caprover PaaS solution. However, without this pull request being accepted and a new docker image built, then all my effort has been wasted. After all was said and done, I ran into my last hurdle, with my research leading me to this pull request.

I am disheartened to see a full 30 day wait on a pull request on a solution that I am certain solves an issue for others as well as myself. I cannot in good conscience promote a One Click App for Caprover until this has been resolved.

To more immediately resolve this problem and to solve it for others in the long term, I have decided to hard fork the version of Ofelia as provided by @rdelcorro , (see https://github.com/rdelcorro/ofelia) as the license for Ofelia is currently MIT.

I am in a unique position that I am self employed with my own software development company and am generally available 7 days a week, 365 days a year as I am on-call and always keep a 4G LTE/5G modem and my laptop on me at all times with a 4G LTE smart watch and phone which alerts me to opened issues, etc. This affords me the ability to pledge that I can do my best to maintain the project in a manor that benefits everyone. (as much as possible, anyway).

The new project name will be Chronos. I will be publishing the new project at https://github.com/PremoWeb/Chronos. It will remain MIT licensed.

Thank you for building a great piece of software. Let's improve upon it!

maietta avatar Oct 01 '21 21:10 maietta

The project will be licensed under GPLv3, so that people are encouraged to contribute changes that could benefit everyone.

And you think changing the license from MIT to GPLv3 is an improvement? Do you really expect people who redistribute your new project to open source all their code?

teosoft123 avatar Oct 01 '21 21:10 teosoft123

The project will be licensed under GPLv3, so that people are encouraged to contribute changes that could benefit everyone.

And you think changing the license from MIT to GPLv3 is an improvement? Do you really expect people who redistribute your new project to open source all their code?

Realistically, no. But for liability sakes, yes. It's a formality that transcends speculation. It provides legal recourse in the event something awful happens like a huge multinational corporation decides to use and improve the code but refuses to hand over a copy so everyone can benefit. Without it, there is nothing we can do. With it, we have at least some semblance of legal recourse. We as in, the community at large or it's contributors.

maietta avatar Oct 01 '21 21:10 maietta

The project will be licensed under GPLv3, so that people are encouraged to contribute changes that could benefit everyone.

And you think changing the license from MIT to GPLv3 is an improvement? Do you really expect people who redistribute your new project to open source all their code?

Realistically, no. But for liability sakes, yes. It's a formality that transcends speculation. It provides legal recourse in the event something awful happens like a huge multinational corporation decides to use and improve the code but refuses to hand over a copy so everyone can benefit. Without it, there is nothing we can do. With it, we have at least some semblance of legal recourse. We as in, the community at large or it's contributors.

I. for once won't be able to use your new project commercially and instead will be forced to continue using the MIT-licensed one. And thus I won't be able to contribute even if I want to. My company explicitly bans use of any GPL open source projects, unless we don't redistribute. I don't think my case in in any way unique. Perhaps you will reconsider the license change.

teosoft123 avatar Oct 01 '21 21:10 teosoft123

The project will be licensed under GPLv3, so that people are encouraged to contribute changes that could benefit everyone.

And you think changing the license from MIT to GPLv3 is an improvement? Do you really expect people who redistribute your new project to open source all their code?

Realistically, no. But for liability sakes, yes. It's a formality that transcends speculation. It provides legal recourse in the event something awful happens like a huge multinational corporation decides to use and improve the code but refuses to hand over a copy so everyone can benefit. Without it, there is nothing we can do. With it, we have at least some semblance of legal recourse. We as in, the community at large or it's contributors.

I. for once won't be able to use your new project commercially and instead will be forced to continue using the MIT-licensed one. And thus I won't be able to contribute even if I want to. My company explicitly bans use of any GPL open source projects, unless we don't redistribute. I don't think my case in in any way unique. Perhaps you will reconsider the license change.

As a self-imposed general rule, virtually all projects I work on are GPLv3 licensed. In this particular case and after talking it over with one of my colleagues, I can see your side and fully agree with what you're saying. We'll go ahead and keep the MIT license for this project and move to benefit in another way, which is to encourage sponsorships. We'll accept donations for our work and where applicable, chip in to the contributors as funds are available. In this way, we can minimize any negative impact for the very unlikely scenario where some big company benefits and the contributors and users of the software don't.

I'll make the adjustment in my previous post, leaving this one intact. In the spirit of free and open software, I feel this is the best move.

maietta avatar Oct 01 '21 22:10 maietta

I'll make the adjustment in my previous post, leaving this one intact. In the spirit of free and open software, I feel this is the best move.

Thank you, Nick!

teosoft123 avatar Oct 01 '21 22:10 teosoft123

As the author of this PR I thank you for keeping this project alive. The owner does not seem to have time for the community and that’s ok. We just need to pass it over and keep it running. I am using this version for almost a year now and it works great. I will start contributing to your fork if the license remains unchanged as discussed above. Thanks

rdelcorro avatar Oct 01 '21 23:10 rdelcorro

As the author of this PR I thank you for keeping this project alive. The owner does not seem to have time for the community and that’s ok. We just need to pass it over and keep it running. I am using this version for almost a year now and it works great. I will start contributing to your fork if the license remains unchanged as discussed above. Thanks

You're welcome. I just began learning GoLang last year but only really started delving into it this year so I welcome contributions from people more skilled than me in this language. I'm really enjoy Go and looking forward to using it in many more projects in the future. I also very much appreciate that the original owner has put so much into making a solid alternative to cron for use in Docker Swarm.

I aim to have this operational tonight. I have the first docker image pushed to github's container registry. You can follow the project at https://github.com/PremoWeb/Chadburn. The docker image is at https://github.com/PremoWeb/Chronos/pkgs/container/chadburn. I plan to be auto-building and pushing the image to Github's container registry because Docker Hub imposes unreasonable limits and I am using a Paid Github plan.

Edit: The name was changed from Chonos to Chadburn to avoid conflict with another project.

maietta avatar Oct 02 '21 00:10 maietta

I plan to be auto-building and pushing the image to Github's container registry because Docker Hub imposes unreasonable limits and I am using a Paid Github plan.

Good to hear. As a piece of advice, would you consider pushing releases to Dockerhub? No need to push nightlies, but once in a while, when you feel that you've achieved a milestone, a new release (and a celebration) is justified. People are more familiar with pulling from Dockerhub, largely because it's a default, no need to specify an alternative registry.

teosoft123 avatar Oct 02 '21 00:10 teosoft123

I plan to be auto-building and pushing the image to Github's container registry because Docker Hub imposes unreasonable limits and I am using a Paid Github plan.

Good to hear. As a piece of advice, would you consider pushing releases to Dockerhub? No need to push nightlies, but once in a while, when you feel that you've achieved a milestone, a new release (and a celebration) is justified. People are more familiar with pulling from Dockerhub, largely because it's a default, no need to specify an alternative registry.

Yes. I just need to get the Github action workflows in order for this to work and have it only trigger when I set a new release.

maietta avatar Oct 02 '21 00:10 maietta

I plan to be auto-building and pushing the image to Github's container registry because Docker Hub imposes unreasonable limits and I am using a Paid Github plan.

Good to hear. As a piece of advice, would you consider pushing releases to Dockerhub? No need to push nightlies, but once in a while, when you feel that you've achieved a milestone, a new release (and a celebration) is justified. People are more familiar with pulling from Dockerhub, largely because it's a default, no need to specify an alternative registry.

Yes. I just need to get the Github action workflows in order for this to work and have it only trigger when I set a new release.

I have tagged releases triggering pushes to both Docker Hub and Github Container Reigstry.

maietta avatar Oct 02 '21 00:10 maietta