-
Notifications
You must be signed in to change notification settings - Fork 21
fix: create lazy OpenShift context to avoid check timeouts (#865) #866
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
base: main
Are you sure you want to change the base?
Conversation
6c87476
to
f8af258
Compare
* @see [issue 865](https://github.com/redhat-developer/intellij-kubernetes/issues/865) | ||
* @see ClientAdapter.toOpenShift | ||
**/ | ||
return KubeClientAdapter(kubeClient) |
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.
this is the core of the fix: dont check if the cluster is OpenShift and create a Kubernetes- or OpenShift client. This check is costly and may timeout if the cluster is not reachable.
Create a Kubernetes client instead. It can always be adapted to an OpenShift client when needed.
client as KubeClientAdapter | ||
) | ||
} | ||
return LazyOpenShiftContext(currentContext, modelChange, client as KubeClientAdapter) |
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.
this is the core of the fix: dont check if the cluster is OpenShift and create a Kubernetes- or OpenShift context. This check is costly and may timeout if the cluster is not reachable.
Create a Kubernetes context that async tries to upgrade to an OpenShift context instead.
config.connectionTimeout = TIMEOUT_CONNECTION | ||
config.requestTimeout = TIMEOUT_REQUEST | ||
config.watchReconnectLimit = LIMIT_RECONNECT | ||
config |
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.
This is an aspect of the fix: impose timeouts for connecting, requesting and reconnecting.
} | ||
|
||
private fun createOpenShiftDelegate() { | ||
if (client.canAdaptToOpenShift()) { |
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.
this is the core fix: a context that starts as a Kubernetes context and async tries to create and use an OpenShift context.
Signed-off-by: Andre Dietisheim <[email protected]>
Signed-off-by: Andre Dietisheim <[email protected]>
d5e8a39
to
83a3b6b
Compare
src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/context/LazyOpenShiftContext.kt
Outdated
Show resolved
Hide resolved
…veloper#865) Signed-off-by: Andre Dietisheim <[email protected]>
|
@sbouchet: handing this to @msivasubramaniaan to unload you so that you can please review #858 |
fixes #865