From ce0c03e92d6f1dc8131cb7dc1597e0870be7bbf1 Mon Sep 17 00:00:00 2001 From: Victor Rubezhny Date: Tue, 14 Jul 2026 00:18:55 +0200 Subject: [PATCH 1/3] fix: cancel button hangs during workspace IDE connection (crw-11729) Pass checkCancelled through getStatus/exec, add cancellation monitor that closes streams to unblock reads, use indicator modality state for dialogs, guard exec with pod-running check, and shut down isolated OkHttp clients on cleanup. Issue: https://redhat.atlassian.net/browse/CRW-11796 Signed-off-by: Victor Rubezhny Assisted-By: Claude Opus 4.6 --- .../devtools/gateway/DevSpacesConnection.kt | 7 +-- .../gateway/DevSpacesConnectionProvider.kt | 3 +- .../gateway/openshift/DevWorkspacePods.kt | 54 ++++++++++++++++--- .../gateway/server/RemoteIDEServer.kt | 4 ++ .../devtools/gateway/view/ui/Dialogs.kt | 19 ++++--- .../devworkspace/DevWorkspaceRestartTest.kt | 10 ++-- 6 files changed, 75 insertions(+), 22 deletions(-) diff --git a/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesConnection.kt b/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesConnection.kt index 16e8495a..6fb78c74 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesConnection.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesConnection.kt @@ -46,6 +46,7 @@ class DevSpacesConnection(private val devSpacesContext: DevSpacesContext) { onDevWorkspaceStopped: () -> Unit, onProgress: ((value: ProgressCountdown.ProgressEvent) -> Unit)? = null, checkCancelled: (() -> Unit)? = null, + modalityState: ModalityState? = null, registerRestartWatcher: Boolean? = true ): ThinClientHandle { val workspace = devSpacesContext.devWorkspace @@ -60,7 +61,7 @@ class DevSpacesConnection(private val devSpacesContext: DevSpacesContext) { while (!remoteIdeServerStatus.isReady) { checkCancelled?.invoke() onProgress?.invoke(ProgressCountdown.ProgressEvent( - message = "Waiting for the workspace to get ready...", + message = "Waiting for the workspace to get started...", countdownSeconds = DevWorkspaces.RUNNING_TIMEOUT)) DevWorkspaces(devSpacesContext.client) @@ -76,7 +77,7 @@ class DevSpacesConnection(private val devSpacesContext: DevSpacesContext) { remoteIdeServer = RemoteIDEServer(devSpacesContext) remoteIdeServerStatus = runCatching { - remoteIdeServer.apply { waitServerReady(checkCancelled) }.getStatus() + remoteIdeServer.apply { waitServerReady(checkCancelled) }.getStatus(checkCancelled) }.getOrElse { e -> if (e.isCancellationException()) throw e RemoteIDEServerStatus.empty() @@ -84,7 +85,7 @@ class DevSpacesConnection(private val devSpacesContext: DevSpacesContext) { checkCancelled?.invoke() if (!remoteIdeServerStatus.isReady) { - val restartWorkspace = Dialogs.ideNotResponding() + val restartWorkspace = Dialogs.ideNotResponding(modalityState) if (restartWorkspace) { // User chose "Restart Pod": stop the Pod and try starting from scratch diff --git a/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesConnectionProvider.kt b/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesConnectionProvider.kt index 0ead8bf8..3be89ce9 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesConnectionProvider.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesConnectionProvider.kt @@ -246,7 +246,8 @@ class DevSpacesConnectionProvider : GatewayConnectionProvider { }, checkCancelled = { if (indicator.isCanceled) throw CancellationException("User cancelled the operation") - } + }, + modalityState = indicator.modalityState ) } catch (e: ApiException) { throw e.toWorkspaceException( diff --git a/src/main/kotlin/com/redhat/devtools/gateway/openshift/DevWorkspacePods.kt b/src/main/kotlin/com/redhat/devtools/gateway/openshift/DevWorkspacePods.kt index 5deeaea3..4bd8a68f 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/openshift/DevWorkspacePods.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/openshift/DevWorkspacePods.kt @@ -38,6 +38,18 @@ class DevWorkspacePods(private val client: ApiClient) { private val logger = logger() + // Bounded by OkHttp's default 10s connect/read/write timeouts on the shared client + fun isPodRunning(pod: V1Pod): Boolean { + val name = pod.metadata?.name ?: return false + val namespace = pod.metadata?.namespace ?: return false + return try { + val current = CoreV1Api(client).readNamespacedPod(name, namespace).execute() + current.status?.phase == "Running" + } catch (_: Exception) { + false + } + } + // Example: // https://github.com/kubernetes-client/java/blob/master/examples/examples-release-latest/src/main/java/io/kubernetes/client/examples/ExecExample.java suspend fun exec( @@ -79,16 +91,34 @@ class DevWorkspacePods(private val client: ApiClient) { stdoutJob = scope.launch { readStream(io.stdout, stdout, checkCancelled) } stderrJob = scope.launch { readStream(io.stderr, stderr, checkCancelled) } + if (checkCancelled != null) { + scope.launch { + try { + while (isActive) { + checkCancelled.invoke() + delay(200) + } + } catch (_: Throwable) { + runCatching { io.stdout.close() } + runCatching { io.stderr.close() } + } + } + } + scope.launch { try { listOfNotNull(stdoutJob, stderrJob).joinAll() + checkCancelled?.invoke() closed.await() checkCancelled?.invoke() - cont.resume(stdout.toString()) + if (cont.isActive) cont.resume(stdout.toString()) } catch (e: Throwable) { if (e.isCancellationException()) cont.cancel(e) else if (cont.isActive) cont.resumeWithException(e) + } finally { + scope.cancel() + shutdownExecClient(execClientApi) } } }, @@ -97,6 +127,7 @@ class DevWorkspacePods(private val client: ApiClient) { }, onError = { err, _ -> closed.complete(Unit) + shutdownExecClient(execClientApi) cont.resumeWithException(err) }, timeoutMs = timeout * 1000, @@ -111,22 +142,33 @@ class DevWorkspacePods(private val client: ApiClient) { execHandle.future.cancel(true) } catch (_: Throwable) {} scope.cancel() + shutdownExecClient(execClientApi) } } catch (e: Exception) { + shutdownExecClient(execClientApi) if (cont.isActive) cont.resumeWithException(e) } } + private fun shutdownExecClient(client: ApiClient) { + runCatching { client.httpClient.dispatcher.executorService.shutdownNow() } + runCatching { client.httpClient.connectionPool.evictAll() } + } + private fun readStream( input: InputStream, output: StringBuilder, checkCancelled: (() -> Unit)? ) { - while (true) { - checkCancelled?.invoke() - val b = input.read() - if (b == -1) break - output.append(b.toChar()) + try { + while (true) { + checkCancelled?.invoke() + val b = input.read() + if (b == -1) break + output.append(b.toChar()) + } + } catch (_: IOException) { + // Stream was closed (possibly due to cancellation) } } diff --git a/src/main/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServer.kt b/src/main/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServer.kt index 1c63cb08..a854eb71 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServer.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServer.kt @@ -44,6 +44,10 @@ class RemoteIDEServer(private val devSpacesContext: DevSpacesContext) { @Throws(CancellationException::class) suspend fun getStatus(checkCancelled: (() -> Unit)? = null): RemoteIDEServerStatus = withContext(Dispatchers.IO) { + checkCancelled?.invoke() + if (!DevWorkspacePods(devSpacesContext.client).isPodRunning(pod)) { + return@withContext RemoteIDEServerStatus.empty() + } checkCancelled?.invoke() val output = DevWorkspacePods(devSpacesContext.client).exec( pod = pod, diff --git a/src/main/kotlin/com/redhat/devtools/gateway/view/ui/Dialogs.kt b/src/main/kotlin/com/redhat/devtools/gateway/view/ui/Dialogs.kt index 6b0fe08c..2b757dd9 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/view/ui/Dialogs.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/view/ui/Dialogs.kt @@ -1,6 +1,7 @@ package com.redhat.devtools.gateway.view.ui import com.intellij.openapi.application.ApplicationManager +import com.intellij.openapi.application.ModalityState import com.intellij.openapi.ui.Messages import java.util.concurrent.atomic.AtomicInteger import javax.swing.Icon @@ -32,10 +33,11 @@ object Dialogs { title: String, options: Array, defaultOptionIndex: Int = 0, - icon: Icon? = Messages.getWarningIcon() + icon: Icon? = Messages.getWarningIcon(), + modalityState: ModalityState? = null ): Int { val result = AtomicInteger(-1) - ApplicationManager.getApplication().invokeAndWait { + ApplicationManager.getApplication().invokeAndWait({ result.set( Messages.showDialog( message, @@ -45,7 +47,7 @@ object Dialogs { icon ) ) - } + }, modalityState ?: ModalityState.defaultModalityState()) return result.get() } @@ -64,14 +66,16 @@ object Dialogs { title: String, buttons: Array, confirmOptionIndex: Int, - defaultOptionIndex: Int = 0 + defaultOptionIndex: Int = 0, + modalityState: ModalityState? = null ): Boolean { return showInEdt( message, title, buttons, defaultOptionIndex, - Messages.getWarningIcon() + Messages.getWarningIcon(), + modalityState ) == confirmOptionIndex } @@ -96,14 +100,15 @@ object Dialogs { * * @return true if the user wants to restart the Pod, false if they want to cancel the connection */ - fun ideNotResponding(): Boolean { + fun ideNotResponding(modalityState: ModalityState? = null): Boolean { return confirm( message = "The workspace IDE is not responding properly.\n" + "Would you like to try restarting the Pod or cancel the connection?", title = "Cannot Connect to workspace IDE", buttons = arrayOf("Cancel Connection", "Restart Pod and try again"), confirmOptionIndex = 1, - defaultOptionIndex = 0 + defaultOptionIndex = 0, + modalityState = modalityState ) } diff --git a/src/test/kotlin/com/redhat/devtools/gateway/devworkspace/DevWorkspaceRestartTest.kt b/src/test/kotlin/com/redhat/devtools/gateway/devworkspace/DevWorkspaceRestartTest.kt index e27fe702..b975c97a 100644 --- a/src/test/kotlin/com/redhat/devtools/gateway/devworkspace/DevWorkspaceRestartTest.kt +++ b/src/test/kotlin/com/redhat/devtools/gateway/devworkspace/DevWorkspaceRestartTest.kt @@ -67,7 +67,7 @@ class DevWorkspaceRestartTest { every { pods.list(any(), any()) } returns V1PodList().items(emptyList()) // Default: IDE ready and connection succeed (`just Awaits` never completes — hangs runTest) coJustRun { remoteIDEServer.waitServerReady() } - coEvery { devSpacesConnection.connect(any(), any(), any(), any(), any(), any()) } returns mockk(relaxed = true) + coEvery { devSpacesConnection.connect(any(), any(), any(), any(), any(), any(), any()) } returns mockk(relaxed = true) } @Test @@ -186,7 +186,7 @@ class DevWorkspaceRestartTest { // when restart.restart(thinClient, indicator) // then - coVerify { devSpacesConnection.connect(any(), any(), any(), any(), any(), any()) } + coVerify { devSpacesConnection.connect(any(), any(), any(), any(), any(), any(), any()) } } @Test @@ -196,7 +196,7 @@ class DevWorkspaceRestartTest { // then coVerifyOrder { remoteIDEServer.waitServerReady() - devSpacesConnection.connect(any(), any(), any(), any(), any(), any()) + devSpacesConnection.connect(any(), any(), any(), any(), any(), any(), any()) } } @@ -241,7 +241,7 @@ class DevWorkspaceRestartTest { } coVerifyOrder { remoteIDEServer.waitServerReady() - devSpacesConnection.connect(any(), any(), any(), any(), any(), any()) + devSpacesConnection.connect(any(), any(), any(), any(), any(), any(), any()) } } @@ -297,7 +297,7 @@ class DevWorkspaceRestartTest { fun `#doRestart fails when IDE connection fails`() = runTest { // given val exception = RuntimeException("Connection failed") - coEvery { devSpacesConnection.connect(any(), any(), any(), any(), any(), any()) } throws exception + coEvery { devSpacesConnection.connect(any(), any(), any(), any(), any(), any(), any()) } throws exception // when/then val result = runCatching { restart.restart(thinClient, indicator) } From a41344e4992de982c93913c67a897508dfd9f3f5 Mon Sep 17 00:00:00 2001 From: Andre Dietisheim Date: Tue, 14 Jul 2026 12:12:42 +0200 Subject: [PATCH 2/3] refactor: extracted code to separate methods for better readability Signed-off-by: Andre Dietisheim --- .../gateway/openshift/DevWorkspacePods.kt | 85 ++++++++++++------- 1 file changed, 52 insertions(+), 33 deletions(-) diff --git a/src/main/kotlin/com/redhat/devtools/gateway/openshift/DevWorkspacePods.kt b/src/main/kotlin/com/redhat/devtools/gateway/openshift/DevWorkspacePods.kt index 4bd8a68f..957764dd 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/openshift/DevWorkspacePods.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/openshift/DevWorkspacePods.kt @@ -14,6 +14,7 @@ package com.redhat.devtools.gateway.openshift import com.intellij.openapi.diagnostic.logger import com.redhat.devtools.gateway.util.isCancellationException import io.kubernetes.client.PortForward +import io.kubernetes.client.custom.IOTrio import io.kubernetes.client.openapi.ApiClient import io.kubernetes.client.openapi.ApiException import io.kubernetes.client.openapi.apis.CoreV1Api @@ -85,42 +86,11 @@ class DevWorkspacePods(private val client: ApiClient) { container = container, command = command, onOpen = { io -> - stdoutStream = io.stdout - stderrStream = io.stderr + launchCheckCancelled(checkCancelled, scope, io) stdoutJob = scope.launch { readStream(io.stdout, stdout, checkCancelled) } stderrJob = scope.launch { readStream(io.stderr, stderr, checkCancelled) } - - if (checkCancelled != null) { - scope.launch { - try { - while (isActive) { - checkCancelled.invoke() - delay(200) - } - } catch (_: Throwable) { - runCatching { io.stdout.close() } - runCatching { io.stderr.close() } - } - } - } - - scope.launch { - try { - listOfNotNull(stdoutJob, stderrJob).joinAll() - checkCancelled?.invoke() - closed.await() - - checkCancelled?.invoke() - if (cont.isActive) cont.resume(stdout.toString()) - } catch (e: Throwable) { - if (e.isCancellationException()) cont.cancel(e) - else if (cont.isActive) cont.resumeWithException(e) - } finally { - scope.cancel() - shutdownExecClient(execClientApi) - } - } + launchJoinStdOutStdErr(scope, stdoutJob, stderrJob, checkCancelled, closed, cont, stdout, execClientApi) }, onClosed = { _, _ -> closed.complete(Unit) @@ -150,6 +120,55 @@ class DevWorkspacePods(private val client: ApiClient) { } } + private fun launchJoinStdOutStdErr( + scope: CoroutineScope, + stdoutJob: Job, + stderrJob: Job, + checkCancelled: (() -> Unit)?, + closed: CompletableDeferred, + cont: CancellableContinuation, + stdout: StringBuilder, + execClientApi: ApiClient + ) { + scope.launch { + try { + listOfNotNull(stdoutJob, stderrJob).joinAll() + checkCancelled?.invoke() + closed.await() + + checkCancelled?.invoke() + if (cont.isActive) cont.resume(stdout.toString()) + } catch (e: Throwable) { + if (e.isCancellationException()) cont.cancel(e) + else if (cont.isActive) cont.resumeWithException(e) + } finally { + scope.cancel() + shutdownExecClient(execClientApi) + } + } + } + + private fun launchCheckCancelled( + checkCancelled: (() -> Unit)?, + scope: CoroutineScope, + io: IOTrio + ) { + if (checkCancelled == null) { + return + } + scope.launch { + try { + while (isActive) { + checkCancelled.invoke() + delay(200) + } + } catch (_: Throwable) { + runCatching { io.stdout.close() } + runCatching { io.stderr.close() } + } + } + } + private fun shutdownExecClient(client: ApiClient) { runCatching { client.httpClient.dispatcher.executorService.shutdownNow() } runCatching { client.httpClient.connectionPool.evictAll() } From 80561d2fdfe96ca005e4d61280072730af3462a5 Mon Sep 17 00:00:00 2001 From: Andre Dietisheim Date: Tue, 14 Jul 2026 12:28:21 +0200 Subject: [PATCH 3/3] tests: added tests for code that prevents stuck Signed-off-by: Andre Dietisheim --- .../gateway/openshift/DevWorkspacePodsTest.kt | 338 +++++++++++++++++- .../gateway/server/RemoteIDEServerTest.kt | 43 +++ 2 files changed, 380 insertions(+), 1 deletion(-) diff --git a/src/test/kotlin/com/redhat/devtools/gateway/openshift/DevWorkspacePodsTest.kt b/src/test/kotlin/com/redhat/devtools/gateway/openshift/DevWorkspacePodsTest.kt index 7f48f2b2..f1435d6e 100644 --- a/src/test/kotlin/com/redhat/devtools/gateway/openshift/DevWorkspacePodsTest.kt +++ b/src/test/kotlin/com/redhat/devtools/gateway/openshift/DevWorkspacePodsTest.kt @@ -19,15 +19,22 @@ import io.kubernetes.client.openapi.models.V1ListMeta import io.kubernetes.client.openapi.models.V1ObjectMeta import io.kubernetes.client.openapi.models.V1Pod import io.kubernetes.client.openapi.models.V1PodList +import io.kubernetes.client.openapi.models.V1PodStatus import io.mockk.every import io.mockk.mockk import io.mockk.mockkConstructor +import io.mockk.mockkObject import io.mockk.slot +import io.mockk.unmockkAll import io.mockk.unmockkConstructor +import io.mockk.unmockkObject import io.mockk.verify import kotlinx.coroutines.CancellationException import kotlinx.coroutines.delay +import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking +import okhttp3.ConnectionPool +import okhttp3.Dispatcher import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThatThrownBy import org.junit.jupiter.api.AfterEach @@ -36,8 +43,11 @@ import org.junit.jupiter.api.Test import java.io.ByteArrayInputStream import java.io.ByteArrayOutputStream import java.io.IOException +import java.io.PipedInputStream +import java.io.PipedOutputStream import java.net.ServerSocket import java.net.Socket +import java.util.concurrent.TimeUnit import kotlin.time.Duration.Companion.milliseconds class DevWorkspacePodsTest { @@ -69,7 +79,7 @@ class DevWorkspacePodsTest { @AfterEach fun afterEach() { - unmockkConstructor(PortForward::class) + unmockkAll() } @Test @@ -340,6 +350,173 @@ class DevWorkspacePodsTest { unmockkConstructor(CoreV1Api::class) } + @Test + fun `#isPodRunning returns true when pod phase is Running`() { + // given + val runningPod = V1Pod().apply { + metadata = V1ObjectMeta().apply { + name = "running-pod" + namespace = "test-ns" + } + } + + mockkConstructor(CoreV1Api::class) + val response = mockk(relaxed = true) + every { + anyConstructed().readNamespacedPod("running-pod", "test-ns") + } returns mockk { every { execute() } returns response } + every { response.status } returns V1PodStatus().apply { phase = "Running" } + + // when + val result = pods.isPodRunning(runningPod) + + // then + assertThat(result).isTrue() + unmockkConstructor(CoreV1Api::class) + } + + @Test + fun `#isPodRunning returns false when pod phase is Pending`() { + // given + val pendingPod = V1Pod().apply { + metadata = V1ObjectMeta().apply { + name = "pending-pod" + namespace = "test-ns" + } + } + + mockkConstructor(CoreV1Api::class) + val response = mockk(relaxed = true) + every { + anyConstructed().readNamespacedPod("pending-pod", "test-ns") + } returns mockk { every { execute() } returns response } + every { response.status } returns V1PodStatus().apply { phase = "Pending" } + + // when + val result = pods.isPodRunning(pendingPod) + + // then + assertThat(result).isFalse() + unmockkConstructor(CoreV1Api::class) + } + + @Test + fun `#isPodRunning returns false when pod phase is Failed`() { + // given + val failedPod = V1Pod().apply { + metadata = V1ObjectMeta().apply { + name = "failed-pod" + namespace = "test-ns" + } + } + + mockkConstructor(CoreV1Api::class) + val response = mockk(relaxed = true) + every { + anyConstructed().readNamespacedPod("failed-pod", "test-ns") + } returns mockk { every { execute() } returns response } + every { response.status } returns V1PodStatus().apply { phase = "Failed" } + + // when + val result = pods.isPodRunning(failedPod) + + // then + assertThat(result).isFalse() + unmockkConstructor(CoreV1Api::class) + } + + @Test + fun `#isPodRunning returns false when pod phase is Succeeded`() { + // given + val succeededPod = V1Pod().apply { + metadata = V1ObjectMeta().apply { + name = "succeeded-pod" + namespace = "test-ns" + } + } + + mockkConstructor(CoreV1Api::class) + val response = mockk(relaxed = true) + every { + anyConstructed().readNamespacedPod("succeeded-pod", "test-ns") + } returns mockk { every { execute() } returns response } + every { response.status } returns V1PodStatus().apply { phase = "Succeeded" } + + // when + val result = pods.isPodRunning(succeededPod) + + // then + assertThat(result).isFalse() + unmockkConstructor(CoreV1Api::class) + } + + @Test + fun `#isPodRunning returns false when pod has no name`() { + // given — pod with null name + val noNamePod = V1Pod().apply { + metadata = V1ObjectMeta().apply { + namespace = "test-ns" + } + } + + // when + val result = pods.isPodRunning(noNamePod) + + // then — no API call needed + assertThat(result).isFalse() + } + + @Test + fun `#isPodRunning returns false when pod has no namespace`() { + // given — pod with null namespace + val noNsPod = V1Pod().apply { + metadata = V1ObjectMeta().apply { + name = "my-pod" + } + } + + // when + val result = pods.isPodRunning(noNsPod) + + // then — no API call needed + assertThat(result).isFalse() + } + + @Test + fun `#isPodRunning returns false when pod has no metadata`() { + // given + val noMetaPod = V1Pod() + + // when + val result = pods.isPodRunning(noMetaPod) + + // then + assertThat(result).isFalse() + } + + @Test + fun `#isPodRunning returns false when API call throws exception`() { + // given + val pod = V1Pod().apply { + metadata = V1ObjectMeta().apply { + name = "error-pod" + namespace = "test-ns" + } + } + + mockkConstructor(CoreV1Api::class) + every { + anyConstructed().readNamespacedPod("error-pod", "test-ns") + } returns mockk { every { execute() } throws ApiException("connection refused") } + + // when + val result = pods.isPodRunning(pod) + + // then + assertThat(result).isFalse() + unmockkConstructor(CoreV1Api::class) + } + @Test fun `#waitForPodsDeleted continues polling after API errors`() = runBlocking { // given @@ -375,4 +552,163 @@ class DevWorkspacePodsTest { assertThat(callCount).isGreaterThanOrEqualTo(3) // Had pods, error, then success unmockkConstructor(CoreV1Api::class) } + + @Test + fun `#exec cancels cleanly when checkCancelled throws`() = runBlocking { + // given — ContainerAwareExec returns a fake process that produces data + val stdout = PipedOutputStream() + val stdin = PipedInputStream(stdout) + val fakeProcess = mockk(relaxed = true) + every { fakeProcess.inputStream } returns stdin + every { fakeProcess.errorStream } returns ByteArrayInputStream(ByteArray(0)) + every { fakeProcess.outputStream } returns mockk(relaxed = true) + every { fakeProcess.isAlive } returns true + + mockkConstructor(ContainerAwareExec::class) + val fakeHandle = ContainerAwareExec.ExecHandle( + future = java.util.concurrent.CompletableFuture.completedFuture(0), + job = mockk(relaxed = true) + ) + every { + anyConstructed().containerAwareExec( + any(), any(), any(), any(), any(), any(), any(), any(), any() + ) + } answers { + val onOpen = it.invocation.args[4] as java.util.function.Consumer + val io = io.kubernetes.client.custom.IOTrio() + io.stdout = fakeProcess.inputStream + io.stderr = fakeProcess.errorStream + io.stdin = fakeProcess.outputStream + onOpen.accept(io) + fakeHandle + } + + mockkObject(ApiClientUtils) + val execClient = mockk(relaxed = true) + every { ApiClientUtils.cloneForExec(any()) } returns execClient + + val scope = kotlinx.coroutines.CoroutineScope(kotlinx.coroutines.SupervisorJob()) + val testPod = V1Pod().apply { + metadata = V1ObjectMeta().apply { + name = "test-pod" + namespace = "test-ns" + } + } + + // when — launch exec, then cancel it + val job = scope.launch { + pods.exec( + pod = testPod, + command = arrayOf("echo"), + container = "test-container", + timeout = 60, + checkCancelled = { throw CancellationException("user cancelled") } + ) + } + delay(100) // let exec start + job.cancel() + job.join() + + // then — should complete without hanging + assertThat(job.isCancelled).isTrue() + } + + @Test + fun `#exec propagates error from process`() = runBlocking { + // given — ContainerAwareExec throws on exec + mockkConstructor(ContainerAwareExec::class) + every { + anyConstructed().containerAwareExec( + any(), any(), any(), any(), any(), any(), any(), any(), any() + ) + } throws IOException("connection refused") + + mockkObject(ApiClientUtils) + val execClient = mockk(relaxed = true) + every { ApiClientUtils.cloneForExec(any()) } returns execClient + + val testPod = V1Pod().apply { + metadata = V1ObjectMeta().apply { + name = "test-pod" + namespace = "test-ns" + } + } + + // when / then + assertThatThrownBy { + runBlocking { + pods.exec( + pod = testPod, + command = arrayOf("echo"), + container = "test-container" + ) + } + }.isInstanceOf(IOException::class.java) + + // then — exec client should be shut down + verify { + execClient.httpClient.dispatcher.executorService.shutdownNow() + execClient.httpClient.connectionPool.evictAll() + } + } + + @Test + fun `#exec shuts down exec client on cancellation`() = runBlocking { + // given — ContainerAwareExec returns a blocking fake process + val fakeProcess = mockk(relaxed = true) + every { fakeProcess.inputStream } returns PipedInputStream() + every { fakeProcess.errorStream } returns ByteArrayInputStream(ByteArray(0)) + every { fakeProcess.outputStream } returns mockk(relaxed = true) + every { fakeProcess.isAlive } returns true + + mockkConstructor(ContainerAwareExec::class) + val fakeHandle = ContainerAwareExec.ExecHandle( + future = java.util.concurrent.CompletableFuture.completedFuture(0), + job = mockk(relaxed = true) + ) + every { + anyConstructed().containerAwareExec( + any(), any(), any(), any(), any(), any(), any(), any(), any() + ) + } answers { + val onOpen = it.invocation.args[4] as java.util.function.Consumer + val io = io.kubernetes.client.custom.IOTrio() + io.stdout = fakeProcess.inputStream + io.stderr = fakeProcess.errorStream + io.stdin = fakeProcess.outputStream + onOpen.accept(io) + fakeHandle + } + + mockkObject(ApiClientUtils) + val execClient = mockk(relaxed = true) + every { ApiClientUtils.cloneForExec(any()) } returns execClient + + val scope = kotlinx.coroutines.CoroutineScope(kotlinx.coroutines.SupervisorJob()) + val testPod = V1Pod().apply { + metadata = V1ObjectMeta().apply { + name = "test-pod" + namespace = "test-ns" + } + } + + // when — launch and cancel + val job = scope.launch { + pods.exec( + pod = testPod, + command = arrayOf("echo"), + container = "test-container", + timeout = 60 + ) + } + delay(100) + job.cancel() + job.join() + + // then — exec client dispatcher and connection pool should be shut down + verify { + execClient.httpClient.dispatcher.executorService.shutdownNow() + execClient.httpClient.connectionPool.evictAll() + } + } } \ No newline at end of file diff --git a/src/test/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServerTest.kt b/src/test/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServerTest.kt index 76215d3c..73059c28 100644 --- a/src/test/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServerTest.kt +++ b/src/test/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServerTest.kt @@ -18,6 +18,7 @@ import io.kubernetes.client.openapi.models.V1ObjectMeta import io.kubernetes.client.openapi.models.V1Pod import io.kubernetes.client.openapi.models.V1PodSpec import io.mockk.* +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.runBlocking import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.AfterEach @@ -197,6 +198,48 @@ class RemoteIDEServerTest { ) } + @Test + fun `#getStatus skips exec when pod phase is not Running`() { + // given — isPodRunning returns false for non-Running phases (Pending, Failed, etc.) + coEvery { + anyConstructed().isPodRunning(any()) + } returns false + + // when + val result = runBlocking { + remoteIDEServer.getStatus() + } + + // then — exec should never be called, preventing 60s hang on stuck pod + assertThat(result).isEqualTo(RemoteIDEServerStatus.empty()) + coVerify(exactly = 0) { + anyConstructed().exec(any(), any(), any(), any(), any()) + } + } + + @Test + fun `#getStatus throws CancellationException before checking pod running`() { + // given — checkCancelled throws immediately + coEvery { + anyConstructed().isPodRunning(any()) + } returns true + + // when / then + assertThrows { + runBlocking { + remoteIDEServer.getStatus { throw CancellationException("User cancelled") } + } + } + + // then — neither isPodRunning nor exec should have been called + coVerify(exactly = 0) { + anyConstructed().isPodRunning(any()) + } + coVerify(exactly = 0) { + anyConstructed().exec(any(), any(), any(), any(), any()) + } + } + private fun projectInfo(name: String): ProjectInfo { return ProjectInfo( name,