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

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

Open
vkruoso opened this issue Aug 24, 2020 · 9 comments · May be fixed by #9
Open

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

vkruoso opened this issue Aug 24, 2020 · 9 comments · May be fixed by #9

Comments

@vkruoso
Copy link
Contributor

vkruoso commented Aug 24, 2020

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:

@Ulexus
Copy link
Member

Ulexus commented Aug 24, 2020

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

@vkruoso
Copy link
Contributor Author

vkruoso commented Aug 26, 2020

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
Copy link
Member

Ulexus commented Aug 26, 2020

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
Copy link
Contributor Author

vkruoso commented Aug 26, 2020

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
Copy link
Contributor Author

vkruoso commented Oct 14, 2020

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 added a commit to vkruoso/kubetemplate that referenced this issue Oct 14, 2020
This allows the engine to be used to watch and retrieve data
from binary secrets. The current secret implementation assumes
that the value is a string and hence does not work with binary
data.

Besides the fact that this is a template engine and binary data
make very little sense, this engine can be used as a generic
k8s watcher.

Related to CyCoreSystems/asterisk-config#8.

Signed-off-by: Vinicius Ruoso <[email protected]>
vkruoso added a commit to vkruoso/asterisk-config that referenced this issue Oct 14, 2020
The Asterisk config is provided by a kubernetes secret. We are
adding support re-render the templates when a secret is updated.
This allow us to react much faster to configuration changes.

Closes CyCoreSystems#8.

Signed-off-by: Vinicius Ruoso <[email protected]>
vkruoso added a commit to vkruoso/asterisk-config that referenced this issue Oct 14, 2020
The Asterisk config is provided by a kubernetes secret. We are
adding support re-render the templates when a secret is updated.
This allow us to react much faster to configuration changes.

Closes CyCoreSystems#8.

Signed-off-by: Vinicius Ruoso <[email protected]>
vkruoso added a commit to vkruoso/asterisk-config that referenced this issue Oct 14, 2020
The Asterisk config is provided by a kubernetes secret. We are
adding support re-render the templates when a secret is updated.
This allow us to react much faster to configuration changes.

Closes CyCoreSystems#8.

Signed-off-by: Vinicius Ruoso <[email protected]>
@vkruoso vkruoso linked a pull request Oct 14, 2020 that will close this issue
@vkruoso
Copy link
Contributor Author

vkruoso commented Oct 14, 2020

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 added a commit to vkruoso/asterisk-config that referenced this issue Oct 14, 2020
The Asterisk config is provided by a kubernetes secret. We are
adding support re-render the templates when a secret is updated.
This allow us to react much faster to configuration changes.

Closes CyCoreSystems#8.

Signed-off-by: Vinicius Ruoso <[email protected]>
@Ulexus
Copy link
Member

Ulexus commented Oct 22, 2020

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
Copy link
Contributor Author

vkruoso commented Mar 25, 2021

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

vkruoso added a commit to vkruoso/asterisk-config that referenced this issue Jun 2, 2022
The Asterisk config is provided by a kubernetes secret. We are
adding support re-render the templates when a secret is updated.
This allow us to react much faster to configuration changes.

Closes CyCoreSystems#8.

Signed-off-by: Vinicius Ruoso <[email protected]>
@vkruoso
Copy link
Contributor Author

vkruoso commented Jun 2, 2022

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

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

Successfully merging a pull request may close this issue.

2 participants