asterisk-config icon indicating copy to clipboard operation
asterisk-config copied to clipboard

Re-render is not triggered on secret value change

Open vkruoso opened this issue 4 years ago • 9 comments

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

vkruoso avatar Aug 24 '20 22:08 vkruoso

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

Ulexus avatar Aug 24 '20 22:08 Ulexus

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.

vkruoso avatar Aug 26 '20 12:08 vkruoso

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!

Ulexus avatar Aug 26 '20 14:08 Ulexus

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 avatar Aug 26 '20 20:08 vkruoso

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 avatar Oct 14 '20 18:10 vkruoso

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.

vkruoso avatar Oct 14 '20 21:10 vkruoso

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.

Ulexus avatar Oct 22 '20 16:10 Ulexus

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

vkruoso avatar Mar 25 '21 03:03 vkruoso

@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).

vkruoso avatar Jun 02 '22 17:06 vkruoso