From f501828c76dcfdde7ca2ac773ccb33126191dd04 Mon Sep 17 00:00:00 2001 From: Andre Dietisheim Date: Mon, 24 Jul 2023 14:42:17 +0200 Subject: [PATCH 1/2] correctly synchronize accessing client when config changes (#640) Signed-off-by: Andre Dietisheim --- .../intellij/kubernetes/model/AllContexts.kt | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/AllContexts.kt b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/AllContexts.kt index 99e9d64c7..e75936b44 100644 --- a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/AllContexts.kt +++ b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/AllContexts.kt @@ -247,14 +247,16 @@ open class AllContexts( } protected open fun onKubeConfigChanged(fileConfig: io.fabric8.kubernetes.api.model.Config) { - val client = client.get() ?: return - val clientConfig = client.config.configuration - if (ConfigHelper.areEqual(fileConfig, clientConfig)) { - return + synchronized(this) { + val client = client.get() ?: return + val clientConfig = client.config.configuration + if (ConfigHelper.areEqual(fileConfig, clientConfig)) { + return + } + this.client.reset() // create new client when accessed + client.close() + refresh() } - client.close() - this.client.reset() // create new client when accessed - refresh() } /** for testing purposes */ From c9bd27e40b74ad278309ff2c35815986aecf7373 Mon Sep 17 00:00:00 2001 From: Andre Dietisheim Date: Mon, 24 Jul 2023 18:54:35 +0200 Subject: [PATCH 2/2] wait for save of kubeconfig to finish (#640) Signed-off-by: Andre Dietisheim --- .../intellij/kubernetes/model/AllContexts.kt | 2 +- .../kubernetes/model/client/ClientConfig.kt | 59 ++++++++++--------- .../model/client/ClientConfigTest.kt | 20 +++---- .../intellij/kubernetes/model/mocks/Mocks.kt | 3 + 4 files changed, 43 insertions(+), 41 deletions(-) diff --git a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/AllContexts.kt b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/AllContexts.kt index e75936b44..d853c12f1 100644 --- a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/AllContexts.kt +++ b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/AllContexts.kt @@ -143,6 +143,7 @@ open class AllContexts( this.client.get() ) this.current?.close() + newClient.config.save().join() all.clear() // causes reload of all contexts when accessed afterwards val newCurrent = this.current // gets new current from all if (toWatch != null) { @@ -197,7 +198,6 @@ open class AllContexts( private fun replaceClient(new: ClientAdapter, old: ClientAdapter?) : ClientAdapter { old?.close() - new.config.save() this.client.set(new) return new } diff --git a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientConfig.kt b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientConfig.kt index 329045b98..7320470b5 100644 --- a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientConfig.kt +++ b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientConfig.kt @@ -10,19 +10,21 @@ ******************************************************************************/ package com.redhat.devtools.intellij.kubernetes.model.client -import com.intellij.openapi.application.ApplicationManager import com.redhat.devtools.intellij.common.utils.ConfigHelper +import com.redhat.devtools.intellij.kubernetes.CompletableFutureUtils.PLATFORM_EXECUTOR import io.fabric8.kubernetes.api.model.Context import io.fabric8.kubernetes.api.model.NamedContext import io.fabric8.kubernetes.client.Client import io.fabric8.kubernetes.client.Config import io.fabric8.kubernetes.client.internal.KubeConfigUtils +import java.util.concurrent.CompletableFuture +import java.util.concurrent.Executor /** * An adapter to access [io.fabric8.kubernetes.client.Config]. * It also saves the kube config [KubeConfigUtils] when it changes the client config. */ -open class ClientConfig(private val client: Client) { +open class ClientConfig(private val client: Client, private val executor: Executor = PLATFORM_EXECUTOR) { open var currentContext: NamedContext? get() { @@ -45,26 +47,33 @@ open class ClientConfig(private val client: Client) { KubeConfigAdapter() } - fun save() { - runAsync { - if (!kubeConfig.exists()) { - return@runAsync - } - val fromFile = kubeConfig.load() ?: return@runAsync - val currentContextInFile = KubeConfigUtils.getCurrentContext(fromFile) - if (setCurrentContext( - currentContext, - currentContextInFile, - fromFile - ).or( // no short-circuit - setCurrentNamespace( - currentContext?.context, - currentContextInFile?.context) - ) - ) { - kubeConfig.save(fromFile) - } - } + fun save(): CompletableFuture { + return CompletableFuture.supplyAsync( + { + if (!kubeConfig.exists()) { + return@supplyAsync false + } + val fromFile = kubeConfig.load() ?: return@supplyAsync false + val currentContextInFile = KubeConfigUtils.getCurrentContext(fromFile) + if (setCurrentContext( + currentContext, + currentContextInFile, + fromFile + ).or( // no short-circuit + setCurrentNamespace( + currentContext?.context, + currentContextInFile?.context + ) + ) + ) { + kubeConfig.save(fromFile) + return@supplyAsync true + } else { + return@supplyAsync false + } + }, + executor + ) } private fun setCurrentContext( @@ -111,10 +120,4 @@ open class ClientConfig(private val client: Client) { fun isCurrent(context: NamedContext): Boolean { return context == currentContext } - - /** for testing purposes */ - protected open fun runAsync(runnable: () -> Unit) { - ApplicationManager.getApplication().executeOnPooledThread(runnable) - } - } \ No newline at end of file diff --git a/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientConfigTest.kt b/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientConfigTest.kt index a0a37f5ac..659e14565 100644 --- a/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientConfigTest.kt +++ b/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientConfigTest.kt @@ -88,7 +88,7 @@ class ClientConfigTest { doReturn(false) .whenever(kubeConfig).exists() // when - clientConfig.save() + clientConfig.save().join() // then verify(kubeConfig, never()).save(any()) } @@ -97,7 +97,7 @@ class ClientConfigTest { fun `#save should NOT save if kubeConfig has same current context same namespace and same current context as client config`() { // given // when - clientConfig.save() + clientConfig.save().join() // then verify(kubeConfig, never()).save(any()) } @@ -108,7 +108,7 @@ class ClientConfigTest { f8clientConfig.currentContext.name = namedContext3.name assertThat(f8kubeConfig.currentContext).isNotEqualTo(f8clientConfig.currentContext.name) // when - clientConfig.save() + clientConfig.save().join() // then verify(kubeConfig).save(any()) } @@ -126,7 +126,7 @@ class ClientConfigTest { newAllContexts.add(newCurrentContext) f8kubeConfig.contexts = newAllContexts // when - clientConfig.save() + clientConfig.save().join() // then verify(kubeConfig).save(any()) } @@ -140,7 +140,7 @@ class ClientConfigTest { assertThat(KubeConfigUtils.getCurrentContext(f8kubeConfig)) .isNotEqualTo(f8clientConfig.currentContext) // when - clientConfig.save() + clientConfig.save().join() // then verify(kubeConfig).save(argThat { this.currentContext == newCurrentContext.name @@ -159,7 +159,7 @@ class ClientConfigTest { assertThat(KubeConfigUtils.getCurrentContext(f8kubeConfig).context.namespace) .isNotEqualTo(f8clientConfig.currentContext.context.namespace) // when - clientConfig.save() + clientConfig.save().join() // then verify(kubeConfig).save(argThat { this.currentContext == this@ClientConfigTest.currentContext.name @@ -211,10 +211,6 @@ class ClientConfigTest { .build() } - private class TestableClientConfig(client: Client, override val kubeConfig: KubeConfigAdapter) : ClientConfig(client) { - override fun runAsync(runnable: () -> Unit) { - // dont use jetbrains application threadpool - runnable.invoke() - } - } + private class TestableClientConfig(client: Client, override val kubeConfig: KubeConfigAdapter) + : ClientConfig(client, { it.run() }) } diff --git a/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/mocks/Mocks.kt b/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/mocks/Mocks.kt index a5327916a..f52a97ab9 100644 --- a/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/mocks/Mocks.kt +++ b/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/mocks/Mocks.kt @@ -38,6 +38,7 @@ import io.fabric8.kubernetes.client.Watch import io.fabric8.kubernetes.client.Watcher import io.fabric8.kubernetes.client.dsl.ExecWatch import io.fabric8.kubernetes.client.dsl.LogWatch +import java.util.concurrent.CompletableFuture import org.mockito.Mockito object Mocks { @@ -210,6 +211,7 @@ object Mocks { allContexts: List, configuration: io.fabric8.kubernetes.client.Config = mock() ): ClientConfig { + val saveFuture: CompletableFuture = mock() return mock { on { this.currentContext } doReturn currentContext on { isCurrent(any()) } doAnswer { invocation -> @@ -217,6 +219,7 @@ object Mocks { } on { this.allContexts } doReturn allContexts on { this.configuration } doReturn configuration + on { this.save() } doReturn saveFuture } }