Skip to content

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ package com.redhat.devtools.intellij.kubernetes.model
import com.intellij.openapi.application.ApplicationManager
import com.intellij.openapi.diagnostic.logger
import com.redhat.devtools.intellij.common.utils.ConfigWatcher
import com.redhat.devtools.intellij.common.utils.ExecHelper
import com.redhat.devtools.intellij.kubernetes.model.client.ClientAdapter
import com.redhat.devtools.intellij.kubernetes.model.client.ClientConfig
import com.redhat.devtools.intellij.kubernetes.model.context.Context
Expand Down Expand Up @@ -77,7 +76,7 @@ interface IAllContexts {

open class AllContexts(
private val contextFactory: (ClientAdapter<out KubernetesClient>, IResourceModelObservable) -> IActiveContext<out HasMetadata, out KubernetesClient>? =
IActiveContext.Factory::create,
IActiveContext.Factory::createLazyOpenShift,
private val modelChange: IResourceModelObservable,
private val clientFactory: (
namespace: String?,
Expand Down Expand Up @@ -223,7 +222,7 @@ open class AllContexts(
}

protected open fun reportTelemetry(context: IActiveContext<out HasMetadata, out KubernetesClient>) {
ExecHelper.submit {
runAsync {
val telemetry = TelemetryService.instance.action(NAME_PREFIX_CONTEXT + "use")
.property(PROP_IS_OPENSHIFT, context.isOpenShift().toString())
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ open class ResourceModel : IResourceModel {
}

protected open val allContexts: IAllContexts by lazy {
AllContexts(IActiveContext.Factory::create, modelChange)
AllContexts(IActiveContext.Factory::createLazyOpenShift, modelChange)
}

override fun setCurrentContext(context: IContext) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ open class OSClientAdapter(client: OpenShiftClient, private val kubeClient: Kube
ClientConfig(kubeClient.configuration)
}

override fun toOpenShift(): OSClientAdapter {
return this
}

override fun isOpenShift(): Boolean {
return true
}
Expand All @@ -52,17 +56,33 @@ open class KubeClientAdapter(client: KubernetesClient) :
override fun isOpenShift(): Boolean {
return false
}

override fun toOpenShift(): OSClientAdapter {
val kubeClient = get()
val osClient = kubeClient.adapt(NamespacedOpenShiftClient::class.java)
return OSClientAdapter(osClient, kubeClient)
}
}

abstract class ClientAdapter<C : KubernetesClient>(private val fabric8Client: C) {

companion object Factory {

const val TIMEOUT_CONNECTION = 5000
const val TIMEOUT_REQUEST = 5000
const val LIMIT_RECONNECT = 2

fun create(
namespace: String? = null,
context: String? = null,
clientBuilder: KubernetesClientBuilder? = null,
createConfig: (context: String?) -> Config = { context -> Config.autoConfigure(context) },
createConfig: (context: String?) -> Config = { context ->
val config = Config.autoConfigure(context)
config.connectionTimeout = TIMEOUT_CONNECTION
config.requestTimeout = TIMEOUT_REQUEST
config.watchReconnectLimit = LIMIT_RECONNECT
config
Copy link
Collaborator Author

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.

},
externalTrustManagerProvider: ((toIntegrate: List<X509ExtendedTrustManager>) -> X509TrustManager)? = null
): ClientAdapter<out KubernetesClient> {
KubeConfigEnvValue.copyToSystem()
Expand All @@ -76,12 +96,14 @@ abstract class ClientAdapter<C : KubernetesClient>(private val fabric8Client: C)
setSslContext(httpClientBuilder, config, trustManager)
}
.build()
return if (ClusterHelper.isOpenShift(kubeClient)) {
val osClient = kubeClient.adapt(NamespacedOpenShiftClient::class.java)
OSClientAdapter(osClient, kubeClient)
} else {
KubeClientAdapter(kubeClient)
}
/**
* Always create kubernetes client.
* Upgrade existing client to OpenShift only async bcs checking if cluster is OpenShift is costly
* and may timeout if cluster is not reachable.
* @see [issue 865](https://github.com/redhat-developer/intellij-kubernetes/issues/865)
* @see ClientAdapter.toOpenShift
**/
return KubeClientAdapter(kubeClient)
Copy link
Collaborator Author

@adietish adietish Apr 24, 2025

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.

}

private fun setSslContext(
Expand Down Expand Up @@ -114,6 +136,8 @@ abstract class ClientAdapter<C : KubernetesClient>(private val fabric8Client: C)
ClientConfig(fabric8Client.configuration)
}

abstract fun toOpenShift(): OSClientAdapter

abstract fun isOpenShift(): Boolean

fun get(): C {
Expand Down Expand Up @@ -145,6 +169,10 @@ abstract class ClientAdapter<C : KubernetesClient>(private val fabric8Client: C)
}
}

fun canAdaptToOpenShift(): Boolean {
return ClusterHelper.isOpenShift(fabric8Client)
}

open fun close() {
clients.values.forEach{ it.close() }
fabric8Client.close()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,9 @@ import java.net.URL

abstract class ActiveContext<N : HasMetadata, C : KubernetesClient>(
context: NamedContext,
private val modelChange: IResourceModelObservable,
protected val modelChange: IResourceModelObservable,
val client: ClientAdapter<out C>,
protected open val dashboard: IDashboard,
protected open val dashboard: IDashboard? = null,
private var singleResourceOperator: NonCachingSingleResourceOperator = NonCachingSingleResourceOperator(client),
) : Context(context), IActiveContext<N, C> {

Expand All @@ -72,8 +72,6 @@ abstract class ActiveContext<N : HasMetadata, C : KubernetesClient>(
ClusterHelper.getClusterInfo(client.get())
}

protected abstract val namespaceKind : ResourceKind<N>

private val extensionName: ExtensionPointName<IResourceOperatorFactory<HasMetadata, KubernetesClient, IResourceOperator<HasMetadata>>> =
ExtensionPointName("com.redhat.devtools.intellij.kubernetes.resourceOperators")

Expand Down Expand Up @@ -307,7 +305,7 @@ abstract class ActiveContext<N : HasMetadata, C : KubernetesClient>(
override fun stopWatch(kind: ResourceKind<out HasMetadata>) {
logger<ActiveContext<*, *>>().debug("Stop watching $kind resources.")
watch.stopWatch(kind)
// don't notify invalidation change because this would cause UI to reload
// don't notify invalidation change because this would cause the UI to reload
// and therefore to repopulate the cache immediately.
// Any resource operation that eventually happens while the watch is not active would cause the cache
// to become out-of-sync and it would therefore return invalid resources when asked to do so
Expand Down Expand Up @@ -489,27 +487,32 @@ abstract class ActiveContext<N : HasMetadata, C : KubernetesClient>(
override fun close() {
logger<ActiveContext<*, *>>().debug("Closing context $name.")
watch.close()
dashboard.close()
dashboard?.close()
}

private fun <P: IResourceOperator<out HasMetadata>> getAllResourceOperators(type: Class<P>)
: MutableMap<ResourceKind<out HasMetadata>, P> {
val operators = mutableMapOf<ResourceKind<out HasMetadata>, P>()
operators.putAll(
getInternalResourceOperators(client)
getInternalResourceOperators()
.filterIsInstance(type)
.associateBy { it.kind })
operators.putAll(
getExtensionResourceOperators(client)
getExtensionResourceOperators()
.filterIsInstance(type)
.associateBy { it.kind })
return operators
}

protected abstract fun getInternalResourceOperators(client: ClientAdapter<out C>): List<IResourceOperator<out HasMetadata>>
abstract override fun getInternalResourceOperators(): List<IResourceOperator<out HasMetadata>>

protected open fun getExtensionResourceOperators(client: ClientAdapter<out C>): List<IResourceOperator<out HasMetadata>> {
protected open fun getExtensionResourceOperators(): List<IResourceOperator<out HasMetadata>> {
return extensionName.extensionList
.map { it.create(client.get()) }
}

override fun getDashboardUrl(): String? {
return dashboard?.get()
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ interface IContext {
val namespace: String?
}

open class Context(private val context: NamedContext): IContext {
open class Context(protected val context: NamedContext): IContext {
override val active: Boolean = false
override val name: String?
get() = context.name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import com.redhat.devtools.intellij.kubernetes.model.IResourceModelObservable
import com.redhat.devtools.intellij.kubernetes.model.client.ClientAdapter
import com.redhat.devtools.intellij.kubernetes.model.client.KubeClientAdapter
import com.redhat.devtools.intellij.kubernetes.model.client.OSClientAdapter
import com.redhat.devtools.intellij.kubernetes.model.resource.IResourceOperator
import com.redhat.devtools.intellij.kubernetes.model.resource.ResourceKind
import com.redhat.devtools.intellij.kubernetes.model.resource.kubernetes.KubernetesReplicas.Replicator
import io.fabric8.kubernetes.api.model.GenericKubernetesResource
Expand All @@ -23,30 +24,29 @@ import io.fabric8.kubernetes.api.model.apiextensions.v1.CustomResourceDefinition
import io.fabric8.kubernetes.client.KubernetesClient
import io.fabric8.kubernetes.client.Watch
import io.fabric8.kubernetes.client.Watcher
import io.fabric8.openshift.api.model.Project
import io.fabric8.openshift.client.OpenShiftClient
import java.net.URL

interface IActiveContext<N: HasMetadata, C: KubernetesClient>: IContext {

companion object Factory {
fun create(
fun createLazyOpenShift(
client: ClientAdapter<out KubernetesClient>,
observable: IResourceModelObservable
modelChange: IResourceModelObservable
): IActiveContext<out HasMetadata, out KubernetesClient>? {
val currentContext = client.config.currentContext ?: return null
return if (client.isOpenShift()) {
OpenShiftContext(
currentContext,
observable,
client as OSClientAdapter
)
} else {
KubernetesContext(
currentContext,
observable,
client as KubeClientAdapter
)
}
return LazyOpenShiftContext(currentContext, modelChange, client as KubeClientAdapter)
Copy link
Collaborator Author

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.

}

fun createOpenShift(
client: OSClientAdapter,
modelChange: IResourceModelObservable
): IActiveContext<Project, OpenShiftClient>? {
val currentContext = client.config.currentContext ?: return null
return OpenShiftContext(currentContext, modelChange, client)
}

}

/**
Expand All @@ -65,6 +65,8 @@ interface IActiveContext<N: HasMetadata, C: KubernetesClient>: IContext {
}
}

val namespaceKind : ResourceKind<out HasMetadata>

/**
* The master url for this context. This is the url of the cluster for this context.
*/
Expand All @@ -75,6 +77,16 @@ interface IActiveContext<N: HasMetadata, C: KubernetesClient>: IContext {
*/
val version: ClusterInfo

/**
* Returns the list of internal [IResourceOperator]s that are available in this context.
* [IResourceOperator]s that are contributed by registrations to the extension point are not included.
*
* @return the list of [IResourceOperator]s that are available in this context.
* @see IResourceOperator
* @see com.redhat.devtools.intellij.kubernetes.model.context.ActiveContext.getExtensionResourceOperators
*/
fun getInternalResourceOperators(): List<IResourceOperator<out HasMetadata>>

/**
* Returns {@code true} if this context is an OpenShift context. This is true for context with an OpenShift cluster.
*/
Expand Down Expand Up @@ -129,7 +141,7 @@ interface IActiveContext<N: HasMetadata, C: KubernetesClient>: IContext {
fun getAllResources(definition: CustomResourceDefinition): Collection<GenericKubernetesResource>

/**
* Returns the latest version of the given resource from cluster. Returns `null` if none was found.
* Returns the latest version of the given resource from the cluster. Returns `null` if none was found.
*
* @param resource which is to be requested from cluster
*
Expand Down Expand Up @@ -265,7 +277,7 @@ interface IActiveContext<N: HasMetadata, C: KubernetesClient>: IContext {
/**
* Notifies the context that the given resource was replaced in the cluster.
* Replaces the resource with the given new version if it exists.
* Does nothing otherwiese.
* Does nothing otherwise.
*
*
* @param resource the new (version) of the resource
Expand All @@ -279,7 +291,7 @@ interface IActiveContext<N: HasMetadata, C: KubernetesClient>: IContext {
*
* @return the url of the Dashboard for this context
*/
fun getDashboardUrl(): String
fun getDashboardUrl(): String?

/**
* Closes and disposes this context.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ open class KubernetesContext(
context: NamedContext,
modelChange: IResourceModelObservable,
client: KubeClientAdapter,
) : ActiveContext<Namespace, KubernetesClient>(
) : ActiveContext<HasMetadata, KubernetesClient>(
context,
modelChange,
client,
Expand All @@ -43,7 +43,7 @@ open class KubernetesContext(
)
) {

override val namespaceKind : ResourceKind<Namespace> = NamespacesOperator.KIND
override val namespaceKind : ResourceKind<out HasMetadata> = NamespacesOperator.KIND

private val replicasOperator = KubernetesReplicas(
NonCachingSingleResourceOperator(client),
Expand All @@ -54,12 +54,14 @@ open class KubernetesContext(
}
)

override fun getInternalResourceOperators(client: ClientAdapter<out KubernetesClient>)
override fun getInternalResourceOperators()
: List<IResourceOperator<out HasMetadata>> {
return OperatorFactory.createKubernetes(client)
}

override fun isOpenShift() = false
override fun isOpenShift(): Boolean {
return false
}

override fun setReplicas(replicas: Int, replicator: Replicator) {
replicasOperator.set(replicas, replicator)
Expand All @@ -69,9 +71,4 @@ open class KubernetesContext(
return replicasOperator.get(resource)
}

override fun getDashboardUrl(): String {
return dashboard.get()
}


}
Loading
Loading