Conversation
✅ Deploy Preview for openpolicyagent ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
32b73e0 to
1f06457
Compare
|
Is a stateful disk-based watch approach like this not something of a cloud native anti-pattern? 🤔 (And apologies if this has all been covered in previous discussions. As you know I've not been able to follow everything that's been going on recently). |
1b47102 to
567315f
Compare
|
@anderseknert no worries I don't think this has been covered in previous discussions yet, so good to ask now! I don't think it is an anti-pattern, I think this is similar to how Kubernetes ConfigMaps can be updated live. CoreDNS also lets configs be updated automatically. Prometheus has something similar but needs a trigger. I do agree better documentation is needed to be very explicit about the pros and cons. @charlieegan3 also mentioned maybe adding a new flag or something similar to make this reload behavior opt-in. Which could also help us be more opinionated and let users make the right choice, in case I am wrong and this is an anti-pattern 😅 Would a new flag like @johanfylling, @philipaconrad or @srenatus more input on this would be great 👀 in case this needs more design review. |
Right, that's kinda the point I was trying/failed to make. Managing lifetimes and state is the role of Kubernetes in that type of deployment. If you want to run something scheduled you shouldn't configure cron jobs in your container distros, but use Kubernetes jobs and scheduling. Same with health checks, orchestration, metrics, whatnot. Having OPA itself decide to shutdown/restart triggered by some opaque (seen from the outside) event seems to me like it would go against that. But to also reiterate, I'm not necessarily opposed :) OPA runs on Windows monoliths, mainframes, in cars and what have you. If we need to better support a use-case where this would help, we should do it. Just trying to make sure we're clear about when to use what, and when not to. Making this explicit / opt-in sounds reasonable to me. |
12f538e to
6223580
Compare
|
Thank you @anderseknert for the explanation, that makes sense. I've pushed an update to have this be opt-in using a new flag |
|
I agree it's most likely not the pure gospel of immutable infrastructure. That said, if OPA is a sidecar and you can only restart the entire pod, it might be nice to have a compromise available. Since it's a niche thing, the simpler the approach, the better. This looks good to me. What will happen to existing connections? Presumably they'll be dropped, but that's probably OK. |
The configuration file watcher needs to parse every plugin section to check a new configuration before restarting under it, and that logic only existed inside discovery. Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
Both registered a commit trigger and never took it back off, and Manager.Stop closed a store it does not own, so swapping either one over a live store left a trigger behind pointing at the old instance and closed the store underneath the new one. Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
Reconfiguring in place could only reach part of the configuration, so the rest was refused and the docs needed a table of what OPA would not do. A restart reaches all of it: the file is validated first, then the server and the plugins are rebuilt from it over the store that outlives them, behind an opt-in --watch-config flag since a restart costs far more than --watch's reload. Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
6223580 to
b1fd730
Compare
|
So idle connections get closed, waits for in-progress requests to finish, new connections are refused, and then any long running requests never stop running. So no in-progress request is dropped. |
This branch has not been deployed
3 participants
Follow up to: #9192 (comment)
Following a good suggestion from @charlieegan3, instead of trying to reload configuration in place restarting the server gives us a way to update everything without having to document a list of settings that don't work. If necessary we can revisit this in the future and make it more of a hybrid solution if there is a demand for it.