Re-render is not triggered on secret value change

CyCoreSystems / asterisk-config

Kubernetes dynamic configuration engine for Asterisk

Other

64 stars 25 forks source link

Re-render is not triggered on secret value change #8

Open vkruoso opened 4 years ago

vkruoso commented 4 years ago

When changing the zip file in the secret that is mounted in the asterisk-config volume, it does not detect it and re-renders the template. This makes it hard to have dynamic configuration except from the already implemented reactivity based on the network changes.

Possible solutions:

Watch the file system for changes on the mounted secret and re-renders the full config and overwrite the original (this have a drawback of taking some time, depending on the cluster, to update due to https://kubernetes.io/docs/concepts/configuration/secret/#mounted-secrets-are-updated-automatically)
Watch for secret changes via k8s API and trigger the re-render the same way

Ulexus commented 4 years ago

Yeah, I've been meaning to do that before. It should be watching the Secret for changes.

vkruoso commented 4 years ago

Given that the mounted secret may take some time to take effect, watching the secret only have value if the we have RBAC that allow the process to read the secret contents. Do you think this is a good approach or the file system approach is better despite the delay?

I'm willing to open a PR for it.

Ulexus commented 4 years ago

For me, I'd just exit the execution of asterisk-config and let kubernetes recreate the container with the updated Secret. That, then, would not require any special RBAC. That is, admittedly, somewhat ugly, since that will register a restart of the container.

An optional RBAC for reading the Secret would be fine, to avoid the restart, but I wouldn't want it to be a mandatory default.

Please feel free to PR, thanks!

vkruoso commented 4 years ago

Did hack it real quick here the restart when the file changes. It works great. This also simplifies something: to deterministically re-render the config, we need to start from scratch: actually removing the current asterisk config from the exportDir, extract everything again and so on. I'll keep it like this for a while, but will attempt to implement the watcher later. Thanks for the hack idea.

vkruoso commented 4 years ago

I've got an implementation, but I'm is struggling with the kubetemplate dependency on github.com/ericchiang/k8s. That is an old library (and looks like it is not maintained anymore) and the watcher I've implemented uses the standard kubernetes client-go library. Do you feel it would be better to make the change on kubetemplate or adapt to use the old watcher?

vkruoso commented 4 years ago

I've got an implementation, but I'm is struggling with the kubetemplate dependency on github.com/ericchiang/k8s. That is an old library (and looks like it is not maintained anymore) and the watcher I've implemented uses the standard kubernetes client-go library. Do you feel it would be better to make the change on kubetemplate or adapt to use the old watcher?

I went on the easier path: using kubetemplate. I just had to add support to watch/get a binary secret, as the current method only works for strings. Please let me know if I'm missing something. I'm already using this internally.

Ulexus commented 4 years ago

Yeah, I need to get off the ericciang/k8s client. Before the client was split from the kubernetes core, it was much faster and lighter weight. Now, there isn't really a need for it. I'll make another issue to handle that.

vkruoso commented 3 years ago

Hey! Long time. Do you plan to have the PR merged or should we take another approach?

vkruoso commented 2 years ago

@Ulexus I've updated the PR #9 based on the most recent improvements. It has been running for a few days without any issues for us. Also, the #12 is a requirement as the current docker build it not working (at least for me).