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
Use bearer token file and periodically recreate client #328
Use bearer token file and periodically recreate client #328
Conversation
I don't think any of this change is required other then to bump the library and modify it to use bearer token path instead of the token directly. Not This lib depends upon https://github.com/ManageIQ/kubeclient which is where ManageIQ/kubeclient#532 was merged in 2022-Feb-13. Actually, it looks like we could simply bump the dependency and it would already reload the bearer token from the path |
Closing as I believe the change merged here #330 addresses the issue |
No, kubeclient only implemented token refresh on master branch, on top of None of the 4.y.z releases — in fact no released version of kubeclient — have token refresh. Options if you want it soon:
|
end | ||
|
||
log.debug 'Creating K8S client' | ||
@client = Kubeclient::Client.new( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since it is possible this method executes periodically on a timer, seems we may require some mutex to ensure we are not recreating the client in the middle of trying to use it to access the API server
token_read_period = 60 | ||
while true | ||
log.debug "Sleeping for #{token_read_period}" | ||
sleep(token_read_period) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know the answer but asking the question: is there a need to "pass" instead of or in conjunction with "sleep" or ar they the same
@@ -161,6 +161,61 @@ def initialize | |||
@prev_time = Time.now | |||
end | |||
|
|||
def create_k8s_client |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider moving all this code into a module to reduce the complexity of the main filter file
begin | ||
@client.api_valid? | ||
rescue KubeException => e | ||
raise Fluent::ConfigError, "Invalid Kubernetes API #{@apiVersion} endpoint #{@kubernetes_url}: #{e.message}" | ||
end | ||
|
||
if present?(@bearer_token_file) | ||
k8s_client_thread = Thread.new(self, &:periodically_recreate_k8s_client) | ||
k8s_client_thread.abort_on_exception = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this abort the entire fluent process? I have concerns of some failure where fluent will continue to process messages but will render this plugin completely unusable with no immediate indicator for admins
Here is another way of doing it, reactively refresh the token when you get an Unauthorized error: #337 |
Closing in favor of #337 |
Recreate k8s client periodically.
As discussed in #327 this seems a better approach than waiting for the token to expire.
NB: I couldn't reopen #326 hence this new PR (sorry for the mess)
Fixes #323